Auto merge of rust-lang#114330 - RalfJung:dagling-ptr-deref, r=<try>

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`

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

compiler/rustc_abi/src/lib.rs

@@ -680,6 +680,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,5 +1,4 @@
-use crate::const_eval::CheckAlignment;
-use crate::errors::ConstEvalError;
+use std::mem;
 
 use either::{Left, Right};
 
@@ -15,7 +14,9 @@ use rustc_span::source_map::Span;
 use rustc_target::abi::{self, Abi};
 
 use super::{CanAccessStatics, CompileTimeEvalContext, CompileTimeInterpreter};
+use crate::const_eval::CheckAlignment;
 use crate::errors;
+use crate::errors::ConstEvalError;
 use crate::interpret::eval_nullary_intrinsic;
 use crate::interpret::{
     intern_const_alloc_recursive, CtfeValidationMode, GlobalId, Immediate, InternKind, InterpCx,
@@ -74,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)
@@ -290,14 +291,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
         key.param_env,
         // Statics (and promoteds inside statics) may access other statics, because unlike consts
         // they do not have to behave "as if" they were evaluated at runtime.
-        CompileTimeInterpreter::new(
-            CanAccessStatics::from(is_static),
-            if tcx.sess.opts.unstable_opts.extra_const_ub_checks {
-                CheckAlignment::Error
-            } else {
-                CheckAlignment::FutureIncompat
-            },
-        ),
+        CompileTimeInterpreter::new(CanAccessStatics::from(is_static), CheckAlignment::Error),
     );
 
     let res = ecx.load_mir(cid.instance.def, cid.promoted);

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -1,10 +1,9 @@
 use rustc_hir::def::DefKind;
-use rustc_hir::{LangItem, CRATE_HIR_ID};
+use rustc_hir::LangItem;
 use rustc_middle::mir;
 use rustc_middle::mir::interpret::PointerArithmetic;
 use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
 use rustc_middle::ty::{self, TyCtxt};
-use rustc_session::lint::builtin::INVALID_ALIGNMENT;
 use std::borrow::Borrow;
 use std::hash::Hash;
 use std::ops::ControlFlow;
@@ -21,11 +20,11 @@ use rustc_target::abi::{Align, Size};
 use rustc_target::spec::abi::Abi as CallAbi;
 
 use crate::errors::{LongRunning, LongRunningWarn};
+use crate::fluent_generated as fluent;
 use crate::interpret::{
     self, compile_time_machine, AllocId, ConstAllocation, FnArg, FnVal, Frame, ImmTy, InterpCx,
     InterpResult, OpTy, PlaceTy, Pointer, Scalar,
 };
-use crate::{errors, fluent_generated as fluent};
 
 use super::error::*;
 
@@ -65,22 +64,11 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
 
 #[derive(Copy, Clone)]
 pub enum CheckAlignment {
-    /// Ignore alignment when following relocations.
+    /// Ignore all alignment requirements.
     /// This is mainly used in interning.
     No,
     /// Hard error when dereferencing a misaligned pointer.
     Error,
-    /// Emit a future incompat lint when dereferencing a misaligned pointer.
-    FutureIncompat,
-}
-
-impl CheckAlignment {
-    pub fn should_check(&self) -> bool {
-        match self {
-            CheckAlignment::No => false,
-            CheckAlignment::Error | CheckAlignment::FutureIncompat => true,
-        }
-    }
 }
 
 #[derive(Copy, Clone, PartialEq)]
@@ -358,48 +346,15 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
     const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error
 
     #[inline(always)]
-    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment {
-        ecx.machine.check_alignment
+    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
+        matches!(ecx.machine.check_alignment, CheckAlignment::Error)
     }
 
     #[inline(always)]
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool {
         ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks || layout.abi.is_uninhabited()
     }
 
-    fn alignment_check_failed(
-        ecx: &InterpCx<'mir, 'tcx, Self>,
-        has: Align,
-        required: Align,
-        check: CheckAlignment,
-    ) -> InterpResult<'tcx, ()> {
-        let err = err_ub!(AlignmentCheckFailed { has, required }).into();
-        match check {
-            CheckAlignment::Error => Err(err),
-            CheckAlignment::No => span_bug!(
-                ecx.cur_span(),
-                "`alignment_check_failed` called when no alignment check requested"
-            ),
-            CheckAlignment::FutureIncompat => {
-                let (_, backtrace) = err.into_parts();
-                backtrace.print_backtrace();
-                let (span, frames) = super::get_span_and_frames(&ecx);
-
-                ecx.tcx.emit_spanned_lint(
-                    INVALID_ALIGNMENT,
-                    ecx.stack().iter().find_map(|frame| frame.lint_root()).unwrap_or(CRATE_HIR_ID),
-                    span,
-                    errors::AlignmentCheckFailed {
-                        has: has.bytes(),
-                        required: required.bytes(),
-                        frames,
-                    },
-                );
-                Ok(())
-            }
-        }
-    }
-
     fn load_mir(
         ecx: &InterpCx<'mir, 'tcx, Self>,
         instance: ty::InstanceDef<'tcx>,

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,
                 )?;
 
@@ -565,16 +564,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)?;
@@ -583,10 +574,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)
@@ -615,7 +605,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(
@@ -671,7 +664,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

@@ -11,11 +11,9 @@ use rustc_middle::mir;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::def_id::DefId;
-use rustc_target::abi::{Align, Size};
+use rustc_target::abi::Size;
 use rustc_target::spec::abi::Abi as CallAbi;
 
-use crate::const_eval::CheckAlignment;
-
 use super::{
     AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
     InterpResult, MPlaceTy, MemoryKind, OpTy, PlaceTy, Pointer, Provenance,
@@ -134,21 +132,14 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
     const POST_MONO_CHECKS: bool = true;
 
     /// Whether memory accesses should be alignment-checked.
-    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment;
+    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
     /// Whether, when checking alignment, we should look at the actual address and thus support
     /// custom alignment logic based on whatever the integer address happens to be.
     ///
     /// If this returns true, Provenance::OFFSET_IS_ADDR must be true.
     fn use_addr_for_alignment_check(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
-    fn alignment_check_failed(
-        ecx: &InterpCx<'mir, 'tcx, Self>,
-        has: Align,
-        required: Align,
-        check: CheckAlignment,
-    ) -> InterpResult<'tcx, ()>;
-
     /// Whether to enforce the validity invariant for a specific layout.
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool;
 
@@ -434,6 +425,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)
     }