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

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

This implements https://github.com/rust-lang/reference/pull/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:
- https://github.com/rust-lang/reference/pull/1387
- https://github.com/rust-lang/rust/pull/115524

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

@@ -216,10 +216,8 @@ 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 dereferencing a pointer (i.e., creating a place).
-    DerefTest,
     /// We are access memory.
     MemoryAccessTest,
     /// We are doing pointer arithmetic.
@@ -230,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,
@@ -263,6 +270,13 @@ pub struct ScalarSizeMismatch {
     pub data_size: u64,
 }
 
+/// Information about a misaligned pointer.
+#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
+pub struct Misalignment {
+    pub has: Align,
+    pub required: Align,
+}
+
 macro_rules! impl_into_diagnostic_arg_through_debug {
     ($($ty:ty),*$(,)?) => {$(
         impl IntoDiagnosticArg for $ty {
@@ -324,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 { required: Align, has: Align },
+    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, 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;

compiler/rustc_middle/src/mir/syntax.rs

@@ -986,18 +986,15 @@ pub type AssertMessage<'tcx> = AssertKind<Operand<'tcx>>;
 ///    pointee's type. The resulting address is the address that was stored in the pointer. If the
 ///    pointee type is unsized, the pointer additionally stored the value of the metadata.
 ///
-/// Computing a place may cause UB. One possibility is that the pointer used for a `Deref` may not
-/// be suitably aligned. Another possibility is that the place is not in bounds, meaning it does not
-/// point to an actual allocation.
-///
-/// However, if this is actually UB and when the UB kicks in is undecided. This is being discussed
-/// in [UCG#319]. The options include that every place must obey those rules, that only some places
-/// must obey them, or that places impose no rules of their own.
-///
-/// [UCG#319]: https://github.com/rust-lang/unsafe-code-guidelines/issues/319
-///
-/// Rust currently requires that every place obey those two rules. This is checked by Miri and taken
-/// advantage of by codegen (via `gep inbounds`). That is possibly subject to change.
+/// The "validity invariant" of places is the same as that of raw pointers, meaning that e.g.
+/// `*ptr` on a dangling or unaligned pointer is never UB. (Later doing a load/store on that place
+/// or turning it into a reference can be UB though!) The only ways for a place computation can
+/// cause UB are:
+/// - On a `Deref` projection, we do an actual load of the inner place, with all the usual
+///   consequences (the inner place must be based on an aligned pointer, it must point to allocated
+///   memory, the aliasig model must allow reads, this must not be a data race).
+/// - For the projections that perform pointer arithmetic, the offset must in-bounds of an
+///   allocation (i.e., the preconditions of `ptr::offset` must be met).
 #[derive(Copy, Clone, PartialEq, Eq, Hash, TyEncodable, HashStable, TypeFoldable, TypeVisitable)]
 pub struct Place<'tcx> {
     pub local: Local,