more precise error for 'based on misaligned pointer' case

Author: RalfJung

compiler/rustc_const_eval/messages.ftl

@@ -1,11 +1,15 @@
 const_eval_address_space_full =
     there are no more free addresses in the address space
-const_eval_align_check_failed = accessing memory with alignment {$has}, but alignment {$required} is required
+
 const_eval_align_offset_invalid_align =
     `align_offset` called with non-power-of-two align: {$target_align}
 
 const_eval_alignment_check_failed =
-    accessing memory with alignment {$has}, but alignment {$required} is required
+    {$msg ->
+     [AccessedPtr] accessing memory
+     *[other] accessing memory based on pointer
+    } with alignment {$has}, but alignment {$required} is required
+
 const_eval_already_reported =
     an error has already been reported elsewhere (this should not usually be printed)
 const_eval_assume_false =

compiler/rustc_const_eval/src/errors.rs

@@ -390,15 +390,6 @@ pub struct LiveDrop<'tcx> {
     pub dropped_at: Option<Span>,
 }
 
-#[derive(LintDiagnostic)]
-#[diag(const_eval_align_check_failed)]
-pub struct AlignmentCheckFailed {
-    pub has: u64,
-    pub required: u64,
-    #[subdiagnostic]
-    pub frames: Vec<FrameNote>,
-}
-
 #[derive(Diagnostic)]
 #[diag(const_eval_error, code = "E0080")]
 pub struct ConstEvalError {
@@ -568,9 +559,10 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
 
                 builder.set_arg("bad_pointer_message", bad_pointer_message(msg, handler));
             }
-            AlignmentCheckFailed(Misalignment { required, has }) => {
+            AlignmentCheckFailed(Misalignment { required, has }, msg) => {
                 builder.set_arg("required", required.bytes());
                 builder.set_arg("has", has.bytes());
+                builder.set_arg("msg", format!("{msg:?}"));
             }
             WriteToReadOnly(alloc) | DerefFunctionPointer(alloc) | DerefVTablePointer(alloc) => {
                 builder.set_arg("allocation", alloc);

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -21,8 +21,8 @@ use rustc_target::abi::{Align, HasDataLayout, Size};
 use crate::fluent_generated as fluent;
 
 use super::{
-    alloc_range, AllocBytes, AllocId, AllocMap, AllocRange, Allocation, CheckInAllocMsg,
-    GlobalAlloc, InterpCx, InterpResult, Machine, MayLeak, Misalignment, Pointer,
+    alloc_range, AllocBytes, AllocId, AllocMap, AllocRange, Allocation, CheckAlignMsg,
+    CheckInAllocMsg, GlobalAlloc, InterpCx, InterpResult, Machine, MayLeak, Misalignment, Pointer,
     PointerArithmetic, Provenance, Scalar,
 };
 
@@ -425,7 +425,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 }
                 // Must be aligned.
                 if M::enforce_alignment(self) && align.bytes() > 1 {
-                    self.check_misalign(Self::offset_misalignment(addr, align))?;
+                    self.check_misalign(
+                        Self::offset_misalignment(addr, align),
+                        CheckAlignMsg::AccessedPtr,
+                    )?;
                 }
                 None
             }
@@ -449,7 +452,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // Test align. Check this last; if both bounds and alignment are violated
                 // we want the error to be about the bounds.
                 if M::enforce_alignment(self) && align.bytes() > 1 {
-                    self.check_misalign(self.alloc_misalignment(ptr, offset, align, alloc_align))?;
+                    self.check_misalign(
+                        self.alloc_misalignment(ptr, offset, align, alloc_align),
+                        CheckAlignMsg::AccessedPtr,
+                    )?;
                 }
 
                 // We can still be zero-sized in this branch, in which case we have to
@@ -460,9 +466,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     }
 
     #[inline(always)]
-    pub(super) fn check_misalign(&self, misaligned: Option<Misalignment>) -> InterpResult<'tcx> {
+    pub(super) fn check_misalign(
+        &self,
+        misaligned: Option<Misalignment>,
+        msg: CheckAlignMsg,
+    ) -> InterpResult<'tcx> {
         if let Some(misaligned) = misaligned {
-            throw_ub!(AlignmentCheckFailed(misaligned))
+            throw_ub!(AlignmentCheckFailed(misaligned, msg))
         }
         Ok(())
     }

compiler/rustc_const_eval/src/interpret/place.rs

@@ -15,9 +15,9 @@ use rustc_middle::ty::Ty;
 use rustc_target::abi::{Abi, Align, FieldIdx, HasDataLayout, Size, FIRST_VARIANT};
 
 use super::{
-    alloc_range, mir_assign_valid_types, AllocId, AllocRef, AllocRefMut, ImmTy, Immediate,
-    InterpCx, InterpResult, Machine, MemoryKind, Misalignment, OffsetMode, OpTy, Operand, Pointer,
-    PointerArithmetic, Projectable, Provenance, Readable, Scalar,
+    alloc_range, mir_assign_valid_types, AllocId, AllocRef, AllocRefMut, CheckAlignMsg, ImmTy,
+    Immediate, InterpCx, InterpResult, Machine, MemoryKind, Misalignment, OffsetMode, OpTy,
+    Operand, Pointer, PointerArithmetic, Projectable, Provenance, Readable, Scalar,
 };
 
 #[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
@@ -461,7 +461,7 @@ where
         // We check alignment separately, and *after* checking everything else.
         // If an access is both OOB and misaligned, we want to see the bounds error.
         let a = self.get_ptr_alloc(mplace.ptr(), size, Align::ONE)?;
-        self.check_misalign(mplace.mplace.misaligned)?;
+        self.check_misalign(mplace.mplace.misaligned, CheckAlignMsg::BasedOn)?;
         Ok(a)
     }
 
@@ -477,7 +477,7 @@ where
         // We check alignment separately, and raise that error *after* checking everything else.
         // If an access is both OOB and misaligned, we want to see the bounds error.
         // However we have to call `check_misalign` first to make the borrow checker happy.
-        let misalign_err = self.check_misalign(mplace.mplace.misaligned);
+        let misalign_err = self.check_misalign(mplace.mplace.misaligned, CheckAlignMsg::BasedOn);
         let a = self.get_ptr_alloc_mut(mplace.ptr(), size, Align::ONE)?;
         misalign_err?;
         Ok(a)
@@ -881,8 +881,8 @@ where
             dest_size,
             /*nonoverlapping*/ true,
         )?;
-        self.check_misalign(src.mplace.misaligned)?;
-        self.check_misalign(dest.mplace.misaligned)?;
+        self.check_misalign(src.mplace.misaligned, CheckAlignMsg::BasedOn)?;
+        self.check_misalign(dest.mplace.misaligned, CheckAlignMsg::BasedOn)?;
         Ok(())
     }
 

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -385,7 +385,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                 CheckInAllocMsg::InboundsTest, // will anyway be replaced by validity message
             ),
             self.path,
-            Ub(AlignmentCheckFailed(Misalignment { required, has })) => UnalignedPtr {
+            Ub(AlignmentCheckFailed(Misalignment { required, has }, _msg)) => UnalignedPtr {
                 ptr_kind,
                 required_bytes: required.bytes(),
                 found_bytes: has.bytes()

compiler/rustc_middle/src/mir/interpret/error.rs

@@ -216,7 +216,7 @@ pub enum InvalidProgramInfo<'tcx> {
 }
 
 /// Details of why a pointer had to be in-bounds.
-#[derive(Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
+#[derive(Debug, Copy, Clone)]
 pub enum CheckInAllocMsg {
     /// We are access memory.
     MemoryAccessTest,
@@ -228,7 +228,16 @@ pub enum CheckInAllocMsg {
     InboundsTest,
 }
 
-#[derive(Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
+/// Details of which pointer is not aligned.
+#[derive(Debug, Copy, Clone)]
+pub enum CheckAlignMsg {
+    /// The accessed pointer did not have proper alignment.
+    AccessedPtr,
+    /// The access ocurred with a place that was based on a misaligned pointer.
+    BasedOn,
+}
+
+#[derive(Debug, Copy, Clone)]
 pub enum InvalidMetaKind {
     /// Size of a `[T]` is too big
     SliceTooBig,
@@ -329,7 +338,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
     /// Using an integer as a pointer in the wrong way.
     DanglingIntPointer(u64, CheckInAllocMsg),
     /// Used a pointer with bad alignment.
-    AlignmentCheckFailed(Misalignment),
+    AlignmentCheckFailed(Misalignment, CheckAlignMsg),
     /// Writing to read-only memory.
     WriteToReadOnly(AllocId),
     /// Trying to access the data behind a function pointer.

compiler/rustc_middle/src/mir/interpret/mod.rs

@@ -142,11 +142,12 @@ use crate::ty::GenericArgKind;
 use crate::ty::{self, Instance, Ty, TyCtxt};
 
 pub use self::error::{
-    struct_error, BadBytesAccess, CheckInAllocMsg, ErrorHandled, EvalToAllocationRawResult,
-    EvalToConstValueResult, EvalToValTreeResult, ExpectedKind, InterpError, InterpErrorInfo,
-    InterpResult, InvalidMetaKind, InvalidProgramInfo, MachineStopType, Misalignment, PointerKind,
-    ReportedErrorInfo, ResourceExhaustionInfo, ScalarSizeMismatch, UndefinedBehaviorInfo,
-    UnsupportedOpInfo, ValidationErrorInfo, ValidationErrorKind,
+    struct_error, BadBytesAccess, CheckAlignMsg, CheckInAllocMsg, ErrorHandled,
+    EvalToAllocationRawResult, EvalToConstValueResult, EvalToValTreeResult, ExpectedKind,
+    InterpError, InterpErrorInfo, InterpResult, InvalidMetaKind, InvalidProgramInfo,
+    MachineStopType, Misalignment, PointerKind, ReportedErrorInfo, ResourceExhaustionInfo,
+    ScalarSizeMismatch, UndefinedBehaviorInfo, UnsupportedOpInfo, ValidationErrorInfo,
+    ValidationErrorKind,
 };
 
 pub use self::value::Scalar;

src/tools/miri/tests/fail/const-ub-checks.stderr

@@ -2,7 +2,7 @@ error[E0080]: evaluation of constant value failed
   --> $DIR/const-ub-checks.rs:LL:CC
    |
 LL |     ptr.read();
-   |     ^^^^^^^^^^ accessing memory with alignment ALIGN, but alignment ALIGN is required
+   |     ^^^^^^^^^^ accessing memory based on pointer with alignment ALIGN, but alignment ALIGN is required
 
 note: erroneous constant encountered
   --> $DIR/const-ub-checks.rs:LL:CC

src/tools/miri/tests/fail/dangling_pointers/out_of_bounds_read.rs

@@ -0,0 +1,8 @@
+#![feature(pointer_byte_offsets)]
+
+fn main() {
+    let v: Vec<u16> = vec![1, 2];
+    // This read is also misaligned. We make sure that the OOB message has priority.
+    let x = unsafe { *v.as_ptr().wrapping_byte_add(5) }; //~ ERROR: out-of-bounds
+    panic!("this should never print: {}", x);
+}

src/tools/miri/tests/fail/dangling_pointers/out_of_bounds_read1.stderr renamed to src/tools/miri/tests/fail/dangling_pointers/out_of_bounds_read.stderr

@@ -1,18 +1,18 @@
-error: Undefined Behavior: memory access failed: ALLOC has size 2, so pointer to 1 byte starting at offset 5 is out-of-bounds
-  --> $DIR/out_of_bounds_read1.rs:LL:CC
+error: Undefined Behavior: memory access failed: ALLOC has size 4, so pointer to 2 bytes starting at offset 5 is out-of-bounds
+  --> $DIR/out_of_bounds_read.rs:LL:CC
    |
-LL |     let x = unsafe { *v.as_ptr().wrapping_offset(5) };
-   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: ALLOC has size 2, so pointer to 1 byte starting at offset 5 is out-of-bounds
+LL |     let x = unsafe { *v.as_ptr().wrapping_byte_add(5) };
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: ALLOC has size 4, so pointer to 2 bytes starting at offset 5 is out-of-bounds
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
 help: ALLOC was allocated here:
-  --> $DIR/out_of_bounds_read1.rs:LL:CC
+  --> $DIR/out_of_bounds_read.rs:LL:CC
    |
-LL |     let v: Vec<u8> = vec![1, 2];
-   |                      ^^^^^^^^^^
+LL |     let v: Vec<u16> = vec![1, 2];
+   |                       ^^^^^^^^^^
    = note: BACKTRACE (of the first span):
-   = note: inside `main` at $DIR/out_of_bounds_read1.rs:LL:CC
+   = note: inside `main` at $DIR/out_of_bounds_read.rs:LL:CC
    = note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace