Auto merge of #3229 - saethlin:protector-gc-ice, r=RalfJung

Visit the AllocIds and BorTags in borrow state FrameExtra

Fixes https://github.com/rust-lang/miri/issues/3228

I said

> The obvious way would be to visit the AllocIds in borrow_tracker::FrameExtra in the GC.

Since I have had no new ideas, that's what this does.

2 files changed, 25 insertions, 8 deletions

diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs
index 8fae5269229..a6961208ffb 100644
--- a/src/tools/miri/src/borrow_tracker/mod.rs
+++ b/src/tools/miri/src/borrow_tracker/mod.rs
@@ -65,7 +65,7 @@ pub struct FrameState {
     /// incremental updates of the global list of protected tags stored in the
     /// `stacked_borrows::GlobalState` upon function return, and if we attempt to pop a protected
     /// tag, to identify which call is responsible for protecting the tag.
-    /// See `Stack::item_popped` for more explanation.
+    /// See `Stack::item_invalidated` for more explanation.
     /// Tree Borrows also needs to know which allocation these tags
     /// belong to so that it can perform a read through them immediately before
     /// the frame gets popped.
@@ -76,8 +76,10 @@ pub struct FrameState {
 }
 
 impl VisitProvenance for FrameState {
-    fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
-        // `protected_tags` are already recorded by `GlobalStateInner`.
+    fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
+        for (id, tag) in &self.protected_tags {
+            visit(Some(*id), Some(*tag));
+        }
     }
 }
 
@@ -98,7 +100,7 @@ pub struct GlobalStateInner {
     /// An item is protected if its tag is in this set, *and* it has the "protected" bit set.
     /// We add tags to this when they are created with a protector in `reborrow`, and
     /// we remove tags from this when the call which is protecting them returns, in
-    /// `GlobalStateInner::end_call`. See `Stack::item_popped` for more details.
+    /// `GlobalStateInner::end_call`. See `Stack::item_invalidated` for more details.
     protected_tags: FxHashMap<BorTag, ProtectorKind>,
     /// The pointer ids to trace
     tracked_pointer_tags: FxHashSet<BorTag>,
@@ -111,10 +113,8 @@ pub struct GlobalStateInner {
 }
 
 impl VisitProvenance for GlobalStateInner {
-    fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
-        for &tag in self.protected_tags.keys() {
-            visit(None, Some(tag));
-        }
+    fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
+        // All the provenance in protected_tags is also stored in FrameState, and visited there.
         // The only other candidate is base_ptr_tags, and that does not need visiting since we don't ever
         // GC the bottommost/root tag.
     }
diff --git a/src/tools/miri/tests/pass/protector-gc.rs b/src/tools/miri/tests/pass/protector-gc.rs
new file mode 100644
index 00000000000..4e422c323ea
--- /dev/null
+++ b/src/tools/miri/tests/pass/protector-gc.rs
@@ -0,0 +1,17 @@
+// When we pop a stack frame with weak protectors, we need to check if the protected pointer's
+// allocation is still live. If the provenance GC only knows about the BorTag that is protected,
+// we can ICE. This test checks that we don't.
+// See https://github.com/rust-lang/miri/issues/3228
+
+#[path = "../utils/mod.rs"]
+mod utils;
+
+#[allow(unused)]
+fn oof(mut b: Box<u8>) {
+    b = Box::new(0u8);
+    utils::run_provenance_gc();
+}
+
+fn main() {
+    oof(Box::new(0u8));
+}