Skip to content

Commit c46f24d

Browse files
rust-timerrust-timer
authored
Unrolled build for #124293
Rollup merge of #124293 - oli-obk:miri_intrinsic_fallback_body, r=RalfJung Let miri and const eval execute intrinsics' fallback bodies fixes rust-lang/miri#3397 r? ``@RalfJung``
2 parents 7dd170f + 4e97c6c commit c46f24d

File tree

13 files changed

+121
-48
lines changed

13 files changed

+121
-48
lines changed

‎compiler/rustc_const_eval/src/const_eval/dummy_machine.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for DummyMachine {
105105
_destination: &interpret::MPlaceTy<'tcx, Self::Provenance>,
106106
_target: Option<BasicBlock>,
107107
_unwind: UnwindAction,
108-
) -> interpret::InterpResult<'tcx> {
108+
) -> interpret::InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
109109
unimplemented!()
110110
}
111111

‎compiler/rustc_const_eval/src/const_eval/machine.rs

+24-7
Original file line numberDiff line numberDiff line change
@@ -459,16 +459,26 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
459459
dest: &MPlaceTy<'tcx, Self::Provenance>,
460460
target: Option<mir::BasicBlock>,
461461
_unwind: mir::UnwindAction,
462-
) -> InterpResult<'tcx> {
462+
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
463463
// Shared intrinsics.
464464
if ecx.emulate_intrinsic(instance, args, dest, target)? {
465-
return Ok(());
465+
return Ok(None);
466466
}
467467
let intrinsic_name = ecx.tcx.item_name(instance.def_id());
468468

469469
// CTFE-specific intrinsics.
470470
let Some(ret) = target else {
471-
throw_unsup_format!("intrinsic `{intrinsic_name}` is not supported at compile-time");
471+
// Handle diverging intrinsics. We can't handle any of them (that are not already
472+
// handled above), but check if there is a fallback body.
473+
if ecx.tcx.intrinsic(instance.def_id()).unwrap().must_be_overridden {
474+
throw_unsup_format!(
475+
"intrinsic `{intrinsic_name}` is not supported at compile-time"
476+
);
477+
}
478+
return Ok(Some(ty::Instance {
479+
def: ty::InstanceDef::Item(instance.def_id()),
480+
args: instance.args,
481+
}));
472482
};
473483
match intrinsic_name {
474484
sym::ptr_guaranteed_cmp => {
@@ -536,14 +546,21 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
536546
// not the optimization stage.)
537547
sym::is_val_statically_known => ecx.write_scalar(Scalar::from_bool(false), dest)?,
538548
_ => {
539-
throw_unsup_format!(
540-
"intrinsic `{intrinsic_name}` is not supported at compile-time"
541-
);
549+
// We haven't handled the intrinsic, let's see if we can use a fallback body.
550+
if ecx.tcx.intrinsic(instance.def_id()).unwrap().must_be_overridden {
551+
throw_unsup_format!(
552+
"intrinsic `{intrinsic_name}` is not supported at compile-time"
553+
);
554+
}
555+
return Ok(Some(ty::Instance {
556+
def: ty::InstanceDef::Item(instance.def_id()),
557+
args: instance.args,
558+
}));
542559
}
543560
}
544561

545562
ecx.go_to_block(ret);
546-
Ok(())
563+
Ok(None)
547564
}
548565

549566
fn assert_panic(

‎compiler/rustc_const_eval/src/interpret/intrinsics.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
414414
}
415415
self.copy_op(&self.project_index(&input, index)?, dest)?;
416416
}
417-
sym::likely | sym::unlikely | sym::black_box => {
417+
sym::black_box => {
418418
// These just return their argument
419419
self.copy_op(&args[0], dest)?;
420420
}

‎compiler/rustc_const_eval/src/interpret/machine.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -216,14 +216,17 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
216216

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

228231
/// Called to evaluate `Assert` MIR terminators that trigger a panic.
229232
fn assert_panic(

‎compiler/rustc_const_eval/src/interpret/terminator.rs

+16-2
Original file line numberDiff line numberDiff line change
@@ -539,14 +539,28 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
539539
ty::InstanceDef::Intrinsic(def_id) => {
540540
assert!(self.tcx.intrinsic(def_id).is_some());
541541
// FIXME: Should `InPlace` arguments be reset to uninit?
542-
M::call_intrinsic(
542+
if let Some(fallback) = M::call_intrinsic(
543543
self,
544544
instance,
545545
&self.copy_fn_args(args),
546546
destination,
547547
target,
548548
unwind,
549-
)
549+
)? {
550+
assert!(!self.tcx.intrinsic(fallback.def_id()).unwrap().must_be_overridden);
551+
assert!(matches!(fallback.def, ty::InstanceDef::Item(_)));
552+
return self.eval_fn_call(
553+
FnVal::Instance(fallback),
554+
(caller_abi, caller_fn_abi),
555+
args,
556+
with_caller_location,
557+
destination,
558+
target,
559+
unwind,
560+
);
561+
} else {
562+
Ok(())
563+
}
550564
}
551565
ty::InstanceDef::VTableShim(..)
552566
| ty::InstanceDef::ReifyShim(..)

‎compiler/rustc_resolve/src/macros.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ pub(crate) fn registered_tools(tcx: TyCtxt<'_>, (): ()) -> RegisteredTools {
141141
}
142142
// We implicitly add `rustfmt`, `clippy`, `diagnostic` to known tools,
143143
// but it's not an error to register them explicitly.
144-
let predefined_tools = [sym::clippy, sym::rustfmt, sym::diagnostic];
144+
let predefined_tools = [sym::clippy, sym::rustfmt, sym::diagnostic, sym::miri];
145145
registered_tools.extend(predefined_tools.iter().cloned().map(Ident::with_dummy_span));
146146
registered_tools
147147
}

‎library/core/src/intrinsics.rs

+10-2
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,7 @@ pub const unsafe fn assume(b: bool) {
987987
#[unstable(feature = "core_intrinsics", issue = "none")]
988988
#[rustc_intrinsic]
989989
#[rustc_nounwind]
990+
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
990991
pub const fn likely(b: bool) -> bool {
991992
b
992993
}
@@ -1006,6 +1007,7 @@ pub const fn likely(b: bool) -> bool {
10061007
#[unstable(feature = "core_intrinsics", issue = "none")]
10071008
#[rustc_intrinsic]
10081009
#[rustc_nounwind]
1010+
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
10091011
pub const fn unlikely(b: bool) -> bool {
10101012
b
10111013
}
@@ -2469,6 +2471,7 @@ extern "rust-intrinsic" {
24692471
#[rustc_nounwind]
24702472
#[rustc_do_not_const_check]
24712473
#[inline]
2474+
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
24722475
pub const fn ptr_guaranteed_cmp<T>(ptr: *const T, other: *const T) -> u8 {
24732476
(ptr == other) as u8
24742477
}
@@ -2733,8 +2736,10 @@ pub const fn ub_checks() -> bool {
27332736
#[unstable(feature = "core_intrinsics", issue = "none")]
27342737
#[rustc_nounwind]
27352738
#[rustc_intrinsic]
2739+
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
27362740
pub const unsafe fn const_allocate(_size: usize, _align: usize) -> *mut u8 {
2737-
// const eval overrides this function, but runtime code should always just return null pointers.
2741+
// const eval overrides this function, but runtime code for now just returns null pointers.
2742+
// See <https://github.com/rust-lang/rust/issues/93935>.
27382743
crate::ptr::null_mut()
27392744
}
27402745

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

27572765
/// `ptr` must point to a vtable.
27582766
/// The intrinsic will return the size stored in that vtable.

‎src/tools/miri/src/machine.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -986,7 +986,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
986986
dest: &MPlaceTy<'tcx, Provenance>,
987987
ret: Option<mir::BasicBlock>,
988988
unwind: mir::UnwindAction,
989-
) -> InterpResult<'tcx> {
989+
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
990990
ecx.call_intrinsic(instance, args, dest, ret, unwind)
991991
}
992992

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

+4-3
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@ pub enum AtomicOp {
1414
impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
1515
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
1616
/// Calls the atomic intrinsic `intrinsic`; the `atomic_` prefix has already been removed.
17+
/// Returns `Ok(true)` if the intrinsic was handled.
1718
fn emulate_atomic_intrinsic(
1819
&mut self,
1920
intrinsic_name: &str,
2021
args: &[OpTy<'tcx, Provenance>],
2122
dest: &MPlaceTy<'tcx, Provenance>,
22-
) -> InterpResult<'tcx> {
23+
) -> InterpResult<'tcx, bool> {
2324
let this = self.eval_context_mut();
2425

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

116-
_ => throw_unsup_format!("unimplemented intrinsic: `atomic_{intrinsic_name}`"),
117+
_ => return Ok(false),
117118
}
118-
Ok(())
119+
Ok(true)
119120
}
120121
}
121122

‎src/tools/miri/src/shims/intrinsics/mod.rs

+27-26
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use rustc_middle::{
1111
ty::{self, FloatTy},
1212
};
1313
use rustc_target::abi::Size;
14+
use rustc_span::{sym, Symbol};
1415

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

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

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

5557
// Some intrinsics are special and need the "ret".
5658
match intrinsic_name {
57-
"catch_unwind" => return this.handle_catch_unwind(args, dest, ret),
59+
"catch_unwind" => {
60+
this.handle_catch_unwind(args, dest, ret)?;
61+
return Ok(None);
62+
}
5863
_ => {}
5964
}
6065

6166
// The rest jumps to `ret` immediately.
62-
this.emulate_intrinsic_by_name(intrinsic_name, instance.args, args, dest)?;
67+
if !this.emulate_intrinsic_by_name(intrinsic_name, instance.args, args, dest)? {
68+
// We haven't handled the intrinsic, let's see if we can use a fallback body.
69+
if this.tcx.intrinsic(instance.def_id()).unwrap().must_be_overridden {
70+
throw_unsup_format!("unimplemented intrinsic: `{intrinsic_name}`")
71+
}
72+
let intrinsic_fallback_checks_ub = Symbol::intern("intrinsic_fallback_checks_ub");
73+
if this.tcx.get_attrs_by_path(instance.def_id(), &[sym::miri, intrinsic_fallback_checks_ub]).next().is_none() {
74+
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");
75+
}
76+
return Ok(Some(ty::Instance {
77+
def: ty::InstanceDef::Item(instance.def_id()),
78+
args: instance.args,
79+
}))
80+
}
6381

6482
trace!("{:?}", this.dump_place(&dest.clone().into()));
6583
this.go_to_block(ret);
66-
Ok(())
84+
Ok(None)
6785
}
6886

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

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

86105
match intrinsic_name {
87-
// Miri overwriting CTFE intrinsics.
88-
"ptr_guaranteed_cmp" => {
89-
let [left, right] = check_arg_count(args)?;
90-
let left = this.read_immediate(left)?;
91-
let right = this.read_immediate(right)?;
92-
let val = this.wrapping_binary_op(mir::BinOp::Eq, &left, &right)?;
93-
// We're type punning a bool as an u8 here.
94-
this.write_scalar(val.to_scalar(), dest)?;
95-
}
96-
"const_allocate" => {
97-
// For now, for compatibility with the run-time implementation of this, we just return null.
98-
// See <https://github.com/rust-lang/rust/issues/93935>.
99-
this.write_null(dest)?;
100-
}
101-
"const_deallocate" => {
102-
// complete NOP
103-
}
104-
105106
// Raw memory accesses
106107
"volatile_load" => {
107108
let [place] = check_arg_count(args)?;
@@ -425,9 +426,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
425426
throw_machine_stop!(TerminationInfo::Abort(format!("trace/breakpoint trap")))
426427
}
427428

428-
name => throw_unsup_format!("unimplemented intrinsic: `{name}`"),
429+
_ => return Ok(false),
429430
}
430431

431-
Ok(())
432+
Ok(true)
432433
}
433434
}

‎src/tools/miri/src/shims/intrinsics/simd.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@ pub(crate) enum MinMax {
1616
impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
1717
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
1818
/// Calls the simd intrinsic `intrinsic`; the `simd_` prefix has already been removed.
19+
/// Returns `Ok(true)` if the intrinsic was handled.
1920
fn emulate_simd_intrinsic(
2021
&mut self,
2122
intrinsic_name: &str,
2223
generic_args: ty::GenericArgsRef<'tcx>,
2324
args: &[OpTy<'tcx, Provenance>],
2425
dest: &MPlaceTy<'tcx, Provenance>,
25-
) -> InterpResult<'tcx> {
26+
) -> InterpResult<'tcx, bool> {
2627
let this = self.eval_context_mut();
2728
match intrinsic_name {
2829
#[rustfmt::skip]
@@ -743,9 +744,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
743744
}
744745
}
745746

746-
name => throw_unsup_format!("unimplemented intrinsic: `simd_{name}`"),
747+
_ => return Ok(false),
747748
}
748-
Ok(())
749+
Ok(true)
749750
}
750751

751752
fn fminmax_op(

‎src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.rs

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#![feature(rustc_attrs, effects)]
2+
3+
#[rustc_intrinsic]
4+
#[rustc_nounwind]
5+
#[rustc_do_not_const_check]
6+
#[inline]
7+
pub const fn ptr_guaranteed_cmp<T>(ptr: *const T, other: *const T) -> u8 {
8+
(ptr == other) as u8
9+
}
10+
11+
fn main() {
12+
ptr_guaranteed_cmp::<()>(std::ptr::null(), std::ptr::null());
13+
//~^ ERROR: can only use intrinsic fallback bodies that check UB.
14+
}

‎src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.stderr

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
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
2+
--> $DIR/intrinsic_fallback_checks_ub.rs:LL:CC
3+
|
4+
LL | ptr_guaranteed_cmp::<()>(std::ptr::null(), std::ptr::null());
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
6+
|
7+
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
8+
= note: BACKTRACE:
9+
= note: inside `main` at $DIR/intrinsic_fallback_checks_ub.rs:LL:CC
10+
11+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
12+
13+
error: aborting due to 1 previous error
14+

0 commit comments

Comments
 (0)