Skip to content

Commit

Permalink
[config impact] Do not use cost analysis results
Browse files Browse the repository at this point in the history
Summary:
This diff the config impact checker not to depend on cost analysis
results, to give more flexibility of turning on/off checkers.

Reviewed By: ezgicicek

Differential Revision: D49776014

fbshipit-source-id: 5b3c476fe04aeb591222655b29b479295cce1d5f
  • Loading branch information
skcho authored and facebook-github-bot committed Oct 3, 2023
1 parent 8205df2 commit 3cf8b32
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 76 deletions.
4 changes: 2 additions & 2 deletions infer/documentation/issues/CONFIG_IMPACT.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Infer reports this issue when an *expensive* function is called without a *config check*. The
*config* is usually a boolean value that enables experimental new features and it is defined per
application/codebase, e.g. gatekeepers. To determine whether a function is expensive or not, the
checker relies on [Cost analysis](/docs/next/checker-cost) results and modeled functions that are
assumed to be expensive, e.g. string operations, regular expression match, or DB accesses.
checker relies on modeled functions that are assumed to be expensive, e.g. string operations,
regular expression match, or DB accesses.

Similar to [Cost analysis](/docs/next/checker-cost), this issue type is reported only in
differential mode, i.e. when there are original code and modified one and we can compare Infer's
Expand Down
5 changes: 1 addition & 4 deletions infer/src/backend/registerCheckers.ml
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,7 @@ let all_checkers =
; { checker= ConfigImpactAnalysis
; callbacks=
(let 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
interprocedural Payloads.Fields.config_impact_analysis ConfigImpactAnalysis.checker
in
[(checker, Clang); (checker, Java)] ) }
; { checker= LineageShape
Expand Down
2 changes: 1 addition & 1 deletion infer/src/base/Checker.ml
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,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= [Cost] }
; activates= [] }
| Cost ->
{ id= "cost"
; kind=
Expand Down
77 changes: 22 additions & 55 deletions infer/src/cost/ConfigImpactAnalysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,8 @@ module Dom = struct
; +PatternMatch.Java.implements_android "content.SharedPreferences"
&::+ (fun _ method_name -> String.is_prefix method_name ~prefix:"get")
&--> KnownExpensive
; +PatternMatch.Java.implements_arrays &:: "sort" $ any_arg $+...$--> KnownExpensive
; +PatternMatch.Java.implements_collections &:: "sort" $ any_arg $+...$--> KnownExpensive
; +PatternMatch.Java.implements_google "common.base.Preconditions"
&:: "checkArgument" $ any_arg $+ any_arg $+...$--> KnownExpensive
; +PatternMatch.Java.implements_google "common.base.Preconditions"
Expand All @@ -769,6 +771,7 @@ module Dom = struct
&:: "checkState" $ any_arg $+ any_arg $+...$--> KnownExpensive
; +PatternMatch.Java.implements_lang "String" &:: "concat" &--> KnownExpensive
; +PatternMatch.Java.implements_lang "StringBuilder" &:: "append" &--> KnownExpensive
; +PatternMatch.Java.implements_list &:: "sort" $ any_arg $+...$--> KnownExpensive
; +PatternMatch.Java.implements_regex "Pattern" &:: "compile" &--> KnownExpensive
; +PatternMatch.Java.implements_regex "Pattern" &:: "matcher" &--> KnownExpensive
; +PatternMatch.Java.implements_kotlin_intrinsics &::.*--> KnownCheap
Expand Down Expand Up @@ -892,8 +895,7 @@ module Dom = struct
astate )


let call tenv analyze_dependency ~(instantiated_cost : CostInstantiate.instantiated_cost option)
ret_typ ~callee formals args location
let call tenv analyze_dependency ret_typ ~callee formals args location
( { condition_checks= config_checks, latent_config_checks
; unchecked_callees
; unchecked_callees_cond } as astate ) =
Expand Down Expand Up @@ -935,29 +937,13 @@ module Dom = struct
in
let astate =
if ConfigChecks.is_top config_checks then
let (callee_summary : Summary.t option) =
match analyze_dependency callee with
| None ->
None
| Some (_, analysis_data, _) ->
analysis_data
in
let (callee_summary : Summary.t option) = analyze_dependency callee in
let expensiveness_model = get_expensiveness_model tenv callee args in
let has_expensive_callee =
Option.exists callee_summary ~f:Summary.has_known_expensive_callee
in
let is_cheap_call =
match instantiated_cost with
| Some Cheap ->
true
| Some (Symbolic _) ->
false
| Some NoModel | None ->
(* When the cost of the callee is unknown by some reasons, we apply a heuristic to
ignore Kotlin's getter as cheap. *)
is_kotlin_getter callee args
in
let is_unmodeled_call = match instantiated_cost with Some NoModel -> true | _ -> false in
(* We apply a heuristic to ignore Kotlin's getter as cheap. *)
let is_cheap_call = is_kotlin_getter callee args in
match mode with
| `Strict -> (
let is_static = Procname.is_static callee in
Expand Down Expand Up @@ -987,7 +973,7 @@ module Dom = struct
add_callee_name ~is_known_expensive:false )
| `Normal -> (
match expensiveness_model with
| None when is_cheap_call && not has_expensive_callee ->
| None when not has_expensive_callee ->
(* If callee is cheap by heuristics, ignore it. *)
astate
| Some KnownCheap ->
Expand All @@ -1014,8 +1000,8 @@ module Dom = struct
in
(* If callee's summary is not leaf, use it. *)
join_callee_summary callee_summary callee_summary_cond
| None when Procname.is_objc_init callee || is_unmodeled_call ->
(* If callee is unknown ObjC initializer or has no cost model, ignore it. *)
| None when Procname.is_objc_init callee ->
(* If callee is unknown ObjC initializer, ignore it. *)
astate
| _ ->
(* Otherwise, add callee's name. *)
Expand All @@ -1032,11 +1018,8 @@ module Dom = struct
end

type analysis_data =
{ interproc:
(BufferOverrunAnalysisSummary.t option * Summary.t option * CostDomain.summary option)
InterproceduralAnalysis.t
{ interproc: Summary.t InterproceduralAnalysis.t
; get_formals: Procname.t -> (Pvar.t * Typ.t) list option
; get_instantiated_cost: CostInstantiate.Call.t -> CostInstantiate.instantiated_cost option
; is_param: Pvar.t -> bool }

module TransferFunctions = struct
Expand Down Expand Up @@ -1103,9 +1086,9 @@ module TransferFunctions = struct

let add_ret analyze_dependency id callee astate =
match analyze_dependency callee with
| Some (_, Some {Summary.ret= ret_val}, _) ->
| Some {Summary.ret= ret_val} ->
Dom.add_mem (Loc.of_id id) ret_val astate
| _ ->
| None ->
astate


Expand Down Expand Up @@ -1134,11 +1117,8 @@ module TransferFunctions = struct
else None


let call
{ interproc= {proc_desc; tenv; analyze_dependency}
; get_formals
; get_instantiated_cost
; is_param } node idx ((ret_id, ret_typ) as ret) callee args captured_vars location astate =
let call {interproc= {proc_desc; tenv; analyze_dependency}; get_formals; is_param}
(ret_id, ret_typ) callee args location astate =
let callee =
get_kotlin_lazy_method ~caller:(Procdesc.get_proc_name proc_desc) ~callee
|> Option.value_map ~default:callee ~f:(fun invoke ->
Expand All @@ -1159,24 +1139,13 @@ module TransferFunctions = struct
Dom.add_mem (Loc.of_pvar pvar) (Val.of_config config) astate
| None ->
(* normal function calls *)
let call =
CostInstantiate.Call.
{ loc= location
; pname= callee
; node= CFG.Node.to_instr idx node
; args
; captured_vars
; ret }
in
let instantiated_cost = get_instantiated_cost call in
let formals = get_formals callee in
Dom.call tenv analyze_dependency ~instantiated_cost ret_typ ~callee formals args location
astate
Dom.call tenv analyze_dependency ret_typ ~callee formals args location astate
|> add_ret analyze_dependency ret_id callee


let exec_instr ({Dom.condition_checks} as astate)
({interproc= {tenv; analyze_dependency}; is_param} as analysis_data) node idx instr =
({interproc= {tenv; analyze_dependency}; is_param} as analysis_data) _node _idx instr =
match (instr : Sil.instr) with
| Load {id; e} -> (
match FbGKInteraction.get_config ~is_param e with
Expand Down Expand Up @@ -1221,11 +1190,10 @@ module TransferFunctions = struct
, _ )
when Procname.equal dispatch_sync BuiltinDecl.dispatch_sync ->
let args = List.map captured_vars ~f:(fun (exp, _, typ, _) -> (exp, typ)) in
call analysis_data node idx ret callee args [] location astate
| Call (ret, Const (Cfun callee), args, location, _) ->
call analysis_data node idx ret callee args [] location astate
| Call (ret, Closure {name= callee; captured_vars}, args, location, _) ->
call analysis_data node idx ret callee args captured_vars location astate
call analysis_data ret callee args location astate
| Call (ret, Const (Cfun callee), args, location, _)
| Call (ret, Closure {name= callee}, args, location, _) ->
call analysis_data ret callee args location astate
| Prune (e, _, _, _) ->
Dom.prune e astate
| _ ->
Expand Down Expand Up @@ -1257,12 +1225,11 @@ let checker ({InterproceduralAnalysis.proc_desc} as analysis_data) =
let get_formals pname =
Attributes.load pname |> Option.map ~f:ProcAttributes.get_pvar_formals
in
let get_instantiated_cost = CostInstantiate.get_instantiated_cost analysis_data in
let is_param =
let formals = Procdesc.get_pvar_formals proc_desc in
fun pvar -> List.exists formals ~f:(fun (formal, _) -> Pvar.equal pvar formal)
in
let analysis_data = {interproc= analysis_data; get_formals; get_instantiated_cost; is_param} in
let analysis_data = {interproc= analysis_data; get_formals; is_param} 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 pname ~has_call_stmt astate )
5 changes: 1 addition & 4 deletions infer/src/cost/ConfigImpactAnalysis.mli
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,4 @@ module Summary : sig
val instantiate_unchecked_callees_cond : all_configs:LatentConfigs.t -> t -> t
end

val checker :
(BufferOverrunAnalysisSummary.t option * Summary.t option * CostDomain.summary option)
InterproceduralAnalysis.t
-> Summary.t option
val checker : Summary.t InterproceduralAnalysis.t -> Summary.t option
6 changes: 0 additions & 6 deletions infer/src/cost/costInstantiate.ml
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,3 @@ let get_cost_if_expensive analysis_data call =
let open IOption.Let_syntax in
let* call_args = prepare_call_args analysis_data call in
match get_instantiated_cost call_args with Symbolic cost -> Some cost | Cheap | NoModel -> None


let get_instantiated_cost analysis_data call =
let open IOption.Let_syntax in
let+ call_args = prepare_call_args analysis_data call in
get_instantiated_cost call_args
4 changes: 0 additions & 4 deletions infer/src/cost/costInstantiate.mli
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,4 @@ end
type 'a interproc_analysis =
(BufferOverrunAnalysisSummary.t option * 'a * CostDomain.summary option) InterproceduralAnalysis.t

type instantiated_cost = Cheap | NoModel | Symbolic of CostDomain.BasicCost.t

val get_cost_if_expensive : 'a interproc_analysis -> Call.t -> CostDomain.BasicCost.t option

val get_instantiated_cost : 'a interproc_analysis -> Call.t -> instantiated_cost option

0 comments on commit 3cf8b32

Please sign in to comment.