From f76ed887419cc8765f8d9d5ec14affedea9edd21 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Fri, 23 Aug 2019 03:54:32 -0700 Subject: [PATCH] [clang] wrap test determinator in frontend exception catcher Summary: The clang frontend has bugs. When a bug we know about happens some exception is raised and, most of the time, logged away so as not to crash the whole process. This catching of exceptions wasn't done from testDeterminator so it could crash where capture didn't. This diff wraps the crashy function in test determinator to avoid that. Reviewed By: ngorogiannis Differential Revision: D16963178 fbshipit-source-id: 87a4ff70b --- infer/src/clang/Capture.ml | 6 ++-- infer/src/clang/astToRangeMap.ml | 13 ++++++-- infer/src/clang/astToRangeMap.mli | 3 +- infer/src/clang/cFrontend_config.ml | 2 ++ infer/src/clang/cFrontend_config.mli | 2 ++ infer/src/clang/cFrontend_decl.ml | 35 +++------------------- infer/src/clang/cFrontend_errors.ml | 28 +++++++++++++++++ infer/src/clang/cFrontend_errors.mli | 9 ++++++ infer/src/clang/cLocation.mli | 2 +- infer/src/clang/cModule_type.ml | 4 +-- infer/src/infer.ml | 4 +-- infer/src/integration/testDeterminator.ml | 2 +- infer/src/integration/testDeterminator.mli | 6 +++- 13 files changed, 72 insertions(+), 44 deletions(-) diff --git a/infer/src/clang/Capture.ml b/infer/src/clang/Capture.ml index da8f783ad..e1c600527 100644 --- a/infer/src/clang/Capture.ml +++ b/infer/src/clang/Capture.ml @@ -101,8 +101,10 @@ let run_clang_frontend ast_source = if Config.linters then AL.do_frontend_checks trans_unit_ctx ast_decl ; if Config.export_changed_functions then ( let source_file = trans_unit_ctx.CFrontend_config.source_file in - let process_fn = AstToRangeMap.process_ast ast_decl (Tenv.create ()) source_file in - TestDeterminator.test_to_run_clang source_file ~process_ast_fn:process_fn + let process_ast_fn = + AstToRangeMap.process_ast trans_unit_ctx ast_decl (Tenv.create ()) source_file + in + TestDeterminator.test_to_run_clang source_file ~process_ast_fn ~changed_lines_file:Config.modified_lines ~test_samples_file:None ; TestDeterminator.emit_relevant_methods () ) ; if Config.capture then CFrontend.do_source_file trans_unit_ctx ast_decl ; diff --git a/infer/src/clang/astToRangeMap.ml b/infer/src/clang/astToRangeMap.ml index 8db30fbfe..f5154ce06 100644 --- a/infer/src/clang/astToRangeMap.ml +++ b/infer/src/clang/astToRangeMap.ml @@ -5,11 +5,20 @@ * LICENSE file in the root directory of this source tree. *) open! IStd +module F = Format module L = Logging -let process_ast ast tenv default_source_file f = +let process_ast trans_unit_ctx ast tenv default_source_file f = let open Clang_ast_t in let mk_key decl = CType_decl.CProcname.from_decl ~tenv decl in + let call_f decl range = + CFrontend_errors.protect trans_unit_ctx + ~recover:(fun () -> ()) + ~pp_context:(fun f () -> + F.fprintf f "%a: processing %s" SourceFile.pp default_source_file + (Clang_ast_j.string_of_decl decl) ) + ~f:(fun () -> f (mk_key decl) range) + in let rec extract_location decl = match decl with | ObjCMethodDecl (di, _, _) @@ -19,7 +28,7 @@ let process_ast ast tenv default_source_file f = | CXXConstructorDecl (di, _, _, _, _) | CXXDestructorDecl (di, _, _, _, _) -> let range = CLocation.location_of_decl_info default_source_file di in - f (mk_key decl) range + call_f decl range | _ -> ( match Clang_ast_proj.get_decl_context_tuple decl with | Some (decls, _) -> diff --git a/infer/src/clang/astToRangeMap.mli b/infer/src/clang/astToRangeMap.mli index 0823bd8f6..ad4d92fb5 100644 --- a/infer/src/clang/astToRangeMap.mli +++ b/infer/src/clang/astToRangeMap.mli @@ -7,7 +7,8 @@ open! IStd val process_ast : - Clang_ast_t.decl + CFrontend_config.translation_unit_context + -> Clang_ast_t.decl -> Tenv.t -> SourceFile.t -> (Typ.Procname.t -> Location.t * Location.t -> unit) diff --git a/infer/src/clang/cFrontend_config.ml b/infer/src/clang/cFrontend_config.ml index c2e503e66..961f4e01e 100644 --- a/infer/src/clang/cFrontend_config.ml +++ b/infer/src/clang/cFrontend_config.ml @@ -20,6 +20,8 @@ let equal_clang_lang = [%compare.equal: clang_lang] type translation_unit_context = {lang: clang_lang; source_file: SourceFile.t; integer_type_widths: Typ.IntegerWidths.t} +type decl_trans_context = [`DeclTraversal | `Translation] + (** Constants *) let alloc = "alloc" diff --git a/infer/src/clang/cFrontend_config.mli b/infer/src/clang/cFrontend_config.mli index 620c843ba..654ea59c1 100644 --- a/infer/src/clang/cFrontend_config.mli +++ b/infer/src/clang/cFrontend_config.mli @@ -18,6 +18,8 @@ val equal_clang_lang : clang_lang -> clang_lang -> bool type translation_unit_context = {lang: clang_lang; source_file: SourceFile.t; integer_type_widths: Typ.IntegerWidths.t} +type decl_trans_context = [`DeclTraversal | `Translation] + (** Constants *) val alloc : string diff --git a/infer/src/clang/cFrontend_decl.ml b/infer/src/clang/cFrontend_decl.ml index 6d42458b5..5aa96401c 100644 --- a/infer/src/clang/cFrontend_decl.ml +++ b/infer/src/clang/cFrontend_decl.ml @@ -12,34 +12,6 @@ module F = Format module L = Logging -let protect ~f ~recover ~pp_context (trans_unit_ctx : CFrontend_config.translation_unit_context) = - let log_and_recover ~print fmt = - recover () ; - incr CFrontend_config.procedures_failed ; - (if print then L.internal_error else L.(debug Capture Quiet)) ("%a@\n" ^^ fmt) pp_context () - in - try f () with - (* Always keep going in case of known limitations of the frontend, crash otherwise (by not - catching the exception) unless `--keep-going` was passed. Print errors we should fix - (t21762295) to the console. *) - | CFrontend_errors.Unimplemented e -> - ClangLogging.log_caught_exception trans_unit_ctx "Unimplemented" e.position e.source_range - e.ast_node ; - log_and_recover ~print:false "Unimplemented feature:@\n %s@\n" e.msg - | CFrontend_errors.IncorrectAssumption e -> - (* FIXME(t21762295): we do not expect this to happen but it does *) - ClangLogging.log_caught_exception trans_unit_ctx "IncorrectAssumption" e.position - e.source_range e.ast_node ; - log_and_recover ~print:true "Known incorrect assumption in the frontend: %s@\n" e.msg - | exn -> - let trace = Backtrace.get () in - IExn.reraise_if exn ~f:(fun () -> - L.internal_error "%a: %a@\n%!" pp_context () Exn.pp exn ; - not Config.keep_going ) ; - log_and_recover ~print:true "Frontend error: %a@\nBacktrace:@\n%s" Exn.pp exn - (Backtrace.to_string trace) - - module CFrontend_decl_funct (T : CModule_type.CTranslation) : CModule_type.CFrontend = struct let model_exists procname = (not Config.biabduction_models_mode) && Summary.OnDisk.has_model procname @@ -52,6 +24,7 @@ module CFrontend_decl_funct (T : CModule_type.CTranslation) : CModule_type.CFron "@\n@\n>>---------- ADDING METHOD: '%a' ---------<<@\n@\n" Typ.Procname.pp procname ; incr CFrontend_config.procedures_attempted ; let recover () = + incr CFrontend_config.procedures_failed ; Typ.Procname.Hash.remove cfg procname ; let method_kind = ms.CMethodSignature.method_kind in CMethod_trans.create_external_procdesc trans_unit_ctx cfg procname method_kind None @@ -90,7 +63,7 @@ module CFrontend_decl_funct (T : CModule_type.CTranslation) : CModule_type.CFron | exception Caml.Not_found -> () in - protect ~f ~recover ~pp_context trans_unit_ctx + CFrontend_errors.protect ~f ~recover ~pp_context trans_unit_ctx let function_decl trans_unit_ctx tenv cfg func_decl block_data_opt = @@ -446,9 +419,9 @@ module CFrontend_decl_funct (T : CModule_type.CTranslation) : CModule_type.CFron in let method_decls, no_method_decls = List.partition_tf ~f:is_method_decl decl_list in List.iter ~f:translate no_method_decls ; - protect + CFrontend_errors.protect ~f:(fun () -> ignore (CType_decl.add_types_from_decl_to_tenv tenv dec)) - ~recover:Fn.id + ~recover:(fun () -> ()) ~pp_context:(fun fmt () -> F.fprintf fmt "Error adding types from decl '%a'" (Pp.to_string ~f:Clang_ast_j.string_of_decl) diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 70b9f507d..dedc42225 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -7,6 +7,7 @@ open! IStd module F = Format +module L = Logging type exception_details = { msg: string @@ -26,3 +27,30 @@ let unimplemented position source_range ?ast_node fmt = let incorrect_assumption position source_range ?ast_node fmt = F.kasprintf (fun msg -> raise (IncorrectAssumption {msg; position; source_range; ast_node})) fmt + + +let protect ~f ~recover ~pp_context (trans_unit_ctx : CFrontend_config.translation_unit_context) = + let log_and_recover ~print fmt = + recover () ; + (if print then L.internal_error else L.(debug Capture Quiet)) ("%a@\n" ^^ fmt) pp_context () + in + try f () with + (* Always keep going in case of known limitations of the frontend, crash otherwise (by not + catching the exception) unless `--keep-going` was passed. Print errors we should fix + (t21762295) to the console. *) + | Unimplemented e -> + ClangLogging.log_caught_exception trans_unit_ctx "Unimplemented" e.position e.source_range + e.ast_node ; + log_and_recover ~print:false "Unimplemented feature:@\n %s@\n" e.msg + | IncorrectAssumption e -> + (* FIXME(t21762295): we do not expect this to happen but it does *) + ClangLogging.log_caught_exception trans_unit_ctx "IncorrectAssumption" e.position + e.source_range e.ast_node ; + log_and_recover ~print:true "Known incorrect assumption in the frontend: %s@\n" e.msg + | exn -> + let trace = Backtrace.get () in + IExn.reraise_if exn ~f:(fun () -> + L.internal_error "%a: %a@\n%!" pp_context () Exn.pp exn ; + not Config.keep_going ) ; + log_and_recover ~print:true "Frontend error: %a@\nBacktrace:@\n%s" Exn.pp exn + (Backtrace.to_string trace) diff --git a/infer/src/clang/cFrontend_errors.mli b/infer/src/clang/cFrontend_errors.mli index 6c7d03929..febfbb13c 100644 --- a/infer/src/clang/cFrontend_errors.mli +++ b/infer/src/clang/cFrontend_errors.mli @@ -36,3 +36,12 @@ val incorrect_assumption : -> 'a (** Used to mark places in the frontend that incorrectly assume something to be impossible. TODO(t21762295) get rid of all instances of this. *) + +val protect : + f:(unit -> unit) + -> recover:(unit -> unit) + -> pp_context:(Format.formatter -> unit -> unit) + -> CFrontend_config.translation_unit_context + -> unit +(** Catch frontend errors in [f] to avoid crashing due to bugs in the frontend. Upon error [recover] + is run and [pp_context] is used to provide more info to the user. *) diff --git a/infer/src/clang/cLocation.mli b/infer/src/clang/cLocation.mli index e9ecb2fea..cb2a1103a 100644 --- a/infer/src/clang/cLocation.mli +++ b/infer/src/clang/cLocation.mli @@ -14,7 +14,7 @@ val clang_to_sil_location : SourceFile.t -> Clang_ast_t.source_location -> Locat val should_translate_lib : SourceFile.t -> Clang_ast_t.source_range - -> CModule_type.decl_trans_context + -> CFrontend_config.decl_trans_context -> translate_when_used:bool -> bool diff --git a/infer/src/clang/cModule_type.ml b/infer/src/clang/cModule_type.ml index ca2e519c2..9a05edc3c 100644 --- a/infer/src/clang/cModule_type.ml +++ b/infer/src/clang/cModule_type.ml @@ -12,8 +12,6 @@ type block_data = CContext.t * Clang_ast_t.qual_type * Typ.Procname.t * (Pvar.t type instr_type = [`ClangStmt of Clang_ast_t.stmt | `CXXConstructorInit of Clang_ast_t.cxx_ctor_initializer] -type decl_trans_context = [`DeclTraversal | `Translation] - module type CTranslation = sig (** Translates instructions: (statements and expressions) from the ast into sil *) @@ -42,7 +40,7 @@ module type CFrontend = sig CFrontend_config.translation_unit_context -> Tenv.t -> Cfg.t - -> decl_trans_context + -> CFrontend_config.decl_trans_context -> Clang_ast_t.decl -> unit end diff --git a/infer/src/infer.ml b/infer/src/infer.ml index db8519487..399da4033 100644 --- a/infer/src/infer.ml +++ b/infer/src/infer.ml @@ -132,8 +132,8 @@ let () = L.progress "Logs in %s@." (Config.results_dir ^/ Config.log_file) ; L.progress "Execution ID %Ld@." Config.execution_id ) ; ( if Config.test_determinator then ( - TestDeterminator.test_to_run_java Config.modified_lines Config.profiler_samples - Config.method_decls_info ; + TestDeterminator.test_to_run_java ~changed_lines_file:Config.modified_lines + ~test_samples_file:Config.profiler_samples ~code_graph_file:Config.method_decls_info ; TestDeterminator.emit_tests_to_run () ) else match Config.command with diff --git a/infer/src/integration/testDeterminator.ml b/infer/src/integration/testDeterminator.ml index fe80a446b..6f312e9b6 100644 --- a/infer/src/integration/testDeterminator.ml +++ b/infer/src/integration/testDeterminator.ml @@ -283,7 +283,7 @@ let test_to_run_clang source_file ~process_ast_fn ~changed_lines_file ~test_samp relevant_tests := List.append test_to_run !relevant_tests -let test_to_run_java changed_lines_file test_samples_file code_graph_file = +let test_to_run_java ~changed_lines_file ~test_samples_file ~code_graph_file = L.(debug TestDeterminator Quiet) "***** Start Java Test Determinator *****@\n" ; if is_test_determinator_init () then () else init_java changed_lines_file test_samples_file code_graph_file ; diff --git a/infer/src/integration/testDeterminator.mli b/infer/src/integration/testDeterminator.mli index b534af1be..7184ffda7 100644 --- a/infer/src/integration/testDeterminator.mli +++ b/infer/src/integration/testDeterminator.mli @@ -7,7 +7,11 @@ open! IStd -val test_to_run_java : string option -> string option -> string option -> unit +val test_to_run_java : + changed_lines_file:string option + -> test_samples_file:string option + -> code_graph_file:string option + -> unit val test_to_run_clang : SourceFile.t