[buck] do not generate non-deterministic data inside buck-out

Summary:
This messes up with the Buck cache: if infer recaptures a file with only trivial changes that shouldn't affect the capture Buck will still believe it has to reanalyze everything that depends on that file if there's non-deterministic data in infer's output.

Re-use the `--buck` flag used by the Java Buck integration for mostly the same purposes. Add a few special cases for the flavours integration (eg: keep capture data).

Change perf stats registration to take `Config.buck_cache_mode` into account instead of relying on each call site to handle that peculiarity correctly. Also, there's no need to create the perf stats directories before calling the registration function since it will do that too.

Reviewed By: jeremydubreil

Differential Revision: D5192311

fbshipit-source-id: 334ea6e

Makefile

@@ -35,7 +35,7 @@ DIRECT_TESTS += \
   cpp_bufferoverrun cpp_errors cpp_frontend cpp_quandary cpp_siof cpp_threadsafety \
 
 ifneq ($(BUCK),no)
-BUILD_SYSTEMS_TESTS += buck-clang-db buck_flavors
+BUILD_SYSTEMS_TESTS += buck-clang-db buck_flavors buck_flavors_deterministic
 endif
 ifneq ($(CMAKE),no)
 BUILD_SYSTEMS_TESTS += clang_compilation_db cmake inferconfig

infer/src/backend/InferAnalyze.re

@@ -211,7 +211,5 @@ let register_perf_stats_report () => {
     };
   let stats_base = Config.perf_stats_prefix ^ Filename.basename cluster ^ ".json";
   let stats_file = Filename.concat stats_dir stats_base;
-  Utils.create_dir Config.results_dir;
-  Utils.create_dir stats_dir;
   PerfStats.register_report_at_exit stats_file
 };

infer/src/backend/InferPrint.re

@@ -1313,9 +1313,7 @@ let main ::report_csv ::report_json => {
     (Stats, init_stats_format_list ()),
     (Summary, init_summary_format_list ())
   ];
-  if (not Config.buck_cache_mode) {
+  register_perf_stats_report ();
-    register_perf_stats_report ()
-  };
   print_issues formats_by_report_kind
 };
 

infer/src/backend/PerfStats.ml

@@ -141,22 +141,24 @@ let stats () =
   }
 
 let register_report_at_exit file =
-  Epilogues.register ~f:(fun () ->
+  if not Config.buck_cache_mode then
-      try
+    Epilogues.register ~f:(fun () ->
-        let json_stats = to_json (stats ()) in
         try
-          Unix.mkdir_p (Filename.dirname file);
+          let json_stats = to_json (stats ()) in
-          let stats_oc = open_out file in
+          try
-          let stats_fd = Unix.descr_of_out_channel stats_oc in
+            Unix.mkdir_p (Filename.dirname file);
-          ignore (Unix.flock stats_fd Unix.Flock_command.lock_exclusive);
+            let stats_oc = open_out file in
-          Yojson.Basic.pretty_to_channel stats_oc json_stats;
+            let stats_fd = Unix.descr_of_out_channel stats_oc in
-          ignore (Unix.flock stats_fd Unix.Flock_command.unlock);
+            ignore (Unix.flock stats_fd Unix.Flock_command.lock_exclusive);
-          Out_channel.close stats_oc
+            Yojson.Basic.pretty_to_channel stats_oc json_stats;
+            flush stats_oc;
+            ignore (Unix.flock stats_fd Unix.Flock_command.unlock);
+            Out_channel.close stats_oc
+          with exc ->
+            Format.eprintf "Info: failed to write stats to %s@\n%s@\n%s@\n%s@."
+              file (Exn.to_string exc) (Yojson.Basic.pretty_to_string json_stats)
+              (Printexc.get_backtrace ())
         with exc ->
-          Format.eprintf "Info: failed to write stats to %s@\n%s@\n%s@\n%s@."
+          Format.eprintf "Info: failed to compute stats for %s@\n%s@\n%s@."
-            file (Exn.to_string exc) (Yojson.Basic.pretty_to_string json_stats)
+            file (Exn.to_string exc) (Printexc.get_backtrace ())
-            (Printexc.get_backtrace ())
+      ) ("stats reporting in " ^ file)
-      with exc ->
-        Format.eprintf "Info: failed to compute stats for %s@\n%s@\n%s@."
-          file (Exn.to_string exc) (Printexc.get_backtrace ())
-    ) ("stats reporting in " ^ file)

infer/src/backend/PerfStats.mli

@@ -17,4 +17,6 @@ val from_json : Yojson.Basic.json -> perf_stats
 
 val aggregate : perf_stats list -> Yojson.Basic.json
 
+(** Create performance report when the current process terminates. Automatically disabled when
+    [Config.buck_cache_mode] is true. *)
 val register_report_at_exit : string -> unit

infer/src/backend/infer.ml

@@ -135,35 +135,45 @@ let create_results_dir () =
   Unix.mkdir_p (Config.results_dir ^/ Config.specs_dir_name);
   L.setup_log_file ()
 
+(* Clean up the results dir to select only what's relevant to go in the Buck cache. In particular,
+   get rid of non-deterministic outputs.*)
 let clean_results_dir () =
-  let dirs = ["classnames"; "filelists"; "multicore"; "sources"; "log";
+  (* In Buck flavors mode we keep all capture data, but in Java mode we keep only the tenv *)
-              "attributes"; "backend_stats"; "reporting_stats"; "frontend_stats"] in
+  let should_delete_dir =
-  let suffixes = [".cfg"; ".cg"; ".txt"; ".csv"; ".json"] in
+    let dirs_to_delete =
+      "backend_stats" :: "classnames" :: "filelists" :: "frontend_stats" :: "multicore" ::
+      "reporting_stats" :: "sources" ::
+      if Config.flavors then []
+      else ["attributes"] in
+    List.mem ~equal:String.equal dirs_to_delete in
+  let should_delete_file =
+    let suffixes_to_delete =
+      ".txt" :: ".csv" :: ".json" ::
+      if Config.flavors then []
+      else [ ".cfg"; ".cg" ] in
+    fun name ->
+      (* Keep the JSON report *)
+      not (String.equal (Filename.basename name) "report.json")
+      && List.exists ~f:(Filename.check_suffix name) suffixes_to_delete in
   let rec clean name =
+    let rec cleandir dir = match Unix.readdir dir with
+      | entry ->
+          if should_delete_dir entry then (
+            rmtree (name ^/ entry)
+          ) else if not (String.equal entry Filename.current_dir_name
+                         || String.equal entry Filename.parent_dir_name) then (
+            clean (name ^/ entry)
+          );
+          cleandir dir (* next entry *)
+      | exception End_of_file ->
+          Unix.closedir dir in
     match Unix.opendir name with
-    | dir -> (
+    | dir ->
-        let rec cleandir dir =
-          match Unix.readdir dir with
-          | entry ->
-              if (List.exists ~f:(String.equal entry) dirs) then (
-                rmtree (name ^/ entry)
-              ) else if not (String.equal entry Filename.current_dir_name
-                             || String.equal entry Filename.parent_dir_name) then (
-                clean (name ^/ entry)
-              );
-              cleandir dir
-          | exception End_of_file ->
-              Unix.closedir dir in
         cleandir dir
-      )
-    | exception Unix.Unix_error (Unix.ENOTDIR, _, _)
-      when String.equal (Filename.basename name) "report.json" ->
-        (* Keep the JSON report *)
-        ()
     | exception Unix.Unix_error (Unix.ENOTDIR, _, _) ->
-        if not (String.equal (Filename.basename name) "report.json")
+        if should_delete_file name then
-        && List.exists ~f:(Filename.check_suffix name) suffixes then
+          Unix.unlink name;
-          Unix.unlink name
+        ()
     | exception Unix.Unix_error (Unix.ENOENT, _, _) ->
         () in
   clean Config.results_dir
@@ -187,7 +197,6 @@ let register_perf_stats_report () =
   let stats_dir = Filename.concat Config.results_dir Config.backend_stats_dir_name in
   let stats_base = Config.perf_stats_prefix ^ ".json" in
   let stats_file = Filename.concat stats_dir stats_base in
-  Unix.mkdir_p stats_dir;
   PerfStats.register_report_at_exit stats_file
 
 let reset_duplicates_file () =
@@ -297,6 +306,10 @@ let capture = function
               ["--xcode-developer-dir"; d]) @
           "--" ::
           if in_buck_mode && Config.flavors then
+            (* let children infer processes know that they are inside Buck *)
+            let infer_args_with_buck = String.concat ~sep:(String.of_char CLOpt.env_var_sep)
+                (Option.to_list (Sys.getenv CLOpt.args_env_var) @ ["--buck"]) in
+            Unix.putenv ~key:CLOpt.args_env_var ~data:infer_args_with_buck;
             Buck.add_flavors_to_buck_command build_cmd
           else build_cmd
         ) in
@@ -437,8 +450,13 @@ let assert_supported_build_system build_system = match build_system with
           (`Clang, "buck with flavors")
         else if Option.is_some Config.buck_compilation_database then
           (`Clang, "buck compilation database")
-        else
+        else (
-          (`Java, string_of_build_system build_system) in
+          if Config.reactive_mode then
+            L.user_error
+              "WARNING: The reactive analysis mode is not compatible with the Buck integration for \
+               Java";
+          (`Java, string_of_build_system build_system)
+        ) in
       assert_supported_mode analyzer build_string
   | BAnalyze ->
       ()
@@ -505,9 +523,7 @@ let infer_mode () =
      anyway, pretend that we are not called from another make to prevent make falling back to a
      mono-threaded execution. *)
   Unix.unsetenv "MAKEFLAGS";
-  if not Config.buck_cache_mode then register_perf_stats_report () ;
+  register_perf_stats_report () ;
-  if Config.buck_cache_mode && Config.reactive_mode then
-    failwith "The reactive analysis mode is not compatible with the Buck integration for Java";
   if not Config.buck_cache_mode then touch_start_file_unless_continue () ;
   capture driver_mode ;
   analyze driver_mode ;
@@ -519,8 +535,8 @@ let infer_mode () =
     if Config.fail_on_bug then
       fail_on_issue_epilogue () ;
   );
-  if Config.buck_cache_mode then
+  if Config.buck_cache_mode then clean_results_dir ();
-    clean_results_dir ()
+  ()
 
 let differential_mode () =
   (* at least one report must be passed in input to compute differential *)

infer/src/java/jMain.ml

@@ -20,13 +20,11 @@ let register_perf_stats_report source_file =
   let stats_dir = Filename.concat Config.results_dir Config.frontend_stats_dir_name in
   let abbrev_source_file = DB.source_file_encoding source_file in
   let stats_file = Config.perf_stats_prefix ^ "_" ^ abbrev_source_file ^ ".json" in
-  Utils.create_dir Config.results_dir ;
-  Utils.create_dir stats_dir ;
   PerfStats.register_report_at_exit (Filename.concat stats_dir stats_file)
 
 
 let init_global_state source_file =
-  if not Config.buck_cache_mode then register_perf_stats_report source_file ;
+  register_perf_stats_report source_file ;
   Config.curr_language := Config.Java;
   DB.Results_dir.init source_file;
   Ident.NameGenerator.reset ();

infer/tests/build_systems/buck_flavors_deterministic/Makefile

@@ -0,0 +1,54 @@
+# 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 = ../..
+ROOT_DIR = $(TESTS_DIR)/../..
+
+ANALYZER = infer
+BUCK_TARGET = //src:hello
+SOURCES = $(wildcard src/hello.c)
+OBJECTS = buck-out/gen/src/hello\#compile-hello.c.o1f717d69,default/hello.c.o
+INFER_OPTIONS = --report-custom-error --developer-mode --project-root $(TESTS_DIR) --no-failures-allowed
+INFERPRINT_OPTIONS = --project-root $(TESTS_DIR) --issues-tests
+CLEAN_EXTRA = buck-out capture_hash-1.sha capture_hash-2.sha
+
+include $(TESTS_DIR)/base.make
+
+# Buck passes -a capture to infer
+export INFER_STRICT_MODE=0
+
+$(OBJECTS): $(JAVA_SOURCE_FILES)
+	$(QUIET)$(call silent_on_success,Compiling Buck flavors tests,\
+	NO_BUCKD=1 $(BUCK) build --no-cache $(BUCK_TARGET))
+
+differences.exp.test: $(CLANG_DEPS) $(SOURCES) $(MAKEFILE_LIST)
+	$(QUIET)$(REMOVE_DIR) buck-out && \
+	$(call silent_on_success,Running Buck flavors capture a first time,\
+	NO_BUCKD=1 \
+	$(INFER_BIN) $(INFER_OPTIONS) capture --flavors --results-dir $(CURDIR)/infer-out -- \
+	  $(BUCK) build --no-cache $(BUCK_TARGET) && \
+	find buck-out/gen/src/infer-out-* -type f | xargs cat | $(SHASUM) > capture_hash-1.sha)
+	$(QUIET)$(REMOVE_DIR) buck-out && \
+	$(call silent_on_success,Running Buck flavors capture a second time,\
+	NO_BUCKD=1 \
+	$(INFER_BIN) $(INFER_OPTIONS) capture --flavors --results-dir $(CURDIR)/infer-out -- \
+	  $(BUCK) build --no-cache $(BUCK_TARGET) && \
+	find buck-out/gen/src/infer-out-* -type f | xargs cat | $(SHASUM) > capture_hash-2.sha)
+	$(QUIET)$(call silent_on_success,Computing difference,\
+	diff capture_hash-1.sha capture_hash-2.sha && \
+	  echo "capture is deterministic" > $@ || \
+	  echo "capture is not deterministic!" > $@; \
+	)
+	$(QUIET)$(REMOVE) capture_hash-1.sha capture_hash-2.sha
+
+.PHONY: test
+test: differences.exp.test
+	$(QUIET)diff -u differences.exp $<
+
+.PHONY: replace
+replace: differences.exp.test
+	$(QUIET)cp $< differences.exp

infer/tests/build_systems/buck_flavors_deterministic/differences.exp

@@ -0,0 +1 @@
+capture is deterministic

infer/tests/build_systems/buck_flavors_deterministic/src/BUCK

@@ -0,0 +1,6 @@
+cxx_library(
+  name = 'hello',
+  srcs = [
+    'hello.c', 'hello2.c',
+  ],
+)

infer/tests/build_systems/buck_flavors_deterministic/src/hello.c

@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2015 - 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 test() {
+  int* s = NULL;
+  *s = 42;
+}

infer/tests/build_systems/buck_flavors_deterministic/src/hello2.c

@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2015 - 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 test2() {
+  int* s = NULL;
+  *s = 42;
+}