From bfccb5222e5fab988f67166560cb7fcdbe8124be Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 8 Jun 2017 07:16:01 -0700 Subject: [PATCH] [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 | 2 +- infer/src/backend/InferAnalyze.re | 2 - infer/src/backend/InferPrint.re | 4 +- infer/src/backend/PerfStats.ml | 36 +++++---- infer/src/backend/PerfStats.mli | 2 + infer/src/backend/infer.ml | 80 +++++++++++-------- infer/src/java/jMain.ml | 4 +- .../buck_flavors_deterministic/.buckconfig | 0 .../buck_flavors_deterministic/Makefile | 54 +++++++++++++ .../differences.exp | 1 + .../buck_flavors_deterministic/src/BUCK | 6 ++ .../buck_flavors_deterministic/src/hello.c | 15 ++++ .../buck_flavors_deterministic/src/hello2.c | 15 ++++ 13 files changed, 163 insertions(+), 58 deletions(-) create mode 100644 infer/tests/build_systems/buck_flavors_deterministic/.buckconfig create mode 100644 infer/tests/build_systems/buck_flavors_deterministic/Makefile create mode 100644 infer/tests/build_systems/buck_flavors_deterministic/differences.exp create mode 100644 infer/tests/build_systems/buck_flavors_deterministic/src/BUCK create mode 100644 infer/tests/build_systems/buck_flavors_deterministic/src/hello.c create mode 100644 infer/tests/build_systems/buck_flavors_deterministic/src/hello2.c diff --git a/Makefile b/Makefile index 3ba6d4482..15bb04dad 100644 --- a/Makefile +++ b/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 diff --git a/infer/src/backend/InferAnalyze.re b/infer/src/backend/InferAnalyze.re index c8768e228..d4663b2b8 100644 --- a/infer/src/backend/InferAnalyze.re +++ b/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 }; diff --git a/infer/src/backend/InferPrint.re b/infer/src/backend/InferPrint.re index f2ba33283..512ca3122 100644 --- a/infer/src/backend/InferPrint.re +++ b/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 }; diff --git a/infer/src/backend/PerfStats.ml b/infer/src/backend/PerfStats.ml index 02fffef24..912a1bc64 100644 --- a/infer/src/backend/PerfStats.ml +++ b/infer/src/backend/PerfStats.ml @@ -141,22 +141,24 @@ let stats () = } let register_report_at_exit file = - Epilogues.register ~f:(fun () -> - try - let json_stats = to_json (stats ()) in + if not Config.buck_cache_mode then + Epilogues.register ~f:(fun () -> try - Unix.mkdir_p (Filename.dirname file); - let stats_oc = open_out file in - let stats_fd = Unix.descr_of_out_channel stats_oc in - ignore (Unix.flock stats_fd Unix.Flock_command.lock_exclusive); - Yojson.Basic.pretty_to_channel stats_oc json_stats; - ignore (Unix.flock stats_fd Unix.Flock_command.unlock); - Out_channel.close stats_oc + let json_stats = to_json (stats ()) in + try + Unix.mkdir_p (Filename.dirname file); + let stats_oc = open_out file in + let stats_fd = Unix.descr_of_out_channel stats_oc in + ignore (Unix.flock stats_fd Unix.Flock_command.lock_exclusive); + 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@." - file (Exn.to_string exc) (Yojson.Basic.pretty_to_string json_stats) - (Printexc.get_backtrace ()) - 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) + 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) diff --git a/infer/src/backend/PerfStats.mli b/infer/src/backend/PerfStats.mli index a19e69ff5..7aac6afea 100644 --- a/infer/src/backend/PerfStats.mli +++ b/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 diff --git a/infer/src/backend/infer.ml b/infer/src/backend/infer.ml index 7a9ba0c81..9ec0633fd 100644 --- a/infer/src/backend/infer.ml +++ b/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"; - "attributes"; "backend_stats"; "reporting_stats"; "frontend_stats"] in - let suffixes = [".cfg"; ".cg"; ".txt"; ".csv"; ".json"] in + (* In Buck flavors mode we keep all capture data, but in Java mode we keep only the tenv *) + let should_delete_dir = + 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 -> ( - 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 + | dir -> 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") - && List.exists ~f:(Filename.check_suffix name) suffixes then - Unix.unlink name + if should_delete_file name then + 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 - (`Java, string_of_build_system build_system) in + else ( + 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 () ; - if Config.buck_cache_mode && Config.reactive_mode then - failwith "The reactive analysis mode is not compatible with the Buck integration for Java"; + register_perf_stats_report () ; 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 - clean_results_dir () + if Config.buck_cache_mode then clean_results_dir (); + () let differential_mode () = (* at least one report must be passed in input to compute differential *) diff --git a/infer/src/java/jMain.ml b/infer/src/java/jMain.ml index 77d22d2c8..7606a8371 100644 --- a/infer/src/java/jMain.ml +++ b/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 (); diff --git a/infer/tests/build_systems/buck_flavors_deterministic/.buckconfig b/infer/tests/build_systems/buck_flavors_deterministic/.buckconfig new file mode 100644 index 000000000..e69de29bb diff --git a/infer/tests/build_systems/buck_flavors_deterministic/Makefile b/infer/tests/build_systems/buck_flavors_deterministic/Makefile new file mode 100644 index 000000000..4a9f082c1 --- /dev/null +++ b/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 diff --git a/infer/tests/build_systems/buck_flavors_deterministic/differences.exp b/infer/tests/build_systems/buck_flavors_deterministic/differences.exp new file mode 100644 index 000000000..1f4e11f6a --- /dev/null +++ b/infer/tests/build_systems/buck_flavors_deterministic/differences.exp @@ -0,0 +1 @@ +capture is deterministic diff --git a/infer/tests/build_systems/buck_flavors_deterministic/src/BUCK b/infer/tests/build_systems/buck_flavors_deterministic/src/BUCK new file mode 100644 index 000000000..ee8db4c9a --- /dev/null +++ b/infer/tests/build_systems/buck_flavors_deterministic/src/BUCK @@ -0,0 +1,6 @@ +cxx_library( + name = 'hello', + srcs = [ + 'hello.c', 'hello2.c', + ], +) diff --git a/infer/tests/build_systems/buck_flavors_deterministic/src/hello.c b/infer/tests/build_systems/buck_flavors_deterministic/src/hello.c new file mode 100644 index 000000000..a3c29e227 --- /dev/null +++ b/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 + +void test() { + int* s = NULL; + *s = 42; +} diff --git a/infer/tests/build_systems/buck_flavors_deterministic/src/hello2.c b/infer/tests/build_systems/buck_flavors_deterministic/src/hello2.c new file mode 100644 index 000000000..6910ad214 --- /dev/null +++ b/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 + +void test2() { + int* s = NULL; + *s = 42; +}