re-architect the tag visitor traits

RalfJung · RalfJung · commit b30d3a9c625b · 2022-10-02T16:26:55.000+02:00
diff --git a/src/concurrency/data_race.rs b/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/concurrency/thread.rs b/src/concurrency/thread.rs
@@ -32,7 +32,7 @@ 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.
-pub trait MachineCallback<'mir, 'tcx>: VisitMachineValues {
+pub trait MachineCallback<'mir, 'tcx>: VisitTags {
     fn call(&self, ecx: &mut InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>) -> InterpResult<'tcx>;
 }
 
@@ -183,25 +183,21 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {
     }
 }
 
-impl VisitMachineValues for Thread<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+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;
 
-        if let Some(payload) = panic_payload {
-            visit.visit(*payload);
-        }
-        if let Some(error) = last_error {
-            visit.visit(**error);
-        }
+        panic_payload.visit_tags(visit);
+        last_error.visit_tags(visit);
         for frame in stack {
-            frame.visit_machine_values(visit)
+            frame.visit_tags(visit)
         }
     }
 }
 
-impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+impl VisitTags for Frame<'_, '_, Provenance, FrameData<'_>> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         let Frame {
             return_place,
             locals,
@@ -214,17 +210,15 @@ impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {
         } = self;
 
         // Return place.
-        if let Place::Ptr(mplace) = **return_place {
-            visit.visit(mplace);
-        }
+        return_place.visit_tags(visit);
         // Locals.
         for local in locals.iter() {
             if let LocalValue::Live(value) = &local.value {
-                visit.visit(value);
+                value.visit_tags(visit);
             }
         }
 
-        extra.visit_machine_values(visit);
+        extra.visit_tags(visit);
     }
 }
 
@@ -300,8 +294,8 @@ impl<'mir, 'tcx> Default for ThreadManager<'mir, 'tcx> {
     }
 }
 
-impl VisitMachineValues for ThreadManager<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+impl VisitTags for ThreadManager<'_, '_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         let ThreadManager {
             threads,
             thread_local_alloc_ids,
@@ -312,13 +306,13 @@ impl VisitMachineValues for ThreadManager<'_, '_> {
         } = self;
 
         for thread in threads {
-            thread.visit_machine_values(visit);
+            thread.visit_tags(visit);
         }
-        for ptr in thread_local_alloc_ids.borrow().values().copied() {
-            visit.visit(ptr);
+        for ptr in thread_local_alloc_ids.borrow().values() {
+            ptr.visit_tags(visit);
         }
         for callback in timeout_callbacks.values() {
-            callback.callback.visit_machine_values(visit);
+            callback.callback.visit_tags(visit);
         }
     }
 }
diff --git a/src/concurrency/weak_memory.rs b/src/concurrency/weak_memory.rs
@@ -108,15 +108,15 @@ pub struct StoreBufferAlloc {
     store_buffers: RefCell<RangeObjectMap<StoreBuffer>>,
 }
 
-impl VisitMachineValues for StoreBufferAlloc {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
-        for val in self
-            .store_buffers
+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))
         {
-            visit.visit(val);
+            val.visit_tags(visit);
         }
     }
 }
diff --git a/src/intptrcast.rs b/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/lib.rs b/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 _, ProvenanceVisitor, 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/machine.rs b/src/machine.rs
@@ -64,12 +64,12 @@ impl<'tcx> std::fmt::Debug for FrameData<'tcx> {
     }
 }
 
-impl VisitMachineValues for FrameData<'_> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+impl VisitTags for FrameData<'_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         let FrameData { catch_unwind, stacked_borrows: _, timing: _ } = self;
 
         if let Some(catch_unwind) = catch_unwind {
-            catch_unwind.visit_machine_values(visit);
+            catch_unwind.visit_tags(visit);
         }
     }
 }
@@ -262,17 +262,13 @@ pub struct AllocExtra {
     pub weak_memory: Option<weak_memory::AllocExtra>,
 }
 
-impl VisitMachineValues for AllocExtra {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
-        let AllocExtra { stacked_borrows, data_race: _, weak_memory } = self;
+impl VisitTags for AllocExtra {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let AllocExtra { stacked_borrows, data_race, weak_memory } = self;
 
-        if let Some(stacked_borrows) = stacked_borrows {
-            stacked_borrows.borrow().visit_machine_values(visit);
-        }
-
-        if let Some(weak_memory) = weak_memory {
-            weak_memory.visit_machine_values(visit);
-        }
+        stacked_borrows.visit_tags(visit);
+        data_race.visit_tags(visit);
+        weak_memory.visit_tags(visit);
     }
 }
 
@@ -613,8 +609,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
     }
 }
 
-impl VisitMachineValues for MiriMachine<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+impl VisitTags for MiriMachine<'_, '_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         let MiriMachine {
             threads,
             tls,
@@ -624,25 +620,50 @@ impl VisitMachineValues for MiriMachine<'_, '_> {
             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: _,
+            external_so_lib: _,
+            gc_interval: _,
+            since_gc: _,
         } = self;
 
-        threads.visit_machine_values(visit);
-        tls.visit_machine_values(visit);
-        env_vars.visit_machine_values(visit);
-        dir_handler.visit_machine_values(visit);
-
-        if let Some(argc) = argc {
-            visit.visit(argc);
-        }
-        if let Some(argv) = argv {
-            visit.visit(argv);
-        }
-        if let Some(cmd_line) = cmd_line {
-            visit.visit(cmd_line);
-        }
-        for ptr in extern_statics.values().copied() {
-            visit.visit(ptr);
+        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/shims/env.rs b/src/shims/env.rs
@@ -36,15 +36,15 @@ pub struct EnvVars<'tcx> {
     pub(crate) environ: Option<MPlaceTy<'tcx, Provenance>>,
 }
 
-impl VisitMachineValues for EnvVars<'_> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+impl VisitTags for EnvVars<'_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         let EnvVars { map, environ } = self;
 
         for ptr in map.values() {
-            visit.visit(*ptr);
+            ptr.visit_tags(visit);
         }
         if let Some(env) = environ {
-            visit.visit(**env);
+            env.visit_tags(visit);
         }
     }
 }
diff --git a/src/shims/panic.rs b/src/shims/panic.rs
@@ -35,11 +35,11 @@ pub struct CatchUnwindData<'tcx> {
     ret: mir::BasicBlock,
 }
 
-impl VisitMachineValues for CatchUnwindData<'_> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+impl VisitTags for CatchUnwindData<'_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
         let CatchUnwindData { catch_fn, data, dest: _, ret: _ } = self;
-        visit.visit(catch_fn);
-        visit.visit(data);
+        catch_fn.visit_tags(visit);
+        data.visit_tags(visit);
     }
 }
 
diff --git a/src/shims/time.rs b/src/shims/time.rs
@@ -219,7 +219,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.register_timeout_callback(
             active_thread,
             Time::Monotonic(timeout_time),
-            Box::new(Callback { active_thread }),
+            Box::new(UnblockCallback { thread_to_unblock: active_thread }),
         );
 
         Ok(0)
@@ -242,24 +242,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.register_timeout_callback(
             active_thread,
             Time::Monotonic(timeout_time),
-            Box::new(Callback { active_thread }),
+            Box::new(UnblockCallback { thread_to_unblock: active_thread }),
         );
 
         Ok(())
     }
 }
 
-struct Callback {
-    active_thread: ThreadId,
+struct UnblockCallback {
+    thread_to_unblock: ThreadId,
 }
 
-impl VisitMachineValues for Callback {
-    fn visit_machine_values(&self, _visit: &mut ProvenanceVisitor) {}
+impl VisitTags for UnblockCallback {
+    fn visit_tags(&self, _visit: &mut dyn FnMut(SbTag)) {}
 }
 
-impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for Callback {
+impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for UnblockCallback {
     fn call(&self, ecx: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> {
-        ecx.unblock_thread(self.active_thread);
+        ecx.unblock_thread(self.thread_to_unblock);
         Ok(())
     }
 }
diff --git a/src/shims/tls.rs b/src/shims/tls.rs
@@ -235,15 +235,15 @@ impl<'tcx> TlsData<'tcx> {
     }
 }
 
-impl VisitMachineValues for TlsData<'_> {
-    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+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.visit(scalar);
+            scalar.visit_tags(visit);
         }
         for (_, scalar) in macos_thread_dtors.values() {
-            visit.visit(scalar);
+            scalar.visit_tags(visit);
         }
     }
 }
diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs
diff --git a/src/shims/unix/linux/sync.rs b/src/shims/unix/linux/sync.rs
diff --git a/src/shims/unix/sync.rs b/src/shims/unix/sync.rs
diff --git a/src/stacked_borrows/mod.rs b/src/stacked_borrows/mod.rs
diff --git a/src/tag_gc.rs b/src/tag_gc.rs

Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ pub enum SchedulingAction {`
`32`	`32`
`33`	`33`	`/// Timeout callbacks can be created by synchronization primitives to tell the`
`34`	`34`	`/// scheduler that they should be called once some period of time passes.`
`35`		`-pub trait MachineCallback<'mir, 'tcx>: VisitMachineValues {`
	`35`	`+pub trait MachineCallback<'mir, 'tcx>: VisitTags {`
`36`	`36`	`fn call(&self, ecx: &mut InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>) -> InterpResult<'tcx>;`
`37`	`37`	`}`
`38`	`38`
`@@ -183,25 +183,21 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {`
`183`	`183`	`}`
`184`	`184`	`}`
`185`	`185`
`186`		`-impl VisitMachineValues for Thread<'_, '_> {`
`187`		`- fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
	`186`	`+impl VisitTags for Thread<'_, '_> {`
	`187`	`+ fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {`
`188`	`188`	`let Thread { panic_payload, last_error, stack, state: _, thread_name: _, join_status: _ } =`
`189`	`189`	`self;`
`190`	`190`
`191`		`- if let Some(payload) = panic_payload {`
`192`		`- visit.visit(*payload);`
`193`		`- }`
`194`		`- if let Some(error) = last_error {`
`195`		`- visit.visit(**error);`
`196`		`- }`
	`191`	`+ panic_payload.visit_tags(visit);`
	`192`	`+ last_error.visit_tags(visit);`
`197`	`193`	`for frame in stack {`
`198`		`- frame.visit_machine_values(visit)`
	`194`	`+ frame.visit_tags(visit)`
`199`	`195`	`}`
`200`	`196`	`}`
`201`	`197`	`}`
`202`	`198`
`203`		`-impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {`
`204`		`- fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
	`199`	`+impl VisitTags for Frame<'_, '_, Provenance, FrameData<'_>> {`
	`200`	`+ fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {`
`205`	`201`	`let Frame {`
`206`	`202`	`return_place,`
`207`	`203`	`locals,`
`@@ -214,17 +210,15 @@ impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {`
`214`	`210`	`} = self;`
`215`	`211`
`216`	`212`	`// Return place.`
`217`		`- if let Place::Ptr(mplace) = **return_place {`
`218`		`- visit.visit(mplace);`
`219`		`- }`
	`213`	`+ return_place.visit_tags(visit);`
`220`	`214`	`// Locals.`
`221`	`215`	`for local in locals.iter() {`
`222`	`216`	`if let LocalValue::Live(value) = &local.value {`
`223`		`- visit.visit(value);`
	`217`	`+ value.visit_tags(visit);`
`224`	`218`	`}`
`225`	`219`	`}`
`226`	`220`
`227`		`- extra.visit_machine_values(visit);`
	`221`	`+ extra.visit_tags(visit);`
`228`	`222`	`}`
`229`	`223`	`}`
`230`	`224`
`@@ -300,8 +294,8 @@ impl<'mir, 'tcx> Default for ThreadManager<'mir, 'tcx> {`
`300`	`294`	`}`
`301`	`295`	`}`
`302`	`296`
`303`		`-impl VisitMachineValues for ThreadManager<'_, '_> {`
`304`		`- fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
	`297`	`+impl VisitTags for ThreadManager<'_, '_> {`
	`298`	`+ fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {`
`305`	`299`	`let ThreadManager {`
`306`	`300`	`threads,`
`307`	`301`	`thread_local_alloc_ids,`
`@@ -312,13 +306,13 @@ impl VisitMachineValues for ThreadManager<'_, '_> {`
`312`	`306`	`} = self;`
`313`	`307`
`314`	`308`	`for thread in threads {`
`315`		`- thread.visit_machine_values(visit);`
	`309`	`+ thread.visit_tags(visit);`
`316`	`310`	`}`
`317`		`- for ptr in thread_local_alloc_ids.borrow().values().copied() {`
`318`		`- visit.visit(ptr);`
	`311`	`+ for ptr in thread_local_alloc_ids.borrow().values() {`
	`312`	`+ ptr.visit_tags(visit);`
`319`	`313`	`}`
`320`	`314`	`for callback in timeout_callbacks.values() {`
`321`		`- callback.callback.visit_machine_values(visit);`
	`315`	`+ callback.callback.visit_tags(visit);`
`322`	`316`	`}`
`323`	`317`	`}`
`324`	`318`	`}`
Original file line number	Diff line number	Diff line change
`@@ -108,15 +108,15 @@ pub struct StoreBufferAlloc {`
`108`	`108`	`store_buffers: RefCell<RangeObjectMap<StoreBuffer>>,`
`109`	`109`	`}`
`110`	`110`
`111`		`-impl VisitMachineValues for StoreBufferAlloc {`
`112`		`- fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`113`		`- for val in self`
`114`		`- .store_buffers`
	`111`	`+impl VisitTags for StoreBufferAlloc {`
	`112`	`+ fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {`
	`113`	`+ let Self { store_buffers } = self;`
	`114`	`+ for val in store_buffers`
`115`	`115`	`.borrow()`
`116`	`116`	`.iter()`
`117`	`117`	`.flat_map(\|buf\| buf.buffer.iter().map(\|element\| &element.val))`
`118`	`118`	`{`
`119`		`- visit.visit(val);`
	`119`	`+ val.visit_tags(visit);`
`120`	`120`	`}`
`121`	`121`	`}`
`122`	`122`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,15 +36,15 @@ pub struct EnvVars<'tcx> {`
`36`	`36`	`pub(crate) environ: Option<MPlaceTy<'tcx, Provenance>>,`
`37`	`37`	`}`
`38`	`38`
`39`		`-impl VisitMachineValues for EnvVars<'_> {`
`40`		`- fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
	`39`	`+impl VisitTags for EnvVars<'_> {`
	`40`	`+ fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {`
`41`	`41`	`let EnvVars { map, environ } = self;`
`42`	`42`
`43`	`43`	`for ptr in map.values() {`
`44`		`- visit.visit(*ptr);`
	`44`	`+ ptr.visit_tags(visit);`
`45`	`45`	`}`
`46`	`46`	`if let Some(env) = environ {`
`47`		`- visit.visit(**env);`
	`47`	`+ env.visit_tags(visit);`
`48`	`48`	`}`
`49`	`49`	`}`
`50`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,11 +35,11 @@ pub struct CatchUnwindData<'tcx> {`
`35`	`35`	`ret: mir::BasicBlock,`
`36`	`36`	`}`
`37`	`37`
`38`		`-impl VisitMachineValues for CatchUnwindData<'_> {`
`39`		`- fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
	`38`	`+impl VisitTags for CatchUnwindData<'_> {`
	`39`	`+ fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {`
`40`	`40`	`let CatchUnwindData { catch_fn, data, dest: _, ret: _ } = self;`
`41`		`- visit.visit(catch_fn);`
`42`		`- visit.visit(data);`
	`41`	`+ catch_fn.visit_tags(visit);`
	`42`	`+ data.visit_tags(visit);`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`
Original file line number	Diff line number	Diff line change
`@@ -235,15 +235,15 @@ impl<'tcx> TlsData<'tcx> {`
`235`	`235`	`}`
`236`	`236`	`}`
`237`	`237`
`238`		`-impl VisitMachineValues for TlsData<'_> {`
`239`		`- fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
	`238`	`+impl VisitTags for TlsData<'_> {`
	`239`	`+ fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {`
`240`	`240`	`let TlsData { keys, macos_thread_dtors, next_key: _, dtors_running: _ } = self;`
`241`	`241`
`242`	`242`	`for scalar in keys.values().flat_map(\|v\| v.data.values()) {`
`243`		`- visit.visit(scalar);`
	`243`	`+ scalar.visit_tags(visit);`
`244`	`244`	`}`
`245`	`245`	`for (_, scalar) in macos_thread_dtors.values() {`
`246`		`- visit.visit(scalar);`
	`246`	`+ scalar.visit_tags(visit);`
`247`	`247`	`}`
`248`	`248`	`}`
`249`	`249`	`}`