diff --git a/changelog.d/9808-element-shape-transfer-gate.md b/changelog.d/9808-element-shape-transfer-gate.md new file mode 100644 index 0000000000..b7d3b19b3f --- /dev/null +++ b/changelog.d/9808-element-shape-transfer-gate.md @@ -0,0 +1,25 @@ +**A relocated array whose element-shape proof does not exist no longer takes +the side table to find that out** (#9792). + +`transfer_element_shape` runs from `layout_transfer` for every relocated +array — growth forwarding, a copying minor, an old-gen defrag. It already +computes `had_bit` for free from two header words it has read anyway, and +then took `ELEMENT_SHAPES`' `RefCell` and hashed both addresses regardless, +for two removes that on the overwhelmingly common path remove nothing. It now +returns when neither address advertises a proof. + +The gate has exactly one safe shape and both halves are pinned by +`a_transfer_skips_the_table_only_when_neither_address_advertises_a_proof`. +Skipping on `!had_bit` alone would be wrong: a destination that still +advertises a proof describes storage the move has just replaced, so that case +keeps the full fail-closed path. + +What skipping leaves behind is a record at an address whose bit is clear, and +that state was already part of the design rather than new: the bit is the sole +authority for a read (`element_shape_proof` returns `None` before touching the +table, and `note_element_store` is gated on the same bit), and `establish` +draws every identity from `ELEMENT_SHAPE_PROOF_SEQ` rather than from whatever +record sits at the address — precisely so a survivor cannot donate its epoch +to the next array proven there. `prune_dead_element_shape_owners` drops it on +the next collection, the same footprint-only guarantee a fail-closed transfer +already relied on. The test asserts both defences directly. diff --git a/crates/perry-runtime/src/array/element_shape.rs b/crates/perry-runtime/src/array/element_shape.rs index 0158dbad33..29f0f245cc 100644 --- a/crates/perry-runtime/src/array/element_shape.rs +++ b/crates/perry-runtime/src/array/element_shape.rs @@ -672,6 +672,25 @@ pub(crate) fn transfer_element_shape(old_user: usize, new_user: usize) { // exactly the versioning a consumer guards on. let had_bit = array_gc_header(old_user as *const ArrayHeader) .is_some_and(|old_header| header_has_bit(old_header)); + // #9792: neither address advertises a proof, so there is nothing to + // move and nothing to fail closed about — the `clear_bit` below would + // clear a bit that is already clear. Skipping is what the siblings in + // `gc::layout_tables` do with their emptiness flag, decided here from + // the header words this function has already read rather than from a + // side-table probe. + // + // What it leaves behind is a record at an address whose bit is clear, + // and that state is already part of the design: the bit is the sole + // authority for a read (`element_shape_proof` returns `None` without + // touching the table), and `establish` draws every identity from + // `ELEMENT_SHAPE_PROOF_SEQ` rather than from whatever record sits at + // the address, precisely so a survivor cannot donate its epoch to the + // next array established there. `prune_dead_element_shape_owners` + // drops it on the next collection, which is the same footprint-only + // guarantee a fail-closed transfer already relied on. + if !had_bit && !header_has_bit(new_header) { + return; + } let moved = ELEMENT_SHAPES.with(|m| { let mut map = m.borrow_mut(); map.remove(&new_user); @@ -795,6 +814,16 @@ pub(crate) unsafe fn test_element_shape_bit_set(arr: *const ArrayHeader) -> bool array_gc_header(arr).is_some_and(|header| header_has_bit(header)) } +/// Clear the advertising bit and leave the record in the table — the survivor +/// state a skipped [`transfer_element_shape`] produces, and the one the +/// bit-is-authority rule has to hold up under. +#[cfg(test)] +pub(crate) unsafe fn test_clear_element_shape_bit_only(arr: *mut ArrayHeader) { + if let Some(header) = array_gc_header(arr) { + clear_bit(header); + } +} + #[cfg(test)] #[path = "element_shape_tests.rs"] mod tests; diff --git a/crates/perry-runtime/src/array/element_shape_tests.rs b/crates/perry-runtime/src/array/element_shape_tests.rs index 80199a91a0..fa93b4f76c 100644 --- a/crates/perry-runtime/src/array/element_shape_tests.rs +++ b/crates/perry-runtime/src/array/element_shape_tests.rs @@ -541,6 +541,71 @@ fn a_fail_closed_transfer_leaves_no_record_for_the_next_array_to_inherit() { ); } +#[test] +fn a_transfer_skips_the_table_only_when_neither_address_advertises_a_proof() { + // #9792: `transfer_element_shape` runs for every relocated array, and its + // `had_bit` verdict is free — two header words it has already read. When + // neither address advertises a proof it takes the side table anyway, for + // two removes that can only remove what no reader could reach. The gate + // that skips that has exactly ONE safe shape, and this pins both halves. + // + // Arm 1 — neither bit set: skipping is correct, and the record it leaves + // behind stays unreadable and cannot donate its identity. + // Arm 2 — the DESTINATION still advertises one: skipping would leave a + // live proof describing storage that has just been overwritten, so the + // gate must NOT fire on `!had_bit` alone. + let _serialized = test_serialize(); + + // Arm 1. + let src = built_from_pushes(CLASS_A, 2); + let dst = built_from_pushes(CLASS_A, 2); + let survivor = proof(dst).expect("proven").epoch; + unsafe { clear_element_shape(src) }; + unsafe { test_clear_element_shape_bit_only(dst) }; + assert!( + test_element_shape_record_exists(dst as usize), + "the fixture must actually leave a record behind the cleared bit" + ); + + transfer_element_shape(src as usize, dst as usize); + + assert!( + proof(dst).is_none(), + "the bit is the sole authority for a read, so a record behind a \ + cleared bit must not read as a proof" + ); + let reproven = unsafe { ensure_element_shape(dst) }.expect("still homogeneous"); + assert_ne!( + reproven.epoch, survivor, + "establishing draws a fresh identity from ELEMENT_SHAPE_PROOF_SEQ, so \ + a survivor record can never donate its epoch" + ); + + // Arm 2: source proves nothing, destination still advertises a proof. + let src2 = built_from_pushes(CLASS_A, 2); + let dst2 = built_from_pushes(CLASS_A, 2); + unsafe { clear_element_shape(src2) }; + assert!( + unsafe { test_element_shape_bit_set(dst2) }, + "the fixture must leave the destination advertising a proof" + ); + + transfer_element_shape(src2 as usize, dst2 as usize); + + unsafe { + assert!( + !test_element_shape_bit_set(dst2), + "a transfer whose source proved nothing must still fail the \ + destination closed — gating on `!had_bit` alone would leave a \ + live proof over storage the move has just replaced" + ); + } + assert!( + !test_element_shape_record_exists(dst2 as usize), + "and it must take the destination's record with it" + ); +} + // --------------------------------------------------------------------------- // Lifecycle hooks — what stops a recycled address inheriting a stale identity // ---------------------------------------------------------------------------