diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 900ec497f..80048da33 100644 --- a/infer/src/base/IssueType.ml +++ b/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" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index ecdbc37d7..d52764fe2 100644 --- a/infer/src/base/IssueType.mli +++ b/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 diff --git a/infer/src/checkers/hoisting.ml b/infer/src/checkers/hoisting.ml index b0c9753c0..aa91465e2 100644 --- a/infer/src/checkers/hoisting.ml +++ b/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 diff --git a/infer/tests/codetoanalyze/java/hoisting/issues.exp b/infer/tests/codetoanalyze/java/hoisting/issues.exp index 08bfbb8c8..2d2c8af56 100644 --- a/infer/tests/codetoanalyze/java/hoisting/issues.exp +++ b/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]