Skip to content

Commit

Permalink
Rollup merge of #121598 - RalfJung:catch_unwind, r=oli-obk
Browse files Browse the repository at this point in the history
rename 'try' intrinsic to 'catch_unwind'

The intrinsic has nothing to do with `try` blocks, and corresponds to the stable `catch_unwind` function, so this makes a lot more sense IMO.

Also rename Miri's special function while we are at it, to reflect the level of abstraction it works on: it's an unwinding mechanism, on which Rust implements panics.
  • Loading branch information
matthiaskrgr authored Feb 26, 2024
2 parents 849b5b2 + af6b696 commit 8048afe
Show file tree
Hide file tree
Showing 12 changed files with 27 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/concurrency/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub struct Thread<'mir, 'tcx> {
join_status: ThreadJoinStatus,

/// 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.
/// the argument of the call to `miri_start_unwind` (the panic payload) when unwinding.
/// This is pointer-sized, and matches the `Payload` type in `src/libpanic_unwind/miri.rs`.
///
/// In real unwinding, the payload gets passed as an argument to the landing pad,
Expand Down
6 changes: 3 additions & 3 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let ret = match ret {
None =>
match link_name.as_str() {
"miri_start_panic" => {
// `check_shim` happens inside `handle_miri_start_panic`.
this.handle_miri_start_panic(abi, link_name, args, unwind)?;
"miri_start_unwind" => {
// `check_shim` happens inside `handle_miri_start_unwind`.
this.handle_miri_start_unwind(abi, link_name, args, unwind)?;
return Ok(None);
}
// This matches calls to the foreign item `panic_impl`.
Expand Down
2 changes: 1 addition & 1 deletion src/shims/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

// Some intrinsics are special and need the "ret".
match intrinsic_name {
"try" => return this.handle_try(args, dest, ret),
"catch_unwind" => return this.handle_catch_unwind(args, dest, ret),
_ => {}
}

Expand Down
16 changes: 8 additions & 8 deletions src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
//! The core pieces of the runtime are:
//! - An implementation of `__rust_maybe_catch_panic` that pushes the invoked stack frame with
//! some extra metadata derived from the panic-catching arguments of `__rust_maybe_catch_panic`.
//! - A hack in `libpanic_unwind` that calls the `miri_start_panic` intrinsic instead of the
//! - A hack in `libpanic_unwind` that calls the `miri_start_unwind` intrinsic instead of the
//! target-native panic runtime. (This lives in the rustc repo.)
//! - An implementation of `miri_start_panic` that stores its argument (the panic payload), and then
//! - An implementation of `miri_start_unwind` that stores its argument (the panic payload), and then
//! immediately returns, but on the *unwind* edge (not the normal return edge), thus initiating unwinding.
//! - A hook executed each time a frame is popped, such that if the frame pushed by `__rust_maybe_catch_panic`
//! gets popped *during unwinding*, we take the panic payload and store it according to the extra
Expand Down Expand Up @@ -44,9 +44,9 @@ impl VisitProvenance for CatchUnwindData<'_> {

impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Handles the special `miri_start_panic` intrinsic, which is called
/// Handles the special `miri_start_unwind` intrinsic, which is called
/// by libpanic_unwind to delegate the actual unwinding process to Miri.
fn handle_miri_start_panic(
fn handle_miri_start_unwind(
&mut self,
abi: Abi,
link_name: Symbol,
Expand All @@ -55,7 +55,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();

trace!("miri_start_panic: {:?}", this.frame().instance);
trace!("miri_start_unwind: {:?}", this.frame().instance);

// Get the raw pointer stored in arg[0] (the panic payload).
let [payload] = this.check_shim(abi, Abi::Rust, link_name, args)?;
Expand All @@ -69,7 +69,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}

/// Handles the `try` intrinsic, the underlying implementation of `std::panicking::try`.
fn handle_try(
fn handle_catch_unwind(
&mut self,
args: &[OpTy<'tcx, Provenance>],
dest: &PlaceTy<'tcx, Provenance>,
Expand All @@ -85,7 +85,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// what that is), and returns 1.
// The `payload` is passed (by libstd) to `__rust_panic_cleanup`, which is then expected to
// return a `Box<dyn Any + Send + 'static>`.
// In Miri, `miri_start_panic` is passed exactly that type, so we make the `payload` simply
// In Miri, `miri_start_unwind` is passed exactly that type, so we make the `payload` simply
// a pointer to `Box<dyn Any + Send + 'static>`.

// Get all the arguments.
Expand Down Expand Up @@ -141,7 +141,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// We set the return value of `try` to 1, since there was a panic.
this.write_scalar(Scalar::from_i32(1), &catch_unwind.dest)?;

// The Thread's `panic_payload` holds what was passed to `miri_start_panic`.
// The Thread's `panic_payload` holds what was passed to `miri_start_unwind`.
// This is exactly the second argument we need to pass to `catch_fn`.
let payload = this.active_thread_mut().panic_payloads.pop().unwrap();

Expand Down
2 changes: 1 addition & 1 deletion tests/fail/function_calls/check_callback_abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fn main() {
unsafe {
// Make sure we check the ABI when Miri itself invokes a function
// as part of a shim implementation.
std::intrinsics::r#try(
std::intrinsics::catch_unwind(
//~^ ERROR: calling a function with ABI C using caller ABI Rust
std::mem::transmute::<extern "C" fn(*mut u8), _>(try_fn),
std::ptr::null_mut(),
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/function_calls/check_callback_abi.stderr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error: Undefined Behavior: calling a function with ABI C using caller ABI Rust
--> $DIR/check_callback_abi.rs:LL:CC
|
LL | / std::intrinsics::r#try(
LL | / std::intrinsics::catch_unwind(
LL | |
LL | | std::mem::transmute::<extern "C" fn(*mut u8), _>(try_fn),
LL | | std::ptr::null_mut(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
#![feature(c_unwind)]

extern "C" {
fn miri_start_panic(payload: *mut u8) -> !;
fn miri_start_unwind(payload: *mut u8) -> !;
}

fn main() {
unsafe { miri_start_panic(&mut 0) }
unsafe { miri_start_unwind(&mut 0) }
//~^ ERROR: unwinding past a stack frame that does not allow unwinding
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed.
If you have a use-case for it, please file an issue.
error: Undefined Behavior: unwinding past a stack frame that does not allow unwinding
--> $DIR/bad_miri_start_panic.rs:LL:CC
--> $DIR/bad_miri_start_unwind.rs:LL:CC
|
LL | unsafe { miri_start_panic(&mut 0) }
| ^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding
LL | unsafe { miri_start_unwind(&mut 0) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `main` at $DIR/bad_miri_start_panic.rs:LL:CC
= note: inside `main` at $DIR/bad_miri_start_unwind.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

Expand Down
4 changes: 2 additions & 2 deletions tests/fail/panic/unwind_panic_abort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
//! Unwinding despite `-C panic=abort` is an error.

extern "Rust" {
fn miri_start_panic(payload: *mut u8) -> !;
fn miri_start_unwind(payload: *mut u8) -> !;
}

fn main() {
unsafe {
miri_start_panic(&mut 0); //~ ERROR: unwinding past a stack frame that does not allow unwinding
miri_start_unwind(&mut 0); //~ ERROR: unwinding past a stack frame that does not allow unwinding
}
}
4 changes: 2 additions & 2 deletions tests/fail/panic/unwind_panic_abort.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: unwinding past a stack frame that does not allow unwinding
--> $DIR/unwind_panic_abort.rs:LL:CC
|
LL | miri_start_panic(&mut 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding
LL | miri_start_unwind(&mut 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Expand Down
2 changes: 1 addition & 1 deletion tests/pass/function_calls/disable_abi_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn main() {
unsafe {
let _ = malloc(0);
std::mem::transmute::<fn(), extern "C" fn()>(foo)();
std::intrinsics::r#try(
std::intrinsics::catch_unwind(
std::mem::transmute::<extern "C" fn(*mut u8), _>(try_fn),
std::ptr::null_mut(),
|_, _| unreachable!(),
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/miri_extern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ extern "Rust" {
///
/// This is internal and unstable and should not be used; we give it here
/// just to be complete.
pub fn miri_start_panic(payload: *mut u8) -> !;
pub fn miri_start_unwind(payload: *mut u8) -> !;

/// Miri-provided extern function to get the internal unique identifier for the allocation that a pointer
/// points to. If this pointer is invalid (not pointing to an allocation), interpretation will abort.
Expand Down

0 comments on commit 8048afe

Please sign in to comment.