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
1 change: 1 addition & 0 deletions changelog.d/9822-retained-growth-verifier.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix a false evacuation-verifier abort when a copying minor encounters a retained, non-moving array-growth alias, such as Solid's effect dependency array. Verification still follows the full forwarding chain and rejects nursery evacuation originals; old-page evacuation retains its strict checks.
2 changes: 1 addition & 1 deletion crates/perry-runtime/src/gc/copying.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1552,7 +1552,7 @@ pub(super) fn run_copied_minor_attempt(
if gc_verify_evacuation_enabled() {
let phase_start = trace_phase_start(trace);
let valid_ptrs = build_valid_pointer_set();
verify_evacuated_no_stale_forwarded_refs(&valid_ptrs);
verify_evacuated_no_stale_forwarded_refs(EvacuationVerifier::copying_minor(&valid_ptrs));
trace_phase_record(trace, "evacuation_verify", phase_start);
}

Expand Down
4 changes: 3 additions & 1 deletion crates/perry-runtime/src/gc/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,9 @@ impl GcCycleState {
trace_phase_record(&mut self.trace, "reference_rewrite", phase_start);
if gc_verify_evacuation_enabled() {
let phase_start = trace_phase_start(&self.trace);
verify_evacuated_no_stale_forwarded_refs(valid_ptrs);
verify_evacuated_no_stale_forwarded_refs(EvacuationVerifier::all_forwarded(
valid_ptrs,
));
trace_phase_record(&mut self.trace, "evacuation_verify", phase_start);
}
let released =
Expand Down
37 changes: 11 additions & 26 deletions crates/perry-runtime/src/gc/roots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ pub(super) enum RuntimeRootVisitMode<'a> {
valid_ptrs: &'a ValidPointerSet,
},
Verify {
valid_ptrs: &'a ValidPointerSet,
verifier: EvacuationVerifier<'a>,
surface: &'static str,
},
Copy {
Expand Down Expand Up @@ -807,12 +807,9 @@ impl<'a> RuntimeRootVisitor<'a> {
}
}

pub(super) fn for_verify(valid_ptrs: &'a ValidPointerSet, surface: &'static str) -> Self {
pub(super) fn for_verify(verifier: EvacuationVerifier<'a>, surface: &'static str) -> Self {
Self {
mode: RuntimeRootVisitMode::Verify {
valid_ptrs,
surface,
},
mode: RuntimeRootVisitMode::Verify { verifier, surface },
root_source_stats: None,
young_scope: false,
}
Expand Down Expand Up @@ -909,11 +906,8 @@ impl<'a> RuntimeRootVisitor<'a> {
RuntimeRootVisitMode::Rewrite { valid_ptrs } => {
try_rewrite_nanboxed_value(bits, valid_ptrs)
}
RuntimeRootVisitMode::Verify {
valid_ptrs,
surface,
} => {
if let Some(new_bits) = try_rewrite_nanboxed_value(bits, valid_ptrs) {
RuntimeRootVisitMode::Verify { verifier, surface } => {
if let Some(new_bits) = verifier.stale_nanboxed_value(bits) {
panic_stale_forwarded_reference(surface, 0, bits, new_bits);
}
None
Expand Down Expand Up @@ -941,11 +935,8 @@ impl<'a> RuntimeRootVisitor<'a> {
collector.rewrite_value_bits(bits)
}
RuntimeRootVisitMode::Rewrite { valid_ptrs } => try_rewrite_value(bits, valid_ptrs),
RuntimeRootVisitMode::Verify {
valid_ptrs,
surface,
} => {
if let Some(new_bits) = try_rewrite_value(bits, valid_ptrs) {
RuntimeRootVisitMode::Verify { verifier, surface } => {
if let Some(new_bits) = verifier.stale_value(bits) {
panic_stale_forwarded_reference(surface, 0, bits, new_bits);
}
None
Expand Down Expand Up @@ -981,11 +972,8 @@ impl<'a> RuntimeRootVisitor<'a> {
RuntimeRootVisitMode::CopyingMark { collector } => collector.visit_raw_addr(addr),
RuntimeRootVisitMode::CopyingRewrite { collector } => collector.rewrite_raw_addr(addr),
RuntimeRootVisitMode::Rewrite { valid_ptrs } => try_rewrite_raw_addr(addr, valid_ptrs),
RuntimeRootVisitMode::Verify {
valid_ptrs,
surface,
} => {
if let Some(new_addr) = try_rewrite_raw_addr(addr, valid_ptrs) {
RuntimeRootVisitMode::Verify { verifier, surface } => {
if let Some(new_addr) = verifier.stale_raw_addr(addr) {
panic_stale_forwarded_reference(
surface,
0,
Expand All @@ -1012,11 +1000,8 @@ impl<'a> RuntimeRootVisitor<'a> {
RuntimeRootVisitMode::CopyingCheck { .. } => None,
RuntimeRootVisitMode::CopyingMark { .. } => None,
RuntimeRootVisitMode::CopyingRewrite { collector } => collector.rewrite_raw_addr(addr),
RuntimeRootVisitMode::Verify {
valid_ptrs,
surface,
} => {
if let Some(new_addr) = try_rewrite_raw_addr(addr, valid_ptrs) {
RuntimeRootVisitMode::Verify { verifier, surface } => {
if let Some(new_addr) = verifier.stale_raw_addr(addr) {
panic_stale_forwarded_reference(surface, 0, addr as u64, new_addr as u64);
}
None
Expand Down
152 changes: 152 additions & 0 deletions crates/perry-runtime/src/gc/tests/forwarding_verification.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
use super::super::*;
use super::support::*;

// Solid's effect.sources array grows after promotion. Its owning computation
// retains the old array address, which remains a supported growth alias.
#[test]
fn copying_verifier_accepts_retained_array_growth_alias_in_old_field() {
let _guard = CopyingNurseryTestGuard::new(2);
let _verify_guard = VerifyEvacuationTestGuard::on();
let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
let (stub, _) = unsafe { alloc_old_test_array(1) };
let (holder, field) = unsafe { alloc_old_test_object(1) };
unsafe {
layout_init_pointer_free(stub as *mut u8);
layout_init_pointer_free(holder as *mut u8);
crate::object::store_object_field_slot(holder, 0, ptr_bits(stub as usize));
}
let grown = crate::array::js_array_push_f64(stub, 42.0);
assert_ne!(stub, grown);
assert!(crate::arena::pointer_in_old_gen(stub as usize));
assert!(crate::arena::pointer_in_old_gen(grown as usize));
js_shadow_slot_set(0, ptr_bits(holder as usize));
let young = young_leaf();
js_shadow_slot_set(1, ptr_bits(young));

let trace = collect_minor_trace(GcTriggerKind::Direct);

assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false);
assert!(trace.phase_us.contains_key("evacuation_verify"));
assert_ne!(js_shadow_slot_get(1), ptr_bits(young));
assert_eq!(unsafe { *field }, ptr_bits(stub as usize));
assert_eq!(crate::array::js_array_get_f64(stub, 1), 42.0);
}

#[test]
fn copying_verifier_accepts_retained_growth_chains_across_root_formats() {
let _guard = CopyingNurseryTestGuard::new(0);
let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
let (stub, _) = unsafe { alloc_old_test_array(1) };
let next = crate::array::js_array_grow(stub, 2);
let target = crate::array::js_array_grow(next, 4);
assert_ne!(stub, next);
assert_ne!(next, target);
let valid_ptrs = build_valid_pointer_set();
let verifier = EvacuationVerifier::copying_minor(&valid_ptrs);
let bits = ptr_bits(stub as usize);

// The ordinary rewrite and non-copying verifier still canonicalize every
// hop, including old arrays moved during old-page evacuation.
assert_eq!(
try_rewrite_value(bits, &valid_ptrs),
Some(ptr_bits(target as usize))
);
assert_eq!(
EvacuationVerifier::all_forwarded(&valid_ptrs).stale_value(bits),
Some(ptr_bits(target as usize))
);
let mut visitor = RuntimeRootVisitor::for_verify(verifier, "retained growth root");
assert_eq!(visitor.visit_nanbox_bits(bits), None);
assert_eq!(visitor.visit_heap_word_bits(stub as usize as u64), None);
assert_eq!(
visitor.visit_tagged_raw_addr(stub as usize, POINTER_TAG),
None
);
assert_eq!(visitor.visit_metadata_raw_addr(stub as usize), None);
verify_copy_only_scanner_bits(bits, verifier, "retained copy-only root");
let mut context = verifier;
perry_ffi_verify_root(
f64::from_bits(bits),
&mut context as *mut EvacuationVerifier<'_> as *mut c_void,
);
}

#[test]
fn copying_verifier_rejects_from_space_hops_even_through_retained_stubs() {
let _guard = CopyingNurseryTestGuard::new(0);
let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
let (stub, _) = unsafe { alloc_old_test_array(1) };
let from_space = crate::array::js_array_alloc(1);
let (target, _) = unsafe { alloc_old_test_array(1) };
assert!(super::super::fromspace_scan::is_from_space(
crate::arena::classify_heap_space(from_space as usize)
));
unsafe {
// Sabotage: array growth forbids an old -> young forwarding edge.
// The verifier must still reject it if it somehow occurs, even when
// the first hop is a retained old array and only the second is stale.
set_forwarding_address(
header_from_user_ptr(stub as *const u8),
from_space as *mut u8,
);
}
let valid_ptrs = build_valid_pointer_set();
let verifier = EvacuationVerifier::copying_minor(&valid_ptrs);
assert_eq!(
verifier.stale_raw_addr(stub as usize),
Some(from_space as usize),
"a retained stub must not reference even an unforwarded young array"
);
unsafe {
set_forwarding_address(
header_from_user_ptr(from_space as *const u8),
target as *mut u8,
);
}
for source in [stub, from_space] {
let bits = ptr_bits(source as usize);
assert_eq!(
verifier.stale_raw_addr(source as usize),
Some(target as usize)
);
assert_eq!(
verifier.stale_value(source as usize as u64),
Some(target as usize as u64)
);
assert_eq!(
verifier.stale_nanboxed_value(bits),
Some(ptr_bits(target as usize))
);
for format in 0..4 {
let failure = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
let mut visitor = RuntimeRootVisitor::for_verify(verifier, "from-space control");
match format {
0 => {
visitor.visit_nanbox_bits(bits);
}
1 => {
visitor.visit_heap_word_bits(source as usize as u64);
}
2 => {
visitor.visit_tagged_raw_addr(source as usize, POINTER_TAG);
}
_ => {
visitor.visit_metadata_raw_addr(source as usize);
}
}
}));
assert!(
failure.is_err(),
"root format {format} must reject a stale hop"
);
}
let failure = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| unsafe {
verify_slot(&bits, verifier, "from-space heap control");
}));
assert!(failure.is_err());
}
// Leave a valid retained alias for subsequent tests' heap walks.
unsafe {
set_forwarding_address(header_from_user_ptr(stub as *const u8), target as *mut u8);
}
}
1 change: 1 addition & 0 deletions crates/perry-runtime/src/gc/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ mod error_side_tables;
mod evacuation;
mod forwarded_stub_membership;
mod forwarding_target_validation;
mod forwarding_verification;
mod fromspace_protect;
mod fromspace_scan;
mod global_bootstrap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ fn test_evacuation_verify_detects_stale_forwarded_root_slot() {
js_shadow_slot_set(0, fixture.nursery_bits);

assert_panics_with("shadow stack roots", || {
verify_mutable_root_slots(&fixture.valid_ptrs);
verify_mutable_root_slots(EvacuationVerifier::all_forwarded(&fixture.valid_ptrs));
});

js_shadow_frame_pop(shadow);
Expand All @@ -994,8 +994,10 @@ fn test_evacuation_verify_detects_stale_forwarded_runtime_scanner_slot() {
);

assert_panics_with("runtime mutable root scanner", || {
let mut visitor =
RuntimeRootVisitor::for_verify(&fixture.valid_ptrs, "runtime mutable root scanner");
let mut visitor = RuntimeRootVisitor::for_verify(
EvacuationVerifier::all_forwarded(&fixture.valid_ptrs),
"runtime mutable root scanner",
);
promise_mutable_root_scanner(&mut visitor);
});

Expand All @@ -1020,7 +1022,7 @@ fn test_evacuation_verify_detects_stale_forwarded_dirty_range_slot() {
}

assert_panics_with("remembered dirty ranges", || {
verify_remembered_dirty_ranges(&valid_ptrs);
verify_remembered_dirty_ranges(EvacuationVerifier::all_forwarded(&valid_ptrs));
});

remembered_set_clear();
Expand All @@ -1036,7 +1038,11 @@ fn test_evacuation_verify_detects_stale_forwarded_heap_field() {
let header = header_from_user_ptr(old_obj as *const u8);
(*header).gc_flags |= GC_FLAG_MARKED;
assert_panics_with("heap fields", || {
verify_heap_object_fields(header, &fixture.valid_ptrs, "heap fields");
verify_heap_object_fields(
header,
EvacuationVerifier::all_forwarded(&fixture.valid_ptrs),
"heap fields",
);
});
(*header).gc_flags &= !GC_FLAG_MARKED;
}
Expand All @@ -1051,7 +1057,7 @@ fn test_evacuation_verify_copy_only_pinned_root_allows_non_forwarded_target() {
}
verify_copy_only_scanner_bits(
POINTER_TAG | (user as u64 & POINTER_MASK),
&valid_ptrs,
EvacuationVerifier::all_forwarded(&valid_ptrs),
"copy-only root scanner",
);
unsafe {
Expand All @@ -1065,7 +1071,7 @@ fn test_evacuation_verify_copy_only_root_rejects_forwarded_target() {
assert_panics_with("copy-only root scanner", || {
verify_copy_only_scanner_bits(
fixture.nursery_bits,
&fixture.valid_ptrs,
EvacuationVerifier::all_forwarded(&fixture.valid_ptrs),
"copy-only root scanner",
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ fn test_class_inheritance_side_table_roots_mark_and_rewrite() {
// A verify pass must not panic now that the slots point at the live
// (non-forwarded) evacuated objects.
scan_class_inheritance_roots_mut(&mut RuntimeRootVisitor::for_verify(
&valid_ptrs,
EvacuationVerifier::all_forwarded(&valid_ptrs),
"class inheritance side-table roots (test)",
));

Expand Down
Loading
Loading