From b50c1b243e09284b7bbfb81c1819d358d024168d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Nov 2018 14:17:40 +0100 Subject: [PATCH 1/4] Make const_eval_raw query return just an AllocId --- src/librustc/ich/impls_ty.rs | 4 ++ src/librustc/mir/interpret/error.rs | 3 +- src/librustc/mir/interpret/mod.rs | 4 +- src/librustc/mir/interpret/value.rs | 15 ++++-- src/librustc/ty/query/mod.rs | 4 +- src/librustc_mir/const_eval.rs | 46 ++++++++++--------- src/librustc_mir/interpret/memory.rs | 15 +++--- src/librustc_mir/interpret/operand.rs | 3 +- src/librustc_mir/interpret/place.rs | 19 ++++++-- src/librustc_mir/transform/const_prop.rs | 2 +- src/test/ui/consts/const-err4.rs | 2 +- src/test/ui/consts/const-err4.stderr | 6 ++- .../const-pointer-values-in-various-types.rs | 4 +- ...nst-pointer-values-in-various-types.stderr | 12 +++-- .../const-eval/union-const-eval-field.rs | 3 +- .../const-eval/union-const-eval-field.stderr | 9 ++-- src/test/ui/consts/const-eval/union-ice.rs | 2 +- .../ui/consts/const-eval/union-ice.stderr | 8 ++-- 18 files changed, 97 insertions(+), 64 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 64685446e8f..679107160a6 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -317,6 +317,10 @@ impl_stable_hash_for!( ByRef(id, alloc, offset), } ); +impl_stable_hash_for!(struct ::mir::interpret::RawConst<'tcx> { + alloc_id, + ty, +}); impl_stable_hash_for! { impl for struct mir::interpret::Pointer { diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index f1c95e0f000..f1ac4b21058 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -16,7 +16,7 @@ use ty::{self, Ty, layout}; use ty::layout::{Size, Align, LayoutError}; use rustc_target::spec::abi::Abi; -use super::{Pointer, InboundsCheck, ScalarMaybeUndef}; +use super::{RawConst, Pointer, InboundsCheck, ScalarMaybeUndef}; use backtrace::Backtrace; @@ -46,6 +46,7 @@ impl ErrorHandled { } } +pub type ConstEvalRawResult<'tcx> = Result, ErrorHandled>; pub type ConstEvalResult<'tcx> = Result<&'tcx ty::Const<'tcx>, ErrorHandled>; #[derive(Clone, Debug, RustcEncodable, RustcDecodable)] diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index ec25431bd1f..9369b6e56f1 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -22,10 +22,10 @@ mod pointer; pub use self::error::{ EvalError, EvalResult, EvalErrorKind, AssertMessage, ConstEvalErr, struct_error, - FrameInfo, ConstEvalResult, ErrorHandled, + FrameInfo, ConstEvalRawResult, ConstEvalResult, ErrorHandled, }; -pub use self::value::{Scalar, ConstValue, ScalarMaybeUndef}; +pub use self::value::{Scalar, ScalarMaybeUndef, RawConst, ConstValue}; pub use self::allocation::{ InboundsCheck, Allocation, AllocationExtra, diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 47c42c9431a..4bcba9d5467 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -10,12 +10,20 @@ use std::fmt; -use ty::layout::{HasDataLayout, Size}; -use ty::subst::Substs; -use hir::def_id::DefId; +use crate::ty::{Ty, subst::Substs, layout::{HasDataLayout, Size}}; +use crate::hir::def_id::DefId; use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_extend, truncate}; +/// Represents the result of a raw const operation, pre-validation. +#[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Hash)] +pub struct RawConst<'tcx> { + // the value lives here, at offset 0, and that allocation definitely is a `AllocType::Memory` + // (so you can use `AllocMap::unwrap_memory`). + pub alloc_id: AllocId, + pub ty: Ty<'tcx>, +} + /// Represents a constant value in Rust. Scalar and ScalarPair are optimizations which /// matches the LocalValue optimizations for easy conversions between Value and ConstValue. #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Hash)] @@ -23,6 +31,7 @@ pub enum ConstValue<'tcx> { /// Never returned from the `const_eval` query, but the HIR contains these frequently in order /// to allow HIR creation to happen for everything before needing to be able to run constant /// evaluation + /// FIXME: The query should then return a type that does not even have this variant. Unevaluated(DefId, &'tcx Substs<'tcx>), /// Used only for types with layout::abi::Scalar ABI and ZSTs diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 89e7f4db502..22bd1e26ba3 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -27,7 +27,7 @@ use middle::stability::{self, DeprecationEntry}; use middle::lib_features::LibFeatures; use middle::lang_items::{LanguageItems, LangItem}; use middle::exported_symbols::{SymbolExportLevel, ExportedSymbol}; -use mir::interpret::ConstEvalResult; +use mir::interpret::{ConstEvalRawResult, ConstEvalResult}; use mir::mono::CodegenUnit; use mir; use mir::interpret::GlobalId; @@ -309,7 +309,7 @@ define_queries! { <'tcx> /// validation. Please add a comment to every use site explaining why using `const_eval` /// isn't sufficient [] fn const_eval_raw: const_eval_raw_dep_node(ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>) - -> ConstEvalResult<'tcx>, + -> ConstEvalRawResult<'tcx>, /// Results of evaluating const items or constants embedded in /// other items (such as enum variant explicit discriminants). diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 51046399ec2..4e727de5358 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -31,8 +31,8 @@ use rustc::util::common::ErrorReported; use syntax::ast::Mutability; use syntax::source_map::{Span, DUMMY_SP}; -use interpret::{self, - PlaceTy, MemPlace, OpTy, Operand, Immediate, Scalar, ConstValue, Pointer, +use crate::interpret::{self, + PlaceTy, MPlaceTy, MemPlace, OpTy, Operand, Immediate, Scalar, RawConst, ConstValue, Pointer, EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup, Allocation, AllocId, MemoryKind, snapshot, RefTracking, @@ -94,11 +94,13 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( cid: GlobalId<'tcx>, mir: &'mir mir::Mir<'tcx>, param_env: ty::ParamEnv<'tcx>, -) -> EvalResult<'tcx, OpTy<'tcx>> { +) -> EvalResult<'tcx, MPlaceTy<'tcx>> { let mut ecx = mk_borrowck_eval_cx(tcx, cid.instance, mir, DUMMY_SP).unwrap(); eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env) } +// FIXME: This thing is a bad hack. We should get rid of it. Ideally constants are always +// in an allocation. pub fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, op: OpTy<'tcx>, @@ -150,7 +152,7 @@ fn eval_body_and_ecx<'a, 'mir, 'tcx>( cid: GlobalId<'tcx>, mir: Option<&'mir mir::Mir<'tcx>>, param_env: ty::ParamEnv<'tcx>, -) -> (EvalResult<'tcx, OpTy<'tcx>>, CompileTimeEvalContext<'a, 'mir, 'tcx>) { +) -> (EvalResult<'tcx, MPlaceTy<'tcx>>, CompileTimeEvalContext<'a, 'mir, 'tcx>) { // we start out with the best span we have // and try improving it down the road when more information is available let span = tcx.def_span(cid.instance.def_id()); @@ -166,7 +168,7 @@ fn eval_body_using_ecx<'mir, 'tcx>( cid: GlobalId<'tcx>, mir: Option<&'mir mir::Mir<'tcx>>, param_env: ty::ParamEnv<'tcx>, -) -> EvalResult<'tcx, OpTy<'tcx>> { +) -> EvalResult<'tcx, MPlaceTy<'tcx>> { debug!("eval_body_using_ecx: {:?}, {:?}", cid, param_env); let tcx = ecx.tcx.tcx; let mut mir = match mir { @@ -206,7 +208,7 @@ fn eval_body_using_ecx<'mir, 'tcx>( ecx.memory.intern_static(ret.ptr.to_ptr()?.alloc_id, mutability)?; debug!("eval_body_using_ecx done: {:?}", *ret); - Ok(ret.into()) + Ok(ret) } impl<'tcx> Into> for ConstEvalError { @@ -534,15 +536,17 @@ pub fn error_to_const_error<'a, 'mir, 'tcx>( ConstEvalErr { error: error.kind, stacktrace, span: ecx.tcx.span } } -fn validate_const<'a, 'tcx>( +fn validate_and_turn_into_const<'a, 'tcx>( tcx: ty::TyCtxt<'a, 'tcx, 'tcx>, - constant: &'tcx ty::Const<'tcx>, + constant: RawConst<'tcx>, key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>, ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { let cid = key.value; let ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap(); let val = (|| { - let op = ecx.const_to_op(constant)?; + let op = ecx.raw_const_to_mplace(constant)?.into(); + // FIXME: Once the visitor infrastructure landed, change validation to + // work directly on `MPlaceTy`. let mut ref_tracking = RefTracking::new(op); while let Some((op, path)) = ref_tracking.todo.pop() { ecx.validate_operand( @@ -552,7 +556,10 @@ fn validate_const<'a, 'tcx>( /* const_mode */ true, )?; } - Ok(constant) + // Now that we validated, turn this into a proper constant + let def_id = cid.instance.def.def_id(); + let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none(); + op_to_const(&ecx, op, normalize) })(); val.map_err(|error| { @@ -591,14 +598,14 @@ pub fn const_eval_provider<'a, 'tcx>( } } tcx.const_eval_raw(key).and_then(|val| { - validate_const(tcx, val, key) + validate_and_turn_into_const(tcx, val, key) }) } pub fn const_eval_raw_provider<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>, -) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { +) -> ::rustc::mir::interpret::ConstEvalRawResult<'tcx> { // Because the constant is computed twice (once per value of `Reveal`), we are at risk of // reporting the same error twice here. To resolve this, we check whether we can evaluate the // constant in the more restrictive `Reveal::UserFacing`, which most likely already was @@ -648,16 +655,11 @@ pub fn const_eval_raw_provider<'a, 'tcx>( }; let (res, ecx) = eval_body_and_ecx(tcx, cid, None, key.param_env); - res.and_then(|op| { - let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none(); - if !normalize { - // Sanity check: These must always be a MemPlace - match op.op { - Operand::Indirect(_) => { /* all is good */ }, - Operand::Immediate(_) => bug!("const eval gave us an Immediate"), - } - } - op_to_const(&ecx, op, normalize) + res.and_then(|place| { + Ok(RawConst { + alloc_id: place.to_ptr().expect("we allocated this ptr!").alloc_id, + ty: place.layout.ty + }) }).map_err(|error| { let err = error_to_const_error(&ecx, error); // errors in statics are always emitted as fatal errors diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index c5a242f334c..7dd42c66649 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -28,7 +28,7 @@ use rustc_data_structures::fx::{FxHashSet, FxHashMap}; use syntax::ast::Mutability; use super::{ - Pointer, AllocId, Allocation, ConstValue, GlobalId, AllocationExtra, InboundsCheck, + Pointer, AllocId, Allocation, GlobalId, AllocationExtra, InboundsCheck, EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, Machine, AllocMap, MayLeak, ScalarMaybeUndef, ErrorHandled, }; @@ -374,14 +374,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { ErrorHandled::Reported => EvalErrorKind::ReferencedConstant.into(), ErrorHandled::TooGeneric => EvalErrorKind::TooGeneric.into(), } - }).map(|const_val| { - if let ConstValue::ByRef(_, allocation, _) = const_val.val { - // We got tcx memory. Let the machine figure out whether and how to - // turn that into memory with the right pointer tag. - M::adjust_static_allocation(allocation) - } else { - bug!("Matching on non-ByRef static") - } + }).map(|raw_const| { + let allocation = tcx.alloc_map.lock().unwrap_memory(raw_const.alloc_id); + // We got tcx memory. Let the machine figure out whether and how to + // turn that into memory with the right pointer tag. + M::adjust_static_allocation(allocation) }) } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index b7910ad3bce..16f1e487f59 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -536,7 +536,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // Also used e.g. when miri runs into a constant. - pub(super) fn const_value_to_op( + // FIXME: Can we avoid converting with ConstValue and Const? We should be using RawConst. + fn const_value_to_op( &self, val: ConstValue<'tcx>, ) -> EvalResult<'tcx, Operand> { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index fa76eeb2fed..104d66f7bde 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -20,12 +20,10 @@ use rustc::mir; use rustc::ty::{self, Ty}; use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx}; -use rustc::mir::interpret::{ - GlobalId, AllocId, Allocation, Scalar, EvalResult, Pointer, PointerArithmetic -}; use super::{ + GlobalId, AllocId, Allocation, Scalar, EvalResult, Pointer, PointerArithmetic, EvalContext, Machine, AllocMap, AllocationExtra, - Immediate, ImmTy, ScalarMaybeUndef, Operand, OpTy, MemoryKind + RawConst, Immediate, ImmTy, ScalarMaybeUndef, Operand, OpTy, MemoryKind }; #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] @@ -981,6 +979,19 @@ where Ok(OpTy { op, layout: place.layout }) } + pub fn raw_const_to_mplace( + &self, + raw: RawConst<'tcx>, + ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { + // This must be an allocation in `tcx` + assert!(self.tcx.alloc_map.lock().get(raw.alloc_id).is_some()); + let layout = self.layout_of(raw.ty)?; + Ok(MPlaceTy::from_aligned_ptr( + Pointer::new(raw.alloc_id, Size::ZERO).with_default_tag(), + layout, + )) + } + /// Turn a place with a `dyn Trait` type into a place with the actual dynamic type. /// Also return some more information so drop doesn't have to run the same code twice. pub(super) fn unpack_dyn_trait(&self, mplace: MPlaceTy<'tcx, M::PointerTag>) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 885d70dc430..6b8233c941e 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -309,7 +309,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { eval_promoted(this.tcx, cid, this.mir, this.param_env) })?; trace!("evaluated promoted {:?} to {:?}", promoted, res); - Some((res, source_info.span)) + Some((res.into(), source_info.span)) }, _ => None, } diff --git a/src/test/ui/consts/const-err4.rs b/src/test/ui/consts/const-err4.rs index 0bbc254453c..129177e9a1a 100644 --- a/src/test/ui/consts/const-err4.rs +++ b/src/test/ui/consts/const-err4.rs @@ -16,7 +16,7 @@ union Foo { enum Bar { Boo = [unsafe { Foo { b: () }.a }; 4][3], - //~^ ERROR evaluation of constant value failed + //~^ ERROR it is undefined behavior to use this value } fn main() { diff --git a/src/test/ui/consts/const-err4.stderr b/src/test/ui/consts/const-err4.stderr index bb50f38062e..2f58712be28 100644 --- a/src/test/ui/consts/const-err4.stderr +++ b/src/test/ui/consts/const-err4.stderr @@ -1,8 +1,10 @@ -error[E0080]: evaluation of constant value failed +error[E0080]: it is undefined behavior to use this value --> $DIR/const-err4.rs:18:11 | LL | Boo = [unsafe { Foo { b: () }.a }; 4][3], - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error: aborting due to previous error diff --git a/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.rs b/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.rs index 23e1ab013ef..cc5ddb44016 100644 --- a/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.rs +++ b/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.rs @@ -37,7 +37,7 @@ fn main() { //~^ ERROR it is undefined behavior to use this value const I32_REF_U128_UNION: u128 = unsafe { Nonsense { int_32_ref: &3 }.uint_128 }; - //~^ ERROR any use of this value will cause an error + //~^ ERROR it is undefined behavior to use this value const I32_REF_I8_UNION: i8 = unsafe { Nonsense { int_32_ref: &3 }.int_8 }; //~^ ERROR any use of this value will cause an error @@ -52,7 +52,7 @@ fn main() { //~^ ERROR it is undefined behavior to use this value const I32_REF_I128_UNION: i128 = unsafe { Nonsense { int_32_ref: &3 }.int_128 }; - //~^ ERROR any use of this value will cause an error + //~^ ERROR it is undefined behavior to use this value const I32_REF_F32_UNION: f32 = unsafe { Nonsense { int_32_ref: &3 }.float_32 }; //~^ ERROR any use of this value will cause an error diff --git a/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr b/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr index 0126743eede..60079193ee8 100644 --- a/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr +++ b/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr @@ -40,11 +40,13 @@ LL | const I32_REF_U64_UNION: u64 = unsafe { Nonsense { int_32_ref: &3 }.uin | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior -error: any use of this value will cause an error +error[E0080]: it is undefined behavior to use this value --> $DIR/const-pointer-values-in-various-types.rs:39:5 | LL | const I32_REF_U128_UNION: u128 = unsafe { Nonsense { int_32_ref: &3 }.uint_128 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error: any use of this value will cause an error --> $DIR/const-pointer-values-in-various-types.rs:42:5 @@ -78,11 +80,13 @@ LL | const I32_REF_I64_UNION: i64 = unsafe { Nonsense { int_32_ref: &3 }.int | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior -error: any use of this value will cause an error +error[E0080]: it is undefined behavior to use this value --> $DIR/const-pointer-values-in-various-types.rs:54:5 | LL | const I32_REF_I128_UNION: i128 = unsafe { Nonsense { int_32_ref: &3 }.int_128 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error: any use of this value will cause an error --> $DIR/const-pointer-values-in-various-types.rs:57:5 diff --git a/src/test/ui/consts/const-eval/union-const-eval-field.rs b/src/test/ui/consts/const-eval/union-const-eval-field.rs index c0bfbc17629..2bdad3af889 100644 --- a/src/test/ui/consts/const-eval/union-const-eval-field.rs +++ b/src/test/ui/consts/const-eval/union-const-eval-field.rs @@ -34,7 +34,8 @@ const fn read_field2() -> Field2 { } const fn read_field3() -> Field3 { - const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR any use of this value + const FIELD3: Field3 = unsafe { UNION.field3 }; + //~^ ERROR it is undefined behavior to use this value FIELD3 } diff --git a/src/test/ui/consts/const-eval/union-const-eval-field.stderr b/src/test/ui/consts/const-eval/union-const-eval-field.stderr index 565cd916ffc..035a95a14c8 100644 --- a/src/test/ui/consts/const-eval/union-const-eval-field.stderr +++ b/src/test/ui/consts/const-eval/union-const-eval-field.stderr @@ -1,10 +1,11 @@ -error: any use of this value will cause an error +error[E0080]: it is undefined behavior to use this value --> $DIR/union-const-eval-field.rs:37:5 | -LL | const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR any use of this value - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes +LL | const FIELD3: Field3 = unsafe { UNION.field3 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits | - = note: #[deny(const_err)] on by default + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error: aborting due to previous error +For more information about this error, try `rustc --explain E0080`. diff --git a/src/test/ui/consts/const-eval/union-ice.rs b/src/test/ui/consts/const-eval/union-ice.rs index 0e4f1e09171..6bd63472b21 100644 --- a/src/test/ui/consts/const-eval/union-ice.rs +++ b/src/test/ui/consts/const-eval/union-ice.rs @@ -20,7 +20,7 @@ union DummyUnion { const UNION: DummyUnion = DummyUnion { field1: 1065353216 }; -const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR will cause an error +const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR it is undefined behavior to use this value const FIELD_PATH: Struct = Struct { //~ ERROR it is undefined behavior to use this value a: 42, diff --git a/src/test/ui/consts/const-eval/union-ice.stderr b/src/test/ui/consts/const-eval/union-ice.stderr index 98c2c1472aa..a6b49e0cda3 100644 --- a/src/test/ui/consts/const-eval/union-ice.stderr +++ b/src/test/ui/consts/const-eval/union-ice.stderr @@ -1,10 +1,10 @@ -error: any use of this value will cause an error +error[E0080]: it is undefined behavior to use this value --> $DIR/union-ice.rs:23:1 | -LL | const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR will cause an error - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes +LL | const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR it is undefined behavior to use this value + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits | - = note: #[deny(const_err)] on by default + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value --> $DIR/union-ice.rs:25:1 From ba82f54b04f429d29b0d7c5aebbae2addd5669fd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Nov 2018 16:16:27 +0100 Subject: [PATCH 2/4] use RawConst in miri --- src/librustc_mir/const_eval.rs | 14 +++++++---- src/librustc_mir/interpret/eval_context.rs | 14 +++++++---- src/librustc_mir/interpret/operand.rs | 28 ++++++---------------- src/librustc_mir/interpret/place.rs | 10 ++------ src/librustc_mir/transform/const_prop.rs | 6 +++-- 5 files changed, 32 insertions(+), 40 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 4e727de5358..3b32fe21adf 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -99,8 +99,7 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env) } -// FIXME: This thing is a bad hack. We should get rid of it. Ideally constants are always -// in an allocation. +// FIXME: These two conversion functions are bad hacks. We should just always use allocations. pub fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, op: OpTy<'tcx>, @@ -146,6 +145,13 @@ pub fn op_to_const<'tcx>( }; Ok(ty::Const::from_const_value(ecx.tcx.tcx, val, op.layout.ty)) } +pub fn const_to_op<'tcx>( + ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, + cnst: &ty::Const<'tcx>, +) -> EvalResult<'tcx, OpTy<'tcx>> { + let op = ecx.const_value_to_op(cnst.val)?; + Ok(OpTy { op, layout: ecx.layout_of(cnst.ty)? }) +} fn eval_body_and_ecx<'a, 'mir, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -496,7 +502,7 @@ pub fn const_field<'a, 'tcx>( let ecx = mk_eval_cx(tcx, instance, param_env).unwrap(); let result = (|| { // get the operand again - let op = ecx.const_to_op(value)?; + let op = const_to_op(&ecx, value)?; // downcast let down = match variant { None => op, @@ -523,7 +529,7 @@ pub fn const_variant_index<'a, 'tcx>( ) -> EvalResult<'tcx, VariantIdx> { trace!("const_variant_index: {:?}, {:?}", instance, val); let ecx = mk_eval_cx(tcx, instance, param_env).unwrap(); - let op = ecx.const_to_op(val)?; + let op = const_to_op(&ecx, val)?; Ok(ecx.read_discriminant(op)?.1) } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index ce7269d1e78..1d7eae50e71 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -588,18 +588,22 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc Ok(()) } - pub fn const_eval(&self, gid: GlobalId<'tcx>) -> EvalResult<'tcx, &'tcx ty::Const<'tcx>> { + pub fn const_eval_raw( + &self, + gid: GlobalId<'tcx>, + ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { let param_env = if self.tcx.is_static(gid.instance.def_id()).is_some() { ty::ParamEnv::reveal_all() } else { self.param_env }; - self.tcx.const_eval(param_env.and(gid)).map_err(|err| { + let val = self.tcx.const_eval_raw(param_env.and(gid)).map_err(|err| { match err { - ErrorHandled::Reported => EvalErrorKind::ReferencedConstant.into(), - ErrorHandled::TooGeneric => EvalErrorKind::TooGeneric.into(), + ErrorHandled::Reported => EvalErrorKind::ReferencedConstant, + ErrorHandled::TooGeneric => EvalErrorKind::TooGeneric, } - }) + })?; + self.raw_const_to_mplace(val) } pub fn dump_place(&self, place: Place) { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 16f1e487f59..5d993cfee08 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -13,7 +13,7 @@ use std::convert::TryInto; -use rustc::{mir, ty}; +use rustc::mir; use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx}; use rustc::mir::interpret::{ @@ -535,9 +535,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Also used e.g. when miri runs into a constant. - // FIXME: Can we avoid converting with ConstValue and Const? We should be using RawConst. - fn const_value_to_op( + // Used when miri runs into a constant, and by CTFE. + // FIXME: CTFE should use allocations, then we can make this private (embed it into + // `eval_operand`, ideally). + pub(crate) fn const_value_to_op( &self, val: ConstValue<'tcx>, ) -> EvalResult<'tcx, Operand> { @@ -545,10 +546,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> match val { ConstValue::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; - self.global_to_op(GlobalId { + Ok(*OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None, - }) + })?)) } ConstValue::ByRef(id, alloc, offset) => { // We rely on mutability being set correctly in that allocation to prevent writes @@ -566,21 +567,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Ok(Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag()), } } - pub fn const_to_op( - &self, - cnst: &ty::Const<'tcx>, - ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { - let op = self.const_value_to_op(cnst.val)?; - Ok(OpTy { op, layout: self.layout_of(cnst.ty)? }) - } - - pub(super) fn global_to_op( - &self, - gid: GlobalId<'tcx> - ) -> EvalResult<'tcx, Operand> { - let cv = self.const_eval(gid)?; - self.const_value_to_op(cv.val) - } /// Read discriminant, return the runtime value as well as the variant index. pub fn read_discriminant( diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 104d66f7bde..9f248d46350 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -553,16 +553,10 @@ where Ok(match *mir_place { Promoted(ref promoted) => { let instance = self.frame().instance; - let op = self.global_to_op(GlobalId { + self.const_eval_raw(GlobalId { instance, promoted: Some(promoted.0), - })?; - let mplace = op.to_mem_place(); // these are always in memory - let ty = self.monomorphize(promoted.1, self.substs()); - MPlaceTy { - mplace, - layout: self.layout_of(ty)?, - } + })? } Static(ref static_) => { diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 6b8233c941e..661ca4773b4 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -29,7 +29,9 @@ use rustc::ty::layout::{ }; use interpret::{self, EvalContext, ScalarMaybeUndef, Immediate, OpTy, MemoryKind}; -use const_eval::{CompileTimeInterpreter, error_to_const_error, eval_promoted, mk_borrowck_eval_cx}; +use const_eval::{ + CompileTimeInterpreter, const_to_op, error_to_const_error, eval_promoted, mk_borrowck_eval_cx +}; use transform::{MirPass, MirSource}; pub struct ConstProp; @@ -262,7 +264,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.const_to_op(c.literal) { + match const_to_op(&self.ecx, c.literal) { Ok(op) => { Some((op, c.span)) }, From c462e44c13ce33359dc50dbeb4f60ef552acd438 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Nov 2018 13:51:29 +0100 Subject: [PATCH 3/4] we now do proper validation on scalars --- src/test/ui/consts/const-err4.stderr | 2 +- .../const-pointer-values-in-various-types.stderr | 4 ++-- src/test/ui/consts/const-eval/ub-enum.rs | 10 +++++----- src/test/ui/consts/const-eval/ub-enum.stderr | 8 ++++---- .../ui/consts/const-eval/union-const-eval-field.stderr | 2 +- src/test/ui/consts/const-eval/union-ice.stderr | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/test/ui/consts/const-err4.stderr b/src/test/ui/consts/const-err4.stderr index 2f58712be28..38a8f75a5c2 100644 --- a/src/test/ui/consts/const-err4.stderr +++ b/src/test/ui/consts/const-err4.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const-err4.rs:18:11 | LL | Boo = [unsafe { Foo { b: () }.a }; 4][3], - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior diff --git a/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr b/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr index 60079193ee8..786338222e3 100644 --- a/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr +++ b/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr @@ -44,7 +44,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const-pointer-values-in-various-types.rs:39:5 | LL | const I32_REF_U128_UNION: u128 = unsafe { Nonsense { int_32_ref: &3 }.uint_128 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior @@ -84,7 +84,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const-pointer-values-in-various-types.rs:54:5 | LL | const I32_REF_I128_UNION: i128 = unsafe { Nonsense { int_32_ref: &3 }.int_128 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior diff --git a/src/test/ui/consts/const-eval/ub-enum.rs b/src/test/ui/consts/const-eval/ub-enum.rs index 89b44946441..2bf85e25a21 100644 --- a/src/test/ui/consts/const-eval/ub-enum.rs +++ b/src/test/ui/consts/const-eval/ub-enum.rs @@ -16,12 +16,12 @@ enum Enum { A = 0, } union TransmuteEnum { - a: &'static u8, - out: Enum, + in1: &'static u8, + out1: Enum, } // A pointer is guaranteed non-null -const BAD_ENUM: Enum = unsafe { TransmuteEnum { a: &1 }.out }; +const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; //~^ ERROR is undefined behavior // (Potentially) invalid enum discriminant @@ -48,8 +48,8 @@ const BAD_ENUM3: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; const BAD_ENUM4: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; //~^ ERROR is undefined behavior -// Undef enum discriminant. In an arry to avoid `Scalar` layout. -const BAD_ENUM_UNDEF: [Enum2; 2] = [unsafe { TransmuteEnum2 { in3: () }.out1 }; 2]; +// Undef enum discriminant. +const BAD_ENUM_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; //~^ ERROR is undefined behavior // Pointer value in an enum with a niche that is not just 0. diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 5aae3a2f351..509faaa46f8 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -1,8 +1,8 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:24:1 | -LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { a: &1 }.out }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant +LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior @@ -33,8 +33,8 @@ LL | const BAD_ENUM4: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:52:1 | -LL | const BAD_ENUM_UNDEF: [Enum2; 2] = [unsafe { TransmuteEnum2 { in3: () }.out1 }; 2]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes at [0], but expected a valid enum discriminant +LL | const BAD_ENUM_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior diff --git a/src/test/ui/consts/const-eval/union-const-eval-field.stderr b/src/test/ui/consts/const-eval/union-const-eval-field.stderr index 035a95a14c8..ca7cf2b028c 100644 --- a/src/test/ui/consts/const-eval/union-const-eval-field.stderr +++ b/src/test/ui/consts/const-eval/union-const-eval-field.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/union-const-eval-field.rs:37:5 | LL | const FIELD3: Field3 = unsafe { UNION.field3 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior diff --git a/src/test/ui/consts/const-eval/union-ice.stderr b/src/test/ui/consts/const-eval/union-ice.stderr index a6b49e0cda3..7cadef24617 100644 --- a/src/test/ui/consts/const-eval/union-ice.stderr +++ b/src/test/ui/consts/const-eval/union-ice.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/union-ice.rs:23:1 | LL | const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR it is undefined behavior to use this value - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior From 612febcc4bec60dba58472269c2cf30860315a3d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 17 Nov 2018 15:45:09 +0100 Subject: [PATCH 4/4] explain why we can use raw --- src/librustc_mir/interpret/eval_context.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 1d7eae50e71..59083413582 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -597,6 +597,10 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc } else { self.param_env }; + // We use `const_eval_raw` here, and get an unvalidated result. That is okay: + // Our result will later be validated anyway, and there seems no good reason + // to have to fail early here. This is also more consistent with + // `Memory::get_static_alloc` which has to use `const_eval_raw` to avoid cycles. let val = self.tcx.const_eval_raw(param_env.and(gid)).map_err(|err| { match err { ErrorHandled::Reported => EvalErrorKind::ReferencedConstant,