const-eval interning: accept interior mutable pointers in final value

compiler/rustc_const_eval/src/check_consts/check.rs

@@ -538,8 +538,9 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                             // final value.
                             // Note: This is only sound if every local that has a `StorageDead` has a
                             // `StorageDead` in every control flow path leading to a `return` terminator.
-                            // The good news is that interning will detect if any unexpected mutable
-                            // pointer slips through.
+                            // If anything slips through, there's no safety net -- safe code can create
+                            // references to variants of `!Freeze` enums as long as that variant is `Freeze`,
+                            // so interning can't protect us here.
                             if self.local_is_transient(place.local) {
                                 self.check_op(ops::TransientCellBorrow);
                             } else {

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -10,21 +10,19 @@ use rustc_middle::traits::Reveal;
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_session::lint;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::{Span, DUMMY_SP};
 use rustc_target::abi::{self, Abi};
 use tracing::{debug, instrument, trace};
 
 use super::{CanAccessMutGlobal, CompileTimeInterpCx, CompileTimeMachine};
 use crate::const_eval::CheckAlignment;
-use crate::errors::{self, ConstEvalError, DanglingPtrInFinal};
 use crate::interpret::{
     create_static_alloc, eval_nullary_intrinsic, intern_const_alloc_recursive, throw_exhaust,
     CtfeValidationMode, GlobalId, Immediate, InternKind, InternResult, InterpCx, InterpError,
     InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, StackPopCleanup,
 };
-use crate::CTRL_C_RECEIVED;
+use crate::{errors, CTRL_C_RECEIVED};
 
 // Returns a pointer to where the result lives
 #[instrument(level = "trace", skip(ecx, body))]
@@ -105,18 +103,15 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
             return Err(ecx
                 .tcx
                 .dcx()
-                .emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })
+                .emit_err(errors::DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })
                 .into());
         }
         Err(InternResult::FoundBadMutablePointer) => {
-            // only report mutable pointers if there were no dangling pointers
-            let err_diag = errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind };
-            ecx.tcx.emit_node_span_lint(
-                lint::builtin::CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
-                ecx.machine.best_lint_scope(*ecx.tcx),
-                err_diag.span,
-                err_diag,
-            )
+            return Err(ecx
+                .tcx
+                .dcx()
+                .emit_err(errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind })
+                .into());
         }
     }
 
@@ -448,7 +443,12 @@ fn report_eval_error<'tcx>(
         error,
         DUMMY_SP,
         || super::get_span_and_frames(ecx.tcx, ecx.stack()),
-        |span, frames| ConstEvalError { span, error_kind: kind, instance, frame_notes: frames },
+        |span, frames| errors::ConstEvalError {
+            span,
+            error_kind: kind,
+            instance,
+            frame_notes: frames,
+        },
     )
 }
 

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -718,16 +718,29 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
         _kind: mir::RetagKind,
         val: &ImmTy<'tcx, CtfeProvenance>,
     ) -> InterpResult<'tcx, ImmTy<'tcx, CtfeProvenance>> {
-        // If it's a frozen shared reference that's not already immutable, make it immutable.
+        // If it's a frozen shared reference that's not already immutable, potentially make it immutable.
         // (Do nothing on `None` provenance, that cannot store immutability anyway.)
         if let ty::Ref(_, ty, mutbl) = val.layout.ty.kind()
             && *mutbl == Mutability::Not
-            && val.to_scalar_and_meta().0.to_pointer(ecx)?.provenance.is_some_and(|p| !p.immutable())
-            // That next check is expensive, that's why we have all the guards above.
-            && ty.is_freeze(*ecx.tcx, ecx.param_env)
+            && val
+                .to_scalar_and_meta()
+                .0
+                .to_pointer(ecx)?
+                .provenance
+                .is_some_and(|p| !p.immutable())
         {
+            // That next check is expensive, that's why we have all the guards above.
+            let is_immutable = ty.is_freeze(*ecx.tcx, ecx.param_env);
             let place = ecx.ref_to_mplace(val)?;
-            let new_place = place.map_provenance(CtfeProvenance::as_immutable);
+            let new_place = if is_immutable {
+                place.map_provenance(CtfeProvenance::as_immutable)
+            } else {
+                // Even if it is not immutable, remember that it is a shared reference.
+                // This allows it to become part of the final value of the constant.
+                // (See <https://github.com/rust-lang/rust/pull/128543> for why we allow this
+                // even when there is interior mutability.)
+                place.map_provenance(CtfeProvenance::as_shared_ref)
+            };
             Ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout))
         } else {
             Ok(val.clone())

compiler/rustc_const_eval/src/errors.rs

@@ -35,13 +35,10 @@ pub(crate) struct NestedStaticInThreadLocal {
     pub span: Span,
 }
 
-#[derive(LintDiagnostic)]
+#[derive(Diagnostic)]
 #[diag(const_eval_mutable_ptr_in_final)]
 pub(crate) struct MutablePtrInFinal {
-    // rust-lang/rust#122153: This was marked as `#[primary_span]` under
-    // `derive(Diagnostic)`. Since we expect we may hard-error in future, we are
-    // keeping the field (and skipping it under `derive(LintDiagnostic)`).
-    #[skip_arg]
+    #[primary_span]
     pub span: Span,
     pub kind: InternKind,
 }

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -20,6 +20,7 @@ use rustc_hir as hir;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
 use rustc_middle::mir::interpret::{ConstAllocation, CtfeProvenance, InterpResult};
 use rustc_middle::query::TyCtxtAt;
+use rustc_middle::span_bug;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::sym;
@@ -223,45 +224,59 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
             continue;
         }
 
-        // Crucially, we check this *before* checking whether the `alloc_id`
-        // has already been interned. The point of this check is to ensure that when
-        // there are multiple pointers to the same allocation, they are *all* immutable.
-        // Therefore it would be bad if we only checked the first pointer to any given
-        // allocation.
+        // Ensure that this is derived from a shared reference. Crucially, we check this *before*
+        // checking whether the `alloc_id` has already been interned. The point of this check is to
+        // ensure that when there are multiple pointers to the same allocation, they are *all*
+        // derived from a shared reference. Therefore it would be bad if we only checked the first
+        // pointer to any given allocation.
         // (It is likely not possible to actually have multiple pointers to the same allocation,
         // so alternatively we could also check that and ICE if there are multiple such pointers.)
+        // See <https://github.com/rust-lang/rust/pull/128543> for why we are checking for "shared
+        // reference" and not "immutable", i.e., for why we are allowing interior-mutable shared
+        // references: they can actually be created in safe code while pointing to apparently
+        // "immutable" values, via promotion or tail expression lifetime extension of
+        // `&None::<Cell<T>>`.
+        // We also exclude promoteds from this as `&mut []` can be promoted, which is a mutable
+        // reference pointing to an immutable (zero-sized) allocation. We rely on the promotion
+        // analysis not screwing up to ensure that it is sound to intern promoteds as immutable.
         if intern_kind != InternKind::Promoted
             && inner_mutability == Mutability::Not
-            && !prov.immutable()
+            && !prov.shared_ref()
         {
-            if ecx.tcx.try_get_global_alloc(alloc_id).is_some()
-                && !just_interned.contains(&alloc_id)
-            {
+            let is_already_global = ecx.tcx.try_get_global_alloc(alloc_id).is_some();
+            if is_already_global && !just_interned.contains(&alloc_id) {
                 // This is a pointer to some memory from another constant. We encounter mutable
                 // pointers to such memory since we do not always track immutability through
                 // these "global" pointers. Allowing them is harmless; the point of these checks
                 // during interning is to justify why we intern the *new* allocations immutably,
-                // so we can completely ignore existing allocations. We also don't need to add
-                // this to the todo list, since after all it is already interned.
+                // so we can completely ignore existing allocations.
+                // We can also skip the rest of this loop iteration, since after all it is already
+                // interned.
                 continue;
             }
-            // Found a mutable pointer inside a const where inner allocations should be
-            // immutable. We exclude promoteds from this, since things like `&mut []` and
-            // `&None::<Cell<i32>>` lead to promotion that can produce mutable pointers. We rely
-            // on the promotion analysis not screwing up to ensure that it is sound to intern
-            // promoteds as immutable.
-            trace!("found bad mutable pointer");
-            // Prefer dangling pointer errors over mutable pointer errors
-            if result.is_ok() {
-                result = Err(InternResult::FoundBadMutablePointer);
+            // If this is a dangling pointer, that's actually fine -- the problematic case is
+            // when there is memory there that someone might expect to be mutable, but we make it immutable.
+            let dangling = !is_already_global && !ecx.memory.alloc_map.contains_key(&alloc_id);
+            if !dangling {
+                // Found a mutable reference inside a const where inner allocations should be
+                // immutable.
+                if !ecx.tcx.sess.opts.unstable_opts.unleash_the_miri_inside_of_you {
+                    span_bug!(
+                        ecx.tcx.span,
+                        "the static const safety checks accepted mutable references they should not have accepted"
+                    );
+                }
+                // Prefer dangling pointer errors over mutable pointer errors
+                if result.is_ok() {
+                    result = Err(InternResult::FoundBadMutablePointer);
+                }
             }
         }
         if ecx.tcx.try_get_global_alloc(alloc_id).is_some() {
             // Already interned.
             debug_assert!(!ecx.memory.alloc_map.contains_key(&alloc_id));
             continue;
         }
-        just_interned.insert(alloc_id);
         // We always intern with `inner_mutability`, and furthermore we ensured above that if
         // that is "immutable", then there are *no* mutable pointers anywhere in the newly
         // interned memory -- justifying that we can indeed intern immutably. However this also
@@ -272,6 +287,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
         // pointers before deciding which allocations can be made immutable; but for now we are
         // okay with losing some potential for immutability here. This can anyway only affect
         // `static mut`.
+        just_interned.insert(alloc_id);
         match intern_shallow(ecx, alloc_id, inner_mutability) {
             Ok(nested) => todo.extend(nested),
             Err(()) => {

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -14,14 +14,14 @@ use hir::def::DefKind;
 use rustc_ast::Mutability;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
-use rustc_middle::bug;
 use rustc_middle::mir::interpret::ValidationErrorKind::{self, *};
 use rustc_middle::mir::interpret::{
     alloc_range, ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance,
     UnsupportedOpInfo, ValidationErrorInfo,
 };
 use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout};
 use rustc_middle::ty::{self, Ty, TyCtxt};
+use rustc_middle::{bug, span_bug};
 use rustc_span::symbol::{sym, Symbol};
 use rustc_target::abi::{
     Abi, FieldIdx, FieldsShape, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange,
@@ -617,6 +617,13 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
                         if ptr_expected_mutbl == Mutability::Mut
                             && alloc_actual_mutbl == Mutability::Not
                         {
+                            if !self.ecx.tcx.sess.opts.unstable_opts.unleash_the_miri_inside_of_you
+                            {
+                                span_bug!(
+                                    self.ecx.tcx.span,
+                                    "the static const safety checks accepted mutable references they should not have accepted"
+                                );
+                            }
                             throw_validation_failure!(self.path, MutableRefToImmutable);
                         }
                         // In a const, everything must be completely immutable.

compiler/rustc_lint/src/lib.rs

@@ -576,6 +576,10 @@ fn register_builtins(store: &mut LintStore) {
          <https://github.com/rust-lang/rust/issues/107457> for more information",
     );
     store.register_removed("writes_through_immutable_pointer", "converted into hard error");
+    store.register_removed(
+        "const_eval_mutable_ptr_in_final_value",
+        "partially allowed now, otherwise turned into a hard error",
+    );
 }
 
 fn register_internals(store: &mut LintStore) {

compiler/rustc_lint_defs/src/builtin.rs

@@ -29,7 +29,6 @@ declare_lint_pass! {
         CENUM_IMPL_DROP_CAST,
         COHERENCE_LEAK_CHECK,
         CONFLICTING_REPR_HINTS,
-        CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
         CONST_EVALUATABLE_UNCHECKED,
         CONST_ITEM_MUTATION,
         DEAD_CODE,
@@ -2804,51 +2803,6 @@ declare_lint! {
     @feature_gate = strict_provenance;
 }
 
-declare_lint! {
-    /// The `const_eval_mutable_ptr_in_final_value` lint detects if a mutable pointer
-    /// has leaked into the final value of a const expression.
-    ///
-    /// ### Example
-    ///
-    /// ```rust
-    /// pub enum JsValue {
-    ///     Undefined,
-    ///     Object(std::cell::Cell<bool>),
-    /// }
-    ///
-    /// impl ::std::ops::Drop for JsValue {
-    ///     fn drop(&mut self) {}
-    /// }
-    ///
-    /// const UNDEFINED: &JsValue = &JsValue::Undefined;
-    ///
-    /// fn main() {
-    /// }
-    /// ```
-    ///
-    /// {{produces}}
-    ///
-    /// ### Explanation
-    ///
-    /// In the 1.77 release, the const evaluation machinery adopted some
-    /// stricter rules to reject expressions with values that could
-    /// end up holding mutable references to state stored in static memory
-    /// (which is inherently immutable).
-    ///
-    /// This is a [future-incompatible] lint to ease the transition to an error.
-    /// See [issue #122153] for more details.
-    ///
-    /// [issue #122153]: https://github.com/rust-lang/rust/issues/122153
-    /// [future-incompatible]: ../index.md#future-incompatible-lints
-    pub CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
-    Warn,
-    "detects a mutable pointer that has leaked into final value of a const expression",
-    @future_incompatible = FutureIncompatibleInfo {
-        reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
-        reference: "issue #122153 <https://github.com/rust-lang/rust/issues/122153>",
-    };
-}
-
 declare_lint! {
     /// The `const_evaluatable_unchecked` lint detects a generic constant used
     /// in a type.