Add blame_range to crashcontext analysis.

Reviewed By: jberdine

Differential Revision: D3674674

fbshipit-source-id: 8d2cc25

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 = [];
   }
 

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,

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 *)

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:
+(* 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))

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 = {

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

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));
+  }
+
 }

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);
   }