From 235a6b7d065a2fc55ceee323e85b9309b16e84bf Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 12:32:22 +0100 Subject: [PATCH 01/11] Expose const -> op functions that don't allow violiting const eval invariants --- src/librustc_mir/const_eval.rs | 4 +- src/librustc_mir/interpret/operand.rs | 77 +++++++++++------------- src/librustc_mir/transform/const_prop.rs | 2 +- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 7be7f4b439289..a4b2d6d36878d 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -476,7 +476,7 @@ pub fn const_field<'a, 'tcx>( let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); let result = (|| { // get the operand again - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(value), value.ty)?; + let op = ecx.const_to_op(value, None)?; // downcast let down = match variant { None => op, @@ -502,7 +502,7 @@ pub fn const_variant_index<'a, 'tcx>( ) -> EvalResult<'tcx, VariantIdx> { trace!("const_variant_index: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(val), val.ty)?; + let op = ecx.const_to_op(val, None)?; Ok(ecx.read_discriminant(op)?.1) } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 7da907028eebf..4f34ffc128e69 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -12,7 +12,7 @@ use rustc::mir::interpret::{ EvalResult, EvalErrorKind, }; use super::{ - EvalContext, Machine, AllocMap, Allocation, AllocationExtra, + EvalContext, Machine, MemPlace, MPlaceTy, PlaceTy, Place, MemoryKind, }; pub use rustc::mir::interpret::ScalarMaybeUndef; @@ -545,14 +545,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Move(ref place) => self.eval_place_to_op(place, layout)?, - Constant(ref constant) => { - let layout = from_known_layout(layout, || { - let ty = self.monomorphize(mir_op.ty(self.mir(), *self.tcx))?; - self.layout_of(ty) - })?; - let op = self.const_value_to_op(*constant.literal)?; - OpTy { op, layout } - } + Constant(ref constant) => self.lazy_const_to_op(*constant.literal, layout)?, }; trace!("{:?}: {:?}", mir_op, *op); Ok(op) @@ -568,38 +561,55 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. - fn const_value_to_op( + // Used when Miri runs into a constant, and by CTFE. + pub fn lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, - ) -> EvalResult<'tcx, Operand> { - trace!("const_value_to_op: {:?}", val); - let val = match val { + layout: Option>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + trace!("const_to_op: {:?}", val); + match val { ty::LazyConst::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; - return Ok(*OpTy::from(self.const_eval_raw(GlobalId { + return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None, })?)); }, - ty::LazyConst::Evaluated(c) => c, - }; - match val.val { + ty::LazyConst::Evaluated(c) => self.const_to_op(c, layout), + } + } + + // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + pub fn const_to_op( + &self, + val: ty::Const<'tcx>, + layout: Option>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + let layout = from_known_layout(layout, || { + let ty = self.monomorphize(val.ty)?; + self.layout_of(ty) + })?; + let op = match val.val { ConstValue::ByRef(id, alloc, offset) => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. - Ok(Operand::Indirect( + Operand::Indirect( MemPlace::from_ptr(Pointer::new(id, offset), alloc.align) - ).with_default_tag()) + ).with_default_tag() }, ConstValue::Slice(a, b) => - Ok(Operand::Immediate(Immediate::ScalarPair( + Operand::Immediate(Immediate::ScalarPair( a.into(), Scalar::from_uint(b, self.tcx.data_layout.pointer_size).into(), - )).with_default_tag()), + )).with_default_tag(), ConstValue::Scalar(x) => - Ok(Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag()), - } + Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag(), + }; + Ok(OpTy { + op, + layout, + }) } /// Read discriminant, return the runtime value as well as the variant index. @@ -699,23 +709,4 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } }) } - -} - -impl<'a, 'mir, 'tcx, M> EvalContext<'a, 'mir, 'tcx, M> -where - M: Machine<'a, 'mir, 'tcx, PointerTag=()>, - // FIXME: Working around https://github.com/rust-lang/rust/issues/24159 - M::MemoryMap: AllocMap, Allocation<(), M::AllocExtra>)>, - M::AllocExtra: AllocationExtra<(), M::MemoryExtra>, -{ - // FIXME: CTFE should use allocations, then we can remove this. - pub(crate) fn lazy_const_to_op( - &self, - cnst: ty::LazyConst<'tcx>, - ty: ty::Ty<'tcx>, - ) -> EvalResult<'tcx, OpTy<'tcx>> { - let op = self.const_value_to_op(cnst)?; - Ok(OpTy { op, layout: self.layout_of(ty)? }) - } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7da00c4ea0c36..d69a5130b24d0 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -253,7 +253,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.lazy_const_to_op(*c.literal, c.ty) { + match self.ecx.lazy_const_to_op(*c.literal, None) { Ok(op) => { Some((op, c.span)) }, From bd18cc5708328600a94a02444caf27a5fb6ca20c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 12:36:23 +0100 Subject: [PATCH 02/11] Remove an intermediate value from discriminant reading --- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/step.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4f34ffc128e69..a638c008e760f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -508,7 +508,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Evaluate a place with the goal of reading from it. This lets us sometimes // avoid allocations. - fn eval_place_to_op( + pub(super) fn eval_place_to_op( &self, mir_place: &mir::Place<'tcx>, layout: Option>, diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 97ef2b5fa3485..656c13c16d9ed 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -266,8 +266,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } Discriminant(ref place) => { - let place = self.eval_place(place)?; - let discr_val = self.read_discriminant(self.place_to_op(place)?)?.0; + let op = self.eval_place_to_op(place, None)?; + let discr_val = self.read_discriminant(op)?.0; let size = dest.layout.size; self.write_scalar(Scalar::from_uint(discr_val, size), dest)?; } From b2bf37aec075099a8b58c1a52ebca944f318d530 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 13:27:54 +0100 Subject: [PATCH 03/11] Burn some invariants we keep up into code --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/operand.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index a4b2d6d36878d..9eac125d7a434 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -76,7 +76,7 @@ pub fn op_to_const<'tcx>( _ => false, }; let normalized_op = if normalize { - ecx.try_read_immediate(op)? + Ok(*ecx.read_immediate(op).expect("normalization works on validated constants")) } else { match *op { Operand::Indirect(mplace) => Err(mplace), diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index a638c008e760f..0394ad2b0a65f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -269,7 +269,7 @@ pub(super) fn from_known_layout<'tcx>( impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { /// Try reading an immediate in memory; this is interesting particularly for ScalarPair. /// Returns `None` if the layout does not permit loading this as a value. - pub(super) fn try_read_immediate_from_mplace( + fn try_read_immediate_from_mplace( &self, mplace: MPlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Option>> { @@ -323,7 +323,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> /// Note that for a given layout, this operation will either always fail or always /// succeed! Whether it succeeds depends on whether the layout can be represented /// in a `Immediate`, not on which data is stored there currently. - pub(crate) fn try_read_immediate( + pub(super) fn try_read_immediate( &self, src: OpTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Result, MemPlace>> { From f7c493121d4989895dd9c213ed4e877429229b86 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:29:27 +0100 Subject: [PATCH 04/11] Reuse the `Pointer` type instead of passing reassembling it at many use sites --- src/librustc/ich/impls_ty.rs | 2 +- src/librustc/mir/interpret/value.rs | 5 ++--- src/librustc/ty/structural_impls.rs | 4 ++-- src/librustc_codegen_llvm/consts.rs | 2 +- src/librustc_codegen_ssa/mir/operand.rs | 4 ++-- src/librustc_codegen_ssa/mir/place.rs | 4 ++-- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/hair/pattern/_match.rs | 12 +++++------- src/librustc_mir/interpret/operand.rs | 4 ++-- src/librustc_mir/monomorphize/collector.rs | 2 +- src/librustc_typeck/check/mod.rs | 2 +- 11 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 6f04a68a6ed61..04e03d0d95aa2 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -307,7 +307,7 @@ impl_stable_hash_for!( impl<'tcx> for enum mir::interpret::ConstValue<'tcx> [ mir::interpret::ConstValue ] { Scalar(val), Slice(a, b), - ByRef(id, alloc, offset), + ByRef(ptr, alloc), } ); impl_stable_hash_for!(struct crate::mir::interpret::RawConst<'tcx> { diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 5ec7de4308a13..1e5ba2c176bd2 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -31,9 +31,8 @@ pub enum ConstValue<'tcx> { /// it. Slice(Scalar, u64), - /// An allocation together with an offset into the allocation. - /// Invariant: the `AllocId` matches the allocation. - ByRef(AllocId, &'tcx Allocation, Size), + /// An allocation together with a pointer into the allocation. + ByRef(Pointer, &'tcx Allocation), } #[cfg(target_arch = "x86_64")] diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index d09cfa84a1690..0d1dc4ee2032f 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -499,8 +499,8 @@ impl<'a, 'tcx> Lift<'tcx> for ConstValue<'a> { match *self { ConstValue::Scalar(x) => Some(ConstValue::Scalar(x)), ConstValue::Slice(x, y) => Some(ConstValue::Slice(x, y)), - ConstValue::ByRef(x, alloc, z) => Some(ConstValue::ByRef( - x, alloc.lift_to_tcx(tcx)?, z, + ConstValue::ByRef(ptr, alloc) => Some(ConstValue::ByRef( + ptr, alloc.lift_to_tcx(tcx)?, )), } } diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index ca9e2c87be237..26d3bd9124707 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -71,7 +71,7 @@ pub fn codegen_static_initializer( let static_ = cx.tcx.const_eval(param_env.and(cid))?; let alloc = match static_.val { - ConstValue::ByRef(_, alloc, n) if n.bytes() == 0 => alloc, + ConstValue::ByRef(ptr, alloc) if ptr.offset.bytes() == 0 => alloc, _ => bug!("static const eval returned {:#?}", static_), }; Ok((const_alloc_to_llvm(cx, alloc), alloc)) diff --git a/src/librustc_codegen_ssa/mir/operand.rs b/src/librustc_codegen_ssa/mir/operand.rs index 2c6d968bb032a..3cac1befaf4ef 100644 --- a/src/librustc_codegen_ssa/mir/operand.rs +++ b/src/librustc_codegen_ssa/mir/operand.rs @@ -101,8 +101,8 @@ impl<'a, 'tcx: 'a, V: CodegenObject> OperandRef<'tcx, V> { let b_llval = bx.cx().const_usize(b); OperandValue::Pair(a_llval, b_llval) }, - ConstValue::ByRef(_, alloc, offset) => { - return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, offset))); + ConstValue::ByRef(ptr, alloc) => { + return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, ptr.offset))); }, }; diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index 9d6826d8756b7..1edcbfead2c94 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -417,8 +417,8 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let layout = cx.layout_of(self.monomorphize(&ty)); match bx.tcx().const_eval(param_env.and(cid)) { Ok(val) => match val.val { - mir::interpret::ConstValue::ByRef(_, alloc, offset) => { - bx.cx().from_const_alloc(layout, alloc, offset) + mir::interpret::ConstValue::ByRef(ptr, alloc) => { + bx.cx().from_const_alloc(layout, alloc, ptr.offset) } _ => bug!("promoteds should have an allocation: {:?}", val), }, diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 9eac125d7a434..94b9f0eadd0e7 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -96,7 +96,7 @@ pub fn op_to_const<'tcx>( // FIXME shouldn't it be the case that `mark_static_initialized` has already // interned this? I thought that is the entire point of that `FinishStatic` stuff? let alloc = ecx.tcx.intern_const_alloc(alloc); - ConstValue::ByRef(ptr.alloc_id, alloc, ptr.offset) + ConstValue::ByRef(ptr, alloc) }, Ok(Immediate::Scalar(x)) => ConstValue::Scalar(x.not_undef()?), diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1c7e1aa4d71e0..bf2f0d5d81f47 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -172,7 +172,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const}; use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size}; use rustc::mir::Field; -use rustc::mir::interpret::{ConstValue, Pointer, Scalar}; +use rustc::mir::interpret::{ConstValue, Scalar}; use rustc::util::common::ErrorReported; use syntax::attr::{SignedInt, UnsignedInt}; @@ -214,9 +214,8 @@ impl<'a, 'tcx> LiteralExpander<'a, 'tcx> { match (val, &crty.sty, &rty.sty) { // the easy case, deref a reference (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => ConstValue::ByRef( - p.alloc_id, + p, self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id), - p.offset, ), // unsize array to slice if pattern is array but match value or other patterns are slice (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => { @@ -1428,7 +1427,7 @@ fn slice_pat_covered_by_const<'tcx>( suffix: &[Pattern<'tcx>] ) -> Result { let data: &[u8] = match (const_val.val, &const_val.ty.sty) { - (ConstValue::ByRef(id, alloc, offset), ty::Array(t, n)) => { + (ConstValue::ByRef(ptr, alloc), ty::Array(t, n)) => { if *t != tcx.types.u8 { // FIXME(oli-obk): can't mix const patterns with slice patterns and get // any sort of exhaustiveness/unreachable check yet @@ -1436,7 +1435,6 @@ fn slice_pat_covered_by_const<'tcx>( // are definitely unreachable. return Ok(false); } - let ptr = Pointer::new(id, offset); let n = n.assert_usize(tcx).unwrap(); alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap() }, @@ -1778,8 +1776,8 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( let (opt_ptr, n, ty) = match value.ty.sty { ty::TyKind::Array(t, n) => { match value.val { - ConstValue::ByRef(id, alloc, offset) => ( - Some((Pointer::new(id, offset), alloc)), + ConstValue::ByRef(ptr, alloc) => ( + Some((ptr, alloc)), n.unwrap_usize(cx.tcx), t, ), diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 0394ad2b0a65f..1d6aff749c593 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -591,11 +591,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> self.layout_of(ty) })?; let op = match val.val { - ConstValue::ByRef(id, alloc, offset) => { + ConstValue::ByRef(ptr, alloc) => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. Operand::Indirect( - MemPlace::from_ptr(Pointer::new(id, offset), alloc.align) + MemPlace::from_ptr(ptr, alloc.align) ).with_default_tag() }, ConstValue::Slice(a, b) => diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index a76aa7454cbe4..32c9d5c0c4467 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1257,7 +1257,7 @@ fn collect_const<'a, 'tcx>( ConstValue::Slice(Scalar::Ptr(ptr), _) | ConstValue::Scalar(Scalar::Ptr(ptr)) => collect_miri(tcx, ptr.alloc_id, output), - ConstValue::ByRef(_id, alloc, _offset) => { + ConstValue::ByRef(_ptr, alloc) => { for &((), id) in alloc.relocations.values() { collect_miri(tcx, id, output); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 91e44a1588268..cbc8fde53a263 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1464,7 +1464,7 @@ fn maybe_check_static_with_link_section(tcx: TyCtxt, id: DefId, span: Span) { }; let param_env = ty::ParamEnv::reveal_all(); if let Ok(static_) = tcx.const_eval(param_env.and(cid)) { - let alloc = if let ConstValue::ByRef(_, allocation, _) = static_.val { + let alloc = if let ConstValue::ByRef(_, allocation) = static_.val { allocation } else { bug!("Matching on non-ByRef static") From 7db96a37e7b4b38cdd0354d715cecd56dfdd03b0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:33:54 +0100 Subject: [PATCH 05/11] Reintroduce the invariant comment for clarity --- src/librustc/mir/interpret/value.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 1e5ba2c176bd2..956182fc8b275 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -32,6 +32,7 @@ pub enum ConstValue<'tcx> { Slice(Scalar, u64), /// An allocation together with a pointer into the allocation. + /// Invariant: the pointer's `AllocId` resolves to the allocation. ByRef(Pointer, &'tcx Allocation), } From bee3c670dbf974d097f41447a1214b27bbf9acca Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Feb 2019 14:52:34 +0100 Subject: [PATCH 06/11] Update src/librustc_mir/interpret/operand.rs Co-Authored-By: oli-obk --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 1d6aff749c593..3ec042efb26f8 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -580,7 +580,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + // Used when Miri runs into a constant, and by CTFE. pub fn const_to_op( &self, val: ty::Const<'tcx>, From 525983a2a4ac3029dd9979d924ef444f48a4d7b3 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:05:47 +0100 Subject: [PATCH 07/11] Make validity checking use `MPlaceTy` instead of `OpTy` --- src/librustc_mir/const_eval.rs | 12 +++++------- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/interpret/validity.rs | 23 +++++++++++------------ 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 94b9f0eadd0e7..3c2c0af0bae80 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -523,13 +523,11 @@ fn validate_and_turn_into_const<'a, 'tcx>( let cid = key.value; let ecx = mk_eval_cx(tcx, tcx.def_span(key.value.instance.def_id()), key.param_env); let val = (|| { - 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() { + let mplace = ecx.raw_const_to_mplace(constant)?; + let mut ref_tracking = RefTracking::new(mplace); + while let Some((mplace, path)) = ref_tracking.todo.pop() { ecx.validate_operand( - op, + mplace.into(), path, Some(&mut ref_tracking), true, // const mode @@ -538,7 +536,7 @@ fn validate_and_turn_into_const<'a, 'tcx>( // 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) + op_to_const(&ecx, mplace.into(), normalize) })(); val.map_err(|error| { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index b29e09900f6b1..abc18f1364f87 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -58,7 +58,7 @@ impl<'tcx, Tag> ::std::ops::Deref for PlaceTy<'tcx, Tag> { } /// A MemPlace with its layout. Constructing it is only possible in this module. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] pub struct MPlaceTy<'tcx, Tag=()> { mplace: MemPlace, pub layout: TyLayout<'tcx>, diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 8b97d9ded7449..0163d53f027cf 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -11,7 +11,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, Machine, EvalContext, ValueVisitor, + OpTy, Machine, EvalContext, ValueVisitor, MPlaceTy, }; macro_rules! validation_failure { @@ -74,13 +74,13 @@ pub enum PathElem { } /// State for tracking recursive validation of references -pub struct RefTracking<'tcx, Tag> { - pub seen: FxHashSet<(OpTy<'tcx, Tag>)>, - pub todo: Vec<(OpTy<'tcx, Tag>, Vec)>, +pub struct RefTracking { + pub seen: FxHashSet, + pub todo: Vec<(T, Vec)>, } -impl<'tcx, Tag: Copy+Eq+Hash> RefTracking<'tcx, Tag> { - pub fn new(op: OpTy<'tcx, Tag>) -> Self { +impl<'tcx, T: Copy + Eq + Hash> RefTracking { + pub fn new(op: T) -> Self { let mut ref_tracking = RefTracking { seen: FxHashSet::default(), todo: vec![(op, Vec::new())], @@ -151,7 +151,7 @@ struct ValidityVisitor<'rt, 'a: 'rt, 'mir: 'rt, 'tcx: 'a+'rt+'mir, M: Machine<'a /// starts must not be changed! `visit_fields` and `visit_array` rely on /// this stack discipline. path: Vec, - ref_tracking: Option<&'rt mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&'rt mut RefTracking>>, const_mode: bool, ecx: &'rt EvalContext<'a, 'mir, 'tcx, M>, } @@ -399,16 +399,15 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // before. Proceed recursively even for integer pointers, no // reason to skip them! They are (recursively) valid for some ZST, // but not for others (e.g., `!` is a ZST). - let op = place.into(); - if ref_tracking.seen.insert(op) { - trace!("Recursing below ptr {:#?}", *op); + if ref_tracking.seen.insert(place) { + trace!("Recursing below ptr {:#?}", *place); // We need to clone the path anyway, make sure it gets created // with enough space for the additional `Deref`. let mut new_path = Vec::with_capacity(self.path.len()+1); new_path.clone_from(&self.path); new_path.push(PathElem::Deref); // Remember to come back to this later. - ref_tracking.todo.push((op, new_path)); + ref_tracking.todo.push((place, new_path)); } } } @@ -598,7 +597,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> &self, op: OpTy<'tcx, M::PointerTag>, path: Vec, - ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&mut RefTracking>>, const_mode: bool, ) -> EvalResult<'tcx> { trace!("validate_operand: {:?}, {:?}", *op, op.layout.ty); From 27e438ad948f9e430281e77b0abe3885b64f3bd0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:51:53 +0100 Subject: [PATCH 08/11] Make `may_normalize` explicit in the type system --- src/librustc_mir/const_eval.rs | 72 ++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 3c2c0af0bae80..2f8e3189d12e9 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -21,7 +21,7 @@ use syntax::ast::Mutability; use syntax::source_map::{Span, DUMMY_SP}; use crate::interpret::{self, - PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Operand, Immediate, Scalar, Pointer, + PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Immediate, Scalar, Pointer, RawConst, ConstValue, EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup, Allocation, AllocId, MemoryKind, @@ -62,45 +62,46 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env) } -// FIXME: These two conversion functions are bad hacks. We should just always use allocations. -pub fn op_to_const<'tcx>( +fn mplace_to_const<'tcx>( + ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, + mplace: MPlaceTy<'tcx>, +) -> EvalResult<'tcx, ty::Const<'tcx>> { + let MemPlace { ptr, align, meta } = *mplace; + // extract alloc-offset pair + assert!(meta.is_none()); + let ptr = ptr.to_ptr()?; + let alloc = ecx.memory.get(ptr.alloc_id)?; + assert!(alloc.align >= align); + assert!(alloc.bytes.len() as u64 - ptr.offset.bytes() >= mplace.layout.size.bytes()); + let mut alloc = alloc.clone(); + alloc.align = align; + // FIXME shouldn't it be the case that `mark_static_initialized` has already + // interned this? I thought that is the entire point of that `FinishStatic` stuff? + let alloc = ecx.tcx.intern_const_alloc(alloc); + let val = ConstValue::ByRef(ptr, alloc); + Ok(ty::Const { val, ty: mplace.layout.ty }) +} + +fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, op: OpTy<'tcx>, - may_normalize: bool, ) -> EvalResult<'tcx, ty::Const<'tcx>> { // We do not normalize just any data. Only scalar layout and slices. - let normalize = may_normalize - && match op.layout.abi { - layout::Abi::Scalar(..) => true, - layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), - _ => false, - }; + let normalize = match op.layout.abi { + layout::Abi::Scalar(..) => true, + layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), + _ => false, + }; let normalized_op = if normalize { - Ok(*ecx.read_immediate(op).expect("normalization works on validated constants")) + Err(*ecx.read_immediate(op).expect("normalization works on validated constants")) } else { - match *op { - Operand::Indirect(mplace) => Err(mplace), - Operand::Immediate(val) => Ok(val) - } + op.try_as_mplace() }; let val = match normalized_op { - Err(MemPlace { ptr, align, meta }) => { - // extract alloc-offset pair - assert!(meta.is_none()); - let ptr = ptr.to_ptr()?; - let alloc = ecx.memory.get(ptr.alloc_id)?; - assert!(alloc.align >= align); - assert!(alloc.bytes.len() as u64 - ptr.offset.bytes() >= op.layout.size.bytes()); - let mut alloc = alloc.clone(); - alloc.align = align; - // FIXME shouldn't it be the case that `mark_static_initialized` has already - // interned this? I thought that is the entire point of that `FinishStatic` stuff? - let alloc = ecx.tcx.intern_const_alloc(alloc); - ConstValue::ByRef(ptr, alloc) - }, - Ok(Immediate::Scalar(x)) => + Ok(mplace) => return mplace_to_const(ecx, mplace), + Err(Immediate::Scalar(x)) => ConstValue::Scalar(x.not_undef()?), - Ok(Immediate::ScalarPair(a, b)) => + Err(Immediate::ScalarPair(a, b)) => ConstValue::Slice(a.not_undef()?, b.to_usize(ecx)?), }; Ok(ty::Const { val, ty: op.layout.ty }) @@ -486,7 +487,7 @@ pub fn const_field<'a, 'tcx>( let field = ecx.operand_field(down, field.index() as u64)?; // and finally move back to the const world, always normalizing because // this is not called for statics. - op_to_const(&ecx, field, true) + op_to_const(&ecx, field) })(); result.map_err(|error| { let err = error_to_const_error(&ecx, error); @@ -535,8 +536,11 @@ fn validate_and_turn_into_const<'a, 'tcx>( } // 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, mplace.into(), normalize) + if tcx.is_static(def_id).is_some() || cid.promoted.is_some() { + mplace_to_const(&ecx, mplace) + } else { + op_to_const(&ecx, mplace.into()) + } })(); val.map_err(|error| { From 4fdeb2da747231b03f85786f4ef6ce04d88da864 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:54:02 +0100 Subject: [PATCH 09/11] Add `eval` prefix to clarify what the function does --- src/librustc_mir/interpret/operand.rs | 4 ++-- src/librustc_mir/transform/const_prop.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3ec042efb26f8..08f6667039a34 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -545,7 +545,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Move(ref place) => self.eval_place_to_op(place, layout)?, - Constant(ref constant) => self.lazy_const_to_op(*constant.literal, layout)?, + Constant(ref constant) => self.eval_lazy_const_to_op(*constant.literal, layout)?, }; trace!("{:?}: {:?}", mir_op, *op); Ok(op) @@ -562,7 +562,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // Used when Miri runs into a constant, and by CTFE. - pub fn lazy_const_to_op( + pub fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, layout: Option>, diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d69a5130b24d0..1acc7b6e0c57f 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -253,7 +253,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.lazy_const_to_op(*c.literal, None) { + match self.ecx.eval_lazy_const_to_op(*c.literal, None) { Ok(op) => { Some((op, c.span)) }, From 4b085337bbb3434604f90503538e0211eb3bb8f6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 15:05:14 +0100 Subject: [PATCH 10/11] Update docs and visibilities of const to op methods --- src/librustc_mir/interpret/operand.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 08f6667039a34..311c5c6d10be3 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -561,7 +561,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when Miri runs into a constant, and by CTFE. + // Used when Miri runs into a constant, and by const propagation. pub fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, @@ -580,8 +580,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when Miri runs into a constant, and by CTFE. - pub fn const_to_op( + // Used when the miri-engine runs into a constant. + crate fn const_to_op( &self, val: ty::Const<'tcx>, layout: Option>, From 1fe7eb00944c3c41059e16daa7b401bc8b04447c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 15:16:02 +0100 Subject: [PATCH 11/11] Limit the visibility further and expand on a comment --- src/librustc_mir/interpret/operand.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 311c5c6d10be3..de362a7a96a7f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -562,7 +562,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // Used when Miri runs into a constant, and by const propagation. - pub fn eval_lazy_const_to_op( + crate fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, layout: Option>, @@ -580,7 +580,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when the miri-engine runs into a constant. + // Used when the miri-engine runs into a constant and for extracting information from constants + // in patterns via the `const_eval` module crate fn const_to_op( &self, val: ty::Const<'tcx>,