From 6e3b02eaee63c14b0e01843ac0fbd28a89e9693c Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Tue, 20 Apr 2021 02:52:34 -0700 Subject: [PATCH] [ConfigImpact] Filter known expensive callees when cost is constant Summary: This diff filters known expensive callees when cost is constant. previous: ``` foo() { known_expensive_call(); } ``` current: ``` foo() { known_expensive_call(); goo(); } // cost is constant goo() { known_expensive_call(); unknown_call(); } ``` When callee's cost is constant and its summary includes known expensive callees, the checker addressed it as a non-constant-cost callee, i.e., it copies all ungated callees from the callee's summary. However, sometimes this full-copying introduces unexpected issues. For example, suppose a callee `goo` is added and `goo`'s cost is constant as above. Since it includes `known_expensive_call`, all ungated callees of its summary is copied to the caller `foo`'s summary: * `foo`'s ungated callees (before): {`known_expensive_call`} * `foo`'s ungated callees (after): {`known_expensive_call`, `unknown_call`} As a result, it would report about `unknown_call` is added. However, this is not what we intended: In the example, `unknown_call` is reported because it is called in the same function with `known_expensive_call`, not because it is expensive. To fix that issue, this diff filters known expensive callees from `goo`'s summary in that case. Reviewed By: ezgicicek Differential Revision: D27852552 fbshipit-source-id: d207eef1c --- infer/src/cost/ConfigImpactAnalysis.ml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/infer/src/cost/ConfigImpactAnalysis.ml b/infer/src/cost/ConfigImpactAnalysis.ml index 578b87873..88b863934 100644 --- a/infer/src/cost/ConfigImpactAnalysis.ml +++ b/infer/src/cost/ConfigImpactAnalysis.ml @@ -116,6 +116,8 @@ module UncheckedCallees = struct let pp_without_location f x = UncheckedCallee.pp_without_location_list f (elements x) let has_known_expensive_callee x = exists (fun {is_known_expensive} -> is_known_expensive) x + + let filter_known_expensive x = filter (fun {is_known_expensive} -> is_known_expensive) x end module UncheckedCalleesCond = struct @@ -129,6 +131,15 @@ module UncheckedCalleesCond = struct let replace_location_by_call location fields_map = map (UncheckedCallees.replace_location_by_call location) fields_map + + + let filter_known_expensive fields_map = + fold + (fun fields callees acc -> + let expensive_callees = UncheckedCallees.filter_known_expensive callees in + if UncheckedCallees.is_empty expensive_callees then acc + else add fields expensive_callees acc ) + fields_map empty end module Loc = struct @@ -423,6 +434,15 @@ module Dom = struct ; unchecked_callees_cond= callee_summary_cond ; has_call_stmt } when has_call_stmt -> + let callee_summary, callee_summary_cond = + if is_cheap_call then + (* In this case, the callee is cheap by the heuristics, but its summary includes + some known expensive callees. Thus, it filters the known expensive callees + only. *) + ( UncheckedCallees.filter_known_expensive callee_summary + , UncheckedCalleesCond.filter_known_expensive callee_summary_cond ) + else (callee_summary, callee_summary_cond) + in (* If callee's summary is not leaf, use it. *) join_unchecked_callees (UncheckedCallees.replace_location_by_call location callee_summary)