Skip to content

Commit

Permalink
various small nits
Browse files Browse the repository at this point in the history
- share implementation with miri_starting_unwind
- make test use a custom unwinding class
- extend comments
- use NeedsUnwind more consistently
  • Loading branch information
RalfJung committed May 19, 2024
1 parent 42cb1ff commit 5e41ff5
Show file tree
Hide file tree
Showing 26 changed files with 72 additions and 69 deletions.
2 changes: 1 addition & 1 deletion src/tools/miri/ci/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ case $HOST_TARGET in
TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice
# Partially supported targets (tier 2)
BASIC="empty_main integer vec string btreemap hello hashmap heap_alloc align" # ensures we have the basics: stdout/stderr, system allocator, randomness (for HashMap initialization)
UNIX="panic/panic concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there
UNIX="panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there
TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs
TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs
TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX pthread-sync
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/intrinsics/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
args: instance.args,
}))
}
EmulateItemResult::NeedsJumping => {
EmulateItemResult::NeedsReturn => {
trace!("{:?}", this.dump_place(&dest.clone().into()));
this.return_to_block(ret)?;
Ok(None)
Expand Down Expand Up @@ -446,6 +446,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
_ => return Ok(EmulateItemResult::NotSupported),
}

Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/intrinsics/simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}

fn fminmax_op(
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
AllocatorKind::Default => {
default(this)?;
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
}
Expand Down
19 changes: 9 additions & 10 deletions src/tools/miri/src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}

// The rest either implements the logic, or falls back to `lookup_exported_symbol`.
match this.emulate_foreign_item_inner(link_name, abi, args, dest, unwind)? {
EmulateItemResult::NeedsJumping => {
match this.emulate_foreign_item_inner(link_name, abi, args, dest)? {
EmulateItemResult::NeedsReturn => {
trace!("{:?}", this.dump_place(&dest.clone().into()));
this.return_to_block(ret)?;
}
Expand Down Expand Up @@ -210,7 +210,6 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
abi: Abi,
args: &[OpTy<'tcx, Provenance>],
dest: &MPlaceTy<'tcx, Provenance>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx, EmulateItemResult> {
let this = self.eval_context_mut();

Expand All @@ -222,7 +221,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// by the specified `.so` file; we should continue and check if it corresponds to
// a provided shim.
if this.call_native_fn(link_name, dest, args)? {
return Ok(EmulateItemResult::NeedsJumping);
return Ok(EmulateItemResult::NeedsReturn);
}
}

Expand Down Expand Up @@ -267,9 +266,9 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
match link_name.as_str() {
// Miri-specific extern functions
"miri_start_unwind" => {
// `check_shim` happens inside `handle_miri_start_unwind`.
this.handle_miri_start_unwind(abi, link_name, args, unwind)?;
return Ok(EmulateItemResult::AlreadyJumped);
let [payload] = this.check_shim(abi, Abi::Rust, link_name, args)?;
this.handle_miri_start_unwind(payload)?;
return Ok(EmulateItemResult::NeedsUnwind);
}
"miri_run_provenance_gc" => {
let [] = this.check_shim(abi, Abi::Rust, link_name, args)?;
Expand Down Expand Up @@ -484,7 +483,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
"__rust_alloc" => return this.emulate_allocator(default),
"miri_alloc" => {
default(this)?;
return Ok(EmulateItemResult::NeedsJumping);
return Ok(EmulateItemResult::NeedsReturn);
}
_ => unreachable!(),
}
Expand Down Expand Up @@ -544,7 +543,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
"miri_dealloc" => {
default(this)?;
return Ok(EmulateItemResult::NeedsJumping);
return Ok(EmulateItemResult::NeedsReturn);
}
_ => unreachable!(),
}
Expand Down Expand Up @@ -965,6 +964,6 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
};
// We only fall through to here if we did *not* hit the `_` arm above,
// i.e., if we actually emulated the function with one of the shims.
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
4 changes: 2 additions & 2 deletions src/tools/miri/src/shims/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ pub use unix::{DirTable, FdTable};
/// What needs to be done after emulating an item (a shim or an intrinsic) is done.
pub enum EmulateItemResult {
/// The caller is expected to jump to the return block.
NeedsJumping,
NeedsReturn,
/// The caller is expected to jump to the unwind block.
NeedsUnwind,
/// Jumping has already been taken care of.
/// Jumping to the next block has already been taken care of.
AlreadyJumped,
/// The item is not supported.
NotSupported,
Expand Down
13 changes: 1 addition & 12 deletions src/tools/miri/src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use rustc_ast::Mutability;
use rustc_middle::{mir, ty};
use rustc_span::Symbol;
use rustc_target::spec::abi::Abi;
use rustc_target::spec::PanicStrategy;

Expand Down Expand Up @@ -46,25 +45,15 @@ impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir,
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// 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_unwind(
&mut self,
abi: Abi,
link_name: Symbol,
args: &[OpTy<'tcx, Provenance>],
unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
fn handle_miri_start_unwind(&mut self, payload: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> {
let this = self.eval_context_mut();

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)?;
let payload = this.read_scalar(payload)?;
let thread = this.active_thread_mut();
thread.panic_payloads.push(payload);

// Jump to the unwind block to begin unwinding.
this.unwind_to_block(unwind)?;
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/android/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
30 changes: 22 additions & 8 deletions src/tools/miri/src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,14 +640,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.write_scalar(Scalar::from_target_usize(len, this), dest)?;
}
"_Unwind_RaiseException" => {
trace!("_Unwind_RaiseException: {:?}", this.frame().instance);

// Get the raw pointer stored in arg[0] (the panic payload).
// This is not formally part of POSIX, but it is very wide-spread on POSIX systems.
// It was originally specified as part of the Itanium C++ ABI:
// https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html#base-throw.
// On Linux it is
// documented as part of the LSB:
// https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/baselib--unwind-raiseexception.html
// Basically every other UNIX uses the exact same api though. Arm also references
// back to the Itanium C++ ABI for the definition of `_Unwind_RaiseException` for
// arm64:
// https://github.com/ARM-software/abi-aa/blob/main/cppabi64/cppabi64.rst#toc-entry-35
// For arm32 they did something custom, but similar enough that the same
// `_Unwind_RaiseException` impl in miri should work:
// https://github.com/ARM-software/abi-aa/blob/main/ehabi32/ehabi32.rst
if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd" | "illumos" | "solaris" | "android" | "macos") {
throw_unsup_format!(
"`_Unwind_RaiseException` is not supported on {}",
this.tcx.sess.target.os
);
}
// This function looks and behaves excatly like miri_start_unwind.
let [payload] = this.check_shim(abi, Abi::C { unwind: true }, link_name, args)?;
let payload = this.read_scalar(payload)?;
let thread = this.active_thread_mut();
thread.panic_payloads.push(payload);

this.handle_miri_start_unwind(payload)?;
return Ok(EmulateItemResult::NeedsUnwind);
}

Expand Down Expand Up @@ -771,6 +785,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
};

Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/freebsd/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/linux/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
_ => return Ok(EmulateItemResult::NotSupported),
};

Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/macos/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
_ => return Ok(EmulateItemResult::NotSupported),
};

Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/solarish/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/wasi/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
_ => return Ok(EmulateItemResult::NotSupported),
}

Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/aesni.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
// with an external crate.
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/avx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/avx2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:

_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/sse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/sse2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/sse3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/sse41.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/x86/ssse3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsReturn)
}
}
Loading

0 comments on commit 5e41ff5

Please sign in to comment.