const-eval: error when initializing a static writes to that static

Author: RalfJung

compiler/rustc_const_eval/messages.ftl

@@ -352,7 +352,7 @@ const_eval_realloc_or_alloc_with_offset =
         *[other] {""}
     } {$ptr} which does not point to the beginning of an object
 
-const_eval_recursive_static = encountered static that tried to initialize itself with itself
+const_eval_recursive_static = encountered static that tried to access itself during initialization
 
 const_eval_remainder_by_zero =
     calculating the remainder with a divisor of zero

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -62,9 +62,13 @@ pub struct CompileTimeMachine<'tcx> {
 
     /// If `Some`, we are evaluating the initializer of the static with the given `LocalDefId`,
     /// storing the result in the given `AllocId`.
-    /// Used to prevent reads from a static's base allocation, as that may allow for self-initialization loops.
+    /// Used to prevent accesses to a static's base allocation, as that may allow for self-initialization loops.
     pub(crate) static_root_ids: Option<(AllocId, LocalDefId)>,
 
+    /// Set to `true` when the last stack frame gets popped, at which point we have to allow
+    /// accesses to `static_root_ids` for the final return value copy.
+    last_frame_popped: bool,
+
     /// A cache of "data range" computations for unions (i.e., the offsets of non-padding bytes).
     union_data_ranges: FxHashMap<Ty<'tcx>, RangeSet>,
 }
@@ -101,6 +105,7 @@ impl<'tcx> CompileTimeMachine<'tcx> {
             can_access_mut_global,
             check_alignment,
             static_root_ids: None,
+            last_frame_popped: false,
             union_data_ranges: FxHashMap::default(),
         }
     }
@@ -705,24 +710,40 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
         interp_ok(())
     }
 
-    fn before_alloc_read(ecx: &InterpCx<'tcx, Self>, alloc_id: AllocId) -> InterpResult<'tcx> {
+    fn before_alloc_access(
+        tcx: TyCtxtAt<'tcx>,
+        machine: &Self,
+        alloc_id: AllocId,
+    ) -> InterpResult<'tcx> {
+        if machine.last_frame_popped {
+            // Get out of the way for the final copy.
+            return interp_ok(());
+        }
         // Check if this is the currently evaluated static.
-        if Some(alloc_id) == ecx.machine.static_root_ids.map(|(id, _)| id) {
+        if Some(alloc_id) == machine.static_root_ids.map(|(id, _)| id) {
             return Err(ConstEvalErrKind::RecursiveStatic).into();
         }
         // If this is another static, make sure we fire off the query to detect cycles.
         // But only do that when checks for static recursion are enabled.
-        if ecx.machine.static_root_ids.is_some() {
-            if let Some(GlobalAlloc::Static(def_id)) = ecx.tcx.try_get_global_alloc(alloc_id) {
-                if ecx.tcx.is_foreign_item(def_id) {
+        if machine.static_root_ids.is_some() {
+            if let Some(GlobalAlloc::Static(def_id)) = tcx.try_get_global_alloc(alloc_id) {
+                if tcx.is_foreign_item(def_id) {
                     throw_unsup!(ExternStatic(def_id));
                 }
-                ecx.ctfe_query(|tcx| tcx.eval_static_initializer(def_id))?;
+                tcx.eval_static_initializer(def_id)?;
             }
         }
         interp_ok(())
     }
 
+    fn before_stack_pop(ecx: &mut InterpCx<'tcx, Self>) -> InterpResult<'tcx> {
+        // Remember when we are popping the last stack frame.
+        if ecx.stack().len() == 1 {
+            ecx.machine.last_frame_popped = true;
+        }
+        interp_ok(())
+    }
+
     fn cached_union_data_range<'e>(
         ecx: &'e mut InterpCx<'tcx, Self>,
         ty: Ty<'tcx>,

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -443,7 +443,11 @@ pub trait Machine<'tcx>: Sized {
     ///
     /// Used to prevent statics from self-initializing by reading from their own memory
     /// as it is being initialized.
-    fn before_alloc_read(_ecx: &InterpCx<'tcx, Self>, _alloc_id: AllocId) -> InterpResult<'tcx> {
+    fn before_alloc_access(
+        _tcx: TyCtxtAt<'tcx>,
+        _machine: &Self,
+        _alloc_id: AllocId,
+    ) -> InterpResult<'tcx> {
         interp_ok(())
     }
 

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -720,7 +720,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         // do this after `check_and_deref_ptr` to ensure some basic sanity has already been checked.
         if !self.memory.validation_in_progress.get() {
             if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(ptr, size_i64) {
-                M::before_alloc_read(self, alloc_id)?;
+                M::before_alloc_access(self.tcx, &self.machine, alloc_id)?;
             }
         }
 
@@ -821,6 +821,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         if let Some((alloc_id, offset, prov, alloc, machine)) = ptr_and_alloc {
             let range = alloc_range(offset, size);
             if !validation_in_progress {
+                // For writes, it's okay to only call those when there actually is a non-zero
+                // amount of bytes to be written: a zero-sized write doesn't manifest anything.
+                M::before_alloc_access(tcx, machine, alloc_id)?;
                 M::before_memory_write(
                     tcx,
                     machine,
@@ -1396,6 +1399,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         let src_parts = self.get_ptr_access(src, size)?;
         let dest_parts = self.get_ptr_access(dest, size * num_copies)?; // `Size` multiplication
 
+        // Similar to `get_ptr_alloc`, we need to call `before_alloc_access` even for zero-sized
+        // reads. However, just like in `get_ptr_alloc_mut`, the write part is okay to skip for
+        // zero-sized writes.
+        if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(src, size.bytes().try_into().unwrap())
+        {
+            M::before_alloc_access(tcx, &self.machine, alloc_id)?;
+        }
+
         // FIXME: we look up both allocations twice here, once before for the `check_ptr_access`
         // and once below to get the underlying `&[mut] Allocation`.
 
@@ -1408,12 +1419,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         let src_range = alloc_range(src_offset, size);
         assert!(!self.memory.validation_in_progress.get(), "we can't be copying during validation");
 
-        // Trigger read hooks.
-        // For the overlapping case, it is crucial that we trigger the read hooks
+        // Trigger read hook.
+        // For the overlapping case, it is crucial that we trigger the read hook
         // before the write hook -- the aliasing model cares about the order.
-        if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(src, size.bytes() as i64) {
-            M::before_alloc_read(self, alloc_id)?;
-        }
         M::before_memory_read(
             tcx,
             &self.machine,
@@ -1438,16 +1446,18 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         let provenance = src_alloc
             .provenance()
             .prepare_copy(src_range, dest_offset, num_copies, self)
-            .map_err(|e| e.to_interp_error(dest_alloc_id))?;
+            .map_err(|e| e.to_interp_error(src_alloc_id))?;
         // Prepare a copy of the initialization mask.
         let init = src_alloc.init_mask().prepare_copy(src_range);
 
-        // Destination alloc preparations and access hooks.
-        let (dest_alloc, extra) = self.get_alloc_raw_mut(dest_alloc_id)?;
+        // Destination alloc preparations...
+        let (dest_alloc, machine) = self.get_alloc_raw_mut(dest_alloc_id)?;
         let dest_range = alloc_range(dest_offset, size * num_copies);
+        // ...and access hooks.
+        M::before_alloc_access(tcx, machine, dest_alloc_id)?;
         M::before_memory_write(
             tcx,
-            extra,
+            machine,
             &mut dest_alloc.extra,
             dest,
             (dest_alloc_id, dest_prov),

tests/ui/consts/recursive-static-write.rs

@@ -0,0 +1,24 @@
+//! Ensure that writing to `S` while initializing `S` errors.
+//! Regression test for <https://github.com/rust-lang/rust/issues/142404>.
+#![allow(dead_code)]
+
+struct Foo {
+    x: i32,
+    y: (),
+}
+
+static S: Foo = Foo {
+    x: 0,
+    y: unsafe {
+        (&raw const S.x).cast_mut().write(1); //~ERROR access itself during initialization
+    },
+};
+
+static mut S2: Foo = Foo {
+    x: 0,
+    y: unsafe {
+        S2.x = 1; //~ERROR access itself during initialization
+    },
+};
+
+fn main() {}

tests/ui/consts/recursive-static-write.stderr

@@ -0,0 +1,15 @@
+error[E0080]: encountered static that tried to access itself during initialization
+  --> $DIR/recursive-static-write.rs:13:9
+   |
+LL |         (&raw const S.x).cast_mut().write(1);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `S` failed here
+
+error[E0080]: encountered static that tried to access itself during initialization
+  --> $DIR/recursive-static-write.rs:20:9
+   |
+LL |         S2.x = 1;
+   |         ^^^^^^^^ evaluation of `S2` failed here
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/recursive-zst-static.default.stderr

@@ -1,20 +1,20 @@
-error[E0080]: encountered static that tried to initialize itself with itself
+error[E0080]: encountered static that tried to access itself during initialization
   --> $DIR/recursive-zst-static.rs:10:18
    |
 LL | static FOO: () = FOO;
    |                  ^^^ evaluation of `FOO` failed here
 
 error[E0391]: cycle detected when evaluating initializer of static `A`
-  --> $DIR/recursive-zst-static.rs:13:16
+  --> $DIR/recursive-zst-static.rs:13:1
    |
 LL | static A: () = B;
-   |                ^
+   | ^^^^^^^^^^^^
    |
 note: ...which requires evaluating initializer of static `B`...
-  --> $DIR/recursive-zst-static.rs:14:16
+  --> $DIR/recursive-zst-static.rs:14:1
    |
 LL | static B: () = A;
-   |                ^
+   | ^^^^^^^^^^^^
    = note: ...which again requires evaluating initializer of static `A`, completing the cycle
    = note: cycle used when running analysis passes on this crate
    = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

tests/ui/consts/recursive-zst-static.rs

@@ -8,7 +8,7 @@
 // See https://github.com/rust-lang/rust/issues/71078 for more details.
 
 static FOO: () = FOO;
-//~^ ERROR encountered static that tried to initialize itself with itself
+//~^ ERROR encountered static that tried to access itself during initialization
 
 static A: () = B; //~ ERROR cycle detected when evaluating initializer of static `A`
 static B: () = A;

tests/ui/consts/recursive-zst-static.unleash.stderr

@@ -1,20 +1,20 @@
-error[E0080]: encountered static that tried to initialize itself with itself
+error[E0080]: encountered static that tried to access itself during initialization
   --> $DIR/recursive-zst-static.rs:10:18
    |
 LL | static FOO: () = FOO;
    |                  ^^^ evaluation of `FOO` failed here
 
 error[E0391]: cycle detected when evaluating initializer of static `A`
-  --> $DIR/recursive-zst-static.rs:13:16
+  --> $DIR/recursive-zst-static.rs:13:1
    |
 LL | static A: () = B;
-   |                ^
+   | ^^^^^^^^^^^^
    |
 note: ...which requires evaluating initializer of static `B`...
-  --> $DIR/recursive-zst-static.rs:14:16
+  --> $DIR/recursive-zst-static.rs:14:1
    |
 LL | static B: () = A;
-   |                ^
+   | ^^^^^^^^^^^^
    = note: ...which again requires evaluating initializer of static `A`, completing the cycle
    = note: cycle used when running analysis passes on this crate
    = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

tests/ui/consts/write-to-static-mut-in-static.rs

@@ -3,8 +3,9 @@ pub static mut B: () = unsafe { A = 1; };
 //~^ ERROR modifying a static's initial value
 
 pub static mut C: u32 = unsafe { C = 1; 0 };
+//~^ ERROR static that tried to access itself during initialization
 
 pub static D: u32 = D;
-//~^ ERROR static that tried to initialize itself with itself
+//~^ ERROR static that tried to access itself during initialization
 
 fn main() {}