Forbid references to statics in valtrees

Author: oli-obk

compiler/rustc_const_eval/messages.ftl

@@ -310,6 +310,8 @@ const_eval_realloc_or_alloc_with_offset =
 
 const_eval_recursive_static = encountered static that tried to initialize itself with itself
 
+const_eval_ref_to_static = encountered a reference pointing to a static variable in a constant
+
 const_eval_remainder_by_zero =
     calculating the remainder with a divisor of zero
 const_eval_remainder_overflow =
@@ -399,7 +401,6 @@ const_eval_unwind_past_top =
 
 ## The `front_matter`s here refer to either `const_eval_front_matter_invalid_value` or `const_eval_front_matter_invalid_value_with_path`.
 ## (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`

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -80,6 +80,8 @@ pub enum CheckAlignment {
 
 #[derive(Copy, Clone, PartialEq)]
 pub(crate) enum GlobalAccessPermissions {
+    /// Not allowed to read from static items at all
+    Nothing,
     /// Allowed to read from immutable statics
     Static,
     /// Allowed to read from mutable statics
@@ -686,7 +688,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
         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();
@@ -697,19 +699,26 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
                 Mutability::Mut => Err(ConstEvalErrKind::ModifiedGlobal.into()),
             }
         } else {
-            // Read access. These are usually allowed, with some exceptions.
+            let check_mutability = || match alloc.mutability {
+                Mutability::Mut => {
+                    // Machine configuration does not allow us to read statics (e.g., `const`
+                    // initializer).
+                    Err(ConstEvalErrKind::ConstAccessesMutGlobal.into())
+                }
+                // Immutable global, this read is fine.
+                Mutability::Not => Ok(()),
+            };
             match machine.global_access_permissions {
                 // Machine configuration allows us read from anything (e.g., `static` initializer).
                 GlobalAccessPermissions::StaticMut => Ok(()),
-                GlobalAccessPermissions::Static => match alloc.mutability {
-                    Mutability::Mut => {
-                        // Machine configuration does not allow us to read statics (e.g., `const`
-                        // initializer).
+                GlobalAccessPermissions::Static => check_mutability(),
+                GlobalAccessPermissions::Nothing => {
+                    if static_def_id.is_none() {
+                        check_mutability()
+                    } else {
                         Err(ConstEvalErrKind::ConstAccessesMutGlobal.into())
                     }
-                    // Immutable global, this read is fine.
-                    Mutability::Not => Ok(()),
-                },
+                }
             }
         }
     }

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -31,6 +31,8 @@ pub(crate) enum ValTreeCreationError {
     NodesOverflow,
     /// Values of this type, or this particular value, are not supported as valtrees.
     NonSupportedType,
+    /// Tried to create a valtree with a reference to a static.
+    StaticRef,
 }
 pub(crate) type ValTreeCreationResult<'tcx> = Result<ty::ValTree<'tcx>, ValTreeCreationError>;
 

compiler/rustc_const_eval/src/const_eval/valtrees.rs

@@ -13,6 +13,7 @@ use super::machine::CompileTimeInterpCx;
 use super::{ValTreeCreationError, ValTreeCreationResult, VALTREE_MAX_NODES};
 use crate::const_eval::GlobalAccessPermissions;
 use crate::errors::MaxNumNodesInConstErr;
+use crate::errors::StaticRefErr;
 use crate::interpret::MPlaceTy;
 use crate::interpret::{
     intern_const_alloc_recursive, ImmTy, Immediate, InternKind, MemPlaceMeta, MemoryKind, PlaceTy,
@@ -138,6 +139,16 @@ fn const_to_valtree_inner<'tcx>(
 
         ty::Ref(_, _, _)  => {
             let derefd_place = ecx.deref_pointer(place)?;
+            if let Some(prov) = derefd_place.ptr().provenance {
+                match ecx.tcx.global_alloc(prov.alloc_id()) {
+                    mir::interpret::GlobalAlloc::Function(_) => unreachable!(),
+                    mir::interpret::GlobalAlloc::VTable(_, _) => unreachable!(),
+                    mir::interpret::GlobalAlloc::Static(_) => {
+                        return Err(ValTreeCreationError::StaticRef);
+                    },
+                    mir::interpret::GlobalAlloc::Memory(_) => {},
+                }
+            }
             const_to_valtree_inner(ecx, &derefd_place, num_nodes)
         }
 
@@ -236,7 +247,7 @@ pub(crate) fn eval_to_valtree<'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_to_read_const_val(
+    let mut ecx = mk_eval_cx_to_read_const_val(
         tcx,
         DUMMY_SP,
         param_env,
@@ -248,6 +259,14 @@ pub(crate) fn eval_to_valtree<'tcx>(
     debug!(?place);
 
     let mut num_nodes = 0;
+
+    // To preserve provenance of static items, it is crucial that we do not
+    // treat them as constants and just read their values.
+    // This flag will cause valtree creation to ICE if a reference to a static
+    // is read, so valtree creation needs to eagerly catch those cases and handle
+    // them in custom ways. Currently by reporting a hard error, but we can opt to
+    // create a special valtree node for statics in the future.
+    ecx.machine.global_access_permissions = GlobalAccessPermissions::Nothing;
     let valtree_result = const_to_valtree_inner(&ecx, &place, &mut num_nodes);
 
     match valtree_result {
@@ -262,6 +281,10 @@ pub(crate) fn eval_to_valtree<'tcx>(
                         tcx.dcx().emit_err(MaxNumNodesInConstErr { span, global_const_id });
                     Err(handled.into())
                 }
+                ValTreeCreationError::StaticRef => {
+                    let handled = tcx.dcx().emit_err(StaticRefErr { span });
+                    Err(handled.into())
+                }
                 ValTreeCreationError::NonSupportedType => Ok(None),
             }
         }

compiler/rustc_const_eval/src/errors.rs

@@ -126,6 +126,13 @@ pub(crate) struct MaxNumNodesInConstErr {
     pub global_const_id: String,
 }
 
+#[derive(Diagnostic)]
+#[diag(const_eval_ref_to_static)]
+pub(crate) struct StaticRefErr {
+    #[primary_span]
+    pub span: Option<Span>,
+}
+
 #[derive(Diagnostic)]
 #[diag(const_eval_unallowed_fn_pointer_call)]
 pub(crate) struct UnallowedFnPointerCall {

tests/ui/consts/const_refs_to_static.rs

@@ -14,6 +14,4 @@ const C2: *const i32 = unsafe { std::ptr::addr_of!(S_MUT) };
 fn main() {
     assert_eq!(*C1, 0);
     assert_eq!(unsafe { *C2 }, 0);
-    // Computing this pattern will read from an immutable static. That's fine.
-    assert!(matches!(&0, C1));
 }

tests/ui/consts/const_refs_to_static_fail_invalid.rs

@@ -48,4 +48,20 @@ fn mutable() {
     }
 }
 
+fn immutable() {
+    static S: i32 = 0;
+
+    const C: &i32 = unsafe { &S };
+    //~^ ERROR: encountered a reference pointing to a static variable in a constant
+
+    // This could be ok, but we'd need to teach valtrees to support
+    // preserving statics, because valtree creation is shared with
+    // const generics, which can't just erase the information about
+    // the static's address.
+    match &42 {
+        C => {} //~ERROR: could not evaluate constant pattern
+        _ => {}
+    }
+}
+
 fn main() {}

tests/ui/consts/const_refs_to_static_fail_invalid.stderr

@@ -49,6 +49,18 @@ error: could not evaluate constant pattern
 LL |         C => {}
    |         ^
 
-error: aborting due to 6 previous errors
+error: encountered a reference pointing to a static variable in a constant
+  --> $DIR/const_refs_to_static_fail_invalid.rs:54:5
+   |
+LL |     const C: &i32 = unsafe { &S };
+   |     ^^^^^^^^^^^^^
+
+error: could not evaluate constant pattern
+  --> $DIR/const_refs_to_static_fail_invalid.rs:62:9
+   |
+LL |         C => {}
+   |         ^
+
+error: aborting due to 8 previous errors
 
 For more information about this error, try `rustc --explain E0080`.

tests/ui/statics/const_generics.rs

@@ -1,7 +1,6 @@
 //! check that we lose the information that `BAR` points to `FOO`
 //! when going through a const generic.
 
-//@ run-pass
 // With optimizations, LLVM will deduplicate the constant `X` whose
 // value is `&42` to just be a reference to the static. This is correct,
 // but obscures the issue we're trying to show.
@@ -19,4 +18,5 @@ fn foo<const X: &'static usize>() {
 
 fn main() {
     foo::<BAR>();
+    //~^ ERROR: encountered a reference pointing to a static variable in a constant
 }

tests/ui/statics/const_generics.stderr

@@ -0,0 +1,8 @@
+error: encountered a reference pointing to a static variable in a constant
+  --> $DIR/const_generics.rs:20:11
+   |
+LL |     foo::<BAR>();
+   |           ^^^
+
+error: aborting due to 1 previous error
+