Rollup merge of #143327 - RalfJung:miri-type-validity-error, r=oli-obk

miri: improve errors for type validity assertion failures

Miri has pretty nice errors for type validity violations, printing which field in the type the problem occurs at and so on.

However, we don't see these errors when using e.g. `mem::zeroed` as that uses `assert_zero_valid` to bail out before Miri can detect the UB.

Similar to what we did with `@saethlin's` UB checks, I think we should disable such language UB checks in Miri so that we can get better error messages. If we go for this we should probably say this in the intrinsic docs as well so that people don't think they can rely on these intrinsics catching anything.

Furthermore, I slightly changed `MaybeUninit::assume_init` so that the `.value` field does not show up in error messages any more.

`@rust-lang/miri` what do you think?

14 files changed, 82 insertions, 107 deletions

diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs
index 317b1229a90..76fa744361a 100644
--- a/compiler/rustc_const_eval/src/const_eval/machine.rs
+++ b/compiler/rustc_const_eval/src/const_eval/machine.rs
@@ -10,7 +10,7 @@ use rustc_hir::{self as hir, CRATE_HIR_ID, LangItem};
 use rustc_middle::mir::AssertMessage;
 use rustc_middle::mir::interpret::ReportedErrorInfo;
 use rustc_middle::query::TyCtxtAt;
-use rustc_middle::ty::layout::{HasTypingEnv, TyAndLayout};
+use rustc_middle::ty::layout::{HasTypingEnv, TyAndLayout, ValidityRequirement};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_middle::{bug, mir};
 use rustc_span::{Span, Symbol, sym};
@@ -23,8 +23,8 @@ use crate::fluent_generated as fluent;
 use crate::interpret::{
     self, AllocId, AllocInit, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame,
     GlobalAlloc, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, RangeSet, Scalar,
-    compile_time_machine, interp_ok, throw_exhaust, throw_inval, throw_ub, throw_ub_custom,
-    throw_unsup, throw_unsup_format,
+    compile_time_machine, err_inval, interp_ok, throw_exhaust, throw_inval, throw_ub,
+    throw_ub_custom, throw_unsup, throw_unsup_format,
 };
 
 /// When hitting this many interpreted terminators we emit a deny by default lint
@@ -462,6 +462,44 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
             // (We know the value here in the machine of course, but this is the runtime of that code,
             // not the optimization stage.)
             sym::is_val_statically_known => ecx.write_scalar(Scalar::from_bool(false), dest)?,
+
+            // We handle these here since Miri does not want to have them.
+            sym::assert_inhabited
+            | sym::assert_zero_valid
+            | sym::assert_mem_uninitialized_valid => {
+                let ty = instance.args.type_at(0);
+                let requirement = ValidityRequirement::from_intrinsic(intrinsic_name).unwrap();
+
+                let should_panic = !ecx
+                    .tcx
+                    .check_validity_requirement((requirement, ecx.typing_env().as_query_input(ty)))
+                    .map_err(|_| err_inval!(TooGeneric))?;
+
+                if should_panic {
+                    let layout = ecx.layout_of(ty)?;
+
+                    let msg = match requirement {
+                        // For *all* intrinsics we first check `is_uninhabited` to give a more specific
+                        // error message.
+                        _ if layout.is_uninhabited() => format!(
+                            "aborted execution: attempted to instantiate uninhabited type `{ty}`"
+                        ),
+                        ValidityRequirement::Inhabited => bug!("handled earlier"),
+                        ValidityRequirement::Zero => format!(
+                            "aborted execution: attempted to zero-initialize type `{ty}`, which is invalid"
+                        ),
+                        ValidityRequirement::UninitMitigated0x01Fill => format!(
+                            "aborted execution: attempted to leave type `{ty}` uninitialized, which is invalid"
+                        ),
+                        ValidityRequirement::Uninit => bug!("assert_uninit_valid doesn't exist"),
+                    };
+
+                    Self::panic_nounwind(ecx, &msg)?;
+                    // Skip the `return_to_block` at the end (we panicked, we do not return).
+                    return interp_ok(None);
+                }
+            }
+
             _ => {
                 // 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 {
diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs
index d7cede71293..378ed6d0e10 100644
--- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs
+++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs
@@ -7,7 +7,7 @@ use std::assert_matches::assert_matches;
 use rustc_abi::Size;
 use rustc_apfloat::ieee::{Double, Half, Quad, Single};
 use rustc_middle::mir::{self, BinOp, ConstValue, NonDivergingIntrinsic};
-use rustc_middle::ty::layout::{TyAndLayout, ValidityRequirement};
+use rustc_middle::ty::layout::TyAndLayout;
 use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_middle::{bug, ty};
 use rustc_span::{Symbol, sym};
@@ -17,8 +17,8 @@ use super::memory::MemoryKind;
 use super::util::ensure_monomorphic_enough;
 use super::{
     Allocation, CheckInAllocMsg, ConstAllocation, ImmTy, InterpCx, InterpResult, Machine, OpTy,
-    PlaceTy, Pointer, PointerArithmetic, Provenance, Scalar, err_inval, err_ub_custom,
-    err_unsup_format, interp_ok, throw_inval, throw_ub_custom, throw_ub_format,
+    PlaceTy, Pointer, PointerArithmetic, Provenance, Scalar, err_ub_custom, err_unsup_format,
+    interp_ok, throw_inval, throw_ub_custom, throw_ub_format,
 };
 use crate::fluent_generated as fluent;
 
@@ -372,41 +372,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 self.exact_div(&val, &size, dest)?;
             }
 
-            sym::assert_inhabited
-            | sym::assert_zero_valid
-            | sym::assert_mem_uninitialized_valid => {
-                let ty = instance.args.type_at(0);
-                let requirement = ValidityRequirement::from_intrinsic(intrinsic_name).unwrap();
-
-                let should_panic = !self
-                    .tcx
-                    .check_validity_requirement((requirement, self.typing_env.as_query_input(ty)))
-                    .map_err(|_| err_inval!(TooGeneric))?;
-
-                if should_panic {
-                    let layout = self.layout_of(ty)?;
-
-                    let msg = match requirement {
-                        // For *all* intrinsics we first check `is_uninhabited` to give a more specific
-                        // error message.
-                        _ if layout.is_uninhabited() => format!(
-                            "aborted execution: attempted to instantiate uninhabited type `{ty}`"
-                        ),
-                        ValidityRequirement::Inhabited => bug!("handled earlier"),
-                        ValidityRequirement::Zero => format!(
-                            "aborted execution: attempted to zero-initialize type `{ty}`, which is invalid"
-                        ),
-                        ValidityRequirement::UninitMitigated0x01Fill => format!(
-                            "aborted execution: attempted to leave type `{ty}` uninitialized, which is invalid"
-                        ),
-                        ValidityRequirement::Uninit => bug!("assert_uninit_valid doesn't exist"),
-                    };
-
-                    M::panic_nounwind(self, &msg)?;
-                    // Skip the `return_to_block` at the end (we panicked, we do not return).
-                    return interp_ok(true);
-                }
-            }
             sym::simd_insert => {
                 let index = u64::from(self.read_scalar(&args[1])?.to_u32()?);
                 let elem = &args[2];
diff --git a/library/core/src/intrinsics/mod.rs b/library/core/src/intrinsics/mod.rs
index 4250de9fb2b..5827fa93bfb 100644
--- a/library/core/src/intrinsics/mod.rs
+++ b/library/core/src/intrinsics/mod.rs
@@ -472,7 +472,8 @@ pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
 }
 
 /// A guard for unsafe functions that cannot ever be executed if `T` is uninhabited:
-/// This will statically either panic, or do nothing.
+/// This will statically either panic, or do nothing. It does not *guarantee* to ever panic,
+/// and should only be called if an assertion failure will imply language UB in the following code.
 ///
 /// This intrinsic does not have a stable counterpart.
 #[rustc_intrinsic_const_stable_indirect]
@@ -481,7 +482,9 @@ pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
 pub const fn assert_inhabited<T>();
 
 /// A guard for unsafe functions that cannot ever be executed if `T` does not permit
-/// zero-initialization: This will statically either panic, or do nothing.
+/// zero-initialization: This will statically either panic, or do nothing. It does not *guarantee*
+/// to ever panic, and should only be called if an assertion failure will imply language UB in the
+/// following code.
 ///
 /// This intrinsic does not have a stable counterpart.
 #[rustc_intrinsic_const_stable_indirect]
@@ -489,7 +492,9 @@ pub const fn assert_inhabited<T>();
 #[rustc_intrinsic]
 pub const fn assert_zero_valid<T>();
 
-/// A guard for `std::mem::uninitialized`. This will statically either panic, or do nothing.
+/// A guard for `std::mem::uninitialized`. This will statically either panic, or do nothing. It does
+/// not *guarantee* to ever panic, and should only be called if an assertion failure will imply
+/// language UB in the following code.
 ///
 /// This intrinsic does not have a stable counterpart.
 #[rustc_intrinsic_const_stable_indirect]
diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs
index 63a479ed8dd..fc35e54bb0d 100644
--- a/library/core/src/mem/maybe_uninit.rs
+++ b/library/core/src/mem/maybe_uninit.rs
@@ -616,7 +616,9 @@ impl<T> MaybeUninit<T> {
         // This also means that `self` must be a `value` variant.
         unsafe {
             intrinsics::assert_inhabited::<T>();
-            ManuallyDrop::into_inner(self.value)
+            // We do this via a raw ptr read instead of `ManuallyDrop::into_inner` so that there's
+            // no trace of `ManuallyDrop` in Miri's error messages here.
+            (&raw const self.value).cast::<T>().read()
         }
     }
 
diff --git a/src/tools/miri/src/intrinsics/mod.rs b/src/tools/miri/src/intrinsics/mod.rs
index ed1851a19ae..4efa7dd4dcf 100644
--- a/src/tools/miri/src/intrinsics/mod.rs
+++ b/src/tools/miri/src/intrinsics/mod.rs
@@ -457,6 +457,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 throw_machine_stop!(TerminationInfo::Abort(format!("trace/breakpoint trap")))
             }
 
+            "assert_inhabited" | "assert_zero_valid" | "assert_mem_uninitialized_valid" => {
+                // Make these a NOP, so we get the better Miri-native error messages.
+            }
+
             _ => return interp_ok(EmulateItemResult::NotSupported),
         }
 
diff --git a/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs b/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs
index dd2dd346231..1dc334486c3 100644
--- a/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs
+++ b/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs
@@ -20,7 +20,7 @@ fn main() {
         let mut buf: MaybeUninit<[u8; 4]> = std::mem::MaybeUninit::uninit();
         // Read 4 bytes from a 3-byte file.
         assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 4), 3);
-        buf.assume_init(); //~ERROR: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
+        buf.assume_init(); //~ERROR: encountered uninitialized memory, but expected an integer
         assert_eq!(libc::close(fd), 0);
     }
     remove_file(&path).unwrap();
diff --git a/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.stderr b/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.stderr
index fadb31e3a8f..83119f087ff 100644
--- a/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.stderr
+++ b/src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.stderr
@@ -1,8 +1,8 @@
-error: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
+error: Undefined Behavior: constructing invalid value at [3]: encountered uninitialized memory, but expected an integer
   --> tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs:LL:CC
    |
-LL | ...   buf.assume_init();
-   |       ^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
+LL |         buf.assume_init();
+   |         ^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
diff --git a/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.rs b/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.rs
index dd3246d8120..34fef6b9ee5 100644
--- a/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.rs
+++ b/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.rs
@@ -1,11 +1,6 @@
-//@normalize-stderr-test: "\|.*::abort\(\).*" -> "| ABORT()"
-//@normalize-stderr-test: "\| +\^+" -> "| ^"
-//@normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> ""
-//@normalize-stderr-test: "\n +at [^\n]+" -> ""
-//@error-in-other-file: aborted execution
 #![feature(never_type)]
 
 #[allow(deprecated, invalid_value)]
 fn main() {
-    let _ = unsafe { std::mem::uninitialized::<!>() };
+    let _ = unsafe { std::mem::uninitialized::<!>() }; //~ERROR: constructing invalid value
 }
diff --git a/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.stderr b/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.stderr
index 3db8a5be205..36642208afe 100644
--- a/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.stderr
+++ b/src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.stderr
@@ -1,27 +1,13 @@
-
-thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
-aborted execution: attempted to instantiate uninhabited type `!`
-note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
-note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
-thread caused non-unwinding panic. aborting.
-error: abnormal termination: the program aborted execution
-  --> RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
-   |
-LL | ABORT()
-   | ^ abnormal termination occurred here
-   |
-   = note: BACKTRACE:
-   = note: inside `std::sys::pal::PLATFORM::abort_internal` at RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
-   = note: inside `std::panicking::rust_panic_with_hook` at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside closure at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside `std::sys::backtrace::__rust_end_short_backtrace::<{closure@std::panicking::begin_panic_handler::{closure#0}}, !>` at RUSTLIB/std/src/sys/backtrace.rs:LL:CC
-   = note: inside `std::panicking::begin_panic_handler` at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside `core::panicking::panic_nounwind` at RUSTLIB/core/src/panicking.rs:LL:CC
-note: inside `main`
+error: Undefined Behavior: constructing invalid value: encountered a value of the never type `!`
   --> tests/fail/intrinsics/uninit_uninhabited_type.rs:LL:CC
    |
 LL |     let _ = unsafe { std::mem::uninitialized::<!>() };
-   | ^
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at tests/fail/intrinsics/uninit_uninhabited_type.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 
diff --git a/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.rs b/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.rs
index 3d355bad626..cca2c5ae984 100644
--- a/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.rs
+++ b/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.rs
@@ -1,10 +1,4 @@
-//@normalize-stderr-test: "\|.*::abort\(\).*" -> "| ABORT()"
-//@normalize-stderr-test: "\| +\^+" -> "| ^"
-//@normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> ""
-//@normalize-stderr-test: "\n +at [^\n]+" -> ""
-//@error-in-other-file: aborted execution
-
 #[allow(deprecated, invalid_value)]
 fn main() {
-    let _ = unsafe { std::mem::zeroed::<fn()>() };
+    let _ = unsafe { std::mem::zeroed::<fn()>() }; //~ERROR: constructing invalid value
 }
diff --git a/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.stderr b/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.stderr
index a1e476328b0..53f3f8d1404 100644
--- a/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.stderr
+++ b/src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.stderr
@@ -1,27 +1,13 @@
-
-thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
-aborted execution: attempted to zero-initialize type `fn()`, which is invalid
-note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
-note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
-thread caused non-unwinding panic. aborting.
-error: abnormal termination: the program aborted execution
-  --> RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
-   |
-LL | ABORT()
-   | ^ abnormal termination occurred here
-   |
-   = note: BACKTRACE:
-   = note: inside `std::sys::pal::PLATFORM::abort_internal` at RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
-   = note: inside `std::panicking::rust_panic_with_hook` at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside closure at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside `std::sys::backtrace::__rust_end_short_backtrace::<{closure@std::panicking::begin_panic_handler::{closure#0}}, !>` at RUSTLIB/std/src/sys/backtrace.rs:LL:CC
-   = note: inside `std::panicking::begin_panic_handler` at RUSTLIB/std/src/panicking.rs:LL:CC
-   = note: inside `core::panicking::panic_nounwind` at RUSTLIB/core/src/panicking.rs:LL:CC
-note: inside `main`
+error: Undefined Behavior: constructing invalid value: encountered a null function pointer
   --> tests/fail/intrinsics/zero_fn_ptr.rs:LL:CC
    |
 LL |     let _ = unsafe { std::mem::zeroed::<fn()>() };
-   | ^
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at tests/fail/intrinsics/zero_fn_ptr.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 
diff --git a/src/tools/miri/tests/fail/validity/uninit_float.stderr b/src/tools/miri/tests/fail/validity/uninit_float.stderr
index 1b948b062e1..0c859d72764 100644
--- a/src/tools/miri/tests/fail/validity/uninit_float.stderr
+++ b/src/tools/miri/tests/fail/validity/uninit_float.stderr
@@ -1,4 +1,4 @@
-error: Undefined Behavior: constructing invalid value at .value[0]: encountered uninitialized memory, but expected a floating point number
+error: Undefined Behavior: constructing invalid value at [0]: encountered uninitialized memory, but expected a floating point number
   --> tests/fail/validity/uninit_float.rs:LL:CC
    |
 LL |     let _val: [f32; 1] = unsafe { std::mem::uninitialized() };
diff --git a/src/tools/miri/tests/fail/validity/uninit_integer.stderr b/src/tools/miri/tests/fail/validity/uninit_integer.stderr
index b17bdee65da..5d31e2659ee 100644
--- a/src/tools/miri/tests/fail/validity/uninit_integer.stderr
+++ b/src/tools/miri/tests/fail/validity/uninit_integer.stderr
@@ -1,4 +1,4 @@
-error: Undefined Behavior: constructing invalid value at .value[0]: encountered uninitialized memory, but expected an integer
+error: Undefined Behavior: constructing invalid value at [0]: encountered uninitialized memory, but expected an integer
   --> tests/fail/validity/uninit_integer.rs:LL:CC
    |
 LL |     let _val = unsafe { std::mem::MaybeUninit::<[usize; 1]>::uninit().assume_init() };
diff --git a/src/tools/miri/tests/fail/validity/uninit_raw_ptr.stderr b/src/tools/miri/tests/fail/validity/uninit_raw_ptr.stderr
index 269af6061c2..d2e9408adbe 100644
--- a/src/tools/miri/tests/fail/validity/uninit_raw_ptr.stderr
+++ b/src/tools/miri/tests/fail/validity/uninit_raw_ptr.stderr
@@ -1,4 +1,4 @@
-error: Undefined Behavior: constructing invalid value at .value[0]: encountered uninitialized memory, but expected a raw pointer
+error: Undefined Behavior: constructing invalid value at [0]: encountered uninitialized memory, but expected a raw pointer
   --> tests/fail/validity/uninit_raw_ptr.rs:LL:CC
    |
 LL |     let _val = unsafe { std::mem::MaybeUninit::<[*const u8; 1]>::uninit().assume_init() };