From db1401f7ab537b2de33a8e1c229a4b86db82d230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 20:15:41 +0200 Subject: [PATCH 1/2] fix(gc): allow retained array-growth aliases in copying verification --- changelog.d/4644-retained-growth-verifier.md | 1 + crates/perry-runtime/src/gc/copying.rs | 2 +- crates/perry-runtime/src/gc/cycle.rs | 4 +- crates/perry-runtime/src/gc/roots.rs | 37 ++-- .../src/gc/tests/forwarding_verification.rs | 152 ++++++++++++++++ crates/perry-runtime/src/gc/tests/mod.rs | 1 + .../tests/runtime_roots/callback_scanners.rs | 20 ++- .../runtime_roots/side_table_scanners.rs | 2 +- crates/perry-runtime/src/gc/verify.rs | 163 +++++++++++++----- scripts/gc_runtime_root_holders.json | 4 +- 10 files changed, 305 insertions(+), 81 deletions(-) create mode 100644 changelog.d/4644-retained-growth-verifier.md create mode 100644 crates/perry-runtime/src/gc/tests/forwarding_verification.rs diff --git a/changelog.d/4644-retained-growth-verifier.md b/changelog.d/4644-retained-growth-verifier.md new file mode 100644 index 0000000000..f67793ceb8 --- /dev/null +++ b/changelog.d/4644-retained-growth-verifier.md @@ -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. diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 9724feb04c..b9f8519701 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -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); } diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index b34e9eff3e..66777e520a 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -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 = diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index 054bee16f7..c40c29cf9c 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -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 { @@ -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, } @@ -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 @@ -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 @@ -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, @@ -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 diff --git a/crates/perry-runtime/src/gc/tests/forwarding_verification.rs b/crates/perry-runtime/src/gc/tests/forwarding_verification.rs new file mode 100644 index 0000000000..5e74aa4d40 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/forwarding_verification.rs @@ -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); + } +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 2d3300e1cd..186fe29187 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -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; diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs index 26aaa9fc0d..bcf72ccefd 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs @@ -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); @@ -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); }); @@ -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(); @@ -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; } @@ -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 { @@ -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", ); }); diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs index 10ee93d3fe..4e14a61002 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs @@ -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)", )); diff --git a/crates/perry-runtime/src/gc/verify.rs b/crates/perry-runtime/src/gc/verify.rs index 0332bcf3b8..9d6bb0b26f 100644 --- a/crates/perry-runtime/src/gc/verify.rs +++ b/crates/perry-runtime/src/gc/verify.rs @@ -26,9 +26,11 @@ pub(super) fn try_rewrite_nanboxed_value(bits: u64, valid_ptrs: &ValidPointerSet /// #8174: refuses a forwarding target that is not a heap object start, in /// lockstep with [`CopyingNurseryCollector::rewrite_raw_addr`](super::copying). /// -/// The lockstep is the point. This function is what the VERIFY pass runs -/// (`RuntimeRootVisitMode::Verify`), and it panics whenever it can rewrite a -/// slot the rewrite pass left alone. Tightening only the rewrite pass would +/// The lockstep is the point. The VERIFY pass shares this forwarding walker +/// and panics when it finds a stale alias the rewrite pass left alone. +/// Copying verification permits retained array-growth aliases through +/// [`EvacuationVerifier`], without changing source/target validation. +/// Tightening only the rewrite pass would /// therefore have turned a silently-corrupt rewrite into a `PERRY_GC_VERIFY_ /// EVACUATION` abort blaming an innocent scanner — the two walkers must reach /// the same verdict or the verifier is measuring the difference between them @@ -36,6 +38,82 @@ pub(super) fn try_rewrite_nanboxed_value(bits: u64, valid_ptrs: &ValidPointerSet /// stronger than the copier's heap-region test, so this only changes the case /// where a genuinely LIVE forwarded object's target word is corrupt. pub(super) fn try_rewrite_raw_addr(ptr_addr: usize, valid_ptrs: &ValidPointerSet) -> Option { + follow_forwarding_raw_addr(ptr_addr, valid_ptrs, |_, _| true) +} + +/// The copying minor retains non-moving array-growth stubs. Other evacuation +/// paths rewrite every forwarding alias before releasing moved originals. +/// Carry that distinction through every verifier surface, including FFI roots. +#[derive(Clone, Copy)] +pub(super) struct EvacuationVerifier<'a> { + valid_ptrs: &'a ValidPointerSet, + copying_minor: bool, +} + +impl<'a> EvacuationVerifier<'a> { + pub(super) fn all_forwarded(valid_ptrs: &'a ValidPointerSet) -> Self { + Self { + valid_ptrs, + copying_minor: false, + } + } + + /// Must run before the copying minor resets from-space and flips survivors. + pub(super) fn copying_minor(valid_ptrs: &'a ValidPointerSet) -> Self { + Self { + valid_ptrs, + copying_minor: true, + } + } + + pub(super) fn stale_raw_addr(self, addr: usize) -> Option { + follow_forwarding_raw_addr(addr, self.valid_ptrs, |source, target| { + if !self.copying_minor { + return true; + } + // Only array growth creates permanent forwarding aliases. The + // copying minor neither moves nor frees these non-moving sources. + // Both ends must be retained arrays. An old -> young growth edge + // is forbidden even if the young target was never forwarded. + // Follow the whole chain so an indirect unsafe hop is also caught. + !self.retained_growth_array(source) || !self.retained_growth_array(target) + }) + } + + fn retained_growth_array(self, addr: usize) -> bool { + if !self.valid_ptrs.contains(&addr) { + return false; + } + let header = unsafe { header_from_user_ptr(addr as *const u8) }; + (unsafe { (*header).obj_type == GC_TYPE_ARRAY }) + && matches!( + crate::arena::classify_heap_space(addr), + crate::arena::HeapSpace::Old + | crate::arena::HeapSpace::Longlived + | crate::arena::HeapSpace::PromotedYoung + ) + } + + pub(super) fn stale_value(self, bits: u64) -> Option { + let word = decode_root_word(bits)?; + Some(word.encode(self.stale_raw_addr(word.addr())?)) + } + + pub(super) fn stale_nanboxed_value(self, bits: u64) -> Option { + let tag = bits & TAG_MASK; + if tag != POINTER_TAG && tag != STRING_TAG && tag != BIGINT_TAG { + return None; + } + let addr = self.stale_raw_addr((bits & POINTER_MASK) as usize)?; + Some(tag | (addr as u64 & POINTER_MASK)) + } +} + +fn follow_forwarding_raw_addr( + ptr_addr: usize, + valid_ptrs: &ValidPointerSet, + must_rewrite: impl Fn(usize, usize) -> bool, +) -> Option { if ptr_addr == 0 { return None; } @@ -57,8 +135,8 @@ pub(super) fn try_rewrite_raw_addr(ptr_addr: usize, valid_ptrs: &ValidPointerSet if !accept_forwarding_target(next) { return None; } + rewrote |= must_rewrite(current, next); current = next; - rewrote = true; } } rewrote.then_some(current) @@ -87,9 +165,13 @@ pub(super) unsafe fn rewrite_slot(slot: *mut u64, valid_ptrs: &ValidPointerSet) } #[inline] -pub(super) unsafe fn verify_slot(slot: *const u64, valid_ptrs: &ValidPointerSet, surface: &str) { +pub(super) unsafe fn verify_slot( + slot: *const u64, + verifier: EvacuationVerifier<'_>, + surface: &str, +) { let bits = *slot; - if let Some(new_bits) = try_rewrite_value(bits, valid_ptrs) { + if let Some(new_bits) = verifier.stale_value(bits) { panic_stale_forwarded_reference(surface, slot as usize, bits, new_bits); } } @@ -1122,7 +1204,7 @@ pub(super) fn verify_minor_unmarked_young_children_report(phase: &str) { pub(super) unsafe fn verify_heap_object_fields( header: *mut GcHeader, - valid_ptrs: &ValidPointerSet, + verifier: EvacuationVerifier<'_>, surface: &'static str, ) { let flags = (*header).gc_flags; @@ -1131,7 +1213,7 @@ pub(super) unsafe fn verify_heap_object_fields( } visit_gc_rewrite_slots(header, |slot| unsafe { slot.record_layout_read(); - verify_slot(slot.slot as *const u64, valid_ptrs, surface); + verify_slot(slot.slot as *const u64, verifier, surface); }); } @@ -1263,13 +1345,13 @@ pub(super) fn rewrite_mutable_registered_roots_with_sources( visit_ffi_mutable_registered_roots_with_sources(&mut visitor, root_sources); } -pub(super) fn verify_mutable_root_slots(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_mutable_root_slots(verifier: EvacuationVerifier<'_>) { visit_mutable_root_slots(|slot| unsafe { let bits = slot.read(); if bits == 0 { return; } - if let Some(new_bits) = try_rewrite_value(bits, valid_ptrs) { + if let Some(new_bits) = verifier.stale_value(bits) { let surface = match slot.kind { MutableRootSlotKind::ShadowStack => "shadow stack roots", MutableRootSlotKind::NativeStack => "native stack-map roots", @@ -1280,9 +1362,9 @@ pub(super) fn verify_mutable_root_slots(valid_ptrs: &ValidPointerSet) { }); } -pub(super) fn verify_mutable_registered_roots(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_mutable_registered_roots(verifier: EvacuationVerifier<'_>) { let scanners: Vec = MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()); - let mut visitor = RuntimeRootVisitor::for_verify(valid_ptrs, "runtime mutable root scanner"); + let mut visitor = RuntimeRootVisitor::for_verify(verifier, "runtime mutable root scanner"); for entry in scanners { (entry.scanner)(&mut visitor); } @@ -1291,72 +1373,67 @@ pub(super) fn verify_mutable_registered_roots(valid_ptrs: &ValidPointerSet) { pub(super) fn verify_copy_only_scanner_bits( bits: u64, - valid_ptrs: &ValidPointerSet, + verifier: EvacuationVerifier<'_>, surface: &'static str, ) { - if let Some(new_bits) = try_rewrite_nanboxed_value(bits, valid_ptrs) { + if let Some(new_bits) = verifier.stale_nanboxed_value(bits) { panic_stale_forwarded_reference(surface, 0, bits, new_bits); } } -pub(super) struct RegisteredRootVerifyContext { - pub(super) valid_ptrs: *const ValidPointerSet, -} - pub(super) extern "C" fn perry_ffi_verify_root(value: f64, ctx: *mut c_void) { if ctx.is_null() { return; } - let ctx = unsafe { &*(ctx as *const RegisteredRootVerifyContext) }; - if ctx.valid_ptrs.is_null() { - return; - } - let valid_ptrs = unsafe { &*ctx.valid_ptrs }; - verify_copy_only_scanner_bits(value.to_bits(), valid_ptrs, "ffi copy-only root scanner"); + let verifier = unsafe { *(ctx as *const EvacuationVerifier<'_>) }; + verify_copy_only_scanner_bits(value.to_bits(), verifier, "ffi copy-only root scanner"); } -pub(super) fn verify_copy_only_registered_roots(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_copy_only_registered_roots(verifier: EvacuationVerifier<'_>) { let scanners: Vec = ROOT_SCANNERS.with(|s| s.borrow().clone()); for scanner in scanners { scanner(&mut |value: f64| { - verify_copy_only_scanner_bits(value.to_bits(), valid_ptrs, "copy-only root scanner"); + verify_copy_only_scanner_bits(value.to_bits(), verifier, "copy-only root scanner"); }); } let ffi_scanners: Vec = FFI_ROOT_SCANNERS.with(|s| s.borrow().clone()); - let mut ctx = RegisteredRootVerifyContext { - valid_ptrs: valid_ptrs as *const ValidPointerSet, - }; - let ctx = &mut ctx as *mut RegisteredRootVerifyContext as *mut c_void; + let mut ctx = verifier; + let ctx = &mut ctx as *mut EvacuationVerifier<'_> as *mut c_void; for scanner in ffi_scanners { scanner(perry_ffi_verify_root, ctx); } } -pub(super) fn verify_remembered_dirty_ranges(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_remembered_dirty_ranges(verifier: EvacuationVerifier<'_>) { let snapshot = remembered_dirty_snapshot(); let mut stats = RememberedSetTraceStats::default(); let mut verify_dirty_slot = |slot: *mut u64, _stats: &mut RememberedSetTraceStats| unsafe { - verify_slot(slot as *const u64, valid_ptrs, "remembered dirty ranges"); + verify_slot(slot as *const u64, verifier, "remembered dirty ranges"); }; - scan_remembered_dirty_slot_ranges(&snapshot, valid_ptrs, &mut stats, &mut verify_dirty_slot); + scan_remembered_dirty_slot_ranges( + &snapshot, + verifier.valid_ptrs, + &mut stats, + &mut verify_dirty_slot, + ); for header_addr in snapshot.fallback_headers { let user_ptr = header_addr + GC_HEADER_SIZE; - if !valid_ptrs.contains(&user_ptr) { + if !verifier.valid_ptrs.contains(&user_ptr) { continue; } unsafe { verify_heap_object_fields( header_addr as *mut GcHeader, - valid_ptrs, + verifier, "remembered fallback headers", ); } } } -pub(super) fn verify_heap_objects(valid_ptrs: &ValidPointerSet) { +pub(super) fn verify_heap_objects(verifier: EvacuationVerifier<'_>) { let verify_one = |header: *mut GcHeader| unsafe { let flags = (*header).gc_flags; if flags & GC_FLAG_FORWARDED != 0 { @@ -1373,7 +1450,7 @@ pub(super) fn verify_heap_objects(valid_ptrs: &ValidPointerSet) { return; } } - verify_heap_object_fields(header, valid_ptrs, "heap fields"); + verify_heap_object_fields(header, verifier, "heap fields"); }; crate::arena::arena_walk_objects(|hp| verify_one(hp as *mut GcHeader)); MALLOC_STATE.with(|s| { @@ -1384,12 +1461,12 @@ pub(super) fn verify_heap_objects(valid_ptrs: &ValidPointerSet) { }); } -pub(super) fn verify_evacuated_no_stale_forwarded_refs(valid_ptrs: &ValidPointerSet) { - verify_mutable_root_slots(valid_ptrs); - verify_mutable_registered_roots(valid_ptrs); - verify_copy_only_registered_roots(valid_ptrs); - verify_remembered_dirty_ranges(valid_ptrs); - verify_heap_objects(valid_ptrs); +pub(super) fn verify_evacuated_no_stale_forwarded_refs(verifier: EvacuationVerifier<'_>) { + verify_mutable_root_slots(verifier); + verify_mutable_registered_roots(verifier); + verify_copy_only_registered_roots(verifier); + verify_remembered_dirty_ranges(verifier); + verify_heap_objects(verifier); } /// Top-level Phase C4b-γ-2 entry: rewrite every reference site we diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index d2dc1990d0..33fbe97090 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -276,7 +276,7 @@ "file": "crates/perry-runtime/src/gc/census.rs", "name": "PASS1_MARKED", "verdict": "non_moving_snapshot", - "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-pinned 2026-09-05 for the #9740 hot-TLS conversion of this file: the sole change is `thread_local!` \u2192 `crate::perry_thread_local!`, a macro-name swap with identical declaration syntax and `.with()` call sites. No control flow, no phase boundary, and no storage semantics change. Re-audited 2026-09-06 (train128) after #9794's GC diagnostics touched `gc/mod.rs` and `gc/policy.rs`: both gain diagnostic module declarations and counters only \u2014 no mark/sweep control flow, and the census bracketing in `step_mark_propagation` / `step_sweep` is unchanged. Re-audited for #9794's GC diagnostics: `gc/mod.rs` gains `mod diag_sites;` / `mod survival_diag;`, a re-export, a `diag_sites::full_started(...)` call at TRIGGER time (before mark propagation begins), and exit-time reporting. Nothing executes between mark-complete and sweep-entry, so the window is unchanged.", + "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-pinned 2026-09-05 for the #9740 hot-TLS conversion of this file: the sole change is `thread_local!` \u2192 `crate::perry_thread_local!`, a macro-name swap with identical declaration syntax and `.with()` call sites. No control flow, no phase boundary, and no storage semantics change. Re-audited 2026-09-06 (train128) after #9794's GC diagnostics touched `gc/mod.rs` and `gc/policy.rs`: both gain diagnostic module declarations and counters only \u2014 no mark/sweep control flow, and the census bracketing in `step_mark_propagation` / `step_sweep` is unchanged. Re-audited for #9794's GC diagnostics: `gc/mod.rs` gains `mod diag_sites;` / `mod survival_diag;`, a re-export, a `diag_sites::full_started(...)` call at TRIGGER time (before mark propagation begins), and exit-time reporting. Nothing executes between mark-complete and sweep-entry, so the window is unchanged. Re-audited 2026-09-06 for the retained array-growth verifier fix: the cycle.rs change passes the existing non-copying evacuation verifier an explicit all-forwarded policy. That call remains in minor finalization, outside the synchronous full-cycle census window; its root and heap reads do not allocate GC objects, move objects, or invoke JS callbacks. The mark-complete and sweep-entry boundaries are unchanged.", "window": { "start": { "file": "crates/perry-runtime/src/gc/census.rs", @@ -292,7 +292,7 @@ }, "sources": { "crates/perry-runtime/src/gc/census.rs": "388414f9629f196e84673e91bebd04bdcdcabdaa180252d2dfe4b82d1b49ca5a", - "crates/perry-runtime/src/gc/cycle.rs": "763d552271b8e983a796b4e9648cd8ee984a0602b2b56aeefdb8713c0049c31f", + "crates/perry-runtime/src/gc/cycle.rs": "2e2f5adca2229f74409e01a1cb571e2147cd8a33f58d0976711fce98d4777309", "crates/perry-runtime/src/gc/mod.rs": "7dd42b9506a97e6844fd3225dc53dfd59512631784750f58ff72208d68595481", "crates/perry-runtime/src/gc/policy.rs": "fa8e9fa188d50bd92c3fbe23950a195906baf387f3a208497791bdd23f1c72db", "crates/perry-runtime/src/gc/progress.rs": "a5ad3971bbe4047229ca57325234780daa85921dbc778e1c08dff4ad07ccfb96" From 350d1b4639046d0cb7557dd11bd1390b01cd82ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 20:16:07 +0200 Subject: [PATCH 2/2] docs: number retained-growth verifier changeset for PR 9822 --- ...tained-growth-verifier.md => 9822-retained-growth-verifier.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{4644-retained-growth-verifier.md => 9822-retained-growth-verifier.md} (100%) diff --git a/changelog.d/4644-retained-growth-verifier.md b/changelog.d/9822-retained-growth-verifier.md similarity index 100% rename from changelog.d/4644-retained-growth-verifier.md rename to changelog.d/9822-retained-growth-verifier.md