user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #2559 - RalfJung:gc-refactor, r=saethlin

Browse source 
GC: factor out visiting all machine values

`@saethlin` that is roughly what I had in mind.

I think some parts of the state are skipped by the visitor. I listed the ones that I found in FIXMEs but I am not sure if that list is complete.
This commit is contained in:
bors 2022-09-23 15:37:21 +00:00
commit 0d5748e8e9
5 changed files with 105 additions and 82 deletions
Download patch file Download diff file Expand all files Collapse all files

31
src/tools/miri/src/concurrency/thread.rs
View file

@ -290,10 +290,6 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
&mut self.threads[self.active_thread].stack
}
pub fn iter(&self) -> impl Iterator<Item = &Thread<'mir, 'tcx>> {
self.threads.iter()
}
pub fn all_stacks(
&self,
) -> impl Iterator<Item = &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>]> {
@ -628,6 +624,33 @@ 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> {

2
src/tools/miri/src/lib.rs
View file

@ -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 _;
pub use crate::tag_gc::{EvalContextExt as _, VisitMachineValues};
/// Insert rustc arguments at the beginning of the argument list that Miri wants to be
/// set per default, for maximal validation power.

14
src/tools/miri/src/machine.rs
View file

@ -291,6 +291,9 @@ impl<'mir, 'tcx: 'mir> PrimitiveLayouts<'tcx> {
}
/// The machine itself.
///
/// If you add anything here that stores machine values, remember to update
/// `visit_all_machine_values`!
pub struct MiriMachine<'mir, 'tcx> {
// We carry a copy of the global `TyCtxt` for convenience, so methods taking just `&Evaluator` have `tcx` access.
pub tcx: TyCtxt<'tcx>,
@ -586,6 +589,17 @@ 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);
}
}
/// A rustc InterpCx for Miri.
pub type MiriInterpCx<'mir, 'tcx> = InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>;

13
src/tools/miri/src/shims/tls.rs
View file

@ -233,10 +233,17 @@ 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 VisitMachineValues for TlsData<'_> {
fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
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)));
}
for (_, scalar) in macos_thread_dtors.values() {
visit(&Operand::Immediate(Immediate::Scalar(*scalar)));
}
}
}

127
src/tools/miri/src/tag_gc.rs
View file

@ -1,8 +1,34 @@
use crate::*;
use rustc_data_structures::fx::FxHashSet;
use crate::*;
pub trait VisitMachineValues {
fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>));
}
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();
// Memory.
this.memory.alloc_map().iter(|it| {
for (_id, (_kind, alloc)) in it {
visit_alloc(acc, alloc);
}
});
// And all the other machine values.
this.machine.visit_machine_values(&mut |op| visit_operand(acc, op));
}
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.
@ -12,90 +38,43 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
let mut tags = FxHashSet::default();
for thread in this.machine.threads.iter() {
if let Some(Scalar::Ptr(
Pointer { provenance: Provenance::Concrete { sb, .. }, .. },
_,
)) = thread.panic_payload
{
tags.insert(sb);
}
}
self.find_tags_in_tls(&mut tags);
self.find_tags_in_memory(&mut tags);
self.find_tags_in_locals(&mut tags)?;
self.remove_unreachable_tags(tags);
Ok(())
}
fn find_tags_in_tls(&mut self, tags: &mut FxHashSet<SbTag>) {
let this = self.eval_context_mut();
this.machine.tls.iter(|scalar| {
if let Scalar::Ptr(Pointer { provenance: Provenance::Concrete { sb, .. }, .. }, _) =
scalar
{
tags.insert(*sb);
}
});
}
fn find_tags_in_memory(&mut self, tags: &mut FxHashSet<SbTag>) {
let this = self.eval_context_mut();
this.memory.alloc_map().iter(|it| {
for (_id, (_kind, alloc)) in it {
for (_size, prov) in alloc.provenance().iter() {
if let Provenance::Concrete { sb, .. } = prov {
tags.insert(*sb);
}
}
}
});
}
fn find_tags_in_locals(&mut self, tags: &mut FxHashSet<SbTag>) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
for frame in this.machine.threads.all_stacks().flatten() {
// Handle the return place of each frame
if let Ok(return_place) = frame.return_place.try_as_mplace() {
if let Some(Provenance::Concrete { sb, .. }) = return_place.ptr.provenance {
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);
}
}
};
for local in frame.locals.iter() {
let LocalValue::Live(value) = local.value else {
continue;
};
match value {
Operand::Immediate(Immediate::Scalar(Scalar::Ptr(ptr, _))) =>
if let Provenance::Concrete { sb, .. } = ptr.provenance {
tags.insert(sb);
},
Operand::Immediate(Immediate::ScalarPair(s1, s2)) => {
if let Scalar::Ptr(ptr, _) = s1 {
if let Provenance::Concrete { sb, .. } = ptr.provenance {
tags.insert(sb);
}
}
if let Scalar::Ptr(ptr, _) = s2 {
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);
}
}
Operand::Immediate(Immediate::Uninit)
| Operand::Immediate(Immediate::Scalar(Scalar::Int(_))) => {}
}
}
}
},
|tags, alloc| {
for (_size, prov) in alloc.provenance().iter() {
if let Provenance::Concrete { sb, .. } = prov {
tags.insert(*sb);
}
}
},
);
self.remove_unreachable_tags(tags);
Ok(())
}