Rollup merge of rust-lang#143046 - RalfJung:zst-unsafe-cell, r=lcnr,oli-obk

const validation: properly ignore zero-sized UnsafeCell

Fixes rust-lang#142948
r? `@oli-obk`

Author: matthiaskrgr

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -383,7 +383,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     /// Returns the actual dynamic size and alignment of the place at the given type.
     /// Only the "meta" (metadata) part of the place matters.
     /// This can fail to provide an answer for extern types.
-    pub(super) fn size_and_align_of(
+    pub(super) fn size_and_align_from_meta(
         &self,
         metadata: &MemPlaceMeta<M::Provenance>,
         layout: &TyAndLayout<'tcx>,
@@ -409,7 +409,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 // adjust alignment and size for them?
                 let field = layout.field(self, layout.fields.count() - 1);
                 let Some((unsized_size, mut unsized_align)) =
-                    self.size_and_align_of(metadata, &field)?
+                    self.size_and_align_from_meta(metadata, &field)?
                 else {
                     // A field with an extern type. We don't know the actual dynamic size
                     // or the alignment.
@@ -471,11 +471,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         }
     }
     #[inline]
-    pub fn size_and_align_of_mplace(
+    pub fn size_and_align_of_val(
         &self,
-        mplace: &MPlaceTy<'tcx, M::Provenance>,
+        val: &impl Projectable<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, Option<(Size, Align)>> {
-        self.size_and_align_of(&mplace.meta(), &mplace.layout)
+        self.size_and_align_from_meta(&val.meta(), &val.layout())
     }
 
     /// Jump to the given block.

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -125,7 +125,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 // dereferenceable!
                 let place = self.ref_to_mplace(&self.read_immediate(&args[0])?)?;
                 let (size, align) = self
-                    .size_and_align_of_mplace(&place)?
+                    .size_and_align_of_val(&place)?
                     .ok_or_else(|| err_unsup_format!("`extern type` does not have known layout"))?;
 
                 let result = match intrinsic_name {

compiler/rustc_const_eval/src/interpret/place.rs

@@ -470,7 +470,7 @@ where
     ) -> InterpResult<'tcx, Option<AllocRef<'_, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
     {
         let (size, _align) = self
-            .size_and_align_of_mplace(mplace)?
+            .size_and_align_of_val(mplace)?
             .unwrap_or((mplace.layout.size, mplace.layout.align.abi));
         // We check alignment separately, and *after* checking everything else.
         // If an access is both OOB and misaligned, we want to see the bounds error.
@@ -486,7 +486,7 @@ where
     ) -> InterpResult<'tcx, Option<AllocRefMut<'_, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
     {
         let (size, _align) = self
-            .size_and_align_of_mplace(mplace)?
+            .size_and_align_of_val(mplace)?
             .unwrap_or((mplace.layout.size, mplace.layout.align.abi));
         // 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.
@@ -888,11 +888,11 @@ where
         trace!("copy_op: {:?} <- {:?}: {}", *dest, src, dest.layout().ty);
 
         let dest = dest.force_mplace(self)?;
-        let Some((dest_size, _)) = self.size_and_align_of_mplace(&dest)? else {
+        let Some((dest_size, _)) = self.size_and_align_of_val(&dest)? else {
             span_bug!(self.cur_span(), "copy_op needs (dynamically) sized values")
         };
         if cfg!(debug_assertions) {
-            let src_size = self.size_and_align_of_mplace(&src)?.unwrap().0;
+            let src_size = self.size_and_align_of_val(&src)?.unwrap().0;
             assert_eq!(src_size, dest_size, "Cannot copy differently-sized data");
         } else {
             // As a cheap approximation, we compare the fixed parts of the size.
@@ -980,7 +980,7 @@ where
         kind: MemoryKind<M::MemoryKind>,
         meta: MemPlaceMeta<M::Provenance>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
-        let Some((size, align)) = self.size_and_align_of(&meta, &layout)? else {
+        let Some((size, align)) = self.size_and_align_from_meta(&meta, &layout)? else {
             span_bug!(self.cur_span(), "cannot allocate space for `extern` type, size is not known")
         };
         let ptr = self.allocate_ptr(size, align, kind, AllocInit::Uninit)?;

compiler/rustc_const_eval/src/interpret/projection.rs

@@ -168,7 +168,7 @@ where
             // Re-use parent metadata to determine dynamic field layout.
             // With custom DSTS, this *will* execute user-defined code, but the same
             // happens at run-time so that's okay.
-            match self.size_and_align_of(&base_meta, &field_layout)? {
+            match self.size_and_align_from_meta(&base_meta, &field_layout)? {
                 Some((_, align)) => {
                     // For packed types, we need to cap alignment.
                     let align = if let ty::Adt(def, _) = base.layout().ty.kind()

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -494,7 +494,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
         }
         // Make sure this is dereferenceable and all.
         let size_and_align = try_validation!(
-            self.ecx.size_and_align_of_mplace(&place),
+            self.ecx.size_and_align_of_val(&place),
             self.path,
             Ub(InvalidMeta(msg)) => match msg {
                 InvalidMetaKind::SliceTooBig => InvalidMetaSliceTooLarge { ptr_kind },
@@ -906,7 +906,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
         let (_prov, start_offset) = mplace.ptr().into_parts();
         let (size, _align) = self
             .ecx
-            .size_and_align_of_mplace(&mplace)?
+            .size_and_align_of_val(&mplace)?
             .unwrap_or((mplace.layout.size, mplace.layout.align.abi));
         // If there is no padding at all, we can skip the rest: check for
         // a single data range covering the entire value.
@@ -1086,8 +1086,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
     ) -> InterpResult<'tcx> {
         // Special check for CTFE validation, preventing `UnsafeCell` inside unions in immutable memory.
         if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
-            if !val.layout.is_zst() && !val.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.typing_env)
-            {
+            // Unsized unions are currently not a thing, but let's keep this code consistent with
+            // the check in `visit_value`.
+            let zst = self.ecx.size_and_align_of_val(val)?.is_some_and(|(s, _a)| s.bytes() == 0);
+            if !zst && !val.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.typing_env) {
                 if !self.in_mutable_memory(val) {
                     throw_validation_failure!(self.path, UnsafeCellInImmutable);
                 }
@@ -1131,7 +1133,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
 
         // Special check preventing `UnsafeCell` in the inner part of constants
         if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
-            if !val.layout.is_zst()
+            // Exclude ZST values. We need to compute the dynamic size/align to properly
+            // handle slices and trait objects.
+            let zst = self.ecx.size_and_align_of_val(val)?.is_some_and(|(s, _a)| s.bytes() == 0);
+            if !zst
                 && let Some(def) = val.layout.ty.ty_adt_def()
                 && def.is_unsafe_cell()
             {

compiler/rustc_mir_transform/src/gvn.rs

@@ -1625,7 +1625,7 @@ fn op_to_prop_const<'tcx>(
     // If this constant is already represented as an `Allocation`,
     // try putting it into global memory to return it.
     if let Either::Left(mplace) = op.as_mplace_or_imm() {
-        let (size, _align) = ecx.size_and_align_of_mplace(&mplace).discard_err()??;
+        let (size, _align) = ecx.size_and_align_of_val(&mplace).discard_err()??;
 
         // Do not try interning a value that contains provenance.
         // Due to https://github.com/rust-lang/rust/issues/79738, doing so could lead to bugs.

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

@@ -814,7 +814,7 @@ trait EvalContextPrivExt<'tcx, 'ecx>: crate::MiriInterpCxExt<'tcx> {
         info: RetagInfo, // diagnostics info about this retag
     ) -> InterpResult<'tcx, MPlaceTy<'tcx>> {
         let this = self.eval_context_mut();
-        let size = this.size_and_align_of_mplace(place)?.map(|(size, _)| size);
+        let size = this.size_and_align_of_val(place)?.map(|(size, _)| size);
         // FIXME: If we cannot determine the size (because the unsized tail is an `extern type`),
         // bail out -- we cannot reasonably figure out which memory range to reborrow.
         // See https://github.com/rust-lang/unsafe-code-guidelines/issues/276.

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

@@ -469,7 +469,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // - if the pointer is not reborrowed (raw pointer) then we override the size
         //   to do a zero-length reborrow.
         let reborrow_size = this
-            .size_and_align_of_mplace(place)?
+            .size_and_align_of_val(place)?
             .map(|(size, _)| size)
             .unwrap_or(place.layout.size);
         trace!("Creating new permission: {:?} with size {:?}", new_perm, reborrow_size);

src/tools/miri/src/helpers.rs

@@ -489,7 +489,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         trace!("visit_frozen(place={:?}, size={:?})", *place, size);
         debug_assert_eq!(
             size,
-            this.size_and_align_of_mplace(place)?
+            this.size_and_align_of_val(place)?
                 .map(|(size, _)| size)
                 .unwrap_or_else(|| place.layout.size)
         );
@@ -530,7 +530,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     trace!("unsafe_cell_action on {:?}", place.ptr());
                     // We need a size to go on.
                     let unsafe_cell_size = this
-                        .size_and_align_of_mplace(place)?
+                        .size_and_align_of_val(place)?
                         .map(|(size, _)| size)
                         // for extern types, just cover what we can
                         .unwrap_or_else(|| place.layout.size);

tests/ui/consts/unsafe_cell_in_const.rs

@@ -0,0 +1,15 @@
+//! Ensure we do not complain about zero-sized `UnsafeCell` in a const in any form.
+//! See <https://github.com/rust-lang/rust/issues/142948>.
+
+//@ check-pass
+use std::cell::UnsafeCell;
+
+const X1: &mut UnsafeCell<[i32; 0]> = UnsafeCell::from_mut(&mut []);
+
+const X2: &mut UnsafeCell<[i32]> = UnsafeCell::from_mut(&mut []);
+
+trait Trait {}
+impl Trait for [i32; 0] {}
+const X3: &mut UnsafeCell<dyn Trait> = UnsafeCell::from_mut(&mut []);
+
+fn main() {}