From c9d254c084554143ab5917b28a1f380197fb43cb Mon Sep 17 00:00:00 2001 From: Andrzej Kotulski Date: Tue, 13 Dec 2016 09:20:37 -0800 Subject: [PATCH] Initial version of reactive capture Summary: Allow backend to trigger compilation of extra files when it needs them. This will allow infer to capture less files initally and possibly speed up compilation Reviewed By: cristianoc, jberdine Differential Revision: D4231581 fbshipit-source-id: 181abea --- infer/src/IR/AttributesTable.re | 6 +- infer/src/IR/AttributesTable.rei | 2 +- infer/src/IR/Cfg.re | 3 + infer/src/backend/OndemandCapture.ml | 64 +++++++++++++++++++ infer/src/backend/exe_env.ml | 28 ++++---- infer/src/backend/ondemand.ml | 23 ++++--- infer/src/backend/ondemand.mli | 3 +- infer/src/clang/cFrontend.ml | 2 + .../integration/CaptureCompilationDatabase.ml | 4 +- .../CaptureCompilationDatabase.mli | 2 + .../clang_compilation_db/Makefile | 11 +++- .../clang_compilation_db/issues.exp | 4 ++ 12 files changed, 126 insertions(+), 26 deletions(-) create mode 100644 infer/src/backend/OndemandCapture.ml diff --git a/infer/src/IR/AttributesTable.re b/infer/src/IR/AttributesTable.re index b218180e2..2186acaff 100644 --- a/infer/src/IR/AttributesTable.re +++ b/infer/src/IR/AttributesTable.re @@ -109,11 +109,15 @@ let load_attributes proc_name => proc_attributes }; -let load_defined_attributes proc_name => +let load_defined_attributes cache_none::cache_none proc_name => try (Procname.Hash.find defined_attr_tbl proc_name) { | Not_found => let proc_attributes = load_attr defined_only::true proc_name; if (proc_attributes != None) { + /* procedure just got defined, replace attribute in attr_tbl with defined version */ + Procname.Hash.replace attr_tbl proc_name proc_attributes; + Procname.Hash.add defined_attr_tbl proc_name proc_attributes + } else if cache_none { Procname.Hash.add defined_attr_tbl proc_name proc_attributes }; proc_attributes diff --git a/infer/src/IR/AttributesTable.rei b/infer/src/IR/AttributesTable.rei index aeeab0c81..38d6cff0f 100644 --- a/infer/src/IR/AttributesTable.rei +++ b/infer/src/IR/AttributesTable.rei @@ -20,7 +20,7 @@ let load_attributes: Procname.t => option ProcAttributes.t; /** Load attrubutes for the procedure but only if is_defined is true */ -let load_defined_attributes: Procname.t => option ProcAttributes.t; +let load_defined_attributes: cache_none::bool => Procname.t => option ProcAttributes.t; /** Given the name of an ObjC class, extract the type from the tenv where the class was defined. We diff --git a/infer/src/IR/Cfg.re b/infer/src/IR/Cfg.re index 4c81a1415..2e47f5170 100644 --- a/infer/src/IR/Cfg.re +++ b/infer/src/IR/Cfg.re @@ -328,6 +328,9 @@ let store_cfg_to_file source_file::source_file (filename: DB.filename) (cfg: cfg | None => () } }; + /* NOTE: it's important to write attribute files to disk before writing .cfg file to disk. + OndemandCapture module relies on it - it uses existance of .cfg file as a barrier to make + sure that all attributes were written to disk (but not necessarily flushed) */ save_attributes source_file cfg; Serialization.to_file cfg_serializer filename cfg }; diff --git a/infer/src/backend/OndemandCapture.ml b/infer/src/backend/OndemandCapture.ml new file mode 100644 index 000000000..201c2e1bd --- /dev/null +++ b/infer/src/backend/OndemandCapture.ml @@ -0,0 +1,64 @@ +(* + * Copyright (c) 2016 - 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! Utils + +let compilation_db = lazy (CompilationDatabase.from_json_files !Config.clang_compilation_db_files) + +(** Given proc_attributes try to produce proc_attributes' where proc_attributes'.is_defined = true + It may trigger capture of extra files to do so and when it does, it waits for + frontend to finish before returning *) +let try_capture (attributes : ProcAttributes.t) : ProcAttributes.t option = + let lazy cdb = compilation_db in + if Option.is_none + (AttributesTable.load_defined_attributes ~cache_none:false attributes.proc_name) then ( + let decl_file = attributes.loc.file in + let definition_file_opt = SourceFile.of_header decl_file in + let try_compile definition_file = + let source_dir = DB.source_dir_from_source_file definition_file in + (* Use cfg_filename as a proxy to find out whether definition_file was already captured. + If it was, there is no point in trying to capture it again. + Treat existance of cfg_filename as a barrier - if it exists it means that + all attributes files have been created - write logic is defined in + Cfg.store_cfg_to_file *) + let cfg_filename = DB.source_dir_get_internal_file source_dir ".cfg" in + if not (DB.file_exists cfg_filename) then ( + CaptureCompilationDatabase.capture_file_in_database cdb definition_file; + if Config.debug_mode && + Option.is_none + (AttributesTable.load_defined_attributes ~cache_none:false attributes.proc_name) then ( + (* peek at the results to know if capture succeeded, but only in debug mode *) + Logging.out + "Captured file %a to get procedure %a but it wasn't found there@." + SourceFile.pp definition_file + Procname.pp attributes.proc_name + ) + ) else ( + Logging.out + "Wanted to capture file %a to get procedure %a but file was already captured@." + SourceFile.pp definition_file + Procname.pp attributes.proc_name + ) + in + Option.may try_compile definition_file_opt + ); + (* It's important to call load_defined_attributes again in all cases to make sure we try + reading from disk again no matter which condition happened. If previous call to + load_defined_attributes is None, it may mean couple of things: + - proc_name hasn't been captured yet, so it needs to get captured (most likely scenario) + - there was a race and proc_name got captured by the time we checked whether + cfg_filename exists. In this case it's important to refetch attributes from disk because + contents may have changed (attributes file for proc_name may be there now) + - proc_name can't be captured (there is no definition we know of). In that case + result will stay None. At this point we know(?) we won't be able to find definition + for it ever so we can cache None. + Caveat: it's possible that procedure will be captured in some other unrelated file + later - infer may ignore it then. + It also relies on retry mechanism in deserialization code to deal with half-written + attributes files *) + AttributesTable.load_defined_attributes ~cache_none:true attributes.proc_name diff --git a/infer/src/backend/exe_env.ml b/infer/src/backend/exe_env.ml index 36db88bfd..05b2a3deb 100644 --- a/infer/src/backend/exe_env.ml +++ b/infer/src/backend/exe_env.ml @@ -122,17 +122,23 @@ let get_file_data exe_env pname = Some (Procname.Hash.find exe_env.proc_map pname) with Not_found -> begin - match AttributesTable.load_attributes pname with - | None -> - L.err "can't find tenv_cfg_object for %a@." Procname.pp pname; - None - | Some proc_attributes -> - let source_file = proc_attributes.ProcAttributes.source_file_captured in - let source_dir = DB.source_dir_from_source_file source_file in - let cg_fname = DB.source_dir_get_internal_file source_dir ".cg" in - let file_data = create_file_data exe_env.file_map source_file cg_fname in - Procname.Hash.replace exe_env.proc_map pname file_data; - Some file_data + let source_file_opt = + match AttributesTable.load_attributes pname with + | None -> + L.err "can't find tenv_cfg_object for %a@." Procname.pp pname; + None + | Some proc_attributes when Config.reactive_capture -> + let get_captured_file {ProcAttributes.source_file_captured} = source_file_captured in + OndemandCapture.try_capture proc_attributes |> Option.map ~f:get_captured_file + | Some proc_attributes -> + Some proc_attributes.ProcAttributes.source_file_captured in + let get_file_data_for_source source_file = + let source_dir = DB.source_dir_from_source_file source_file in + let cg_fname = DB.source_dir_get_internal_file source_dir ".cg" in + let file_data = create_file_data exe_env.file_map source_file cg_fname in + Procname.Hash.replace exe_env.proc_map pname file_data; + file_data in + Option.map ~f:get_file_data_for_source source_file_opt end (** return the source file associated to the procedure *) diff --git a/infer/src/backend/ondemand.ml b/infer/src/backend/ondemand.ml index 9c4ba1594..a6f7d2363 100644 --- a/infer/src/backend/ondemand.ml +++ b/infer/src/backend/ondemand.ml @@ -14,7 +14,8 @@ open! IStd module L = Logging module F = Format -(** Directories to analyze from the ondemand file. *) +(** Optional set of source dirs to analyze in on-demand mode. If None then all source dirs + will be analyzed *) let dirs_to_analyze = let process_changed_files changed_files = SourceFile.Set.fold @@ -45,7 +46,7 @@ let unset_callbacks () = let nesting = ref 0 -let should_be_analyzed proc_attributes proc_name = +let should_be_analyzed proc_name proc_attributes = let currently_analyzed () = match Specs.get_summary proc_name with | None -> false @@ -62,8 +63,12 @@ let should_be_analyzed proc_attributes proc_name = let procedure_should_be_analyzed proc_name = match AttributesTable.load_attributes proc_name with + | Some proc_attributes when Config.reactive_capture && proc_attributes.is_defined = false -> + (* try to capture procedure first *) + let defined_proc_attributes = OndemandCapture.try_capture proc_attributes in + Option.value_map ~f:(should_be_analyzed proc_name) ~default:false defined_proc_attributes | Some proc_attributes -> - should_be_analyzed proc_attributes proc_name + should_be_analyzed proc_name proc_attributes | None -> false @@ -120,11 +125,11 @@ let run_proc_analysis ~propagate_exceptions analyze_proc curr_pdesc callee_pdesc let source = Option.value_map ~f:(fun (attributes : ProcAttributes.t) -> - let attribute_pname = attributes.proc_name in - if not (Procname.equal callee_pname attribute_pname) then - failwith ("ERROR: "^(Procname.to_string callee_pname) - ^" not equal to "^(Procname.to_string attribute_pname)); - attributes.loc.file) + let attribute_pname = attributes.proc_name in + if not (Procname.equal callee_pname attribute_pname) then + failwith ("ERROR: "^(Procname.to_string callee_pname) + ^" not equal to "^(Procname.to_string attribute_pname)); + attributes.loc.file) ~default:SourceFile.empty attributes_opt in let call_graph = @@ -192,7 +197,7 @@ let analyze_proc_desc ~propagate_exceptions curr_pdesc callee_pdesc = let proc_attributes = Procdesc.get_attributes callee_pdesc in match !callbacks_ref with | Some callbacks - when should_be_analyzed proc_attributes callee_pname -> + when should_be_analyzed callee_pname proc_attributes -> run_proc_analysis ~propagate_exceptions callbacks.analyze_ondemand curr_pdesc callee_pdesc | _ -> () diff --git a/infer/src/backend/ondemand.mli b/infer/src/backend/ondemand.mli index d1636a5da..dac0413e1 100644 --- a/infer/src/backend/ondemand.mli +++ b/infer/src/backend/ondemand.mli @@ -11,7 +11,8 @@ open! IStd (** Module for on-demand analysis. *) -(** Optional set of source dirs to analyze in on-demand mode. *) +(** Optional set of source dirs to analyze in on-demand mode. If None then all source dirs + will be analyzed *) val dirs_to_analyze : String.Set.t option type analyze_ondemand = SourceFile.t -> Procdesc.t -> unit diff --git a/infer/src/clang/cFrontend.ml b/infer/src/clang/cFrontend.ml index 345314555..2467edd82 100644 --- a/infer/src/clang/cFrontend.ml +++ b/infer/src/clang/cFrontend.ml @@ -53,6 +53,8 @@ let do_source_file translation_unit_context ast = (* This could be moved in the cfg_infer module *) let source_dir = DB.source_dir_from_source_file source_file in let tenv_file = DB.source_dir_get_internal_file source_dir ".tenv" in + (* Naming scheme of .cfg file matters for OndemandCapture module. If it + changes here, it should be changed there as well*) let cfg_file = DB.source_dir_get_internal_file source_dir ".cfg" in let cg_file = DB.source_dir_get_internal_file source_dir ".cg" in Cg.store_to_file cg_file call_graph; diff --git a/infer/src/integration/CaptureCompilationDatabase.ml b/infer/src/integration/CaptureCompilationDatabase.ml index a056f6278..72bad72f9 100644 --- a/infer/src/integration/CaptureCompilationDatabase.ml +++ b/infer/src/integration/CaptureCompilationDatabase.ml @@ -143,6 +143,8 @@ let get_compilation_database_files_xcodebuild () = Logging.stderr "There was an error executing the build command"; exit 1 - let capture_files_in_database compilation_database = run_compilation_database compilation_database (should_capture_file_from_index ()) + +let capture_file_in_database compilation_database source_file = + run_compilation_database compilation_database (SourceFile.equal source_file) diff --git a/infer/src/integration/CaptureCompilationDatabase.mli b/infer/src/integration/CaptureCompilationDatabase.mli index a82831a9b..0079305e7 100644 --- a/infer/src/integration/CaptureCompilationDatabase.mli +++ b/infer/src/integration/CaptureCompilationDatabase.mli @@ -14,6 +14,8 @@ open! IStd is passed, we only capture the files there *) val capture_files_in_database : CompilationDatabase.t -> unit +val capture_file_in_database : CompilationDatabase.t -> SourceFile.t -> unit + (** Get the compilation database files that contain the compilation given by the buck command. It will be the compilation of the passed targets only or also the dependencies according to the flag --use-compilation-database deps | no-deps *) diff --git a/infer/tests/build_systems/clang_compilation_db/Makefile b/infer/tests/build_systems/clang_compilation_db/Makefile index 393b053d1..bb56da97e 100644 --- a/infer/tests/build_systems/clang_compilation_db/Makefile +++ b/infer/tests/build_systems/clang_compilation_db/Makefile @@ -37,9 +37,16 @@ infer-out-no-index/report.json: $(CMAKE_BUILD_DIR)/compile_commands.json $(INFER $(call silent_on_success,\ $(INFER_BIN) -a $(ANALYZER) $(INFER_OPTIONS) -o infer-out-no-index -- clang-compilation-database $<) -issues.exp.test: infer-out-with-index/report.json infer-out-no-index/report.json +infer-out-reactive-capture/report.json: $(CMAKE_BUILD_DIR)/compile_commands.json $(INFER_BIN) $(SOURCES) + $(call silent_on_success,\ + $(INFER_BIN) -a $(ANALYZER) $(INFER_OPTIONS) -o infer-out-reactive-capture --reactive-capture \ + --changed-files-index $(CMAKE_DIR)/index.txt -- clang-compilation-database $<) + +issues.exp.test: infer-out-with-index/report.json infer-out-no-index/report.json infer-out-reactive-capture/report.json $(INFERPRINT_BIN) -q -a $(ANALYZER) $(INFERPRINT_OPTIONS) $@.with-index \ --from-json-report infer-out-with-index/report.json $(INFERPRINT_BIN) -q -a $(ANALYZER) $(INFERPRINT_OPTIONS) $@.no-index \ --from-json-report infer-out-no-index/report.json - cat $@.with-index $@.no-index > $@ + $(INFERPRINT_BIN) -q -a $(ANALYZER) $(INFERPRINT_OPTIONS) $@.reactive-capture \ + --from-json-report infer-out-reactive-capture/report.json + cat $@.with-index $@.no-index $@.reactive-capture > $@ diff --git a/infer/tests/build_systems/clang_compilation_db/issues.exp b/infer/tests/build_systems/clang_compilation_db/issues.exp index 048ebd3ed..b1fc92f76 100644 --- a/infer/tests/build_systems/clang_compilation_db/issues.exp +++ b/infer/tests/build_systems/clang_compilation_db/issues.exp @@ -5,3 +5,7 @@ hello.cpp, test0, 2, NULL_DEREFERENCE, [start of procedure test0()] hello.cpp, test1, 2, NULL_DEREFERENCE, [start of procedure test1(),start of procedure deref1()] hello.cpp, test2, 2, NULL_DEREFERENCE, [start of procedure test2(),start of procedure deref2()] lib3.h, test, 1, NULL_DEREFERENCE, [start of procedure test(),start of procedure deref3()] +hello.cpp, test0, 2, NULL_DEREFERENCE, [start of procedure test0()] +hello.cpp, test1, 2, NULL_DEREFERENCE, [start of procedure test1(),start of procedure deref1()] +hello.cpp, test2, 2, NULL_DEREFERENCE, [start of procedure test2(),start of procedure deref2()] +lib3.h, test, 1, NULL_DEREFERENCE, [start of procedure test(),start of procedure deref3()]