miri: ensure validity of references and pointers we dereference and cast

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -1605,8 +1605,8 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                             ),
                         }
                     }
-                    CastKind::Subtype => {
-                        bug!("CastKind::Subtype shouldn't exist in borrowck")
+                    CastKind::Subtype | CastKind::BoxDerefTransmute => {
+                        bug!("CastKind::{cast_kind:?} shouldn't exist in borrowck")
                     }
                 }
             }

compiler/rustc_codegen_cranelift/src/base.rs

@@ -823,7 +823,11 @@ fn codegen_stmt<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, cur_block: Block, stmt:
                     let operand = codegen_operand(fx, operand);
                     crate::unsize::coerce_unsized_into(fx, operand, lval);
                 }
-                Rvalue::Cast(CastKind::Transmute | CastKind::Subtype, ref operand, _to_ty) => {
+                Rvalue::Cast(
+                    CastKind::Transmute | CastKind::BoxDerefTransmute | CastKind::Subtype,
+                    ref operand,
+                    _to_ty,
+                ) => {
                     let operand = codegen_operand(fx, operand);
                     lval.write_cvalue_transmute(fx, operand);
                 }

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -619,7 +619,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                             bug!("Unsupported cast of {operand:?} to {cast:?}");
                         })
                     }
-                    mir::CastKind::Transmute | mir::CastKind::Subtype => {
+                    mir::CastKind::Transmute | mir::CastKind::BoxDerefTransmute | mir::CastKind::Subtype => {
                         self.codegen_transmute_operand(bx, operand, cast)
                     }
                 };

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -17,7 +17,7 @@ use super::{
     throw_ub_format,
 };
 use crate::enter_trace_span;
-use crate::interpret::Writeable;
+use crate::interpret::{Projectable, Writeable};
 
 impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     pub fn cast(
@@ -31,10 +31,66 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         // possible.
         let cast_layout =
             if cast_ty == dest.layout.ty { dest.layout } else { self.layout_of(cast_ty)? };
-        // FIXME: In which cases should we trigger UB when the source is uninit?
+
+        // Check that the input is valid.
+        // Can be skipped for transmuts and unsizing as those do validation below.
+        if !matches!(
+            cast_kind,
+            CastKind::Transmute
+                | CastKind::Subtype
+                | CastKind::BoxDerefTransmute
+                | CastKind::PointerCoercion(PointerCoercion::Unsize, _)
+        ) && M::enforce_validity(self, src.layout)
+        {
+            match src.layout.ty.kind() {
+                ty::RawPtr { .. } => {
+                    // We only need to check anything for wide pointers.
+                    if matches!(src.layout.backend_repr, rustc_abi::BackendRepr::ScalarPair { .. })
+                    {
+                        self.deref_pointer(src)?;
+                    }
+                }
+                ty::FnPtr { .. } => {
+                    let ptr = self.read_pointer(src)?;
+                    self.get_ptr_fn(ptr)?;
+                }
+                ty::Closure(_closure, args) => {
+                    // Can only happen for non-capturing closures, which have nothing to validate.
+                    let args = args.as_closure();
+                    assert!(args.upvar_tys().is_empty());
+                }
+                // Types that have no requirements or whose requirements are checked by the actual
+                // cast operation.
+                ty::Int(..)
+                | ty::Uint(..)
+                | ty::Float(..)
+                | ty::Bool
+                | ty::Char
+                | ty::FnDef(..) => {}
+
+                _ => {
+                    span_bug!(
+                        self.cur_span(),
+                        "unexpected input type in non-transmute/unsize cast: {}",
+                        src.layout.ty
+                    )
+                }
+            }
+        }
+
         match cast_kind {
             CastKind::PointerCoercion(PointerCoercion::Unsize, _) => {
                 self.unsize_into(src, cast_layout, dest)?;
+                // Validate the entire thing and reset any padding in the output.
+                // It is enough to validate the output because we are only adding metadata,
+                // not discarding anything from the input that may have been invalid.
+                if M::enforce_validity(self, dest.layout()) {
+                    self.validate_place(
+                        dest,
+                        M::enforce_validity_recursively(self, dest.layout()),
+                        /*reset_provenance_and_padding*/ true,
+                    )?;
+                }
             }
 
             CastKind::PointerExposeProvenance => {
@@ -133,7 +189,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 }
             }
 
-            CastKind::Transmute | CastKind::Subtype => {
+            CastKind::Transmute | CastKind::Subtype | CastKind::BoxDerefTransmute => {
                 assert!(src.layout.is_sized());
                 assert!(dest.layout.is_sized());
                 assert_eq!(cast_ty, dest.layout.ty); // we otherwise ignore `cast_ty` enirely...
@@ -147,6 +203,17 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                     );
                 }
 
+                if matches!(cast_kind, CastKind::BoxDerefTransmute) {
+                    // Do the extra UB checking by making the input an actual `Box<T>` pointer
+                    // and dereferencing it.
+                    let ptr = self.read_immediate(src)?;
+                    let pointee_ty = cast_ty.builtin_deref(true).unwrap();
+                    let box_ty = Ty::new_box(*self.tcx, pointee_ty);
+                    let ptr = ptr.transmute(self.layout_of(box_ty)?, self)?;
+                    self.deref_pointer(&ptr)?;
+                }
+
+                // This does validation at `src` and `dest` type.
                 self.copy_op_allow_transmute(src, dest)?;
             }
         }
@@ -266,6 +333,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         // Let's make sure v is sign-extended *if* it has a signed type.
         let signed = src_layout.backend_repr.is_signed(); // Also asserts that abi is `Scalar`.
 
+        // We go through the actual type of `src` to ensure the value is valid.
         let v = match src_layout.ty.kind() {
             ty::Uint(_) | ty::RawPtr(..) | ty::FnPtr(..) => scalar.to_uint(src_layout.size)?,
             ty::Int(_) => scalar.to_int(src_layout.size)? as u128, // we will cast back to `i128` below if the sign matters
@@ -458,6 +526,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         }
     }
 
+    /// Perform an unsizing coercion. The caller is responsible for checking validity afterwards!
     pub fn unsize_into(
         &mut self,
         src: &OpTy<'tcx, M::Provenance>,
@@ -489,7 +558,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                     if src_field.layout.is_1zst() && cast_ty_field.is_1zst() {
                         // Skip 1-ZST fields.
                     } else if src_field.layout.ty == cast_ty_field.ty {
-                        self.copy_op(&src_field, &dst_field)?;
+                        // The caller performs validation.
+                        self.copy_op_no_validate(
+                            &src_field, &dst_field, /* allow_transmute */ false,
+                        )?;
                     } else {
                         if found_cast_field {
                             span_bug!(self.cur_span(), "unsize_into: more than one field to cast");

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -472,7 +472,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 
                 // Check that the memory between them is dereferenceable at all, starting from the
                 // origin pointer: `dist` is `a - b`, so it is based on `b`.
-                self.check_ptr_access_signed(b, dist, CheckInAllocMsg::Dereferenceable)
+                self.check_ptr_access_signed(b, dist, CheckInAllocMsg::Dereferenceable("pointer"))
                     .map_err_kind(|_| {
                         // This could mean they point to different allocations, or they point to the same allocation
                         // but not the entire range between the pointers is in-bounds.
@@ -494,7 +494,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 self.check_ptr_access_signed(
                     a,
                     dist.checked_neg().unwrap(), // i64::MIN is impossible as no allocation can be that large
-                    CheckInAllocMsg::Dereferenceable,
+                    CheckInAllocMsg::Dereferenceable("pointer"),
                 )
                 .map_err_kind(|_| {
                     // Make the error more specific.

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -1071,14 +1071,17 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         expected_trait: Option<&'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>>,
     ) -> InterpResult<'tcx, Ty<'tcx>> {
         trace!("get_ptr_vtable({:?})", ptr);
-        let (alloc_id, offset, _tag) = self.ptr_get_alloc_id(ptr, 0)?;
+        let (alloc_id, offset, _tag) = self.ptr_get_alloc_id(ptr, 0).map_err_kind(|err| {
+            let err_ub!(DanglingIntPointer { addr, .. }) = err else { bug!() };
+            err_ub!(InvalidVTablePointer(Pointer::without_provenance(addr)))
+        })?;
         if offset.bytes() != 0 {
-            throw_ub!(InvalidVTablePointer(Pointer::new(alloc_id, offset)))
+            throw_ub!(InvalidVTablePointer(Pointer::new(alloc_id, offset).into()))
         }
         let Some(GlobalAlloc::VTable(ty, vtable_dyn_type)) =
             self.tcx.try_get_global_alloc(alloc_id)
         else {
-            throw_ub!(InvalidVTablePointer(Pointer::new(alloc_id, offset)))
+            throw_ub!(InvalidVTablePointer(Pointer::new(alloc_id, offset).into()))
         };
         if let Some(expected_dyn_type) = expected_trait {
             self.check_vtable_for_type(vtable_dyn_type, expected_dyn_type)?;
@@ -1720,11 +1723,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         size: i64,
     ) -> InterpResult<'tcx, (AllocId, Size, M::ProvenanceExtra)> {
         self.ptr_try_get_alloc_id(ptr, size)
-            .map_err(|offset| {
+            .map_err(|addr| {
                 err_ub!(DanglingIntPointer {
-                    addr: offset,
+                    addr,
                     inbounds_size: size,
-                    msg: CheckInAllocMsg::Dereferenceable
+                    msg: CheckInAllocMsg::Dereferenceable("pointer")
                 })
             })
             .into()

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -640,7 +640,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     /// Try returning an immediate for the operand. If the layout does not permit loading this as an
     /// immediate, return where in memory we can find the data.
     /// Note that for a given layout, this operation will either always return Left or Right!
-    /// succeed!  Whether it returns Left depends on whether the layout can be represented
+    /// Whether it returns Left depends on whether the layout can be represented
     /// in an `Immediate`, not on which data is stored there currently.
     ///
     /// This is an internal function that should not usually be used; call `read_immediate` instead.

compiler/rustc_const_eval/src/interpret/operator.rs

@@ -428,8 +428,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         }
     }
 
-    /// Returns the result of the specified operation, whether it overflowed, and
-    /// the result type.
+    /// Returns the result of the specified operation.
     pub fn unary_op(
         &self,
         un_op: mir::UnOp,
@@ -486,6 +485,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             }
             ty::RawPtr(..) | ty::Ref(..) => {
                 assert_eq!(un_op, PtrMetadata);
+                self.deref_pointer(val)?; // validity check
                 let (_, meta) = val.to_scalar_and_meta();
                 interp_ok(match meta {
                     MemPlaceMeta::Meta(scalar) => {

compiler/rustc_const_eval/src/interpret/place.rs

@@ -13,9 +13,10 @@ use tracing::field::Empty;
 use tracing::{instrument, trace};
 
 use super::{
-    AllocInit, AllocRef, AllocRefMut, CheckAlignMsg, CtfeProvenance, ImmTy, Immediate, InterpCx,
-    InterpResult, Machine, MemoryKind, Misalignment, OffsetMode, OpTy, Operand, Pointer,
-    Projectable, Provenance, Scalar, alloc_range, interp_ok, mir_assign_valid_types,
+    AllocInit, AllocRef, AllocRefMut, CheckAlignMsg, CheckInAllocMsg, CtfeProvenance, ImmTy,
+    Immediate, InterpCx, InterpResult, Machine, MemoryKind, Misalignment, OffsetMode, OpTy,
+    Operand, Pointer, Projectable, Provenance, Scalar, alloc_range, err_ub, err_ub_format,
+    interp_ok, mir_assign_valid_types, throw_ub_format,
 };
 use crate::enter_trace_span;
 
@@ -462,22 +463,74 @@ where
 
     /// Take an operand, representing a pointer, and dereference it to a place.
     /// Corresponds to the `*` operator in Rust.
+    /// Unlike `imm_ptr_to_mplace`, this checks that the pointer is valid for its type.
     #[instrument(skip(self), level = "trace")]
     pub fn deref_pointer(
         &self,
         src: &impl Projectable<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
-        if src.layout().ty.is_box() {
-            // Derefer should have removed all Box derefs.
-            // Some `Box` are not immediates (if they have a custom allocator)
-            // so the code below would fail.
+        let ptr_ty = src.layout().ty;
+        if !(ptr_ty.is_ref() || ptr_ty.is_raw_ptr() || ptr_ty.is_box_global(*self.tcx)) {
             bug!("dereferencing {}", src.layout().ty);
         }
 
         let val = self.read_immediate(src)?;
+        // Construct a place for that pointer.
         trace!("deref to {} on {:?}", val.layout.ty, *val);
-
         let mplace = self.imm_ptr_to_mplace(&val)?;
+
+        // This is conceptually a typed load from `src` to get the pointer. Most of the time when
+        // we do typed loads for primitive operations, all relevant invariants are checked
+        // implicitly, e.g. when we call `to_bool()` on a Boolean.
+        // But here, we do need to specifically check for metadata validity, null, alignment, and
+        // dereferenceability, or they will not be checked anywhere at all.
+        // This duplicates some of the logic in the validity check, but so far we found no
+        // good way to share that logic.
+        if ptr_ty.is_ref() || ptr_ty.is_box() {
+            let kind = if ptr_ty.is_ref() { "reference" } else { "box" };
+
+            // Null check.
+            let scalar_ptr = Scalar::from_maybe_pointer(mplace.ptr(), self);
+            if self.scalar_may_be_null(scalar_ptr)? {
+                let maybe = !M::Provenance::OFFSET_IS_ADDR && matches!(scalar_ptr, Scalar::Ptr(..));
+                throw_ub_format!(
+                    "dereferencing a {maybe}null {kind}",
+                    maybe = if maybe { "maybe-" } else { "" }
+                );
+            }
+
+            // Dereferencability and alignment check. This also implicitly checks metadata validity.
+            let (size, align) = self
+                .size_and_align_of_val(&mplace)?
+                .unwrap_or_else(|| (mplace.layout.size, mplace.layout.align.abi));
+            self.check_ptr_access(mplace.ptr(), size, CheckInAllocMsg::Dereferenceable(kind))?;
+            self.check_ptr_align(mplace.ptr(), align).map_err_kind(|err| {
+                let err_ub!(AlignmentCheckFailed(Misalignment { required, has }, _msg)) = err else { bug!() };
+                err_ub_format!(
+                    "encountered an unaligned {kind} (required {required_bytes} byte alignment but found {found_bytes})",
+                    required_bytes = required.bytes(),
+                    found_bytes = has.bytes()
+                )
+            })?;
+        } else {
+            assert!(ptr_ty.is_raw_ptr());
+            // For raw pointers, the validity invariant is pretty weak, but we do require the vtable
+            // to make sense, so we do have to check that if there is one.
+            if mplace.layout.is_unsized() {
+                let tail = self.tcx.struct_tail_for_codegen(mplace.layout.ty, self.typing_env);
+                match tail.kind() {
+                    ty::Dynamic(data, _) => {
+                        let vtable = mplace.meta().unwrap_meta().to_pointer(self)?;
+                        self.get_ptr_vtable_ty(vtable, Some(data))?;
+                    }
+                    ty::Slice(..) | ty::Str | ty::Foreign(..) => {
+                        // Nothing to check (`read_immediate` already ensured initialization).
+                    }
+                    _ => bug!("Unexpected unsized type tail: {:?}", tail),
+                }
+            }
+        }
+
         interp_ok(mplace)
     }
 
@@ -831,8 +884,13 @@ where
         self.copy_op_inner(src, dest, /* allow_transmute */ false)
     }
 
-    /// Copies the data from an operand to a place.
-    /// `allow_transmute` indicates whether the layouts may disagree.
+    /// Perform a typed copy of the data from an operand to a place.
+    ///
+    /// `allow_transmute` indicates whether the layouts may disagree. In that case there are
+    /// technically *two* typed copies: `src` is a not-yet-loaded value, so we're doing a typed copy
+    /// at `src` type from there to some intermediate storage. And then we're doing a second typed
+    /// copy at `dest` type from that intermediate storage to `dest`. As an optimization, we only
+    /// make a single direct copy here, but we still have to ensure the data is valid at both types.
     #[inline(always)]
     #[instrument(skip(self), level = "trace")]
     fn copy_op_inner(
@@ -841,11 +899,6 @@ where
         dest: &impl Writeable<'tcx, M::Provenance>,
         allow_transmute: bool,
     ) -> InterpResult<'tcx> {
-        // These are technically *two* typed copies: `src` is a not-yet-loaded value,
-        // so we're doing a typed copy at `src` type from there to some intermediate storage.
-        // And then we're doing a second typed copy at `dest` type from that intermediate storage to
-        // `dest`. But as an optimization, we only make a single direct copy here.
-
         // Do the actual copy.
         self.copy_op_no_validate(src, dest, allow_transmute)?;
 
@@ -874,10 +927,10 @@ where
         interp_ok(())
     }
 
-    /// Copies the data from an operand to a place.
+    /// Perform an untyped copy of the data from an operand to a place.
+    /// You are responsible for validating that things get copied at the right type.
+    ///
     /// `allow_transmute` indicates whether the layouts may disagree.
-    /// Also, if you use this you are responsible for validating that things get copied at the
-    /// right type.
     #[instrument(skip(self), level = "trace")]
     pub(super) fn copy_op_no_validate(
         &mut self,