From 49d232fab9bec519ed796506eaf4f3590a681ef5 Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Tue, 22 Sep 2026 04:32:14 +0000 Subject: [PATCH] test(gc): a zero-slot test fixture has room for the named-store floor (fixes #10941) `alloc_{nursery,old}_test_object(0)` allocated exactly an `ObjectHeader` and left the receiver unstamped, on the reasoning recorded above it that "a zero-slot fixture needs no descriptor at all - the derived bound is 0 either way". A named-property write does not respect that bound. The inline/overflow boundary is `max(object_live_slot_count(obj), INLINE_SLOT_FLOOR)` and the floor is 2, so the first two keys written to a zero-slot fixture store into inline slots 0 and 1 of an object that has none. Those two words are the next cell. Every caller before PR #10938 only ever set a `[[Prototype]]` on one, so nothing had written a named property and the hazard was invisible; it presents as a wrong read now and a SIGSEGV somewhere unrelated later. Both fixtures now allocate `max(field_count, INLINE_SLOT_FLOOR)` slots while PUBLISHING the bound as `field_count`, so the collector still traces exactly `field_count` slots and the descriptor-count accounting the original comment protects is unchanged. Witness: `gc::tests::zero_slot_fixture` asserts the ALLOCATION, for both the nursery and the old-generation fixture, and reddens by name when the change is reverted. An end-to-end pin - six named writes, read back - was written and deliberately dropped: without the fix it does not fail, it dumps core, which under `--test-threads=1` takes the other ~4,200 results with it. That is recorded in the module doc. Suite: 4218 passed / 0 failed / 6 ignored, `--test-threads=1`, both arms. --- crates/perry-runtime/src/gc/tests/mod.rs | 1 + crates/perry-runtime/src/gc/tests/support.rs | 30 +++++++-- .../src/gc/tests/zero_slot_fixture.rs | 66 +++++++++++++++++++ 3 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/zero_slot_fixture.rs diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 7093b9a5cc..df17d6c908 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -82,6 +82,7 @@ mod smoke; mod start_bitmap; mod step_bounds; pub(super) mod support; +mod zero_slot_fixture; mod survival_diag; mod sweep_described_runs; mod sweep_hole_rebuild; diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index e187ce488e..c7d978e914 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -852,7 +852,18 @@ pub(super) unsafe fn alloc_old_test_object( crate::object::shapes::shape_descriptor_ensure(std::ptr::null(), 0, field_count) .expect("shape id range exhausted in a test fixture") }; - let payload = std::mem::size_of::() + field_count as usize * 8; + // #10941: a named-property write does not respect "the derived bound is + // 0". The inline/overflow boundary is + // `max(object_live_slot_count(obj), INLINE_SLOT_FLOOR)` with a floor of 2, + // so the first two keys written to a zero-slot fixture store into inline + // slots 0 and 1 of an object that has none — those two words are THE NEXT + // CELL. It presents as a wrong read now and a SIGSEGV somewhere unrelated + // during the next collection. Allocate to the floor; the PUBLISHED bound + // stays `field_count`, so the collector still traces exactly `field_count` + // slots and the descriptor-count accounting sibling tests assert on is + // unchanged. + let allocated_slots = std::cmp::max(field_count as usize, crate::object::INLINE_SLOT_FLOOR); + let payload = std::mem::size_of::() + allocated_slots * 8; let obj = crate::arena::arena_alloc_gc_old(payload, 8, GC_TYPE_OBJECT) as *mut crate::object::ObjectHeader; (*obj).class_id = 0; @@ -860,7 +871,7 @@ pub(super) unsafe fn alloc_old_test_object( (*obj).meta = std::ptr::null_mut(); let fields = (obj as *mut u8).add(std::mem::size_of::()) as *mut u64; - for i in 0..field_count as usize { + for i in 0..allocated_slots { *fields.add(i) = 0; } (obj, fields) @@ -880,7 +891,18 @@ pub(super) unsafe fn alloc_nursery_test_object( crate::object::shapes::shape_descriptor_ensure(std::ptr::null(), 0, field_count) .expect("shape id range exhausted in a test fixture") }; - let payload = std::mem::size_of::() + field_count as usize * 8; + // #10941: a named-property write does not respect "the derived bound is + // 0". The inline/overflow boundary is + // `max(object_live_slot_count(obj), INLINE_SLOT_FLOOR)` with a floor of 2, + // so the first two keys written to a zero-slot fixture store into inline + // slots 0 and 1 of an object that has none — those two words are THE NEXT + // CELL. It presents as a wrong read now and a SIGSEGV somewhere unrelated + // during the next collection. Allocate to the floor; the PUBLISHED bound + // stays `field_count`, so the collector still traces exactly `field_count` + // slots and the descriptor-count accounting sibling tests assert on is + // unchanged. + let allocated_slots = std::cmp::max(field_count as usize, crate::object::INLINE_SLOT_FLOOR); + let payload = std::mem::size_of::() + allocated_slots * 8; let obj = crate::arena::arena_alloc_gc(payload, 8, GC_TYPE_OBJECT) as *mut crate::object::ObjectHeader; (*obj).class_id = 0; @@ -888,7 +910,7 @@ pub(super) unsafe fn alloc_nursery_test_object( (*obj).meta = std::ptr::null_mut(); let fields = (obj as *mut u8).add(std::mem::size_of::()) as *mut u64; - for i in 0..field_count as usize { + for i in 0..allocated_slots { *fields.add(i) = 0; } (obj, fields) diff --git a/crates/perry-runtime/src/gc/tests/zero_slot_fixture.rs b/crates/perry-runtime/src/gc/tests/zero_slot_fixture.rs new file mode 100644 index 0000000000..059a2daaea --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/zero_slot_fixture.rs @@ -0,0 +1,66 @@ +//! #10941: a zero-slot test fixture must still have room for a named store. +//! +//! `alloc_{nursery,old}_test_object(0)` allocated exactly an `ObjectHeader` +//! and left the receiver unstamped, on the reasoning that "the derived bound +//! is 0 either way". A named-property write does not respect that bound: the +//! inline/overflow boundary is +//! `max(object_live_slot_count(obj), INLINE_SLOT_FLOOR)` and the floor is 2, +//! so the first two keys stored into a zero-slot fixture land in inline slots +//! 0 and 1 of an object that has none — THE NEXT CELL. +//! +//! Every caller before PR #10938 only ever set a `[[Prototype]]` on one, so +//! nothing had written a named property to one and the hazard was invisible. +//! It presents as a wrong read now and a SIGSEGV inside an unrelated +//! collection later, which is why this is pinned structurally: the assertions +//! below are about the ALLOCATION, and they cannot pass vacuously. +//! +//! An end-to-end pin — write six named properties to a zero-slot fixture and +//! read them back — was written and then deliberately dropped: without the +//! fix it does not fail, it DUMPS CORE, which under `--test-threads=1` takes +//! the other ~4,200 results in the process with it. The structural assertions +//! redden with a message instead, and they redden for the same reason. + +use super::support::{alloc_nursery_test_object, alloc_old_test_object}; + +unsafe fn payload_bytes(obj: *mut crate::object::ObjectHeader) -> usize { + let header = (obj as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; + (*header).size as usize - crate::gc::GC_HEADER_SIZE +} + +fn floor_bytes() -> usize { + std::mem::size_of::() + crate::object::INLINE_SLOT_FLOOR * 8 +} + +#[test] +fn a_zero_slot_nursery_fixture_has_room_for_the_named_store_floor() { + let _global = crate::gc::global_side_table_test_lock(); + unsafe { + let (obj, _) = alloc_nursery_test_object(0); + let payload = payload_bytes(obj); + assert!( + payload >= floor_bytes(), + "a named-property write on this fixture stores into inline slots 0 \ + and 1 — the store path's floor is \ + `max(object_live_slot_count(obj), INLINE_SLOT_FLOOR)` and never \ + zero — but the allocation carries {payload} payload bytes against \ + the {} it would need. Those words are the NEXT CELL (#10941).", + floor_bytes() + ); + } +} + +#[test] +fn a_zero_slot_old_fixture_has_room_for_the_named_store_floor() { + let _global = crate::gc::global_side_table_test_lock(); + unsafe { + let (obj, _) = alloc_old_test_object(0); + let payload = payload_bytes(obj); + assert!( + payload >= floor_bytes(), + "the old-generation twin of the nursery fixture has the same hole: \ + {payload} payload bytes against the {} a named store needs \ + (#10941).", + floor_bytes() + ); + } +}