const-eval interner: from-scratch rewrite using mutability information from provenance rather than types

1 files changed, 116 insertions, 29 deletions

diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs
index 8b44b87647d..f028d7802ae 100644
--- a/compiler/rustc_const_eval/src/interpret/validity.rs
+++ b/compiler/rustc_const_eval/src/interpret/validity.rs
@@ -9,12 +9,13 @@ 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, Misalignment, PointerKind, ValidationErrorInfo,
-    ValidationErrorKind, ValidationErrorKind::*,
+    ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance,
+    ValidationErrorInfo, ValidationErrorKind, ValidationErrorKind::*,
 };
 use rustc_middle::ty;
 use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
@@ -123,15 +124,41 @@ 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.
+    /// Validation of a `static`
+    Static { mutbl: Mutability },
+    /// Validation of a `const` (including promoteds).
+    /// `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::Static { .. } => false,
+            CtfeValidationMode::Const { allow_immutable_unsafe_cell, .. } => {
+                allow_immutable_unsafe_cell
+            }
+        }
+    }
+
+    fn allow_static_ptrs(self) -> bool {
+        match self {
+            CtfeValidationMode::Static { .. } => true, // statics can point to statics
+            CtfeValidationMode::Const { allow_static_ptrs, .. } => allow_static_ptrs,
+        }
+    }
+
+    fn may_contain_mutable_ref(self) -> bool {
+        match self {
+            CtfeValidationMode::Static { mutbl } => mutbl == Mutability::Mut,
+            CtfeValidationMode::Const { .. } => false,
+        }
+    }
 }
 
 /// State for tracking recursive validation of references
@@ -418,6 +445,22 @@ 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` inside
+                    // `Option<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()) {
@@ -428,16 +471,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.
@@ -454,14 +510,29 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                         if alloc.inner().mutability == Mutability::Mut
                             && matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
                         {
-                            // This should be unreachable, but if someone manages to copy a pointer
-                            // out of a `static`, then that pointer might point to mutable memory,
-                            // and we would catch that here.
-                            throw_validation_failure!(self.path, PtrToMut { ptr_kind });
+                            // This is impossible: this can only be some inner allocation of a
+                            // `static mut` (everything else either hits the `GlobalAlloc::Static`
+                            // case or is interned immutably). To get such a pointer we'd have to
+                            // load it from a static, but such loads lead to a CTFE error.
+                            span_bug!(
+                                self.ecx.tcx.span,
+                                "encountered reference to mutable memory inside a `const`"
+                            );
+                        }
+                        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;
@@ -532,11 +603,9 @@ 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 { .. }))
+                if self.ctfe_mode.is_some_and(|c| !c.may_contain_mutable_ref())
                     && *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);
@@ -642,6 +711,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>
@@ -705,10 +787,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, .. })) {
-            if !op.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.param_env) {
-                throw_validation_failure!(self.path, UnsafeCell);
+        // 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.is_zst() && !op.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.param_env) {
+                if !self.in_mutable_memory(op) {
+                    throw_validation_failure!(self.path, UnsafeCellInImmutable);
+                }
             }
         }
         Ok(())
@@ -730,11 +814,14 @@ 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, .. }))
+        if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
+            if !op.layout.is_zst()
+                && let Some(def) = op.layout.ty.ty_adt_def()
                 && def.is_unsafe_cell()
             {
-                throw_validation_failure!(self.path, UnsafeCell);
+                if !self.in_mutable_memory(op) {
+                    throw_validation_failure!(self.path, UnsafeCellInImmutable);
+                }
             }
         }