From e1f19192c07caf2fa0a2ea4fd6692e3de9cddbcd Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Mon, 22 Aug 2016 04:03:17 -0700 Subject: [PATCH] move --fail-on-issue functionality to OCaml Summary: This was causing headaches as catching error code 2 may hide real issues. While there, move crashcontext finalizer code to crashcontext.ml, and create a .mli file for that module. Reviewed By: jberdine Differential Revision: D3742785 fbshipit-source-id: 3032451 --- infer/lib/python/infer.py | 10 ----- infer/lib/python/inferlib/analyze.py | 4 -- infer/lib/python/inferlib/config.py | 4 -- infer/src/backend/config.ml | 9 +++- infer/src/backend/config.mli | 2 + infer/src/backend/crashcontext.ml | 28 ++++++++++++- infer/src/backend/crashcontext.mli | 10 +++++ infer/src/backend/infer.ml | 63 ++++++++++------------------ infer/src/backend/jsonbug.atd | 2 + 9 files changed, 70 insertions(+), 62 deletions(-) create mode 100644 infer/src/backend/crashcontext.mli diff --git a/infer/lib/python/infer.py b/infer/lib/python/infer.py index 27c31ba73..01d53a215 100755 --- a/infer/lib/python/infer.py +++ b/infer/lib/python/infer.py @@ -224,15 +224,5 @@ def main(): args.infer_out, buck_out_for_stats_aggregator) logging.info(output) - if args.fail_on_bug: - bugs_filename = os.path.join(args.infer_out, - config.JSON_REPORT_FILENAME) - try: - bugs = utils.load_json_from_path(bugs_filename) - if len(bugs) > 0: - sys.exit(config.BUG_FOUND_ERROR_CODE) - except OSError: - pass - if __name__ == '__main__': main() diff --git a/infer/lib/python/inferlib/analyze.py b/infer/lib/python/inferlib/analyze.py index 8ddda175a..74a54de8b 100644 --- a/infer/lib/python/inferlib/analyze.py +++ b/infer/lib/python/inferlib/analyze.py @@ -113,10 +113,6 @@ base_group.add_argument('-st', '--stacktrace', use with e.g. -a crashcontext. See tests/codetoanalyze/java/crashcontext/*.json for examples of the expected format.''') -base_group.add_argument('--fail-on-bug', action='store_true', - help='''Exit with error code %d if Infer found - something to report''' - % config.BUG_FOUND_ERROR_CODE) base_group.add_argument('--android-harness', action='store_true', help='''[experimental] Create harness to detect bugs diff --git a/infer/lib/python/inferlib/config.py b/infer/lib/python/inferlib/config.py index 3acf55d1a..0a530961c 100644 --- a/infer/lib/python/inferlib/config.py +++ b/infer/lib/python/inferlib/config.py @@ -61,10 +61,6 @@ BUCK_INFER_OUT = 'infer' SUPRESS_WARNINGS_OUTPUT_FILENAME_OPTION = 'SuppressWarningsOutputFilename' -# exit value when infer finds something to report -BUG_FOUND_ERROR_CODE = 2 - - # list of possible analyzers ANALYZER_INFER = 'infer' ANALYZER_ERADICATE = 'eradicate' diff --git a/infer/src/backend/config.ml b/infer/src/backend/config.ml index 417e034b1..86b57d486 100644 --- a/infer/src/backend/config.ml +++ b/infer/src/backend/config.ml @@ -109,6 +109,9 @@ let default_in_zip_results_dir = "infer" (** Dotty output filename **) let dotty_output = "icfg.dot" +(** exit code to use for the --fail-on-issue option *) +let fail_on_issue_exit_code = 2 + (** If true, filter out errors in low likelyhood buckets, and only show then in developer mode *) let filter_buckets = false @@ -124,6 +127,9 @@ let idempotent_getters = true level *) let incremental_procs = true +(** Our Python script does its own argument parsing and will fail with this error on failure *) +let infer_py_argparse_error_exit_code = 22 + (** Name of the infer configuration file *) let inferconfig_file = ".inferconfig" @@ -716,7 +722,8 @@ and err_file = and fail_on_bug = CLOpt.mk_bool ~deprecated:["-fail-on-bug"] ~long:"fail-on-issue" ~default:false ~exes:CLOpt.[Toplevel] - "Exit with error code 2 if Infer found something to report" + (Printf.sprintf "Exit with error code %d if Infer found something to report" + fail_on_issue_exit_code) and failures_allowed = CLOpt.mk_bool ~deprecated_no:["-no_failures_allowed"] ~long:"failures-allowed" ~default:true diff --git a/infer/src/backend/config.mli b/infer/src/backend/config.mli index c84b495b3..9477562eb 100644 --- a/infer/src/backend/config.mli +++ b/infer/src/backend/config.mli @@ -73,11 +73,13 @@ val csl_analysis : bool val default_failure_name : string val default_in_zip_results_dir : string val dotty_output : string +val fail_on_issue_exit_code : int val filter_buckets : bool val frontend_stats_dir_name : string val global_tenv_filename : string val idempotent_getters : bool val incremental_procs : bool +val infer_py_argparse_error_exit_code : int val initial_analysis_time : float val ivar_attributes : string val lint_issues_dir_name : string diff --git a/infer/src/backend/crashcontext.ml b/infer/src/backend/crashcontext.ml index d2c3ce594..d1ebec246 100644 --- a/infer/src/backend/crashcontext.ml +++ b/infer/src/backend/crashcontext.ml @@ -76,4 +76,30 @@ let collect_all_summaries root_out_dir stacktrace_file = let path_matcher path = Str.string_match path_regexp path 0 in let method_summaries = DB.paths_matching root_out_dir path_matcher in - stitch_summaries stacktrace_file method_summaries out_file; + stitch_summaries stacktrace_file method_summaries out_file + +let crashcontext_epilogue ~in_buck_mode = + (* check whether this is the top-level infer process *) + let top_level_infer = + (* if the '--buck' option was passed, then this is the top level process iff the build + command starts with 'buck' *) + if Config.buck then in_buck_mode + (* otherwise, we assume javac as the build command and thus only one process *) + else true in + if top_level_infer then + (* if we are the top-level process, then find the output directory and collect all + crashcontext summaries under it in a single crashcontext.json file *) + let root_out_dir = if in_buck_mode then begin + let project_root = match Config.project_root with + | Some root -> root + | None -> Filename.dirname Config.results_dir in + let buck_out = match Config.buck_out with + | Some dir -> dir + | None -> "buck-out" in + Filename.concat project_root buck_out + end + else Config.results_dir in + match Config.stacktrace with + | None -> failwith "Detected -a crashcontext without --stacktrace, this should have been \ + checked earlier." + | Some s -> collect_all_summaries root_out_dir s diff --git a/infer/src/backend/crashcontext.mli b/infer/src/backend/crashcontext.mli new file mode 100644 index 000000000..e6d481f98 --- /dev/null +++ b/infer/src/backend/crashcontext.mli @@ -0,0 +1,10 @@ +(* + * 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. + *) + +val crashcontext_epilogue : in_buck_mode:bool -> unit diff --git a/infer/src/backend/infer.ml b/infer/src/backend/infer.ml index 12ec50209..258eba1a6 100644 --- a/infer/src/backend/infer.ml +++ b/infer/src/backend/infer.ml @@ -25,6 +25,15 @@ let set_env_for_clang_wrapper () = if Config.llvm then Unix.putenv "LLVM_MODE" "1" +(** as the Config.fail_on_bug flag mandates, exit with error when an issue is reported *) +let fail_on_issue_epilogue () = + let issues_json = DB.Results_dir.(path_to_filename Abs_root ["report.json"]) in + match read_file (DB.filename_to_string issues_json) with + | Some lines -> + let issues = Jsonbug_j.report_of_string @@ String.concat "" lines in + if issues <> [] then exit Config.fail_on_issue_exit_code + | None -> () + let () = set_env_for_clang_wrapper () ; (* The infer executable in the bin directory is a symbolic link to the real binary in the lib @@ -48,7 +57,7 @@ let () = in let infer_py = (Filename.dirname real_exe) // "python" // "infer.py" in let build_cmd = IList.rev Config.rest in - let buck = match build_cmd with "buck" :: _ -> true | _ -> false in + let in_buck_mode = match build_cmd with "buck" :: _ -> true | _ -> false in let args_py = Array.of_list ( infer_py :: @@ -59,14 +68,14 @@ let () = ["--analyzer"; IList.assoc (=) a (IList.map (fun (n,a) -> (a,n)) Config.string_to_analyzer)]) @ (match Config.blacklist with - | Some s when buck -> ["--blacklist-regex"; s] + | Some s when in_buck_mode -> ["--blacklist-regex"; s] | _ -> []) @ (if not Config.create_harness then [] else ["--android-harness"]) @ (if not Config.buck then [] else ["--buck"]) @ (match IList.rev Config.buck_build_args with - | args when buck -> + | args when in_buck_mode -> IList.map (fun arg -> ["--Xbuck"; "'" ^ arg ^ "'"]) args |> IList.flatten | _ -> []) @ (if not Config.continue_capture then [] else @@ -75,15 +84,13 @@ let () = ["--debug"]) @ (if not Config.debug_exceptions then [] else ["--debug-exceptions"]) @ - (if not Config.fail_on_bug then [] else - ["--fail-on-bug"]) @ (if Config.filtering then [] else ["--no-filtering"]) @ (if not Config.frontend_debug then [] else ["--frontend-debug"]) @ (if not Config.frontend_stats then [] else ["--frontend-stats"]) @ - (if not Config.flavors || not buck then [] else + (if not Config.flavors || not in_buck_mode then [] else ["--use-flavors"]) @ (match Config.infer_cache with None -> [] | Some s -> ["--infer_cache"; s]) @ @@ -104,44 +111,16 @@ let () = ) in let pid = Unix.create_process args_py.(0) args_py Unix.stdin Unix.stdout Unix.stderr in let _, status = Unix.waitpid [] pid in - if status = Unix.WEXITED 22 then - (* swallow infer.py argument parsing error *) - Config.print_usage_exit (); - (* collect crashcontext summaries *) - let analysis_is_crashcontext = match Config.analyzer with - | Some Crashcontext -> true - | _ -> false in - (if analysis_is_crashcontext then - (* check whether this is the top-level infer process *) - let top_level_infer = - (* if the '--buck' option was passed, then this is the top level process iff the build - command starts with 'buck' *) - if Config.buck then buck - (* otherwise, we assume javac as the build command and thus only one process *) - else true in - if top_level_infer then - (* if we are the top-level process, then find the output directory and collect all - crashcontext summaries under it in a single crashcontext.json file *) - let root_out_dir = if buck then begin - let project_root = match Config.project_root with - | Some root -> root - | None -> Filename.dirname Config.results_dir in - let buck_out = match Config.buck_out with - | Some dir -> dir - | None -> "buck-out" in - Filename.concat project_root buck_out - end - else Config.results_dir in - match Config.stacktrace with - | None -> failwith "Detected -a crashcontext without --stacktrace, \ - this should have been checked earlier." - | Some s -> Crashcontext.collect_all_summaries root_out_dir s - ); let exit_code = match status with | Unix.WEXITED i -> i | _ -> 1 in + if exit_code = Config.infer_py_argparse_error_exit_code then + (* swallow infer.py argument parsing error *) + Config.print_usage_exit (); + if Config.analyzer = Some Config.Crashcontext then + Crashcontext.crashcontext_epilogue ~in_buck_mode; if exit_code <> 0 then ( - if not (exit_code = 2 && Config.fail_on_bug) then - prerr_endline ("Failed to execute: " ^ (String.concat " " (Array.to_list args_py))) ; + prerr_endline ("Failed to execute: " ^ (String.concat " " (Array.to_list args_py))) ; exit exit_code - ) + ); + if Config.fail_on_bug then fail_on_issue_epilogue () diff --git a/infer/src/backend/jsonbug.atd b/infer/src/backend/jsonbug.atd index f5495e1e2..0044bc367 100644 --- a/infer/src/backend/jsonbug.atd +++ b/infer/src/backend/jsonbug.atd @@ -36,6 +36,8 @@ type jsonbug = { ?infer_source_loc: loc option; } +type report = jsonbug list + type json_trace = { trace : json_trace_item list; }