[dispatch-once-static-init] Improve domain with traces

Summary:
We keep a list of traces as the main element of the domain. We only add an element to the domain when there is a call to `dispatch_once` or when the summary is non-empty.
Once we report an issue (only in a static initializer), we then don't keep track of the calls in the summary anymore.

Reviewed By: jvillard

Differential Revision: D66537767

fbshipit-source-id: 68ac7560cb0507ea05d99763fa2b63169fac666e

infer/src/backend/Payloads.ml

@@ -65,7 +65,8 @@ let all_fields =
     ~config_impact_analysis:(fun f -> mk f ConfigImpactAnalysis ConfigImpactAnalysis.Summary.pp)
     ~cost:(fun f -> mk f Cost CostDomain.pp_summary)
     ~disjunctive_demo:(fun f -> mk f DisjunctiveDemo DisjunctiveDemo.pp_domain)
-    ~dispatch_once_static_init:(fun f -> mk f DisjunctiveDemo DispatchOnceStaticInit.Summary.pp)
+    ~dispatch_once_static_init:(fun f ->
+      mk f DispatchOnceStaticInit DispatchOnceStaticInit.Summary.pp )
     ~litho_required_props:(fun f -> mk f LithoRequiredProps LithoDomain.pp_summary)
     ~pulse:(fun f -> mk_full f Pulse PulseSummary.pp)
     ~purity:(fun f -> mk f Purity PurityDomain.pp_summary)

infer/src/checkers/DispatchOnceStaticInit.ml

@@ -6,17 +6,27 @@
  *)
 open! IStd
 module F = Format
+module L = Logging
 
 module Mem = struct
-  type t = {loc: Location.t} [@@deriving compare]
+  type kind = [`Call | `Dispatch_once] [@@deriving compare, equal]
 
-  let pp fmt {loc} = F.fprintf fmt "calls_dispatch_once at %a" Location.pp loc
+  type trace_elem = {call_site: CallSite.t; kind: kind} [@@deriving compare, equal]
+
+  type t = trace_elem list [@@deriving compare, equal]
+
+  let pp_trace_elem fmt {call_site} = F.fprintf fmt "call to %a" CallSite.pp call_site
+
+  let pp fmt trace = (Pp.comma_seq pp_trace_elem) fmt trace
+
+  let add_call call trace_elems = call :: trace_elems
 end
 
 module Summary = struct
   include AbstractDomain.FiniteSet (Mem)
 
-  let add_calls_dispatch_once loc astate = add {Mem.loc} astate
+  let add_call t astate =
+    if is_empty astate then add (Mem.add_call t []) astate else map (Mem.add_call t) astate
 end
 
 module TransferFunctions = struct
@@ -31,33 +41,48 @@ module TransferFunctions = struct
       (instr : Sil.instr) =
     match instr with
     | Call (_, Exp.Const (Const.Cfun procname), _, loc, _) ->
+        let call_site = CallSite.make procname loc in
         let astate =
           match analyze_dependency procname with
           | Ok summary ->
-              Summary.join astate summary
+              L.d_printf "Applying summary of callee `%a`@\n" Procname.pp procname ;
+              L.d_printf "Summary: %a @\n" Summary.pp summary ;
+              if not (Summary.is_empty summary) then
+                let astate' = Summary.add_call {call_site; kind= `Call} summary in
+                Summary.join astate astate'
+              else astate
           | Error _ ->
               astate
         in
         let calls_dispatch_once = String.equal "_dispatch_once" (Procname.get_method procname) in
-        if calls_dispatch_once then Summary.add_calls_dispatch_once loc astate else astate
+        if calls_dispatch_once then Summary.add_call {call_site; kind= `Dispatch_once} astate
+        else astate
     | _ ->
         astate
 end
 
 module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions)
 
-let make_trace loc =
-  let trace_elem = Errlog.make_trace_element 0 loc "Call to `dispatch_once` here" [] in
-  [trace_elem]
+let make_trace trace_elems {Mem.call_site} =
+  let loc = CallSite.loc call_site in
+  let procname = CallSite.pname call_site in
+  let trace_elem =
+    let s = F.asprintf "Call to %a" Procname.pp procname in
+    Errlog.make_trace_element 0 loc s []
+  in
+  trace_elem :: trace_elems
 
 
-let report_issue proc_desc err_log {Mem.loc} =
-  let ltr = make_trace loc in
+let report_issue proc_desc err_log trace_elems =
+  let ltr = List.fold ~f:make_trace ~init:[] trace_elems |> List.rev in
+  let {Mem.call_site; kind} = List.hd_exn trace_elems in
+  let loc = CallSite.loc call_site in
+  let kind_to_string kind = match kind with `Call -> "an indirect" | `Dispatch_once -> "a" in
   let message =
     F.asprintf
-      "There is a call to `disptach_once` at line %a from a static constructor. This could cause a \
-       deadlock."
-      Location.pp loc
+      "There is %s call to `disptach_once` at line %a from a static constructor. This could cause \
+       a deadlock."
+      (kind_to_string kind) Location.pp loc
   in
   Reporting.log_issue proc_desc err_log ~ltr ~loc DispatchOnceStaticInit
     IssueType.dispatch_once_in_static_init message
@@ -67,14 +92,15 @@ let checker ({InterproceduralAnalysis.proc_desc; err_log} as analysis_data) =
   let attributes = Procdesc.get_attributes proc_desc in
   let initial = Summary.empty in
   let summary_opt = Analyzer.compute_post analysis_data ~initial proc_desc in
-  ( if attributes.ProcAttributes.is_static_ctor then
-      match summary_opt with
-      | Some domain -> (
-        match Summary.choose_opt domain with
-        | Some mem ->
-            report_issue proc_desc err_log mem
-        | None ->
-            () )
+  if attributes.ProcAttributes.is_static_ctor then
+    match summary_opt with
+    | Some domain -> (
+      match Summary.choose_opt domain with
+      | Some mem ->
+          report_issue proc_desc err_log mem ;
+          None
       | None ->
-          () ) ;
-  summary_opt
+          summary_opt )
+    | None ->
+        summary_opt
+  else summary_opt

infer/src/checkers/DispatchOnceStaticInit.mli

@@ -8,7 +8,11 @@ open! IStd
 module F = Format
 
 module Mem : sig
-  type t = {loc: Location.t}
+  type kind = [`Call | `Dispatch_once] [@@deriving compare, equal]
+
+  type trace_elem = {call_site: CallSite.t; kind: kind} [@@deriving compare, equal]
+
+  type t = trace_elem list [@@deriving compare, equal]
 
   val compare : t -> t -> int
 

infer/tests/codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m

@@ -23,14 +23,23 @@ + (instancetype)getInstance {
 
 @end
 
-__attribute__((constructor)) static void initializer_test_interproc_bad(void) {
+__attribute__((constructor)) static void initializer_test_interproc_bad() {
   [Manager getInstance];
 }
 
-__attribute__((constructor)) static void initializer_test_intraproc_bad(void) {
+void foo_good() { initializer_test_interproc_bad(); }
+
+__attribute__((constructor)) static void initializer_test_intraproc_bad() {
   static Manager* manager;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
     manager = [Manager new];
   });
 }
+
+__attribute__((constructor)) static void
+initializer_test_interproc_condition_bad(BOOL flag) {
+  if (flag) {
+    [Manager getInstance];
+  }
+}

infer/tests/codetoanalyze/objc/dispatch-once-static-init/issues.exp

@@ -1,2 +1,3 @@
-codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m, initializer_test_interproc_bad, -8, DISPATCH_ONCE_IN_STATIC_INIT, no_bucket, ERROR, [macro expanded here,Call to `dispatch_once` here]
-codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m, initializer_test_intraproc_bad, 3, DISPATCH_ONCE_IN_STATIC_INIT, no_bucket, ERROR, [macro expanded here,Call to `dispatch_once` here]
+codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m, initializer_test_interproc_bad, 1, DISPATCH_ONCE_IN_STATIC_INIT, no_bucket, ERROR, [Call to Manager.getInstance,macro expanded here,Call to _dispatch_once]
+codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m, initializer_test_intraproc_bad, 3, DISPATCH_ONCE_IN_STATIC_INIT, no_bucket, ERROR, [macro expanded here,Call to _dispatch_once]
+codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m, initializer_test_interproc_condition_bad, 3, DISPATCH_ONCE_IN_STATIC_INIT, no_bucket, ERROR, [Call to Manager.getInstance,macro expanded here,Call to _dispatch_once]