introduce new "diff" command

Summary:
First steps towards implementing diff analysis functionalities inside infer
itself. What works: run infer, checkout parent, re-run infer, checkout top
revision, compute the reportdiff (but no final surfacing on the console). Lots
of TODO still, inlined in the code.

Reviewed By: jberdine

Differential Revision: D5364226

fbshipit-source-id: 5b7f9a5

.gitignore

@@ -41,6 +41,7 @@ duplicates.txt
 /infer/tests/build_systems/differential_*/**/Diff*.java
 /infer/tests/build_systems/differential_*/infer-current
 /infer/tests/build_systems/differential_*/infer-previous
+/infer/tests/build_systems/diff/src
 /_release
 
 # generated by oUnit

Makefile

@@ -20,6 +20,7 @@ BUILD_SYSTEMS_TESTS += \
   clang_with_M_flag \
   clang_with_MD_flag \
   delete_results_dir \
+  diff \
   fail_on_issue \
   j1 \
   linters \

infer/src/backend/InferPrint.ml

@@ -322,7 +322,9 @@ let potential_exception_message = "potential exception at line"
 module IssuesJson = struct
   let is_first_item = ref true
 
-  let pp_json_open fmt () = F.fprintf fmt "[@?"
+  let pp_json_open fmt () =
+    is_first_item := true ;
+    F.fprintf fmt "[@?"
 
   let pp_json_close fmt () = F.fprintf fmt "]@\n@?"
 

infer/src/backend/infer.ml

@@ -73,6 +73,8 @@ let setup_results_dir () =
    -> assert_results_dir "have you run capture before?"
   | Clang | Report | ReportDiff
    -> create_results_dir ()
+  | Diff
+   -> remove_results_dir () ; create_results_dir ()
   | Capture | Compile | Run
    -> let driver_mode = Lazy.force Driver.mode_from_command_line in
       if not
@@ -124,3 +126,5 @@ let () =
         ~previous_report:Config.report_previous
   | Capture | Compile | Run
    -> run (Lazy.force Driver.mode_from_command_line)
+  | Diff
+   -> Diff.diff (Lazy.force Driver.mode_from_command_line)

infer/src/base/CommandDoc.ml

@@ -23,6 +23,7 @@ let command_to_name =
   [ (Analyze, "analyze")
   ; (Capture, "capture")
   ; (Compile, "compile")
+  ; (Diff, "diff")
   ; (Report, "report")
   ; (ReportDiff, "reportdiff")
   ; (Run, "run") ]
@@ -125,6 +126,13 @@ let compile =
       ]
     ~see_also:CLOpt.([Capture])
 
+let diff =
+  mk_command_doc ~title:"Infer Differential Analysis of a Project"
+    ~short_description:"Report the difference between two versions of a project"
+    ~synopsis:"$(b,infer) $(b,diff) $(i,[options])"
+    ~description:[`P "EXPERIMENTAL AND IN NO WAY READY TO USE"]
+    ~see_also:CLOpt.([ReportDiff; Run])
+
 let infer =
   mk_command_doc ~title:"Infer Static Analyzer"
     ~short_description:"static analysis for Java and C/C++/Objective-C/Objective-C++"
@@ -258,6 +266,7 @@ let command_to_data =
   [ mk Analyze analyze
   ; mk Capture capture
   ; mk Compile compile
+  ; mk Diff diff
   ; mk Report report
   ; mk ReportDiff reportdiff
   ; mk Run run ]

infer/src/base/CommandLineOption.ml

@@ -89,6 +89,7 @@ type command =
   | Capture
   | Clang
   | Compile
+  | Diff
   | Report
   | ReportDiff
   | Run
@@ -96,7 +97,7 @@ type command =
 
 let equal_command = [%compare.equal : command]
 
-let all_commands = [Analyze; Capture; Clang; Compile; Report; ReportDiff; Run]
+let all_commands = [Analyze; Capture; Clang; Compile; Diff; Report; ReportDiff; Run]
 
 type command_doc =
   { title: Cmdliner.Manpage.title

infer/src/base/CommandLineOption.mli

@@ -33,6 +33,7 @@ type command =
   | Compile
       (** set up the infer environment then run the compilation commands without capturing the
                 source files *)
+  | Diff  (** orchestrate a diff analysis *)
   | Report  (** post-process infer results and reports *)
   | ReportDiff  (** compute the difference of two infer reports *)
   | Run  (** orchestrate the capture, analysis, and reporting of a compilation command *)

infer/src/base/Config.ml

@@ -357,7 +357,7 @@ let startup_action =
     match initial_command with
     | Some Clang
      -> NoParse
-    | None | Some (Analyze | Capture | Compile | Report | ReportDiff | Run)
+    | None | Some (Analyze | Capture | Compile | Diff | Report | ReportDiff | Run)
      -> InferCommand
 
 let exe_usage =
@@ -411,7 +411,7 @@ let () =
      -> assert false (* filtered out *)
     | Report
      -> `Add
-    | Analyze | Capture | Compile | ReportDiff | Run
+    | Analyze | Capture | Compile | Diff | ReportDiff | Run
      -> `Reject
   in
   (* make sure we generate doc for all the commands we know about *)
@@ -759,6 +759,12 @@ and copy_propagation =
   CLOpt.mk_bool ~deprecated:["copy-propagation"] ~long:"copy-propagation"
     "Perform copy-propagation on the IR"
 
+and current_to_previous_script =
+  CLOpt.mk_string_opt ~long:"current-to-previous-script"
+    ~in_help:CLOpt.([(Diff, manual_generic)])
+    ~meta:"shell"
+    "Specify a script to checkout a previous version of the project to compare against, assuming we are on the current version already."
+
 and cxx, cxx_infer_headers =
   let cxx_infer_headers =
     CLOpt.mk_bool ~long:"cxx-infer-headers" ~default:true
@@ -1255,6 +1261,12 @@ and precondition_stats =
   CLOpt.mk_bool ~deprecated:["precondition_stats"] ~long:"precondition-stats"
     "Print stats about preconditions to standard output"
 
+and previous_to_current_script =
+  CLOpt.mk_string_opt ~long:"previous-to-current-script"
+    ~in_help:CLOpt.([(Diff, manual_generic)])
+    ~meta:"shell"
+    "Specify a script to checkout the current version of the project. The project is supposed to already be at that current version when running $(b,infer diff); the script is used after having analyzed the current and previous versions of the project, to restore the project to the current version."
+
 and print_active_checkers =
   CLOpt.mk_bool ~long:"print-active-checkers"
     ~in_help:CLOpt.([(Analyze, manual_generic)])
@@ -1813,11 +1825,7 @@ and compute_analytics = !compute_analytics
 
 and continue_capture = !continue
 
-and linter = !linter
+and current_to_previous_script = !current_to_previous_script
-
-and default_linters = !default_linters
-
-and linters_ignore_clang_failures = !linters_ignore_clang_failures
 
 and copy_propagation = !copy_propagation
 
@@ -1839,6 +1847,8 @@ and debug_exceptions = !debug_exceptions
 
 and debug_mode = !debug
 
+and default_linters = !default_linters
+
 and dependency_mode = !dependencies
 
 and developer_mode = !developer_mode
@@ -1925,12 +1935,16 @@ and join_cond = !join_cond
 
 and latex = !latex
 
+and linter = !linter
+
 and linters_def_file = !linters_def_file
 
 and linters_def_folder = !linters_def_folder
 
 and linters_developer_mode = !linters_developer_mode
 
+and linters_ignore_clang_failures = !linters_ignore_clang_failures
+
 and load_average =
   match !load_average with None when !buck -> Some (float_of_int ncpu) | _ -> !load_average
 
@@ -1982,6 +1996,8 @@ and pmd_xml = !pmd_xml
 
 and precondition_stats = !precondition_stats
 
+and previous_to_current_script = !previous_to_current_script
+
 and printf_args = !printf_args
 
 and print_active_checkers = !print_active_checkers

infer/src/base/Config.mli

@@ -322,16 +322,14 @@ val compute_analytics : bool
 
 val continue_capture : bool
 
-val default_linters : bool
-
-val linters_ignore_clang_failures : bool
-
 val copy_propagation : bool
 
 val crashcontext : bool
 
 val create_harness : bool
 
+val current_to_previous_script : string option
+
 val cxx : bool
 
 val cxx_infer_headers : bool
@@ -346,6 +344,8 @@ val debug_exceptions : bool
 
 val debug_mode : bool
 
+val default_linters : bool
+
 val dependency_mode : bool
 
 val developer_mode : bool
@@ -469,6 +469,8 @@ val linters_def_folder : string list
 
 val linters_developer_mode : bool
 
+val linters_ignore_clang_failures : bool
+
 val load_analysis_results : string option
 
 val log_file : string
@@ -502,6 +504,8 @@ val pmd_xml : bool
 
 val precondition_stats : bool
 
+val previous_to_current_script : string option
+
 val print_active_checkers : bool
 
 val print_builtins : bool

infer/src/integration/Diff.ml

@@ -0,0 +1,65 @@
+(*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *)
+open! IStd
+module F = Format
+module L = Logging
+
+type revision = Current | Previous
+
+let string_of_revision = function Current -> "current" | Previous -> "previous"
+
+let pp_revision f r = F.fprintf f "%s" (string_of_revision r)
+
+let checkout revision =
+  let script_opt =
+    match revision with
+    | Current
+     -> Config.previous_to_current_script
+    | Previous
+     -> Config.current_to_previous_script
+  in
+  match script_opt with
+  | None
+   -> L.(die UserError)
+        "Please specify a script to checkout the %a revision of your project using --checkout-%a <script>."
+        pp_revision revision pp_revision revision
+  | Some script
+   -> L.progress "Checking out %a version:@\n  %s@\n" pp_revision revision script ;
+      let (), exit_or_signal = Utils.with_process_in script Utils.consume_in in
+      Result.iter_error exit_or_signal ~f:(fun _ ->
+          L.(die ExternalError)
+            "Failed to checkout %a revision: %s" pp_revision revision
+            (Unix.Exit_or_signal.to_string_hum exit_or_signal) )
+
+let save_report revision =
+  let report_name = Config.results_dir ^/ F.asprintf "report-%a.json" pp_revision revision in
+  Unix.rename ~src:Config.(results_dir ^/ report_json) ~dst:report_name ;
+  L.progress "Results for the %a revision stored in %s@\n" pp_revision revision report_name ;
+  report_name
+
+let diff driver_mode =
+  (* TODO(t15553258) run gen-build script if specified *)
+  (* run capture *)
+  Driver.capture driver_mode ~changed_files:None ;
+  (* run analysis TODO(t15553258) add --reactive and --changed_files_index *)
+  Driver.analyze_and_report driver_mode ~changed_files:None ;
+  let current_report = Some (save_report Current) in
+  (* TODO(t15553258) bail if nothing to analyze (configurable, some people might care about bugs
+     fixed more than about time to analyze) *)
+  checkout Previous ;
+  (* TODO(t15553258) run gen-build script if specified *)
+  (* run capture TODO(t15553258) add --reactive and --continue *)
+  Driver.capture driver_mode ~changed_files:None ;
+  (* run analysis TODO(t15553258) add --reactive and --changed_files_index *)
+  Driver.analyze_and_report driver_mode ~changed_files:None ;
+  checkout Current ;
+  let previous_report = Some (save_report Previous) in
+  (* compute differential *)
+  ReportDiff.reportdiff ~current_report ~previous_report ; (* TODO(t15553258) report new bugs! *)
+                                                           ()

infer/src/integration/Diff.mli

@@ -0,0 +1,13 @@
+(*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *)
+
+open! IStd
+
+val diff : Driver.mode -> unit
+(** orchestrates the analysis of a code change *)

infer/tests/build_systems/codetoanalyze/some_bugs.c

@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+#include <stdlib.h>
+
+void test1() {
+  int* s1 = NULL;
+  *s1 = 42;
+}
+
+void test2() {
+  int* s2 = NULL;
+  *s2 = 42;
+}

infer/tests/build_systems/codetoanalyze/some_different_bugs.c

@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+#include <stdlib.h>
+
+void test1() {
+  int* s1 = NULL;
+  *s1 = 42;
+}
+
+void test3() {
+  int* s3 = malloc(1);
+  if (s3 != NULL) {
+    *s3 = 42;
+  }
+}

infer/tests/build_systems/diff/Makefile

@@ -0,0 +1,63 @@
+# Copyright (c) 2017 - present Facebook, Inc.
+# All rights reserved.
+#
+# This source code is licensed under the BSD style license found in the
+# LICENSE file in the root directory of this source tree. An additional grant
+# of patent rights can be found in the PATENTS file in the same directory.
+
+TESTS_DIR = ../..
+
+include $(TESTS_DIR)/base.make
+
+SRC_DIR = $(CURDIR)/../codetoanalyze
+INFER_OUT = infer-out
+INFER_OPTIONS = \
+  --previous-to-current-script '$(COPY) $(SRC_DIR)/some_bugs.c src/hello.c' \
+  --current-to-previous-script '$(COPY) $(SRC_DIR)/some_different_bugs.c src/hello.c' \
+  --report-hook '/bin/true' \
+  -- clang -c src/hello.c
+
+SOURCES = $(SRC_DIR)/some_bugs.c $(SRC_DIR)/some_different_bugs.c
+
+default: analyze
+
+.PHONY: analyze
+analyze: $(INFER_OUT)/differential/introduced.json
+
+$(INFER_OUT)/differential/introduced.json: $(SOURCES) $(CLANG_DEPS)
+	$(QUIET)$(MKDIR_P) src
+	$(QUIET)$(COPY) $(SRC_DIR)/some_bugs.c src/hello.c
+	$(QUIET)$(call silent_on_success,Running diff analysis in $(TEST_REL_DIR),\
+	  $(INFER_BIN) -o $(INFER_OUT) --project-root $(CURDIR) diff \
+	        $(INFER_OPTIONS))
+
+introduced.exp.test: $(INFER_OUT)/differential/introduced.json $(INFER_BIN)
+	$(QUIET)$(INFER_BIN) report \
+		--from-json-report $(INFER_OUT)/differential/introduced.json \
+		--issues-tests introduced.exp.test
+	$(QUIET)$(INFER_BIN) report \
+		--from-json-report $(INFER_OUT)/differential/fixed.json \
+		--issues-tests fixed.exp.test
+	$(QUIET)$(INFER_BIN) report \
+		--from-json-report $(INFER_OUT)/differential/preexisting.json \
+		--issues-tests preexisting.exp.test
+
+.PHONY: print
+print: introduced.exp.test
+
+.PHONY: test
+test: print
+	$(QUIET)$(call check_no_diff,introduced.exp,introduced.exp.test)
+	$(QUIET)$(call check_no_diff,fixed.exp,fixed.exp.test)
+	$(QUIET)$(call check_no_diff,preexisting.exp,preexisting.exp.test)
+
+.PHONY: replace
+replace: $(EXPECTED_TEST_OUTPUT)
+	$(COPY) introduced.exp.test introduced.exp
+	$(COPY) fixed.exp.test fixed.exp
+	$(COPY) preexisting.exp.test preexisting.exp
+
+.PHONY: clean
+clean:
+	$(REMOVE_DIR) *.exp.test $(INFER_OUT) $(CURRENT_DIR) $(PREVIOUS_DIR) \
+		$(CLEAN_EXTRA)

infer/tests/build_systems/diff/fixed.exp

@@ -0,0 +1 @@
+src/hello.c, test3, 3, MEMORY_LEAK, [start of procedure test3(),Condition is true]

infer/tests/build_systems/diff/introduced.exp

@@ -0,0 +1 @@
+src/hello.c, test2, 2, NULL_DEREFERENCE, [start of procedure test2()]

infer/tests/build_systems/diff/preexisting.exp

@@ -0,0 +1 @@
+src/hello.c, test1, 2, NULL_DEREFERENCE, [start of procedure test1()]