From 83dec62b2661ffa925bb63544a4b075577b07c16 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 21 Mar 2023 08:52:31 +0000 Subject: [PATCH 1/2] Add a layout argument to `enforce_validity`. This is in preparation of checking the validity only of certain types. --- compiler/rustc_const_eval/src/const_eval/machine.rs | 4 ++-- compiler/rustc_const_eval/src/interpret/machine.rs | 5 +++-- compiler/rustc_const_eval/src/interpret/place.rs | 4 ++-- compiler/rustc_mir_transform/src/const_prop.rs | 2 +- compiler/rustc_mir_transform/src/dataflow_const_prop.rs | 3 ++- src/tools/miri/src/machine.rs | 2 +- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index a44f70ed05906..5675973df8cfb 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -2,7 +2,7 @@ use rustc_hir::def::DefKind; use rustc_hir::{LangItem, CRATE_HIR_ID}; use rustc_middle::mir; use rustc_middle::mir::interpret::PointerArithmetic; -use rustc_middle::ty::layout::FnAbiOf; +use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout}; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::lint::builtin::INVALID_ALIGNMENT; use std::borrow::Borrow; @@ -335,7 +335,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } #[inline(always)] - fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { + fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool { ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks } diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index c134d3a6b2f2a..aca68dc454bee 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -8,6 +8,7 @@ use std::hash::Hash; use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece}; use rustc_middle::mir; +use rustc_middle::ty::layout::TyAndLayout; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::def_id::DefId; use rustc_target::abi::{Align, Size}; @@ -145,8 +146,8 @@ pub trait Machine<'mir, 'tcx>: Sized { check: CheckAlignment, ) -> InterpResult<'tcx, ()>; - /// Whether to enforce the validity invariant - fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; + /// Whether to enforce the validity invariant for a specific layout. + fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool; /// Whether function calls should be [ABI](CallAbi)-checked. fn enforce_abi(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 3c463500a609e..ff6db143ddfd9 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -461,7 +461,7 @@ where ) -> InterpResult<'tcx> { self.write_immediate_no_validate(src, dest)?; - if M::enforce_validity(self) { + if M::enforce_validity(self, dest.layout) { // Data got changed, better make sure it matches the type! self.validate_operand(&self.place_to_op(dest)?)?; } @@ -616,7 +616,7 @@ where ) -> InterpResult<'tcx> { self.copy_op_no_validate(src, dest, allow_transmute)?; - if M::enforce_validity(self) { + if M::enforce_validity(self, dest.layout) { // Data got changed, better make sure it matches the type! self.validate_operand(&self.place_to_op(dest)?)?; } diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index de7b8c63fc87a..a735dcec2d2ff 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -174,7 +174,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> } #[inline(always)] - fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { + fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool { false // for now, we don't enforce validity } fn alignment_check_failed( diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 49ded10ba1fed..a7218a4f2500d 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -8,6 +8,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::DefKind; use rustc_middle::mir::visit::{MutVisitor, Visitor}; use rustc_middle::mir::*; +use rustc_middle::ty::layout::TyAndLayout; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_mir_dataflow::value_analysis::{Map, State, TrackElem, ValueAnalysis, ValueOrPlace}; use rustc_mir_dataflow::{lattice::FlatSet, Analysis, ResultsVisitor, SwitchIntEdgeEffects}; @@ -548,7 +549,7 @@ impl<'mir, 'tcx> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachi unimplemented!() } - fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { + fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool { unimplemented!() } fn alignment_check_failed( diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index c4baeb2a73bf5..cc1964de332c2 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -812,7 +812,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } #[inline(always)] - fn enforce_validity(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { + fn enforce_validity(ecx: &MiriInterpCx<'mir, 'tcx>, _layout: TyAndLayout<'tcx>) -> bool { ecx.machine.validate } From f066d6785dc37445e06230b7704faef75489a80f Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 21 Mar 2023 10:20:00 +0000 Subject: [PATCH 2/2] Detect uninhabited types early in const eval. --- .../rustc_const_eval/src/const_eval/machine.rs | 4 ++-- tests/ui/consts/const-eval/ub-uninhabit.rs | 4 ++-- tests/ui/consts/const-eval/ub-uninhabit.stderr | 18 ++++++------------ .../validate_uninhabited_zsts.32bit.stderr | 9 +++------ .../validate_uninhabited_zsts.64bit.stderr | 9 +++------ .../const-eval/validate_uninhabited_zsts.rs | 2 +- tests/ui/consts/issue-64506.rs | 3 ++- tests/ui/consts/issue-64506.stderr | 9 +++++++++ 8 files changed, 28 insertions(+), 30 deletions(-) create mode 100644 tests/ui/consts/issue-64506.stderr diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 5675973df8cfb..350ce529ef538 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -335,8 +335,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } #[inline(always)] - fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool { - ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks + fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool { + ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks || layout.abi.is_uninhabited() } fn alignment_check_failed( diff --git a/tests/ui/consts/const-eval/ub-uninhabit.rs b/tests/ui/consts/const-eval/ub-uninhabit.rs index 4c4ef216d8628..10edae437ee74 100644 --- a/tests/ui/consts/const-eval/ub-uninhabit.rs +++ b/tests/ui/consts/const-eval/ub-uninhabit.rs @@ -14,12 +14,12 @@ union MaybeUninit { } const BAD_BAD_BAD: Bar = unsafe { MaybeUninit { uninit: () }.init }; -//~^ ERROR it is undefined behavior to use this value +//~^ ERROR evaluation of constant value failed const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) }; //~^ ERROR it is undefined behavior to use this value const BAD_BAD_ARRAY: [Bar; 1] = unsafe { MaybeUninit { uninit: () }.init }; -//~^ ERROR it is undefined behavior to use this value +//~^ ERROR evaluation of constant value failed fn main() {} diff --git a/tests/ui/consts/const-eval/ub-uninhabit.stderr b/tests/ui/consts/const-eval/ub-uninhabit.stderr index 0ae376d03fc37..733975fc0e9ce 100644 --- a/tests/ui/consts/const-eval/ub-uninhabit.stderr +++ b/tests/ui/consts/const-eval/ub-uninhabit.stderr @@ -1,11 +1,8 @@ -error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-uninhabit.rs:16:1 +error[E0080]: evaluation of constant value failed + --> $DIR/ub-uninhabit.rs:16:35 | LL | const BAD_BAD_BAD: Bar = unsafe { MaybeUninit { uninit: () }.init }; - | ^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a value of uninhabited type Bar - | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. - = note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a value of uninhabited type Bar error[E0080]: it is undefined behavior to use this value --> $DIR/ub-uninhabit.rs:19:1 @@ -18,14 +15,11 @@ LL | const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) }; HEX_DUMP } -error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-uninhabit.rs:22:1 +error[E0080]: evaluation of constant value failed + --> $DIR/ub-uninhabit.rs:22:42 | LL | const BAD_BAD_ARRAY: [Bar; 1] = unsafe { MaybeUninit { uninit: () }.init }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at [0]: encountered a value of uninhabited type Bar - | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. - = note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at [0]: encountered a value of uninhabited type Bar error: aborting due to 3 previous errors diff --git a/tests/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr b/tests/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr index 69fb1a59d4f39..231005d7e3975 100644 --- a/tests/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr +++ b/tests/ui/consts/const-eval/validate_uninhabited_zsts.32bit.stderr @@ -24,14 +24,11 @@ note: inside `FOO` LL | const FOO: [empty::Empty; 3] = [foo(); 3]; | ^^^^^ -error[E0080]: it is undefined behavior to use this value - --> $DIR/validate_uninhabited_zsts.rs:21:1 +error[E0080]: evaluation of constant value failed + --> $DIR/validate_uninhabited_zsts.rs:21:42 | LL | const BAR: [empty::Empty; 3] = [unsafe { std::mem::transmute(()) }; 3]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at [0].0: encountered a value of uninhabited type empty::Void - | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. - = note: the raw bytes of the constant (size: 0, align: 1) {} + | ^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .0: encountered a value of uninhabited type empty::Void warning: the type `empty::Empty` does not permit zero-initialization --> $DIR/validate_uninhabited_zsts.rs:21:42 diff --git a/tests/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr b/tests/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr index 69fb1a59d4f39..231005d7e3975 100644 --- a/tests/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr +++ b/tests/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr @@ -24,14 +24,11 @@ note: inside `FOO` LL | const FOO: [empty::Empty; 3] = [foo(); 3]; | ^^^^^ -error[E0080]: it is undefined behavior to use this value - --> $DIR/validate_uninhabited_zsts.rs:21:1 +error[E0080]: evaluation of constant value failed + --> $DIR/validate_uninhabited_zsts.rs:21:42 | LL | const BAR: [empty::Empty; 3] = [unsafe { std::mem::transmute(()) }; 3]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at [0].0: encountered a value of uninhabited type empty::Void - | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. - = note: the raw bytes of the constant (size: 0, align: 1) {} + | ^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .0: encountered a value of uninhabited type empty::Void warning: the type `empty::Empty` does not permit zero-initialization --> $DIR/validate_uninhabited_zsts.rs:21:42 diff --git a/tests/ui/consts/const-eval/validate_uninhabited_zsts.rs b/tests/ui/consts/const-eval/validate_uninhabited_zsts.rs index c0b3262150575..b6783175dd379 100644 --- a/tests/ui/consts/const-eval/validate_uninhabited_zsts.rs +++ b/tests/ui/consts/const-eval/validate_uninhabited_zsts.rs @@ -19,7 +19,7 @@ pub mod empty { const FOO: [empty::Empty; 3] = [foo(); 3]; const BAR: [empty::Empty; 3] = [unsafe { std::mem::transmute(()) }; 3]; -//~^ ERROR it is undefined behavior to use this value +//~^ ERROR evaluation of constant value failed //~| WARN the type `empty::Empty` does not permit zero-initialization fn main() { diff --git a/tests/ui/consts/issue-64506.rs b/tests/ui/consts/issue-64506.rs index db3e85a7bdfd1..9275a8a072dde 100644 --- a/tests/ui/consts/issue-64506.rs +++ b/tests/ui/consts/issue-64506.rs @@ -1,4 +1,4 @@ -// check-pass +// check-fail #[derive(Copy, Clone)] pub struct ChildStdin { @@ -14,6 +14,7 @@ const FOO: () = { b: (), } let x = unsafe { Foo { b: () }.a }; + //~^ ERROR: evaluation of constant value failed let x = &x.inner; }; diff --git a/tests/ui/consts/issue-64506.stderr b/tests/ui/consts/issue-64506.stderr new file mode 100644 index 0000000000000..31a5b1df837c5 --- /dev/null +++ b/tests/ui/consts/issue-64506.stderr @@ -0,0 +1,9 @@ +error[E0080]: evaluation of constant value failed + --> $DIR/issue-64506.rs:16:22 + | +LL | let x = unsafe { Foo { b: () }.a }; + | ^^^^^^^^^^^^^^^ constructing invalid value at .inner: encountered a value of uninhabited type AnonPipe + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0080`.