From 821d23b32930c68e36a5a246e734fa3a655970d9 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 23 Apr 2024 13:32:30 +0000 Subject: [PATCH] Ensure miri only uses fallback bodies that have manually been vetted to preserve all UB that the native intrinsic would have --- .../rustc_const_eval/src/const_eval/machine.rs | 3 ++- compiler/rustc_resolve/src/macros.rs | 2 +- library/core/src/intrinsics.rs | 3 +++ src/tools/miri/src/shims/intrinsics/atomic.rs | 1 + src/tools/miri/src/shims/intrinsics/mod.rs | 8 ++++++++ src/tools/miri/src/shims/intrinsics/simd.rs | 1 + .../tests/fail/intrinsic_fallback_checks_ub.rs | 14 ++++++++++++++ .../tests/fail/intrinsic_fallback_checks_ub.stderr | 14 ++++++++++++++ 8 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.rs create mode 100644 src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.stderr diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 506b877bbc7..e9cf5a3d769 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -468,7 +468,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, // CTFE-specific intrinsics. let Some(ret) = target else { - // Handle diverging intrinsics. + // 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" diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 82e41b6c314..35bf3f761df 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -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 } diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 14b4ce39ab4..4c7b511cfde 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -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 } @@ -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 } @@ -2526,6 +2528,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(ptr: *const T, other: *const T) -> u8 { (ptr == other) as u8 } diff --git a/src/tools/miri/src/shims/intrinsics/atomic.rs b/src/tools/miri/src/shims/intrinsics/atomic.rs index f566ed34707..4c5e2192e5c 100644 --- a/src/tools/miri/src/shims/intrinsics/atomic.rs +++ b/src/tools/miri/src/shims/intrinsics/atomic.rs @@ -14,6 +14,7 @@ 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, diff --git a/src/tools/miri/src/shims/intrinsics/mod.rs b/src/tools/miri/src/shims/intrinsics/mod.rs index cc70b22a16f..2bf6980200f 100644 --- a/src/tools/miri/src/shims/intrinsics/mod.rs +++ b/src/tools/miri/src/shims/intrinsics/mod.rs @@ -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 _; @@ -48,6 +49,7 @@ 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, }; @@ -63,9 +65,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // The rest jumps to `ret` immediately. 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, @@ -78,6 +85,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } /// 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, diff --git a/src/tools/miri/src/shims/intrinsics/simd.rs b/src/tools/miri/src/shims/intrinsics/simd.rs index 6e768dfdea4..dca4c4ea5ad 100644 --- a/src/tools/miri/src/shims/intrinsics/simd.rs +++ b/src/tools/miri/src/shims/intrinsics/simd.rs @@ -16,6 +16,7 @@ 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, diff --git a/src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.rs b/src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.rs new file mode 100644 index 00000000000..93c9d3d7814 --- /dev/null +++ b/src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.rs @@ -0,0 +1,14 @@ +#![feature(rustc_attrs, effects)] + +#[rustc_intrinsic] +#[rustc_nounwind] +#[rustc_do_not_const_check] +#[inline] +pub const fn ptr_guaranteed_cmp(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. +} diff --git a/src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.stderr b/src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.stderr new file mode 100644 index 00000000000..b37e05c62f9 --- /dev/null +++ b/src/tools/miri/tests/fail/intrinsic_fallback_checks_ub.stderr @@ -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 +