From 2bdc4e557346a3dd674dfbcc86c24ecd02dae6ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Tue, 13 Apr 2021 01:44:36 -0700 Subject: [PATCH] [ConfigImpact] Take cost insantiation and models into account Summary: In D27430485 (https://github.com/facebook/infer/commit/a6ab4d38cfdce34307c333d25af6501aef13e2f2), we used the static cost of the callee to determine whether it was cheap/expensive. This diff improves on that by taking the whole instantiated cost of the function call (not just the callee's cost). Also, if the callee is an unmodeled call, we consider it to be expensive as before. Note: cost instantiation was used by hoisting. I refactored bunch of code there to reuse as much as code possible. Reviewed By: skcho Differential Revision: D27649302 fbshipit-source-id: 07d11f3dd --- infer/src/backend/registerCheckers.ml | 6 +- infer/src/cost/ConfigImpactAnalysis.ml | 36 ++--- infer/src/cost/ConfigImpactAnalysis.mli | 4 +- infer/src/cost/cost.ml | 2 - infer/src/cost/costInstantiate.ml | 124 ++++++++++++++++++ infer/src/cost/costInstantiate.mli | 28 ++++ infer/src/cost/hoisting.ml | 80 +---------- .../objc/fb-config-impact/Makefile | 2 +- 8 files changed, 185 insertions(+), 97 deletions(-) create mode 100644 infer/src/cost/costInstantiate.ml create mode 100644 infer/src/cost/costInstantiate.mli diff --git a/infer/src/backend/registerCheckers.ml b/infer/src/backend/registerCheckers.ml index c5ee38aea..25140f7fa 100644 --- a/infer/src/backend/registerCheckers.ml +++ b/infer/src/backend/registerCheckers.ml @@ -194,8 +194,10 @@ let all_checkers = ; { checker= ConfigImpactAnalysis ; callbacks= (let checker = - interprocedural2 Payloads.Fields.config_impact_analysis Payloads.Fields.cost - ConfigImpactAnalysis.checker + interprocedural3 + ~set_payload:(Field.fset Payloads.Fields.config_impact_analysis) + Payloads.Fields.buffer_overrun_analysis Payloads.Fields.config_impact_analysis + Payloads.Fields.cost ConfigImpactAnalysis.checker in [(checker, Clang); (checker, Java)] ) } ] diff --git a/infer/src/cost/ConfigImpactAnalysis.ml b/infer/src/cost/ConfigImpactAnalysis.ml index 4cc780e5a..b02c7b875 100644 --- a/infer/src/cost/ConfigImpactAnalysis.ml +++ b/infer/src/cost/ConfigImpactAnalysis.ml @@ -366,26 +366,22 @@ module Dom = struct dispatch tenv pname args |> Option.is_some - let call tenv analyze_dependency callee args location + let call tenv analyze_dependency ~is_cheap_call callee args location ({config_checks; field_checks; unchecked_callees; unchecked_callees_cond} as astate) = if ConfigChecks.is_top config_checks then - let (callee_summary : Summary.t option), (cost_summary : CostDomain.summary option) = + let (callee_summary : Summary.t option) = match analyze_dependency callee with | None -> - (None, None) - | Some (_, analysis_data) -> + None + | Some (_, (_, analysis_data, _)) -> analysis_data in let is_expensive = is_known_expensive_method tenv callee args in let has_expensive_callee = Option.exists callee_summary ~f:Summary.has_known_expensive_callee in - if - (not is_expensive) && (not has_expensive_callee) - && 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. *) + if is_cheap_call && (not is_expensive) && not has_expensive_callee then + (* If callee is cheap by heuristics, ignore it. *) astate else let new_unchecked_callees, new_unchecked_callees_cond = @@ -430,11 +426,17 @@ module Dom = struct else astate end +type analysis_data = + { interproc: + (BufferOverrunAnalysisSummary.t option * Summary.t option * CostDomain.summary option) + InterproceduralAnalysis.t + ; get_is_cheap_call: CostInstantiate.Call.t -> bool } + module TransferFunctions = struct module CFG = ProcCfg.Normal module Domain = Dom - type analysis_data = (Summary.t option * CostDomain.summary option) InterproceduralAnalysis.t + type nonrec analysis_data = analysis_data let is_java_boolean_value_method pname = Procname.get_class_name pname |> Option.exists ~f:(String.equal "java.lang.Boolean") @@ -480,7 +482,7 @@ module TransferFunctions = struct fun tenv pname -> dispatch tenv pname |> Option.is_some - let exec_instr astate {InterproceduralAnalysis.tenv; analyze_dependency} _node instr = + let exec_instr astate {interproc= {tenv; analyze_dependency}; get_is_cheap_call} node instr = match (instr : Sil.instr) with | Load {id; e= Lvar pvar} -> Dom.load_config id pvar astate @@ -495,15 +497,17 @@ module TransferFunctions = struct Dom.boolean_value ret id astate | Call (_, Const (Cfun callee), _, _, _) when is_known_cheap_method tenv callee -> astate - | Call ((ret, _), Const (Cfun callee), args, location, _) -> ( + | Call (((ret_id, _) as ret), Const (Cfun callee), args, location, _) -> ( match FbGKInteraction.get_config_check tenv callee args with | Some (`Config config) -> - Dom.call_config_check ret config astate + Dom.call_config_check ret_id config astate | Some (`Exp _) -> astate | None -> (* normal function calls *) - Dom.call tenv analyze_dependency callee args location astate ) + let call = CostInstantiate.Call.{instr; loc= location; pname= callee; node; args; ret} in + let is_cheap_call = get_is_cheap_call call in + Dom.call tenv analyze_dependency ~is_cheap_call callee args location astate ) | Prune (e, _, _, _) -> Dom.prune e astate | _ -> @@ -525,6 +529,8 @@ let has_call_stmt proc_desc = let checker ({InterproceduralAnalysis.proc_desc} as analysis_data) = + let get_is_cheap_call = CostInstantiate.get_is_cheap_call analysis_data in + let analysis_data = {interproc= analysis_data; get_is_cheap_call} in 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 ) diff --git a/infer/src/cost/ConfigImpactAnalysis.mli b/infer/src/cost/ConfigImpactAnalysis.mli index fc4f78919..39c33b8d3 100644 --- a/infer/src/cost/ConfigImpactAnalysis.mli +++ b/infer/src/cost/ConfigImpactAnalysis.mli @@ -42,4 +42,6 @@ module Summary : sig end val checker : - (Summary.t option * CostDomain.summary option) InterproceduralAnalysis.t -> Summary.t option + (BufferOverrunAnalysisSummary.t option * Summary.t option * CostDomain.summary option) + InterproceduralAnalysis.t + -> Summary.t option diff --git a/infer/src/cost/cost.ml b/infer/src/cost/cost.ml index 408977691..0a9271bec 100644 --- a/infer/src/cost/cost.ml +++ b/infer/src/cost/cost.ml @@ -81,8 +81,6 @@ module InstrBasicCostWithReason = struct | None -> ( match callee_cost_opt with | Some callee_cost -> - L.debug Analysis Verbose "@\nInstantiated cost : %a@\n" BasicCostWithReason.pp_hum - callee_cost ; callee_cost | _ -> ScubaLogging.cost_log_message ~label:"unmodeled_function_operation_cost" diff --git a/infer/src/cost/costInstantiate.ml b/infer/src/cost/costInstantiate.ml new file mode 100644 index 000000000..863515968 --- /dev/null +++ b/infer/src/cost/costInstantiate.ml @@ -0,0 +1,124 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) +open! IStd +module F = Format +module InstrCFG = ProcCfg.NormalOneInstrPerNode + +module Call = struct + type t = + { instr: Sil.instr + ; loc: Location.t + ; pname: Procname.t + ; node: Procdesc.Node.t + ; args: (Exp.t * Typ.t) list + ; ret: Ident.t * Typ.t } + [@@deriving compare] + + let pp fmt {pname; loc} = + F.fprintf fmt "loop-invariant call to %a, at %a " Procname.pp pname Location.pp loc +end + +type cost_args = + { tenv: Tenv.t + ; integer_type_widths: Typ.IntegerWidths.t + ; get_callee_cost_summary_and_formals: + Procname.t -> (CostDomain.summary * (Pvar.t * Typ.t) list) option + ; inferbo_invariant_map: BufferOverrunAnalysis.invariant_map + ; inferbo_get_summary: BufferOverrunAnalysisSummary.get_summary + ; call: Call.t } + +type 'a interproc_analysis = + (BufferOverrunAnalysisSummary.t option * 'a * CostDomain.summary option) InterproceduralAnalysis.t + +let get_symbolic_cost + { tenv + ; integer_type_widths + ; get_callee_cost_summary_and_formals + ; inferbo_invariant_map + ; inferbo_get_summary + ; call= Call.{instr; pname; node; ret; args} } = + let last_node = Option.value_exn (InstrCFG.of_instr_opt node instr) in + let inferbo_mem = + let instr_node_id = InstrCFG.Node.id last_node in + Option.value_exn (BufferOverrunAnalysis.extract_pre instr_node_id inferbo_invariant_map) + in + let loc = InstrCFG.Node.loc last_node in + let get_symbolic cost = + if CostDomain.BasicCost.is_symbolic cost then `SymbolicCost cost else `Cheap + in + let get_summary pname = Option.map ~f:fst (get_callee_cost_summary_and_formals pname) in + match get_callee_cost_summary_and_formals pname with + | Some (CostDomain.{post= cost_record}, callee_formals) -> + let callee_cost = CostDomain.get_operation_cost cost_record in + if CostDomain.BasicCost.is_symbolic callee_cost.cost then + (Cost.instantiate_cost ~default_closure_cost:Ints.NonNegativeInt.one integer_type_widths + ~inferbo_caller_mem:inferbo_mem ~callee_pname:pname ~callee_formals ~args ~callee_cost + ~loc) + .cost |> get_symbolic + else `Cheap + | None -> + let fun_arg_list = + List.map args ~f:(fun (exp, typ) -> + ProcnameDispatcher.Call.FuncArg.{exp; typ; arg_payload= ()} ) + in + CostModels.Call.dispatch tenv pname fun_arg_list + |> Option.value_map ~default:`NoModel ~f:(fun model -> + let model_env = + let node_hash = InstrCFG.Node.hash last_node in + BufferOverrunUtils.ModelEnv.mk_model_env pname ~node_hash loc tenv + integer_type_widths inferbo_get_summary + in + model CostUtils.CostModelEnv.{get_summary; model_env} ~ret inferbo_mem |> get_symbolic ) + + +let prepare_call_args + ({InterproceduralAnalysis.proc_desc; exe_env; analyze_dependency} as analysis_data) call = + let proc_name = Procdesc.get_proc_name proc_desc in + let tenv = Exe_env.get_proc_tenv exe_env proc_name in + let integer_type_widths = Exe_env.get_integer_type_widths exe_env proc_name in + let inferbo_invariant_map = + BufferOverrunAnalysis.cached_compute_invariant_map + (InterproceduralAnalysis.bind_payload ~f:fst3 analysis_data) + in + let open IOption.Let_syntax in + let get_callee_cost_summary_and_formals callee_pname = + let* callee_pdesc, (_inferbo, _, callee_costs_summary) = analyze_dependency callee_pname in + let+ callee_costs_summary = callee_costs_summary in + (callee_costs_summary, Procdesc.get_pvar_formals callee_pdesc) + in + let inferbo_get_summary callee_pname = + let* _callee_pdesc, (inferbo, _purity, _callee_costs_summary) = + analyze_dependency callee_pname + in + inferbo + in + { tenv + ; integer_type_widths + ; get_callee_cost_summary_and_formals + ; inferbo_invariant_map + ; inferbo_get_summary + ; call } + + +let get_cost_if_expensive analysis_data call = + match prepare_call_args analysis_data call |> get_symbolic_cost with + | `SymbolicCost cost -> + Some cost + | `Cheap | `NoModel -> + None + + +let get_is_cheap_call analysis_data call = + match prepare_call_args analysis_data call |> get_symbolic_cost with + | `SymbolicCost _ -> + (* symbolic costs (e.g. 4n+5 or log(n) are considered expensive) *) + false + | `Cheap -> + true + | `NoModel -> + (* unmodeled calls are considered expensive *) + false diff --git a/infer/src/cost/costInstantiate.mli b/infer/src/cost/costInstantiate.mli new file mode 100644 index 000000000..0b04db2d9 --- /dev/null +++ b/infer/src/cost/costInstantiate.mli @@ -0,0 +1,28 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +module Call : sig + type t = + { instr: Sil.instr + ; loc: Location.t + ; pname: Procname.t + ; node: Procdesc.Node.t + ; args: (Exp.t * Typ.t) list + ; ret: Ident.t * Typ.t } + [@@deriving compare] + + val pp : Format.formatter -> t -> unit +end + +type 'a interproc_analysis = + (BufferOverrunAnalysisSummary.t option * 'a * CostDomain.summary option) InterproceduralAnalysis.t + +val get_cost_if_expensive : 'a interproc_analysis -> Call.t -> CostDomain.BasicCost.t option + +val get_is_cheap_call : 'a interproc_analysis -> Call.t -> bool diff --git a/infer/src/cost/hoisting.ml b/infer/src/cost/hoisting.ml index 9f1aaf808..6ca0e35c1 100644 --- a/infer/src/cost/hoisting.ml +++ b/infer/src/cost/hoisting.ml @@ -8,23 +8,8 @@ open! IStd module F = Format module InstrCFG = ProcCfg.NormalOneInstrPerNode module BasicCost = CostDomain.BasicCost - -module Call = struct - type t = - { instr: Sil.instr - ; loc: Location.t - ; pname: Procname.t - ; node: Procdesc.Node.t - ; args: (Exp.t * Typ.t) list - ; ret: Ident.t * Typ.t } - [@@deriving compare] - - let pp fmt {pname; loc} = - F.fprintf fmt "loop-invariant call to %a, at %a " Procname.pp pname Location.pp loc -end - module LoopNodes = AbstractDomain.FiniteSet (Procdesc.Node) -module HoistCalls = AbstractDomain.FiniteSet (Call) +module HoistCalls = AbstractDomain.FiniteSet (CostInstantiate.Call) (** Map loop_header -> instrs that can be hoisted out of the loop *) module LoopHeadToHoistInstrs = Procdesc.NodeMap @@ -71,8 +56,8 @@ let get_hoist_inv_map tenv ~get_callee_purity reaching_defs_invariant_map loop_h loop_head_to_source_nodes LoopHeadToHoistInstrs.empty -let do_report extract_cost_if_expensive proc_desc err_log (Call.{pname; loc} as call) loop_head_loc - = +let do_report extract_cost_if_expensive proc_desc err_log + (CostInstantiate.Call.{pname; loc} as call) loop_head_loc = let exp_desc = F.asprintf "The call to %a at %a is loop-invariant" Procname.pp pname Location.pp loc in @@ -100,43 +85,6 @@ let do_report extract_cost_if_expensive proc_desc err_log (Call.{pname; loc} as Reporting.log_issue proc_desc err_log ~loc ~ltr LoopHoisting issue message -let get_cost_if_expensive tenv integer_type_widths get_callee_cost_summary_and_formals - inferbo_invariant_map inferbo_get_summary Call.{instr; pname; node; ret; args} = - let last_node = Option.value_exn (InstrCFG.of_instr_opt node instr) in - let inferbo_mem = - let instr_node_id = InstrCFG.Node.id last_node in - Option.value_exn (BufferOverrunAnalysis.extract_pre instr_node_id inferbo_invariant_map) - in - let loc = InstrCFG.Node.loc last_node in - let get_summary pname = Option.map ~f:fst (get_callee_cost_summary_and_formals pname) in - let cost_opt = - match get_callee_cost_summary_and_formals pname with - | Some (CostDomain.{post= cost_record}, callee_formals) -> - let callee_cost = CostDomain.get_operation_cost cost_record in - if CostDomain.BasicCost.is_symbolic callee_cost.cost then - Some - (Cost.instantiate_cost ~default_closure_cost:Ints.NonNegativeInt.one integer_type_widths - ~inferbo_caller_mem:inferbo_mem ~callee_pname:pname ~callee_formals ~args - ~callee_cost ~loc) - .cost - else None - | None -> - let fun_arg_list = - List.map args ~f:(fun (exp, typ) -> - ProcnameDispatcher.Call.FuncArg.{exp; typ; arg_payload= ()} ) - in - CostModels.Call.dispatch tenv pname fun_arg_list - |> Option.map ~f:(fun model -> - let model_env = - let node_hash = InstrCFG.Node.hash last_node in - BufferOverrunUtils.ModelEnv.mk_model_env pname ~node_hash loc tenv - integer_type_widths inferbo_get_summary - in - model CostUtils.CostModelEnv.{get_summary; model_env} ~ret inferbo_mem ) - in - Option.filter cost_opt ~f:CostDomain.BasicCost.is_symbolic - - let report_errors proc_desc tenv err_log get_callee_purity reaching_defs_invariant_map loop_head_to_source_nodes extract_cost_if_expensive = (* get dominators *) @@ -162,33 +110,13 @@ let checker ({InterproceduralAnalysis.proc_desc; exe_env; err_log; analyze_dependency} as analysis_data) = let proc_name = Procdesc.get_proc_name proc_desc in let tenv = Exe_env.get_proc_tenv exe_env proc_name in - let integer_type_widths = Exe_env.get_integer_type_widths exe_env proc_name in let cfg = InstrCFG.from_pdesc proc_desc in (* computes reaching defs: node -> (var -> node set) *) let reaching_defs_invariant_map = ReachingDefs.compute_invariant_map proc_desc in let loop_head_to_source_nodes = Loop_control.get_loop_head_to_source_nodes cfg in let extract_cost_if_expensive = if Config.hoisting_report_only_expensive then - let inferbo_invariant_map = - BufferOverrunAnalysis.cached_compute_invariant_map - (InterproceduralAnalysis.bind_payload ~f:fst3 analysis_data) - in - let open IOption.Let_syntax in - let get_callee_cost_summary_and_formals callee_pname = - let* callee_pdesc, (_inferbo, _purity, callee_costs_summary) = - analyze_dependency callee_pname - in - let+ callee_costs_summary = callee_costs_summary in - (callee_costs_summary, Procdesc.get_pvar_formals callee_pdesc) - in - let inferbo_get_summary callee_pname = - let* _callee_pdesc, (inferbo, _purity, _callee_costs_summary) = - analyze_dependency callee_pname - in - inferbo - in - get_cost_if_expensive tenv integer_type_widths get_callee_cost_summary_and_formals - inferbo_invariant_map inferbo_get_summary + CostInstantiate.get_cost_if_expensive analysis_data else fun _ -> None in let get_callee_purity callee_pname = diff --git a/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile b/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile index a7e51fa5c..4e995276b 100644 --- a/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile +++ b/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile @@ -6,7 +6,7 @@ TESTS_DIR = ../../.. CLANG_OPTIONS = -c $(OBJC_CLANG_OPTIONS) -INFER_OPTIONS = --config-impact-analysis-only --config-impact-data-file config_data.json --debug-exceptions \ +INFER_OPTIONS = --config-impact-analysis-only --config-impact-data-file config_data.json --debug-exceptions \ --report-force-relative-path --project-root $(TESTS_DIR) INFERPRINT_OPTIONS = --issues-tests