From 7f9d56b1b54ff1f21d16bfa5985dc770318fcb0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Fri, 26 Feb 2021 05:29:33 -0800 Subject: [PATCH] [ConfigImpact] Only report on functions that occur on a given json config data Summary: Currently, we report on all functions that are not config checked. However, the aim of the analysis is to only report on these for specific functions. Moreover, this has performance implications in practice. This diff instead reports on functions that occur on a json file that is passed by the command line option `config-data-file`. Reviewed By: skcho Differential Revision: D26666336 fbshipit-source-id: 290cd3ada --- infer/man/man1/infer-full.txt | 7 ++++ infer/man/man1/infer-report.txt | 3 ++ infer/man/man1/infer.txt | 4 ++ infer/src/atd/config_impact_data.atd | 12 ++++++ infer/src/atd/dune | 14 +++++++ infer/src/base/Config.ml | 8 ++++ infer/src/base/Config.mli | 2 + infer/src/checkers/ConfigImpactAnalysis.ml | 8 ++-- .../src/checkers/ExternalConfigImpactData.ml | 40 +++++++++++++++++++ .../src/checkers/ExternalConfigImpactData.mli | 10 +++++ .../java/fb-config-impact/Makefile | 2 +- .../objc/fb-config-impact/Makefile | 2 +- .../objc/fb-config-impact/issues.exp | 2 + 13 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 infer/src/atd/config_impact_data.atd create mode 100644 infer/src/checkers/ExternalConfigImpactData.ml create mode 100644 infer/src/checkers/ExternalConfigImpactData.mli diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index 26a0a8f65..49b40f272 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -268,6 +268,10 @@ OPTIONS checkers (Conversely: --no-config-impact-analysis-only) See also infer-analyze(1). + --config-impact-data-file file + [ConfigImpact] Specify the file containing the config data + See also infer-report(1). + --continue Activates: Continue the capture for the reactive analysis, increasing the changed files/procedures. (If a procedure was @@ -1451,6 +1455,9 @@ INTERNAL OPTIONS --compilation-database-reset Set --compilation-database to the empty list. + --config-impact-data-file-reset + Cancel the effect of --config-impact-data-file. + --cost-issues-tests-reset Cancel the effect of --cost-issues-tests. diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt index 63a812cc3..a364c5e09 100644 --- a/infer/man/man1/infer-report.txt +++ b/infer/man/man1/infer-report.txt @@ -35,6 +35,9 @@ OPTIONS `` is a non-empty string used to explain why the issue was filtered. + --config-impact-data-file file + [ConfigImpact] Specify the file containing the config data + --cost-issues-tests file Write a list of cost issues in a format suitable for cost tests to file diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index 85f3696ae..439d891fd 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -268,6 +268,10 @@ OPTIONS checkers (Conversely: --no-config-impact-analysis-only) See also infer-analyze(1). + --config-impact-data-file file + [ConfigImpact] Specify the file containing the config data + See also infer-report(1). + --continue Activates: Continue the capture for the reactive analysis, increasing the changed files/procedures. (If a procedure was diff --git a/infer/src/atd/config_impact_data.atd b/infer/src/atd/config_impact_data.atd new file mode 100644 index 000000000..714e6a8c6 --- /dev/null +++ b/infer/src/atd/config_impact_data.atd @@ -0,0 +1,12 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +type config_item = { + method_name: string; + class_name: string +} +type config_data = config_item list \ No newline at end of file diff --git a/infer/src/atd/dune b/infer/src/atd/dune index 6cfbd5ff6..db9c4ed79 100644 --- a/infer/src/atd/dune +++ b/infer/src/atd/dune @@ -61,6 +61,20 @@ (action (run atdgen -t %{deps}))) +; ATD for config_impact_data + +(rule + (targets config_impact_data_j.ml config_impact_data_j.mli) + (deps config_impact_data.atd) + (action + (run atdgen -j -j-std %{deps}))) + +(rule + (targets config_impact_data_t.ml config_impact_data_t.mli) + (deps config_impact_data.atd) + (action + (run atdgen -t %{deps}))) + ; ATD for java_profiler_samples (rule diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 9e06c3701..9989e94d9 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -913,6 +913,12 @@ and compilation_database_escaped = from Xcode (can be specified multiple times)" +and config_impact_data_file = + CLOpt.mk_path_opt ~long:"config-impact-data-file" + ~in_help:InferCommand.[(Report, manual_generic)] + ~meta:"file" "[ConfigImpact] Specify the file containing the config data" + + (** Continue the capture for reactive mode: If a procedure was changed beforehand, keep the changed marking. *) and continue = @@ -2848,6 +2854,8 @@ and clang_libcxx_include_to_override_regex = !clang_libcxx_include_to_override_r and classpath = !classpath +and config_impact_data_file = !config_impact_data_file + and continue_analysis = !continue_analysis and continue_capture = !continue diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index 50748c978..4f4d22423 100644 --- a/infer/src/base/Config.mli +++ b/infer/src/base/Config.mli @@ -226,6 +226,8 @@ val clang_libcxx_include_to_override_regex : string option val command : InferCommand.t +val config_impact_data_file : string option + val continue_analysis : bool val continue_capture : bool diff --git a/infer/src/checkers/ConfigImpactAnalysis.ml b/infer/src/checkers/ConfigImpactAnalysis.ml index a304423c3..cc695b905 100644 --- a/infer/src/checkers/ConfigImpactAnalysis.ml +++ b/infer/src/checkers/ConfigImpactAnalysis.ml @@ -70,9 +70,11 @@ module UncheckedCallee = struct let report {InterproceduralAnalysis.proc_desc; err_log} x = - let desc = F.asprintf "%a without config check" pp x in - Reporting.log_issue proc_desc err_log ~loc:(get_location x) ~ltr:(make_err_trace x) - ConfigImpactAnalysis IssueType.config_impact_analysis desc + let pname = Procdesc.get_proc_name proc_desc in + if ExternalConfigImpactData.is_in_config_data_file pname then + let desc = F.asprintf "%a without config check" pp x in + Reporting.log_issue proc_desc err_log ~loc:(get_location x) ~ltr:(make_err_trace x) + ConfigImpactAnalysis IssueType.config_impact_analysis desc end module UncheckedCallees = struct diff --git a/infer/src/checkers/ExternalConfigImpactData.ml b/infer/src/checkers/ExternalConfigImpactData.ml new file mode 100644 index 000000000..b733fb4af --- /dev/null +++ b/infer/src/checkers/ExternalConfigImpactData.ml @@ -0,0 +1,40 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd +module L = Logging + +module ConfigProcnameSet = Caml.Set.Make (struct + (* workaround: since there is no way to have @@deriving directive + in atd declaration, we redefine the type *) + type t = Config_impact_data_t.config_item = {method_name: string; class_name: string} + [@@deriving compare] +end) + +let read_file_config_data fname = + let config_list = + try Atdgen_runtime.Util.Json.from_file Config_impact_data_j.read_config_data fname + with e -> + L.user_warning "Failed to read file '%s': %s@." fname (Exn.to_string e) ; + [] + in + List.fold config_list ~init:ConfigProcnameSet.empty ~f:(fun acc itm -> + ConfigProcnameSet.add itm acc ) + + +let is_in_config_data_file = + let config_data = + Option.value_map Config.config_impact_data_file ~default:ConfigProcnameSet.empty + ~f:read_file_config_data + in + fun proc_name -> + ConfigProcnameSet.exists + (fun {method_name; class_name} -> + String.equal method_name (Procname.get_method proc_name) + && String.equal class_name (Procname.get_class_name proc_name |> Option.value ~default:"") + ) + config_data diff --git a/infer/src/checkers/ExternalConfigImpactData.mli b/infer/src/checkers/ExternalConfigImpactData.mli new file mode 100644 index 000000000..611072966 --- /dev/null +++ b/infer/src/checkers/ExternalConfigImpactData.mli @@ -0,0 +1,10 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +val is_in_config_data_file : Procname.t -> bool diff --git a/infer/tests/codetoanalyze/java/fb-config-impact/Makefile b/infer/tests/codetoanalyze/java/fb-config-impact/Makefile index 84613398a..54e4a241d 100644 --- a/infer/tests/codetoanalyze/java/fb-config-impact/Makefile +++ b/infer/tests/codetoanalyze/java/fb-config-impact/Makefile @@ -5,7 +5,7 @@ TESTS_DIR = ../../.. -INFER_OPTIONS = --config-impact-analysis-only --debug-exceptions \ +INFER_OPTIONS = --config-impact-analysis-only --config-impact-data-file config_data.json --debug-exceptions \ --report-force-relative-path INFERPRINT_OPTIONS = --issues-tests SOURCES = $(wildcard *.java) diff --git a/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile b/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile index 49275e8c5..c0bd51bb6 100644 --- a/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile +++ b/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile @@ -6,7 +6,7 @@ TESTS_DIR = ../../.. CLANG_OPTIONS = -c $(OBJC_CLANG_OPTIONS) -INFER_OPTIONS = --config-impact-analysis-only -g --debug-exceptions \ +INFER_OPTIONS = --config-impact-analysis-only --config-impact-data-file config_data.json --debug-exceptions \ --report-force-relative-path --project-root $(TESTS_DIR) INFERPRINT_OPTIONS = --issues-tests diff --git a/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp b/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp index 5299b8098..dde6fc690 100644 --- a/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp +++ b/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp @@ -1 +1,3 @@ ../../facebook/skel/infer/tests/codetoanalyze/objc/fb-config-impact/Basic.m, qe_unchecked_bad, 4, CONFIG_IMPACT, no_bucket, ADVICE, [callee2 is called] +../../facebook/skel/infer/tests/codetoanalyze/objc/fb-config-impact/ClassTest.m, MyClass.call_log_bad:data:, 4, CONFIG_IMPACT, no_bucket, ADVICE, [__objc_alloc_no_fail is called] +../../facebook/skel/infer/tests/codetoanalyze/objc/fb-config-impact/ClassTest.m, MyClass.call_log_bad:data:, 4, CONFIG_IMPACT, no_bucket, ADVICE, [NSKeyedUnarchiver.initForReadingFromData:error: is called]