[siof] add fancy interprocedural reporting

Reviewed By: jvillard

Differential Revision: D4117594

fbshipit-source-id: c55cff7

infer/src/checkers/SinkTrace.ml

@@ -12,14 +12,48 @@ open! Utils
 module F = Format
 module L = Logging
 
-module Make (Spec : TraceElem.S) = struct
+module type S = sig
-  include Trace.Make(struct
+  include Trace.S
-      module Source = Source.Dummy
+
-      module Sink = struct
+  (** a path from some procedure via the given passthroughs to the given call stack, with
-        include Spec
+      passthroughs for each callee *)
+  type sink_path = Passthroughs.t * (Sink.t * Passthroughs.t) list
+
+  (** get a path for each of the reportable flows to a sink in this trace *)
+  val get_reportable_sink_paths : t -> trace_of_pname:(Procname.t -> t) -> sink_path list
+
+  (** convert of the sinks to callee sinks for the given call site *)
+  val to_callee : t -> CallSite.t -> t
+end
+
+module MakeSink(TraceElem : TraceElem.S) = struct
+  include TraceElem
   let get _ _ = []
-      end
+end
 
+module Make (TraceElem : TraceElem.S) = struct
+  include Trace.Make(struct
+      module Source = Source.Dummy
+      module Sink = MakeSink(TraceElem)
       let should_report _ _ = true
     end)
+
+  type sink_path = Passthroughs.t * (Sink.t * Passthroughs.t) list
+
+  let initial =
+    let dummy_source = () in
+    of_source dummy_source
+
+  let get_reportable_sink_paths t ~trace_of_pname =
+    IList.map
+      (fun (passthroughs, _, sinks) -> passthroughs, sinks)
+      (get_reportable_paths t ~trace_of_pname)
+
+  let to_callee t call_site =
+    IList.fold_left
+      (fun t_acc sink ->
+         let callee_sink = Sink.to_callee sink call_site in
+         add_sink callee_sink t_acc)
+      initial
+      (Sinks.elements (sinks t))
 end

infer/src/checkers/SinkTrace.mli

@@ -7,7 +7,23 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  *)
 
-module Make (Spec : TraceElem.S) :
+(** Suffix of a normal trace: just sinks and passthroughs, but no sources *)
-  Trace.S with module Source = Source.Dummy
+module type S = sig
-           and module Sink.Kind = Spec.Kind
+  include Trace.S
-           and type Sink.t = Spec.t
+
+  (** A path from some procedure via the given passthroughs to the given call stack, with
+      passthroughs for each callee *)
+  type sink_path = Passthrough.Set.t * (Sink.t * Passthrough.Set.t) list
+
+  (** get a path for each of the reportable flows to a sink in this trace *)
+  val get_reportable_sink_paths : t -> trace_of_pname:(Procname.t -> t) -> sink_path list
+
+  (** convert of the sinks to callee sinks for the given call site *)
+  val to_callee : t -> CallSite.t -> t
+end
+
+module MakeSink (TraceElem : TraceElem.S) :
+  Sink.S with module Kind = TraceElem.Kind and type t = TraceElem.t
+
+module Make (TraceElem : TraceElem.S) :
+  S with module Source = Source.Dummy and module Sink = MakeSink(TraceElem)

infer/src/checkers/Siof.ml

@@ -75,9 +75,11 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
     | Prune (exp, loc, _, _) ->
         Domain.join astate (get_globals tenv astate pdesc loc exp)
     | Call (_, Const (Cfun callee_pname), params, loc, _) ->
+        let callsite = CallSite.make callee_pname loc in
         let callee_globals =
-          Option.default Domain.initial
+          match Summary.read_summary tenv pdesc callee_pname with
-          @@ Summary.read_summary tenv pdesc callee_pname in
+          | Some (Domain.NonBottom trace) -> Domain.NonBottom (SiofTrace.to_callee trace callsite)
+          | _ -> Domain.Bottom in
         add_params_globals astate tenv pdesc loc params
         |> Domain.join callee_globals
         |>
@@ -100,29 +102,48 @@ module Analyzer =
 
 module Interprocedural = Analyzer.Interprocedural (Summary)
 
-let report_siof pname loc bad_globals =
+let report_siof trace pdesc tenv loc =
-  let pp_desc fmt () =
+  let trace_of_pname pname =
-    let pp_var f v =
+    match Summary.read_summary tenv pdesc pname with
+    | Some (SiofDomain.NonBottom summary) -> summary
+    | _ -> SiofTrace.initial in
+
+  let pp_sink f sink =
     let pp_source f v = match Pvar.get_source_file v with
       | Some source_file when DB.source_file_equal DB.source_file_empty source_file ->
-            Format.fprintf f ""
+          F.fprintf f ""
       | None ->
-            Format.fprintf f ""
+          F.fprintf f ""
       | Some source_file ->
-            Format.fprintf f " from file %s" (DB.source_file_to_string source_file) in
+          F.fprintf f " from file %s" (DB.source_file_to_string source_file) in
-      Format.fprintf f "%s%a" (Pvar.get_simplified_name v) pp_source v in
+    let v = SiofTrace.Sink.kind sink in
-    let pp_set f s = pp_seq pp_var f s in
+    F.fprintf f "%s%a" (Pvar.get_simplified_name v) pp_source v in
-    Format.fprintf fmt
+
+  let pp_path_part f path =
+    let pp_path f path =
+      let pp_sep f () = F.fprintf f " => " in
+      let pp_elem f (sink, _) =
+        CallSite.pp f (SiofTrace.Sink.call_site sink) in
+      (F.pp_print_list ~pp_sep) pp_elem f path in
+    if (IList.length path > 1)
+    then F.fprintf f "Full path: %a" pp_path path in
+
+  let report_one_path (_, path) =
+    let final_sink = fst (IList.hd path) in
+    let description =
+      F.asprintf
         "This global variable initializer accesses the following globals in another translation \
-       unit: %a"
+         unit: %a. %a"
-      pp_set bad_globals in
+        pp_sink final_sink
-  let description = pp_to_string pp_desc () in
+        pp_path_part (IList.rev path) in
+    let caller_pname = Cfg.Procdesc.get_proc_name pdesc in
     let exn = Exceptions.Checkers
         ("STATIC_INITIALIZATION_ORDER_FIASCO", Localise.verbatim_desc description) in
-  Reporting.log_error pname ~loc exn
+    Reporting.log_error caller_pname ~loc exn in
 
+  IList.iter report_one_path (SiofTrace.get_reportable_sink_paths trace ~trace_of_pname)
 
-let siof_check pdesc = function
+let siof_check pdesc tenv = function
   | Some (SiofDomain.NonBottom post) ->
       let attrs = Cfg.Procdesc.get_attributes pdesc in
       let is_orig_file f = match attrs.ProcAttributes.translation_unit with
@@ -137,21 +158,15 @@ let siof_check pdesc = function
           (fun sink -> is_foreign (SiofTrace.Sink.kind sink))
           (SiofTrace.sinks post) in
       if not (SiofTrace.Sinks.is_empty foreign_global_sinks)
-      then
+      then report_siof post pdesc tenv attrs.ProcAttributes.loc;
-        let foreign_globals =
-          IList.map
-            (fun sink -> (SiofTrace.Sink.kind sink))
-            (SiofTrace.Sinks.elements foreign_global_sinks) in
-        report_siof (Cfg.Procdesc.get_proc_name pdesc) attrs.ProcAttributes.loc foreign_globals;
   | Some SiofDomain.Bottom | None ->
       ()
 
-let checker callback =
+let checker ({ Callbacks.tenv; proc_desc } as callback) =
-  let pdesc = callback.Callbacks.proc_desc in
   let post = Interprocedural.checker callback ProcData.empty_extras in
-  let pname = Cfg.Procdesc.get_proc_name pdesc in
+  let pname = Cfg.Procdesc.get_proc_name proc_desc in
   match pname with
   | Procname.C c when Procname.is_globals_initializer c ->
-      siof_check pdesc post
+      siof_check proc_desc tenv post
   | _ ->
       ()

infer/src/checkers/SiofTrace.mli

@@ -16,4 +16,4 @@ sig
   val pp : Format.formatter -> t -> unit
 end
 
-include Trace.S with module Sink.Kind = Globals
+include SinkTrace.S with module Sink.Kind = Globals

infer/src/checkers/Source.mli

@@ -20,4 +20,4 @@ module type S = sig
   val get : CallSite.t -> t option
 end
 
-module Dummy : S
+module Dummy : S with type t = unit

infer/src/checkers/Trace.ml

@@ -75,7 +75,7 @@ module type S = sig
   val pp : F.formatter -> t -> unit
 
   (** pretty-print a path in the context of the given procname *)
-  val pp_path : F.formatter -> Procname.t -> path -> unit
+  val pp_path : Procname.t -> F.formatter -> path -> unit
 end
 
 (** Expand a trace element (i.e., a source or sink) into a list of trace elements bottoming out in
@@ -102,7 +102,7 @@ module Expander (TraceElem : TraceElem.S) = struct
           (* TODO: pick the shortest path to a sink here instead (t14242809) *)
           (* arbitrarily pick one elem and explore it further *)
           expand_ callee_elem ((elem, passthroughs) :: elems_passthroughs_acc)
-      | [] ->
+      | _ ->
           (elem, Passthrough.Set.empty) :: elems_passthroughs_acc in
     expand_ elem0 []
 end
@@ -166,7 +166,7 @@ module Make (Spec : Spec) = struct
         else acc in
       Sources.fold (fun source acc -> Sinks.fold (report_one source) t.sinks acc) t.sources []
 
-  let pp_path fmt cur_pname (cur_passthroughs, sources_passthroughs, sinks_passthroughs) =
+  let pp_path cur_pname fmt (cur_passthroughs, sources_passthroughs, sinks_passthroughs) =
     let pp_passthroughs fmt passthroughs =
       if not (Passthrough.Set.is_empty passthroughs)
       then F.fprintf fmt "(via %a)" Passthrough.Set.pp passthroughs in

infer/src/checkers/Trace.mli

@@ -75,7 +75,7 @@ module type S = sig
   val pp : F.formatter -> t -> unit
 
   (** pretty-print a path in the context of the given procname *)
-  val pp_path : F.formatter -> Procname.t -> path -> unit
+  val pp_path : Procname.t -> F.formatter -> path -> unit
 end
 
 module Make (Spec : Spec) : S with module Source = Spec.Source and module Sink = Spec.Sink

infer/tests/codetoanalyze/cpp/checkers/issues.exp

@@ -1,3 +1,4 @@
-siof/const_use.cpp:17: ERROR: STATIC_INITIALIZATION_ORDER_FIASCO This global variable initializer accesses the following globals in another translation unit: u
+siof/const_use.cpp:17: ERROR: STATIC_INITIALIZATION_ORDER_FIASCO This global variable initializer accesses the following globals in another translation unit: u. 
-siof/pod_across_translation_units-1.cpp:12: ERROR: STATIC_INITIALIZATION_ORDER_FIASCO This global variable initializer accesses the following globals in another translation unit: y from file siof/pod_across_translation_units-2.cpp
+siof/pod_across_translation_units-1.cpp:13: ERROR: STATIC_INITIALIZATION_ORDER_FIASCO This global variable initializer accesses the following globals in another translation unit: y from file siof/pod_across_translation_units-2.cpp. Full path: baz at [line 13] => bar at [line 12] => foo at [line 11]
-siof/siof_across_translation_units-1.cpp:21: ERROR: STATIC_INITIALIZATION_ORDER_FIASCO This global variable initializer accesses the following globals in another translation unit: global_object
+siof/siof_across_translation_units-1.cpp:21: ERROR: STATIC_INITIALIZATION_ORDER_FIASCO This global variable initializer accesses the following globals in another translation unit: another_global_object from file siof/siof_across_translation_units-1.cpp. 
+siof/siof_across_translation_units-1.cpp:21: ERROR: STATIC_INITIALIZATION_ORDER_FIASCO This global variable initializer accesses the following globals in another translation unit: global_object. 

infer/tests/codetoanalyze/cpp/checkers/siof/pod_across_translation_units-1.cpp

@@ -8,7 +8,8 @@
  */
 
 extern int foo();
-
+int bar() { return foo(); }
-static int x = foo(); // BAD: report SIOF here
+int baz() { return bar(); }
+static int x = baz(); // BAD: report SIOF here
 static int x1 = x; // do not report here
 static int x2 = x1; // do not report here