comment tweaks, better validation errors, update UI tests

Author: RalfJung

src/librustc_mir/interpret/memory.rs

@@ -20,7 +20,6 @@ use super::{
     Pointer, AllocId, Allocation, GlobalId, AllocationExtra,
     InterpResult, Scalar, InterpError, GlobalAlloc, PointerArithmetic,
     Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg,
-    InterpError::ValidationFailure,
 };
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)]
@@ -260,13 +259,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
     }
 
     /// Check if the given scalar is allowed to do a memory access of given `size`
-    /// and `align`.  On success, returns `None` for zero-sized accesses (where
+    /// and `align`. On success, returns `None` for zero-sized accesses (where
     /// nothing else is left to do) and a `Pointer` to use for the actual access otherwise.
     /// Crucially, if the input is a `Pointer`, we will test it for liveness
     /// *even of* the size is 0.
     ///
     /// Everyone accessing memory based on a `Scalar` should use this method to get the
-    /// `Pointer` they need.  And even if you already have a `Pointer`, call this method
+    /// `Pointer` they need. And even if you already have a `Pointer`, call this method
     /// to make sure it is sufficiently aligned and not dangling.  Not doing that may
     /// cause ICEs.
     pub fn check_ptr_access(
@@ -292,9 +291,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                     return err!(InvalidNullPointerUsage);
                 }
                 if bits % align.bytes() != 0 {
-                    let bits_pow1 = 1 << bits.trailing_zeros();
+                    // The biggest power of two through which `bits` is divisible.
+                    let bits_pow2 = 1 << bits.trailing_zeros();
                     return err!(AlignmentCheckFailed {
-                        has: Align::from_bytes(bits_pow1).unwrap(),
+                        has: Align::from_bytes(bits_pow2).unwrap(),
                         required: align,
                     });
                 }
@@ -308,7 +308,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                 // checks for overflow.
                 let end_ptr = ptr.offset(size, self)?;
                 end_ptr.check_in_alloc(allocation_size, CheckInAllocMsg::MemoryAccessTest)?;
-                // Test align.  Check this last; if both bounds and alignment are violated
+                // 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.
@@ -323,10 +323,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                 }
                 let offset = ptr.offset.bytes();
                 if offset % align.bytes() != 0 {
-                    // The offset os not aligned enough.
-                    let has = offset % align.bytes();
+                    // The biggest power of two through which `offset` is divisible.
+                    let bits_pow2 = 1 << offset.trailing_zeros();
                     return err!(AlignmentCheckFailed {
-                        has: Align::from_bytes(has).unwrap(),
+                        has: Align::from_bytes(bits_pow2).unwrap(),
                         required: align,
                     })
                 }
@@ -520,15 +520,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
             }
             _ => {
                 if let Ok(alloc) = self.get(id) {
-                    return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align));
+                    Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align))
                 }
-                if let AllocCheck::MaybeDead = liveness {
+                else if let AllocCheck::MaybeDead = liveness {
                     // Deallocated pointers are allowed, we should be able to find
                     // them in the map.
-                    self.dead_alloc_map.get(&id).copied().ok_or_else(||
-                        ValidationFailure("allocation missing in dead_alloc_map".to_string())
-                            .into()
-                    )
+                    Ok(*self.dead_alloc_map.get(&id)
+                        .expect("deallocated pointers should all be recorded in `dead_alloc_map`"))
                 } else {
                     err!(DanglingPointerDeref)
                 }

src/librustc_mir/interpret/operand.rs

@@ -228,10 +228,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpretCx<'mir, 'tcx, M> {
 
         let ptr = match self.memory.check_ptr_access(ptr, mplace.layout.size, ptr_align)? {
             Some(ptr) => ptr,
-            None => return Ok(Some(ImmTy {
+            None => return Ok(Some(ImmTy { // zero-sized type
                 imm: Immediate::Scalar(Scalar::zst().into()),
                 layout: mplace.layout,
-            })), // zero-sized access
+            })),
         };
 
         match mplace.layout.abi {

src/librustc_mir/interpret/validity.rs

@@ -366,11 +366,15 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
                     match tail.sty {
                         ty::Dynamic(..) => {
                             let vtable = meta.unwrap();
-                            try_validation!(self.ecx.memory.check_ptr_access(
-                                vtable,
-                                3*self.ecx.tcx.data_layout.pointer_size, // drop, size, align
-                                self.ecx.tcx.data_layout.pointer_align.abi,
-                            ), "dangling or unaligned vtable pointer", self.path);
+                            try_validation!(
+                                self.ecx.memory.check_ptr_access(
+                                    vtable,
+                                    3*self.ecx.tcx.data_layout.pointer_size, // drop, size, align
+                                    self.ecx.tcx.data_layout.pointer_align.abi,
+                                ),
+                                "dangling or unaligned vtable pointer or too small vtable",
+                                self.path
+                            );
                             try_validation!(self.ecx.read_drop_type_from_vtable(vtable),
                                 "invalid drop fn in vtable", self.path);
                             try_validation!(self.ecx.read_size_and_align_from_vtable(vtable),
@@ -397,14 +401,22 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
                 let ptr: Option<_> = match self.ecx.memory.check_ptr_access(ptr, size, align) {
                     Ok(ptr) => ptr,
                     Err(err) => {
-                        info!("{:?} is not aligned to {:?}", ptr, align);
+                        info!(
+                            "{:?} did not pass access check for size {:?}, align {:?}",
+                            ptr, size, align
+                        );
                         match err.kind {
                             InterpError::InvalidNullPointerUsage =>
                                 return validation_failure!("NULL reference", self.path),
                             InterpError::AlignmentCheckFailed { required, has } =>
                                 return validation_failure!(format!("unaligned reference \
                                     (required {} byte alignment but found {})",
                                     required.bytes(), has.bytes()), self.path),
+                            InterpError::ReadBytesAsPointer =>
+                                return validation_failure!(
+                                    "integer pointer in non-ZST reference",
+                                    self.path
+                                ),
                             _ =>
                                 return validation_failure!(
                                     "dangling (not entirely in bounds) reference",

src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr

@@ -42,23 +42,23 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/union-ub-fat-ptr.rs:97:1
    |
 LL | const D: &dyn Trait = unsafe { DynTransmute { repr: DynRepr { ptr: &92, vtable: &3 } }.rust};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid drop fn in vtable
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling or unaligned vtable pointer or too small vtable
    |
    = 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/union-ub-fat-ptr.rs:100:1
    |
 LL | const E: &dyn Trait = unsafe { DynTransmute { repr2: DynRepr2 { ptr: &92, vtable: &3 } }.rust};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid drop fn in vtable
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling or unaligned vtable pointer or too small vtable
    |
    = 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/union-ub-fat-ptr.rs:103:1
    |
 LL | const F: &dyn Trait = unsafe { DynTransmute { bad: BadDynRepr { ptr: &92, vtable: 3 } }.rust};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-pointer vtable in fat pointer
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling or unaligned vtable pointer or too small vtable
    |
    = 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