[transitive-access] allow to skip some transitive accesses

Summary:
Some transitive accesses are detected while we don't want to report them if they go through specific method calls.

In practice, the corresponding calls are virtual, then the matcher can no really take into account procname's classname, because the Hack frontend generates virtual call of the form
```
n2 = n0.?.skip_me(n1)
```
The matcher uses then only a list of string.

Facebook We will use this feature to skip all calls like `$event->genCountryISOFromBatch()`

Reviewed By: geralt-encore

Differential Revision:
D55867074

Privacy Context Container: L1208441

fbshipit-source-id: 0ecc9cd636499c30ff46fc30da0f317318ca008f

infer/src/pulse/PulseCallOperations.ml

@@ -431,8 +431,10 @@ let call_aux tenv path caller_proc_desc call_loc callee_pname ret actuals call_k
  L.d_printfln "Will keep at most one disjunct because %a is in block list" Procname.pp
  callee_pname ;
  (* we propagate transitive accesses from callee to caller using *)
+ let skip_transitive_accesses = PulseTransitiveAccessChecker.should_skip_call callee_pname in
  let non_disj =
- NonDisjDomain.apply_summary ~callee_pname ~call_loc non_disj_caller non_disj_callee
+ NonDisjDomain.apply_summary ~callee_pname ~call_loc ~skip_transitive_accesses non_disj_caller
+ non_disj_callee
  in
  (* call {!AbductiveDomain.PrePost.apply} on each pre/post pair in the summary. *)
  let posts, contradiction =

infer/src/pulse/PulseInterproc.ml

@@ -951,11 +951,13 @@ let record_skipped_calls callee_proc_name call_loc callee_summary call_state =
 
 
 let record_transitive_info callee_proc_name call_location callee_summary call_state =
- let astate =
- AbductiveDomain.transfer_transitive_info_to_caller callee_proc_name call_location callee_summary
- call_state.astate
- in
- {call_state with astate}
+ if PulseTransitiveAccessChecker.should_skip_call callee_proc_name then call_state
+ else
+ let astate =
+ AbductiveDomain.transfer_transitive_info_to_caller callee_proc_name call_location
+ callee_summary call_state.astate
+ in
+ {call_state with astate}
 
 
 let apply_unknown_effects callee_summary call_state =

infer/src/pulse/PulseNonDisjunctiveDomain.ml

@@ -636,12 +636,16 @@ module InterDom = struct
 
  let remember_dropped_elements dropped = map (TransitiveInfo.remember_dropped_elements ~dropped)
 
- let apply_summary ~callee_pname ~call_loc ~summary non_disj =
+ let apply_summary ~callee_pname ~call_loc ~summary ~skip_transitive_accesses non_disj =
  match (non_disj, summary) with
  | Top, _ | _, Top ->
  Top
  | NonTop non_disj, NonTop summary ->
- NonTop (TransitiveInfo.apply_summary ~callee_pname ~call_loc ~summary non_disj)
+ let non_disj =
+ if skip_transitive_accesses then non_disj
+ else TransitiveInfo.apply_summary ~callee_pname ~call_loc ~summary non_disj
+ in
+ NonTop non_disj
 end
 
 type t = {intra: IntraDom.t; inter: InterDom.t} [@@deriving abstract_domain]
@@ -728,8 +732,10 @@ type summary = InterDom.t [@@deriving abstract_domain]
 
 let make_summary {inter} = inter
 
-let apply_summary ~callee_pname ~call_loc non_disj summary =
- map_inter (InterDom.apply_summary ~callee_pname ~call_loc ~summary) non_disj
+let apply_summary ~callee_pname ~call_loc ~skip_transitive_accesses non_disj summary =
+ map_inter
+ (InterDom.apply_summary ~callee_pname ~call_loc ~skip_transitive_accesses ~summary)
+ non_disj
 
 
 module Summary = struct

infer/src/pulse/PulseNonDisjunctiveDomain.mli

@@ -100,7 +100,13 @@ val is_lifetime_extended : Var.t -> t -> bool
 
 val remember_dropped_elements : TransitiveInfo.t -> t -> t
 
-val apply_summary : callee_pname:Procname.t -> call_loc:Location.t -> t -> summary -> t
+val apply_summary :
+ callee_pname:Procname.t
+ -> call_loc:Location.t
+ -> skip_transitive_accesses:bool
+ -> t
+ -> summary
+ -> t
 
 val bind : 'a list * t -> f:('a -> t -> 'b list * t) -> 'b list * t
 (** {[

infer/src/pulse/PulseTransitiveAccessChecker.ml

@@ -17,6 +17,8 @@ module Config : sig
 
  val procname_must_be_monitored : Tenv.t -> Procname.t -> bool
 
+ val procname_must_be_skipped : Procname.t -> bool
+
  type context_metadata = {description: string; tag: string}
 
  val find_matching_context : Tenv.t -> Procname.t -> context_metadata option
@@ -25,7 +27,7 @@ end = struct
 
  let regexp_type_of_yojson json = Str.regexp (string_of_yojson json)
 
- type procname_to_monitor =
+ type procname_match_spec =
  { class_names: string list option [@yojson.option]
  ; method_names: string list option [@yojson.option]
  ; class_name_regex: regexp_type option [@yojson.option]
@@ -44,7 +46,7 @@ end = struct
  type t =
  { fieldnames_to_monitor: string list
  ; procnames_to_skip: string list option [@yojson.option]
- ; procnames_to_monitor: procname_to_monitor list
+ ; procnames_to_monitor: procname_match_spec list
  ; contexts: context list }
  [@@deriving of_yojson]
 
@@ -73,6 +75,16 @@ end = struct
  List.exists annotations ~f:match_one_annotation
 
 
+ let procname_must_be_skipped procname =
+ match get () with
+ | None ->
+ false
+ | Some {procnames_to_skip} ->
+ Option.exists procnames_to_skip ~f:(fun names ->
+ let method_name = Procname.get_method procname in
+ List.exists names ~f:(String.equal method_name) )
+
+
  let procname_must_be_monitored tenv procname =
  let class_name = Procname.get_class_type_name procname in
  let method_name = Procname.get_method procname in
@@ -229,6 +241,8 @@ let record_call tenv procname location astate =
  else astate
 
 
+let should_skip_call procname = Config.procname_must_be_skipped procname
+
 let report_errors tenv proc_desc err_log {PulseSummary.pre_post_list; non_disj} =
  let procname = Procdesc.get_proc_name proc_desc in
  match Config.find_matching_context tenv procname with

infer/src/pulse/PulseTransitiveAccessChecker.mli

@@ -7,6 +7,8 @@
 
 open! IStd
 
+val should_skip_call : Procname.t -> bool
+
 val record_load : Exp.t -> Location.t -> PulseExecutionDomain.t list -> PulseExecutionDomain.t list
 
 val record_call :

infer/tests/codetoanalyze/hack/pulse/global_access.hack

@@ -37,6 +37,20 @@ class Parent1 extends Parent2 {
 
 class ExtendsVeryUnsafe extends VeryUnsafe {}
 
+class B {
+ public function skip_me(A $a): int {
+ return $a->get();
+ }
+
+ public function skip_me_too(A $a): int {
+ return $a->get();
+ }
+
+ public function dont_skip_me(A $a): int {
+ return $a->get();
+ }
+}
+
 final class GlobalAccess extends Parent1 {
 
  public function basic_is_entry_bad(A $a): int {
@@ -51,6 +65,18 @@ final class GlobalAccess extends Parent1 {
  return $a->get();
  }
 
+ public function indirect_skip_me_is_entry_ok(A $a, B $b): int {
+ return $b->skip_me($a);
+ }
+
+ public function indirect_skip_me_too_is_entry_ok(A $a, B $b): int {
+ return $b->skip_me($a);
+ }
+
+ public function indirect_dont_skip_me_is_entry_bad(A $a, B $b): int {
+ return $b->dont_skip_me($a);
+ }
+
  public function indirect_is_entry_two_signals_bad(A $a1, A $a2): int {
  $_ = $a1->get();
  return $a2->get();

infer/tests/codetoanalyze/hack/pulse/issues.exp

@@ -131,6 +131,7 @@ genruntest.hack, GenRunTest::GenRunTest$static.inGeneralCaseBad, 6, PULSE_UNAWAI
 global_access.hack, GlobalAccess::GlobalAccess.basic_is_entry_bad, 1, PULSE_TRANSITIVE_ACCESS, no_bucket, ERROR, [access occurs here]
 global_access.hack, GlobalAccess::GlobalAccess.indirect_is_entry_bad, 1, PULSE_TRANSITIVE_ACCESS, no_bucket, ERROR, [when calling `GlobalAccess::A.get` here,access occurs here], {GlobalAccess::Unknown}
 global_access.hack, GlobalAccess::GlobalAccess.indirect_other_is_entry_bad, 1, PULSE_TRANSITIVE_ACCESS, no_bucket, ERROR, [when calling `GlobalAccess::A.get` here,access occurs here], {GlobalAccess::Unknown}
+global_access.hack, GlobalAccess::GlobalAccess.indirect_dont_skip_me_is_entry_bad, 1, PULSE_TRANSITIVE_ACCESS, no_bucket, ERROR, [when calling `GlobalAccess::B.dont_skip_me` here,when calling `GlobalAccess::A.get` here,access occurs here], {GlobalAccess::Unknown}
 global_access.hack, GlobalAccess::GlobalAccess.indirect_is_entry_two_signals_bad, 1, PULSE_TRANSITIVE_ACCESS, no_bucket, ERROR, [when calling `GlobalAccess::A.get` here,access occurs here], {GlobalAccess::Unknown}
 global_access.hack, GlobalAccess::GlobalAccess.indirect_is_entry_two_signals_bad, 2, PULSE_TRANSITIVE_ACCESS, no_bucket, ERROR, [when calling `GlobalAccess::A.get` here,access occurs here], {GlobalAccess::Unknown}
 global_access.hack, GlobalAccess::GlobalAccess.FN_indirect_is_entry_calls_double_get_two_signals_bad, 3, PULSE_TRANSITIVE_ACCESS, no_bucket, ERROR, [when calling `GlobalAccess::A.double_get` here,when calling `GlobalAccess::A.get` here,access occurs here], {GlobalAccess::Unknown}

infer/tests/codetoanalyze/hack/pulse/transitive-access-config.json

@@ -13,6 +13,7 @@
  },
  { "class_name_regex": "BadPattern" }
  ],
+ "procnames_to_skip": ["skip_me", "skip_me_too"],
  "contexts": [
  {
  "initial_caller_class_extends": ["GlobalAccess::EventHandler"],