Skip to content

Commit bc0262d

Browse files
matthiaskrgrmatthiaskrgr
authored
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?
2 parents 5aa7dd8 + 8362508 commit bc0262d

File tree

14 files changed

+82
-107
lines changed

14 files changed

+82
-107
lines changed

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

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_hir::{self as hir, CRATE_HIR_ID, LangItem};
1010
use rustc_middle::mir::AssertMessage;
1111
use rustc_middle::mir::interpret::ReportedErrorInfo;
1212
use rustc_middle::query::TyCtxtAt;
13-
use rustc_middle::ty::layout::{HasTypingEnv, TyAndLayout};
13+
use rustc_middle::ty::layout::{HasTypingEnv, TyAndLayout, ValidityRequirement};
1414
use rustc_middle::ty::{self, Ty, TyCtxt};
1515
use rustc_middle::{bug, mir};
1616
use rustc_span::{Span, Symbol, sym};
@@ -23,8 +23,8 @@ use crate::fluent_generated as fluent;
2323
use crate::interpret::{
2424
self, AllocId, AllocInit, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame,
2525
GlobalAlloc, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, RangeSet, Scalar,
26-
compile_time_machine, interp_ok, throw_exhaust, throw_inval, throw_ub, throw_ub_custom,
27-
throw_unsup, throw_unsup_format,
26+
compile_time_machine, err_inval, interp_ok, throw_exhaust, throw_inval, throw_ub,
27+
throw_ub_custom, throw_unsup, throw_unsup_format,
2828
};
2929

3030
/// 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> {
462462
// (We know the value here in the machine of course, but this is the runtime of that code,
463463
// not the optimization stage.)
464464
sym::is_val_statically_known => ecx.write_scalar(Scalar::from_bool(false), dest)?,
465+
466+
// We handle these here since Miri does not want to have them.
467+
sym::assert_inhabited
468+
| sym::assert_zero_valid
469+
| sym::assert_mem_uninitialized_valid => {
470+
let ty = instance.args.type_at(0);
471+
let requirement = ValidityRequirement::from_intrinsic(intrinsic_name).unwrap();
472+
473+
let should_panic = !ecx
474+
.tcx
475+
.check_validity_requirement((requirement, ecx.typing_env().as_query_input(ty)))
476+
.map_err(|_| err_inval!(TooGeneric))?;
477+
478+
if should_panic {
479+
let layout = ecx.layout_of(ty)?;
480+
481+
let msg = match requirement {
482+
// For *all* intrinsics we first check `is_uninhabited` to give a more specific
483+
// error message.
484+
_ if layout.is_uninhabited() => format!(
485+
"aborted execution: attempted to instantiate uninhabited type `{ty}`"
486+
),
487+
ValidityRequirement::Inhabited => bug!("handled earlier"),
488+
ValidityRequirement::Zero => format!(
489+
"aborted execution: attempted to zero-initialize type `{ty}`, which is invalid"
490+
),
491+
ValidityRequirement::UninitMitigated0x01Fill => format!(
492+
"aborted execution: attempted to leave type `{ty}` uninitialized, which is invalid"
493+
),
494+
ValidityRequirement::Uninit => bug!("assert_uninit_valid doesn't exist"),
495+
};
496+
497+
Self::panic_nounwind(ecx, &msg)?;
498+
// Skip the `return_to_block` at the end (we panicked, we do not return).
499+
return interp_ok(None);
500+
}
501+
}
502+
465503
_ => {
466504
// We haven't handled the intrinsic, let's see if we can use a fallback body.
467505
if ecx.tcx.intrinsic(instance.def_id()).unwrap().must_be_overridden {

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

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::assert_matches::assert_matches;
77
use rustc_abi::Size;
88
use rustc_apfloat::ieee::{Double, Half, Quad, Single};
99
use rustc_middle::mir::{self, BinOp, ConstValue, NonDivergingIntrinsic};
10-
use rustc_middle::ty::layout::{TyAndLayout, ValidityRequirement};
10+
use rustc_middle::ty::layout::TyAndLayout;
1111
use rustc_middle::ty::{Ty, TyCtxt};
1212
use rustc_middle::{bug, ty};
1313
use rustc_span::{Symbol, sym};
@@ -17,8 +17,8 @@ use super::memory::MemoryKind;
1717
use super::util::ensure_monomorphic_enough;
1818
use super::{
1919
Allocation, CheckInAllocMsg, ConstAllocation, ImmTy, InterpCx, InterpResult, Machine, OpTy,
20-
PlaceTy, Pointer, PointerArithmetic, Provenance, Scalar, err_inval, err_ub_custom,
21-
err_unsup_format, interp_ok, throw_inval, throw_ub_custom, throw_ub_format,
20+
PlaceTy, Pointer, PointerArithmetic, Provenance, Scalar, err_ub_custom, err_unsup_format,
21+
interp_ok, throw_inval, throw_ub_custom, throw_ub_format,
2222
};
2323
use crate::fluent_generated as fluent;
2424

@@ -372,41 +372,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
372372
self.exact_div(&val, &size, dest)?;
373373
}
374374

375-
sym::assert_inhabited
376-
| sym::assert_zero_valid
377-
| sym::assert_mem_uninitialized_valid => {
378-
let ty = instance.args.type_at(0);
379-
let requirement = ValidityRequirement::from_intrinsic(intrinsic_name).unwrap();
380-
381-
let should_panic = !self
382-
.tcx
383-
.check_validity_requirement((requirement, self.typing_env.as_query_input(ty)))
384-
.map_err(|_| err_inval!(TooGeneric))?;
385-
386-
if should_panic {
387-
let layout = self.layout_of(ty)?;
388-
389-
let msg = match requirement {
390-
// For *all* intrinsics we first check `is_uninhabited` to give a more specific
391-
// error message.
392-
_ if layout.is_uninhabited() => format!(
393-
"aborted execution: attempted to instantiate uninhabited type `{ty}`"
394-
),
395-
ValidityRequirement::Inhabited => bug!("handled earlier"),
396-
ValidityRequirement::Zero => format!(
397-
"aborted execution: attempted to zero-initialize type `{ty}`, which is invalid"
398-
),
399-
ValidityRequirement::UninitMitigated0x01Fill => format!(
400-
"aborted execution: attempted to leave type `{ty}` uninitialized, which is invalid"
401-
),
402-
ValidityRequirement::Uninit => bug!("assert_uninit_valid doesn't exist"),
403-
};
404-
405-
M::panic_nounwind(self, &msg)?;
406-
// Skip the `return_to_block` at the end (we panicked, we do not return).
407-
return interp_ok(true);
408-
}
409-
}
410375
sym::simd_insert => {
411376
let index = u64::from(self.read_scalar(&args[1])?.to_u32()?);
412377
let elem = &args[2];

‎library/core/src/intrinsics/mod.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,8 @@ pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
472472
}
473473

474474
/// A guard for unsafe functions that cannot ever be executed if `T` is uninhabited:
475-
/// This will statically either panic, or do nothing.
475+
/// This will statically either panic, or do nothing. It does not *guarantee* to ever panic,
476+
/// and should only be called if an assertion failure will imply language UB in the following code.
476477
///
477478
/// This intrinsic does not have a stable counterpart.
478479
#[rustc_intrinsic_const_stable_indirect]
@@ -481,15 +482,19 @@ pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
481482
pub const fn assert_inhabited<T>();
482483

483484
/// A guard for unsafe functions that cannot ever be executed if `T` does not permit
484-
/// zero-initialization: This will statically either panic, or do nothing.
485+
/// zero-initialization: This will statically either panic, or do nothing. It does not *guarantee*
486+
/// to ever panic, and should only be called if an assertion failure will imply language UB in the
487+
/// following code.
485488
///
486489
/// This intrinsic does not have a stable counterpart.
487490
#[rustc_intrinsic_const_stable_indirect]
488491
#[rustc_nounwind]
489492
#[rustc_intrinsic]
490493
pub const fn assert_zero_valid<T>();
491494

492-
/// A guard for `std::mem::uninitialized`. This will statically either panic, or do nothing.
495+
/// A guard for `std::mem::uninitialized`. This will statically either panic, or do nothing. It does
496+
/// not *guarantee* to ever panic, and should only be called if an assertion failure will imply
497+
/// language UB in the following code.
493498
///
494499
/// This intrinsic does not have a stable counterpart.
495500
#[rustc_intrinsic_const_stable_indirect]

‎library/core/src/mem/maybe_uninit.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,9 @@ impl<T> MaybeUninit<T> {
616616
// This also means that `self` must be a `value` variant.
617617
unsafe {
618618
intrinsics::assert_inhabited::<T>();
619-
ManuallyDrop::into_inner(self.value)
619+
// We do this via a raw ptr read instead of `ManuallyDrop::into_inner` so that there's
620+
// no trace of `ManuallyDrop` in Miri's error messages here.
621+
(&raw const self.value).cast::<T>().read()
620622
}
621623
}
622624

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
457457
throw_machine_stop!(TerminationInfo::Abort(format!("trace/breakpoint trap")))
458458
}
459459

460+
"assert_inhabited" | "assert_zero_valid" | "assert_mem_uninitialized_valid" => {
461+
// Make these a NOP, so we get the better Miri-native error messages.
462+
}
463+
460464
_ => return interp_ok(EmulateItemResult::NotSupported),
461465
}
462466

‎src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ fn main() {
2020
let mut buf: MaybeUninit<[u8; 4]> = std::mem::MaybeUninit::uninit();
2121
// Read 4 bytes from a 3-byte file.
2222
assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 4), 3);
23-
buf.assume_init(); //~ERROR: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
23+
buf.assume_init(); //~ERROR: encountered uninitialized memory, but expected an integer
2424
assert_eq!(libc::close(fd), 0);
2525
}
2626
remove_file(&path).unwrap();

‎src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
1+
error: Undefined Behavior: constructing invalid value at [3]: encountered uninitialized memory, but expected an integer
22
--> tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs:LL:CC
33
|
4-
LL | ... buf.assume_init();
5-
| ^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
4+
LL | buf.assume_init();
5+
| ^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

‎src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
1-
//@normalize-stderr-test: "\|.*::abort\(\).*" -> "| ABORT()"
2-
//@normalize-stderr-test: "\| +\^+" -> "| ^"
3-
//@normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> ""
4-
//@normalize-stderr-test: "\n +at [^\n]+" -> ""
5-
//@error-in-other-file: aborted execution
61
#![feature(never_type)]
72

83
#[allow(deprecated, invalid_value)]
94
fn main() {
10-
let _ = unsafe { std::mem::uninitialized::<!>() };
5+
let _ = unsafe { std::mem::uninitialized::<!>() }; //~ERROR: constructing invalid value
116
}

‎src/tools/miri/tests/fail/intrinsics/uninit_uninhabited_type.stderr

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,13 @@
1-
2-
thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
3-
aborted execution: attempted to instantiate uninhabited type `!`
4-
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
5-
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
6-
thread caused non-unwinding panic. aborting.
7-
error: abnormal termination: the program aborted execution
8-
--> RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
9-
|
10-
LL | ABORT()
11-
| ^ abnormal termination occurred here
12-
|
13-
= note: BACKTRACE:
14-
= note: inside `std::sys::pal::PLATFORM::abort_internal` at RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
15-
= note: inside `std::panicking::rust_panic_with_hook` at RUSTLIB/std/src/panicking.rs:LL:CC
16-
= note: inside closure at RUSTLIB/std/src/panicking.rs:LL:CC
17-
= 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
18-
= note: inside `std::panicking::begin_panic_handler` at RUSTLIB/std/src/panicking.rs:LL:CC
19-
= note: inside `core::panicking::panic_nounwind` at RUSTLIB/core/src/panicking.rs:LL:CC
20-
note: inside `main`
1+
error: Undefined Behavior: constructing invalid value: encountered a value of the never type `!`
212
--> tests/fail/intrinsics/uninit_uninhabited_type.rs:LL:CC
223
|
234
LL | let _ = unsafe { std::mem::uninitialized::<!>() };
24-
| ^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at tests/fail/intrinsics/uninit_uninhabited_type.rs:LL:CC
2511

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

‎src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
//@normalize-stderr-test: "\|.*::abort\(\).*" -> "| ABORT()"
2-
//@normalize-stderr-test: "\| +\^+" -> "| ^"
3-
//@normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> ""
4-
//@normalize-stderr-test: "\n +at [^\n]+" -> ""
5-
//@error-in-other-file: aborted execution
6-
71
#[allow(deprecated, invalid_value)]
82
fn main() {
9-
let _ = unsafe { std::mem::zeroed::<fn()>() };
3+
let _ = unsafe { std::mem::zeroed::<fn()>() }; //~ERROR: constructing invalid value
104
}

‎src/tools/miri/tests/fail/intrinsics/zero_fn_ptr.stderr

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,13 @@
1-
2-
thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
3-
aborted execution: attempted to zero-initialize type `fn()`, which is invalid
4-
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
5-
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
6-
thread caused non-unwinding panic. aborting.
7-
error: abnormal termination: the program aborted execution
8-
--> RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
9-
|
10-
LL | ABORT()
11-
| ^ abnormal termination occurred here
12-
|
13-
= note: BACKTRACE:
14-
= note: inside `std::sys::pal::PLATFORM::abort_internal` at RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC
15-
= note: inside `std::panicking::rust_panic_with_hook` at RUSTLIB/std/src/panicking.rs:LL:CC
16-
= note: inside closure at RUSTLIB/std/src/panicking.rs:LL:CC
17-
= 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
18-
= note: inside `std::panicking::begin_panic_handler` at RUSTLIB/std/src/panicking.rs:LL:CC
19-
= note: inside `core::panicking::panic_nounwind` at RUSTLIB/core/src/panicking.rs:LL:CC
20-
note: inside `main`
1+
error: Undefined Behavior: constructing invalid value: encountered a null function pointer
212
--> tests/fail/intrinsics/zero_fn_ptr.rs:LL:CC
223
|
234
LL | let _ = unsafe { std::mem::zeroed::<fn()>() };
24-
| ^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at tests/fail/intrinsics/zero_fn_ptr.rs:LL:CC
2511

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

‎src/tools/miri/tests/fail/validity/uninit_float.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: Undefined Behavior: constructing invalid value at .value[0]: encountered uninitialized memory, but expected a floating point number
1+
error: Undefined Behavior: constructing invalid value at [0]: encountered uninitialized memory, but expected a floating point number
22
--> tests/fail/validity/uninit_float.rs:LL:CC
33
|
44
LL | let _val: [f32; 1] = unsafe { std::mem::uninitialized() };

‎src/tools/miri/tests/fail/validity/uninit_integer.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: Undefined Behavior: constructing invalid value at .value[0]: encountered uninitialized memory, but expected an integer
1+
error: Undefined Behavior: constructing invalid value at [0]: encountered uninitialized memory, but expected an integer
22
--> tests/fail/validity/uninit_integer.rs:LL:CC
33
|
44
LL | let _val = unsafe { std::mem::MaybeUninit::<[usize; 1]>::uninit().assume_init() };

‎src/tools/miri/tests/fail/validity/uninit_raw_ptr.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: Undefined Behavior: constructing invalid value at .value[0]: encountered uninitialized memory, but expected a raw pointer
1+
error: Undefined Behavior: constructing invalid value at [0]: encountered uninitialized memory, but expected a raw pointer
22
--> tests/fail/validity/uninit_raw_ptr.rs:LL:CC
33
|
44
LL | let _val = unsafe { std::mem::MaybeUninit::<[*const u8; 1]>::uninit().assume_init() };

0 commit comments

Comments
 (0)