diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 607bcea7fe801..5381d46972440 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -2,7 +2,7 @@ use std::fmt; use rustc_macros::HashStable; use rustc_apfloat::{Float, ieee::{Double, Single}}; -use crate::ty::{Ty, InferConst, ParamConst, layout::{HasDataLayout, Size, Align}, subst::SubstsRef}; +use crate::ty::{Ty, InferConst, ParamConst, layout::{HasDataLayout, Size}, subst::SubstsRef}; use crate::ty::PlaceholderConst; use crate::hir::def_id::DefId; @@ -45,18 +45,11 @@ pub enum ConstValue<'tcx> { /// A value not represented/representable by `Scalar` or `Slice` ByRef { - /// The alignment exists to allow `const_field` to have `ByRef` access to nonprimitive - /// fields of `repr(packed)` structs. The alignment may be lower than the type of this - /// constant. This permits reads with lower alignment than what the type would normally - /// require. - /// FIXME(RalfJ,oli-obk): The alignment checks are part of miri, but const eval doesn't - /// really need them. Disabling them may be too hard though. - align: Align, - /// Offset into `alloc` - offset: Size, /// The backing memory of the value, may contain more memory than needed for just the value /// in order to share `Allocation`s between values alloc: &'tcx Allocation, + /// Offset into `alloc` + offset: Size, }, /// Used in the HIR by using `Unevaluated` everywhere and later normalizing to one of the other diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index 28b52dcea80f1..649a5244728ba 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -1367,8 +1367,8 @@ impl<'tcx> TypeFoldable<'tcx> for &'tcx ty::Const<'tcx> { impl<'tcx> TypeFoldable<'tcx> for ConstValue<'tcx> { fn super_fold_with>(&self, folder: &mut F) -> Self { match *self { - ConstValue::ByRef { offset, align, alloc } => - ConstValue::ByRef { offset, align, alloc }, + ConstValue::ByRef { alloc, offset } => + ConstValue::ByRef { alloc, offset }, ConstValue::Infer(ic) => ConstValue::Infer(ic.fold_with(folder)), ConstValue::Param(p) => ConstValue::Param(p.fold_with(folder)), ConstValue::Placeholder(p) => ConstValue::Placeholder(p), diff --git a/src/librustc_codegen_llvm/common.rs b/src/librustc_codegen_llvm/common.rs index f00624f3811f1..b0c94a139be08 100644 --- a/src/librustc_codegen_llvm/common.rs +++ b/src/librustc_codegen_llvm/common.rs @@ -11,7 +11,7 @@ use crate::value::Value; use rustc_codegen_ssa::traits::*; use crate::consts::const_alloc_to_llvm; -use rustc::ty::layout::{HasDataLayout, LayoutOf, self, TyLayout, Size, Align}; +use rustc::ty::layout::{HasDataLayout, LayoutOf, self, TyLayout, Size}; use rustc::mir::interpret::{Scalar, GlobalAlloc, Allocation}; use rustc_codegen_ssa::mir::place::PlaceRef; @@ -329,12 +329,12 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> { fn from_const_alloc( &self, layout: TyLayout<'tcx>, - align: Align, alloc: &Allocation, offset: Size, ) -> PlaceRef<'tcx, &'ll Value> { + assert_eq!(alloc.align, layout.align.abi); let init = const_alloc_to_llvm(self, alloc); - let base_addr = self.static_addr_of(init, align, None); + let base_addr = self.static_addr_of(init, alloc.align, None); let llval = unsafe { llvm::LLVMConstInBoundsGEP( self.const_bitcast(base_addr, self.type_i8p()), @@ -342,7 +342,7 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> { 1, )}; let llval = self.const_bitcast(llval, self.type_ptr_to(layout.llvm_type(self))); - PlaceRef::new_sized(llval, layout, align) + PlaceRef::new_sized(llval, layout, alloc.align) } fn const_ptrcast(&self, val: &'ll Value, ty: &'ll Type) -> &'ll Value { diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index e02d14a151e62..0077df3cf5eea 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -72,8 +72,8 @@ pub fn codegen_static_initializer( let alloc = match static_.val { ConstValue::ByRef { - offset, align, alloc, - } if offset.bytes() == 0 && align == alloc.align => { + alloc, offset, + } if offset.bytes() == 0 => { alloc }, _ => bug!("static const eval returned {:#?}", static_), diff --git a/src/librustc_codegen_ssa/mir/operand.rs b/src/librustc_codegen_ssa/mir/operand.rs index 302dcfcc682a3..5e5804b72657b 100644 --- a/src/librustc_codegen_ssa/mir/operand.rs +++ b/src/librustc_codegen_ssa/mir/operand.rs @@ -109,8 +109,8 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { let b_llval = bx.const_usize((end - start) as u64); OperandValue::Pair(a_llval, b_llval) }, - ConstValue::ByRef { offset, align, alloc } => { - return bx.load_operand(bx.from_const_alloc(layout, align, alloc, offset)); + ConstValue::ByRef { alloc, offset } => { + return bx.load_operand(bx.from_const_alloc(layout, alloc, offset)); }, }; diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index b38e58baaf6a4..a632838ba2442 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -466,8 +466,8 @@ impl<'a, 'tcx, 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 { offset, align, alloc } => { - bx.cx().from_const_alloc(layout, align, alloc, offset) + mir::interpret::ConstValue::ByRef { alloc, offset } => { + bx.cx().from_const_alloc(layout, alloc, offset) } _ => bug!("promoteds should have an allocation: {:?}", val), }, diff --git a/src/librustc_codegen_ssa/traits/consts.rs b/src/librustc_codegen_ssa/traits/consts.rs index 248fadfaf0f27..e7ce03f183619 100644 --- a/src/librustc_codegen_ssa/traits/consts.rs +++ b/src/librustc_codegen_ssa/traits/consts.rs @@ -35,7 +35,6 @@ pub trait ConstMethods<'tcx>: BackendTypes { fn from_const_alloc( &self, layout: layout::TyLayout<'tcx>, - align: layout::Align, alloc: &Allocation, offset: layout::Size, ) -> PlaceRef<'tcx, Self::Value>; diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 1b92e4992ffb1..b94294578500f 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -98,7 +98,7 @@ fn op_to_const<'tcx>( Ok(mplace) => { let ptr = mplace.ptr.to_ptr().unwrap(); let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id); - ConstValue::ByRef { offset: ptr.offset, align: mplace.align, alloc } + ConstValue::ByRef { alloc, offset: ptr.offset } }, // see comment on `let try_as_immediate` above Err(ImmTy { imm: Immediate::Scalar(x), .. }) => match x { @@ -112,7 +112,7 @@ fn op_to_const<'tcx>( let mplace = op.assert_mem_place(); let ptr = mplace.ptr.to_ptr().unwrap(); let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id); - ConstValue::ByRef { offset: ptr.offset, align: mplace.align, alloc } + ConstValue::ByRef { alloc, offset: ptr.offset } }, }, Err(ImmTy { imm: Immediate::ScalarPair(a, b), .. }) => { @@ -326,6 +326,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, const STATIC_KIND: Option = None; // no copying of statics allowed + // We do not check for alignment to avoid having to carry an `Align` + // in `ConstValue::ByRef`. + const CHECK_ALIGN: bool = false; + #[inline(always)] fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { false // for now, we don't enforce validity @@ -552,9 +556,8 @@ fn validate_and_turn_into_const<'tcx>( let ptr = mplace.ptr.to_ptr()?; Ok(tcx.mk_const(ty::Const { val: ConstValue::ByRef { - offset: ptr.offset, - align: mplace.align, alloc: ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id), + offset: ptr.offset, }, ty: mplace.layout.ty, })) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 567bac777d265..b1a317ee65f6f 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -218,10 +218,8 @@ impl LiteralExpander<'tcx> { (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => { let alloc = self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id); ConstValue::ByRef { - offset: p.offset, - // FIXME(oli-obk): this should be the type's layout - align: alloc.align, alloc, + offset: p.offset, } }, // unsize array to slice if pattern is array but match value or other patterns are slice diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 78902b1016634..c6686fafd94ee 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -109,6 +109,9 @@ pub trait Machine<'mir, 'tcx>: Sized { /// that is added to the memory so that the work is not done twice. const STATIC_KIND: Option; + /// Whether memory accesses should be alignment-checked. + const CHECK_ALIGN: bool; + /// Whether to enforce the validity invariant fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index a1574cd5935e9..eb98d07f901ba 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -306,11 +306,24 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// /// Most of the time you should use `check_mplace_access`, but when you just have a pointer, /// this method is still appropriate. + #[inline(always)] pub fn check_ptr_access( &self, sptr: Scalar, size: Size, align: Align, + ) -> InterpResult<'tcx, Option>> { + let align = if M::CHECK_ALIGN { Some(align) } else { None }; + self.check_ptr_access_align(sptr, size, align) + } + + /// Like `check_ptr_access`, but *definitely* checks alignment when `align` + /// is `Some` (overriding `M::CHECK_ALIGN`). + pub(super) fn check_ptr_access_align( + &self, + sptr: Scalar, + size: Size, + align: Option, ) -> InterpResult<'tcx, Option>> { fn check_offset_align(offset: u64, align: Align) -> InterpResult<'static> { if offset % align.bytes() == 0 { @@ -338,11 +351,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Ok(bits) => { let bits = bits as u64; // it's ptr-sized assert!(size.bytes() == 0); - // Must be non-NULL and aligned. + // Must be non-NULL. if bits == 0 { throw_unsup!(InvalidNullPointerUsage) } - check_offset_align(bits, align)?; + // Must be aligned. + if let Some(align) = align { + check_offset_align(bits, align)?; + } None } Err(ptr) => { @@ -355,18 +371,20 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { end_ptr.check_in_alloc(allocation_size, CheckInAllocMsg::MemoryAccessTest)?; // Test align. Check this last; if both bounds and alignment are violated // we want the error to be about the bounds. - if alloc_align.bytes() < align.bytes() { - // The allocation itself is not aligned enough. - // FIXME: Alignment check is too strict, depending on the base address that - // got picked we might be aligned even if this check fails. - // We instead have to fall back to converting to an integer and checking - // the "real" alignment. - throw_unsup!(AlignmentCheckFailed { - has: alloc_align, - required: align, - }) + if let Some(align) = align { + if alloc_align.bytes() < align.bytes() { + // The allocation itself is not aligned enough. + // FIXME: Alignment check is too strict, depending on the base address that + // got picked we might be aligned even if this check fails. + // We instead have to fall back to converting to an integer and checking + // the "real" alignment. + throw_unsup!(AlignmentCheckFailed { + has: alloc_align, + required: align, + }); + } + check_offset_align(ptr.offset.bytes(), align)?; } - check_offset_align(ptr.offset.bytes(), align)?; // We can still be zero-sized in this branch, in which case we have to // return `None`. diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index e64a474b4ca71..18a70ae260744 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -548,12 +548,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.layout_of(self.monomorphize(val.ty)?) })?; let op = match val.val { - ConstValue::ByRef { offset, align, alloc } => { + ConstValue::ByRef { alloc, offset } => { let id = self.tcx.alloc_map.lock().create_memory_alloc(alloc); // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen. let ptr = self.tag_static_base_pointer(Pointer::new(id, offset)); - Operand::Indirect(MemPlace::from_ptr(ptr, align)) + Operand::Indirect(MemPlace::from_ptr(ptr, layout.align.abi)) }, ConstValue::Scalar(x) => Operand::Immediate(Immediate::Scalar(tag_scalar(x).into())), diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 072c9afc50ae0..82d6d7db01c8d 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -398,7 +398,9 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // alignment and size determined by the layout (size will be 0, // alignment should take attributes into account). .unwrap_or_else(|| (layout.size, layout.align.abi)); - let ptr: Option<_> = match self.ecx.memory.check_ptr_access(ptr, size, align) { + let ptr: Option<_> = match + self.ecx.memory.check_ptr_access_align(ptr, size, Some(align)) + { Ok(ptr) => ptr, Err(err) => { info!(