From 2f7e4563c671ecb2973a29a9c8369ba18621712e Mon Sep 17 00:00:00 2001 From: Jeremy Dubreil Date: Thu, 27 Sep 2018 16:53:34 -0700 Subject: [PATCH] [eradicate] also lookup the procedure attributes in the super class Summary: Before this diff, the analysis would only lookup the attributes with the classname appearing in the instruction. However, it would fail to find those attributes for inherited and not overridden methods. With this diff, the attributes are now searched recursively in the super classes. Reviewed By: mbouaziz Differential Revision: D10007469 fbshipit-source-id: 77d721cba --- infer/src/absint/PatternMatch.ml | 14 ++++++++++++++ infer/src/absint/PatternMatch.mli | 2 ++ infer/src/eradicate/eradicate.ml | 6 ++++-- infer/src/eradicate/eradicateChecks.ml | 2 +- infer/src/eradicate/typeCheck.ml | 16 ++++++++-------- .../java/eradicate/ParameterNotNullable.java | 2 +- .../codetoanalyze/java/eradicate/issues.exp | 1 - 7 files changed, 30 insertions(+), 13 deletions(-) diff --git a/infer/src/absint/PatternMatch.ml b/infer/src/absint/PatternMatch.ml index 6d552096e..61ebc5aa0 100644 --- a/infer/src/absint/PatternMatch.ml +++ b/infer/src/absint/PatternMatch.ml @@ -353,6 +353,20 @@ let override_iter f tenv proc_name = ignore (override_exists (fun pname -> f pname ; false) tenv proc_name) +let lookup_attributes tenv proc_name = + let found_attributes = ref None in + let f pname = + match Attributes.load pname with + | None -> + false + | Some _ as attributes -> + found_attributes := attributes ; + true + in + ignore (override_find ~check_current_type:true f tenv proc_name) ; + !found_attributes + + (** return the set of instance fields that are assigned to a null literal in [procdesc] *) let get_fields_nullified procdesc = (* walk through the instructions and look for instance fields that are assigned to null *) diff --git a/infer/src/absint/PatternMatch.mli b/infer/src/absint/PatternMatch.mli index fa2d1bc33..b17239818 100644 --- a/infer/src/absint/PatternMatch.mli +++ b/infer/src/absint/PatternMatch.mli @@ -91,6 +91,8 @@ val override_iter : (Typ.Procname.t -> unit) -> Tenv.t -> Typ.Procname.t -> unit (** Apply the given predicate to procname and each override of [procname]. For the moment, this only works for Java *) +val lookup_attributes : Tenv.t -> Typ.Procname.t -> ProcAttributes.t option + val type_get_annotation : Tenv.t -> Typ.t -> Annot.Item.t option val type_get_class_name : Typ.t -> Typ.Name.t option diff --git a/infer/src/eradicate/eradicate.ml b/infer/src/eradicate/eradicate.ml index 2c07478d9..491e68ecb 100644 --- a/infer/src/eradicate/eradicate.ml +++ b/infer/src/eradicate/eradicate.ml @@ -168,7 +168,9 @@ module MkCallback (Extension : ExtensionT) : CallBackT = struct in is_private && same_class in - let private_called = PatternMatch.proc_calls Attributes.load init_pd filter in + let private_called = + PatternMatch.proc_calls (PatternMatch.lookup_attributes tenv) init_pd filter + in let do_called (callee_pn, _) = match Ondemand.get_proc_desc callee_pn with | Some callee_pd -> @@ -216,7 +218,7 @@ module MkCallback (Extension : ExtensionT) : CallBackT = struct let pname_and_pdescs_with f = let res = ref [] in let filter pname = - match Attributes.load pname with + match PatternMatch.lookup_attributes tenv pname with | Some proc_attributes -> f (pname, proc_attributes) | None -> diff --git a/infer/src/eradicate/eradicateChecks.ml b/infer/src/eradicate/eradicateChecks.ml index f1156494b..a11702160 100644 --- a/infer/src/eradicate/eradicateChecks.ml +++ b/infer/src/eradicate/eradicateChecks.ml @@ -478,7 +478,7 @@ let check_overridden_annotations find_canonical_duplicate tenv proc_name proc_de ignore (List.fold2_exn ~f:compare ~init:initial_pos current_params overridden_params) in let check overriden_proc_name = - match Attributes.load overriden_proc_name with + match PatternMatch.lookup_attributes tenv overriden_proc_name with | Some attributes -> ( let overridden_signature = Models.get_modelled_annotated_signature attributes in check_params overriden_proc_name overridden_signature ; diff --git a/infer/src/eradicate/typeCheck.ml b/infer/src/eradicate/typeCheck.ml index 700406e8f..1a8d1b25e 100644 --- a/infer/src/eradicate/typeCheck.ml +++ b/infer/src/eradicate/typeCheck.ml @@ -18,8 +18,8 @@ module ComplexExpressions = struct let procname_instanceof = Typ.Procname.equal BuiltinDecl.__instanceof - let procname_is_false_on_null pn = - match Attributes.load pn with + let procname_is_false_on_null tenv pn = + match PatternMatch.lookup_attributes tenv pn with | Some proc_attributes -> let annotated_signature = Models.get_modelled_annotated_signature proc_attributes in let ret_ann, _ = annotated_signature.AnnotatedSignature.ret in @@ -28,9 +28,9 @@ module ComplexExpressions = struct false - let procname_is_true_on_null pn = + let procname_is_true_on_null tenv pn = let annotated_true_on_null () = - match Attributes.load pn with + match PatternMatch.lookup_attributes tenv pn with | Some proc_attributes -> let annotated_signature = Models.get_modelled_annotated_signature proc_attributes in let ret_ann, _ = annotated_signature.AnnotatedSignature.ret in @@ -493,7 +493,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p , loc , cflags ) -> let callee_attributes = - match Attributes.load callee_pname with + match PatternMatch.lookup_attributes tenv callee_pname with | Some proc_attributes -> proc_attributes | None -> @@ -838,11 +838,11 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p in (* check if the expression is coming from a procedure returning false on null *) let from_is_false_on_null e : Exp.t option = - from_call ComplexExpressions.procname_is_false_on_null e + from_call (ComplexExpressions.procname_is_false_on_null tenv) e in (* check if the expression is coming from a procedure returning true on null *) let from_is_true_on_null e : Exp.t option = - from_call ComplexExpressions.procname_is_true_on_null e + from_call (ComplexExpressions.procname_is_true_on_null tenv) e in (* check if the expression is coming from Map.containsKey *) let from_containsKey e : Exp.t option = @@ -1037,7 +1037,7 @@ let typecheck_node tenv calls_this checks idenv curr_pname curr_pdesc find_canon when Models.is_noreturn callee_pname -> noreturn := true | Sil.Call (_, Exp.Const (Const.Cfun callee_pname), _, _, _) -> - let callee_attributes_opt = Attributes.load callee_pname in + let callee_attributes_opt = PatternMatch.lookup_attributes tenv callee_pname in (* check if the call might throw an exception *) let has_exceptions = match callee_attributes_opt with diff --git a/infer/tests/codetoanalyze/java/eradicate/ParameterNotNullable.java b/infer/tests/codetoanalyze/java/eradicate/ParameterNotNullable.java index 413614bbd..b058b50e4 100644 --- a/infer/tests/codetoanalyze/java/eradicate/ParameterNotNullable.java +++ b/infer/tests/codetoanalyze/java/eradicate/ParameterNotNullable.java @@ -96,7 +96,7 @@ public class ParameterNotNullable { } } - void indirectSignatureLookup_FP(SomeClass c) { + void indirectSignatureLookupOk(SomeClass c) { c.acceptsNullableParameter(null); } } diff --git a/infer/tests/codetoanalyze/java/eradicate/issues.exp b/infer/tests/codetoanalyze/java/eradicate/issues.exp index 7db68d31c..b516e53a4 100644 --- a/infer/tests/codetoanalyze/java/eradicate/issues.exp +++ b/infer/tests/codetoanalyze/java/eradicate/issues.exp @@ -66,7 +66,6 @@ codetoanalyze/java/eradicate/NullMethodCall.java, codetoanalyze.java.eradicate.N codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable$ConstructorCall.(codetoanalyze.java.eradicate.ParameterNotNullable,int), 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [origin,`ParameterNotNullable$ConstructorCall(...)` needs a non-null value in parameter 2 but argument `null` can be null. (Origin: null constant at line 95)] codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.callNull():void, 2, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [origin,`ParameterNotNullable.test(...)` needs a non-null value in parameter 1 but argument `s` can be null. (Origin: null constant at line 33)] codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.callNullable(java.lang.String):void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ParameterNotNullable.test(...)` needs a non-null value in parameter 1 but argument `s` can be null. (Origin: method parameter s)] -codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.indirectSignatureLookup_FP(codetoanalyze.java.eradicate.SomeClass):void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [origin,`SomeClass.acceptsNullableParameter(...)` needs a non-null value in parameter 1 but argument `null` can be null. (Origin: null constant at line 100)] codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.testClassGetResourceArgument(java.lang.Class):java.net.URL, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [origin,`Class.getResource(...)` needs a non-null value in parameter 1 but argument `null` can be null. (Origin: null constant at line 75)] codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.testSystemGetPropertyArgument():java.lang.String, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [origin,`System.getProperty(...)` needs a non-null value in parameter 1 but argument `null` can be null. (Origin: null constant at line 65)] codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.testSystemGetenvBad():java.lang.String, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [origin,`System.getenv(...)` needs a non-null value in parameter 1 but argument `null` can be null. (Origin: null constant at line 71)]