[hoisting] Add new issue type for only invariant-modeled calls

Reviewed By: ngorogiannis

Differential Revision: D13194812

fbshipit-source-id: 0c020789d

infer/src/base/IssueType.ml

@@ -303,6 +303,8 @@ let lock_consistency_violation = from_string "LOCK_CONSISTENCY_VIOLATION"
 
 let logging_private_data = from_string "LOGGING_PRIVATE_DATA"
 
+let loop_invariant_call = from_string "LOOP_INVARIANT_CALL"
+
 let memory_leak = from_string "MEMORY_LEAK"
 
 let missing_fld = from_string "Missing_fld" ~hum:"Missing Field"

infer/src/base/IssueType.mli

@@ -211,6 +211,8 @@ val lock_consistency_violation : t
 
 val logging_private_data : t
 
+val loop_invariant_call : t
+
 val memory_leak : t
 
 val missing_fld : t

infer/src/checkers/hoisting.ml

@@ -64,7 +64,7 @@ let get_hoist_inv_map tenv reaching_defs_invariant_map loop_head_to_source_nodes
     loop_head_to_source_nodes LoopHeadToHoistInstrs.empty
 
 
-let do_report summary Call.({pname; loc}) loop_head_loc =
+let do_report summary Call.({pname; loc}) ~issue loop_head_loc =
   let exp_desc =
     F.asprintf "Loop-invariant call to %a at %a" Typ.Procname.pp pname Location.pp loc
   in
@@ -72,35 +72,40 @@ let do_report summary Call.({pname; loc}) loop_head_loc =
   let message =
     F.asprintf "%s can be moved out of the loop at %a." exp_desc Location.pp loop_head_loc
   in
-  Reporting.log_error summary ~loc ~ltr IssueType.invariant_call message
+  Reporting.log_error summary ~loc ~ltr issue message
 
 
-let should_report tenv proc_desc Call.({pname; node; params}) integer_type_widths
+let model_satisfies ~f tenv pname = InvariantModels.Call.dispatch tenv pname [] |> Option.exists ~f
+
+let get_issue_to_report tenv proc_desc Call.({pname; node; params}) integer_type_widths
     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 ->
+    model_satisfies ~f:InvariantModels.is_variant_for_hoisting tenv pname
+  in
+  let report_invariant =
+    ((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
+    | Some {Summary.payloads= {Payloads.cost= Some {CostDomain.post= cost}}}
+      when CostDomain.BasicCost.is_symbolic cost ->
+        let instr_node_id = InstrCFG.last_of_underlying_node node |> InstrCFG.Node.id in
+        let inferbo_invariant_map = Lazy.force inferbo_invariant_map in
+        let inferbo_mem = BufferOverrunChecker.extract_pre instr_node_id inferbo_invariant_map in
+        (* get the cost of the function call *)
+        Cost.instantiate_cost integer_type_widths ~caller_pdesc:proc_desc
+          ~inferbo_caller_mem:inferbo_mem ~callee_pname:pname ~params ~callee_cost:cost
+        |> CostDomain.BasicCost.is_symbolic
+    | _ ->
         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
-  | Some {Summary.payloads= {Payloads.cost= Some {CostDomain.post= cost}}}
-    when CostDomain.BasicCost.is_symbolic cost ->
-      let instr_node_id = InstrCFG.last_of_underlying_node node |> InstrCFG.Node.id in
-      let inferbo_invariant_map = Lazy.force inferbo_invariant_map in
-      let inferbo_mem = BufferOverrunChecker.extract_pre instr_node_id inferbo_invariant_map in
-      (* get the cost of the function call *)
-      Cost.instantiate_cost integer_type_widths ~caller_pdesc:proc_desc
-        ~inferbo_caller_mem:inferbo_mem ~callee_pname:pname ~params ~callee_cost:cost
-      |> CostDomain.BasicCost.is_symbolic
-  | _ ->
-      false
+  if report_invariant then
+    if model_satisfies ~f:InvariantModels.is_invariant tenv pname then
+      Some IssueType.loop_invariant_call
+    else Some IssueType.invariant_call
+  else None
 
 
 let checker ({Callbacks.tenv; summary; proc_desc; integer_type_widths} as callback_args) :
@@ -131,8 +136,8 @@ let checker ({Callbacks.tenv; summary; proc_desc; integer_type_widths} as callba
       let loop_head_loc = Procdesc.Node.get_loc loop_head in
       HoistCalls.iter
         (fun call ->
-          if should_report tenv proc_desc call integer_type_widths inferbo_invariant_map then
-            do_report summary call loop_head_loc )
+          get_issue_to_report tenv proc_desc call integer_type_widths inferbo_invariant_map
+          |> Option.iter ~f:(fun issue -> do_report summary call ~issue loop_head_loc) )
         inv_instrs )
     loop_head_to_inv_instrs ;
   summary

infer/tests/codetoanalyze/java/hoisting/issues.exp

@@ -19,8 +19,8 @@ codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.irvar_independent_
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.this_modification_outside_hoist(int):int, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.get() at line 127]
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.unmodified_arg_hoist(int[][],int,int[]):void, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.regionFirst(int[]) at line 222]
 codetoanalyze/java/hoisting/HoistInvalidate.java, HoistInvalidate.loop_indirect_hoist(java.util.ArrayList,int,int[]):void, 3, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistInvalidate.get_length(int[]) at line 52]
-codetoanalyze/java/hoisting/HoistModeled.java, HoistModeled.deserialize_hoist(com.fasterxml.jackson.databind.JsonDeserializer,com.fasterxml.jackson.core.JsonParser,com.fasterxml.jackson.databind.DeserializationContext):void, 8, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to Object JsonDeserializer.deserialize(JsonParser,DeserializationContext) at line 33]
-codetoanalyze/java/hoisting/HoistModeled.java, HoistModeled.list_contains_hoist(java.util.List,java.lang.String):int, 3, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to String String.substring(int,int) at line 18]
-codetoanalyze/java/hoisting/HoistModeled.java, HoistModeled.list_contains_hoist(java.util.List,java.lang.String):int, 3, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to boolean List.contains(Object) at line 18]
+codetoanalyze/java/hoisting/HoistModeled.java, HoistModeled.deserialize_hoist(com.fasterxml.jackson.databind.JsonDeserializer,com.fasterxml.jackson.core.JsonParser,com.fasterxml.jackson.databind.DeserializationContext):void, 8, LOOP_INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to Object JsonDeserializer.deserialize(JsonParser,DeserializationContext) at line 33]
+codetoanalyze/java/hoisting/HoistModeled.java, HoistModeled.list_contains_hoist(java.util.List,java.lang.String):int, 3, LOOP_INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to String String.substring(int,int) at line 18]
+codetoanalyze/java/hoisting/HoistModeled.java, HoistModeled.list_contains_hoist(java.util.List,java.lang.String):int, 3, LOOP_INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to boolean List.contains(Object) at line 18]
 codetoanalyze/java/hoisting/HoistNoIndirectMod.java, HoistNoIndirectMod.increment_dont_hoist_FP(int):int, 2, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistNoIndirectMod.calcNext() at line 27]
 codetoanalyze/java/hoisting/HoistNoIndirectMod.java, HoistNoIndirectMod.modify_and_increment_dont_hoist_FP(int):int, 3, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistNoIndirectMod.calcNext() at line 35]