From f3d82a02308e115955f767e8282b53f701ebacb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Wed, 14 Nov 2018 02:40:02 -0800 Subject: [PATCH] [hoisting] Don't report functions modeled as VariantForHoisting but consider them invariant Reviewed By: ddino Differential Revision: D13025250 fbshipit-source-id: ba1e39591 --- infer/src/checkers/hoisting.ml | 15 +++++++-- infer/src/checkers/invariantModels.ml | 33 +++++++++++-------- .../codetoanalyze/java/hoisting/issues.exp | 2 ++ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/infer/src/checkers/hoisting.ml b/infer/src/checkers/hoisting.ml index 85edafab7..5c92793fa 100644 --- a/infer/src/checkers/hoisting.ml +++ b/infer/src/checkers/hoisting.ml @@ -75,8 +75,17 @@ let do_report summary Call.({pname; loc}) loop_head_loc = Reporting.log_error summary ~loc ~ltr IssueType.invariant_call message -let should_report proc_desc Call.({pname; node; params}) inferbo_invariant_map = - (not Config.hoisting_report_only_expensive) +let should_report tenv proc_desc Call.({pname; node; params}) inferbo_invariant_map = + (* If a function is modeled as variant for hoisting (like + List.size or __cast ), we don't want to report it *) + let is_variant_for_hoisting = + match InvariantModels.Call.dispatch tenv pname [] with + | Some inv -> + InvariantModels.is_variant_for_hoisting inv + | None -> + false + in + ((not is_variant_for_hoisting) && not Config.hoisting_report_only_expensive) || (* only report if function call has expensive/symbolic cost *) match Ondemand.analyze_proc_name pname with @@ -120,7 +129,7 @@ let checker ({Callbacks.tenv; summary; proc_desc} as callback_args) : Summary.t let loop_head_loc = Procdesc.Node.get_loc loop_head in HoistCalls.iter (fun call -> - if should_report proc_desc call inferbo_invariant_map then + if should_report tenv proc_desc call inferbo_invariant_map then do_report summary call loop_head_loc ) inv_instrs ) loop_head_to_inv_instrs ; diff --git a/infer/src/checkers/invariantModels.ml b/infer/src/checkers/invariantModels.ml index 318ed6e5f..526cb2561 100644 --- a/infer/src/checkers/invariantModels.ml +++ b/infer/src/checkers/invariantModels.ml @@ -6,27 +6,32 @@ *) open! IStd +module BuiltinInvariantSet = Caml.Set.Make (String) type model = Invariant | Variant | VariantForHoisting -(* Even though functions marked with VariantForHoisting are pure, we - don't want to report them in hoisting. Hence, we model them as - Variant. *) -let is_invariant = function - | Invariant -> - true - | VariantForHoisting -> - not Config.loop_hoisting - | Variant -> - false +let is_invariant = function Invariant -> true | VariantForHoisting -> true | Variant -> false +let is_variant_for_hoisting = function VariantForHoisting -> true | _ -> false + +let invariants = + BuiltinInvariantSet.of_list + [ "__instanceof" + ; "__cast" + ; "__get_array_length" + ; "__get_type_of" + ; "__infer_assume" + ; "__infer_skip" + ; "__infer_fail" ] + + +let invariant_builtins _ s = BuiltinInvariantSet.mem s invariants module Call = struct let dispatch : (Tenv.t, model) ProcnameDispatcher.Call.dispatcher = let open ProcnameDispatcher.Call in make_dispatcher - [ -"__cast" <>--> VariantForHoisting - ; +PatternMatch.implements_collection &:: "iterator" <>--> Variant + [ +PatternMatch.implements_collection &:: "iterator" <>--> Variant ; +PatternMatch.implements_iterator &:: "hasNext" <>--> Variant ; +PatternMatch.implements_enumeration &:: "hasMoreElements" <>--> Variant ; +PatternMatch.implements_enumeration &:: "nextElement" <>--> Variant @@ -63,7 +68,7 @@ module Call = struct ; +PatternMatch.implements_io "Reader" &:: "read" <>--> Variant ; +PatternMatch.implements_io "BufferedReader" &:: "readLine" <>--> Variant ; +PatternMatch.implements_inject "Provider" &:: "get" <>--> Invariant - ; -"__new" <>--> Variant + ; +invariant_builtins <>--> VariantForHoisting ; +(fun _ name -> BuiltinDecl.is_declared (Typ.Procname.from_string_c_fun name)) - <>--> VariantForHoisting ] + <>--> Variant ] end diff --git a/infer/tests/codetoanalyze/java/hoisting/issues.exp b/infer/tests/codetoanalyze/java/hoisting/issues.exp index 05d9fb530..c4a781ee4 100644 --- a/infer/tests/codetoanalyze/java/hoisting/issues.exp +++ b/infer/tests/codetoanalyze/java/hoisting/issues.exp @@ -6,6 +6,8 @@ codetoanalyze/java/hoisting/Hoist.java, Hoist.clash_function_calls_hoist(int):vo codetoanalyze/java/hoisting/Hoist.java, Hoist.dep_not_invariant_dont_hoist(int,int[]):void, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function void Hoist.dep_not_invariant_dont_hoist(int,int[])] codetoanalyze/java/hoisting/Hoist.java, Hoist.dumb_foo():void, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function void Hoist.dumb_foo()] codetoanalyze/java/hoisting/Hoist.java, Hoist.foo(int,int):int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int Hoist.foo(int,int)] +codetoanalyze/java/hoisting/Hoist.java, Hoist.get_array_length_dont_hoist(int[]):void, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function void Hoist.get_array_length_dont_hoist(int[])] +codetoanalyze/java/hoisting/Hoist.java, Hoist.instanceof_dont_hoist(Hoist$EmptyFoo):boolean, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function boolean Hoist.instanceof_dont_hoist(Hoist$EmptyFoo)] codetoanalyze/java/hoisting/Hoist.java, Hoist.legit_hoist(int,int[]):void, 5, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int Hoist.foo(int,int) at line 73] codetoanalyze/java/hoisting/Hoist.java, Hoist.loop_guard_hoist(int,int[]):void, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function void Hoist.loop_guard_hoist(int,int[])] codetoanalyze/java/hoisting/Hoist.java, Hoist.loop_guard_hoist(int,int[]):void, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int Hoist.foo(int,int) at line 65]