From f2e958ef3aebb890b5ce2a0323a8196738ea9555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1zaro=20Clapp=20Jim=C3=A9nez=20Labora?= Date: Thu, 25 Aug 2016 19:28:35 -0700 Subject: [PATCH] Add support for multiple stacktraces in '-a crashcontext' Reviewed By: jberdine Differential Revision: D3735011 fbshipit-source-id: 8a27a60 --- infer/lib/python/inferlib/analyze.py | 7 --- infer/src/backend/DB.ml | 21 ++++--- infer/src/backend/DB.mli | 4 ++ infer/src/backend/config.ml | 11 +++- infer/src/backend/config.mli | 1 + infer/src/backend/crashcontext.ml | 72 ++++++++++++++++------ infer/src/backend/infer.ml | 2 - infer/src/checkers/BoundedCallTree.ml | 85 ++++++++++++++------------ infer/src/unit/BoundedCallTreeTests.ml | 28 ++++++++- 9 files changed, 152 insertions(+), 79 deletions(-) diff --git a/infer/lib/python/inferlib/analyze.py b/infer/lib/python/inferlib/analyze.py index 74a54de8b..0a916456b 100644 --- a/infer/lib/python/inferlib/analyze.py +++ b/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 diff --git a/infer/src/backend/DB.ml b/infer/src/backend/DB.ml index 61ede5db4..14f80082d 100644 --- a/infer/src/backend/DB.ml +++ b/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 - matcher function f *) -let paths_matching root_dir f = - let rec paths path_list dir = Array.fold_left +(** Fold over all file paths recursively under [dir] which match [p]. *) +let fold_paths_matching ~dir ~p ~init ~f = + let rec paths path_list dir = + Array.fold_left (fun acc file -> - let p = Filename.concat dir file in - if Sys.is_directory p then (paths acc p) - else if f p then p :: acc + let path = dir // file in + if Sys.is_directory path then (paths acc path) + 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) diff --git a/infer/src/backend/DB.mli b/infer/src/backend/DB.mli index 94117e1f0..32bc16b70 100644 --- a/infer/src/backend/DB.mli +++ b/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 diff --git a/infer/src/backend/config.ml b/infer/src/backend/config.ml index aba562f32..ad5aa99c3 100644 --- a/infer/src/backend/config.ml +++ b/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 diff --git a/infer/src/backend/config.mli b/infer/src/backend/config.mli index 479cffb12..c5df57d4b 100644 --- a/infer/src/backend/config.mli +++ b/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 diff --git a/infer/src/backend/crashcontext.ml b/infer/src/backend/crashcontext.ml index d1ebec246..bc2958723 100644 --- a/infer/src/backend/crashcontext.ml +++ b/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 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 collect_all_summaries root_summaries_dir stacktrace_file stacktraces_dir = let method_summaries = - DB.paths_matching root_out_dir path_matcher in - stitch_summaries stacktrace_file method_summaries out_file + let path_regexp = Str.regexp ".*crashcontext/.*\\..*\\.json" in + 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 - command starts with 'buck' *) + (* if the '--buck' option was passed, then this is the top level process + 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 - crashcontext summaries under it in a single crashcontext.json file *) - let root_out_dir = if in_buck_mode then begin + (* if we are the top-level process, then find the output directory and + collect all crashcontext summaries under it in a single + 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 - | None -> failwith "Detected -a crashcontext without --stacktrace, this should have been \ - checked earlier." - | Some s -> collect_all_summaries root_out_dir s + collect_all_summaries + root_summaries_dir Config.stacktrace Config.stacktraces_dir diff --git a/infer/src/backend/infer.ml b/infer/src/backend/infer.ml index a0c5ab548..da8cb2fd6 100644 --- a/infer/src/backend/infer.ml +++ b/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]) @ diff --git a/infer/src/checkers/BoundedCallTree.ml b/infer/src/checkers/BoundedCallTree.ml index 864bfe868..87639a594 100644 --- a/infer/src/checkers/BoundedCallTree.ml +++ b/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 - if proc_in_trace then begin - let frame = IList.find matches_proc trace.Stacktrace.frames in - let new_astate = Domain.add pn astate in - if Stacktrace.frame_matches_location frame loc then begin - let pdesc = proc_data.ProcData.pdesc in - output_json_summary pdesc new_astate loc "call_site" get_proc_desc - end; - new_astate + let all_frames = IList.flatten + (IList.map (fun trace -> trace.Stacktrace.frames) traces) in + begin + try + let frame = IList.find matches_proc all_frames in + let new_astate = Domain.add pn astate in + if Stacktrace.frame_matches_location frame loc then begin + let pdesc = proc_data.ProcData.pdesc in + 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: - 1) Check if trace_ref is already set and use that. - 2) If not, load trace from the file specified in Config.stacktrace. *) -let trace_ref = ref None - -let load_trace () = - (* Check Config.stacktrace is set and points to a file, - call Stacktrace.of_json_file *) - let filename = match Config.stacktrace with - | None -> failwith "Missing command line option: '--stacktrace stack.json' \ - must be used when running '-a crashcontext'. This \ - option expects a JSON formated stack trace. See \ - tests/codetoanalyze/java/crashcontext/*.json for \ - examples of the expected format." - | Some fname -> fname in - let new_trace = Stacktrace.of_json_file filename in - trace_ref := Some new_trace; - new_trace +let loaded_stacktraces = + (* Load all stacktraces defined in either Config.stacktrace or + Config.stacktraces_dir. *) + let json_files_in_dir dir = + let stacktrace_path_regexp = Str.regexp ".*\\.json" in + let path_matcher path = Str.string_match stacktrace_path_regexp path 0 in + DB.paths_matching dir path_matcher in + let filenames = match Config.stacktrace, Config.stacktraces_dir with + | None, None -> None + | Some fname, None -> Some [fname] + | None, Some dir -> Some (json_files_in_dir dir) + | Some fname, Some dir -> Some (fname :: (json_files_in_dir dir)) in + match filenames with + | None -> None + | Some files -> Some (IList.map Stacktrace.of_json_file files) let checker { Callbacks.proc_desc; tenv; get_proc_desc; } = - let stacktrace = match !trace_ref with - | None -> load_trace () - | Some t -> t in - let extras = { get_proc_desc; stacktrace; } 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)) + match loaded_stacktraces with + | None -> failwith "Missing command line option. Either \ + '--stacktrace stack.json' or '--stacktrace-dir ./dir' \ + must be used when running '-a crashcontext'. This \ + options expects a JSON formated stack trace or a \ + directory containing multiple such traces, \ + respectively. See \ + 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 diff --git a/infer/src/unit/BoundedCallTreeTests.ml b/infer/src/unit/BoundedCallTreeTests.ml index c32a423f1..6b9ff1d75 100644 --- a/infer/src/unit/BoundedCallTreeTests.ml +++ b/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