Weave the alignment through ByRef

rust-lang · Jun 19, 2019 · fb37bf0 · fb37bf0
1 parent 921f0d9
commit fb37bf0
Show file tree

Hide file tree

Showing 12 changed files with 41 additions and 42 deletions.
diff --git 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}, subst::SubstsRef};
+use crate::ty::{Ty, InferConst, ParamConst, layout::{HasDataLayout, Size, Align}, subst::SubstsRef};
 use crate::ty::PlaceholderConst;
 use crate::hir::def_id::DefId;
 
@@ -45,7 +45,9 @@ pub enum ConstValue<'tcx> {
 
     /// An allocation together with a pointer into the allocation.
     /// Invariant: the pointer's `AllocId` resolves to the allocation.
-    ByRef(Pointer, &'tcx Allocation),
+    /// The alignment exists to allow `const_field` to have `ByRef` access to nonprimitive fields
+    /// of packed structs.
+    ByRef(Pointer, Align, &'tcx Allocation),
 
     /// Used in the HIR by using `Unevaluated` everywhere and later normalizing to one of the other
     /// variants when the code is monomorphic enough for that.

diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs
@@ -1335,7 +1335,7 @@ impl<'tcx> TypeFoldable<'tcx> for &'tcx ty::Const<'tcx> {
 impl<'tcx> TypeFoldable<'tcx> for ConstValue<'tcx> {
     fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
         match *self {
-            ConstValue::ByRef(ptr, alloc) => ConstValue::ByRef(ptr, alloc),
+            ConstValue::ByRef(ptr, align, alloc) => ConstValue::ByRef(ptr, align, alloc),
             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
@@ -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};
+use rustc::ty::layout::{HasDataLayout, LayoutOf, self, TyLayout, Size, Align};
 use rustc::mir::interpret::{Scalar, GlobalAlloc, Allocation};
 use rustc_codegen_ssa::mir::place::PlaceRef;
 
@@ -344,19 +344,20 @@ 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> {
         let init = const_alloc_to_llvm(self, alloc);
-        let base_addr = self.static_addr_of(init, layout.align.abi, None);
+        let base_addr = self.static_addr_of(init, align, None);
 
         let llval = unsafe { llvm::LLVMConstInBoundsGEP(
             self.const_bitcast(base_addr, self.type_i8p()),
             &self.const_usize(offset.bytes()),
             1,
         )};
         let llval = self.const_bitcast(llval, self.type_ptr_to(layout.llvm_type(self)));
-        PlaceRef::new_sized(llval, layout, alloc.align)
+        PlaceRef::new_sized(llval, layout, 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
@@ -71,7 +71,9 @@ pub fn codegen_static_initializer(
     let static_ = cx.tcx.const_eval(param_env.and(cid))?;
 
     let alloc = match static_.val {
-        ConstValue::ByRef(ptr, alloc) if ptr.offset.bytes() == 0 => alloc,
+        ConstValue::ByRef(ptr, align, alloc) if ptr.offset.bytes() == 0 && align == alloc.align => {
+            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
@@ -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(ptr, alloc) => {
-                return bx.load_operand(bx.from_const_alloc(layout, alloc, ptr.offset));
+            ConstValue::ByRef(ptr, align, alloc) => {
+                return bx.load_operand(bx.from_const_alloc(layout, align, alloc, ptr.offset));
             },
         };
 

diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs
@@ -424,8 +424,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(ptr, alloc) => {
-                            bx.cx().from_const_alloc(layout, alloc, ptr.offset)
+                        mir::interpret::ConstValue::ByRef(ptr, align, alloc) => {
+                            bx.cx().from_const_alloc(layout, align, alloc, ptr.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
@@ -34,6 +34,7 @@ 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
@@ -99,7 +99,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(ptr, alloc)
+            ConstValue::ByRef(ptr, mplace.align, alloc)
         },
         // see comment on `let try_as_immediate` above
         Err(ImmTy { imm: Immediate::Scalar(x), .. }) => match x {
@@ -113,7 +113,7 @@ fn op_to_const<'tcx>(
                 let mplace = op.to_mem_place();
                 let ptr = mplace.ptr.to_ptr().unwrap();
                 let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
-                ConstValue::ByRef(ptr, alloc)
+                ConstValue::ByRef(ptr, mplace.align, alloc)
             },
         },
         Err(ImmTy { imm: Immediate::ScalarPair(a, b), .. }) => {
@@ -482,10 +482,7 @@ pub fn const_field<'tcx>(
     trace!("const_field: {:?}, {:?}", field, value);
     let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env);
     // get the operand again
-    let mut op = ecx.eval_const_to_op(value, None).unwrap();
-    // Ignore the alignment when accessing the field, since it may be a field of a
-    // packed struct and thus end up causing an alignment error if we read from it.
-    op.force_unaligned_access();
+    let op = ecx.eval_const_to_op(value, None).unwrap();
     // downcast
     let down = match variant {
         None => op,
@@ -544,7 +541,11 @@ fn validate_and_turn_into_const<'tcx>(
         if tcx.is_static(def_id) || cid.promoted.is_some() {
             let ptr = mplace.ptr.to_ptr()?;
             Ok(tcx.mk_const(ty::Const {
-                val: ConstValue::ByRef(ptr, ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id)),
+                val: ConstValue::ByRef(
+                    ptr,
+                    mplace.align,
+                    ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id),
+                ),
                 ty: mplace.layout.ty,
             }))
         } else {

diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs
@@ -215,10 +215,15 @@ impl LiteralExpander<'tcx> {
         debug!("fold_const_value_deref {:?} {:?} {:?}", val, rty, crty);
         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,
-                self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id),
-            ),
+            (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => {
+                let alloc = self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id);
+                ConstValue::ByRef(
+                    p,
+                    // FIXME(oli-obk): this should be the type's layout
+                    alloc.align,
+                    alloc,
+                )
+            },
             // 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)) => {
                 assert_eq!(t, u);
@@ -1431,7 +1436,7 @@ fn slice_pat_covered_by_const<'tcx>(
     suffix: &[Pattern<'tcx>],
 ) -> Result<bool, ErrorReported> {
     let data: &[u8] = match (const_val.val, &const_val.ty.sty) {
-        (ConstValue::ByRef(ptr, alloc), ty::Array(t, n)) => {
+        (ConstValue::ByRef(ptr, _, alloc), ty::Array(t, n)) => {
             assert_eq!(*t, tcx.types.u8);
             let n = n.assert_usize(tcx).unwrap();
             alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap()
@@ -1753,7 +1758,7 @@ fn specialize<'p, 'a: 'p, 'tcx>(
                     let (alloc, offset, n, ty) = match value.ty.sty {
                         ty::Array(t, n) => {
                             match value.val {
-                                ConstValue::ByRef(ptr, alloc) => (
+                                ConstValue::ByRef(ptr, _, alloc) => (
                                     alloc,
                                     ptr.offset,
                                     n.unwrap_usize(cx.tcx),

diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
@@ -5,7 +5,7 @@ use std::convert::TryInto;
 
 use rustc::{mir, ty};
 use rustc::ty::layout::{
-    self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx, Align,
+    self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx,
 };
 
 use rustc::mir::interpret::{
@@ -179,19 +179,6 @@ impl<'tcx, Tag> From<ImmTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
     }
 }
 
-impl<'tcx, Tag> OpTy<'tcx, Tag> {
-    /// This function exists solely for pattern matching. If we pattern match a packed struct with
-    /// an ADT field, the constant representing that field will have lost the information about the
-    /// packedness. We could clone the allocation and adjust the alignment, but that seems wasteful,
-    /// since the alignment is already encoded in the allocation. We know it is alright, because
-    /// validation checked everything before the initial constant entered match checking.
-    pub(crate) fn force_unaligned_access(&mut self) {
-        if let Operand::Indirect(mplace) = &mut self.op {
-            mplace.align = Align::from_bytes(1).unwrap();
-        }
-    }
-}
-
 impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag>
 {
     #[inline]
@@ -551,11 +538,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpretCx<'mir, 'tcx, M> {
             self.layout_of(self.monomorphize(val.ty)?)
         })?;
         let op = match val.val {
-            ConstValue::ByRef(ptr, _alloc) => {
+            ConstValue::ByRef(ptr, align, _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(ptr);
-                Operand::Indirect(MemPlace::from_ptr(ptr, layout.align.abi))
+                Operand::Indirect(MemPlace::from_ptr(ptr, align))
             },
             ConstValue::Scalar(x) =>
                 Operand::Immediate(Immediate::Scalar(tag_scalar(x).into())),

diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs
@@ -1262,7 +1262,7 @@ fn collect_const<'tcx>(
         ConstValue::Scalar(Scalar::Ptr(ptr)) =>
             collect_miri(tcx, ptr.alloc_id, output),
         ConstValue::Slice { data: alloc, start: _, end: _ } |
-        ConstValue::ByRef(_, alloc) => {
+        ConstValue::ByRef(_, _, 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
@@ -1448,7 +1448,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")