Unrolled build for #143084

Rollup merge of #143084 - RalfJung:const-eval-recursive-static-write, r=oli-obk

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

Fixes #142404 by also calling the relevant hook for writes, not just reads. To avoid erroring during the actual write of the initial value, we neuter the hook when popping the final stack frame.

Calling the hook during writes requires changing its signature since we cannot pass in the entire interpreter any more.

While doing this I also realized a gap in #142575 for zero-sized copies on the read side, so I fixed that and added a test.

r? `@oli-obk`

Author: rust-timer

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,7 +62,7 @@ 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)>,
 
     /// A cache of "data range" computations for unions (i.e., the offsets of non-padding bytes).
@@ -705,19 +705,27 @@ 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.stack.is_empty() {
+            // 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(())

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() {}