Add support for multiple stacktraces in '-a crashcontext'

Reviewed By: jberdine

Differential Revision: D3735011

fbshipit-source-id: 8a27a60

infer/lib/python/inferlib/analyze.py

@@ -106,13 +106,6 @@ base_group.add_argument('-nf', '--no-filtering', action='store_true',
                         help='''Also show the results from the experimental
                         checks. Warning: some checks may contain many false
                         alarms''')
-base_group.add_argument('-st', '--stacktrace',
-                        dest='stacktrace',
-                        default='',
-                        help='''File containing a JSON encoded stacktrace. For
-                        use with e.g. -a crashcontext. See
-                        tests/codetoanalyze/java/crashcontext/*.json for
-                        examples of the expected format.''')
 
 base_group.add_argument('--android-harness', action='store_true',
                         help='''[experimental] Create harness to detect bugs

infer/src/backend/DB.ml

@@ -370,15 +370,20 @@ let file_is_in_cpp_model file =
   let normalized_cpp_models_dir = filename_to_absolute Config.cpp_models_dir in
   string_is_prefix normalized_cpp_models_dir normalized_file_dir
 
-(** Return all absolute paths recursively under root_dir, matching the given
+(** Fold over all file paths recursively under [dir] which match [p]. *)
-    matcher function f *)
+let fold_paths_matching ~dir ~p ~init ~f =
-let paths_matching root_dir f =
+  let rec paths path_list dir =
-  let rec paths path_list dir = Array.fold_left
+    Array.fold_left
       (fun acc file ->
-         let p = Filename.concat dir file in
+         let path = dir // file in
-         if Sys.is_directory p then (paths acc p)
+         if Sys.is_directory path then (paths acc path)
-         else if f p then p :: acc
+         else if p path then f path acc
          else acc)
       path_list
       (Sys.readdir dir) in
-  paths [] root_dir
+  paths init dir
+
+(** Return all absolute paths recursively under root_dir, matching the given
+    matcher function p *)
+let paths_matching dir p =
+  fold_paths_matching ~dir ~p ~init:[] ~f:(fun x xs -> x :: xs)

infer/src/backend/DB.mli

@@ -168,5 +168,9 @@ val is_source_file: string -> bool
 (** Returns true if the file is a C++ model *)
 val file_is_in_cpp_model : string -> bool
 
+(** Fold over all file paths recursively under [dir] which match [p]. *)
+val fold_paths_matching :
+  dir:filename -> p:(filename -> bool) -> init:'a -> f:(filename -> 'a -> 'a) -> 'a
+
 (** Return all file paths recursively under the given directory which match the given predicate *)
 val paths_matching : string -> (string -> bool) -> string list

infer/src/backend/config.ml

@@ -1004,10 +1004,16 @@ and specs_library =
   specs_library
 
 and stacktrace =
-  CLOpt.mk_string_opt ~long:"stacktrace" ~short:"st" ~f:resolve
+  CLOpt.mk_string_opt ~long:"stacktrace" ~short:"st" ~f:resolve ~exes:CLOpt.[Toplevel]
     ~meta:"file" "File path containing a json-encoded Java crash stacktrace. Used to guide the \
                   analysis (only with '-a crashcontext').  See \
-                  tests/codetoanalyze/java/crashcontext/*.json for examples of the expected format"
+                  tests/codetoanalyze/java/crashcontext/*.json for examples of the expected format."
+
+and stacktraces_dir =
+  CLOpt.mk_string_opt ~long:"stacktraces-dir" ~f:resolve ~exes:CLOpt.[Toplevel]
+    ~meta:"dir" "Directory path containing multiple json-encoded Java crash stacktraces. \
+                 Used to guide the  analysis (only with '-a crashcontext').  See \
+                 tests/codetoanalyze/java/crashcontext/*.json for examples of the expected format."
 
 and static_final =
   CLOpt.mk_bool ~deprecated_no:["no-static_final"] ~long:"static-final" ~default:true
@@ -1406,6 +1412,7 @@ and source_file_copy = !source_file_copy
 and spec_abs_level = !spec_abs_level
 and specs_library = !specs_library
 and stacktrace = !stacktrace
+and stacktraces_dir = !stacktraces_dir
 and stats_mode = !stats
 and subtype_multirange = !subtype_multirange
 and svg = !svg

infer/src/backend/config.mli

@@ -235,6 +235,7 @@ val source_file_copy : string option
 val spec_abs_level : int
 val specs_library : string list
 val stacktrace : string option
+val stacktraces_dir : string option
 val stats_mode : bool
 val subtype_multirange : bool
 val svg : bool

infer/src/backend/crashcontext.ml

@@ -68,38 +68,72 @@ let stitch_summaries stacktrace_file summary_files out_file =
   let crashcontext = { Stacktree_j.stack = expanded_frames} in
   Ag_util.Json.to_file Stacktree_j.write_crashcontext_t out_file crashcontext
 
-let collect_all_summaries root_out_dir stacktrace_file =
+let collect_all_summaries root_summaries_dir stacktrace_file stacktraces_dir =
-  let out_dir = Filename.concat root_out_dir "crashcontext" in
-  DB.create_dir out_dir;
-  let out_file = Filename.concat out_dir "crashcontext.json" in
-  let path_regexp = Str.regexp ".*crashcontext/.*\\..*\\.json" in
-  let path_matcher path =  Str.string_match path_regexp path 0 in
   let method_summaries =
-    DB.paths_matching root_out_dir path_matcher in
+    let path_regexp = Str.regexp ".*crashcontext/.*\\..*\\.json" in
-  stitch_summaries stacktrace_file method_summaries out_file
+    let path_matcher path =  Str.string_match path_regexp path 0 in
+    DB.paths_matching root_summaries_dir path_matcher in
+  let pair_for_stacktrace_file = match stacktrace_file with
+    | None -> None
+    | Some file -> begin
+        let crashcontext_dir =
+          Config.results_dir // "crashcontext" in
+        DB.create_dir crashcontext_dir;
+        Some (file, crashcontext_dir // "crashcontext.json")
+      end in
+  let trace_file_regexp = Str.regexp "\\(.*\\)\\.json" in
+  let pairs_for_stactrace_dir = match stacktraces_dir with
+    | None -> []
+    | Some s -> begin
+        let dir = DB.filename_from_string s in
+        let trace_file_matcher path =
+          let path_str = DB.filename_to_string path in
+          Str.string_match trace_file_regexp path_str 0 in
+        let trace_fold stacktrace_file acc =
+          let stacktrace_file_str = DB.filename_to_string stacktrace_file in
+          let out_file =
+            (Str.matched_group 1 stacktrace_file_str) ^ ".crashcontext.json" in
+          (stacktrace_file_str, out_file) :: acc in
+        try
+          DB.fold_paths_matching ~dir ~p:trace_file_matcher ~init:[] ~f:trace_fold
+        with
+        (* trace_fold runs immediately after trace_file_matcher in the
+           DB.fold_paths_matching statement below, so we don't need to
+           call Str.string_match again. *)
+        | Not_found -> assert false
+      end in
+  let input_output_file_pairs = match pair_for_stacktrace_file with
+    | None -> pairs_for_stactrace_dir
+    | Some pair -> pair :: pairs_for_stactrace_dir in
+  let process_stacktrace (stacktrace_file, out_file) =
+    stitch_summaries stacktrace_file method_summaries out_file in
+  IList.iter process_stacktrace input_output_file_pairs
 
 let crashcontext_epilogue ~in_buck_mode =
   (* check whether this is the top-level infer process *)
   let top_level_infer =
-    (* if the '--buck' option was passed, then this is the top level process iff the build
+    (* if the '--buck' option was passed, then this is the top level process
-       command starts with 'buck' *)
+       iff the build command starts with 'buck' *)
     if Config.buck then in_buck_mode
-    (* otherwise, we assume javac as the build command and thus only one process *)
+    (* otherwise, we assume javac as the build command and thus only one
+       process *)
     else true in
   if top_level_infer then
-    (* if we are the top-level process, then find the output directory and collect all
+    (* if we are the top-level process, then find the output directory and
-       crashcontext summaries under it in a single crashcontext.json file *)
+       collect all crashcontext summaries under it in a single
-    let root_out_dir = if in_buck_mode then begin
+       crashcontext.json file.
+       Important: Note that when running under buck, this is not the final
+       infer-out/ directory, but instead it is buck-out/, which contains the
+       infer output directories for every buck target. *)
+    let root_summaries_dir = if in_buck_mode then begin
         let project_root = match Config.project_root with
           | Some root -> root
           | None -> Filename.dirname Config.results_dir in
         let buck_out = match Config.buck_out with
           | Some dir -> dir
           | None -> "buck-out" in
-        Filename.concat project_root buck_out
+        project_root // buck_out
       end
       else Config.results_dir in
-    match Config.stacktrace with
+    collect_all_summaries
-    | None -> failwith "Detected -a crashcontext without --stacktrace, this should have been \
+      root_summaries_dir Config.stacktrace Config.stacktraces_dir
-                        checked earlier."
-    | Some s -> collect_all_summaries root_out_dir s

infer/src/backend/infer.ml

@@ -94,8 +94,6 @@ let () =
          ["--use-flavors"]) @
       (match Config.infer_cache with None -> [] | Some s ->
           ["--infer_cache"; s]) @
-      (match Config.stacktrace with None -> [] | Some s ->
-          ["--stacktrace"; s]) @
       "-j" :: (string_of_int Config.jobs) ::
       (match Config.load_average with None -> [] | Some f ->
           ["-l"; string_of_float f]) @

infer/src/checkers/BoundedCallTree.ml

@@ -38,7 +38,7 @@ module SpecSummary = Summary.Make (struct
 
 type extras_t = {
   get_proc_desc : Procname.t -> Cfg.Procdesc.t option;
-  stacktrace : Stacktrace.t;
+  stacktraces : Stacktrace.t list;
 }
 
 let line_range_of_pdesc pdesc =
@@ -110,7 +110,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
   let exec_instr astate proc_data _ = function
     | Sil.Call (_, Const (Const.Cfun pn), _, loc, _) ->
         let get_proc_desc = proc_data.ProcData.extras.get_proc_desc in
-        let trace = proc_data.ProcData.extras.stacktrace in
+        let traces = proc_data.ProcData.extras.stacktraces in
         let caller = Cfg.Procdesc.get_proc_name proc_data.ProcData.pdesc in
         let matches_proc frame =
           let matches_class pname = match pname with
@@ -127,18 +127,20 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
                 failwith "Proc type not supported by crashcontext: block" in
           frame.Stacktrace.method_str = (Procname.get_method caller) &&
           matches_class caller in
-        let proc_in_trace = IList.exists matches_proc trace.Stacktrace.frames in
+        let all_frames = IList.flatten
-        if proc_in_trace then begin
+            (IList.map (fun trace -> trace.Stacktrace.frames) traces) in
-          let frame = IList.find matches_proc trace.Stacktrace.frames in
+        begin
-          let new_astate = Domain.add pn astate in
+          try
-          if Stacktrace.frame_matches_location frame loc then begin
+            let frame = IList.find matches_proc all_frames in
-            let pdesc = proc_data.ProcData.pdesc in
+            let new_astate = Domain.add pn astate in
-            output_json_summary pdesc new_astate loc "call_site" get_proc_desc
+            if Stacktrace.frame_matches_location frame loc then begin
-          end;
+              let pdesc = proc_data.ProcData.pdesc in
-          new_astate
+              output_json_summary pdesc new_astate loc "call_site" get_proc_desc
+            end;
+            new_astate
+          with
+            Not_found -> astate
         end
-        else
-          astate
     | Sil.Call _ ->
         (* We currently ignore calls through function pointers in C and
            other potential special kinds of procedure calls to be added later,
@@ -155,31 +157,36 @@ module Analyzer =
     (Scheduler.ReversePostorder)
     (TransferFunctions)
 
-(* Stacktrace lookup:
+let loaded_stacktraces =
-   1) Check if trace_ref is already set and use that.
+  (* Load all stacktraces defined in either Config.stacktrace or
-   2) If not, load trace from the file specified in Config.stacktrace. *)
+     Config.stacktraces_dir.  *)
-let trace_ref = ref None
+  let json_files_in_dir dir =
-
+    let stacktrace_path_regexp = Str.regexp ".*\\.json" in
-let load_trace () =
+    let path_matcher path =  Str.string_match stacktrace_path_regexp path 0 in
-  (* Check Config.stacktrace is set and points to a file,
+    DB.paths_matching dir path_matcher in
-     call Stacktrace.of_json_file  *)
+  let filenames = match Config.stacktrace, Config.stacktraces_dir  with
-  let filename = match Config.stacktrace with
+    | None, None -> None
-    | None -> failwith "Missing command line option: '--stacktrace stack.json' \
+    | Some fname, None -> Some [fname]
-                        must be used when running '-a crashcontext'. This \
+    | None, Some dir -> Some (json_files_in_dir dir)
-                        option expects a JSON formated stack trace. See \
+    | Some fname, Some dir -> Some (fname :: (json_files_in_dir dir)) in
-                        tests/codetoanalyze/java/crashcontext/*.json for \
+  match filenames with
-                        examples of the expected format."
+  | None -> None
-    | Some fname -> fname in
+  | Some files -> Some (IList.map Stacktrace.of_json_file files)
-  let new_trace = Stacktrace.of_json_file filename in
-  trace_ref := Some new_trace;
-  new_trace
 
 let checker { Callbacks.proc_desc; tenv; get_proc_desc; } =
-  let stacktrace = match !trace_ref with
+  match loaded_stacktraces with
-    | None -> load_trace ()
+  | None -> failwith "Missing command line option. Either \
-    | Some t -> t in
+                      '--stacktrace stack.json' or '--stacktrace-dir ./dir' \
-  let extras = { get_proc_desc; stacktrace; } in
+                      must be used when running '-a crashcontext'. This \
-  SpecSummary.write_summary
+                      options expects a JSON formated stack trace or a \
-    (Cfg.Procdesc.get_proc_name proc_desc)
+                      directory containing multiple such traces, \
-    (Some (stacktree_of_pdesc proc_desc "proc_start"));
+                      respectively. See \
-  ignore(Analyzer.exec_pdesc (ProcData.make proc_desc tenv extras))
+                      tests/codetoanalyze/java/crashcontext/*.json for \
+                      examples of the expected format."
+  | Some stacktraces -> begin
+      let extras = { get_proc_desc; stacktraces; } in
+      SpecSummary.write_summary
+        (Cfg.Procdesc.get_proc_name proc_desc)
+        (Some (stacktree_of_pdesc proc_desc "proc_start"));
+      ignore(Analyzer.exec_pdesc (ProcData.make proc_desc tenv extras))
+    end

infer/src/unit/BoundedCallTreeTests.ml

@@ -31,7 +31,13 @@ let tests =
       [Stacktrace.make_frame class_name "foo" file_name (Some 16);
        Stacktrace.make_frame class_name "bar" file_name (Some 20)] in
   let extras = { BoundedCallTree.get_proc_desc = mock_get_proc_desc;
-                 stacktrace = trace; } in
+                 stacktraces = [trace]; } in
+  let multi_trace_1 = Stacktrace.make "java.lang.NullPointerException"
+      [Stacktrace.make_frame class_name "foo" file_name (Some 16)] in
+  let multi_trace_2 = Stacktrace.make "java.lang.NullPointerException"
+      [Stacktrace.make_frame class_name "bar" file_name (Some 20)] in
+  let multi_trace_extras = { BoundedCallTree.get_proc_desc = mock_get_proc_desc;
+                             stacktraces = [multi_trace_1; multi_trace_2]; } in
   let caller_foo_name = Procname.from_string_c_fun "foo" in
   let caller_bar_name = Procname.from_string_c_fun "bar" in
   let caller_baz_name = Procname.from_string_c_fun "baz" in
@@ -75,7 +81,25 @@ let tests =
       invariant "{  }"
     ];
   ] |> TestInterpreter.create_tests ~test_pname:caller_baz_name extras in
+  let test_list_multiple_traces_from_foo = [
+    "on_call_add_proc_name_in_any_stack_1",
+    [
+      make_call ~procname:f_proc_name [] []; (* means f() *)
+      invariant "{ f }"
+    ];
+  ] |> TestInterpreter.create_tests
+      ~test_pname:caller_foo_name multi_trace_extras in
+  let test_list_multiple_traces_from_bar = [
+    "on_call_add_proc_name_in_any_stack_2",
+    [
+      make_call ~procname:f_proc_name [] []; (* means f() *)
+      invariant "{ f }"
+    ];
+  ] |> TestInterpreter.create_tests
+      ~test_pname:caller_bar_name multi_trace_extras in
   let test_list = test_list_from_foo @
                   test_list_from_bar @
-                  test_list_from_baz in
+                  test_list_from_baz @
+                  test_list_multiple_traces_from_foo @
+                  test_list_multiple_traces_from_bar in
   "bounded_calltree_test_suite">:::test_list