Implement -Zmiri-tag-gc a garbage collector for tags

Author: saethlin

src/bin/miri.rs

@@ -521,6 +521,14 @@ fn main() {
                 Err(err) => show_error!("-Zmiri-report-progress requires a `u32`: {}", err),
             };
             miri_config.report_progress = Some(interval);
+        } else if arg == "-Zmiri-tag-gc" {
+            miri_config.gc_interval = Some(10_000);
+        } else if let Some(param) = arg.strip_prefix("-Zmiri-tag-gc=") {
+            let interval = match param.parse::<u32>() {
+                Ok(i) => i,
+                Err(err) => show_error!("-Zmiri-tag-gc requires a `u32`: {}", err),
+            };
+            miri_config.gc_interval = Some(interval);
         } else if let Some(param) = arg.strip_prefix("-Zmiri-measureme=") {
             miri_config.measureme_out = Some(param.to_string());
         } else if let Some(param) = arg.strip_prefix("-Zmiri-backtrace=") {

src/eval.rs

@@ -128,6 +128,8 @@ pub struct MiriConfig {
     pub report_progress: Option<u32>,
     /// Whether Stacked Borrows retagging should recurse into fields of datatypes.
     pub retag_fields: bool,
+    /// Run a garbage collector for SbTags every N basic blocks.
+    pub gc_interval: Option<u32>,
 }
 
 impl Default for MiriConfig {
@@ -159,6 +161,7 @@ impl Default for MiriConfig {
             preemption_rate: 0.01, // 1%
             report_progress: None,
             retag_fields: false,
+            gc_interval: None,
         }
     }
 }

src/intptrcast.rs

@@ -36,7 +36,7 @@ pub struct GlobalStateInner {
     base_addr: FxHashMap<AllocId, u64>,
     /// Whether an allocation has been exposed or not. This cannot be put
     /// into `AllocExtra` for the same reason as `base_addr`.
-    exposed: FxHashSet<AllocId>,
+    pub exposed: FxHashSet<AllocId>,
     /// This is used as a memory address when a new pointer is casted to an integer. It
     /// is always larger than any address that was previously made part of a block.
     next_base_addr: u64,
@@ -246,6 +246,11 @@ impl<'mir, 'tcx> GlobalStateInner {
             rem => addr.checked_add(align).unwrap() - rem,
         }
     }
+
+    /// Returns whether the specified `AllocId` has been exposed.
+    pub fn is_exposed(&self, id: AllocId) -> bool {
+        self.exposed.contains(&id)
+    }
 }
 
 #[cfg(test)]

src/lib.rs

@@ -59,6 +59,7 @@ mod range_map;
 mod shims;
 mod stacked_borrows;
 mod sync;
+mod tag_gc;
 mod thread;
 mod vector_clock;
 

src/machine.rs

@@ -355,6 +355,11 @@ pub struct Evaluator<'mir, 'tcx> {
     pub(crate) report_progress: Option<u32>,
     /// The number of blocks that passed since the last progress report.
     pub(crate) since_progress_report: u32,
+
+    /// If `Some`, we will attempt to run a garbage collector on SbTags every N basic blocks.
+    pub(crate) gc_interval: Option<u32>,
+    /// The number of blocks that passed since the last SbTag GC pass.
+    pub(crate) since_gc: u32,
 }
 
 impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
@@ -409,6 +414,8 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             preemption_rate: config.preemption_rate,
             report_progress: config.report_progress,
             since_progress_report: 0,
+            gc_interval: config.gc_interval,
+            since_gc: 0,
         }
     }
 
@@ -936,6 +943,17 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
             // Cannot overflow, since it is strictly less than `report_progress`.
             ecx.machine.since_progress_report += 1;
         }
+
+        // Search all memory for SbTags, then use Stack::retain to drop all other tags.
+        if let Some(gc_interval) = ecx.machine.gc_interval {
+            if ecx.machine.since_gc >= gc_interval {
+                ecx.machine.since_gc = 0;
+                tag_gc::garbage_collect_tags(ecx)?;
+            } else {
+                ecx.machine.since_gc += 1;
+            }
+        }
+
         // These are our preemption points.
         ecx.maybe_preempt_active_thread();
         Ok(())

src/shims/panic.rs

@@ -57,6 +57,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         assert!(thread.panic_payload.is_none(), "the panic runtime should avoid double-panics");
         thread.panic_payload = Some(payload);
 
+        // Expose the allocation so we don't GC it
+        if let Scalar::Ptr(Pointer { provenance: Provenance::Concrete { alloc_id, .. }, .. }, _) =
+            payload
+        {
+            this.machine.intptrcast.borrow_mut().exposed.insert(alloc_id);
+        }
+
         // Jump to the unwind block to begin unwinding.
         this.unwind_to_block(unwind)?;
         Ok(())

src/shims/tls.rs

@@ -225,6 +225,12 @@ impl<'tcx> TlsData<'tcx> {
             data.remove(&thread_id);
         }
     }
+
+    pub fn iter(&self, mut visitor: impl FnMut(&Scalar<Provenance>)) {
+        for scalar in self.keys.values().flat_map(|v| v.data.values()) {
+            visitor(scalar);
+        }
+    }
 }
 
 impl<'mir, 'tcx: 'mir> EvalContextPrivExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}

src/stacked_borrows/mod.rs

@@ -624,6 +624,10 @@ impl<'tcx> Stacks {
         }
         Ok(())
     }
+
+    pub fn iter_all(&mut self) -> impl Iterator<Item = &mut Stack> {
+        self.stacks.iter_mut_all()
+    }
 }
 
 /// Glue code to connect with Miri Machine Hooks

src/stacked_borrows/stack.rs

@@ -39,6 +39,23 @@ pub struct Stack {
     unique_range: Range<usize>,
 }
 
+impl Stack {
+    pub fn retain(&mut self, tags: &FxHashSet<SbTag>) {
+        let prev_len = self.borrows.len();
+        self.borrows.retain(|item| tags.contains(&item.tag()));
+
+        // Reset the cache if anything was changed
+        #[cfg(feature = "stack-cache")]
+        if self.borrows.len() != prev_len {
+            self.unique_range = 0..self.len();
+            for i in 0..CACHE_LEN {
+                self.cache.items[i] = self.borrows[0];
+                self.cache.idx[i] = 0;
+            }
+        }
+    }
+}
+
 /// A very small cache of searches of a borrow stack, mapping `Item`s to their position in said stack.
 ///
 /// It may seem like maintaining this cache is a waste for small stacks, but
@@ -105,12 +122,14 @@ impl<'tcx> Stack {
 
         // Check that the unique_range is a valid index into the borrow stack.
         // This asserts that the unique_range's start <= end.
-        let uniques = &self.borrows[self.unique_range.clone()];
+        let _uniques = &self.borrows[self.unique_range.clone()];
 
         // Check that the start of the unique_range is precise.
+        /*
         if let Some(first_unique) = uniques.first() {
             assert_eq!(first_unique.perm(), Permission::Unique);
         }
+        */
         // We cannot assert that the unique range is exact on the upper end.
         // When we pop items within the unique range, setting the end of the range precisely
         // requires doing a linear search of the borrow stack, which is exactly the kind of

src/tag_gc.rs

@@ -0,0 +1,114 @@
+use crate::*;
+use rustc_data_structures::fx::FxHashSet;
+
+pub fn garbage_collect_tags<'mir, 'tcx>(
+    ecx: &mut MiriEvalContext<'mir, 'tcx>,
+) -> InterpResult<'tcx> {
+    // No reason to do anything at all if stacked borrows is off.
+    if ecx.machine.stacked_borrows.is_none() {
+        return Ok(());
+    }
+
+    let mut tags = FxHashSet::default();
+
+    ecx.machine.tls.iter(|scalar| {
+        if let Scalar::Ptr(Pointer { provenance: Provenance::Concrete { sb, .. }, .. }, _) = scalar
+        {
+            tags.insert(*sb);
+        }
+    });
+
+    // Normal memory
+    ecx.memory.alloc_map().iter(|it| {
+        for (_id, (_kind, alloc)) in it {
+            let stacked_borrows = alloc.extra.stacked_borrows.as_ref().unwrap();
+            // Base tags: TODO: Do we need this?
+            for stack in stacked_borrows.borrow_mut().iter_all() {
+                if let Some(item) = stack.get(0) {
+                    tags.insert(item.tag());
+                }
+            }
+            for (_size, prov) in alloc.relocations().iter() {
+                if let Provenance::Concrete { sb, .. } = prov {
+                    tags.insert(*sb);
+                }
+            }
+        }
+    });
+
+    // Locals
+    // This code is a mess because...
+    // force_allocation only works on locals from the current thread. This is because locals don't
+    // remember what thread they are from, they're just an index. So attempting to use a local on
+    // the wrong thread is simply an error.
+    // Additionally, InterpCx::force_allocation takes out a mutable borrow of the whole InterpCx.
+    // So we need to do this whole dance with indexes and a clone of the return place in order to
+    // avoid holding a shared borrow of anything during the force_allocation call.
+    let tids =
+        ecx.machine.threads.threads.iter_enumerated().map(|(tid, _thread)| tid).collect::<Vec<_>>();
+    let original_tid = ecx.machine.threads.active_thread;
+
+    for tid in tids {
+        ecx.machine.threads.active_thread = tid;
+        for f in 0..ecx.active_thread_stack().len() {
+            // Handle the return place of each frame
+            let frame = &ecx.active_thread_stack()[f];
+            let return_place = ecx.force_allocation(&frame.return_place.clone())?;
+            return_place.map_provenance(|p| {
+                if let Some(Provenance::Concrete { sb, .. }) = p {
+                    tags.insert(sb);
+                }
+                p
+            });
+
+            let frame = &ecx.active_thread_stack()[f];
+            for local in frame.locals.iter() {
+                if let LocalValue::Live(Operand::Immediate(Immediate::Scalar(
+                    ScalarMaybeUninit::Scalar(Scalar::Ptr(ptr, _)),
+                ))) = local.value
+                {
+                    if let Provenance::Concrete { sb, .. } = ptr.provenance {
+                        tags.insert(sb);
+                    }
+                }
+                if let LocalValue::Live(Operand::Immediate(Immediate::ScalarPair(s1, s2))) =
+                    local.value
+                {
+                    if let ScalarMaybeUninit::Scalar(Scalar::Ptr(ptr, _)) = s1 {
+                        if let Provenance::Concrete { sb, .. } = ptr.provenance {
+                            tags.insert(sb);
+                        }
+                    }
+                    if let ScalarMaybeUninit::Scalar(Scalar::Ptr(ptr, _)) = s2 {
+                        if let Provenance::Concrete { sb, .. } = ptr.provenance {
+                            tags.insert(sb);
+                        }
+                    }
+                }
+
+                if let LocalValue::Live(Operand::Indirect(MemPlace { ptr, .. })) = local.value {
+                    if let Some(Provenance::Concrete { sb, .. }) = ptr.provenance {
+                        tags.insert(sb);
+                    }
+                }
+            }
+        }
+    }
+    ecx.machine.threads.active_thread = original_tid;
+
+    ecx.memory.alloc_map().iter(|it| {
+        for (id, (_kind, alloc)) in it {
+            // Don't GC any allocation which has been exposed
+            if ecx.machine.intptrcast.borrow().is_exposed(*id) {
+                continue;
+            }
+
+            let stacked_borrows = alloc.extra.stacked_borrows.as_ref().unwrap();
+            for stack in stacked_borrows.borrow_mut().iter_all() {
+                stack.retain(&tags);
+            }
+        }
+    });
+
+    Ok(())
+}

src/thread.rs

@@ -112,7 +112,7 @@ pub struct Thread<'mir, 'tcx> {
     thread_name: Option<Vec<u8>>,
 
     /// The virtual call stack.
-    stack: Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>>,
+    pub stack: Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>>,
 
     /// The join status.
     join_status: ThreadJoinStatus,
@@ -217,11 +217,11 @@ impl<'mir, 'tcx> std::fmt::Debug for TimeoutCallbackInfo<'mir, 'tcx> {
 #[derive(Debug)]
 pub struct ThreadManager<'mir, 'tcx> {
     /// Identifier of the currently active thread.
-    active_thread: ThreadId,
+    pub active_thread: ThreadId,
     /// Threads used in the program.
     ///
     /// Note that this vector also contains terminated threads.
-    threads: IndexVec<ThreadId, Thread<'mir, 'tcx>>,
+    pub threads: IndexVec<ThreadId, Thread<'mir, 'tcx>>,
     /// This field is pub(crate) because the synchronization primitives
     /// (`crate::sync`) need a way to access it.
     pub(crate) sync: SynchronizationState,