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 https://github.com/rust-lang/rust/issues/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 https://github.com/rust-lang/rust/pull/142575 for zero-sized copies on the read side, so I fixed that and added a test.

r? `@oli-obk`

3 files changed, 40 insertions, 18 deletions

diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs
index a68dcf29988..2ec3f8432c7 100644
--- a/compiler/rustc_const_eval/src/const_eval/machine.rs
+++ b/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(())
diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs
index b9e022c9604..d6d230fbd17 100644
--- a/compiler/rustc_const_eval/src/interpret/machine.rs
+++ b/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(())
     }
 
diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs
index 57bf867e389..69fceb02ff9 100644
--- a/compiler/rustc_const_eval/src/interpret/memory.rs
+++ b/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),