Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog.d/11027-release-gc-sabotage.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### Tests

The remembered-set coverage sabotage witness now runs under release unit tests,
so the dirty-scan remembering arm can no longer regress behind a silently
ignored debug-only assertion.
6 changes: 3 additions & 3 deletions crates/perry-runtime/src/gc/sticky_remembered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ impl StickyRememberedSet {
}

/// How many of this set's entries the remembered set does NOT hold yet —
/// what `restore` would add. Read-only: the debug check of the coverage
/// restore asks this about objects it skipped.
#[cfg(debug_assertions)]
/// what `restore` would add. Read-only: the test/debug check of the
/// coverage restore asks this about objects it skipped.
#[cfg(any(test, debug_assertions))]
pub(super) fn count_not_yet_dirty(&self) -> usize {
let old_missing = super::barrier::DIRTY_OLD_PAGES.with(|s| {
let s = s.borrow();
Expand Down
18 changes: 6 additions & 12 deletions crates/perry-runtime/src/gc/tests/copy_slot_decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,13 @@ fn an_old_parents_edge_is_remembered_from_the_child_the_visit_decoded() {
);
}

/// In a release build `restore_surviving_dirty_coverage` would re-add the page
/// the arm failed to remember, which is why a forgotten remembered-set entry
/// is invisible to a survival check alone. In the debug build `cargo test`
/// runs, the same walk cross-checks the dirty scan's per-slot re-remembering
/// and refuses the disagreement — that refusal is this twin's observable.
/// In a production release build `restore_surviving_dirty_coverage` would
/// re-add the page the arm failed to remember, which is why a forgotten
/// remembered-set entry is invisible to a survival check alone. Unit-test
/// builds retain the same cross-check as debug builds, so this sabotage twin
/// proves the dirty scan's per-slot re-remembering remains load-bearing under
/// `cargo test --release` as well.
#[test]
// Debug-only by construction, as the doc comment above already states: in a
// release build `restore_surviving_dirty_coverage` re-adds the page, so the
// refusal this asserts never happens. Ignored rather than cfg'd out so the
// release run still reports it by name. Do NOT "fix" the test: it is correct,
// the profile changed what the code means. `[profile.gcaudit]` gives release
// codegen with assertions live and is where to exercise this under release.
#[cfg_attr(not(debug_assertions), ignore = "asserts a debug-only cross-check")]
fn sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check() {
let outcome = old_edge_across_two_minors(true);
assert!(
Expand Down
30 changes: 16 additions & 14 deletions crates/perry-runtime/src/gc/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,11 @@ pub(super) unsafe fn remember_evacuated_old_copy_young_slots(
/// pages the sticky restore just inserted. They are skipped; the walk is then
/// proportional to the objects the dirty scan could NOT fully cover
/// (multi-page arrays, owners of out-of-body buffers) instead of to every slot
/// on every dirty page. Under `debug_assertions` the skipped objects are
/// walked anyway and any page the walk would have ADDED is a panic — the
/// machine check of the equivalence argument above.
/// on every dirty page. Under `debug_assertions`, and in unit-test builds, the
/// skipped objects are walked anyway and any page the walk would have ADDED is
/// a panic — the machine check of the equivalence argument above. Including
/// `cfg(test)` keeps the check active in `cargo test --release` without adding
/// work to production release builds.
pub(super) fn restore_surviving_dirty_coverage(
snapshot: &RememberedDirtySnapshot,
covered: &crate::fast_hash::PtrHashSet<usize>,
Expand All @@ -367,7 +369,7 @@ fn restore_surviving_dirty_coverage_impl<const DIAGNOSTICS: bool>(
let mut parents_visited = 0usize;
let mut slots_visited = 0usize;
let mut slots_tracking = 0usize;
#[cfg(debug_assertions)]
#[cfg(any(test, debug_assertions))]
let mut skipped_sticky = StickyRememberedSet::default();
// Mirror scan_remembered_dirty_slots_copying's scan_header guards: the
// external dirty entries can carry headers the harness seeded
Expand Down Expand Up @@ -417,8 +419,8 @@ fn restore_surviving_dirty_coverage_impl<const DIAGNOSTICS: bool>(
crate::arena::old_arena_walk_objects_on_pages(&snapshot.dirty_old_pages, |hp| {
if covered.contains(&(hp as usize)) {
skipped += 1;
#[cfg(debug_assertions)]
debug_visit_covered_parent(hp as *mut GcHeader, &mut skipped_sticky);
#[cfg(any(test, debug_assertions))]
cross_check_covered_parent(hp as *mut GcHeader, &mut skipped_sticky);
return;
}
walked += 1;
Expand All @@ -432,8 +434,8 @@ fn restore_surviving_dirty_coverage_impl<const DIAGNOSTICS: bool>(
}
if covered.contains(&header_addr) {
skipped += 1;
#[cfg(debug_assertions)]
debug_visit_covered_parent(header_addr as *mut GcHeader, &mut skipped_sticky);
#[cfg(any(test, debug_assertions))]
cross_check_covered_parent(header_addr as *mut GcHeader, &mut skipped_sticky);
continue;
}
walked += 1;
Expand All @@ -456,7 +458,7 @@ fn restore_surviving_dirty_coverage_impl<const DIAGNOSTICS: bool>(
}
}
let added = sticky.restore_counted();
#[cfg(debug_assertions)]
#[cfg(any(test, debug_assertions))]
{
let would_add = skipped_sticky.count_not_yet_dirty();
assert_eq!(
Expand Down Expand Up @@ -484,11 +486,11 @@ fn restore_surviving_dirty_coverage_impl<const DIAGNOSTICS: bool>(
}
}

/// Debug twin of the restore's `visit_parent` for a skipped object: re-derive
/// what the full walk would have remembered so the caller can assert it adds
/// nothing beyond what the dirty scan already restored.
#[cfg(debug_assertions)]
fn debug_visit_covered_parent(header: *mut GcHeader, sticky: &mut StickyRememberedSet) {
/// Test/debug twin of the restore's `visit_parent` for a skipped object:
/// re-derive what the full walk would have remembered so the caller can assert
/// it adds nothing beyond what the dirty scan already restored.
#[cfg(any(test, debug_assertions))]
fn cross_check_covered_parent(header: *mut GcHeader, sticky: &mut StickyRememberedSet) {
unsafe {
if header.is_null() {
return;
Expand Down
Loading