Skip to content

Commit 0027b12

Browse files
authored
Rollup merge of #118029 - saethlin:allocid-gc, r=RalfJung
Expand Miri's BorTag GC to a Provenance GC As suggested in #3080 (comment) We previously solved memory growth issues associated with the Stacked Borrows and Tree Borrows runtimes with a GC. But of course we also have state accumulation associated with whole allocations elsewhere in the interpreter, and this PR starts tackling those. To do this, we expand the visitor for the GC so that it can visit a BorTag or an AllocId. Instead of collecting all live AllocIds into a single HashSet, we just collect from the Machine itself then go through an accessor `InterpCx::is_alloc_live` which checks a number of allocation data structures in the core interpreter. This avoids the overhead of all the inserts that collecting their keys would require. r? ``@RalfJung``
2 parents 34aa10e + f46e192 commit 0027b12

40 files changed

+424
-308
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737

3838
- name: Set the tag GC interval to 1 on linux
3939
if: runner.os == 'Linux'
40-
run: echo "MIRIFLAGS=-Zmiri-tag-gc=1" >> $GITHUB_ENV
40+
run: echo "MIRIFLAGS=-Zmiri-provenance-gc=1" >> $GITHUB_ENV
4141

4242
# Cache the global cargo directory, but NOT the local `target` directory which
4343
# we cannot reuse anyway when the nightly changes (and it grows quite large

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -411,10 +411,10 @@ to Miri failing to detect cases of undefined behavior in a program.
411411
without an explicit value), `none` means it never recurses, `scalar` means it only recurses for
412412
types where we would also emit `noalias` annotations in the generated LLVM IR (types passed as
413413
individual scalars or pairs of scalars). Setting this to `none` is **unsound**.
414-
* `-Zmiri-tag-gc=<blocks>` configures how often the pointer tag garbage collector runs. The default
415-
is to search for and remove unreachable tags once every `10000` basic blocks. Setting this to
416-
`0` disables the garbage collector, which causes some programs to have explosive memory usage
417-
and/or super-linear runtime.
414+
* `-Zmiri-provenance-gc=<blocks>` configures how often the pointer provenance garbage collector runs.
415+
The default is to search for and remove unreachable provenance once every `10000` basic blocks. Setting
416+
this to `0` disables the garbage collector, which causes some programs to have explosive memory
417+
usage and/or super-linear runtime.
418418
* `-Zmiri-track-alloc-id=<id1>,<id2>,...` shows a backtrace when the given allocations are
419419
being allocated or freed. This helps in debugging memory leaks and
420420
use after free bugs. Specifying this argument multiple times does not overwrite the previous

src/bin/miri.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,10 +531,10 @@ fn main() {
531531
Err(err) => show_error!("-Zmiri-report-progress requires a `u32`: {}", err),
532532
};
533533
miri_config.report_progress = Some(interval);
534-
} else if let Some(param) = arg.strip_prefix("-Zmiri-tag-gc=") {
534+
} else if let Some(param) = arg.strip_prefix("-Zmiri-provenance-gc=") {
535535
let interval = match param.parse::<u32>() {
536536
Ok(i) => i,
537-
Err(err) => show_error!("-Zmiri-tag-gc requires a `u32`: {}", err),
537+
Err(err) => show_error!("-Zmiri-provenance-gc requires a `u32`: {}", err),
538538
};
539539
miri_config.gc_interval = interval;
540540
} else if let Some(param) = arg.strip_prefix("-Zmiri-measureme=") {

src/borrow_tracker/mod.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ pub struct FrameState {
7575
protected_tags: SmallVec<[(AllocId, BorTag); 2]>,
7676
}
7777

78-
impl VisitTags for FrameState {
79-
fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) {
78+
impl VisitProvenance for FrameState {
79+
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
8080
// `protected_tags` are already recorded by `GlobalStateInner`.
8181
}
8282
}
@@ -110,10 +110,10 @@ pub struct GlobalStateInner {
110110
unique_is_unique: bool,
111111
}
112112

113-
impl VisitTags for GlobalStateInner {
114-
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
113+
impl VisitProvenance for GlobalStateInner {
114+
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
115115
for &tag in self.protected_tags.keys() {
116-
visit(tag);
116+
visit(None, Some(tag));
117117
}
118118
// The only other candidate is base_ptr_tags, and that does not need visiting since we don't ever
119119
// GC the bottommost/root tag.
@@ -236,6 +236,10 @@ impl GlobalStateInner {
236236
tag
237237
})
238238
}
239+
240+
pub fn remove_unreachable_allocs(&mut self, allocs: &LiveAllocs<'_, '_, '_>) {
241+
self.base_ptr_tags.retain(|id, _| allocs.is_live(*id));
242+
}
239243
}
240244

241245
/// Which borrow tracking method to use
@@ -503,11 +507,11 @@ impl AllocState {
503507
}
504508
}
505509

506-
impl VisitTags for AllocState {
507-
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
510+
impl VisitProvenance for AllocState {
511+
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
508512
match self {
509-
AllocState::StackedBorrows(sb) => sb.visit_tags(visit),
510-
AllocState::TreeBorrows(tb) => tb.visit_tags(visit),
513+
AllocState::StackedBorrows(sb) => sb.visit_provenance(visit),
514+
AllocState::TreeBorrows(tb) => tb.visit_provenance(visit),
511515
}
512516
}
513517
}

src/borrow_tracker/stacked_borrows/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,10 +462,10 @@ impl Stacks {
462462
}
463463
}
464464

465-
impl VisitTags for Stacks {
466-
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
465+
impl VisitProvenance for Stacks {
466+
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
467467
for tag in self.exposed_tags.iter().copied() {
468-
visit(tag);
468+
visit(None, Some(tag));
469469
}
470470
}
471471
}

src/borrow_tracker/tree_borrows/diagnostics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ impl<'tcx> Tree {
200200
/// Climb the tree to get the tag of a distant ancestor.
201201
/// Allows operations on tags that are unreachable by the program
202202
/// but still exist in the tree. Not guaranteed to perform consistently
203-
/// if `tag-gc=1`.
203+
/// if `provenance-gc=1`.
204204
fn nth_parent(&self, tag: BorTag, nth_parent: u8) -> Option<BorTag> {
205205
let mut idx = self.tag_mapping.get(&tag).unwrap();
206206
for _ in 0..nth_parent {

src/borrow_tracker/tree_borrows/tree.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -742,11 +742,11 @@ impl Tree {
742742
}
743743
}
744744

745-
impl VisitTags for Tree {
746-
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
745+
impl VisitProvenance for Tree {
746+
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
747747
// To ensure that the root never gets removed, we visit it
748748
// (the `root` node of `Tree` is not an `Option<_>`)
749-
visit(self.nodes.get(self.root).unwrap().tag)
749+
visit(None, Some(self.nodes.get(self.root).unwrap().tag))
750750
}
751751
}
752752

src/concurrency/data_race.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -790,9 +790,9 @@ pub struct VClockAlloc {
790790
alloc_ranges: RefCell<RangeMap<MemoryCellClocks>>,
791791
}
792792

793-
impl VisitTags for VClockAlloc {
794-
fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) {
795-
// No tags here.
793+
impl VisitProvenance for VClockAlloc {
794+
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
795+
// No tags or allocIds here.
796796
}
797797
}
798798

@@ -1404,8 +1404,8 @@ pub struct GlobalState {
14041404
pub track_outdated_loads: bool,
14051405
}
14061406

1407-
impl VisitTags for GlobalState {
1408-
fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) {
1407+
impl VisitProvenance for GlobalState {
1408+
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
14091409
// We don't have any tags.
14101410
}
14111411
}

src/concurrency/init_once.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ pub(super) struct InitOnce<'mir, 'tcx> {
4545
data_race: VClock,
4646
}
4747

48-
impl<'mir, 'tcx> VisitTags for InitOnce<'mir, 'tcx> {
49-
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
48+
impl<'mir, 'tcx> VisitProvenance for InitOnce<'mir, 'tcx> {
49+
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
5050
for waiter in self.waiters.iter() {
51-
waiter.callback.visit_tags(visit);
51+
waiter.callback.visit_provenance(visit);
5252
}
5353
}
5454
}

src/concurrency/sync.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,10 @@ pub(crate) struct SynchronizationState<'mir, 'tcx> {
181181
pub(super) init_onces: IndexVec<InitOnceId, InitOnce<'mir, 'tcx>>,
182182
}
183183

184-
impl<'mir, 'tcx> VisitTags for SynchronizationState<'mir, 'tcx> {
185-
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
184+
impl<'mir, 'tcx> VisitProvenance for SynchronizationState<'mir, 'tcx> {
185+
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
186186
for init_once in self.init_onces.iter() {
187-
init_once.visit_tags(visit);
187+
init_once.visit_provenance(visit);
188188
}
189189
}
190190
}

0 commit comments

Comments
 (0)