const validation: ensure we don't have &mut to read-only memory or Un…

…safeCell in read-only memory

compiler/rustc_const_eval/messages.ftl

@@ -438,7 +438,7 @@ const_eval_validation_invalid_fn_ptr = {$front_matter}: encountered {$value}, bu
 const_eval_validation_invalid_ref_meta = {$front_matter}: encountered invalid reference metadata: total size is bigger than largest supported object
 const_eval_validation_invalid_ref_slice_meta = {$front_matter}: encountered invalid reference metadata: slice is bigger than largest supported object
 const_eval_validation_invalid_vtable_ptr = {$front_matter}: encountered {$value}, but expected a vtable pointer
-const_eval_validation_mutable_ref_in_const = {$front_matter}: encountered mutable reference in a `const`
+const_eval_validation_mutable_ref_to_immutable = {$front_matter}: encountered mutable reference or box pointing to read-only memory
 const_eval_validation_never_val = {$front_matter}: encountered a value of the never type `!`
 const_eval_validation_null_box = {$front_matter}: encountered a null box
 const_eval_validation_null_fn_ptr = {$front_matter}: encountered a null function pointer
@@ -456,7 +456,7 @@ const_eval_validation_unaligned_ref = {$front_matter}: encountered an unaligned
 const_eval_validation_uninhabited_enum_variant = {$front_matter}: encountered an uninhabited enum variant
 const_eval_validation_uninhabited_val = {$front_matter}: encountered a value of uninhabited type `{$ty}`
 const_eval_validation_uninit = {$front_matter}: encountered uninitialized memory, but {$expected}
-const_eval_validation_unsafe_cell = {$front_matter}: encountered `UnsafeCell` in a `const`
+const_eval_validation_unsafe_cell = {$front_matter}: encountered `UnsafeCell` in read-only memory
 
 const_eval_write_to_read_only =
     writing to {$allocation} which is read-only

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -336,10 +336,21 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
                     let mode = match tcx.static_mutability(cid.instance.def_id()) {
                         Some(_) if cid.promoted.is_some() => {
                             // Promoteds in statics are allowed to point to statics.
-                            CtfeValidationMode::Const { inner, allow_static_ptrs: true }
+                            CtfeValidationMode::Const {
+                                allow_immutable_unsafe_cell: false,
+                                allow_static_ptrs: true,
+                            }
                         }
                         Some(_) => CtfeValidationMode::Regular, // a `static`
-                        None => CtfeValidationMode::Const { inner, allow_static_ptrs: false },
+                        None => {
+                            // In normal `const` (not promoted), the outermost allocation is always only copied,
+                            // so having `UnsafeCell` in there is okay despite them being in immutable memory.
+                            let allow_immutable_unsafe_cell = cid.promoted.is_none() && !inner;
+                            CtfeValidationMode::Const {
+                                allow_immutable_unsafe_cell,
+                                allow_static_ptrs: false,
+                            }
+                        }
                     };
                     ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?;
                     inner = true;

compiler/rustc_const_eval/src/errors.rs

@@ -625,13 +625,13 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
 
             PointerAsInt { .. } => const_eval_validation_pointer_as_int,
             PartialPointer => const_eval_validation_partial_pointer,
-            MutableRefInConst => const_eval_validation_mutable_ref_in_const,
+            MutableRefToImmutable => const_eval_validation_mutable_ref_to_immutable,
             NullFnPtr => const_eval_validation_null_fn_ptr,
             NeverVal => const_eval_validation_never_val,
             NullablePtrOutOfRange { .. } => const_eval_validation_nullable_ptr_out_of_range,
             PtrOutOfRange { .. } => const_eval_validation_ptr_out_of_range,
             OutOfRange { .. } => const_eval_validation_out_of_range,
-            UnsafeCell => const_eval_validation_unsafe_cell,
+            UnsafeCellInImmutable => const_eval_validation_unsafe_cell,
             UninhabitedVal { .. } => const_eval_validation_uninhabited_val,
             InvalidEnumTag { .. } => const_eval_validation_invalid_enum_tag,
             UninhabitedEnumVariant => const_eval_validation_uninhabited_enum_variant,
@@ -778,10 +778,10 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
             NullPtr { .. }
             | PtrToStatic { .. }
             | PtrToMut { .. }
-            | MutableRefInConst
+            | MutableRefToImmutable
             | NullFnPtr
             | NeverVal
-            | UnsafeCell
+            | UnsafeCellInImmutable
             | InvalidMetaSliceTooLarge { .. }
             | InvalidMetaTooLarge { .. }
             | DanglingPtrUseAfterFree { .. }

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -9,11 +9,12 @@ use std::num::NonZeroUsize;
 
 use either::{Left, Right};
 
+use hir::def::DefKind;
 use rustc_ast::Mutability;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_middle::mir::interpret::{
-    ExpectedKind, InterpError, InvalidMetaKind, PointerKind, ValidationErrorInfo,
+    ExpectedKind, InterpError, InvalidMetaKind, PointerKind, Provenance, ValidationErrorInfo,
     ValidationErrorKind, ValidationErrorKind::*,
 };
 use rustc_middle::ty;
@@ -123,15 +124,34 @@ pub enum PathElem {
 }
 
 /// Extra things to check for during validation of CTFE results.
+#[derive(Copy, Clone)]
 pub enum CtfeValidationMode {
     /// Regular validation, nothing special happening.
     Regular,
     /// Validation of a `const`.
-    /// `inner` says if this is an inner, indirect allocation (as opposed to the top-level const
-    /// allocation). Being an inner allocation makes a difference because the top-level allocation
-    /// of a `const` is copied for each use, but the inner allocations are implicitly shared.
+    /// `allow_immutable_unsafe_cell` says whether we allow `UnsafeCell` in immutable memory (which is the
+    /// case for the top-level allocation of a `const`, where this is fine because the allocation will be
+    /// copied at each use site).
     /// `allow_static_ptrs` says if pointers to statics are permitted (which is the case for promoteds in statics).
-    Const { inner: bool, allow_static_ptrs: bool },
+    Const { allow_immutable_unsafe_cell: bool, allow_static_ptrs: bool },
+}
+
+impl CtfeValidationMode {
+    fn allow_immutable_unsafe_cell(self) -> bool {
+        match self {
+            CtfeValidationMode::Regular => false,
+            CtfeValidationMode::Const { allow_immutable_unsafe_cell, .. } => {
+                allow_immutable_unsafe_cell
+            }
+        }
+    }
+
+    fn allow_static_ptrs(self) -> bool {
+        match self {
+            CtfeValidationMode::Regular => true, // statics can point to statics
+            CtfeValidationMode::Const { allow_static_ptrs, .. } => allow_static_ptrs,
+        }
+    }
 }
 
 /// State for tracking recursive validation of references
@@ -412,6 +432,21 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
         }
         // Recursive checking
         if let Some(ref_tracking) = self.ref_tracking.as_deref_mut() {
+            // Determine whether this pointer expects to be pointing to something mutable.
+            let ptr_expected_mutbl = match ptr_kind {
+                PointerKind::Box => Mutability::Mut,
+                PointerKind::Ref => {
+                    let tam = value.layout.ty.builtin_deref(false).unwrap();
+                    // ZST never require mutability. We do not take into account interior mutability here
+                    // since we cannot know if there really is an `UnsafeCell` -- so we check that
+                    // in the recursive descent behind this reference.
+                    if size == Size::ZERO || tam.mutbl == Mutability::Not {
+                        Mutability::Not
+                    } else {
+                        Mutability::Mut
+                    }
+                }
+            };
             // Proceed recursively even for ZST, no reason to skip them!
             // `!` is a ZST and we want to validate it.
             if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr()) {
@@ -422,16 +457,29 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                         // Special handling for pointers to statics (irrespective of their type).
                         assert!(!self.ecx.tcx.is_thread_local_static(did));
                         assert!(self.ecx.tcx.is_static(did));
-                        if matches!(
-                            self.ctfe_mode,
-                            Some(CtfeValidationMode::Const { allow_static_ptrs: false, .. })
-                        ) {
+                        if self.ctfe_mode.is_some_and(|c| !c.allow_static_ptrs()) {
                             // See const_eval::machine::MemoryExtra::can_access_statics for why
                             // this check is so important.
                             // This check is reachable when the const just referenced the static,
                             // but never read it (so we never entered `before_access_global`).
                             throw_validation_failure!(self.path, PtrToStatic { ptr_kind });
                         }
+                        // Mutability check.
+                        if ptr_expected_mutbl == Mutability::Mut {
+                            if matches!(
+                                self.ecx.tcx.def_kind(did),
+                                DefKind::Static(Mutability::Not)
+                            ) && self
+                                .ecx
+                                .tcx
+                                .type_of(did)
+                                .no_bound_vars()
+                                .expect("statics should not have generic parameters")
+                                .is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all())
+                            {
+                                throw_validation_failure!(self.path, MutableRefToImmutable);
+                            }
+                        }
                         // We skip recursively checking other statics. These statics must be sound by
                         // themselves, and the only way to get broken statics here is by using
                         // unsafe code.
@@ -453,9 +501,20 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                             // and we would catch that here.
                             throw_validation_failure!(self.path, PtrToMut { ptr_kind });
                         }
+                        if ptr_expected_mutbl == Mutability::Mut
+                            && alloc.inner().mutability == Mutability::Not
+                        {
+                            throw_validation_failure!(self.path, MutableRefToImmutable);
+                        }
                     }
-                    // Nothing to check for these.
-                    None | Some(GlobalAlloc::Function(..) | GlobalAlloc::VTable(..)) => {}
+                    Some(GlobalAlloc::Function(..) | GlobalAlloc::VTable(..)) => {
+                        // These are immutable, we better don't allow mutable pointers here.
+                        if ptr_expected_mutbl == Mutability::Mut {
+                            throw_validation_failure!(self.path, MutableRefToImmutable);
+                        }
+                    }
+                    // Dangling, already handled.
+                    None => bug!(),
                 }
             }
             let path = &self.path;
@@ -525,17 +584,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                 }
                 Ok(true)
             }
-            ty::Ref(_, ty, mutbl) => {
-                if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
-                    && *mutbl == Mutability::Mut
-                {
-                    // A mutable reference inside a const? That does not seem right (except if it is
-                    // a ZST).
-                    let layout = self.ecx.layout_of(*ty)?;
-                    if !layout.is_zst() {
-                        throw_validation_failure!(self.path, MutableRefInConst);
-                    }
-                }
+            ty::Ref(..) => {
                 self.check_safe_pointer(value, PointerKind::Ref)?;
                 Ok(true)
             }
@@ -636,6 +685,19 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
             )
         }
     }
+
+    fn in_mutable_memory(&self, op: &OpTy<'tcx, M::Provenance>) -> bool {
+        if let Some(mplace) = op.as_mplace_or_imm().left() {
+            if let Some(alloc_id) = mplace.ptr().provenance.and_then(|p| p.get_alloc_id()) {
+                if self.ecx.tcx.global_alloc(alloc_id).unwrap_memory().inner().mutability
+                    == Mutability::Mut
+                {
+                    return true;
+                }
+            }
+        }
+        false
+    }
 }
 
 impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
@@ -699,10 +761,12 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
         op: &OpTy<'tcx, M::Provenance>,
         _fields: NonZeroUsize,
     ) -> InterpResult<'tcx> {
-        // Special check preventing `UnsafeCell` inside unions in the inner part of constants.
-        if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true, .. })) {
+        // 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 !op.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.param_env) {
-                throw_validation_failure!(self.path, UnsafeCell);
+                if !self.in_mutable_memory(op) {
+                    throw_validation_failure!(self.path, UnsafeCellInImmutable);
+                }
             }
         }
         Ok(())
@@ -724,11 +788,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
         }
 
         // Special check preventing `UnsafeCell` in the inner part of constants
-        if let Some(def) = op.layout.ty.ty_adt_def() {
-            if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true, .. }))
-                && def.is_unsafe_cell()
-            {
-                throw_validation_failure!(self.path, UnsafeCell);
+        if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
+            if let Some(def) = op.layout.ty.ty_adt_def() && def.is_unsafe_cell() {
+                if !self.in_mutable_memory(op) {
+                    throw_validation_failure!(self.path, UnsafeCellInImmutable);
+                }
             }
         }
 

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

@@ -414,13 +414,13 @@ pub enum ValidationErrorKind<'tcx> {
     PtrToUninhabited { ptr_kind: PointerKind, ty: Ty<'tcx> },
     PtrToStatic { ptr_kind: PointerKind },
     PtrToMut { ptr_kind: PointerKind },
-    MutableRefInConst,
+    MutableRefToImmutable,
+    UnsafeCellInImmutable,
     NullFnPtr,
     NeverVal,
     NullablePtrOutOfRange { range: WrappingRange, max_value: u128 },
     PtrOutOfRange { range: WrappingRange, max_value: u128 },
     OutOfRange { value: String, range: WrappingRange, max_value: u128 },
-    UnsafeCell,
     UninhabitedVal { ty: Ty<'tcx> },
     InvalidEnumTag { value: String },
     UninhabitedEnumVariant,

tests/ui/consts/invalid-union.32bit.stderr

@@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/invalid-union.rs:41:1
    |
 LL | fn main() {
-   | ^^^^^^^^^ constructing invalid value at .<deref>.y.<enum-variant(B)>.0: encountered `UnsafeCell` in a `const`
+   | ^^^^^^^^^ constructing invalid value at .<deref>.y.<enum-variant(B)>.0: encountered `UnsafeCell` in read-only memory
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
    = note: the raw bytes of the constant (size: 4, align: 4) {

tests/ui/consts/invalid-union.64bit.stderr

@@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/invalid-union.rs:41:1
    |
 LL | fn main() {
-   | ^^^^^^^^^ constructing invalid value at .<deref>.y.<enum-variant(B)>.0: encountered `UnsafeCell` in a `const`
+   | ^^^^^^^^^ constructing invalid value at .<deref>.y.<enum-variant(B)>.0: encountered `UnsafeCell` in read-only memory
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
    = note: the raw bytes of the constant (size: 8, align: 8) {