From 3f8b9cb9ca5f316e4e3e42bf3e4e49959417ae7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 28 Jul 2026 18:29:34 +0200 Subject: [PATCH 1/3] fix(codegen): initialize typed-shape layout on the standalone-ctor exit (#6921) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lower_new_impl` had one exit that returned a freshly allocated class instance without emitting `js_gc_init_typed_shape_layout`. Every other `new` exit emits it. An instance produced there is left at `GC_LAYOUT_POINTER_FREE` with no `TypedLayoutDescriptor` — the one layout state where the per-store `layout_note_slot` call is load-bearing for GC correctness rather than a precision hint, because it is the only writer of the pointer-mask bit the collector reads. That blocks the full pointer-masked layout-note elision (the larger half of Phase 4b.1 in #6919, which had to narrow to the value-only predicate because of it). Emit the layout init on that exit too, so the invariant "a user-class instance reaching a class-field store carries a typed descriptor, or is explicitly GC_LAYOUT_UNKNOWN" is total. REACHABILITY — I could not build a reproducer, and on investigation the arm appears to be DEAD, which is a stronger claim than #6921 makes. The issue correctly notes that `force_ctor_call` and `ctor_alias_collision` both pre-check `local_constructor_symbol_exists` and so cannot reach it, leaving the `ctx.class_stack` recursion guard as the only way in. But the missing step is that `call_local_constructor_symbol` returns `None` only when `ctx.methods` lacks `(class.name, "_constructor")`, and: - `lower_new_impl` resolves `class` exclusively from `ctx.classes` (new.rs:237 is the only binding, `ctx.classes.get(class_name)`); - `ctx.classes` IS the `class_table` (codegen/mod.rs passes `&class_table` to compile_function / compile_method); - `build_method_names` iterates `class_table.values()` and inserts `(c.name, "{c.name}_constructor")` UNCONDITIONALLY for every entry, local and imported alike (method_registry.rs:112-119). So every class that can reach the branch has the key, and the `None` arm cannot be taken. Empirically: an instrumented compiler that logs on reaching the arm reported ZERO hits across all 430 `test-files/test_gap_*.ts` (codegen-only, `--no-link --no-cache`) plus three hand-written self-referential construction shapes (self-construction in a method, a recursive own constructor, and a field initializer constructing its own class) — all of which do enter the recursion-guarded branch, and all of which take the `Some` arm that already emitted the init. The emitter lands anyway: the invariant should hold by construction at this exit rather than by an accident of the registry that a change to `build_method_names`, or a new `ctx.classes` population path, could silently revoke. Cost is zero — dead path today, and `emit_typed_shape_layout_init` is itself a no-op for a class with no `class_keys_globals` entry. Because the arm is unreachable, NO behavioral test can fail before and pass after; claiming otherwise would be dishonest. What is testable, and what actually carries the risk, is the premise the fix rests on: the instance reaching this exit has had NO constructor run, so the layout init sees all-`undefined` fields. `gc::tests::layout_trace:: typed_shape_layout_init_on_unconstructed_instance_is_conservative` pins all three properties of that: 1. `js_object_alloc_class_inline_keys` really does leave the instance at GC_LAYOUT_POINTER_FREE with no descriptor (why the note is load-bearing there, i.e. why the gap mattered); 2. a raw-f64 mask over `undefined` fields is REFUSED and the object is downgraded to GC_LAYOUT_UNKNOWN — the conservative state — never left POINTER_FREE and never given a mask that misdescribes it (why emitting the init here is SAFE); 3. a pointer-only mask over `undefined` fields IS installed, and a subsequent raw pointer store into that slot is traced with no `layout_note_slot` call at all (why emitting it is USEFUL). Verified green under the full GC stress matrix: PERRY_GC_FORCE_EVACUATE=1, PERRY_GC_VERIFY_EVACUATION=1, PERRY_GEN_GC=0, PERRY_WRITE_BARRIERS=0, and four combinations including all-four-at-once. --- crates/perry-codegen/src/lower_call/new.rs | 35 +++++ .../src/gc/tests/layout_trace.rs | 121 ++++++++++++++++++ 2 files changed, 156 insertions(+) diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index bc5f58dc18..0c7f321eea 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -902,6 +902,41 @@ fn lower_new_impl( ctx.block() .call(DOUBLE, "js_new_target_set", &[(DOUBLE, prev)]); } + // #6921: `call_local_constructor_symbol` returned `None` — this module + // has no `_constructor` entry, so no constructor ran and the + // instance leaves here exactly as `js_object_alloc_class_*` produced + // it. Every OTHER `new` exit initializes the typed-shape layout; this + // one used to return the instance at `GC_LAYOUT_POINTER_FREE` with no + // `TypedLayoutDescriptor`, the one state in which the per-store + // `layout_note_slot` call is load-bearing for GC correctness rather + // than a precision hint — so eliding that note (Phase 4b.1) could + // strand a live child on an object the collector scans zero slots of. + // + // Initialize it here too, so the invariant "a user-class instance + // reaching a class-field store carries a typed descriptor, or is + // explicitly `GC_LAYOUT_UNKNOWN`" is total. This is safe by + // construction rather than by reasoning about this path: the fields + // are still `TAG_UNDEFINED` (no ctor ran), and `init_typed_shape_layout` + // validates every live field word before promoting — a raw-f64 slot + // holding `undefined` fails `layout_raw_f64_bits` and the object lands + // in `GC_LAYOUT_UNKNOWN`, the conservative state, instead of a wrong + // mask. `emit_typed_shape_layout_init` is itself a no-op for a class + // with no `class_keys_globals` entry. + // + // Reachability, measured (not assumed): this arm is currently DEAD. + // `call_local_constructor_symbol` returns `None` only when + // `ctx.methods` lacks `(class.name, "_constructor")`, but + // `lower_new_impl` resolves `class` exclusively from `ctx.classes` + // (the `class_table`), and `build_method_names` iterates + // `class_table.values()` inserting that key unconditionally for every + // entry — local and imported alike. So no class reaching here can miss + // it. An instrumented compiler over the whole `test_gap_*` corpus plus + // hand-written recursive-construction shapes never hit this arm. + // The emitter stays anyway: the invariant must hold by construction at + // this exit, not by an accident of the registry that a future change + // to `build_method_names` (or a new `ctx.classes` population path) + // could silently revoke. + emit_typed_shape_layout_init(ctx, class_name, &obj_handle); return Ok(obj_box); } diff --git a/crates/perry-runtime/src/gc/tests/layout_trace.rs b/crates/perry-runtime/src/gc/tests/layout_trace.rs index 93ea7008fa..45f3b222cb 100644 --- a/crates/perry-runtime/src/gc/tests/layout_trace.rs +++ b/crates/perry-runtime/src/gc/tests/layout_trace.rs @@ -1672,3 +1672,124 @@ fn test_int32_store_without_typed_descriptor_is_left_verbatim() { "with no intact descriptor there is no raw-f64 contract to uphold — bits stay verbatim" ); } + +/// #6921 — the `lower_new_impl` standalone-constructor exit returns a freshly +/// allocated class instance on which NO constructor has run, so the +/// `js_gc_init_typed_shape_layout` that exit now emits sees an object whose +/// every field is still `undefined`. That is the premise the fix rests on, so +/// pin it here rather than reasoning about it. +/// +/// Three properties, in the order they matter: +/// +/// 1. A fresh instance really is left at `GC_LAYOUT_POINTER_FREE` with no +/// descriptor — the one state in which the per-store `layout_note_slot` +/// call is load-bearing for GC correctness rather than a precision hint. +/// That is why an exit which skips the layout init blocks the note elision. +/// 2. A raw-f64 mask CANNOT be honoured over `undefined` fields, and +/// `init_typed_shape_layout` must downgrade to `GC_LAYOUT_UNKNOWN` — the +/// conservative state — never install a mask that fails to describe the +/// live words. This is what makes emitting the init on that exit SAFE. +/// 3. A pointer-only mask IS installed over `undefined` fields, so a later +/// pointer store into that slot is traced even though `layout_note_slot` +/// never ran. This is what makes emitting it USEFUL. +#[test] +fn typed_shape_layout_init_on_unconstructed_instance_is_conservative() { + let _guard = GcTestIsolationGuard::new(); + clear_marks(); + clear_mark_seeds(); + + let layout_state = |obj: *mut crate::object::ObjectHeader| unsafe { + (*header_from_user_ptr(obj as *const u8))._reserved & GC_LAYOUT_STATE_MASK + }; + + // (1) The unconstructed instance, exactly as `js_object_alloc_class_*` + // hands it to the standalone-ctor exit. + let fresh = crate::object::js_object_alloc_class_inline_keys(0, 0, 2, std::ptr::null_mut()); + assert_eq!( + layout_state(fresh), + GC_LAYOUT_POINTER_FREE, + "a fresh class instance is POINTER_FREE — the collector scans zero \ + slots on it until something publishes a pointer bit" + ); + assert!( + !layout_has_typed_descriptor(fresh as usize), + "and carries no typed descriptor" + ); + + // (2) Raw-f64 mask over all-`undefined` fields must land in UNKNOWN. + let raw_only = crate::object::js_object_alloc_class_inline_keys(0, 0, 2, std::ptr::null_mut()); + let raw_mask = [0b01u64]; + js_gc_init_typed_shape_layout( + raw_only as u64, + 2, + raw_mask.as_ptr(), + raw_mask.len() as u32, + std::ptr::null(), + 0, + ); + assert_eq!( + layout_state(raw_only), + GC_LAYOUT_UNKNOWN, + "`undefined` is not raw-f64 bits, so the descriptor must be refused \ + and the object downgraded to the conservative state — never left \ + POINTER_FREE, never given a mask that misdescribes it" + ); + assert!( + !layout_has_typed_descriptor(raw_only as usize), + "a refused descriptor must not be installed" + ); + + // (3) Pointer-only mask over all-`undefined` fields IS installed, and the + // slot is traced on a later store without any `layout_note_slot` call. + let ptr_only = crate::object::js_object_alloc_class_inline_keys(0, 0, 2, std::ptr::null_mut()); + let ptr_mask = [0b01u64]; + js_gc_init_typed_shape_layout( + ptr_only as u64, + 2, + std::ptr::null(), + 0, + ptr_mask.as_ptr(), + ptr_mask.len() as u32, + ); + assert_eq!( + layout_state(ptr_only), + GC_LAYOUT_SIDE_MASK, + "a pointer mask is compatible with `undefined` fields and is installed" + ); + assert_eq!( + test_layout_pointer_slot_count(ptr_only as usize, 2), + Some(1), + "slot 0 is published as pointer-bearing" + ); + + // The child is reachable ONLY through that slot; write it with a raw store + // so no `layout_note_slot` runs, then prove tracing still finds it. + let child = crate::string::js_string_from_bytes(b"6921-child".as_ptr(), 10); + let child_header = unsafe { header_from_user_ptr(child as *mut u8) }; + let fields = unsafe { + (ptr_only as *mut u8).add(std::mem::size_of::()) as *mut u64 + }; + unsafe { + std::ptr::write(fields, STRING_TAG | (child as u64 & POINTER_MASK)); + } + + let valid_ptrs = build_valid_pointer_set(); + let parent_bits = POINTER_TAG | (ptr_only as u64 & POINTER_MASK); + assert!( + try_mark_value(parent_bits, &valid_ptrs), + "test setup: the instance marks as a root" + ); + trace_marked_objects(&valid_ptrs); + unsafe { + assert_ne!( + (*child_header).gc_flags & GC_FLAG_MARKED, + 0, + "the typed descriptor alone must make the pointer slot traceable — \ + this is the liveness `layout_note_slot` would otherwise have had \ + to establish store-by-store" + ); + } + + clear_marks(); + clear_mark_seeds(); +} From c15959c2c18e6645bd3753a2058feecb08d32b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 28 Jul 2026 18:34:37 +0200 Subject: [PATCH 2/3] docs(changelog): fragment for #6930 (typed-shape layout on standalone-ctor exit, #6921) --- changelog.d/6930-typed-shape-layout-ctor-exit.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog.d/6930-typed-shape-layout-ctor-exit.md diff --git a/changelog.d/6930-typed-shape-layout-ctor-exit.md b/changelog.d/6930-typed-shape-layout-ctor-exit.md new file mode 100644 index 0000000000..455b96b717 --- /dev/null +++ b/changelog.d/6930-typed-shape-layout-ctor-exit.md @@ -0,0 +1,9 @@ +**codegen: initialize the typed-shape layout on `lower_new_impl`'s standalone-constructor exit (#6921).** + +`lower_new_impl` had one exit that returned a freshly allocated class instance without emitting `js_gc_init_typed_shape_layout`; every other `new` exit emits it. Such an instance is left at `GC_LAYOUT_POINTER_FREE` with no `TypedLayoutDescriptor` — the one layout state where the per-store `layout_note_slot` call is load-bearing for GC correctness rather than a precision hint, since it is the only writer of the pointer-mask bit the collector reads. That blocked the full pointer-masked layout-note elision (the larger half of Phase 4b.1 in #6919, which had to narrow to the value-only predicate because of it). + +**The arm appears to be unreachable today, and no reproducer exists.** `call_local_constructor_symbol` returns `None` only when `ctx.methods` lacks `(class.name, "_constructor")`, but `lower_new_impl` resolves `class` exclusively from `ctx.classes` (`new.rs:237`), `ctx.classes` *is* the `class_table`, and `build_method_names` iterates `class_table.values()` inserting that key unconditionally for every entry — local and imported alike (`method_registry.rs:112-119`). An instrumented compiler reported zero hits across all 430 `test-files/test_gap_*.ts` (codegen-only) plus three hand-written self-referential construction shapes (self-construction in a method, a recursive own constructor, a field initializer constructing its own class) — all of which do enter the recursion-guarded branch and all of which take the `Some` arm that already emitted the init. + +The emitter lands anyway so the invariant "a user-class instance reaching a class-field store carries a typed descriptor, or is explicitly `GC_LAYOUT_UNKNOWN`" holds by construction at this exit rather than by an accident of the registry that a future change could silently revoke. Cost is zero — dead path today, and `emit_typed_shape_layout_init` is itself a no-op for a class with no `class_keys_globals` entry. + +Because the arm is unreachable, no behavioral test can fail before and pass after. What is testable is the premise the fix rests on — the instance reaching this exit has had no constructor run, so the layout init sees all-`undefined` fields. `gc::tests::layout_trace::typed_shape_layout_init_on_unconstructed_instance_is_conservative` pins that a fresh instance really is `GC_LAYOUT_POINTER_FREE` with no descriptor; that a raw-f64 mask over `undefined` fields is refused and downgraded to `GC_LAYOUT_UNKNOWN` (why emitting the init is safe); and that a pointer-only mask is installed and its slot traced with no `layout_note_slot` call (why emitting it is useful). Green across the GC stress matrix: `PERRY_GC_FORCE_EVACUATE=1`, `PERRY_GC_VERIFY_EVACUATION=1`, `PERRY_GEN_GC=0`, `PERRY_WRITE_BARRIERS=0` and combinations. From 96e10b8ac24cdd08b8486bc0fd49ff10c4166677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 28 Jul 2026 19:07:03 +0200 Subject: [PATCH 3/3] test(gc): root the #6921 premise test's handles across GC points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review on #6930. The test held `ptr_only` and `child` as raw pointers across GC-capable calls — `js_string_from_bytes` allocates, and evacuation moves arena objects, so the subsequent raw field write and the `POINTER_TAG | ptr_only` mark could both have named from-space. This is the same unrooted-handle-across-allocation bug class as #6655, in a test whose whole job is to validate GC correctness. An unsound test is worse than no test: it can pass for the wrong reason and mask exactly the layout/tracing behavior it exists to pin. Fixed properly rather than minimally: - every instance is rooted in a `RuntimeHandleScope` the moment it is allocated, and re-read through the handle after any later allocation (`handle_user`), so no raw pointer crosses a GC point; - `child_header` is derived from the refreshed child handle at the point of use instead of being captured before the trace; - the hand-driven mark starts from `clear_marks()` so a collection during the allocations cannot leave the instance pre-marked and turn the `try_mark_value` setup assertion into a false negative. The registration is the part that is easy to get wrong, so it is now explicit and documented: `GcTestIsolationGuard` (via `ScopedRootScannerRegistryGuard`) `mem::take`s the thread's `MUTABLE_ROOT_SCANNERS`, and the runtime-handle scanner goes with it. A `RuntimeHandleScope` opened inside that guard roots NOTHING until the scanner is put back. `register_runtime_handle_root_scanner_for_tests` moves from `tests/runtime_roots.rs` to `tests/support.rs` (21 call sites there are unaffected — it is glob-imported) and carries a doc comment spelling out the trap. Crucially, the rooting is now LOAD-BEARING rather than decorative. The test drives a real `collect_minor_trace(GcTriggerKind::Direct)` after the child write, with `ConservativeScanDisabledGuard` pinning the native-stack scan OFF — so the raw Rust locals are no longer a safety net and the handle scope is the only thing keeping the objects alive. Verified: with the `register_runtime_handle_root_scanner_for_tests()` line removed the test FAILS (`test_layout_pointer_slot_count` → `None` — the instance and its descriptor were reclaimed); with it, it passes. That collection also buys a new assertion worth having: the typed descriptor must survive the collection and any relocation, since the note-elision premise depends on it staying intact, not merely on being installed once. Stress matrix re-run, 9/9 green: PERRY_GC_FORCE_EVACUATE=1, PERRY_GC_VERIFY_EVACUATION=1, PERRY_GEN_GC=0, PERRY_WRITE_BARRIERS=0 and four combinations including all-four-at-once. `gc::tests::layout_trace` + `gc::tests::runtime_roots` together: 113 passed, 0 failed. (Widening a stress arm to the whole `runtime_roots` suite surfaces 34 failures under `PERRY_GEN_GC=0` / `PERRY_WRITE_BARRIERS=0` — all pre-existing `*_copied_minor_gc` tests that require the copying nursery, which is ineligible without generational GC and barriers. None of them are tests touched here.) --- .../src/gc/tests/layout_trace.rs | 93 ++++++++++++++----- .../src/gc/tests/runtime_roots.rs | 10 +- crates/perry-runtime/src/gc/tests/support.rs | 20 ++++ 3 files changed, 93 insertions(+), 30 deletions(-) diff --git a/crates/perry-runtime/src/gc/tests/layout_trace.rs b/crates/perry-runtime/src/gc/tests/layout_trace.rs index 45f3b222cb..b2cce8a815 100644 --- a/crates/perry-runtime/src/gc/tests/layout_trace.rs +++ b/crates/perry-runtime/src/gc/tests/layout_trace.rs @@ -1695,32 +1695,50 @@ fn test_int32_store_without_typed_descriptor_is_left_verbatim() { #[test] fn typed_shape_layout_init_on_unconstructed_instance_is_conservative() { let _guard = GcTestIsolationGuard::new(); - clear_marks(); - clear_mark_seeds(); - - let layout_state = |obj: *mut crate::object::ObjectHeader| unsafe { - (*header_from_user_ptr(obj as *const u8))._reserved & GC_LAYOUT_STATE_MASK + // `GcTestIsolationGuard` takes the thread's mutable-root scanner registry, + // which includes the runtime-handle scanner. Put it back BEFORE opening the + // scope below, or the handles are decorative and every object here is an + // unrooted raw pointer held across a GC-capable allocation — the same bug + // class (#6655) this test exists to be free of. + register_runtime_handle_root_scanner_for_tests(); + let scope = RuntimeHandleScope::new(); + clear_marks(); + clear_mark_seeds(); + + // Every allocation below is a GC point, and evacuation MOVES arena objects, + // so nothing may be held as a raw pointer across one. Each instance is + // rooted in `scope` the moment it is created and re-read through + // `handle_user` after any later allocation. + fn handle_user(handle: &RuntimeHandle<'_>) -> usize { + (handle.get_nanbox_u64() & POINTER_MASK) as usize + } + let layout_state = |user: usize| unsafe { + (*header_from_user_ptr(user as *const u8))._reserved & GC_LAYOUT_STATE_MASK }; + let alloc_instance = + || crate::object::js_object_alloc_class_inline_keys(0, 0, 2, std::ptr::null_mut()) as usize; // (1) The unconstructed instance, exactly as `js_object_alloc_class_*` // hands it to the standalone-ctor exit. - let fresh = crate::object::js_object_alloc_class_inline_keys(0, 0, 2, std::ptr::null_mut()); + let fresh = scope.root_nanbox_u64(ptr_bits(alloc_instance())); + let fresh_user = handle_user(&fresh); assert_eq!( - layout_state(fresh), + layout_state(fresh_user), GC_LAYOUT_POINTER_FREE, "a fresh class instance is POINTER_FREE — the collector scans zero \ slots on it until something publishes a pointer bit" ); assert!( - !layout_has_typed_descriptor(fresh as usize), + !layout_has_typed_descriptor(fresh_user), "and carries no typed descriptor" ); // (2) Raw-f64 mask over all-`undefined` fields must land in UNKNOWN. - let raw_only = crate::object::js_object_alloc_class_inline_keys(0, 0, 2, std::ptr::null_mut()); + let raw_only = scope.root_nanbox_u64(ptr_bits(alloc_instance())); + let raw_only_user = handle_user(&raw_only); let raw_mask = [0b01u64]; js_gc_init_typed_shape_layout( - raw_only as u64, + raw_only_user as u64, 2, raw_mask.as_ptr(), raw_mask.len() as u32, @@ -1728,23 +1746,24 @@ fn typed_shape_layout_init_on_unconstructed_instance_is_conservative() { 0, ); assert_eq!( - layout_state(raw_only), + layout_state(raw_only_user), GC_LAYOUT_UNKNOWN, "`undefined` is not raw-f64 bits, so the descriptor must be refused \ and the object downgraded to the conservative state — never left \ POINTER_FREE, never given a mask that misdescribes it" ); assert!( - !layout_has_typed_descriptor(raw_only as usize), + !layout_has_typed_descriptor(raw_only_user), "a refused descriptor must not be installed" ); // (3) Pointer-only mask over all-`undefined` fields IS installed, and the // slot is traced on a later store without any `layout_note_slot` call. - let ptr_only = crate::object::js_object_alloc_class_inline_keys(0, 0, 2, std::ptr::null_mut()); + let ptr_only = scope.root_nanbox_u64(ptr_bits(alloc_instance())); + let ptr_only_user = handle_user(&ptr_only); let ptr_mask = [0b01u64]; js_gc_init_typed_shape_layout( - ptr_only as u64, + ptr_only_user as u64, 2, std::ptr::null(), 0, @@ -1752,35 +1771,65 @@ fn typed_shape_layout_init_on_unconstructed_instance_is_conservative() { ptr_mask.len() as u32, ); assert_eq!( - layout_state(ptr_only), + layout_state(ptr_only_user), GC_LAYOUT_SIDE_MASK, "a pointer mask is compatible with `undefined` fields and is installed" ); assert_eq!( - test_layout_pointer_slot_count(ptr_only as usize, 2), + test_layout_pointer_slot_count(ptr_only_user, 2), Some(1), "slot 0 is published as pointer-bearing" ); // The child is reachable ONLY through that slot; write it with a raw store // so no `layout_note_slot` runs, then prove tracing still finds it. - let child = crate::string::js_string_from_bytes(b"6921-child".as_ptr(), 10); - let child_header = unsafe { header_from_user_ptr(child as *mut u8) }; + let child = scope + .root_nanbox_u64(string_bits( + crate::string::js_string_from_bytes(b"6921-child".as_ptr(), 10) as usize, + )); + // Allocating the child was a GC point: re-read the instance rather than + // reusing `ptr_only_user`, which may now name from-space. + let ptr_only_user = handle_user(&ptr_only); let fields = unsafe { - (ptr_only as *mut u8).add(std::mem::size_of::()) as *mut u64 + (ptr_only_user as *mut u8).add(std::mem::size_of::()) + as *mut u64 }; unsafe { - std::ptr::write(fields, STRING_TAG | (child as u64 & POINTER_MASK)); + std::ptr::write(fields, child.get_nanbox_u64()); } + // Force a real collection here, with the conservative native-stack scan + // pinned OFF. That makes the `RuntimeHandleScope` above LOAD-BEARING rather + // than decorative: the raw Rust locals are no longer a safety net, so the + // scope is the only thing keeping these objects alive. Drop the scanner + // registration at the top of this test and this collection reclaims them. + { + let _scan = ConservativeScanDisabledGuard::new(); + let _ = collect_minor_trace(GcTriggerKind::Direct); + } + // Re-read everything through the handles — a copied minor relocates + // nursery objects and rewrites the rooted slots. + let ptr_only_user = handle_user(&ptr_only); + assert_eq!( + test_layout_pointer_slot_count(ptr_only_user, 2), + Some(1), + "the typed descriptor must survive the collection (and any relocation) \ + — the note-elision premise depends on it staying intact, not just on \ + it being installed once" + ); + + // A collection may have left objects marked; start the hand-driven mark + // from a known-clean state so the assertions below mean what they say. + clear_marks(); + clear_mark_seeds(); let valid_ptrs = build_valid_pointer_set(); - let parent_bits = POINTER_TAG | (ptr_only as u64 & POINTER_MASK); assert!( - try_mark_value(parent_bits, &valid_ptrs), + try_mark_value(ptr_bits(handle_user(&ptr_only)), &valid_ptrs), "test setup: the instance marks as a root" ); trace_marked_objects(&valid_ptrs); unsafe { + let child_header = header_from_user_ptr(handle_user(&child) as *const u8); assert_ne!( (*child_header).gc_flags & GC_FLAG_MARKED, 0, diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots.rs b/crates/perry-runtime/src/gc/tests/runtime_roots.rs index 1d0feea78a..6e21a175e6 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots.rs @@ -106,14 +106,8 @@ fn assert_moved_closure_ptr(bits: u64, original: usize) -> usize { rewritten } -fn register_runtime_handle_root_scanner_for_tests() { - gc_register_budgeted_mutable_root_scanner_with_source( - scan_runtime_handle_roots_mut, - scan_runtime_handle_roots_mut_step, - new_runtime_handle_root_scan_state, - MutableRootScannerSource::RuntimeHandles, - ); -} +// `register_runtime_handle_root_scanner_for_tests` moved to `super::support` +// so the layout/tracing tests can root handles the same way (#6930 review). #[test] fn test_scoped_root_scanner_registry_guard_restores_counts() { diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index 325fbfffb4..c0a4ef1028 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -229,6 +229,26 @@ impl Drop for ConservativeScanAutoGuard { } } +/// Put the runtime-handle mutable-root scanner back into this thread's +/// registry. +/// +/// REQUIRED before a `RuntimeHandleScope` roots anything inside +/// `ScopedRootScannerRegistryGuard` / `GcTestIsolationGuard` / +/// `CopyingNurseryTestGuard`: those guards `mem::take` the thread's +/// `MUTABLE_ROOT_SCANNERS` so a collection sees exactly the roots the test +/// installs, and the runtime-handle scanner goes with it. Without this call a +/// `RuntimeHandleScope` inside such a test is decorative — its handles are +/// neither marked nor rewritten, so a raw pointer held across a GC-capable +/// call is silently unrooted and the test can pass for the wrong reason. +pub(super) fn register_runtime_handle_root_scanner_for_tests() { + gc_register_budgeted_mutable_root_scanner_with_source( + scan_runtime_handle_roots_mut, + scan_runtime_handle_roots_mut_step, + new_runtime_handle_root_scan_state, + MutableRootScannerSource::RuntimeHandles, + ); +} + /// Pin this thread's conservative-scan mode to `Disabled` for the guard's /// lifetime, restoring the prior override on drop. ///