From a6ab4d38cfdce34307c333d25af6501aef13e2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Wed, 7 Apr 2021 05:33:39 -0700 Subject: [PATCH] [ConfigImpact] Use cost analysis to determine cheapness Summary: Reporting all ungated (un configed?) function calls causes many FPs. Instead, we rely on complexity analysis to determine whether a function is cheap/expensive: if the callee's complexity is not symbolic (e.g. constant), we consider it as cheap and don't keep track of it. Note that we don't take the instantiated/modeled cost into account yet. So, if we have `foo(int n)` with complexity `O(n)`, and call it as `foo(3)`, we would still keep track of it. Similarly, if `foo` is a modeled function with constant time complexity, we would have no summary for it hence would keep track of it. These will be improved later. Reviewed By: skcho Differential Revision: D27430485 fbshipit-source-id: d5f66320d --- infer/src/backend/registerCheckers.ml | 3 +- infer/src/base/Checker.ml | 2 +- .../ConfigImpactAnalysis.ml | 81 +++++++++++-------- .../ConfigImpactAnalysis.mli | 3 +- 4 files changed, 51 insertions(+), 38 deletions(-) rename infer/src/{checkers => cost}/ConfigImpactAnalysis.ml (85%) rename infer/src/{checkers => cost}/ConfigImpactAnalysis.mli (88%) diff --git a/infer/src/backend/registerCheckers.ml b/infer/src/backend/registerCheckers.ml index 9f5adda62..c5ee38aea 100644 --- a/infer/src/backend/registerCheckers.ml +++ b/infer/src/backend/registerCheckers.ml @@ -194,7 +194,8 @@ let all_checkers = ; { checker= ConfigImpactAnalysis ; callbacks= (let checker = - interprocedural Payloads.Fields.config_impact_analysis ConfigImpactAnalysis.checker + interprocedural2 Payloads.Fields.config_impact_analysis Payloads.Fields.cost + ConfigImpactAnalysis.checker in [(checker, Clang); (checker, Java)] ) } ] diff --git a/infer/src/base/Checker.ml b/infer/src/base/Checker.ml index d5b06a309..4224b5e7b 100644 --- a/infer/src/base/Checker.ml +++ b/infer/src/base/Checker.ml @@ -156,7 +156,7 @@ let config_unsafe checker = "[EXPERIMENTAL] Collects function that are called without config checks." ; cli_flags= Some {deprecated= []; show_in_help= true} ; enabled_by_default= false - ; activates= [] } + ; activates= [Cost] } | Cost -> { id= "cost" ; kind= diff --git a/infer/src/checkers/ConfigImpactAnalysis.ml b/infer/src/cost/ConfigImpactAnalysis.ml similarity index 85% rename from infer/src/checkers/ConfigImpactAnalysis.ml rename to infer/src/cost/ConfigImpactAnalysis.ml index 75c9e27a5..1c259118b 100644 --- a/infer/src/checkers/ConfigImpactAnalysis.ml +++ b/infer/src/cost/ConfigImpactAnalysis.ml @@ -250,7 +250,7 @@ module Dom = struct ; config_fields= Fields.widen ~prev:prev.config_fields ~next:next.config_fields ~num_iters } - let to_summary has_call_stmt {unchecked_callees; unchecked_callees_cond; config_fields} = + let to_summary ~has_call_stmt {unchecked_callees; unchecked_callees_cond; config_fields} = {Summary.unchecked_callees; unchecked_callees_cond; has_call_stmt; config_fields} @@ -327,40 +327,51 @@ module Dom = struct let call analyze_dependency callee location ({config_checks; field_checks; unchecked_callees; unchecked_callees_cond} as astate) = if ConfigChecks.is_top config_checks then - let new_unchecked_callees, new_unchecked_callees_cond = - match analyze_dependency callee with - | Some - ( _ - , { Summary.unchecked_callees= callee_summary - ; unchecked_callees_cond= callee_summary_cond - ; has_call_stmt } ) - when has_call_stmt -> - (* If callee's summary is not leaf, use it. *) - ( UncheckedCallees.replace_location_by_call location callee_summary - , UncheckedCalleesCond.replace_location_by_call location callee_summary_cond ) - | _ -> - (* Otherwise, add callee's name. *) - ( UncheckedCallees.singleton {callee; location; call_type= Direct} - , UncheckedCalleesCond.empty ) - in - if FieldChecks.is_top field_checks then - { astate with - unchecked_callees= UncheckedCallees.join unchecked_callees new_unchecked_callees - ; unchecked_callees_cond= - UncheckedCalleesCond.join unchecked_callees_cond new_unchecked_callees_cond } + let callee_summary = analyze_dependency callee in + if + Option.exists callee_summary ~f:(fun (_, (_, (cost_summary : CostDomain.summary option))) -> + Option.exists cost_summary ~f:(fun (cost_summary : CostDomain.summary) -> + let callee_cost = CostDomain.get_operation_cost cost_summary.post in + not (CostDomain.BasicCost.is_symbolic callee_cost.cost) ) ) + then (* If callee is cheap by heuristics, ignore it. *) + astate else - let fields_to_add = FieldChecks.get_fields field_checks in - let unchecked_callees_cond = - UncheckedCalleesCond.weak_update fields_to_add new_unchecked_callees - unchecked_callees_cond - in - let unchecked_callees_cond = - UncheckedCalleesCond.fold - (fun fields callees acc -> - UncheckedCalleesCond.weak_update (Fields.union fields fields_to_add) callees acc ) - new_unchecked_callees_cond unchecked_callees_cond + let new_unchecked_callees, new_unchecked_callees_cond = + match callee_summary with + | Some + ( _ + , ( Some + { Summary.unchecked_callees= callee_summary + ; unchecked_callees_cond= callee_summary_cond + ; has_call_stmt } + , _callee_cost_summary ) ) + when has_call_stmt -> + (* If callee's summary is not leaf, use it. *) + ( UncheckedCallees.replace_location_by_call location callee_summary + , UncheckedCalleesCond.replace_location_by_call location callee_summary_cond ) + | _ -> + (* Otherwise, add callee's name. *) + ( UncheckedCallees.singleton {callee; location; call_type= Direct} + , UncheckedCalleesCond.empty ) in - {astate with unchecked_callees_cond} + if FieldChecks.is_top field_checks then + { astate with + unchecked_callees= UncheckedCallees.join unchecked_callees new_unchecked_callees + ; unchecked_callees_cond= + UncheckedCalleesCond.join unchecked_callees_cond new_unchecked_callees_cond } + else + let fields_to_add = FieldChecks.get_fields field_checks in + let unchecked_callees_cond = + UncheckedCalleesCond.weak_update fields_to_add new_unchecked_callees + unchecked_callees_cond + in + let unchecked_callees_cond = + UncheckedCalleesCond.fold + (fun fields callees acc -> + UncheckedCalleesCond.weak_update (Fields.union fields fields_to_add) callees acc ) + new_unchecked_callees_cond unchecked_callees_cond + in + {astate with unchecked_callees_cond} else astate end @@ -368,7 +379,7 @@ module TransferFunctions = struct module CFG = ProcCfg.Normal module Domain = Dom - type analysis_data = Summary.t InterproceduralAnalysis.t + type analysis_data = (Summary.t option * CostDomain.summary option) InterproceduralAnalysis.t let is_java_boolean_value_method pname = Procname.get_class_name pname |> Option.exists ~f:(String.equal "java.lang.Boolean") @@ -460,4 +471,4 @@ let has_call_stmt proc_desc = let checker ({InterproceduralAnalysis.proc_desc} as analysis_data) = Option.map (Analyzer.compute_post analysis_data ~initial:Dom.init proc_desc) ~f:(fun astate -> let has_call_stmt = has_call_stmt proc_desc in - Dom.to_summary has_call_stmt astate ) + Dom.to_summary ~has_call_stmt astate ) diff --git a/infer/src/checkers/ConfigImpactAnalysis.mli b/infer/src/cost/ConfigImpactAnalysis.mli similarity index 88% rename from infer/src/checkers/ConfigImpactAnalysis.mli rename to infer/src/cost/ConfigImpactAnalysis.mli index f9250a61d..bb5338b02 100644 --- a/infer/src/checkers/ConfigImpactAnalysis.mli +++ b/infer/src/cost/ConfigImpactAnalysis.mli @@ -39,4 +39,5 @@ module Summary : sig val instantiate_unchecked_callees_cond : all_config_fields:Fields.t -> t -> t end -val checker : Summary.t InterproceduralAnalysis.t -> Summary.t option +val checker : + (Summary.t option * CostDomain.summary option) InterproceduralAnalysis.t -> Summary.t option