From 204ba90e24a458d39e33248ee909e2662412a1de Mon Sep 17 00:00:00 2001 From: David Pichardie Date: Wed, 31 Jul 2024 03:47:53 -0700 Subject: [PATCH] [hack][stats] count unknown calls on resources Summary: we only increment the counter when we detect a resource attribute is 'washed' because of an unknown call Reviewed By: geralt-encore Differential Revision: D60456603 Privacy Context Container: L1122176 fbshipit-source-id: 2cf2b61fd9b6322309b3b56c3ca509d0328824b9 --- infer/src/base/Stats.ml | 8 +++++++- infer/src/base/Stats.mli | 2 ++ infer/src/pulse/PulseAbductiveDomain.ml | 4 ++++ infer/src/pulse/PulseAbductiveDomain.mli | 2 ++ infer/src/pulse/PulseAttribute.ml | 16 ++++++++++++++++ infer/src/pulse/PulseAttribute.mli | 2 ++ infer/src/pulse/PulseCallOperations.ml | 14 ++++++++++++-- .../codetoanalyze/hack/pulse/hh/unknown.hack | 1 + .../tests/codetoanalyze/hack/pulse/unknown.hack | 8 ++++++++ 9 files changed, 54 insertions(+), 3 deletions(-) diff --git a/infer/src/base/Stats.ml b/infer/src/base/Stats.ml index 814ed6b3c2d..460f2952dd9 100644 --- a/infer/src/base/Stats.ml +++ b/infer/src/base/Stats.ml @@ -173,6 +173,7 @@ type t = ; mutable pulse_disjuncts_dropped: IntCounter.t ; mutable pulse_interrupted_loops: IntCounter.t ; mutable pulse_unknown_calls: IntCounter.t + ; mutable pulse_unknown_calls_on_hack_resource: IntCounter.t ; mutable pulse_summaries_contradictions: IntCounter.t ; mutable pulse_summaries_unsat_for_caller: IntCounter.t ; mutable pulse_summaries_unsat_for_caller_percent: IntCounter.t @@ -262,7 +263,8 @@ let pp fmt stats = ~pulse_args_length_contradictions:(pp_int_field fmt) ~pulse_captured_vars_length_contradictions:(pp_int_field fmt) ~pulse_disjuncts_dropped:(pp_int_field fmt) ~pulse_interrupted_loops:(pp_int_field fmt) - ~pulse_unknown_calls:(pp_int_field fmt) ~pulse_summaries_contradictions:(pp_int_field fmt) + ~pulse_unknown_calls:(pp_int_field fmt) ~pulse_unknown_calls_on_hack_resource:(pp_int_field fmt) + ~pulse_summaries_contradictions:(pp_int_field fmt) ~pulse_summaries_count:(pp_pulse_summaries_count fmt) ~pulse_summaries_count_0_continue_program:(pp_int_field fmt) ~pulse_summaries_count_0_percent:(pp_int_field fmt) @@ -287,6 +289,7 @@ let log_to_file ; pulse_disjuncts_dropped ; pulse_interrupted_loops ; pulse_unknown_calls + ; pulse_unknown_calls_on_hack_resource ; pulse_summaries_contradictions ; pulse_summaries_unsat_for_caller ; pulse_summaries_unsat_for_caller_percent @@ -309,6 +312,7 @@ let log_to_file F.fprintf fmt "pulse_disjuncts_dropped: %d@\n" pulse_disjuncts_dropped ; F.fprintf fmt "pulse_interrupted_loops: %d@\n" pulse_interrupted_loops ; F.fprintf fmt "pulse_unknown_calls: %d@\n" pulse_unknown_calls ; + F.fprintf fmt "pulse_unknown_calls_on_hack_resource: %d@\n" pulse_unknown_calls_on_hack_resource ; F.fprintf fmt "pulse_summaries_contradictions: %d@\n" pulse_summaries_contradictions ; F.fprintf fmt "pulse_summaries_unsat_for_caller: %d@\n" pulse_summaries_unsat_for_caller ; F.fprintf fmt "pulse_summaries_with_some_unreachable_nodes: %d@\n" @@ -421,6 +425,8 @@ let add_pulse_interrupted_loops n = add Fields.pulse_interrupted_loops n let incr_pulse_unknown_calls () = incr Fields.pulse_unknown_calls +let incr_pulse_unknown_calls_on_hack_resource () = incr Fields.pulse_unknown_calls_on_hack_resource + let incr_pulse_summaries_contradictions () = incr Fields.pulse_summaries_contradictions let incr_pulse_summaries_unsat_for_caller () = incr Fields.pulse_summaries_unsat_for_caller diff --git a/infer/src/base/Stats.mli b/infer/src/base/Stats.mli index b98f8943cbd..15c2be89fa5 100644 --- a/infer/src/base/Stats.mli +++ b/infer/src/base/Stats.mli @@ -42,6 +42,8 @@ val add_pulse_interrupted_loops : int -> unit val incr_pulse_unknown_calls : unit -> unit +val incr_pulse_unknown_calls_on_hack_resource : unit -> unit + val incr_pulse_summaries_contradictions : unit -> unit val incr_pulse_summaries_unsat_for_caller : unit -> unit diff --git a/infer/src/pulse/PulseAbductiveDomain.ml b/infer/src/pulse/PulseAbductiveDomain.ml index f38080f9ef4..b24a6466bd8 100644 --- a/infer/src/pulse/PulseAbductiveDomain.ml +++ b/infer/src/pulse/PulseAbductiveDomain.ml @@ -2358,6 +2358,10 @@ module AddressAttributes = struct add_static_type tenv typ (CanonValue.canon' astate v) location astate + let get_allocation_attr v astate = + SafeAttributes.get_allocation (CanonValue.canon' astate v) astate + + let remove_allocation_attr v astate = SafeAttributes.remove_allocation_attr (CanonValue.canon' astate v) astate diff --git a/infer/src/pulse/PulseAbductiveDomain.mli b/infer/src/pulse/PulseAbductiveDomain.mli index 2728f9d8723..41e743c940d 100644 --- a/infer/src/pulse/PulseAbductiveDomain.mli +++ b/infer/src/pulse/PulseAbductiveDomain.mli @@ -221,6 +221,8 @@ module AddressAttributes : sig val remove_taint_attrs : AbstractValue.t -> t -> t + val get_allocation_attr : AbstractValue.t -> t -> (Attribute.allocator * Trace.t) option + val get_static_type : AbstractValue.t -> t -> Typ.Name.t option val get_closure_proc_name : AbstractValue.t -> t -> Procname.t option diff --git a/infer/src/pulse/PulseAttribute.ml b/infer/src/pulse/PulseAttribute.ml index cc294a15dc6..831d9e77244 100644 --- a/infer/src/pulse/PulseAttribute.ml +++ b/infer/src/pulse/PulseAttribute.ml @@ -663,6 +663,22 @@ module Attribute = struct false + let is_hack_resource allocator = + match allocator with + | CMalloc + | CustomMalloc _ + | CRealloc + | CustomRealloc _ + | CppNew + | CppNewArray + | ObjCAlloc + | JavaResource _ + | CSharpResource _ -> + false + | HackAsync | HackBuilderResource _ -> + true + + let filter_unreachable subst f_keep attr = let filter_aux things ~get_addr ~set_addr = let module Hashtbl = Stdlib.Hashtbl in diff --git a/infer/src/pulse/PulseAttribute.mli b/infer/src/pulse/PulseAttribute.mli index ee3c436d64f..61489503b97 100644 --- a/infer/src/pulse/PulseAttribute.mli +++ b/infer/src/pulse/PulseAttribute.mli @@ -33,6 +33,8 @@ type allocator = val pp_allocator : F.formatter -> allocator -> unit +val is_hack_resource : allocator -> bool + (** Describes the source of taint in taint propagation. NOTE: [history] is ignored in equality and comparison. *) diff --git a/infer/src/pulse/PulseCallOperations.ml b/infer/src/pulse/PulseCallOperations.ml index 1c225eded9c..c2f8fdddc63 100644 --- a/infer/src/pulse/PulseCallOperations.ml +++ b/infer/src/pulse/PulseCallOperations.ml @@ -167,8 +167,18 @@ let unknown_call tenv ({PathContext.timestamp} as path) call_loc (reason : CallE (Attributes.singleton (UnknownEffect (reason, hist))) astate in - fold_on_reachable_from_arg astate (fun reachable_actual -> - AddressAttributes.remove_allocation_attr reachable_actual ) + let some_resource_found, astate = + fold_on_reachable_from_arg (false, astate) + (fun reachable_actual (some_resource_found, astate) -> + let some_resource_found = + some_resource_found + || AddressAttributes.get_allocation_attr reachable_actual astate + |> Option.exists ~f:(fun (attr, _) -> Attribute.is_hack_resource attr) + in + (some_resource_found, AddressAttributes.remove_allocation_attr reachable_actual astate) ) + in + if some_resource_found then Stats.incr_pulse_unknown_calls_on_hack_resource () ; + astate in let add_skipped_proc astate = let** astate, f = diff --git a/infer/tests/codetoanalyze/hack/pulse/hh/unknown.hack b/infer/tests/codetoanalyze/hack/pulse/hh/unknown.hack index 4707b132d20..c3038aee8e1 100644 --- a/infer/tests/codetoanalyze/hack/pulse/hh/unknown.hack +++ b/infer/tests/codetoanalyze/hack/pulse/hh/unknown.hack @@ -9,4 +9,5 @@ class Unknown {} class UnknownClass { public static function explicitSinkAllArgs(SensitiveClass $sc): void {} + public static function mayAwait(Awaitable $arg): void {} } diff --git a/infer/tests/codetoanalyze/hack/pulse/unknown.hack b/infer/tests/codetoanalyze/hack/pulse/unknown.hack index ae2b238d227..6093afbaf2e 100644 --- a/infer/tests/codetoanalyze/hack/pulse/unknown.hack +++ b/infer/tests/codetoanalyze/hack/pulse/unknown.hack @@ -20,3 +20,11 @@ function basicFlowReturnBad(Unknown $sc): void { $res = $tainted->myUnknownFun(); UnknownClass::explicitSinkAllArgs($res); } + +async function genAndUnknownOk(): Awaitable { + $x = async { + return 42; + }; + UnknownClass::mayAwait($x); + return; +}