[ConfigImpact] Add a new checker: ConfigImpactAnalysis

Summary: This checker is for internal usage.

Reviewed By: ezgicicek

Differential Revision: D26453750

fbshipit-source-id: a9397f2e4

Makefile

@@ -172,6 +172,7 @@ DIRECT_TESTS += \
 
 ifeq ($(IS_FACEBOOK_TREE),yes)
 DIRECT_TESTS += \
+  java_fb-config-impact \
   java_fb-gk-interaction \
   java_fb-performance
 endif

infer/man/man1/infer-analyze.txt

@@ -57,6 +57,15 @@ OPTIONS
            other checkers (Conversely:
            --no-config-checks-between-markers-only)
 
+       --config-impact-analysis
+           Activates: checker config-impact-analysis: [EXPERIMENTAL] Collects
+           function that are called without config checks. (Conversely:
+           --no-config-impact-analysis)
+
+       --config-impact-analysis-only
+           Activates: Enable config-impact-analysis and disable all other
+           checkers (Conversely: --no-config-impact-analysis-only)
+
        --continue-analysis
            Activates: Continue the analysis after more targets are captured
            by --continue. The other analysis options should be given the same

infer/man/man1/infer-full.txt

@@ -258,6 +258,16 @@ OPTIONS
            other checkers (Conversely:
            --no-config-checks-between-markers-only)           See also infer-analyze(1).
 
+       --config-impact-analysis
+           Activates: checker config-impact-analysis: [EXPERIMENTAL] Collects
+           function that are called without config checks. (Conversely:
+           --no-config-impact-analysis)           See also infer-analyze(1).
+
+       --config-impact-analysis-only
+           Activates: Enable config-impact-analysis and disable all other
+           checkers (Conversely: --no-config-impact-analysis-only)       
+    See also infer-analyze(1).
+
        --continue
            Activates: Continue the capture for the reactive analysis,
            increasing the changed files/procedures. (If a procedure was
@@ -418,6 +428,7 @@ OPTIONS
              CONDITION_ALWAYS_FALSE (disabled by default),
              CONDITION_ALWAYS_TRUE (disabled by default),
              CONFIG_CHECKS_BETWEEN_MARKERS (disabled by default),
+             CONFIG_IMPACT (disabled by default),
              CONSTANT_ADDRESS_DEREFERENCE (disabled by default),
              CREATE_INTENT_FROM_URI (enabled by default),
              CROSS_SITE_SCRIPTING (enabled by default),

infer/man/man1/infer-report.txt

@@ -121,6 +121,7 @@ OPTIONS
              CONDITION_ALWAYS_FALSE (disabled by default),
              CONDITION_ALWAYS_TRUE (disabled by default),
              CONFIG_CHECKS_BETWEEN_MARKERS (disabled by default),
+             CONFIG_IMPACT (disabled by default),
              CONSTANT_ADDRESS_DEREFERENCE (disabled by default),
              CREATE_INTENT_FROM_URI (enabled by default),
              CROSS_SITE_SCRIPTING (enabled by default),

infer/man/man1/infer.txt

@@ -258,6 +258,16 @@ OPTIONS
            other checkers (Conversely:
            --no-config-checks-between-markers-only)           See also infer-analyze(1).
 
+       --config-impact-analysis
+           Activates: checker config-impact-analysis: [EXPERIMENTAL] Collects
+           function that are called without config checks. (Conversely:
+           --no-config-impact-analysis)           See also infer-analyze(1).
+
+       --config-impact-analysis-only
+           Activates: Enable config-impact-analysis and disable all other
+           checkers (Conversely: --no-config-impact-analysis-only)       
+    See also infer-analyze(1).
+
        --continue
            Activates: Continue the capture for the reactive analysis,
            increasing the changed files/procedures. (If a procedure was
@@ -418,6 +428,7 @@ OPTIONS
              CONDITION_ALWAYS_FALSE (disabled by default),
              CONDITION_ALWAYS_TRUE (disabled by default),
              CONFIG_CHECKS_BETWEEN_MARKERS (disabled by default),
+             CONFIG_IMPACT (disabled by default),
              CONSTANT_ADDRESS_DEREFERENCE (disabled by default),
              CREATE_INTENT_FROM_URI (enabled by default),
              CROSS_SITE_SCRIPTING (enabled by default),

infer/src/backend/Payloads.ml

@@ -14,6 +14,7 @@ type t =
   ; buffer_overrun_analysis: BufferOverrunAnalysisSummary.t option
   ; buffer_overrun_checker: BufferOverrunCheckerSummary.t option
   ; config_checks_between_markers: ConfigChecksBetweenMarkers.Summary.t option
+  ; config_impact_analysis: ConfigImpactAnalysis.Summary.t option
   ; cost: CostDomain.summary option
   ; lab_resource_leaks: ResourceLeakDomain.summary option
   ; dotnet_resource_leaks: ResourceLeakCSDomain.summary option
@@ -46,6 +47,7 @@ let fields =
     ~buffer_overrun_checker:(fun f -> mk f "BufferOverrunChecker" BufferOverrunCheckerSummary.pp)
     ~config_checks_between_markers:(fun f ->
       mk f "ConfigChecksBetweenMarkers" ConfigChecksBetweenMarkers.Summary.pp )
+    ~config_impact_analysis:(fun f -> mk f "ConfigImpactAnalysis" ConfigImpactAnalysis.Summary.pp)
     ~cost:(fun f -> mk f "Cost" CostDomain.pp_summary)
     ~litho_required_props:(fun f -> mk f "Litho Required Props" LithoDomain.pp_summary)
     ~pulse:(fun f -> mk f "Pulse" PulseSummary.pp)
@@ -71,6 +73,7 @@ let empty =
   ; buffer_overrun_analysis= None
   ; buffer_overrun_checker= None
   ; config_checks_between_markers= None
+  ; config_impact_analysis= None
   ; cost= None
   ; lab_resource_leaks= None
   ; dotnet_resource_leaks= None

infer/src/backend/Payloads.mli

@@ -18,6 +18,7 @@ include sig
     ; buffer_overrun_analysis: BufferOverrunAnalysisSummary.t option
     ; buffer_overrun_checker: BufferOverrunCheckerSummary.t option
     ; config_checks_between_markers: ConfigChecksBetweenMarkers.Summary.t option
+    ; config_impact_analysis: ConfigImpactAnalysis.Summary.t option
     ; cost: CostDomain.summary option
     ; lab_resource_leaks: ResourceLeakDomain.summary option
     ; dotnet_resource_leaks: ResourceLeakCSDomain.summary option

infer/src/backend/registerCheckers.ml

@@ -197,7 +197,13 @@ let all_checkers =
            interprocedural Payloads.Fields.config_checks_between_markers
              ConfigChecksBetweenMarkers.checker
          in
-         [(checker, Clang); (checker, Java)] ) } ]
+         [(checker, Clang); (checker, Java)] ) }
+  ; { checker= ConfigImpactAnalysis
+    ; callbacks=
+        (let checker =
+           interprocedural Payloads.Fields.config_impact_analysis ConfigImpactAnalysis.checker
+         in
+         [(checker, Java)] ) } ]
 
 
 let get_active_checkers () =

infer/src/base/Checker.ml

@@ -14,6 +14,7 @@ type t =
   | BufferOverrunAnalysis
   | BufferOverrunChecker
   | ConfigChecksBetweenMarkers
+  | ConfigImpactAnalysis
   | Cost
   | Eradicate
   | FragmentRetainsView
@@ -128,12 +129,35 @@ let config_unsafe checker =
       ; kind=
           UserFacing
             { title= "Config Checks between Markers"
-            ; markdown_body= "This checker is currently only useful for certain Facebook code." }
+            ; markdown_body=
+                "This checker collects config checkings in some program regions determined by \
+                 pairs of marker-starts and marker-ends. The set of config checking functions, \
+                 marker-start functions, and marker-end functions is hardcoded and empty by \
+                 default for now, so to use this checker, please modify the code directly in \
+                 [FbGKInteraction.ml](https://github.com/facebook/infer/tree/master/infer/src/opensource)."
+            }
       ; support= supports_clang_and_java_experimental
       ; short_documentation= "[EXPERIMENTAL] Collects config checks between marker start and end."
       ; cli_flags= Some {deprecated= []; show_in_help= true}
       ; enabled_by_default= false
       ; activates= [] }
+  | ConfigImpactAnalysis ->
+      { id= "config-impact-analysis"
+      ; kind=
+          UserFacing
+            { title= "Config Impact Analysis"
+            ; markdown_body=
+                "This checker collects functions whose execution isn't gated by certain \
+                 pre-defined gating functions. The set of gating functions is hardcoded and empty \
+                 by default for now, so to use this checker, please modify the code directly in \
+                 [FbGKInteraction.ml](https://github.com/facebook/infer/tree/master/infer/src/opensource)."
+            }
+      ; support= supports_clang_and_java_experimental
+      ; short_documentation=
+          "[EXPERIMENTAL] Collects function that are called without config checks."
+      ; cli_flags= Some {deprecated= []; show_in_help= true}
+      ; enabled_by_default= false
+      ; activates= [] }
   | Cost ->
       { id= "cost"
       ; kind=

infer/src/base/Checker.mli

@@ -13,6 +13,7 @@ type t =
   | BufferOverrunAnalysis
   | BufferOverrunChecker
   | ConfigChecksBetweenMarkers
+  | ConfigImpactAnalysis
   | Cost
   | Eradicate
   | FragmentRetainsView

infer/src/base/IssueType.ml

@@ -414,6 +414,11 @@ let config_checks_between_markers =
     ~user_documentation:"A config checking is done between a marker's start and end"
 
 
+let config_impact_analysis =
+  register ~enabled:false ~id:"CONFIG_IMPACT" Advice ConfigImpactAnalysis
+    ~user_documentation:"A function is called without a config check"
+
+
 let constant_address_dereference =
   register ~enabled:false ~id:"CONSTANT_ADDRESS_DEREFERENCE" Warning Pulse
     ~user_documentation:[%blob "../../documentation/issues/CONSTANT_ADDRESS_DEREFERENCE.md"]

infer/src/base/IssueType.mli

@@ -135,6 +135,8 @@ val condition_always_true : t
 
 val config_checks_between_markers : t
 
+val config_impact_analysis : t
+
 val constant_address_dereference : t
 
 val create_intent_from_uri : t

infer/src/checkers/ConfigImpactAnalysis.ml

@@ -0,0 +1,239 @@
+(*
+ * 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 ConfigName = FbGKInteraction.ConfigName
+
+module Branch = struct
+  type t = True | False | Top
+
+  let pp f = function
+    | True ->
+        F.pp_print_string f "true branch"
+    | False ->
+        F.pp_print_string f "false branch"
+    | Top ->
+        AbstractDomain.TopLiftedUtils.pp_top f
+
+
+  let leq ~lhs ~rhs =
+    match (lhs, rhs) with True, True | False, False | _, Top -> true | _, _ -> false
+
+
+  let join x y = match (x, y) with True, True -> True | False, False -> False | _, _ -> Top
+
+  let widen ~prev ~next ~num_iters:_ = join prev next
+
+  let top = Top
+
+  let is_top = function Top -> true | True | False -> false
+
+  let neg = function True -> False | False -> True | Top -> Top
+end
+
+module ConfigChecks = AbstractDomain.SafeInvertedMap (ConfigName) (Branch)
+
+module UncheckedCallee = struct
+  type t = {callee: Procname.t; location: Location.t} [@@deriving compare]
+
+  let pp f {callee; location} =
+    F.fprintf f "%a is called at %a" Procname.pp callee Location.pp location
+
+
+  let get_location {location} = location
+
+  let report {InterproceduralAnalysis.proc_desc; err_log} x =
+    let desc = F.asprintf "%a without config check" pp x in
+    let trace =
+      (* TODO *)
+      []
+    in
+    Reporting.log_issue proc_desc err_log ~loc:(get_location x) ~ltr:trace ConfigImpactAnalysis
+      IssueType.config_impact_analysis desc
+end
+
+module UncheckedCallees = struct
+  include AbstractDomain.FiniteSet (UncheckedCallee)
+
+  let report analysis_data unchecked_callees =
+    iter (UncheckedCallee.report analysis_data) unchecked_callees
+end
+
+module Loc = struct
+  type t = Ident of Ident.t | Pvar of Pvar.t [@@deriving compare]
+
+  let pp f = function Ident id -> Ident.pp f id | Pvar pvar -> Pvar.pp Pp.text f pvar
+
+  let of_id id = Ident id
+
+  let of_pvar pvar = Pvar pvar
+end
+
+module ConfigLifted = AbstractDomain.Flat (ConfigName)
+
+module Val = struct
+  type t = {config: ConfigLifted.t}
+
+  let pp f {config} = F.fprintf f "@[config:@,%a@]" ConfigLifted.pp config
+
+  let leq ~lhs ~rhs = ConfigLifted.leq ~lhs:lhs.config ~rhs:rhs.config
+
+  let join x y = {config= ConfigLifted.join x.config y.config}
+
+  let widen ~prev ~next ~num_iters =
+    {config= ConfigLifted.widen ~prev:prev.config ~next:next.config ~num_iters}
+
+
+  let bottom = {config= ConfigLifted.bottom}
+
+  let of_config config = {config= ConfigLifted.v config}
+
+  let get_config_opt {config} = ConfigLifted.get config
+end
+
+module Mem = struct
+  include AbstractDomain.Map (Loc) (Val)
+
+  let lookup loc mem = find_opt loc mem |> Option.value ~default:Val.bottom
+end
+
+module Summary = struct
+  type t = {unchecked_callees: UncheckedCallees.t}
+
+  let pp f {unchecked_callees} =
+    F.fprintf f "@[unchecked callees:@,%a@]" UncheckedCallees.pp unchecked_callees
+end
+
+module Dom = struct
+  type t = {config_checks: ConfigChecks.t; unchecked_callees: UncheckedCallees.t; mem: Mem.t}
+
+  let pp f {config_checks; unchecked_callees; mem} =
+    F.fprintf f "@[@[config checks:@,%a@]@ @[unchecked callees:@,%a@]@ @[mem:%,%a@]@]"
+      ConfigChecks.pp config_checks UncheckedCallees.pp unchecked_callees Mem.pp mem
+
+
+  let leq ~lhs ~rhs =
+    ConfigChecks.leq ~lhs:lhs.config_checks ~rhs:rhs.config_checks
+    && UncheckedCallees.leq ~lhs:lhs.unchecked_callees ~rhs:rhs.unchecked_callees
+    && Mem.leq ~lhs:lhs.mem ~rhs:rhs.mem
+
+
+  let join x y =
+    { config_checks= ConfigChecks.join x.config_checks y.config_checks
+    ; unchecked_callees= UncheckedCallees.join x.unchecked_callees y.unchecked_callees
+    ; mem= Mem.join x.mem y.mem }
+
+
+  let widen ~prev ~next ~num_iters =
+    { config_checks= ConfigChecks.widen ~prev:prev.config_checks ~next:next.config_checks ~num_iters
+    ; unchecked_callees=
+        UncheckedCallees.widen ~prev:prev.unchecked_callees ~next:next.unchecked_callees ~num_iters
+    ; mem= Mem.widen ~prev:prev.mem ~next:next.mem ~num_iters }
+
+
+  let to_summary {unchecked_callees} = {Summary.unchecked_callees}
+
+  let init =
+    {config_checks= ConfigChecks.top; unchecked_callees= UncheckedCallees.bottom; mem= Mem.bottom}
+
+
+  let add_mem loc v ({mem} as astate) = {astate with mem= Mem.add loc v mem}
+
+  let copy_mem ~tgt ~src ({mem} as astate) = add_mem tgt (Mem.lookup src mem) astate
+
+  let call_config_check ret config astate = add_mem (Loc.of_id ret) (Val.of_config config) astate
+
+  let load_config id pvar astate = copy_mem ~tgt:(Loc.of_id id) ~src:(Loc.of_pvar pvar) astate
+
+  let store_config pvar id astate = copy_mem ~tgt:(Loc.of_pvar pvar) ~src:(Loc.of_id id) astate
+
+  let boolean_value id_tgt id_src astate =
+    copy_mem ~tgt:(Loc.of_id id_tgt) ~src:(Loc.of_id id_src) astate
+
+
+  let neg_branch res = Option.map ~f:(fun (config, branch) -> (config, Branch.neg branch)) res
+
+  let rec get_config_check_prune e mem =
+    match (e : Exp.t) with
+    | Var id ->
+        Mem.lookup (Loc.of_id id) mem
+        |> Val.get_config_opt
+        |> Option.map ~f:(fun config -> (config, Branch.True))
+    | UnOp (LNot, e, _) ->
+        get_config_check_prune e mem |> neg_branch
+    | BinOp ((Eq | Ne), Const _, Const _) ->
+        None
+    | (BinOp (Eq, e, (Const _ as const)) | BinOp (Eq, (Const _ as const), e)) when Exp.is_zero const
+      ->
+        get_config_check_prune e mem |> neg_branch
+    | (BinOp (Ne, e, (Const _ as const)) | BinOp (Ne, (Const _ as const), e)) when Exp.is_zero const
+      ->
+        get_config_check_prune e mem
+    | _ ->
+        None
+
+
+  let prune e ({config_checks; mem} as astate) =
+    get_config_check_prune e mem
+    |> Option.value_map ~default:astate ~f:(fun (config, branch) ->
+           {astate with config_checks= ConfigChecks.add config branch config_checks} )
+
+
+  let call callee location ({config_checks; unchecked_callees} as astate) =
+    if ConfigChecks.is_top config_checks then
+      let unchecked_callee = UncheckedCallee.{callee; location} in
+      {astate with unchecked_callees= UncheckedCallees.add unchecked_callee unchecked_callees}
+    else astate
+end
+
+module TransferFunctions = struct
+  module CFG = ProcCfg.NormalOneInstrPerNode
+  module Domain = Dom
+
+  type analysis_data = Summary.t InterproceduralAnalysis.t
+
+  let is_java_boolean_value_method pname =
+    Procname.get_class_name pname |> Option.exists ~f:(String.equal "java.lang.Boolean")
+    && Procname.get_method pname |> String.equal "booleanValue"
+
+
+  let exec_instr astate {InterproceduralAnalysis.tenv} _node instr =
+    match (instr : Sil.instr) with
+    | Load {id; e= Lvar pvar} ->
+        Dom.load_config id pvar astate
+    | Store {e1= Lvar pvar; e2= Var id} ->
+        Dom.store_config pvar id astate
+    | Call ((ret, _), Const (Cfun callee), [(Var id, _)], _, _)
+      when is_java_boolean_value_method callee ->
+        Dom.boolean_value ret id astate
+    | Call ((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
+      | Some (`Exp _) ->
+          astate
+      | None ->
+          (* normal function calls *)
+          Dom.call callee location astate )
+    | Prune (e, _, _, _) ->
+        Dom.prune e astate
+    | _ ->
+        astate
+
+
+  let pp_session_name node fmt =
+    F.fprintf fmt "Config impact function calls %a" CFG.Node.pp_id (CFG.Node.id node)
+end
+
+module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions)
+
+let checker ({InterproceduralAnalysis.proc_desc} as analysis_data) =
+  Option.map (Analyzer.compute_post analysis_data ~initial:Dom.init proc_desc)
+    ~f:(fun ({Dom.unchecked_callees} as astate) ->
+      UncheckedCallees.report analysis_data unchecked_callees ;
+      Dom.to_summary astate )

infer/src/checkers/ConfigImpactAnalysis.mli

@@ -0,0 +1,16 @@
+(*
+ * 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 Summary : sig
+  type t
+
+  val pp : Format.formatter -> t -> unit
+end
+
+val checker : Summary.t InterproceduralAnalysis.t -> Summary.t option

infer/tests/codetoanalyze/java/fb-config-impact/Makefile

@@ -0,0 +1,13 @@
+# 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.
+
+TESTS_DIR = ../../..
+
+INFER_OPTIONS = --config-impact-analysis-only --debug-exceptions \
+  --report-force-relative-path
+INFERPRINT_OPTIONS = --issues-tests
+SOURCES = $(wildcard *.java)
+
+include $(TESTS_DIR)/javac.make