Auto merge of rust-lang#114330 - RalfJung:dagling-ptr-deref, r=oli-obk

don't UB on dangling ptr deref, instead check inbounds on projections

This implements rust-lang/reference#1387 in Miri. See that PR for what the change is about.

Detecting dangling references in `let x = &...;` is now done by validity checking only, so some tests need to have validity checking enabled. There is no longer inherently a "nodangle" check in evaluating the expression `&*ptr` (aside from the aliasing model).

r? `@oli-obk`

Based on:
- rust-lang/reference#1387
- rust-lang#115524

compiler/rustc_abi/src/lib.rs

@@ -681,6 +681,7 @@ impl fmt::Display for AlignFromBytesError {
 
 impl Align {
     pub const ONE: Align = Align { pow2: 0 };
+    // LLVM has a maximal supported alignment of 2^29, we inherit that.
     pub const MAX: Align = Align { pow2: 29 };
 
     #[inline]

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 =
@@ -61,7 +65,6 @@ const_eval_deref_coercion_non_const =
     .target_note = deref defined here
 const_eval_deref_function_pointer =
     accessing {$allocation} which contains a function
-const_eval_deref_test = dereferencing pointer failed
 const_eval_deref_vtable_pointer =
     accessing {$allocation} which contains a vtable
 const_eval_different_allocations =

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -1,3 +1,5 @@
+use std::mem;
+
 use either::{Left, Right};
 
 use rustc_hir::def::DefKind;
@@ -73,9 +75,9 @@ fn eval_body_using_ecx<'mir, 'tcx>(
             None => InternKind::Constant,
         }
     };
-    ecx.machine.check_alignment = CheckAlignment::No; // interning doesn't need to respect alignment
+    let check_alignment = mem::replace(&mut ecx.machine.check_alignment, CheckAlignment::No); // interning doesn't need to respect alignment
     intern_const_alloc_recursive(ecx, intern_kind, &ret)?;
-    // we leave alignment checks off, since this `ecx` will not be used for further evaluation anyway
+    ecx.machine.check_alignment = check_alignment;
 
     debug!("eval_body_using_ecx done: {:?}", ret);
     Ok(ret)

compiler/rustc_const_eval/src/errors.rs

@@ -5,8 +5,9 @@ use rustc_errors::{
 use rustc_hir::ConstContext;
 use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
 use rustc_middle::mir::interpret::{
-    CheckInAllocMsg, ExpectedKind, InterpError, InvalidMetaKind, InvalidProgramInfo, PointerKind,
-    ResourceExhaustionInfo, UndefinedBehaviorInfo, UnsupportedOpInfo, ValidationErrorInfo,
+    CheckInAllocMsg, ExpectedKind, InterpError, InvalidMetaKind, InvalidProgramInfo, Misalignment,
+    PointerKind, ResourceExhaustionInfo, UndefinedBehaviorInfo, UnsupportedOpInfo,
+    ValidationErrorInfo,
 };
 use rustc_middle::ty::{self, Ty};
 use rustc_span::Span;
@@ -389,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 {
@@ -459,7 +451,6 @@ fn bad_pointer_message(msg: CheckInAllocMsg, handler: &Handler) -> String {
     use crate::fluent_generated::*;
 
     let msg = match msg {
-        CheckInAllocMsg::DerefTest => const_eval_deref_test,
         CheckInAllocMsg::MemoryAccessTest => const_eval_memory_access_test,
         CheckInAllocMsg::PointerArithmeticTest => const_eval_pointer_arithmetic_test,
         CheckInAllocMsg::OffsetFromTest => const_eval_offset_from_test,
@@ -568,9 +559,10 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
 
                 builder.set_arg("bad_pointer_message", bad_pointer_message(msg, handler));
             }
-            AlignmentCheckFailed { 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/intern.rs

@@ -161,7 +161,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
 
     #[inline(always)]
     fn ecx(&self) -> &InterpCx<'mir, 'tcx, M> {
-        &self.ecx
+        self.ecx
     }
 
     fn visit_value(&mut self, mplace: &MPlaceTy<'tcx>) -> InterpResult<'tcx> {
@@ -259,15 +259,15 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
                 // to avoid could be expensive: on the potentially larger types, arrays and slices,
                 // rather than on all aggregates unconditionally.
                 if matches!(mplace.layout.ty.kind(), ty::Array(..) | ty::Slice(..)) {
-                    let Some((size, align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
+                    let Some((size, _align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
                         // We do the walk if we can't determine the size of the mplace: we may be
                         // dealing with extern types here in the future.
                         return Ok(true);
                     };
 
                     // If there is no provenance in this allocation, it does not contain references
                     // that point to another allocation, and we can avoid the interning walk.
-                    if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr(), size, align)? {
+                    if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr(), size)? {
                         if !alloc.has_provenance() {
                             return Ok(false);
                         }

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -13,7 +13,7 @@ use rustc_middle::ty::layout::{LayoutOf as _, ValidityRequirement};
 use rustc_middle::ty::GenericArgsRef;
 use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_span::symbol::{sym, Symbol};
-use rustc_target::abi::{Abi, Align, Primitive, Size};
+use rustc_target::abi::{Abi, Primitive, Size};
 
 use super::{
     util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
@@ -349,10 +349,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // Check that the range between them is dereferenceable ("in-bounds or one past the
                 // end of the same allocation"). This is like the check in ptr_offset_inbounds.
                 let min_ptr = if dist >= 0 { b } else { a };
-                self.check_ptr_access_align(
+                self.check_ptr_access(
                     min_ptr,
                     Size::from_bytes(dist.unsigned_abs()),
-                    Align::ONE,
                     CheckInAllocMsg::OffsetFromTest,
                 )?;
 
@@ -571,16 +570,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     pub fn ptr_offset_inbounds(
         &self,
         ptr: Pointer<Option<M::Provenance>>,
-        pointee_ty: Ty<'tcx>,
-        offset_count: i64,
+        offset_bytes: i64,
     ) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
-        // We cannot overflow i64 as a type's size must be <= isize::MAX.
-        let pointee_size = i64::try_from(self.layout_of(pointee_ty)?.size.bytes()).unwrap();
-        // The computed offset, in bytes, must not overflow an isize.
-        // `checked_mul` enforces a too small bound, but no actual allocation can be big enough for
-        // the difference to be noticeable.
-        let offset_bytes =
-            offset_count.checked_mul(pointee_size).ok_or(err_ub!(PointerArithOverflow))?;
         // The offset being in bounds cannot rely on "wrapping around" the address space.
         // So, first rule out overflows in the pointer arithmetic.
         let offset_ptr = ptr.signed_offset(offset_bytes, self)?;
@@ -589,10 +580,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // pointers to be properly aligned (unlike a read/write operation).
         let min_ptr = if offset_bytes >= 0 { ptr } else { offset_ptr };
         // This call handles checking for integer/null pointers.
-        self.check_ptr_access_align(
+        self.check_ptr_access(
             min_ptr,
             Size::from_bytes(offset_bytes.unsigned_abs()),
-            Align::ONE,
             CheckInAllocMsg::PointerArithmeticTest,
         )?;
         Ok(offset_ptr)
@@ -621,7 +611,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let src = self.read_pointer(src)?;
         let dst = self.read_pointer(dst)?;
 
-        self.mem_copy(src, align, dst, align, size, nonoverlapping)
+        self.check_ptr_align(src, align)?;
+        self.check_ptr_align(dst, align)?;
+
+        self.mem_copy(src, dst, size, nonoverlapping)
     }
 
     pub(crate) fn write_bytes_intrinsic(
@@ -677,7 +670,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                          size|
          -> InterpResult<'tcx, &[u8]> {
             let ptr = this.read_pointer(op)?;
-            let Some(alloc_ref) = self.get_ptr_alloc(ptr, size, Align::ONE)? else {
+            let Some(alloc_ref) = self.get_ptr_alloc(ptr, size)? else {
                 // zero-sized access
                 return Ok(&[]);
             };

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -436,6 +436,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
         place: &PlaceTy<'tcx, Self::Provenance>,
     ) -> InterpResult<'tcx> {
         // Without an aliasing model, all we can do is put `Uninit` into the place.
+        // Conveniently this also ensures that the place actually points to suitable memory.
         ecx.write_uninit(place)
     }