From 78f65b6dd71b6494cab3ffe5241170f4f9c4c17b Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 28 Aug 2015 11:33:43 -0200 Subject: [PATCH] [eradicate] handle methods overridden in other files Summary: Errors arising from overriding methods defined in other files were not reported, because during parallel analysis the clusters did not have access to overridden methods, so could not load their annotation. Changed cluster generation to add location information for the methods overridden by the procedures defined in the current cluster. --- infer/src/backend/inferanalyze.ml | 30 ++++++++++++++++--- infer/src/checkers/eradicateChecks.ml | 26 +--------------- infer/src/checkers/patternMatch.ml | 29 ++++++++++++++++++ infer/src/checkers/patternMatch.mli | 4 +++ .../InconsistentSubclassAnnotation.java | 6 +++- ...consistentSubclassAnnotationInterface.java | 17 +++++++++++ .../InconsistentSubclassAnnotationTest.java | 2 +- 7 files changed, 83 insertions(+), 31 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotationInterface.java diff --git a/infer/src/backend/inferanalyze.ml b/infer/src/backend/inferanalyze.ml index 87228d0e2..622b1000a 100644 --- a/infer/src/backend/inferanalyze.ml +++ b/infer/src/backend/inferanalyze.ml @@ -394,11 +394,18 @@ let create_minimal_clusters file_cg exe_env to_analyze_map : cluster list = let all_procs, _ = Cg.get_nodes_and_edges cg in let mapr = ref Procname.Map.empty in let do_proc (pn, isdefined) = - if not isdefined then + let extend_map proc_name = try - let source = Exe_env.get_source exe_env pn in - mapr := Procname.Map.add pn source !mapr; + let source = Exe_env.get_source exe_env proc_name in + mapr := Procname.Map.add proc_name source !mapr; with Not_found -> () in + if isdefined then + let tenv = Exe_env.get_tenv exe_env pn in + (* Add the overridden methods, so they can be found by the cluster. *) + PatternMatch.proc_iter_overridden_methods extend_map tenv pn + else + (* Add the procedures that are called but not defined in the current file. *) + extend_map pn in list_iter do_proc all_procs; !mapr in total_files := !total_files + 1; @@ -455,8 +462,23 @@ let clusters_nfiles clusters = list_fold_left (fun n cluster -> cluster_nfiles c let clusters_naprocs clusters = list_fold_left (fun n cluster -> cluster_naprocs cluster + n) 0 clusters let print_clusters_stats clusters = + let pp_source_map_ce fmt cluster_elem = + let do_map_elem proc_name source_file = F.fprintf fmt "%s " (Procname.to_string proc_name) in + Procname.Map.iter do_map_elem cluster_elem.ce_source_map in + let pp_source_map fmt cluster = + list_iter (pp_source_map_ce fmt) cluster in + let pp_cluster num cluster = + L.err "cluster #%d files: %d active procedures: %d source_map: %a@." + num + (cluster_nfiles cluster) + (cluster_naprocs cluster) + pp_source_map cluster in let i = ref 0 in - list_iter (fun cluster -> incr i; L.err "cluster #%d files: %d active procedures: %d@." !i (cluster_nfiles cluster) (cluster_naprocs cluster)) clusters + list_iter + (fun cluster -> + incr i; + pp_cluster !i cluster) + clusters let cluster_split_prefix (cluster : cluster) size = let rec split (cluster_seen : cluster) (cluster_todo : cluster) n = diff --git a/infer/src/checkers/eradicateChecks.ml b/infer/src/checkers/eradicateChecks.ml index 7a1e43426..ccf668502 100644 --- a/infer/src/checkers/eradicateChecks.ml +++ b/infer/src/checkers/eradicateChecks.ml @@ -539,28 +539,4 @@ let check_overridden_annotations check_params overriden_proc_name overriden_signature | None -> () in - let check_overridden_methods super_class_name = - let super_proc_name = Procname.java_replace_class proc_name super_class_name in - let type_name = Sil.TN_csu (Sil.Class, Mangled.from_string super_class_name) in - match Sil.tenv_lookup tenv type_name with - | Some (Sil.Tstruct (_, _, _, _, _, methods, _)) -> - let is_override pname = - Procname.equal pname super_proc_name && - not (Procname.is_constructor pname) in - list_iter - (fun pname -> - if is_override pname - then check pname) - methods - | _ -> () in - - let super_types = - let type_name = - let class_name = Procname.java_get_class proc_name in - Sil.TN_csu (Sil.Class, Mangled.from_string class_name) in - match Sil.tenv_lookup tenv type_name with - | Some curr_type -> - list_map Mangled.to_string (PatternMatch.type_get_direct_supertypes curr_type) - | None -> [] in - - list_iter check_overridden_methods super_types + PatternMatch.proc_iter_overridden_methods check tenv proc_name diff --git a/infer/src/checkers/patternMatch.ml b/infer/src/checkers/patternMatch.ml index 7fd11d77e..381d5faaa 100644 --- a/infer/src/checkers/patternMatch.ml +++ b/infer/src/checkers/patternMatch.ml @@ -320,3 +320,32 @@ let proc_calls get_proc_desc pname pdesc filter : (Procname.t * Cfg.Procdesc.t) let nodes = Cfg.Procdesc.get_nodes pdesc in list_iter do_node nodes; list_rev !res + + +(** Iterate over all the methods overridden by the procedure. + Only Java supported at the moment. *) +let proc_iter_overridden_methods f tenv proc_name = + let do_super_type tenv super_class_name = + let super_proc_name = + Procname.java_replace_class proc_name (Mangled.to_string super_class_name) in + let type_name = Sil.TN_csu (Sil.Class, super_class_name) in + match Sil.tenv_lookup tenv type_name with + | Some (Sil.Tstruct (_, _, _, _, _, methods, _)) -> + let is_override pname = + Procname.equal pname super_proc_name && + not (Procname.is_constructor pname) in + list_iter + (fun pname -> + if is_override pname + then f pname) + methods + | _ -> () in + + if Procname.is_java proc_name then + let type_name = + let class_name = Procname.java_get_class proc_name in + Sil.TN_csu (Sil.Class, Mangled.from_string class_name) in + match Sil.tenv_lookup tenv type_name with + | Some curr_type -> + list_iter (do_super_type tenv) (type_get_direct_supertypes curr_type) + | None -> () diff --git a/infer/src/checkers/patternMatch.mli b/infer/src/checkers/patternMatch.mli index c5e64a99d..8ed45c912 100644 --- a/infer/src/checkers/patternMatch.mli +++ b/infer/src/checkers/patternMatch.mli @@ -53,6 +53,10 @@ val proc_calls : (Procname.t -> Cfg.Procdesc.t option) -> Procname.t -> Cfg.Proc (Procname.t -> Cfg.Procdesc.t -> bool) -> (Procname.t * Cfg.Procdesc.t) list +(** Iterate over all the methods overridden by the procedure. + Only Java supported at the moment. *) +val proc_iter_overridden_methods : (Procname.t -> unit) -> Sil.tenv -> Procname.t -> unit + val type_get_annotation : Sil.typ -> Sil.item_annotation option (** Get the class name of the type *) diff --git a/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java b/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java index 1857c1455..af451a34d 100644 --- a/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java +++ b/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java @@ -96,7 +96,7 @@ class ConstructorsAreExcluded { } } -public class InconsistentSubclassAnnotation { +public class InconsistentSubclassAnnotation implements InconsistentSubclassAnnotationInterface { public static void callFromSuperclass(SubclassExample.A a) { SubclassExample.T t = a.foo(); @@ -107,4 +107,8 @@ public class InconsistentSubclassAnnotation { a.deref(t); } + public String implementInAnotherFile(String s) { + return ""; + } + } diff --git a/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotationInterface.java b/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotationInterface.java new file mode 100644 index 000000000..f0837fe88 --- /dev/null +++ b/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotationInterface.java @@ -0,0 +1,17 @@ +/* +* Copyright (c) 2015 - present Facebook, Inc. +* All rights reserved. +* +* This source code is licensed under the BSD style license found in the +* LICENSE file in the root directory of this source tree. An additional grant +* of patent rights can be found in the PATENTS file in the same directory. +*/ + +package codetoanalyze.java.eradicate; + +import javax.annotation.Nullable; + +public interface InconsistentSubclassAnnotationInterface { + public String implementInAnotherFile(@Nullable String s); + +} diff --git a/infer/tests/endtoend/java/eradicate/InconsistentSubclassAnnotationTest.java b/infer/tests/endtoend/java/eradicate/InconsistentSubclassAnnotationTest.java index 43e98a219..b164ec2f1 100644 --- a/infer/tests/endtoend/java/eradicate/InconsistentSubclassAnnotationTest.java +++ b/infer/tests/endtoend/java/eradicate/InconsistentSubclassAnnotationTest.java @@ -55,7 +55,7 @@ public class InconsistentSubclassAnnotationTest { SOURCE_FILE, returnMethods); - String[] parameterMethods = {"deref"}; + String[] parameterMethods = {"deref", "implementInAnotherFile"}; errorPatterns.addAll( createPatterns( ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION,