Unrolled build for rust-lang#124715

Rollup merge of rust-lang#124715 - RalfJung:interpret-noreturn, r=compiler-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.

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -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])?;
@@ -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)
  }
 

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -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 => {
@@ -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);
  }
  }
@@ -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)
  }
 

src/tools/miri/src/lib.rs

@@ -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 _;

src/tools/miri/src/shims/alloc.rs

@@ -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.
@@ -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 {
@@ -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)
  }
  }
  }

src/tools/miri/src/shims/foreign_items.rs

@@ -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.
@@ -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));
  }
@@ -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.
@@ -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);
  }
  }
 
@@ -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();
@@ -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" => {
@@ -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;
@@ -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)?;
@@ -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!(),
  }
@@ -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!(),
  }
@@ -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)
  }
 }

src/tools/miri/src/shims/intrinsics/atomic.rs

@@ -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();
@@ -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)
  }
 }