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

std/src/panicking.rs

@@ -298,8 +298,18 @@ pub mod panic_count {
 
     pub const ALWAYS_ABORT_FLAG: usize = 1 << (usize::BITS - 1);
 
-    // Panic count for the current thread.
-    thread_local! { static LOCAL_PANIC_COUNT: Cell<usize> = const { Cell::new(0) } }
+    /// A reason for forcing an immediate abort on panic.
+    #[derive(Debug)]
+    pub enum MustAbort {
+        AlwaysAbort,
+        PanicInHook,
+    }
+
+    // Panic count for the current thread and whether a panic hook is currently
+    // being executed..
+    thread_local! {
+        static LOCAL_PANIC_COUNT: Cell<(usize, bool)> = const { Cell::new((0, false)) }
+    }
 
     // Sum of panic counts from all threads. The purpose of this is to have
     // a fast path in `count_is_zero` (which is used by `panicking`). In any particular
@@ -328,34 +338,39 @@ pub mod panic_count {
     // panicking thread consumes at least 2 bytes of address space.
     static GLOBAL_PANIC_COUNT: AtomicUsize = AtomicUsize::new(0);
 
-    // Return the state of the ALWAYS_ABORT_FLAG and number of panics.
+    // Increases the global and local panic count, and returns whether an
+    // immediate abort is required.
     //
-    // If ALWAYS_ABORT_FLAG is not set, the number is determined on a per-thread
-    // base (stored in LOCAL_PANIC_COUNT), i.e. it is the amount of recursive calls
-    // of the calling thread.
-    // If ALWAYS_ABORT_FLAG is set, the number equals the *global* number of panic
-    // calls. See above why LOCAL_PANIC_COUNT is not used.
-    pub fn increase() -> (bool, usize) {
+    // This also updates thread-local state to keep track of whether a panic
+    // hook is currently executing.
+    pub fn increase(run_panic_hook: bool) -> Option<MustAbort> {
         let global_count = GLOBAL_PANIC_COUNT.fetch_add(1, Ordering::Relaxed);
-        let must_abort = global_count & ALWAYS_ABORT_FLAG != 0;
-        let panics = if must_abort {
-            global_count & !ALWAYS_ABORT_FLAG
-        } else {
-            LOCAL_PANIC_COUNT.with(|c| {
-                let next = c.get() + 1;
-                c.set(next);
-                next
-            })
-        };
-        (must_abort, panics)
+        if global_count & ALWAYS_ABORT_FLAG != 0 {
+            return Some(MustAbort::AlwaysAbort);
+        }
+
+        LOCAL_PANIC_COUNT.with(|c| {
+            let (count, in_panic_hook) = c.get();
+            if in_panic_hook {
+                return Some(MustAbort::PanicInHook);
+            }
+            c.set((count + 1, run_panic_hook));
+            None
+        })
+    }
+
+    pub fn finished_panic_hook() {
+        LOCAL_PANIC_COUNT.with(|c| {
+            let (count, _) = c.get();
+            c.set((count, false));
+        });
     }
 
     pub fn decrease() {
         GLOBAL_PANIC_COUNT.fetch_sub(1, Ordering::Relaxed);
         LOCAL_PANIC_COUNT.with(|c| {
-            let next = c.get() - 1;
-            c.set(next);
-            next
+            let (count, _) = c.get();
+            c.set((count - 1, false));
         });
     }
 
@@ -366,7 +381,7 @@ pub mod panic_count {
     // Disregards ALWAYS_ABORT_FLAG
     #[must_use]
     pub fn get_count() -> usize {
-        LOCAL_PANIC_COUNT.with(|c| c.get())
+        LOCAL_PANIC_COUNT.with(|c| c.get().0)
     }
 
     // Disregards ALWAYS_ABORT_FLAG
@@ -394,7 +409,7 @@ pub mod panic_count {
     #[inline(never)]
     #[cold]
     fn is_zero_slow_path() -> bool {
-        LOCAL_PANIC_COUNT.with(|c| c.get() == 0)
+        LOCAL_PANIC_COUNT.with(|c| c.get().0 == 0)
     }
 }
 
@@ -655,23 +670,22 @@ fn rust_panic_with_hook(
     location: &Location<'_>,
     can_unwind: bool,
 ) -> ! {
-    let (must_abort, panics) = panic_count::increase();
-
-    // If this is the third nested call (e.g., panics == 2, this is 0-indexed),
-    // the panic hook probably triggered the last panic, otherwise the
-    // double-panic check would have aborted the process. In this case abort the
-    // process real quickly as we don't want to try calling it again as it'll
-    // probably just panic again.
-    if must_abort || panics > 2 {
-        if panics > 2 {
-            // Don't try to print the message in this case
-            // - perhaps that is causing the recursive panics.
-            rtprintpanic!("thread panicked while processing panic. aborting.\n");
-        } else {
-            // Unfortunately, this does not print a backtrace, because creating
-            // a `Backtrace` will allocate, which we must to avoid here.
-            let panicinfo = PanicInfo::internal_constructor(message, location, can_unwind);
-            rtprintpanic!("{panicinfo}\npanicked after panic::always_abort(), aborting.\n");
+    let must_abort = panic_count::increase(true);
+
+    // Check if we need to abort immediately.
+    if let Some(must_abort) = must_abort {
+        match must_abort {
+            panic_count::MustAbort::PanicInHook => {
+                // Don't try to print the message in this case
+                // - perhaps that is causing the recursive panics.
+                rtprintpanic!("thread panicked while processing panic. aborting.\n");
+            }
+            panic_count::MustAbort::AlwaysAbort => {
+                // Unfortunately, this does not print a backtrace, because creating
+                // a `Backtrace` will allocate, which we must to avoid here.
+                let panicinfo = PanicInfo::internal_constructor(message, location, can_unwind);
+                rtprintpanic!("{panicinfo}\npanicked after panic::always_abort(), aborting.\n");
+            }
         }
         crate::sys::abort_internal();
     }
@@ -697,16 +711,16 @@ fn rust_panic_with_hook(
     };
     drop(hook);
 
-    if panics > 1 || !can_unwind {
-        // If a thread panics while it's already unwinding then we
-        // have limited options. Currently our preference is to
-        // just abort. In the future we may consider resuming
-        // unwinding or otherwise exiting the thread cleanly.
-        if !can_unwind {
-            rtprintpanic!("thread caused non-unwinding panic. aborting.\n");
-        } else {
-            rtprintpanic!("thread panicked while panicking. aborting.\n");
-        }
+    // Indicate that we have finished executing the panic hook. After this point
+    // it is fine if there is a panic while executing destructors, as long as it
+    // it contained within a `catch_unwind`.
+    panic_count::finished_panic_hook();
+
+    if !can_unwind {
+        // If a thread panics while running destructors or tries to unwind
+        // through a nounwind function (e.g. extern "C") then we cannot continue
+        // unwinding and have to abort immediately.
+        rtprintpanic!("thread caused non-unwinding panic. aborting.\n");
         crate::sys::abort_internal();
     }
 
@@ -716,7 +730,7 @@ fn rust_panic_with_hook(
 /// This is the entry point for `resume_unwind`.
 /// It just forwards the payload to the panic runtime.
 pub fn rust_panic_without_hook(payload: Box<dyn Any + Send>) -> ! {
-    panic_count::increase();
+    panic_count::increase(false);
 
     struct RewrapBox(Box<dyn Any + Send>);