Auto merge of #119614 - RalfJung:const-refs-to-static, r=oli-obk

unstably allow constants to refer to statics and read from immutable statics

I am not aware of any fundamental reason why we cannot allow constants to mention statics. What we really need is that constants do not *read from* statics that can change their value:
- This would break the principle that "constants behave as-if their expression was inlined everywhere and executed at runtime". This is enforced by halting const-eval interpretation when a read from a mutable global occurs.
- When building a valtree we want to be sure that the constant and everything it refers to is truly immutable. This is enforced by aborting valtree construction when a read from a mutable global occurs.

r? `@oli-obk` -- if you are okay with experimenting with this feature, I will create a tracking issue.
Based on and blocked on #119044; only the last commit is new.

compiler/rustc_const_eval/messages.ftl

@@ -30,7 +30,9 @@ const_eval_closure_non_const =
     cannot call non-const closure in {const_eval_const_context}s
 const_eval_consider_dereferencing =
     consider dereferencing here
-const_eval_const_accesses_static = constant accesses static
+
+const_eval_const_accesses_mut_global =
+    constant accesses mutable global memory
 
 const_eval_const_context = {$kind ->
     [const] constant
@@ -319,12 +321,6 @@ const_eval_size_overflow =
 const_eval_stack_frame_limit_reached =
     reached the configured maximum number of stack frames
 
-const_eval_static_access =
-    {const_eval_const_context}s cannot refer to statics
-    .help = consider extracting the value of the `static` to a `const`, and referring to that
-    .teach_note = `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable.
-    .teach_help = To fix this, the value can be extracted to a `const` and then used.
-
 const_eval_thread_local_access =
     thread-local statics cannot be accessed at compile-time
 
@@ -415,6 +411,10 @@ const_eval_upcast_mismatch =
 ## (We'd love to sort this differently to make that more clear but tidy won't let us...)
 const_eval_validation_box_to_static = {$front_matter}: encountered a box pointing to a static variable in a constant
 const_eval_validation_box_to_uninhabited = {$front_matter}: encountered a box pointing to uninhabited type {$ty}
+
+const_eval_validation_const_ref_to_extern = {$front_matter}: encountered reference to `extern` static in `const`
+const_eval_validation_const_ref_to_mutable = {$front_matter}: encountered reference to mutable memory in `const`
+
 const_eval_validation_dangling_box_no_provenance = {$front_matter}: encountered a dangling box ({$pointer} has no provenance)
 const_eval_validation_dangling_box_out_of_bounds = {$front_matter}: encountered a dangling box (going beyond the bounds of its allocation)
 const_eval_validation_dangling_box_use_after_free = {$front_matter}: encountered a dangling box (use-after-free)

compiler/rustc_const_eval/src/const_eval/error.rs

@@ -17,7 +17,7 @@ use crate::interpret::{ErrorHandled, InterpError, InterpErrorInfo, MachineStopTy
 /// The CTFE machine has some custom error kinds.
 #[derive(Clone, Debug)]
 pub enum ConstEvalErrKind {
-    ConstAccessesStatic,
+    ConstAccessesMutGlobal,
     ModifiedGlobal,
     AssertFailure(AssertKind<ConstInt>),
     Panic { msg: Symbol, line: u32, col: u32, file: Symbol },
@@ -28,7 +28,7 @@ impl MachineStopType for ConstEvalErrKind {
         use crate::fluent_generated::*;
         use ConstEvalErrKind::*;
         match self {
-            ConstAccessesStatic => const_eval_const_accesses_static,
+            ConstAccessesMutGlobal => const_eval_const_accesses_mut_global,
             ModifiedGlobal => const_eval_modified_global,
             Panic { .. } => const_eval_panic,
             AssertFailure(x) => x.diagnostic_message(),
@@ -37,7 +37,7 @@ impl MachineStopType for ConstEvalErrKind {
     fn add_args(self: Box<Self>, adder: &mut dyn FnMut(DiagnosticArgName, DiagnosticArgValue)) {
         use ConstEvalErrKind::*;
         match *self {
-            ConstAccessesStatic | ModifiedGlobal => {}
+            ConstAccessesMutGlobal | ModifiedGlobal => {}
             AssertFailure(kind) => kind.add_args(adder),
             Panic { msg, line, col, file } => {
                 adder("msg".into(), msg.into_diagnostic_arg());

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -11,7 +11,7 @@ use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::Span;
 use rustc_target::abi::{self, Abi};
 
-use super::{CanAccessStatics, CompileTimeEvalContext, CompileTimeInterpreter};
+use super::{CanAccessMutGlobal, CompileTimeEvalContext, CompileTimeInterpreter};
 use crate::const_eval::CheckAlignment;
 use crate::errors;
 use crate::errors::ConstEvalError;
@@ -90,14 +90,14 @@ pub(crate) fn mk_eval_cx<'mir, 'tcx>(
     tcx: TyCtxt<'tcx>,
     root_span: Span,
     param_env: ty::ParamEnv<'tcx>,
-    can_access_statics: CanAccessStatics,
+    can_access_mut_global: CanAccessMutGlobal,
 ) -> CompileTimeEvalContext<'mir, 'tcx> {
     debug!("mk_eval_cx: {:?}", param_env);
     InterpCx::new(
         tcx,
         root_span,
         param_env,
-        CompileTimeInterpreter::new(can_access_statics, CheckAlignment::No),
+        CompileTimeInterpreter::new(can_access_mut_global, CheckAlignment::No),
     )
 }
 
@@ -200,7 +200,7 @@ pub(crate) fn turn_into_const_value<'tcx>(
         tcx,
         tcx.def_span(key.value.instance.def_id()),
         key.param_env,
-        CanAccessStatics::from(is_static),
+        CanAccessMutGlobal::from(is_static),
     );
 
     let mplace = ecx.raw_const_to_mplace(constant).expect(
@@ -277,9 +277,11 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
         tcx,
         tcx.def_span(def),
         key.param_env,
-        // Statics (and promoteds inside statics) may access other statics, because unlike consts
+        // Statics (and promoteds inside statics) may access mutable global memory, because unlike consts
         // they do not have to behave "as if" they were evaluated at runtime.
-        CompileTimeInterpreter::new(CanAccessStatics::from(is_static), CheckAlignment::Error),
+        // For consts however we want to ensure they behave "as if" they were evaluated at runtime,
+        // so we have to reject reading mutable global memory.
+        CompileTimeInterpreter::new(CanAccessMutGlobal::from(is_static), CheckAlignment::Error),
     );
     eval_in_interpreter(ecx, cid, is_static)
 }
@@ -358,15 +360,18 @@ pub fn const_validate_mplace<'mir, 'tcx>(
                 // Promoteds in statics are consts that re allowed to point to statics.
                 CtfeValidationMode::Const {
                     allow_immutable_unsafe_cell: false,
-                    allow_static_ptrs: true,
+                    allow_extern_static_ptrs: true,
                 }
             }
             Some(mutbl) => CtfeValidationMode::Static { mutbl }, // a `static`
             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 }
+                CtfeValidationMode::Const {
+                    allow_immutable_unsafe_cell,
+                    allow_extern_static_ptrs: false,
+                }
             }
         };
         ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?;

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -51,13 +51,10 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
     /// The virtual call stack.
     pub(super) stack: Vec<Frame<'mir, 'tcx>>,
 
-    /// We need to make sure consts never point to anything mutable, even recursively. That is
-    /// relied on for pattern matching on consts with references.
-    /// To achieve this, two pieces have to work together:
-    /// * Interning makes everything outside of statics immutable.
-    /// * Pointers to allocations inside of statics can never leak outside, to a non-static global.
-    /// This boolean here controls the second part.
-    pub(super) can_access_statics: CanAccessStatics,
+    /// Pattern matching on consts with references would be unsound if those references
+    /// could point to anything mutable. Therefore, when evaluating consts and when constructing valtrees,
+    /// we ensure that only immutable global memory can be accessed.
+    pub(super) can_access_mut_global: CanAccessMutGlobal,
 
     /// Whether to check alignment during evaluation.
     pub(super) check_alignment: CheckAlignment,
@@ -73,26 +70,26 @@ pub enum CheckAlignment {
 }
 
 #[derive(Copy, Clone, PartialEq)]
-pub(crate) enum CanAccessStatics {
+pub(crate) enum CanAccessMutGlobal {
     No,
     Yes,
 }
 
-impl From<bool> for CanAccessStatics {
+impl From<bool> for CanAccessMutGlobal {
     fn from(value: bool) -> Self {
         if value { Self::Yes } else { Self::No }
     }
 }
 
 impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
     pub(crate) fn new(
-        can_access_statics: CanAccessStatics,
+        can_access_mut_global: CanAccessMutGlobal,
         check_alignment: CheckAlignment,
     ) -> Self {
         CompileTimeInterpreter {
             num_evaluated_steps: 0,
             stack: Vec::new(),
-            can_access_statics,
+            can_access_mut_global,
             check_alignment,
         }
     }
@@ -680,7 +677,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         machine: &Self,
         alloc_id: AllocId,
         alloc: ConstAllocation<'tcx>,
-        static_def_id: Option<DefId>,
+        _static_def_id: Option<DefId>,
         is_write: bool,
     ) -> InterpResult<'tcx> {
         let alloc = alloc.inner();
@@ -692,22 +689,15 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
             }
         } else {
             // Read access. These are usually allowed, with some exceptions.
-            if machine.can_access_statics == CanAccessStatics::Yes {
+            if machine.can_access_mut_global == CanAccessMutGlobal::Yes {
                 // Machine configuration allows us read from anything (e.g., `static` initializer).
                 Ok(())
-            } else if static_def_id.is_some() {
-                // Machine configuration does not allow us to read statics
-                // (e.g., `const` initializer).
-                // See const_eval::machine::MemoryExtra::can_access_statics for why
-                // this check is so important: if we could read statics, we could read pointers
-                // to mutable allocations *inside* statics. These allocations are not themselves
-                // statics, so pointers to them can get around the check in `validity.rs`.
-                Err(ConstEvalErrKind::ConstAccessesStatic.into())
+            } else if alloc.mutability == Mutability::Mut {
+                // Machine configuration does not allow us to read statics (e.g., `const`
+                // initializer).
+                Err(ConstEvalErrKind::ConstAccessesMutGlobal.into())
             } else {
                 // Immutable global, this read is fine.
-                // But make sure we never accept a read from something mutable, that would be
-                // unsound. The reason is that as the content of this allocation may be different
-                // now and at run-time, so if we permit reading now we might return the wrong value.
                 assert_eq!(alloc.mutability, Mutability::Not);
                 Ok(())
             }

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -1,12 +1,11 @@
 // Not in interpret to make sure we do not use private implementation details
 
-use crate::errors::MaxNumNodesInConstErr;
-use crate::interpret::InterpCx;
 use rustc_middle::mir;
-use rustc_middle::mir::interpret::{EvalToValTreeResult, GlobalId};
+use rustc_middle::mir::interpret::InterpErrorInfo;
 use rustc_middle::query::TyCtxtAt;
-use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_span::DUMMY_SP;
+use rustc_middle::ty::{self, Ty};
+
+use crate::interpret::{format_interp_error, InterpCx};
 
 mod error;
 mod eval_queries;
@@ -18,56 +17,26 @@ pub use error::*;
 pub use eval_queries::*;
 pub use fn_queries::*;
 pub use machine::*;
-pub(crate) use valtrees::{const_to_valtree_inner, valtree_to_const_value};
+pub(crate) use valtrees::{eval_to_valtree, valtree_to_const_value};
 
 // We forbid type-level constants that contain more than `VALTREE_MAX_NODES` nodes.
 const VALTREE_MAX_NODES: usize = 100000;
 
 pub(crate) enum ValTreeCreationError {
     NodesOverflow,
+    /// Values of this type, or this particular value, are not supported as valtrees.
     NonSupportedType,
-    Other,
 }
 pub(crate) type ValTreeCreationResult<'tcx> = Result<ty::ValTree<'tcx>, ValTreeCreationError>;
 
-/// Evaluates a constant and turns it into a type-level constant value.
-pub(crate) fn eval_to_valtree<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    param_env: ty::ParamEnv<'tcx>,
-    cid: GlobalId<'tcx>,
-) -> EvalToValTreeResult<'tcx> {
-    let const_alloc = tcx.eval_to_allocation_raw(param_env.and(cid))?;
-
-    // FIXME Need to provide a span to `eval_to_valtree`
-    let ecx = mk_eval_cx(
-        tcx,
-        DUMMY_SP,
-        param_env,
-        // It is absolutely crucial for soundness that
-        // we do not read from static items or other mutable memory.
-        CanAccessStatics::No,
-    );
-    let place = ecx.raw_const_to_mplace(const_alloc).unwrap();
-    debug!(?place);
-
-    let mut num_nodes = 0;
-    let valtree_result = const_to_valtree_inner(&ecx, &place, &mut num_nodes);
-
-    match valtree_result {
-        Ok(valtree) => Ok(Some(valtree)),
-        Err(err) => {
-            let did = cid.instance.def_id();
-            let global_const_id = cid.display(tcx);
-            match err {
-                ValTreeCreationError::NodesOverflow => {
-                    let span = tcx.hir().span_if_local(did);
-                    tcx.dcx().emit_err(MaxNumNodesInConstErr { span, global_const_id });
-
-                    Ok(None)
-                }
-                ValTreeCreationError::NonSupportedType | ValTreeCreationError::Other => Ok(None),
-            }
-        }
+impl From<InterpErrorInfo<'_>> for ValTreeCreationError {
+    fn from(err: InterpErrorInfo<'_>) -> Self {
+        ty::tls::with(|tcx| {
+            bug!(
+                "Unexpected Undefined Behavior error during valtree construction: {}",
+                format_interp_error(tcx.dcx(), err),
+            )
+        })
     }
 }
 
@@ -78,7 +47,7 @@ pub(crate) fn try_destructure_mir_constant_for_user_output<'tcx>(
     ty: Ty<'tcx>,
 ) -> Option<mir::DestructuredConstant<'tcx>> {
     let param_env = ty::ParamEnv::reveal_all();
-    let ecx = mk_eval_cx(tcx.tcx, tcx.span, param_env, CanAccessStatics::No);
+    let ecx = mk_eval_cx(tcx.tcx, tcx.span, param_env, CanAccessMutGlobal::No);
     let op = ecx.const_val_to_op(val, ty, None).ok()?;
 
     // We go to `usize` as we cannot allocate anything bigger anyway.