Auto merge of #2566 - saethlin:gc-cleanup, r=oli-obk

Expand VisitMachineValues to cover more pointers in the interpreter

Follow-on to https://github.com/rust-lang/miri/pull/2559

This is making me want to write a proc macro :thinking:

r? `@RalfJung`

17 files changed, 464 insertions, 127 deletions

diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs
index 2e54ddaaba1..d0fc349f1ac 100644
--- a/src/tools/miri/src/concurrency/data_race.rs
+++ b/src/tools/miri/src/concurrency/data_race.rs
@@ -696,6 +696,12 @@ pub struct VClockAlloc {
     alloc_ranges: RefCell<RangeMap<MemoryCellClocks>>,
 }
 
+impl VisitTags for VClockAlloc {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {
+        // No tags here.
+    }
+}
+
 impl VClockAlloc {
     /// Create a new data-race detector for newly allocated memory.
     pub fn new_allocation(
@@ -1239,6 +1245,12 @@ pub struct GlobalState {
     pub track_outdated_loads: bool,
 }
 
+impl VisitTags for GlobalState {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {
+        // We don't have any tags.
+    }
+}
+
 impl GlobalState {
     /// Create a new global state, setup with just thread-id=0
     /// advanced to timestamp = 1.
diff --git a/src/tools/miri/src/concurrency/range_object_map.rs b/src/tools/miri/src/concurrency/range_object_map.rs
index 50d3f8c9b20..dfe2e9f05da 100644
--- a/src/tools/miri/src/concurrency/range_object_map.rs
+++ b/src/tools/miri/src/concurrency/range_object_map.rs
@@ -132,6 +132,10 @@ impl<T> RangeObjectMap<T> {
     pub fn remove_from_pos(&mut self, pos: Position) {
         self.v.remove(pos);
     }
+
+    pub fn iter(&self) -> impl Iterator<Item = &T> {
+        self.v.iter().map(|e| &e.data)
+    }
 }
 
 impl<T> Index<Position> for RangeObjectMap<T> {
diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs
index a9d144eff54..ec1da4138d4 100644
--- a/src/tools/miri/src/concurrency/thread.rs
+++ b/src/tools/miri/src/concurrency/thread.rs
@@ -32,9 +32,11 @@ pub enum SchedulingAction {
 
 /// Timeout callbacks can be created by synchronization primitives to tell the
 /// scheduler that they should be called once some period of time passes.
-type TimeoutCallback<'mir, 'tcx> = Box<
-    dyn FnOnce(&mut InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>) -> InterpResult<'tcx> + 'tcx,
->;
+pub trait MachineCallback<'mir, 'tcx>: VisitTags {
+    fn call(&self, ecx: &mut InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>) -> InterpResult<'tcx>;
+}
+
+type TimeoutCallback<'mir, 'tcx> = Box<dyn MachineCallback<'mir, 'tcx> + 'tcx>;
 
 /// A thread identifier.
 #[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
@@ -181,6 +183,46 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {
     }
 }
 
+impl VisitTags for Thread<'_, '_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let Thread { panic_payload, last_error, stack, state: _, thread_name: _, join_status: _ } =
+            self;
+
+        panic_payload.visit_tags(visit);
+        last_error.visit_tags(visit);
+        for frame in stack {
+            frame.visit_tags(visit)
+        }
+    }
+}
+
+impl VisitTags for Frame<'_, '_, Provenance, FrameData<'_>> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let Frame {
+            return_place,
+            locals,
+            extra,
+            body: _,
+            instance: _,
+            return_to_block: _,
+            loc: _,
+            // There are some private fields we cannot access; they contain no tags.
+            ..
+        } = self;
+
+        // Return place.
+        return_place.visit_tags(visit);
+        // Locals.
+        for local in locals.iter() {
+            if let LocalValue::Live(value) = &local.value {
+                value.visit_tags(visit);
+            }
+        }
+
+        extra.visit_tags(visit);
+    }
+}
+
 /// A specific moment in time.
 #[derive(Debug)]
 pub enum Time {
@@ -253,6 +295,29 @@ impl<'mir, 'tcx> Default for ThreadManager<'mir, 'tcx> {
     }
 }
 
+impl VisitTags for ThreadManager<'_, '_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let ThreadManager {
+            threads,
+            thread_local_alloc_ids,
+            timeout_callbacks,
+            active_thread: _,
+            yield_active_thread: _,
+            sync: _,
+        } = self;
+
+        for thread in threads {
+            thread.visit_tags(visit);
+        }
+        for ptr in thread_local_alloc_ids.borrow().values() {
+            ptr.visit_tags(visit);
+        }
+        for callback in timeout_callbacks.values() {
+            callback.callback.visit_tags(visit);
+        }
+    }
+}
+
 impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
     pub(crate) fn init(ecx: &mut MiriInterpCx<'mir, 'tcx>) {
         if ecx.tcx.sess.target.os.as_ref() != "windows" {
@@ -625,33 +690,6 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
     }
 }
 
-impl VisitMachineValues for ThreadManager<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
-        // FIXME some other fields also contain machine values
-        let ThreadManager { threads, .. } = self;
-
-        for thread in threads {
-            // FIXME: implement VisitMachineValues for `Thread` and `Frame` instead.
-            // In particular we need to visit the `last_error` and `catch_unwind` fields.
-            if let Some(payload) = thread.panic_payload {
-                visit(&Operand::Immediate(Immediate::Scalar(payload)))
-            }
-            for frame in &thread.stack {
-                // Return place.
-                if let Place::Ptr(mplace) = *frame.return_place {
-                    visit(&Operand::Indirect(mplace));
-                }
-                // Locals.
-                for local in frame.locals.iter() {
-                    if let LocalValue::Live(value) = &local.value {
-                        visit(value);
-                    }
-                }
-            }
-        }
-    }
-}
-
 // Public interface to thread management.
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
@@ -930,7 +968,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         // 2. Make the scheduler the only place that can change the active
         //    thread.
         let old_thread = this.set_active_thread(thread);
-        callback(this)?;
+        callback.call(this)?;
         this.set_active_thread(old_thread);
         Ok(())
     }
diff --git a/src/tools/miri/src/concurrency/weak_memory.rs b/src/tools/miri/src/concurrency/weak_memory.rs
index bac403e9ec7..9d7a49c0b43 100644
--- a/src/tools/miri/src/concurrency/weak_memory.rs
+++ b/src/tools/miri/src/concurrency/weak_memory.rs
@@ -108,6 +108,19 @@ pub struct StoreBufferAlloc {
     store_buffers: RefCell<RangeObjectMap<StoreBuffer>>,
 }
 
+impl VisitTags for StoreBufferAlloc {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let Self { store_buffers } = self;
+        for val in store_buffers
+            .borrow()
+            .iter()
+            .flat_map(|buf| buf.buffer.iter().map(|element| &element.val))
+        {
+            val.visit_tags(visit);
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(super) struct StoreBuffer {
     // Stores to this location in modification order
diff --git a/src/tools/miri/src/intptrcast.rs b/src/tools/miri/src/intptrcast.rs
index b9e5def8fa7..9722b7643e4 100644
--- a/src/tools/miri/src/intptrcast.rs
+++ b/src/tools/miri/src/intptrcast.rs
@@ -44,6 +44,12 @@ pub struct GlobalStateInner {
     provenance_mode: ProvenanceMode,
 }
 
+impl VisitTags for GlobalStateInner {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {
+        // Nothing to visit here.
+    }
+}
+
 impl GlobalStateInner {
     pub fn new(config: &MiriConfig) -> Self {
         GlobalStateInner {
diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs
index bf4e21319b5..463feb4dcc8 100644
--- a/src/tools/miri/src/lib.rs
+++ b/src/tools/miri/src/lib.rs
@@ -112,7 +112,7 @@ pub use crate::range_map::RangeMap;
 pub use crate::stacked_borrows::{
     CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, SbTag, Stack, Stacks,
 };
-pub use crate::tag_gc::{EvalContextExt as _, VisitMachineValues};
+pub use crate::tag_gc::{EvalContextExt as _, VisitTags};
 
 /// Insert rustc arguments at the beginning of the argument list that Miri wants to be
 /// set per default, for maximal validation power.
diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs
index 03df78c1af7..20ae908fce8 100644
--- a/src/tools/miri/src/machine.rs
+++ b/src/tools/miri/src/machine.rs
@@ -63,6 +63,15 @@ impl<'tcx> std::fmt::Debug for FrameData<'tcx> {
     }
 }
 
+impl VisitTags for FrameData<'_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let FrameData { catch_unwind, stacked_borrows, timing: _ } = self;
+
+        catch_unwind.visit_tags(visit);
+        stacked_borrows.visit_tags(visit);
+    }
+}
+
 /// Extra memory kinds
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum MiriMemoryKind {
@@ -251,6 +260,16 @@ pub struct AllocExtra {
     pub weak_memory: Option<weak_memory::AllocExtra>,
 }
 
+impl VisitTags for AllocExtra {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let AllocExtra { stacked_borrows, data_race, weak_memory } = self;
+
+        stacked_borrows.visit_tags(visit);
+        data_race.visit_tags(visit);
+        weak_memory.visit_tags(visit);
+    }
+}
+
 /// Precomputed layouts of primitive types
 pub struct PrimitiveLayouts<'tcx> {
     pub unit: TyAndLayout<'tcx>,
@@ -591,14 +610,65 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
     }
 }
 
-impl VisitMachineValues for MiriMachine<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
-        // FIXME: visit the missing fields: env vars, weak mem, the MemPlace fields in the machine,
-        // DirHandler, extern_statics, the Stacked Borrows base pointers; maybe more.
-        let MiriMachine { threads, tls, .. } = self;
-
-        threads.visit_machine_values(visit);
-        tls.visit_machine_values(visit);
+impl VisitTags for MiriMachine<'_, '_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        #[rustfmt::skip]
+        let MiriMachine {
+            threads,
+            tls,
+            env_vars,
+            argc,
+            argv,
+            cmd_line,
+            extern_statics,
+            dir_handler,
+            stacked_borrows,
+            data_race,
+            intptrcast,
+            file_handler,
+            tcx: _,
+            isolated_op: _,
+            validate: _,
+            enforce_abi: _,
+            clock: _,
+            layouts: _,
+            static_roots: _,
+            profiler: _,
+            string_cache: _,
+            exported_symbols_cache: _,
+            panic_on_unsupported: _,
+            backtrace_style: _,
+            local_crates: _,
+            rng: _,
+            tracked_alloc_ids: _,
+            check_alignment: _,
+            cmpxchg_weak_failure_rate: _,
+            mute_stdout_stderr: _,
+            weak_memory: _,
+            preemption_rate: _,
+            report_progress: _,
+            basic_block_count: _,
+            #[cfg(unix)]
+            external_so_lib: _,
+            gc_interval: _,
+            since_gc: _,
+            num_cpus: _,
+        } = self;
+
+        threads.visit_tags(visit);
+        tls.visit_tags(visit);
+        env_vars.visit_tags(visit);
+        dir_handler.visit_tags(visit);
+        file_handler.visit_tags(visit);
+        data_race.visit_tags(visit);
+        stacked_borrows.visit_tags(visit);
+        intptrcast.visit_tags(visit);
+        argc.visit_tags(visit);
+        argv.visit_tags(visit);
+        cmd_line.visit_tags(visit);
+        for ptr in extern_statics.values() {
+            ptr.visit_tags(visit);
+        }
     }
 }
 
diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs
index 95051c998e5..076d3878de2 100644
--- a/src/tools/miri/src/shims/env.rs
+++ b/src/tools/miri/src/shims/env.rs
@@ -36,6 +36,17 @@ pub struct EnvVars<'tcx> {
     pub(crate) environ: Option<MPlaceTy<'tcx, Provenance>>,
 }
 
+impl VisitTags for EnvVars<'_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let EnvVars { map, environ } = self;
+
+        environ.visit_tags(visit);
+        for ptr in map.values() {
+            ptr.visit_tags(visit);
+        }
+    }
+}
+
 impl<'tcx> EnvVars<'tcx> {
     pub(crate) fn init<'mir>(
         ecx: &mut InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>,
diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs
index 2e8245acf4a..698e025961d 100644
--- a/src/tools/miri/src/shims/panic.rs
+++ b/src/tools/miri/src/shims/panic.rs
@@ -35,6 +35,15 @@ pub struct CatchUnwindData<'tcx> {
     ret: mir::BasicBlock,
 }
 
+impl VisitTags for CatchUnwindData<'_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let CatchUnwindData { catch_fn, data, dest, ret: _ } = self;
+        catch_fn.visit_tags(visit);
+        data.visit_tags(visit);
+        dest.visit_tags(visit);
+    }
+}
+
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     /// Handles the special `miri_start_panic` intrinsic, which is called
diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs
index 24fe5245393..9f04034e1a1 100644
--- a/src/tools/miri/src/shims/time.rs
+++ b/src/tools/miri/src/shims/time.rs
@@ -1,5 +1,6 @@
 use std::time::{Duration, SystemTime};
 
+use crate::concurrency::thread::MachineCallback;
 use crate::*;
 
 /// Returns the time elapsed between the provided time and the unix epoch as a `Duration`.
@@ -218,10 +219,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.register_timeout_callback(
             active_thread,
             Time::Monotonic(timeout_time),
-            Box::new(move |ecx| {
-                ecx.unblock_thread(active_thread);
-                Ok(())
-            }),
+            Box::new(UnblockCallback { thread_to_unblock: active_thread }),
         );
 
         Ok(0)
@@ -244,12 +242,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.register_timeout_callback(
             active_thread,
             Time::Monotonic(timeout_time),
-            Box::new(move |ecx| {
-                ecx.unblock_thread(active_thread);
-                Ok(())
-            }),
+            Box::new(UnblockCallback { thread_to_unblock: active_thread }),
         );
 
         Ok(())
     }
 }
+
+struct UnblockCallback {
+    thread_to_unblock: ThreadId,
+}
+
+impl VisitTags for UnblockCallback {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {}
+}
+
+impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for UnblockCallback {
+    fn call(&self, ecx: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> {
+        ecx.unblock_thread(self.thread_to_unblock);
+        Ok(())
+    }
+}
diff --git a/src/tools/miri/src/shims/tls.rs b/src/tools/miri/src/shims/tls.rs
index d1cee307d77..430dedbc170 100644
--- a/src/tools/miri/src/shims/tls.rs
+++ b/src/tools/miri/src/shims/tls.rs
@@ -235,15 +235,15 @@ impl<'tcx> TlsData<'tcx> {
     }
 }
 
-impl VisitMachineValues for TlsData<'_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+impl VisitTags for TlsData<'_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         let TlsData { keys, macos_thread_dtors, next_key: _, dtors_running: _ } = self;
 
         for scalar in keys.values().flat_map(|v| v.data.values()) {
-            visit(&Operand::Immediate(Immediate::Scalar(*scalar)));
+            scalar.visit_tags(visit);
         }
         for (_, scalar) in macos_thread_dtors.values() {
-            visit(&Operand::Immediate(Immediate::Scalar(*scalar)));
+            scalar.visit_tags(visit);
         }
     }
 }
diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs
index 8464c4589ed..9713cd9265e 100644
--- a/src/tools/miri/src/shims/unix/fs.rs
+++ b/src/tools/miri/src/shims/unix/fs.rs
@@ -256,6 +256,12 @@ pub struct FileHandler {
     handles: BTreeMap<i32, Box<dyn FileDescriptor>>,
 }
 
+impl VisitTags for FileHandler {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {
+        // All our FileDescriptor do not have any tags.
+    }
+}
+
 impl FileHandler {
     pub(crate) fn new(mute_stdout_stderr: bool) -> FileHandler {
         let mut handles: BTreeMap<_, Box<dyn FileDescriptor>> = BTreeMap::new();
@@ -462,6 +468,16 @@ impl Default for DirHandler {
     }
 }
 
+impl VisitTags for DirHandler {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let DirHandler { streams, next_id: _ } = self;
+
+        for dir in streams.values() {
+            dir.entry.visit_tags(visit);
+        }
+    }
+}
+
 fn maybe_sync_file(
     file: &File,
     writable: bool,
diff --git a/src/tools/miri/src/shims/unix/linux/sync.rs b/src/tools/miri/src/shims/unix/linux/sync.rs
index 5a6ce28d25c..5762ee27b84 100644
--- a/src/tools/miri/src/shims/unix/linux/sync.rs
+++ b/src/tools/miri/src/shims/unix/linux/sync.rs
@@ -1,4 +1,4 @@
-use crate::concurrency::thread::Time;
+use crate::concurrency::thread::{MachineCallback, Time};
 use crate::*;
 use rustc_target::abi::{Align, Size};
 use std::time::SystemTime;
@@ -189,18 +189,36 @@ pub fn futex<'tcx>(
                 // Register a timeout callback if a timeout was specified.
                 // This callback will override the return value when the timeout triggers.
                 if let Some(timeout_time) = timeout_time {
+                    struct Callback<'tcx> {
+                        thread: ThreadId,
+                        addr_usize: u64,
+                        dest: PlaceTy<'tcx, Provenance>,
+                    }
+
+                    impl<'tcx> VisitTags for Callback<'tcx> {
+                        fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+                            let Callback { thread: _, addr_usize: _, dest } = self;
+                            dest.visit_tags(visit);
+                        }
+                    }
+
+                    impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for Callback<'tcx> {
+                        fn call(&self, this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> {
+                            this.unblock_thread(self.thread);
+                            this.futex_remove_waiter(self.addr_usize, self.thread);
+                            let etimedout = this.eval_libc("ETIMEDOUT")?;
+                            this.set_last_error(etimedout)?;
+                            this.write_scalar(Scalar::from_machine_isize(-1, this), &self.dest)?;
+
+                            Ok(())
+                        }
+                    }
+
                     let dest = dest.clone();
                     this.register_timeout_callback(
                         thread,
                         timeout_time,
-                        Box::new(move |this| {
-                            this.unblock_thread(thread);
-                            this.futex_remove_waiter(addr_usize, thread);
-                            let etimedout = this.eval_libc("ETIMEDOUT")?;
-                            this.set_last_error(etimedout)?;
-                            this.write_scalar(Scalar::from_machine_isize(-1, this), &dest)?;
-                            Ok(())
-                        }),
+                        Box::new(Callback { thread, addr_usize, dest }),
                     );
                 }
             } else {
diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs
index 2e972a27ffe..5aafe76ade1 100644
--- a/src/tools/miri/src/shims/unix/sync.rs
+++ b/src/tools/miri/src/shims/unix/sync.rs
@@ -3,7 +3,7 @@ use std::time::SystemTime;
 use rustc_hir::LangItem;
 use rustc_middle::ty::{layout::TyAndLayout, query::TyCtxtAt, Ty};
 
-use crate::concurrency::thread::Time;
+use crate::concurrency::thread::{MachineCallback, Time};
 use crate::*;
 
 // pthread_mutexattr_t is either 4 or 8 bytes, depending on the platform.
@@ -851,25 +851,43 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         // We return success for now and override it in the timeout callback.
         this.write_scalar(Scalar::from_i32(0), dest)?;
 
-        // Register the timeout callback.
-        let dest = dest.clone();
-        this.register_timeout_callback(
-            active_thread,
-            timeout_time,
-            Box::new(move |ecx| {
+        struct Callback<'tcx> {
+            active_thread: ThreadId,
+            mutex_id: MutexId,
+            id: CondvarId,
+            dest: PlaceTy<'tcx, Provenance>,
+        }
+
+        impl<'tcx> VisitTags for Callback<'tcx> {
+            fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+                let Callback { active_thread: _, mutex_id: _, id: _, dest } = self;
+                dest.visit_tags(visit);
+            }
+        }
+
+        impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for Callback<'tcx> {
+            fn call(&self, ecx: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> {
                 // We are not waiting for the condvar any more, wait for the
                 // mutex instead.
-                reacquire_cond_mutex(ecx, active_thread, mutex_id)?;
+                reacquire_cond_mutex(ecx, self.active_thread, self.mutex_id)?;
 
                 // Remove the thread from the conditional variable.
-                ecx.condvar_remove_waiter(id, active_thread);
+                ecx.condvar_remove_waiter(self.id, self.active_thread);
 
                 // Set the return value: we timed out.
                 let etimedout = ecx.eval_libc("ETIMEDOUT")?;
-                ecx.write_scalar(etimedout, &dest)?;
+                ecx.write_scalar(etimedout, &self.dest)?;
 
                 Ok(())
-            }),
+            }
+        }
+
+        // Register the timeout callback.
+        let dest = dest.clone();
+        this.register_timeout_callback(
+            active_thread,
+            timeout_time,
+            Box::new(Callback { active_thread, mutex_id, id, dest }),
         );
 
         Ok(())
diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs
index 429c6eb8ef5..2888f8e81fb 100644
--- a/src/tools/miri/src/stacked_borrows/mod.rs
+++ b/src/tools/miri/src/stacked_borrows/mod.rs
@@ -72,6 +72,12 @@ pub struct FrameExtra {
     protected_tags: SmallVec<[SbTag; 2]>,
 }
 
+impl VisitTags for FrameExtra {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {
+        // `protected_tags` are fine to GC.
+    }
+}
+
 /// Extra per-allocation state.
 #[derive(Clone, Debug)]
 pub struct Stacks {
@@ -110,6 +116,13 @@ pub struct GlobalStateInner {
     retag_fields: bool,
 }
 
+impl VisitTags for GlobalStateInner {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {
+        // The only candidate is base_ptr_tags, and that does not need visiting since we don't ever
+        // GC the bottommost tag.
+    }
+}
+
 /// We need interior mutable access to the global state.
 pub type GlobalState = RefCell<GlobalStateInner>;
 
@@ -514,6 +527,14 @@ impl Stacks {
     }
 }
 
+impl VisitTags for Stacks {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        for tag in self.exposed_tags.iter().copied() {
+            visit(tag);
+        }
+    }
+}
+
 /// Map per-stack operations to higher-level per-location-range operations.
 impl<'tcx> Stacks {
     /// Creates a new stack with an initial tag. For diagnostic purposes, we also need to know
diff --git a/src/tools/miri/src/stacked_borrows/stack.rs b/src/tools/miri/src/stacked_borrows/stack.rs
index 494ea08b56e..57de1c21c8b 100644
--- a/src/tools/miri/src/stacked_borrows/stack.rs
+++ b/src/tools/miri/src/stacked_borrows/stack.rs
@@ -43,8 +43,11 @@ impl Stack {
     pub fn retain(&mut self, tags: &FxHashSet<SbTag>) {
         let mut first_removed = None;
 
-        let mut read_idx = 1;
-        let mut write_idx = 1;
+        // For stacks with a known bottom, we never consider removing the bottom-most tag, because
+        // that is the base tag which exists whether or not there are any pointers to the
+        // allocation.
+        let mut read_idx = if self.unknown_bottom.is_some() { 0 } else { 1 };
+        let mut write_idx = read_idx;
         while read_idx < self.borrows.len() {
             let left = self.borrows[read_idx - 1];
             let this = self.borrows[read_idx];
diff --git a/src/tools/miri/src/tag_gc.rs b/src/tools/miri/src/tag_gc.rs
index 0bfc81ce012..5aa653632f3 100644
--- a/src/tools/miri/src/tag_gc.rs
+++ b/src/tools/miri/src/tag_gc.rs
@@ -2,33 +2,155 @@ use rustc_data_structures::fx::FxHashSet;
 
 use crate::*;
 
-pub trait VisitMachineValues {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>));
+pub trait VisitTags {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag));
 }
 
-impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
-pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
-    /// Generic GC helper to visit everything that can store a value. The `acc` offers some chance to
-    /// accumulate everything.
-    fn visit_all_machine_values<T>(
-        &self,
-        acc: &mut T,
-        mut visit_operand: impl FnMut(&mut T, &Operand<Provenance>),
-        mut visit_alloc: impl FnMut(&mut T, &Allocation<Provenance, AllocExtra>),
-    ) {
-        let this = self.eval_context_ref();
+impl<T: VisitTags> VisitTags for Option<T> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        if let Some(x) = self {
+            x.visit_tags(visit);
+        }
+    }
+}
+
+impl<T: VisitTags> VisitTags for std::cell::RefCell<T> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        self.borrow().visit_tags(visit)
+    }
+}
+
+impl VisitTags for SbTag {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        visit(*self)
+    }
+}
+
+impl VisitTags for Provenance {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        if let Provenance::Concrete { sb, .. } = self {
+            visit(*sb);
+        }
+    }
+}
+
+impl VisitTags for Pointer<Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let (prov, _offset) = self.into_parts();
+        prov.visit_tags(visit);
+    }
+}
+
+impl VisitTags for Pointer<Option<Provenance>> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let (prov, _offset) = self.into_parts();
+        prov.visit_tags(visit);
+    }
+}
+
+impl VisitTags for Scalar<Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        match self {
+            Scalar::Ptr(ptr, _) => ptr.visit_tags(visit),
+            Scalar::Int(_) => (),
+        }
+    }
+}
+
+impl VisitTags for Immediate<Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        match self {
+            Immediate::Scalar(s) => {
+                s.visit_tags(visit);
+            }
+            Immediate::ScalarPair(s1, s2) => {
+                s1.visit_tags(visit);
+                s2.visit_tags(visit);
+            }
+            Immediate::Uninit => {}
+        }
+    }
+}
+
+impl VisitTags for MemPlaceMeta<Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        match self {
+            MemPlaceMeta::Meta(m) => m.visit_tags(visit),
+            MemPlaceMeta::None => {}
+        }
+    }
+}
+
+impl VisitTags for MemPlace<Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let MemPlace { ptr, meta } = self;
+        ptr.visit_tags(visit);
+        meta.visit_tags(visit);
+    }
+}
+
+impl VisitTags for MPlaceTy<'_, Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        (**self).visit_tags(visit)
+    }
+}
+
+impl VisitTags for Place<Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        match self {
+            Place::Ptr(p) => p.visit_tags(visit),
+            Place::Local { .. } => {
+                // Will be visited as part of the stack frame.
+            }
+        }
+    }
+}
+
+impl VisitTags for PlaceTy<'_, Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        (**self).visit_tags(visit)
+    }
+}
+
+impl VisitTags for Operand<Provenance> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        match self {
+            Operand::Immediate(imm) => {
+                imm.visit_tags(visit);
+            }
+            Operand::Indirect(p) => {
+                p.visit_tags(visit);
+            }
+        }
+    }
+}
+
+impl VisitTags for Allocation<Provenance, AllocExtra> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        for (_size, prov) in self.provenance().iter() {
+            prov.visit_tags(visit);
+        }
+
+        self.extra.visit_tags(visit);
+    }
+}
 
+impl VisitTags for crate::MiriInterpCx<'_, '_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         // Memory.
-        this.memory.alloc_map().iter(|it| {
+        self.memory.alloc_map().iter(|it| {
             for (_id, (_kind, alloc)) in it {
-                visit_alloc(acc, alloc);
+                alloc.visit_tags(visit);
             }
         });
 
         // And all the other machine values.
-        this.machine.visit_machine_values(&mut |op| visit_operand(acc, op));
+        self.machine.visit_tags(visit);
     }
+}
 
+impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
+pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
     fn garbage_collect_tags(&mut self) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
         // No reason to do anything at all if stacked borrows is off.
@@ -37,43 +159,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
         }
 
         let mut tags = FxHashSet::default();
-
-        let visit_scalar = |tags: &mut FxHashSet<SbTag>, s: &Scalar<Provenance>| {
-            if let Scalar::Ptr(ptr, _) = s {
-                if let Provenance::Concrete { sb, .. } = ptr.provenance {
-                    tags.insert(sb);
-                }
-            }
-        };
-
-        this.visit_all_machine_values(
-            &mut tags,
-            |tags, op| {
-                match op {
-                    Operand::Immediate(Immediate::Scalar(s)) => {
-                        visit_scalar(tags, s);
-                    }
-                    Operand::Immediate(Immediate::ScalarPair(s1, s2)) => {
-                        visit_scalar(tags, s1);
-                        visit_scalar(tags, s2);
-                    }
-                    Operand::Immediate(Immediate::Uninit) => {}
-                    Operand::Indirect(MemPlace { ptr, .. }) => {
-                        if let Some(Provenance::Concrete { sb, .. }) = ptr.provenance {
-                            tags.insert(sb);
-                        }
-                    }
-                }
-            },
-            |tags, alloc| {
-                for (_size, prov) in alloc.provenance().iter() {
-                    if let Provenance::Concrete { sb, .. } = prov {
-                        tags.insert(*sb);
-                    }
-                }
-            },
-        );
-
+        this.visit_tags(&mut |tag| {
+            tags.insert(tag);
+        });
         self.remove_unreachable_tags(tags);
 
         Ok(())