From de607f1b5cc99d8ac773205702e184c2461d5e12 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sat, 29 Apr 2023 07:23:54 +0100 Subject: [PATCH] Add support for nested panics to miri --- src/tools/miri/src/concurrency/thread.rs | 19 ++++++++---- src/tools/miri/src/shims/panic.rs | 5 ++-- .../miri/tests/fail/panic/double_panic.rs | 3 +- .../miri/tests/fail/panic/double_panic.stderr | 29 +++++-------------- .../tests/pass/panic/nested_panic_caught.rs | 25 ++++++++++++++++ .../pass/panic/nested_panic_caught.stderr | 4 +++ 6 files changed, 53 insertions(+), 32 deletions(-) create mode 100644 src/tools/miri/tests/pass/panic/nested_panic_caught.rs create mode 100644 src/tools/miri/tests/pass/panic/nested_panic_caught.stderr diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index d85cac7bcbb58..25c8df43ee26c 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/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>, + /// + /// 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>, /// Last OS error location in memory. It is a 32-bit integer. pub(crate) last_error: Option>, @@ -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) diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index 18ae01a19f914..7aefdfcb976af 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/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()?; diff --git a/src/tools/miri/tests/fail/panic/double_panic.rs b/src/tools/miri/tests/fail/panic/double_panic.rs index 9378adb8609df..adb30714269e8 100644 --- a/src/tools/miri/tests/fail/panic/double_panic.rs +++ b/src/tools/miri/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"); } diff --git a/src/tools/miri/tests/fail/panic/double_panic.stderr b/src/tools/miri/tests/fail/panic/double_panic.stderr index 77d5fc5d7ceb9..b6ac56f15d4b5 100644 --- a/src/tools/miri/tests/fail/panic/double_panic.stderr +++ b/src/tools/miri/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 `::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:: - 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 diff --git a/src/tools/miri/tests/pass/panic/nested_panic_caught.rs b/src/tools/miri/tests/pass/panic/nested_panic_caught.rs new file mode 100644 index 0000000000000..884813150ad2a --- /dev/null +++ b/src/tools/miri/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()); +} diff --git a/src/tools/miri/tests/pass/panic/nested_panic_caught.stderr b/src/tools/miri/tests/pass/panic/nested_panic_caught.stderr new file mode 100644 index 0000000000000..4e2593242df75 --- /dev/null +++ b/src/tools/miri/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: