Auto merge of #110975 - Amanieu:panic_count, r=joshtriplett

Rework handling of recursive panics

This PR makes 2 changes to how recursive panics works (a panic while handling a panic).

1. The panic count is no longer used to determine whether to force an immediate abort. This allows code like the following to work without aborting the process immediately:

```rust
struct Double;

impl Drop for Double {
    fn drop(&mut self) {
        // 2 panics are active at once, but this is fine since it is caught.
        std::panic::catch_unwind(|| panic!("twice"));
    }
}

let _d = Double;

panic!("once");
```

Rustc already generates appropriate code so that any exceptions escaping out of a `Drop` called in the unwind path will immediately abort the process.

2. Any panics while the panic hook is executing will force an immediate abort. This is necessary to avoid potential deadlocks like #110771 where a panic happens while holding the backtrace lock. We don't even try to print the panic message in this case since the panic may have been caused by `Display` impls.

Fixes #110771

Author: bors

src/concurrency/thread.rs

@@ -133,10 +133,15 @@ pub struct Thread<'mir, 'tcx> {
     /// The join status.
     join_status: ThreadJoinStatus,
 
-    /// The temporary used for storing the argument of
-    /// the call to `miri_start_panic` (the panic payload) when unwinding.
+    /// Stack of active panic payloads for the current thread. Used for storing
+    /// the argument of the call to `miri_start_panic` (the panic payload) when unwinding.
     /// This is pointer-sized, and matches the `Payload` type in `src/libpanic_unwind/miri.rs`.
-    pub(crate) panic_payload: Option<Scalar<Provenance>>,
+    ///
+    /// In real unwinding, the payload gets passed as an argument to the landing pad,
+    /// which then forwards it to 'Resume'. However this argument is implicit in MIR,
+    /// so we have to store it out-of-band. When there are multiple active unwinds,
+    /// the innermost one is always caught first, so we can store them as a stack.
+    pub(crate) panic_payloads: Vec<Scalar<Provenance>>,
 
     /// Last OS error location in memory. It is a 32-bit integer.
     pub(crate) last_error: Option<MPlaceTy<'tcx, Provenance>>,
@@ -206,7 +211,7 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {
             stack: Vec::new(),
             top_user_relevant_frame: None,
             join_status: ThreadJoinStatus::Joinable,
-            panic_payload: None,
+            panic_payloads: Vec::new(),
             last_error: None,
             on_stack_empty,
         }
@@ -216,7 +221,7 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {
 impl VisitTags for Thread<'_, '_> {
     fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
         let Thread {
-            panic_payload,
+            panic_payloads: panic_payload,
             last_error,
             stack,
             top_user_relevant_frame: _,
@@ -226,7 +231,9 @@ impl VisitTags for Thread<'_, '_> {
             on_stack_empty: _, // we assume the closure captures no GC-relevant state
         } = self;
 
-        panic_payload.visit_tags(visit);
+        for payload in panic_payload {
+            payload.visit_tags(visit);
+        }
         last_error.visit_tags(visit);
         for frame in stack {
             frame.visit_tags(visit)

src/shims/panic.rs

@@ -63,8 +63,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let [payload] = this.check_shim(abi, Abi::Rust, link_name, args)?;
         let payload = this.read_scalar(payload)?;
         let thread = this.active_thread_mut();
-        assert!(thread.panic_payload.is_none(), "the panic runtime should avoid double-panics");
-        thread.panic_payload = Some(payload);
+        thread.panic_payloads.push(payload);
 
         // Jump to the unwind block to begin unwinding.
         this.unwind_to_block(unwind)?;
@@ -146,7 +145,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
             // The Thread's `panic_payload` holds what was passed to `miri_start_panic`.
             // This is exactly the second argument we need to pass to `catch_fn`.
-            let payload = this.active_thread_mut().panic_payload.take().unwrap();
+            let payload = this.active_thread_mut().panic_payloads.pop().unwrap();
 
             // Push the `catch_fn` stackframe.
             let f_instance = this.get_ptr_fn(catch_unwind.catch_fn)?.as_instance()?;

tests/fail/panic/double_panic.rs

@@ -1,6 +1,4 @@
-//@error-in-other-file: the program aborted
 //@normalize-stderr-test: "\| +\^+" -> "| ^"
-//@normalize-stderr-test: "unsafe \{ libc::abort\(\) \}|crate::intrinsics::abort\(\);" -> "ABORT();"
 //@normalize-stderr-test: "\n  +[0-9]+:[^\n]+" -> "$1"
 //@normalize-stderr-test: "\n at [^\n]+" -> "$1"
 
@@ -11,6 +9,7 @@ impl Drop for Foo {
     }
 }
 fn main() {
+    //~^ERROR: panic in a function that cannot unwind
     let _foo = Foo;
     panic!("first");
 }

tests/fail/panic/double_panic.stderr

@@ -2,30 +2,17 @@ thread 'main' panicked at 'first', $DIR/double_panic.rs:LL:CC
 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 thread 'main' panicked at 'second', $DIR/double_panic.rs:LL:CC
 stack backtrace:
-thread panicked while panicking. aborting.
-error: abnormal termination: the program aborted execution
-  --> RUSTLIB/std/src/sys/PLATFORM/mod.rs:LL:CC
-   |
-LL |     ABORT();
-   | ^ the program aborted execution
-   |
-   = note: inside `std::sys::PLATFORM::abort_internal` at RUSTLIB/std/src/sys/PLATFORM/mod.rs:LL:CC
-   = note: inside `std::panicking::rust_panic_with_hook` at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside closure at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside `std::sys_common::backtrace::__rust_end_short_backtrace::<[closure@std::panicking::begin_panic_handler::{closure#0}], !>` at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC
-   = note: inside `std::panicking::begin_panic_handler` at RUSTLIB/std/src/panicking.rs:LL:CC
-note: inside `<Foo as std::ops::Drop>::drop`
+error: abnormal termination: panic in a function that cannot unwind
   --> $DIR/double_panic.rs:LL:CC
    |
-LL |         panic!("second");
-   | ^
-   = note: inside `std::ptr::drop_in_place::<Foo> - shim(Some(Foo))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
-note: inside `main`
-  --> $DIR/double_panic.rs:LL:CC
+LL | / fn main() {
+LL | |
+LL | |     let _foo = Foo;
+LL | |     panic!("first");
+LL | | }
+   | |_^ panic in a function that cannot unwind
    |
-LL | }
-   | ^
-   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
+   = note: inside `main` at $DIR/double_panic.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 

tests/pass/panic/nested_panic_caught.rs

@@ -0,0 +1,25 @@
+//@normalize-stderr-test: "\| +\^+" -> "| ^"
+//@normalize-stderr-test: "\n  +[0-9]+:[^\n]+" -> "$1"
+//@normalize-stderr-test: "\n at [^\n]+" -> "$1"
+
+// Checks that nested panics work correctly.
+
+use std::panic::catch_unwind;
+
+fn double() {
+    struct Double;
+
+    impl Drop for Double {
+        fn drop(&mut self) {
+            let _ = catch_unwind(|| panic!("twice"));
+        }
+    }
+
+    let _d = Double;
+
+    panic!("once");
+}
+
+fn main() {
+    assert!(catch_unwind(|| double()).is_err());
+}

tests/pass/panic/nested_panic_caught.stderr

@@ -0,0 +1,4 @@
+thread 'main' panicked at 'once', $DIR/nested_panic_caught.rs:LL:CC
+note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
+thread 'main' panicked at 'twice', $DIR/nested_panic_caught.rs:LL:CC
+stack backtrace: