From 598cb10170252ca45173f6e229f6e9585730c9ed Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Wed, 3 Mar 2021 01:47:38 -0800 Subject: [PATCH] [ConfigImpact] Use config-impact-issues.exp instead of issues.exp Summary: This diff uses config-impact-issues.exp instead of issues.exp, like in the cost checker. Reviewed By: ezgicicek Differential Revision: D26723761 fbshipit-source-id: 9c6779479 --- infer/man/man1/infer-full.txt | 18 +++++++++ infer/man/man1/infer-report.txt | 11 ++++++ infer/man/man1/infer-reportdiff.txt | 4 ++ infer/man/man1/infer.txt | 12 ++++++ infer/src/base/Config.ml | 30 +++++++++++++++ infer/src/base/Config.mli | 6 +++ infer/src/base/IssueType.mli | 2 +- infer/src/checkers/ConfigImpactAnalysis.ml | 31 ++++++++------- infer/src/checkers/ConfigImpactAnalysis.mli | 4 ++ infer/src/infer.ml | 19 +++++----- .../src/integration/ConfigImpactIssuesTest.ml | 38 +++++++++++++++++++ .../integration/ConfigImpactIssuesTest.mli | 10 +++++ .../java/fb-config-impact/Makefile | 5 ++- .../objc/fb-config-impact/Makefile | 1 + .../objc/fb-config-impact/issues.exp | 3 -- infer/tests/config-impact.make | 27 +++++++++++++ 16 files changed, 190 insertions(+), 31 deletions(-) create mode 100644 infer/src/integration/ConfigImpactIssuesTest.ml create mode 100644 infer/src/integration/ConfigImpactIssuesTest.mli create mode 100644 infer/tests/config-impact.make diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index 49b40f272..6019e7fe1 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -272,6 +272,14 @@ OPTIONS [ConfigImpact] Specify the file containing the config data See also infer-report(1). + --config-impact-issues-tests file + Write a list of config impact issues in a format suitable for + config impact tests to file See also infer-report(1). + + --config-impact-max-callees-to-print int + Specify the maximum number of unchecked callees to print in the + config impact checker See also infer-report(1) and infer-reportdiff(1). + --continue Activates: Continue the capture for the reactive analysis, increasing the changed files/procedures. (If a procedure was @@ -626,6 +634,10 @@ OPTIONS checkers (Conversely: --no-fragment-retains-view-only) See also infer-analyze(1). + --from-json-config-impact-report config-impact-report.json + Load costs analysis results from a config-impact-report file. + See also infer-report(1). + --from-json-costs-report costs-report.json Load costs analysis results from a costs-report file. See also infer-report(1). @@ -1458,6 +1470,9 @@ INTERNAL OPTIONS --config-impact-data-file-reset Cancel the effect of --config-impact-data-file. + --config-impact-issues-tests-reset + Cancel the effect of --config-impact-issues-tests. + --cost-issues-tests-reset Cancel the effect of --cost-issues-tests. @@ -1564,6 +1579,9 @@ INTERNAL OPTIONS --force-integration-reset Cancel the effect of --force-integration. + --from-json-config-impact-report-reset + Cancel the effect of --from-json-config-impact-report. + --from-json-costs-report-reset Cancel the effect of --from-json-costs-report. diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt index a364c5e09..4f57f080d 100644 --- a/infer/man/man1/infer-report.txt +++ b/infer/man/man1/infer-report.txt @@ -38,6 +38,14 @@ OPTIONS --config-impact-data-file file [ConfigImpact] Specify the file containing the config data + --config-impact-issues-tests file + Write a list of config impact issues in a format suitable for + config impact tests to file + + --config-impact-max-callees-to-print int + Specify the maximum number of unchecked callees to print in the + config impact checker + --cost-issues-tests file Write a list of cost issues in a format suitable for cost tests to file @@ -270,6 +278,9 @@ OPTIONS Deactivates: Do not show the experimental and blacklisted issue types (Conversely: --filtering | -f) + --from-json-config-impact-report config-impact-report.json + Load costs analysis results from a config-impact-report file. + --from-json-costs-report costs-report.json Load costs analysis results from a costs-report file. diff --git a/infer/man/man1/infer-reportdiff.txt b/infer/man/man1/infer-reportdiff.txt index 5e3ef2a35..ab431390e 100644 --- a/infer/man/man1/infer-reportdiff.txt +++ b/infer/man/man1/infer-reportdiff.txt @@ -20,6 +20,10 @@ DESCRIPTION OPTIONS + --config-impact-max-callees-to-print int + Specify the maximum number of unchecked callees to print in the + config impact checker + --cost-tests-only-autoreleasepool Activates: [EXPERIMENTAL] Report only autoreleasepool size results in cost tests (Conversely: --no-cost-tests-only-autoreleasepool) diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index 439d891fd..2ef1b668f 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -272,6 +272,14 @@ OPTIONS [ConfigImpact] Specify the file containing the config data See also infer-report(1). + --config-impact-issues-tests file + Write a list of config impact issues in a format suitable for + config impact tests to file See also infer-report(1). + + --config-impact-max-callees-to-print int + Specify the maximum number of unchecked callees to print in the + config impact checker See also infer-report(1) and infer-reportdiff(1). + --continue Activates: Continue the capture for the reactive analysis, increasing the changed files/procedures. (If a procedure was @@ -626,6 +634,10 @@ OPTIONS checkers (Conversely: --no-fragment-retains-view-only) See also infer-analyze(1). + --from-json-config-impact-report config-impact-report.json + Load costs analysis results from a config-impact-report file. + See also infer-report(1). + --from-json-costs-report costs-report.json Load costs analysis results from a costs-report file. See also infer-report(1). diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 9989e94d9..175b1576a 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -919,6 +919,20 @@ and config_impact_data_file = ~meta:"file" "[ConfigImpact] Specify the file containing the config data" +and config_impact_issues_tests = + CLOpt.mk_path_opt ~long:"config-impact-issues-tests" + ~in_help:InferCommand.[(Report, manual_generic)] + ~meta:"file" + "Write a list of config impact issues in a format suitable for config impact tests to $(i,file)" + + +and config_impact_max_callees_to_print = + CLOpt.mk_int ~long:"config-impact-max-callees-to-print" ~default:3 + ~in_help:InferCommand.[(Report, manual_generic); (ReportDiff, manual_generic)] + ~meta:"int" + "Specify the maximum number of unchecked callees to print in the config impact checker" + + (** Continue the capture for reactive mode: If a procedure was changed beforehand, keep the changed marking. *) and continue = @@ -1331,6 +1345,13 @@ and from_json_report = generated by the analysis)." +and from_json_config_impact_report = + CLOpt.mk_path_opt ~long:"from-json-config-impact-report" + ~in_help:InferCommand.[(Report, manual_generic)] + ~meta:"config-impact-report.json" + "Load costs analysis results from a config-impact-report file." + + and from_json_costs_report = CLOpt.mk_path_opt ~long:"from-json-costs-report" ~in_help:InferCommand.[(Report, manual_generic)] @@ -2856,6 +2877,10 @@ and classpath = !classpath and config_impact_data_file = !config_impact_data_file +and config_impact_issues_tests = !config_impact_issues_tests + +and config_impact_max_callees_to_print = !config_impact_max_callees_to_print + and continue_analysis = !continue_analysis and continue_capture = !continue @@ -2935,6 +2960,11 @@ and from_json_report = ~default:(ResultsDirEntryName.get_path ~results_dir:!results_dir ReportJson) +and from_json_config_impact_report = + Option.value !from_json_config_impact_report + ~default:(ResultsDirEntryName.get_path ~results_dir:!results_dir ReportConfigImpactJson) + + and from_json_costs_report = Option.value !from_json_costs_report ~default:(ResultsDirEntryName.get_path ~results_dir:!results_dir ReportCostsJson) diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index 4f4d22423..e346af793 100644 --- a/infer/src/base/Config.mli +++ b/infer/src/base/Config.mli @@ -228,6 +228,10 @@ val command : InferCommand.t val config_impact_data_file : string option +val config_impact_issues_tests : string option + +val config_impact_max_callees_to_print : int + val continue_analysis : bool val continue_capture : bool @@ -302,6 +306,8 @@ val force_integration : build_system option val from_json_report : string +val from_json_config_impact_report : string + val from_json_costs_report : string val frontend_stats : bool diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index bbce93a85..f8a8ed8ad 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -135,7 +135,7 @@ val condition_always_true : t val config_checks_between_markers : t -val config_impact_analysis : t +val config_impact_analysis : t [@@warning "-32"] val constant_address_dereference : t diff --git a/infer/src/checkers/ConfigImpactAnalysis.ml b/infer/src/checkers/ConfigImpactAnalysis.ml index c066ff903..929a4c590 100644 --- a/infer/src/checkers/ConfigImpactAnalysis.ml +++ b/infer/src/checkers/ConfigImpactAnalysis.ml @@ -47,6 +47,8 @@ module UncheckedCallee = struct and call_type = Direct | Indirect of t + let get_callee {callee} = callee + let get_location {location} = location let pp_common ~with_location f {callee; location; call_type} = @@ -59,34 +61,33 @@ module UncheckedCallee = struct let pp_without_location f x = pp_common ~with_location:false f x + let pp_without_location_list f unchecked_callees = + IList.pp_print_list ~max:Config.config_impact_max_callees_to_print + ~pp_sep:(fun f () -> Format.pp_print_string f ", ") + (fun f unchecked_callee -> Procname.pp f (get_callee unchecked_callee)) + f unchecked_callees + + let replace_location_by_call location x = {x with location; call_type= Indirect x} - let rec make_err_trace x = + let[@warning "-32"] rec make_err_trace x = let desc = F.asprintf "%a" pp_without_location x in let trace_elem = Errlog.make_trace_element 0 (get_location x) desc [] in match x.call_type with Direct -> [trace_elem] | Indirect x -> trace_elem :: make_err_trace x - - - let report {InterproceduralAnalysis.proc_desc; err_log} x = - 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 include AbstractDomain.FiniteSet (UncheckedCallee) - let report analysis_data unchecked_callees = - iter (UncheckedCallee.report analysis_data) unchecked_callees - - let replace_location_by_call location x = map (UncheckedCallee.replace_location_by_call location) x let encode astate = Marshal.to_string astate [] |> Base64.encode_exn + + let decode enc_str = Marshal.from_string (Base64.decode_exn enc_str) 0 + + let pp_without_location f x = UncheckedCallee.pp_without_location_list f (elements x) end module Loc = struct @@ -279,8 +280,6 @@ let has_call_stmt proc_desc = let checker ({InterproceduralAnalysis.proc_desc} as analysis_data) = - Option.map (Analyzer.compute_post analysis_data ~initial:Dom.init proc_desc) - ~f:(fun ({Dom.unchecked_callees} as astate) -> + Option.map (Analyzer.compute_post analysis_data ~initial:Dom.init proc_desc) ~f:(fun astate -> let has_call_stmt = has_call_stmt proc_desc in - UncheckedCallees.report analysis_data unchecked_callees ; Dom.to_summary has_call_stmt astate ) diff --git a/infer/src/checkers/ConfigImpactAnalysis.mli b/infer/src/checkers/ConfigImpactAnalysis.mli index 661020c0c..3dabcd120 100644 --- a/infer/src/checkers/ConfigImpactAnalysis.mli +++ b/infer/src/checkers/ConfigImpactAnalysis.mli @@ -11,6 +11,10 @@ module UncheckedCallees : sig type t val encode : t -> string + + val decode : string -> t + + val pp_without_location : Format.formatter -> t -> unit end module Summary : sig diff --git a/infer/src/infer.ml b/infer/src/infer.ml index e0939f0d4..98e74e7c0 100644 --- a/infer/src/infer.ml +++ b/infer/src/infer.ml @@ -193,16 +193,17 @@ let () = CostIssuesTest.write_from_json ~json_path:Config.from_json_costs_report ~out_path CostIssuesTestField.all_fields in - match (Config.issues_tests, Config.cost_issues_tests) with - | None, None -> + let write_from_config_impact_json out_path = + ConfigImpactIssuesTest.write_from_json ~json_path:Config.from_json_config_impact_report + ~out_path + in + match (Config.issues_tests, Config.cost_issues_tests, Config.config_impact_issues_tests) with + | None, None, None -> if not Config.quiet then L.result "%t" Summary.OnDisk.pp_specs_from_config - | Some out_path, Some cost_out_path -> - write_from_json out_path ; - write_from_cost_json cost_out_path - | None, Some cost_out_path -> - write_from_cost_json cost_out_path - | Some out_path, None -> - write_from_json out_path ) + | out_path, cost_out_path, config_impact_out_path -> + Option.iter out_path ~f:write_from_json ; + Option.iter cost_out_path ~f:write_from_cost_json ; + Option.iter config_impact_out_path ~f:write_from_config_impact_json ) | ReportDiff -> (* at least one report must be passed in input to compute differential *) ( match Config.(report_current, report_previous, costs_current, costs_previous) with diff --git a/infer/src/integration/ConfigImpactIssuesTest.ml b/infer/src/integration/ConfigImpactIssuesTest.ml new file mode 100644 index 000000000..6c057baa4 --- /dev/null +++ b/infer/src/integration/ConfigImpactIssuesTest.ml @@ -0,0 +1,38 @@ +(* + * 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 F = Format + +let pp_custom_of_config_impact_report fmt report = + let pp_custom_of_config_impact_issue fmt (config_impact_item : Jsonbug_t.config_impact_item) = + let open Jsonbug_t in + F.fprintf fmt "%s, %s, {%a}@\n" config_impact_item.procedure_name config_impact_item.loc.file + ConfigImpactAnalysis.UncheckedCallees.pp_without_location + (ConfigImpactAnalysis.UncheckedCallees.decode config_impact_item.unchecked_callees) + in + List.iter ~f:(pp_custom_of_config_impact_issue fmt) report + + +let config_impact_tests_jsonbug_compare (x : Jsonbug_t.config_impact_item) + (y : Jsonbug_t.config_impact_item) = + let open Jsonbug_t in + [%compare: string * string * string] + (x.loc.file, x.procedure_id, x.hash) + (y.loc.file, y.procedure_id, y.hash) + + +let write_from_json ~json_path ~out_path = + Utils.with_file_out out_path ~f:(fun outf -> + let config_impact_report = + Atdgen_runtime.Util.Json.from_file Jsonbug_j.read_config_impact_report json_path + in + let sorted_config_impact_report = + List.sort ~compare:config_impact_tests_jsonbug_compare config_impact_report + in + pp_custom_of_config_impact_report (F.formatter_of_out_channel outf) + sorted_config_impact_report ) diff --git a/infer/src/integration/ConfigImpactIssuesTest.mli b/infer/src/integration/ConfigImpactIssuesTest.mli new file mode 100644 index 000000000..70433117c --- /dev/null +++ b/infer/src/integration/ConfigImpactIssuesTest.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 write_from_json : json_path:string -> out_path:string -> unit diff --git a/infer/tests/codetoanalyze/java/fb-config-impact/Makefile b/infer/tests/codetoanalyze/java/fb-config-impact/Makefile index 54e4a241d..996463e34 100644 --- a/infer/tests/codetoanalyze/java/fb-config-impact/Makefile +++ b/infer/tests/codetoanalyze/java/fb-config-impact/Makefile @@ -5,9 +5,10 @@ TESTS_DIR = ../../.. -INFER_OPTIONS = --config-impact-analysis-only --config-impact-data-file config_data.json --debug-exceptions \ - --report-force-relative-path +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) include $(TESTS_DIR)/javac.make +include $(TESTS_DIR)/config-impact.make diff --git a/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile b/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile index c0bd51bb6..a7e51fa5c 100644 --- a/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile +++ b/infer/tests/codetoanalyze/objc/fb-config-impact/Makefile @@ -14,5 +14,6 @@ SOURCES = $(wildcard *.m) include $(TESTS_DIR)/clang.make include $(TESTS_DIR)/objc.make +include $(TESTS_DIR)/config-impact.make infer-out/report.json: $(MAKEFILE_LIST) diff --git a/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp b/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp index dde6fc690..e69de29bb 100644 --- a/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp +++ b/infer/tests/codetoanalyze/objc/fb-config-impact/issues.exp @@ -1,3 +0,0 @@ -../../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] diff --git a/infer/tests/config-impact.make b/infer/tests/config-impact.make new file mode 100644 index 000000000..9f4f82255 --- /dev/null +++ b/infer/tests/config-impact.make @@ -0,0 +1,27 @@ +# 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. + +# We add issues.exp.test to recipe in order to avoid "infer report"s run parallel +config-impact-issues.exp.test: $(INFER_OUT)/report.json $(INFER_BIN) issues.exp.test + $(QUIET)$(INFER_BIN) report -q --results-dir $(