diff --git a/infer/src/checkers/SinkTrace.ml b/infer/src/checkers/SinkTrace.ml index bdf332d1a..e04de2fd1 100644 --- a/infer/src/checkers/SinkTrace.ml +++ b/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.S + + (** a path from some procedure via the given passthroughs to the given call stack, with + 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 + +module Make (TraceElem : TraceElem.S) = struct include Trace.Make(struct module Source = Source.Dummy - module Sink = struct - include Spec - let get _ _ = [] - end - + 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 diff --git a/infer/src/checkers/SinkTrace.mli b/infer/src/checkers/SinkTrace.mli index dbf0a53f0..744da18c5 100644 --- a/infer/src/checkers/SinkTrace.mli +++ b/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) : - Trace.S with module Source = Source.Dummy - and module Sink.Kind = Spec.Kind - and type Sink.t = Spec.t +(** Suffix of a normal trace: just sinks and passthroughs, but no sources *) +module type S = sig + include Trace.S + + (** 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) diff --git a/infer/src/checkers/siof.ml b/infer/src/checkers/Siof.ml similarity index 66% rename from infer/src/checkers/siof.ml rename to infer/src/checkers/Siof.ml index 3dc2cda27..85f6cb91c 100644 --- a/infer/src/checkers/siof.ml +++ b/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 - @@ Summary.read_summary tenv pdesc callee_pname in + match Summary.read_summary tenv pdesc callee_pname with + | 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 pp_desc fmt () = - let pp_var f v = - 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 "" - | None -> - Format.fprintf f "" - | Some source_file -> - Format.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 pp_set f s = pp_seq pp_var f s in - Format.fprintf fmt - "This global variable initializer accesses the following globals in another translation \ - unit: %a" - pp_set bad_globals in - let description = pp_to_string pp_desc () in - let exn = Exceptions.Checkers - ("STATIC_INITIALIZATION_ORDER_FIASCO", Localise.verbatim_desc description) in - Reporting.log_error pname ~loc exn - - -let siof_check pdesc = function +let report_siof trace pdesc tenv loc = + let trace_of_pname pname = + 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 -> + F.fprintf f "" + | None -> + F.fprintf f "" + | Some source_file -> + F.fprintf f " from file %s" (DB.source_file_to_string source_file) in + let v = SiofTrace.Sink.kind sink in + F.fprintf f "%s%a" (Pvar.get_simplified_name v) pp_source v in + + 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. %a" + pp_sink final_sink + 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 caller_pname ~loc exn in + + IList.iter report_one_path (SiofTrace.get_reportable_sink_paths trace ~trace_of_pname) + +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 - 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; + then report_siof post pdesc tenv attrs.ProcAttributes.loc; | Some SiofDomain.Bottom | None -> () -let checker callback = - let pdesc = callback.Callbacks.proc_desc in +let checker ({ Callbacks.tenv; proc_desc } as callback) = 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 | _ -> () diff --git a/infer/src/checkers/SiofTrace.mli b/infer/src/checkers/SiofTrace.mli index de99f9750..7fac6e2db 100644 --- a/infer/src/checkers/SiofTrace.mli +++ b/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 diff --git a/infer/src/checkers/Source.mli b/infer/src/checkers/Source.mli index f9ac628a9..4916bb67d 100644 --- a/infer/src/checkers/Source.mli +++ b/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 diff --git a/infer/src/checkers/Trace.ml b/infer/src/checkers/Trace.ml index f1db3ab07..f8e6e4ff0 100644 --- a/infer/src/checkers/Trace.ml +++ b/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 diff --git a/infer/src/checkers/Trace.mli b/infer/src/checkers/Trace.mli index e3f863591..21ccdaecc 100644 --- a/infer/src/checkers/Trace.mli +++ b/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 diff --git a/infer/tests/codetoanalyze/cpp/checkers/issues.exp b/infer/tests/codetoanalyze/cpp/checkers/issues.exp index c8b3765d7..7d260e6df 100644 --- a/infer/tests/codetoanalyze/cpp/checkers/issues.exp +++ b/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/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/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/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: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: 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. diff --git a/infer/tests/codetoanalyze/cpp/checkers/siof/pod_across_translation_units-1.cpp b/infer/tests/codetoanalyze/cpp/checkers/siof/pod_across_translation_units-1.cpp index dac91c1cb..007c9d24c 100644 --- a/infer/tests/codetoanalyze/cpp/checkers/siof/pod_across_translation_units-1.cpp +++ b/infer/tests/codetoanalyze/cpp/checkers/siof/pod_across_translation_units-1.cpp @@ -8,7 +8,8 @@ */ extern int foo(); - -static int x = foo(); // BAD: report SIOF here +int bar() { return foo(); } +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