Skip to content

Commit

Permalink
Rollup merge of rust-lang#124293 - oli-obk:miri_intrinsic_fallback_bo…
Browse files Browse the repository at this point in the history
…dy, r=RalfJung

Let miri and const eval execute intrinsics' fallback bodies

fixes rust-lang/miri#3397

r? `@RalfJung`
  • Loading branch information
matthiaskrgr committed May 4, 2024
2 parents 5ec2ad5 + 4e97c6c commit d577d04
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 48 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/dummy_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for DummyMachine {
_destination: &interpret::MPlaceTy<'tcx, Self::Provenance>,
_target: Option<BasicBlock>,
_unwind: UnwindAction,
) -> interpret::InterpResult<'tcx> {
) -> interpret::InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
unimplemented!()
}

Expand Down
31 changes: 24 additions & 7 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,16 +459,26 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
dest: &MPlaceTy<'tcx, Self::Provenance>,
target: Option<mir::BasicBlock>,
_unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
// Shared intrinsics.
if ecx.emulate_intrinsic(instance, args, dest, target)? {
return Ok(());
return Ok(None);
}
let intrinsic_name = ecx.tcx.item_name(instance.def_id());

// CTFE-specific intrinsics.
let Some(ret) = target else {
throw_unsup_format!("intrinsic `{intrinsic_name}` is not supported at compile-time");
// 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 => {
Expand Down Expand Up @@ -536,14 +546,21 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// not the optimization stage.)
sym::is_val_statically_known => ecx.write_scalar(Scalar::from_bool(false), dest)?,
_ => {
throw_unsup_format!(
"intrinsic `{intrinsic_name}` is not supported at compile-time"
);
// We haven't handled the intrinsic, let's see if we can use 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,
}));
}
}

ecx.go_to_block(ret);
Ok(())
Ok(None)
}

fn assert_panic(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
self.copy_op(&self.project_index(&input, index)?, dest)?;
}
sym::likely | sym::unlikely | sym::black_box => {
sym::black_box => {
// These just return their argument
self.copy_op(&args[0], dest)?;
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,17 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {

/// Directly process an intrinsic without pushing a stack frame. It is the hook's
/// responsibility to advance the instruction pointer as appropriate.
///
/// Returns `None` if the intrinsic was fully handled.
/// Otherwise, returns an `Instance` of the function that implements the intrinsic.
fn call_intrinsic(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Self::Provenance>],
destination: &MPlaceTy<'tcx, Self::Provenance>,
target: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx>;
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>>;

/// Called to evaluate `Assert` MIR terminators that trigger a panic.
fn assert_panic(
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,14 +539,28 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ty::InstanceDef::Intrinsic(def_id) => {
assert!(self.tcx.intrinsic(def_id).is_some());
// FIXME: Should `InPlace` arguments be reset to uninit?
M::call_intrinsic(
if let Some(fallback) = M::call_intrinsic(
self,
instance,
&self.copy_fn_args(args),
destination,
target,
unwind,
)
)? {
assert!(!self.tcx.intrinsic(fallback.def_id()).unwrap().must_be_overridden);
assert!(matches!(fallback.def, ty::InstanceDef::Item(_)));
return self.eval_fn_call(
FnVal::Instance(fallback),
(caller_abi, caller_fn_abi),
args,
with_caller_location,
destination,
target,
unwind,
);
} else {
Ok(())
}
}
ty::InstanceDef::VTableShim(..)
| ty::InstanceDef::ReifyShim(..)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub(crate) fn registered_tools(tcx: TyCtxt<'_>, (): ()) -> RegisteredTools {
}
// We implicitly add `rustfmt`, `clippy`, `diagnostic` to known tools,
// but it's not an error to register them explicitly.
let predefined_tools = [sym::clippy, sym::rustfmt, sym::diagnostic];
let predefined_tools = [sym::clippy, sym::rustfmt, sym::diagnostic, sym::miri];
registered_tools.extend(predefined_tools.iter().cloned().map(Ident::with_dummy_span));
registered_tools
}
Expand Down
12 changes: 10 additions & 2 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,7 @@ pub const unsafe fn assume(b: bool) {
#[unstable(feature = "core_intrinsics", issue = "none")]
#[rustc_intrinsic]
#[rustc_nounwind]
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
pub const fn likely(b: bool) -> bool {
b
}
Expand All @@ -1006,6 +1007,7 @@ pub const fn likely(b: bool) -> bool {
#[unstable(feature = "core_intrinsics", issue = "none")]
#[rustc_intrinsic]
#[rustc_nounwind]
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
pub const fn unlikely(b: bool) -> bool {
b
}
Expand Down Expand Up @@ -2469,6 +2471,7 @@ extern "rust-intrinsic" {
#[rustc_nounwind]
#[rustc_do_not_const_check]
#[inline]
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
pub const fn ptr_guaranteed_cmp<T>(ptr: *const T, other: *const T) -> u8 {
(ptr == other) as u8
}
Expand Down Expand Up @@ -2733,8 +2736,10 @@ pub const fn ub_checks() -> bool {
#[unstable(feature = "core_intrinsics", issue = "none")]
#[rustc_nounwind]
#[rustc_intrinsic]
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
pub const unsafe fn const_allocate(_size: usize, _align: usize) -> *mut u8 {
// const eval overrides this function, but runtime code should always just return null pointers.
// const eval overrides this function, but runtime code for now just returns null pointers.
// See <https://github.com/rust-lang/rust/issues/93935>.
crate::ptr::null_mut()
}

Expand All @@ -2752,7 +2757,10 @@ pub const unsafe fn const_allocate(_size: usize, _align: usize) -> *mut u8 {
#[unstable(feature = "core_intrinsics", issue = "none")]
#[rustc_nounwind]
#[rustc_intrinsic]
pub const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize) {}
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
pub const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize) {
// Runtime NOP
}

/// `ptr` must point to a vtable.
/// The intrinsic will return the size stored in that vtable.
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
dest: &MPlaceTy<'tcx, Provenance>,
ret: Option<mir::BasicBlock>,
unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
ecx.call_intrinsic(instance, args, dest, ret, unwind)
}

Expand Down
7 changes: 4 additions & 3 deletions src/tools/miri/src/shims/intrinsics/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ pub enum AtomicOp {
impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Calls the atomic intrinsic `intrinsic`; the `atomic_` prefix has already been removed.
/// Returns `Ok(true)` if the intrinsic was handled.
fn emulate_atomic_intrinsic(
&mut self,
intrinsic_name: &str,
args: &[OpTy<'tcx, Provenance>],
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, bool> {
let this = self.eval_context_mut();

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

_ => throw_unsup_format!("unimplemented intrinsic: `atomic_{intrinsic_name}`"),
_ => return Ok(false),
}
Ok(())
Ok(true)
}
}

Expand Down
53 changes: 27 additions & 26 deletions src/tools/miri/src/shims/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_middle::{
ty::{self, FloatTy},
};
use rustc_target::abi::Size;
use rustc_span::{sym, Symbol};

use crate::*;
use atomic::EvalContextExt as _;
Expand All @@ -26,12 +27,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
dest: &MPlaceTy<'tcx, Provenance>,
ret: Option<mir::BasicBlock>,
_unwind: mir::UnwindAction,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
let this = self.eval_context_mut();

// See if the core engine can handle this intrinsic.
if this.emulate_intrinsic(instance, args, dest, ret)? {
return Ok(());
return Ok(None);
}
let intrinsic_name = this.tcx.item_name(instance.def_id());
let intrinsic_name = intrinsic_name.as_str();
Expand All @@ -48,32 +49,50 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

// All remaining supported intrinsics have a return place.
let ret = match ret {
// FIXME: add fallback body support once we actually have a diverging intrinsic with a fallback body
None => throw_unsup_format!("unimplemented (diverging) intrinsic: `{intrinsic_name}`"),
Some(p) => p,
};

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

// The rest jumps to `ret` immediately.
this.emulate_intrinsic_by_name(intrinsic_name, instance.args, args, dest)?;
if !this.emulate_intrinsic_by_name(intrinsic_name, instance.args, args, dest)? {
// We haven't handled the intrinsic, let's see if we can use a fallback body.
if this.tcx.intrinsic(instance.def_id()).unwrap().must_be_overridden {
throw_unsup_format!("unimplemented intrinsic: `{intrinsic_name}`")
}
let intrinsic_fallback_checks_ub = Symbol::intern("intrinsic_fallback_checks_ub");
if this.tcx.get_attrs_by_path(instance.def_id(), &[sym::miri, intrinsic_fallback_checks_ub]).next().is_none() {
throw_unsup_format!("miri can only use intrinsic fallback bodies that check UB. After verifying that `{intrinsic_name}` does so, add the `#[miri::intrinsic_fallback_checks_ub]` attribute to it; also ping @rust-lang/miri when you do that");
}
return Ok(Some(ty::Instance {
def: ty::InstanceDef::Item(instance.def_id()),
args: instance.args,
}))
}

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

/// Emulates a Miri-supported intrinsic (not supported by the core engine).
/// Returns `Ok(true)` if the intrinsic was handled.
fn emulate_intrinsic_by_name(
&mut self,
intrinsic_name: &str,
generic_args: ty::GenericArgsRef<'tcx>,
args: &[OpTy<'tcx, Provenance>],
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, bool> {
let this = self.eval_context_mut();

if let Some(name) = intrinsic_name.strip_prefix("atomic_") {
Expand All @@ -84,24 +103,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}

match intrinsic_name {
// Miri overwriting CTFE intrinsics.
"ptr_guaranteed_cmp" => {
let [left, right] = check_arg_count(args)?;
let left = this.read_immediate(left)?;
let right = this.read_immediate(right)?;
let val = this.wrapping_binary_op(mir::BinOp::Eq, &left, &right)?;
// We're type punning a bool as an u8 here.
this.write_scalar(val.to_scalar(), dest)?;
}
"const_allocate" => {
// For now, for compatibility with the run-time implementation of this, we just return null.
// See <https://github.com/rust-lang/rust/issues/93935>.
this.write_null(dest)?;
}
"const_deallocate" => {
// complete NOP
}

// Raw memory accesses
"volatile_load" => {
let [place] = check_arg_count(args)?;
Expand Down Expand Up @@ -425,9 +426,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
throw_machine_stop!(TerminationInfo::Abort(format!("trace/breakpoint trap")))
}

name => throw_unsup_format!("unimplemented intrinsic: `{name}`"),
_ => return Ok(false),
}

Ok(())
Ok(true)
}
}
7 changes: 4 additions & 3 deletions src/tools/miri/src/shims/intrinsics/simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ pub(crate) enum MinMax {
impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Calls the simd intrinsic `intrinsic`; the `simd_` prefix has already been removed.
/// Returns `Ok(true)` if the intrinsic was handled.
fn emulate_simd_intrinsic(
&mut self,
intrinsic_name: &str,
generic_args: ty::GenericArgsRef<'tcx>,
args: &[OpTy<'tcx, Provenance>],
dest: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, bool> {
let this = self.eval_context_mut();
match intrinsic_name {
#[rustfmt::skip]
Expand Down Expand Up @@ -743,9 +744,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
}

name => throw_unsup_format!("unimplemented intrinsic: `simd_{name}`"),
_ => return Ok(false),
}
Ok(())
Ok(true)
}

fn fminmax_op(
Expand Down
14 changes: 14 additions & 0 deletions src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(rustc_attrs, effects)]

#[rustc_intrinsic]
#[rustc_nounwind]
#[rustc_do_not_const_check]
#[inline]
pub const fn ptr_guaranteed_cmp<T>(ptr: *const T, other: *const T) -> u8 {
(ptr == other) as u8
}

fn main() {
ptr_guaranteed_cmp::<()>(std::ptr::null(), std::ptr::null());
//~^ ERROR: can only use intrinsic fallback bodies that check UB.
}
14 changes: 14 additions & 0 deletions src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: unsupported operation: miri can only use intrinsic fallback bodies that check UB. After verifying that `ptr_guaranteed_cmp` does so, add the `#[miri::intrinsic_fallback_checks_ub]` attribute to it; also ping @rust-lang/miri when you do that
--> $DIR/intrinsic_fallback_checks_ub.rs:LL:CC
|
LL | ptr_guaranteed_cmp::<()>(std::ptr::null(), std::ptr::null());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ miri can only use intrinsic fallback bodies that check UB. After verifying that `ptr_guaranteed_cmp` does so, add the `#[miri::intrinsic_fallback_checks_ub]` attribute to it; also ping @rust-lang/miri when you do that
|
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
= note: BACKTRACE:
= note: inside `main` at $DIR/intrinsic_fallback_checks_ub.rs:LL:CC

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

error: aborting due to 1 previous error

0 comments on commit d577d04

Please sign in to comment.