From 83f51421830a3939bbd06d780221a794aa69829b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 28 Nov 2024 14:47:21 +0000 Subject: [PATCH] Handle unwinding out of the closure argument of run_compiler with pending delayed bugs --- compiler/rustc_errors/src/lib.rs | 14 +++++++--- compiler/rustc_interface/src/interface.rs | 31 +++++++++++++---------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 8eb55e3084770..c8e45e294b667 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -493,6 +493,7 @@ struct DiagCtxtInner { lint_err_guars: Vec, /// The delayed bugs and their error guarantees. delayed_bugs: Vec<(DelayedDiagInner, ErrorGuaranteed)>, + emitted_delayed_bug: Option, /// The error count shown to the user at the end. deduplicated_err_count: usize, @@ -622,9 +623,7 @@ impl Drop for DiagCtxtInner { // Important: it is sound to produce an `ErrorGuaranteed` when emitting // delayed bugs because they are guaranteed to be emitted here if // necessary. - if self.err_guars.is_empty() { - self.flush_delayed() - } + self.flush_delayed(); // Sanity check: did we use some of the expensive `trimmed_def_paths` functions // unexpectedly, that is, without producing diagnostics? If so, for debugging purposes, we @@ -770,6 +769,7 @@ impl DiagCtxt { registry: _, err_guars, lint_err_guars, + emitted_delayed_bug, delayed_bugs, deduplicated_err_count, deduplicated_warn_count, @@ -790,6 +790,7 @@ impl DiagCtxt { // underlying memory (which `clear` would not do). *err_guars = Default::default(); *lint_err_guars = Default::default(); + *emitted_delayed_bug = None; *delayed_bugs = Default::default(); *deduplicated_err_count = 0; *deduplicated_warn_count = 0; @@ -1421,6 +1422,7 @@ impl DiagCtxtInner { registry: Registry::new(&[]), err_guars: Vec::new(), lint_err_guars: Vec::new(), + emitted_delayed_bug: None, delayed_bugs: Vec::new(), deduplicated_err_count: 0, deduplicated_warn_count: 0, @@ -1705,7 +1707,13 @@ impl DiagCtxtInner { // eventually happened. assert!(self.stashed_diagnostics.is_empty()); + if !self.err_guars.is_empty() { + // If an error happened already. We shouldn't expose delayed bugs. + return; + } + if self.delayed_bugs.is_empty() { + // Nothing to do. return; } diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 71095ca8899e3..a631e9cd93ccc 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -5,9 +5,9 @@ use std::sync::Arc; use rustc_ast::{LitKind, MetaItemKind, token}; use rustc_codegen_ssa::traits::CodegenBackend; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::jobserver; use rustc_data_structures::stable_hasher::StableHasher; use rustc_data_structures::sync::Lrc; -use rustc_data_structures::{defer, jobserver}; use rustc_errors::registry::Registry; use rustc_errors::{DiagCtxtHandle, ErrorGuaranteed}; use rustc_lint::LintStore; @@ -492,21 +492,13 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se // There are two paths out of `f`. // - Normal exit. - // - Panic, e.g. triggered by `abort_if_errors`. + // - Panic, e.g. triggered by `abort_if_errors` or a fatal error. // // We must run `finish_diagnostics` in both cases. let res = { - // If `f` panics, `finish_diagnostics` will run during - // unwinding because of the `defer`. - let sess_abort_guard = defer(|| { - compiler.sess.finish_diagnostics(); - }); + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(&compiler))); - let res = f(&compiler); - - // If `f` doesn't panic, `finish_diagnostics` will run - // normally when `sess_abort_guard` is dropped. - drop(sess_abort_guard); + compiler.sess.finish_diagnostics(); // If error diagnostics have been emitted, we can't return an // error directly, because the return type of this function @@ -515,9 +507,20 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se // mistakenly think that no errors occurred and return a zero // exit code. So we abort (panic) instead, similar to if `f` // had panicked. - compiler.sess.dcx().abort_if_errors(); + if res.is_ok() { + compiler.sess.dcx().abort_if_errors(); + } - res + // Also make sure to flush delayed bugs as if we panicked, the + // bugs would be flushed by the Drop impl of DiagCtxt while + // unwinding, which would result in an abort with + // "panic in a destructor during cleanup". + compiler.sess.dcx().flush_delayed(); + + match res { + Ok(res) => res, + Err(err) => std::panic::resume_unwind(err), + } }; let prof = compiler.sess.prof.clone();