Skip to content

Commit

Permalink
Rollup merge of rust-lang#124715 - RalfJung:interpret-noreturn, r=com…
Browse files Browse the repository at this point in the history 
…piler-errors

interpret, miri: uniform treatments of intrinsics/functions with and without return block

A long time ago we didn't have a `dest: &MPlaceTy<'tcx, Self::Provenance>` for diverging functions, and since `dest` is used so often we special-cased these non-returning intrinsics and functions so that we'd have `dest` available everywhere else. But this has changed a while ago, now only the return block `ret` is optional, and there's a convenient `return_to_block` function for dealing with the `None` case.

So there no longer is any reason to treat diverging intrinsics/functions any different from those that do return.
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored May 4, 2024
2 parents a5cc1f6 + 8e44664 commit 743be1e
Show file tree
Hide file tree
Showing 24 changed files with 189 additions and 237 deletions.
16 changes: 2 additions & 14 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,19 +467,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
let intrinsic_name = ecx.tcx.item_name(instance.def_id());

// CTFE-specific intrinsics.
let Some(ret) = target else {
// Handle diverging intrinsics. We can't handle any of them (that are not already
// handled above), but check if there is a fallback body.
if ecx.tcx.intrinsic(instance.def_id()).unwrap().must_be_overridden {
throw_unsup_format!(
"intrinsic `{intrinsic_name}` is not supported at compile-time"
);
}
return Ok(Some(ty::Instance {
def: ty::InstanceDef::Item(instance.def_id()),
args: instance.args,
}));
};
match intrinsic_name {
sym::ptr_guaranteed_cmp => {
let a = ecx.read_scalar(&args[0])?;
Expand Down Expand Up @@ -559,7 +546,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
}
}

ecx.go_to_block(ret);
// Intrinsic is done, jump to next block.
ecx.return_to_block(target)?;
Ok(None)
}

Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, bool> {
let instance_args = instance.args;
let intrinsic_name = self.tcx.item_name(instance.def_id());
let Some(ret) = ret else {
// We don't support any intrinsic without return place.
return Ok(false);
};

match intrinsic_name {
sym::caller_location => {
Expand Down Expand Up @@ -376,7 +372,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
};

M::panic_nounwind(self, &msg)?;
// Skip the `go_to_block` at the end.
// Skip the `return_to_block` at the end (we panicked, we do not return).
return Ok(true);
}
}
Expand Down Expand Up @@ -437,11 +433,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.write_scalar(Scalar::from_target_usize(align.bytes(), self), dest)?;
}

// Unsupported intrinsic: skip the return_to_block below.
_ => return Ok(false),
}

trace!("{:?}", self.dump_place(&dest.clone().into()));
self.go_to_block(ret);
self.return_to_block(ret)?;
Ok(true)
}

Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ pub use rustc_const_eval::interpret::*;
#[doc(no_inline)]
pub use rustc_const_eval::interpret::{self, AllocMap, PlaceTy, Provenance as _};

pub use crate::shims::EmulateItemResult;
pub use crate::shims::env::{EnvVars, EvalContextExt as _};
pub use crate::shims::foreign_items::{DynSym, EvalContextExt as _};
pub use crate::shims::intrinsics::EvalContextExt as _;
Expand Down
9 changes: 4 additions & 5 deletions src/tools/miri/src/shims/alloc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use rustc_ast::expand::allocator::AllocatorKind;
use rustc_target::abi::{Align, Size};

use crate::*;
use shims::foreign_items::EmulateForeignItemResult;

/// Check some basic requirements for this allocation request:
/// non-zero size, power-of-two alignment.
Expand Down Expand Up @@ -55,12 +54,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn emulate_allocator(
&mut self,
default: impl FnOnce(&mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
) -> InterpResult<'tcx, EmulateItemResult> {
let this = self.eval_context_mut();

let Some(allocator_kind) = this.tcx.allocator_kind(()) else {
// in real code, this symbol does not exist without an allocator
return Ok(EmulateForeignItemResult::NotSupported);
return Ok(EmulateItemResult::NotSupported);
};

match allocator_kind {
Expand All @@ -70,11 +69,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// and not execute any Miri shim. Somewhat unintuitively doing so is done
// by returning `NotSupported`, which triggers the `lookup_exported_symbol`
// fallback case in `emulate_foreign_item`.
return Ok(EmulateForeignItemResult::NotSupported);
return Ok(EmulateItemResult::NotSupported);
}
AllocatorKind::Default => {
default(this)?;
Ok(EmulateForeignItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsJumping)
}
}
}
Expand Down
166 changes: 69 additions & 97 deletions src/tools/miri/src/shims/foreign_items.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,6 @@ impl DynSym {
}
}

/// Returned by `emulate_foreign_item_inner`.
pub enum EmulateForeignItemResult {
/// The caller is expected to jump to the return block.
NeedsJumping,
/// Jumping has already been taken care of.
AlreadyJumped,
/// The item is not supported.
NotSupported,
}

impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Emulates calling a foreign item, failing if the item is not supported.
Expand All @@ -58,84 +48,47 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let this = self.eval_context_mut();
let tcx = this.tcx.tcx;

// First: functions that diverge.
let ret = match ret {
None =>
match link_name.as_str() {
"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`.
// The implementation is provided by the function with the `#[panic_handler]` attribute.
"panic_impl" => {
// We don't use `check_shim` here because we are just forwarding to the lang
// item. Argument count checking will be performed when the returned `Body` is
// called.
this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?;
let panic_impl_id = tcx.lang_items().panic_impl().unwrap();
let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id);
return Ok(Some((
this.load_mir(panic_impl_instance.def, None)?,
panic_impl_instance,
)));
}
"__rust_alloc_error_handler" => {
// Forward to the right symbol that implements this function.
let Some(handler_kind) = this.tcx.alloc_error_handler_kind(()) else {
// in real code, this symbol does not exist without an allocator
throw_unsup_format!(
"`__rust_alloc_error_handler` cannot be called when no alloc error handler is set"
);
};
let name = alloc_error_handler_name(handler_kind);
let handler = this
.lookup_exported_symbol(Symbol::intern(name))?
.expect("missing alloc error handler symbol");
return Ok(Some(handler));
}
#[rustfmt::skip]
| "exit"
| "ExitProcess"
=> {
let exp_abi = if link_name.as_str() == "exit" {
Abi::C { unwind: false }
} else {
Abi::System { unwind: false }
};
let [code] = this.check_shim(abi, exp_abi, link_name, args)?;
// it's really u32 for ExitProcess, but we have to put it into the `Exit` variant anyway
let code = this.read_scalar(code)?.to_i32()?;
throw_machine_stop!(TerminationInfo::Exit { code: code.into(), leak_check: false });
}
"abort" => {
let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
throw_machine_stop!(TerminationInfo::Abort(
"the program aborted execution".to_owned()
))
}
_ => {
if let Some(body) = this.lookup_exported_symbol(link_name)? {
return Ok(Some(body));
}
this.handle_unsupported(format!(
"can't call (diverging) foreign function: {link_name}"
))?;
return Ok(None);
}
},
Some(p) => p,
};
// Some shims forward to other MIR bodies.
match link_name.as_str() {
// This matches calls to the foreign item `panic_impl`.
// The implementation is provided by the function with the `#[panic_handler]` attribute.
"panic_impl" => {
// We don't use `check_shim` here because we are just forwarding to the lang
// item. Argument count checking will be performed when the returned `Body` is
// called.
this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?;
let panic_impl_id = tcx.lang_items().panic_impl().unwrap();
let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id);
return Ok(Some((
this.load_mir(panic_impl_instance.def, None)?,
panic_impl_instance,
)));
}
"__rust_alloc_error_handler" => {
// Forward to the right symbol that implements this function.
let Some(handler_kind) = this.tcx.alloc_error_handler_kind(()) else {
// in real code, this symbol does not exist without an allocator
throw_unsup_format!(
"`__rust_alloc_error_handler` cannot be called when no alloc error handler is set"
);
};
let name = alloc_error_handler_name(handler_kind);
let handler = this
.lookup_exported_symbol(Symbol::intern(name))?
.expect("missing alloc error handler symbol");
return Ok(Some(handler));
}
_ => {}
}

// Second: functions that return immediately.
match this.emulate_foreign_item_inner(link_name, abi, args, dest)? {
EmulateForeignItemResult::NeedsJumping => {
// 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 => {
trace!("{:?}", this.dump_place(&dest.clone().into()));
this.go_to_block(ret);
this.return_to_block(ret)?;
}
EmulateForeignItemResult::AlreadyJumped => (),
EmulateForeignItemResult::NotSupported => {
EmulateItemResult::AlreadyJumped => (),
EmulateItemResult::NotSupported => {
if let Some(body) = this.lookup_exported_symbol(link_name)? {
return Ok(Some(body));
}
Expand Down Expand Up @@ -243,7 +196,8 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
abi: Abi,
args: &[OpTy<'tcx, Provenance>],
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
unwind: mir::UnwindAction,
) -> InterpResult<'tcx, EmulateItemResult> {
let this = self.eval_context_mut();

// First deal with any external C functions in linked .so file.
Expand All @@ -254,7 +208,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_external_c_fct(link_name, dest, args)? {
return Ok(EmulateForeignItemResult::NeedsJumping);
return Ok(EmulateItemResult::NeedsJumping);
}
}

Expand Down Expand Up @@ -298,6 +252,11 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// shim, add it to the corresponding submodule.
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);
}
"miri_run_provenance_gc" => {
let [] = this.check_shim(abi, Abi::Rust, link_name, args)?;
this.run_provenance_gc();
Expand Down Expand Up @@ -362,29 +321,24 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Return value: 0 on success, otherwise the size it would have needed.
this.write_int(if success { 0 } else { needed_size }, dest)?;
}

// Obtains the size of a Miri backtrace. See the README for details.
"miri_backtrace_size" => {
this.handle_miri_backtrace_size(abi, link_name, args, dest)?;
}

// Obtains a Miri backtrace. See the README for details.
"miri_get_backtrace" => {
// `check_shim` happens inside `handle_miri_get_backtrace`.
this.handle_miri_get_backtrace(abi, link_name, args, dest)?;
}

// Resolves a Miri backtrace frame. See the README for details.
"miri_resolve_frame" => {
// `check_shim` happens inside `handle_miri_resolve_frame`.
this.handle_miri_resolve_frame(abi, link_name, args, dest)?;
}

// Writes the function and file names of a Miri backtrace frame into a user provided buffer. See the README for details.
"miri_resolve_frame_names" => {
this.handle_miri_resolve_frame_names(abi, link_name, args)?;
}

// Writes some bytes to the interpreter's stdout/stderr. See the
// README for details.
"miri_write_to_stdout" | "miri_write_to_stderr" => {
Expand All @@ -398,7 +352,6 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
_ => unreachable!(),
};
}

// Promises that a pointer has a given symbolic alignment.
"miri_promise_symbolic_alignment" => {
use rustc_target::abi::AlignFromBytesError;
Expand Down Expand Up @@ -442,6 +395,25 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
}

// Aborting the process.
"exit" | "ExitProcess" => {
let exp_abi = if link_name.as_str() == "exit" {
Abi::C { unwind: false }
} else {
Abi::System { unwind: false }
};
let [code] = this.check_shim(abi, exp_abi, link_name, args)?;
// it's really u32 for ExitProcess, but we have to put it into the `Exit` variant anyway
let code = this.read_scalar(code)?.to_i32()?;
throw_machine_stop!(TerminationInfo::Exit { code: code.into(), leak_check: false });
}
"abort" => {
let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
throw_machine_stop!(TerminationInfo::Abort(
"the program aborted execution".to_owned()
))
}

// Standard C allocation
"malloc" => {
let [size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
Expand Down Expand Up @@ -504,7 +476,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
"__rust_alloc" => return this.emulate_allocator(default),
"miri_alloc" => {
default(this)?;
return Ok(EmulateForeignItemResult::NeedsJumping);
return Ok(EmulateItemResult::NeedsJumping);
}
_ => unreachable!(),
}
Expand Down Expand Up @@ -564,7 +536,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
"miri_dealloc" => {
default(this)?;
return Ok(EmulateForeignItemResult::NeedsJumping);
return Ok(EmulateItemResult::NeedsJumping);
}
_ => unreachable!(),
}
Expand Down Expand Up @@ -966,11 +938,11 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
shims::windows::foreign_items::EvalContextExt::emulate_foreign_item_inner(
this, link_name, abi, args, dest,
),
_ => Ok(EmulateForeignItemResult::NotSupported),
_ => Ok(EmulateItemResult::NotSupported),
},
};
// 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(EmulateForeignItemResult::NeedsJumping)
Ok(EmulateItemResult::NeedsJumping)
}
}
6 changes: 3 additions & 3 deletions src/tools/miri/src/shims/intrinsics/atomic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
intrinsic_name: &str,
args: &[OpTy<'tcx, Provenance>],
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, bool> {
) -> InterpResult<'tcx, EmulateItemResult> {
let this = self.eval_context_mut();

let intrinsic_structure: Vec<_> = intrinsic_name.split('_').collect();
Expand Down Expand Up @@ -114,9 +114,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.atomic_rmw_op(args, dest, AtomicOp::Max, rw_ord(ord)?)?;
}

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

Expand Down
Loading

0 comments on commit 743be1e

Please sign in to comment.