miri: validity checks alignment even when machine otherwise does not

Author: RalfJung

Diff for: 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<M::PointerTag>,
         size: Size,
         align: Align,
+    ) -> InterpResult<'tcx, Option<Pointer<M::PointerTag>>> {
+        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<M::PointerTag>,
+        size: Size,
+        align: Option<Align>,
     ) -> InterpResult<'tcx, Option<Pointer<M::PointerTag>>> {
         fn check_offset_align(offset: u64, align: Align) -> InterpResult<'static> {
             if offset % align.bytes() == 0 {
@@ -343,7 +356,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                     throw_unsup!(InvalidNullPointerUsage)
                 }
                 // Must be aligned.
-                if M::CHECK_ALIGN {
+                if let Some(align) = align {
                     check_offset_align(bits, align)?;
                 }
                 None
@@ -358,7 +371,7 @@ 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 M::CHECK_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

Diff for: 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!(

Diff for: src/test/ui/consts/const-eval/ub-ref.rs

@@ -3,7 +3,8 @@
 
 use std::mem;
 
-const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) }; // Ok (CTFE does not check alignment)
+const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) };
+//~^ ERROR it is undefined behavior to use this value
 
 const NULL: &u16 = unsafe { mem::transmute(0usize) };
 //~^ ERROR it is undefined behavior to use this value

Diff for: src/test/ui/consts/const-eval/ub-ref.stderr

@@ -1,35 +1,43 @@
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref.rs:8:1
+  --> $DIR/ub-ref.rs:6:1
+   |
+LL | const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered unaligned reference (required 2 byte alignment but found 1)
+   |
+   = 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/ub-ref.rs:9:1
    |
 LL | const NULL: &u16 = unsafe { mem::transmute(0usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1
    |
    = 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/ub-ref.rs:11:1
+  --> $DIR/ub-ref.rs:12:1
    |
 LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, 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
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref.rs:14:1
+  --> $DIR/ub-ref.rs:15:1
    |
 LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at .<deref>, but expected 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
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref.rs:17:1
+  --> $DIR/ub-ref.rs:18:1
    |
 LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (created from integer)
    |
    = 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 4 previous errors
+error: aborting due to 5 previous errors
 
 For more information about this error, try `rustc --explain E0080`.