Move alignment checks out of Allocation

Author: oli-obk

src/librustc/mir/interpret/allocation.rs

@@ -15,7 +15,7 @@ use super::{
     truncate,
 };
 
-use ty::layout::{self, Size, Align};
+use ty::layout::{Size, Align};
 use syntax::ast::Mutability;
 use std::iter;
 use mir;
@@ -103,10 +103,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
         size: Size,
-        align: Align,
         check_defined_and_ptr: bool,
     ) -> EvalResult<'tcx, &[u8]> {
-        self.check_align(ptr.into(), align)?;
         self.check_bounds(cx, ptr, size)?;
 
         if check_defined_and_ptr {
@@ -126,14 +124,13 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
     }
 
     #[inline]
-    fn get_bytes(
+    pub fn get_bytes(
         &self,
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
         size: Size,
-        align: Align
     ) -> EvalResult<'tcx, &[u8]> {
-        self.get_bytes_internal(cx, ptr, size, align, true)
+        self.get_bytes_internal(cx, ptr, size, true)
     }
 
     /// It is the caller's responsibility to handle undefined and pointer bytes.
@@ -144,9 +141,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
         size: Size,
-        align: Align
     ) -> EvalResult<'tcx, &[u8]> {
-        self.get_bytes_internal(cx, ptr, size, align, false)
+        self.get_bytes_internal(cx, ptr, size, false)
     }
 
     /// Just calling this already marks everything as defined and removes relocations,
@@ -156,10 +152,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
         size: Size,
-        align: Align,
     ) -> EvalResult<'tcx, &mut [u8]> {
         assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`");
-        self.check_align(ptr.into(), align)?;
         self.check_bounds(cx, ptr, size)?;
 
         self.mark_definedness(ptr, size, true)?;
@@ -201,9 +195,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         size: Size,
         allow_ptr_and_undef: bool,
     ) -> EvalResult<'tcx> {
-        let align = Align::from_bytes(1).unwrap();
         // Check bounds, align and relocations on the edges
-        self.get_bytes_with_undef_and_ptr(cx, ptr, size, align)?;
+        self.get_bytes_with_undef_and_ptr(cx, ptr, size)?;
         // Check undef and ptr
         if !allow_ptr_and_undef {
             self.check_defined(ptr, size)?;
@@ -212,26 +205,13 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         Ok(())
     }
 
-    pub fn read_bytes(
-        &self,
-        cx: &impl HasDataLayout,
-        ptr: Pointer<Tag>,
-        size: Size,
-    ) -> EvalResult<'tcx, &[u8]> {
-        let align = Align::from_bytes(1).unwrap();
-        self.get_bytes(cx, ptr, size, align)
-    }
-
     pub fn write_bytes(
         &mut self,
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
         src: &[u8],
     ) -> EvalResult<'tcx> {
-        let align = Align::from_bytes(1).unwrap();
-        let bytes = self.get_bytes_mut(
-            cx, ptr, Size::from_bytes(src.len() as u64), align,
-        )?;
+        let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64))?;
         bytes.clone_from_slice(src);
         Ok(())
     }
@@ -243,8 +223,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         val: u8,
         count: Size
     ) -> EvalResult<'tcx> {
-        let align = Align::from_bytes(1).unwrap();
-        let bytes = self.get_bytes_mut(cx, ptr, count, align)?;
+        let bytes = self.get_bytes_mut(cx, ptr, count)?;
         for b in bytes {
             *b = val;
         }
@@ -256,13 +235,10 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         &self,
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
-        ptr_align: Align,
         size: Size
     ) -> EvalResult<'tcx, ScalarMaybeUndef<Tag>> {
-        // get_bytes_unchecked tests alignment and relocation edges
-        let bytes = self.get_bytes_with_undef_and_ptr(
-            cx, ptr, size, ptr_align.min(int_align(cx, size))
-        )?;
+        // get_bytes_unchecked tests relocation edges
+        let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size)?;
         // Undef check happens *after* we established that the alignment is correct.
         // We must not return Ok() for unaligned pointers!
         if self.check_defined(ptr, size).is_err() {
@@ -293,17 +269,15 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         &self,
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
-        ptr_align: Align
     ) -> EvalResult<'tcx, ScalarMaybeUndef<Tag>> {
-        self.read_scalar(cx, ptr, ptr_align, cx.data_layout().pointer_size)
+        self.read_scalar(cx, ptr, cx.data_layout().pointer_size)
     }
 
     /// Write a *non-ZST* scalar
     pub fn write_scalar(
         &mut self,
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
-        ptr_align: Align,
         val: ScalarMaybeUndef<Tag>,
         type_size: Size,
     ) -> EvalResult<'tcx> {
@@ -327,9 +301,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         };
 
         {
-            // get_bytes_mut checks alignment
             let endian = cx.data_layout().endian;
-            let dst = self.get_bytes_mut(cx, ptr, type_size, ptr_align)?;
+            let dst = self.get_bytes_mut(cx, ptr, type_size)?;
             write_target_uint(endian, dst, bytes).unwrap();
         }
 
@@ -351,31 +324,13 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
         &mut self,
         cx: &impl HasDataLayout,
         ptr: Pointer<Tag>,
-        ptr_align: Align,
         val: ScalarMaybeUndef<Tag>
     ) -> EvalResult<'tcx> {
         let ptr_size = cx.data_layout().pointer_size;
-        self.write_scalar(cx, ptr.into(), ptr_align, val, ptr_size)
+        self.write_scalar(cx, ptr.into(), val, ptr_size)
     }
 }
 
-fn int_align(
-    cx: &impl HasDataLayout,
-    size: Size,
-) -> Align {
-    // We assume pointer-sized integers have the same alignment as pointers.
-    // We also assume signed and unsigned integers of the same size have the same alignment.
-    let ity = match size.bytes() {
-        1 => layout::I8,
-        2 => layout::I16,
-        4 => layout::I32,
-        8 => layout::I64,
-        16 => layout::I128,
-        _ => bug!("bad integer size: {}", size.bytes()),
-    };
-    ity.align(cx).abi
-}
-
 /// Relocations
 impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
     /// Return all relocations overlapping with the given ptr-offset pair.

src/librustc_mir/interpret/memory.rs

@@ -578,7 +578,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
             Ok(&[])
         } else {
             let ptr = ptr.to_ptr()?;
-            self.get(ptr.alloc_id)?.read_bytes(self, ptr, size)
+            self.get(ptr.alloc_id)?.get_bytes(self, ptr, size)
         }
     }
 }
@@ -656,10 +656,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         length: u64,
         nonoverlapping: bool,
     ) -> EvalResult<'tcx> {
+        self.check_align(src, src_align)?;
+        self.check_align(dest, dest_align)?;
         if size.bytes() == 0 {
             // Nothing to do for ZST, other than checking alignment and non-NULLness.
-            self.check_align(src, src_align)?;
-            self.check_align(dest, dest_align)?;
             return Ok(());
         }
         let src = src.to_ptr()?;
@@ -689,12 +689,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
 
         let tcx = self.tcx.tcx;
 
-        // This also checks alignment, and relocation edges on the src.
+        // This checks relocation edges on the src.
         let src_bytes = self.get(src.alloc_id)?
-            .get_bytes_with_undef_and_ptr(&tcx, src, size, src_align)?
+            .get_bytes_with_undef_and_ptr(&tcx, src, size)?
             .as_ptr();
         let dest_bytes = self.get_mut(dest.alloc_id)?
-            .get_bytes_mut(&tcx, dest, size * length, dest_align)?
+            .get_bytes_mut(&tcx, dest, size * length)?
             .as_mut_ptr();
 
         // SAFE: The above indexing would have panicked if there weren't at least `size` bytes

src/librustc_mir/interpret/operand.rs

@@ -275,12 +275,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
             return Ok(Some(Immediate::Scalar(Scalar::zst().into())));
         }
 
+        // check for integer pointers before alignment to report better errors
         let ptr = ptr.to_ptr()?;
+        self.memory.check_align(ptr.into(), ptr_align)?;
         match mplace.layout.abi {
             layout::Abi::Scalar(..) => {
                 let scalar = self.memory
                     .get(ptr.alloc_id)?
-                    .read_scalar(self, ptr, ptr_align, mplace.layout.size)?;
+                    .read_scalar(self, ptr, mplace.layout.size)?;
                 Ok(Some(Immediate::Scalar(scalar)))
             }
             layout::Abi::ScalarPair(ref a, ref b) => {
@@ -289,13 +291,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 let a_ptr = ptr;
                 let b_offset = a_size.align_to(b.align(self).abi);
                 assert!(b_offset.bytes() > 0); // we later use the offset to test which field to use
-                let b_ptr = ptr.offset(b_offset, self)?.into();
+                let b_ptr = ptr.offset(b_offset, self)?;
                 let a_val = self.memory
                     .get(ptr.alloc_id)?
-                    .read_scalar(self, a_ptr, ptr_align, a_size)?;
+                    .read_scalar(self, a_ptr, a_size)?;
+                self.memory.check_align(b_ptr.into(), b.align(self))?;
                 let b_val = self.memory
                     .get(ptr.alloc_id)?
-                    .read_scalar(self, b_ptr, ptr_align, b_size)?;
+                    .read_scalar(self, b_ptr, b_size)?;
                 Ok(Some(Immediate::ScalarPair(a_val, b_val)))
             }
             _ => Ok(None),

src/librustc_mir/interpret/place.rs

@@ -713,11 +713,12 @@ where
 
         // Nothing to do for ZSTs, other than checking alignment
         if dest.layout.is_zst() {
-            self.memory.check_align(ptr, ptr_align)?;
-            return Ok(());
+            return self.memory.check_align(ptr, ptr_align);
         }
 
+        // check for integer pointers before alignment to report better errors
         let ptr = ptr.to_ptr()?;
+        self.memory.check_align(ptr.into(), ptr_align)?;
         let tcx = &*self.tcx;
         // FIXME: We should check that there are dest.layout.size many bytes available in
         // memory.  The code below is not sufficient, with enough padding it might not
@@ -729,9 +730,8 @@ where
                     _ => bug!("write_immediate_to_mplace: invalid Scalar layout: {:#?}",
                             dest.layout)
                 }
-
                 self.memory.get_mut(ptr.alloc_id)?.write_scalar(
-                    tcx, ptr, ptr_align.min(dest.layout.align.abi), scalar, dest.layout.size
+                    tcx, ptr, scalar, dest.layout.size
                 )
             }
             Immediate::ScalarPair(a_val, b_val) => {
@@ -741,20 +741,22 @@ where
                               dest.layout)
                 };
                 let (a_size, b_size) = (a.size(self), b.size(self));
-                let (a_align, b_align) = (a.align(self).abi, b.align(self).abi);
+                let b_align = b.align(self).abi;
                 let b_offset = a_size.align_to(b_align);
                 let b_ptr = ptr.offset(b_offset, self)?;
 
+                self.memory.check_align(b_ptr.into(), ptr_align.min(b_align))?;
+
                 // It is tempting to verify `b_offset` against `layout.fields.offset(1)`,
                 // but that does not work: We could be a newtype around a pair, then the
                 // fields do not match the `ScalarPair` components.
 
                 self.memory
                     .get_mut(ptr.alloc_id)?
-                    .write_scalar(tcx, ptr, ptr_align.min(a_align), a_val, a_size)?;
+                    .write_scalar(tcx, ptr, a_val, a_size)?;
                 self.memory
                     .get_mut(b_ptr.alloc_id)?
-                    .write_scalar(tcx, b_ptr, ptr_align.min(b_align), b_val, b_size)
+                    .write_scalar(tcx, b_ptr, b_val, b_size)
             }
         }
     }

src/librustc_mir/interpret/terminator.rs

@@ -401,13 +401,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
             // cannot use the shim here, because that will only result in infinite recursion
             ty::InstanceDef::Virtual(_, idx) => {
                 let ptr_size = self.pointer_size();
-                let ptr_align = self.tcx.data_layout.pointer_align.abi;
                 let ptr = self.deref_operand(args[0])?;
                 let vtable = ptr.vtable()?;
+                self.memory.check_align(vtable.into(), self.tcx.data_layout.pointer_align.abi)?;
                 let fn_ptr = self.memory.get(vtable.alloc_id)?.read_ptr_sized(
                     self,
                     vtable.offset(ptr_size * (idx as u64 + 3), self)?,
-                    ptr_align
                 )?.to_ptr()?;
                 let instance = self.memory.get_fn(fn_ptr)?;
 

src/librustc_mir/interpret/traits.rs

@@ -61,16 +61,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         let drop = self.memory.create_fn_alloc(drop).with_default_tag();
         self.memory
             .get_mut(vtable.alloc_id)?
-            .write_ptr_sized(tcx, vtable, ptr_align, Scalar::Ptr(drop).into())?;
+            .write_ptr_sized(tcx, vtable, Scalar::Ptr(drop).into())?;
 
         let size_ptr = vtable.offset(ptr_size, self)?;
         self.memory
             .get_mut(size_ptr.alloc_id)?
-            .write_ptr_sized(tcx, size_ptr, ptr_align, Scalar::from_uint(size, ptr_size).into())?;
+            .write_ptr_sized(tcx, size_ptr, Scalar::from_uint(size, ptr_size).into())?;
         let align_ptr = vtable.offset(ptr_size * 2, self)?;
         self.memory
             .get_mut(align_ptr.alloc_id)?
-            .write_ptr_sized(tcx, align_ptr, ptr_align, Scalar::from_uint(align, ptr_size).into())?;
+            .write_ptr_sized(tcx, align_ptr, Scalar::from_uint(align, ptr_size).into())?;
 
         for (i, method) in methods.iter().enumerate() {
             if let Some((def_id, substs)) = *method {
@@ -79,7 +79,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 let method_ptr = vtable.offset(ptr_size * (3 + i as u64), self)?;
                 self.memory
                     .get_mut(method_ptr.alloc_id)?
-                    .write_ptr_sized(tcx, method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?;
+                    .write_ptr_sized(tcx, method_ptr, Scalar::Ptr(fn_ptr).into())?;
             }
         }
 
@@ -95,10 +95,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         vtable: Pointer<M::PointerTag>,
     ) -> EvalResult<'tcx, (ty::Instance<'tcx>, ty::Ty<'tcx>)> {
         // we don't care about the pointee type, we just want a pointer
-        let pointer_align = self.tcx.data_layout.pointer_align.abi;
+        self.memory.check_align(vtable.into(), self.tcx.data_layout.pointer_align.abi)?;
         let drop_fn = self.memory
             .get(vtable.alloc_id)?
-            .read_ptr_sized(self, vtable, pointer_align)?
+            .read_ptr_sized(self, vtable)?
             .to_ptr()?;
         let drop_instance = self.memory.get_fn(drop_fn)?;
         trace!("Found drop fn: {:?}", drop_instance);
@@ -114,14 +114,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         vtable: Pointer<M::PointerTag>,
     ) -> EvalResult<'tcx, (Size, Align)> {
         let pointer_size = self.pointer_size();
-        let pointer_align = self.tcx.data_layout.pointer_align.abi;
+        self.memory.check_align(vtable.into(), self.tcx.data_layout.pointer_align.abi)?;
         let alloc = self.memory.get(vtable.alloc_id)?;
-        let size = alloc.read_ptr_sized(self, vtable.offset(pointer_size, self)?, pointer_align)?
+        let size = alloc.read_ptr_sized(self, vtable.offset(pointer_size, self)?)?
             .to_bits(pointer_size)? as u64;
         let align = alloc.read_ptr_sized(
             self,
             vtable.offset(pointer_size * 2, self)?,
-            pointer_align
         )?.to_bits(pointer_size)? as u64;
         Ok((Size::from_bytes(size), Align::from_bytes(align).unwrap()))
     }