From d23c99a4ea523300454e4d7fc8354534152f707c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1zaro=20Clapp=20Jim=C3=A9nez=20Labora?= Date: Fri, 5 Aug 2016 16:13:36 -0700 Subject: [PATCH] Add blame_range to crashcontext analysis. Reviewed By: jberdine Differential Revision: D3674674 fbshipit-source-id: 8d2cc25 --- infer/src/backend/crashcontext.ml | 3 +- infer/src/backend/specs.ml | 3 + infer/src/backend/specs.mli | 2 + infer/src/checkers/BoundedCallTree.ml | 113 +++++++++++++----- infer/src/checkers/stacktree.atd | 6 + infer/src/unit/BoundedCallTreeTests.ml | 10 +- .../java/crashcontext/BranchingCallsTest.java | 57 +++++++++ infer/tests/utils/CrashContextResults.java | 16 +++ 8 files changed, 179 insertions(+), 31 deletions(-) diff --git a/infer/src/backend/crashcontext.ml b/infer/src/backend/crashcontext.ml index abb0f8233..54ffd7520 100644 --- a/infer/src/backend/crashcontext.ml +++ b/infer/src/backend/crashcontext.ml @@ -37,7 +37,8 @@ let stracktree_of_frame frame = frame.Stacktrace.method_str; location = Some { Stacktree_j.location_type = "call_site"; file = frame.Stacktrace.file_str; - line = frame.Stacktrace.line_num }; + line = frame.Stacktrace.line_num; + blame_range = [] }; callees = []; } diff --git a/infer/src/backend/specs.ml b/infer/src/backend/specs.ml index 4aec941c4..e0e89f614 100644 --- a/infer/src/backend/specs.ml +++ b/infer/src/backend/specs.ml @@ -321,6 +321,8 @@ type payload = preposts : NormSpec.t list option; (** list of specs *) typestate : unit TypeState.t option; (** final typestate *) calls: call_summary option; + crashcontext_frame: Stacktree_j.stacktree option; + (** Proc location and blame_range info for crashcontext analysis *) } type summary = @@ -753,6 +755,7 @@ let empty_payload = preposts = None; typestate = None; calls = None; + crashcontext_frame = None; } (** [init_summary (depend_list, nodes, diff --git a/infer/src/backend/specs.mli b/infer/src/backend/specs.mli index 9129d1917..fc60d1619 100644 --- a/infer/src/backend/specs.mli +++ b/infer/src/backend/specs.mli @@ -127,6 +127,8 @@ type payload = preposts : NormSpec.t list option; (** list of specs *) typestate : unit TypeState.t option; (** final typestate *) calls: call_summary option; (** list of calls of the form (call, loc) *) + crashcontext_frame: Stacktree_j.stacktree option; + (** Procedure location and blame_range info for crashcontext analysis *) } (** Procedure summary *) diff --git a/infer/src/checkers/BoundedCallTree.ml b/infer/src/checkers/BoundedCallTree.ml index 7f8dd9995..baf2baa8c 100644 --- a/infer/src/checkers/BoundedCallTree.ml +++ b/infer/src/checkers/BoundedCallTree.ml @@ -22,29 +22,84 @@ module ProcnameSet = PrettyPrintable.MakePPSet(struct module Domain = AbstractDomain.FiniteSet(ProcnameSet) +(* Store a single stacktree frame per method. That is, callees is + always []. Instead, the expanded per-method summaries are directly stored + in the output directory as JSON files and *only* for those methods that + will be part of the final crashcontext.json. *) +module SpecSummary = Summary.Make (struct + type summary = Stacktree_j.stacktree option + + let update_payload frame payload = + { payload with Specs.crashcontext_frame = frame } + + let read_from_payload payload = + payload.Specs.crashcontext_frame + end) + +type extras_t = { + get_proc_desc : Procname.t -> Cfg.Procdesc.t option; + stacktrace : Stacktrace.t; +} + +let line_range_of_pdesc pdesc = + let ploc = Cfg.Procdesc.get_loc pdesc in + let start_line = ploc.Location.line in + let end_line = Cfg.Procdesc.fold_instrs + (fun acc _ instr -> + let new_loc = Sil.instr_get_loc instr in + max acc new_loc.Location.line) + start_line + pdesc in + { Stacktree_j.start_line; end_line } + +let stacktree_of_pdesc + pdesc + ?(loc=Cfg.Procdesc.get_loc pdesc) + ?(callees=[]) + location_type = + let procname = Cfg.Procdesc.get_proc_name pdesc in + let frame_loc = + Some { Stacktree_j.location_type = location_type; + file = DB.source_file_to_string loc.Location.file; + line = loc.Location.line; + blame_range = [line_range_of_pdesc pdesc] } in + { Stacktree_j.method_name = Procname.to_unique_id procname; + location = frame_loc; + callees = callees } + +let stacktree_stub_of_procname procname = + { Stacktree_j.method_name = Procname.to_unique_id procname; + location = None; + callees = [] } + module TransferFunctions (CFG : ProcCfg.S) = struct module CFG = CFG module Domain = Domain - type extras = Stacktrace.t + type extras = extras_t - let stacktree_of_summary caller astate loc location_type = + let stacktree_of_astate pdesc astate loc location_type get_proc_desc = let procs = Domain.elements astate in - let method_name = Procname.to_unique_id caller in - let file = DB.source_file_to_string loc.Location.file in - let line = loc.Location.line in - let location = Some { Stacktree_j.location_type ; file ; line } in let callees = IList.map (fun pn -> - { Stacktree_j.method_name = Procname.to_unique_id pn; - location = None; - callees = [] } ) + match SpecSummary.read_summary pdesc pn with + | None | Some None -> (match get_proc_desc pn with + | None -> stacktree_stub_of_procname pn + (* This can happen when the callee is in the same cluster/ buck + target, but it hasn't been checked yet. So we need both the + inter-target lookup (SpecSummary) and the intra-target + lookup (using get_proc_desc). *) + | Some callee_pdesc -> + stacktree_of_pdesc callee_pdesc "proc_start") + | Some (Some stracktree) -> stracktree ) procs in - { Stacktree_j.method_name; location; callees } + stacktree_of_pdesc pdesc ~loc ~callees location_type - let output_summary caller astate loc loc_type = - let stacktree = stacktree_of_summary caller astate loc loc_type in + let output_json_summary pdesc astate loc location_type get_proc_desc = + let caller = Cfg.Procdesc.get_proc_name pdesc in + let stacktree = + stacktree_of_astate pdesc astate loc location_type get_proc_desc in let dir = Filename.concat Config.results_dir "crashcontext" in - let suffix = F.sprintf "%s_%d" loc_type loc.Location.line in + let suffix = F.sprintf "%s_%d" location_type loc.Location.line in let fname = F.sprintf "%s.%s.json" (Procname.to_filename caller) suffix in @@ -54,6 +109,8 @@ 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 caller = Cfg.Procdesc.get_proc_name proc_data.ProcData.pdesc in let matches_proc frame = let matches_class pname = match pname with @@ -70,16 +127,13 @@ 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 - proc_data.ProcData.extras.Stacktrace.frames 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 - proc_data.ProcData.extras.Stacktrace.frames in + 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 - output_summary caller new_astate loc "call_site" + let pdesc = proc_data.ProcData.pdesc in + output_json_summary pdesc new_astate loc "call_site" get_proc_desc end; new_astate end @@ -101,13 +155,14 @@ 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. *) +(* 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 *) + (* 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 \ @@ -119,8 +174,12 @@ let load_trace () = trace_ref := Some new_trace; new_trace -let checker { Callbacks.proc_desc; tenv; } = - let trace = match !trace_ref with +let checker { Callbacks.proc_desc; tenv; get_proc_desc; } = + let stacktrace = match !trace_ref with | None -> load_trace () | Some t -> t in - ignore(Analyzer.exec_pdesc (ProcData.make proc_desc tenv trace)) + 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)) diff --git a/infer/src/checkers/stacktree.atd b/infer/src/checkers/stacktree.atd index 8f2c3871b..d865859ac 100644 --- a/infer/src/checkers/stacktree.atd +++ b/infer/src/checkers/stacktree.atd @@ -1,7 +1,13 @@ +type line_range_t = { + start_line : int; + end_line : int; +} + type location_t = { location_type : string; file : string; line : int; + blame_range : line_range_t list; } type stacktree = { diff --git a/infer/src/unit/BoundedCallTreeTests.ml b/infer/src/unit/BoundedCallTreeTests.ml index 9f9aece7f..56aac69f8 100644 --- a/infer/src/unit/BoundedCallTreeTests.ml +++ b/infer/src/unit/BoundedCallTreeTests.ml @@ -16,6 +16,8 @@ module TestInterpreter = AnalyzerTester.Make (Scheduler.ReversePostorder) (BoundedCallTree.TransferFunctions) +let mock_get_proc_desc _ = None + let tests = let open OUnit2 in let open AnalyzerTester.StructuredSil in @@ -28,6 +30,8 @@ let tests = let trace = Stacktrace.make "java.lang.NullPointerException" [Stacktrace.make_frame class_name "foo" file_name 16; Stacktrace.make_frame class_name "bar" file_name 20] in + let extras = { BoundedCallTree.get_proc_desc = mock_get_proc_desc; + stacktrace = trace; } 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 @@ -56,21 +60,21 @@ let tests = make_call ~procname:f_proc_name [] []; invariant "{ f }" ]; - ] |> TestInterpreter.create_tests ~test_pname:caller_foo_name trace in + ] |> TestInterpreter.create_tests ~test_pname:caller_foo_name extras in let test_list_from_bar = [ "on_call_anywhere_on_stack_add_proc_name", [ make_call ~procname:f_proc_name [] []; (* means f() *) invariant "{ f }" ]; - ] |> TestInterpreter.create_tests ~test_pname:caller_bar_name trace in + ] |> TestInterpreter.create_tests ~test_pname:caller_bar_name extras in let test_list_from_baz = [ "ignore_procs_unrelated_to_trace", [ make_call ~procname:f_proc_name [] []; (* means f() *) invariant "{ }" ]; - ] |> TestInterpreter.create_tests ~test_pname:caller_baz_name trace in + ] |> TestInterpreter.create_tests ~test_pname:caller_baz_name extras in let test_list = test_list_from_foo @ test_list_from_bar @ test_list_from_baz in diff --git a/infer/tests/endtoend/java/crashcontext/BranchingCallsTest.java b/infer/tests/endtoend/java/crashcontext/BranchingCallsTest.java index 310b41300..b4eed297e 100644 --- a/infer/tests/endtoend/java/crashcontext/BranchingCallsTest.java +++ b/infer/tests/endtoend/java/crashcontext/BranchingCallsTest.java @@ -21,18 +21,31 @@ import utils.CrashContextResults; public class BranchingCallsTest { public static final String TAG = "BranchingCallsExample"; + public static final String FILENAME = "BranchingCallsExample.java"; public static final String MAIN_METHOD = "codetoanalyze.java.crashcontext.BranchingCallsExample.main(java.lang.String[]):void"; + public static final int MAIN_METHOD_LOC_START = 33; + public static final int MAIN_METHOD_LOC_LINE = 34; + public static final int MAIN_METHOD_LOC_END = 34; public static final String FOO_METHOD = "codetoanalyze.java.crashcontext.BranchingCallsExample.foo():void"; + public static final int FOO_METHOD_LOC_START = 27; + public static final int FOO_METHOD_LOC_LINE = 27; + public static final int FOO_METHOD_LOC_END = 30; public static final String PRE_BAR_METHOD = "codetoanalyze.java.crashcontext.BranchingCallsExample.pre_bar():void"; + public static final int PRE_BAR_METHOD_LOC_START = 14; + public static final int PRE_BAR_METHOD_LOC_LINE = 14; + public static final int PRE_BAR_METHOD_LOC_END = 15; public static final String BAR_METHOD = "codetoanalyze.java.crashcontext.BranchingCallsExample.bar():void"; + public static final int BAR_METHOD_LOC_START = 22; + public static final int BAR_METHOD_LOC_LINE = 24; + public static final int BAR_METHOD_LOC_END = 24; public static final String POST_BAR_METHOD = "codetoanalyze.java.crashcontext.BranchingCallsExample.post_bar():void"; @@ -82,4 +95,48 @@ public class BranchingCallsTest { crashcontext.hasNotMethod(POST_BAR_METHOD)); } + @Test + public void sourceFileLocations() { + assertThat("Method " + MAIN_METHOD + " should have the following " + + "location information: {file: " + FILENAME + + ", critical line: " + MAIN_METHOD_LOC_LINE + + ", range start: " + MAIN_METHOD_LOC_START + + ", range end: " + MAIN_METHOD_LOC_END + "}", + crashcontext.hasMethodWithLocation(MAIN_METHOD, + FILENAME, + MAIN_METHOD_LOC_LINE, + MAIN_METHOD_LOC_START, + MAIN_METHOD_LOC_END)); + assertThat("Method " + FOO_METHOD + " should have the following " + + "location information: {file: " + FILENAME + + ", critical line: " + FOO_METHOD_LOC_LINE + + ", range start: " + FOO_METHOD_LOC_START + + ", range end: " + FOO_METHOD_LOC_END + "}", + crashcontext.hasMethodWithLocation(FOO_METHOD, + FILENAME, + FOO_METHOD_LOC_LINE, + FOO_METHOD_LOC_START, + FOO_METHOD_LOC_END)); + assertThat("Method " + BAR_METHOD + " should have the following " + + "location information: {file: " + FILENAME + + ", critical line: " + BAR_METHOD_LOC_LINE + + ", range start: " + BAR_METHOD_LOC_START + + ", range end: " + BAR_METHOD_LOC_END + "}", + crashcontext.hasMethodWithLocation(BAR_METHOD, + FILENAME, + BAR_METHOD_LOC_LINE, + BAR_METHOD_LOC_START, + BAR_METHOD_LOC_END)); + assertThat("Method " + PRE_BAR_METHOD + " should have the following " + + "location information: {file: " + FILENAME + + ", critical line: " + PRE_BAR_METHOD_LOC_LINE + + ", range start: " + PRE_BAR_METHOD_LOC_START + + ", range end: " + PRE_BAR_METHOD_LOC_END + "}", + crashcontext.hasMethodWithLocation(PRE_BAR_METHOD, + FILENAME, + PRE_BAR_METHOD_LOC_LINE, + PRE_BAR_METHOD_LOC_START, + PRE_BAR_METHOD_LOC_END)); + } + } diff --git a/infer/tests/utils/CrashContextResults.java b/infer/tests/utils/CrashContextResults.java index 2b4f826ef..21be7339a 100644 --- a/infer/tests/utils/CrashContextResults.java +++ b/infer/tests/utils/CrashContextResults.java @@ -96,6 +96,22 @@ public class CrashContextResults { return false; } + public boolean hasMethodWithLocation(String methodSignature, String filename, + int line, int start, int end) { + for (JsonNode frame : findNodesForMethod(methodSignature)) { + if (frame.path("location").path("file").asText().endsWith(filename) && + line == frame.path("location").path("line").asInt()) { + for (JsonNode blameRange : frame.path("location").path("blame_range")) { + if (blameRange.path("start_line").asInt() == start && + blameRange.path("end_line").asInt() == end) { + return true; + } + } + } + } + return false; + } + public static CrashContextResults loadJSONResults(String tag) throws IOException { return new CrashContextResults(tag); }