From c9ece90ab21afc4e282b611069e023bce959b1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 16 Aug 2026 00:16:43 +0200 Subject: [PATCH 1/3] fix(gc): retire the typed-layout intact claim when no descriptor backs it `GC_OBJ_TYPED_LAYOUT_INTACT` (`0x1000`) is documented to mean "a canonical `TypedLayoutDescriptor` is reachable for this object". #7834's at-allocation bake sets it with no descriptor behind it, and `set_layout_state` masks `!(GC_LAYOUT_STATE_MASK | GC_LAYOUT_ALL_POINTERS)` = `!0xE000`, so the generic pointer-mask branch in `layout_note_slot` could publish `GC_LAYOUT_SIDE_MASK | GC_OBJ_TYPED_LAYOUT_INTACT` **without** a descriptor. Three codegen consumers read that state as a licence to skip a map: `class_field_inline_guard` (which tests the bit and no layout state at all), `element_shape_guard`'s packed `0x1800_80FF` header compare, and `class_field_store_layout_note_is_conforming`, which elides the layout note on `_reserved & 0xD000 == 0x9000`. Clear the bit where the broken state is observable: the fall-through past `layout_note_slot`'s descriptor probe, which is reached only when BOTH maps answered `None`. A descriptor-backed object returns from the `Some(verdict)` arm long before it, so the legitimate `SIDE_MASK | INTACT` case is untouched. Refs #8115, #7834. Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj --- crates/perry-runtime/src/gc/layout.rs | 82 +++- crates/perry-runtime/src/gc/tests/mod.rs | 1 + .../gc/tests/typed_layout_intact_residual.rs | 463 ++++++++++++++++++ docs/engine-plan.md | 90 +++- 4 files changed, 608 insertions(+), 28 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/typed_layout_intact_residual.rs diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index 00c9823ee4..a081d9999e 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -65,7 +65,19 @@ pub(crate) const GC_LAYOUT_ALL_POINTERS: u16 = 0x2000; // either per-object in `TYPED_LAYOUTS` OR (the #6893 common // case) shared by shape in `SHAPE_LAYOUTS`, keyed by the // object's `keys_array` -// holds at all times. (Before #6893 the descriptor was always the per-object +// holds at all times. +// +// #7834 introduced ONE producer that sets the bit without installing a +// descriptor: the inline `new`'s baked header constant +// (`lower_call/new_alloc.rs`), for a class whose pointer mask is statically +// empty. That is sound at birth — the collector's view of a +// `GC_LAYOUT_POINTER_FREE` payload consults no map — but it made the invariant +// hold only until the first store the descriptor path would have downgraded on. +// #8115 closes that: `layout_note_slot` clears the bit the moment BOTH +// descriptor maps answer `None`, which is the one point where the broken state +// is observable. So the invariant above still holds *for every reader*, with +// the bake as a birth-time exception that self-heals on its first contradicting +// store. (Before #6893 the descriptor was always the per-object // `TYPED_LAYOUTS` entry; `shape_install_shared` now sets the bit while routing // same-shape objects through the shared map, so the bit no longer implies a // per-object entry — only that *some* descriptor is reachable.) The descriptor's @@ -547,6 +559,27 @@ pub(crate) fn layout_has_typed_descriptor(user_ptr: usize) -> bool { layout_typed_intact_for_user(user_ptr) } +/// #8115 test probe: would [`layout_note_slot`]'s descriptor probe find a +/// `TypedLayoutDescriptor` for `user_ptr` right now — asked of the two maps, +/// never of the header bit? +/// +/// [`layout_has_typed_descriptor`] above answers a similar question by reading +/// `GC_OBJ_TYPED_LAYOUT_INTACT`, which is the very claim #8115 is about. A test +/// that used it could not tell "the bit is honest" from "the bit lies", so the +/// premise of every intact-bit test has to come from here instead. +/// +/// Note this is the *shape*'s answer, not the object's licence: the shared +/// `SHAPE_LAYOUTS` entry outlives one object's divergence on purpose (see +/// `with_shape_shared_descriptor`), so after `layout_set_typed_unknown` this +/// still reports `true` while the diverged object no longer claims it. +#[cfg(test)] +pub(in crate::gc) fn layout_descriptor_reachable(user_ptr: usize) -> bool { + if with_per_object_descriptor(user_ptr, |_| ()).is_some() { + return true; + } + unsafe { with_shape_shared_descriptor(user_ptr, |_| ()).is_some() } +} + pub(super) unsafe fn layout_set_typed_unknown(header: *mut GcHeader, user_ptr: usize) { set_layout_state(header, GC_LAYOUT_UNKNOWN); header_clear_typed_layout_intact(header); @@ -629,9 +662,10 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits // return `None` and fall through to the pointer-mask path below. // Skipping it removes the per-write TLS touch on the common dynamic-shape // / pointer-free object and array store path (#5094). The inner `if let` - // still tolerates a `None` defensively, so a transiently desynced bit - // can only cost an extra fall-through, never mis-track a slot. - if (*header)._reserved & GC_OBJ_TYPED_LAYOUT_INTACT != 0 { + // still tolerates a `None` defensively — see the #8115 clear below for + // what a `None` costs and why it is no longer merely a fall-through. + let claimed_intact = (*header)._reserved & GC_OBJ_TYPED_LAYOUT_INTACT != 0; + if claimed_intact { // #5094: a plain, non-pointer-bearing double is representation- // compatible with every in-bounds typed object slot. A raw-f64 // slot consumes the bits directly; a boxed slot consumes the same @@ -707,6 +741,46 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits return; } } + // #8115: reaching here with the bit still set means BOTH descriptor maps + // answered `None` — the probe above is exhaustive — so the object is + // INTACT and descriptor-less, and the invariant documented on + // [`GC_OBJ_TYPED_LAYOUT_INTACT`] ("intact ⟹ a canonical descriptor is + // reachable") is false for it. Restore the invariant here, at the one + // place that observes it broken. + // + // The state stores in the generic pointer-mask branch below CANNOT do + // it: `set_layout_state` masks `!(GC_LAYOUT_STATE_MASK | + // GC_LAYOUT_ALL_POINTERS)` = `!0xE000`, and this bit is `0x1000`. So + // before this clear the branch could publish `SIDE_MASK | INTACT` + // WITHOUT a descriptor — a state three separate consumers read as a + // proof they may skip a map: + // + // * `class_field_inline_guard` (codegen) tests this bit ALONE before + // reading/writing a slot as a bare `double`; + // * `element_shape_guard`'s packed `0x1800_80FF` header test folds it + // in for the same license; + // * `class_field_store_layout_note_is_conforming` (codegen + // `expr/helpers.rs`) elides the layout note outright on + // `_reserved & 0xD000 == 0x9000`, whose proof is "a descriptor built + // from this class's mask globals is reachable". + // + // #7834's at-allocation bake is what made that reachable: it stamps + // `POINTER_FREE | INTACT` into the inline `new`'s header constant with + // no descriptor behind it, deliberately, on the argument that the + // generic branch below downgrades correctly. It does — for the + // collector. The bit it leaves behind is the half that was missing: + // `docs/engine-plan.md`'s construction-cost section, item 2, named this + // mechanism exactly — it used to forbid the bake outright, and now + // records the residual and this repair. + // + // Cost: one 16-bit store, only on the fall-through, which for a baked + // object is only ever a store the descriptor path would have called + // `layout_set_typed_unknown` for. Pre-#7834 that is precisely what it + // did — every pointer-free class carried a real descriptor, and any + // non-conforming store evicted it, bit included. + if claimed_intact { + header_clear_typed_layout_intact(header); + } let pointer = layout_pointer_bearing_bits(value_bits); // A result array built by a runtime helper can declare that its live // prefix is pointer-only once, instead of growing a HashMap-backed diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 382a1370dd..2cb28ee920 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -51,4 +51,5 @@ mod teardown; mod telemetry_verifier; mod temp_roots; mod triggers; +mod typed_layout_intact_residual; mod weak_read_barrier; diff --git a/crates/perry-runtime/src/gc/tests/typed_layout_intact_residual.rs b/crates/perry-runtime/src/gc/tests/typed_layout_intact_residual.rs new file mode 100644 index 0000000000..4bb16c295c --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/typed_layout_intact_residual.rs @@ -0,0 +1,463 @@ +//! #8115: `GC_OBJ_TYPED_LAYOUT_INTACT` must never outlive the descriptor it +//! claims — not even for #7834's descriptor-less at-allocation bake. +//! +//! ## The state under test +//! +//! #7834 stamps `GC_LAYOUT_POINTER_FREE | GC_OBJ_TYPED_LAYOUT_INTACT` into the +//! inline `new`'s header constant for a class whose pointer mask is statically +//! empty, and installs **no** descriptor. Its published argument is about the +//! collector, and for the collector it holds: a `POINTER_FREE` payload is +//! skipped without consulting any map, and a later pointer store downgrades +//! through `layout_note_slot`'s generic pointer-mask branch, which needs no +//! descriptor. +//! +//! What that argument does not cover is the bit left behind. `set_layout_state` +//! masks `!(GC_LAYOUT_STATE_MASK | GC_LAYOUT_ALL_POINTERS)` = `!0xE000` and the +//! intact bit is `0x1000`, so before #8115 the generic branch published +//! `GC_LAYOUT_SIDE_MASK | GC_OBJ_TYPED_LAYOUT_INTACT` **with no descriptor +//! behind it** — a state three codegen consumers read as a licence to skip a +//! map (see [`inline_guard_raw_f64_arm_taken`] and +//! [`conforming_note_elision_taken`]). +//! +//! ## Why the state transition alone is not the test +//! +//! Asserting "the state became `SIDE_MASK`" passes on an unfixed tree — that +//! part always worked. The discriminating conjunction is *reaching that state +//! without a descriptor* **and then taking the inline raw-f64 arm*, so every +//! test below ends by evaluating the codegen predicate over the header the +//! runtime actually produced, and one of them carries the read through to the +//! value the emitted fast path would have handed the program. +//! +//! Two premises are asserted from the maps, never from the bit: +//! +//! * `layout_descriptor_reachable` (not `layout_has_typed_descriptor`, which +//! *reads the bit under test*) proves the plant is descriptor-less; +//! * `per_object_slot_mask` proves the generic pointer-mask branch really +//! minted a mask, so a run where `layout_note_slot` returned early cannot +//! pass as a green one. +//! +//! The legitimate `SIDE_MASK | INTACT` case — a real shared descriptor with a +//! non-empty pointer mask, which `perry-codegen`'s +//! `class_field_store_layout_note_is_conforming` depends on — gets its own +//! test, and it must keep passing both predicates. + +use super::super::*; +use super::support::*; +use crate::gc::layout::layout_descriptor_reachable; +use crate::gc::layout_tables::per_object_slot_mask; + +const OBJECT_HEADER_SIZE: usize = std::mem::size_of::(); + +// --------------------------------------------------------------------------- +// Mirrors of the predicates perry-codegen emits over `GcHeader::_reserved`. +// perry-codegen does not depend on perry-runtime, so its constants are textual +// decimals ("4096", "-12288", "-28672"); `codegen_predicate_constants_match` +// below is the anti-drift gate, on `element_shape_guard.rs`'s precedent. +// --------------------------------------------------------------------------- + +/// `expr/class_field_inline_guard.rs:303` and `:543`: +/// `and i16 %reserved, 4096` / `icmp ne i16 %intact, 0`. The whole layout half +/// of the raw-f64 licence — there is no layout-STATE test anywhere in that +/// file. `expr/element_shape_guard.rs` folds the same bit into its packed +/// `0x1800_80FF` header compare. +fn inline_guard_raw_f64_arm_taken(reserved: u16) -> bool { + reserved & 4096 != 0 +} + +/// `expr/write_barrier.rs:713-720`: +/// `and i16 %reserved, -12288` / `icmp eq i16 %masked, -28672`, i.e. +/// `_reserved & 0xD000 == 0x9000`. When it holds, `layout_note_slot` is not +/// called at all for a pointer store into a class-declared pointer slot. +fn conforming_note_elision_taken(reserved: u16) -> bool { + (reserved & 0xD000u16) == 0x9000u16 +} + +#[test] +fn codegen_predicate_constants_match_the_runtime() { + assert_eq!(4096u16, GC_OBJ_TYPED_LAYOUT_INTACT, "intact bit drifted"); + assert_eq!( + 0xD000u16, + GC_LAYOUT_STATE_MASK | GC_OBJ_TYPED_LAYOUT_INTACT, + "conforming-elision mask drifted" + ); + assert_eq!( + 0x9000u16, + GC_LAYOUT_SIDE_MASK | GC_OBJ_TYPED_LAYOUT_INTACT, + "conforming-elision expectation drifted" + ); + // The textual i16 forms perry-codegen emits. + assert_eq!(-12288i16, 0xD000u16 as i16); + assert_eq!(-28672i16, 0x9000u16 as i16); +} + +// --------------------------------------------------------------------------- +// Plants +// --------------------------------------------------------------------------- + +unsafe fn reserved_of(obj: *mut crate::ObjectHeader) -> u16 { + (*header_from_user_ptr(obj as *const u8))._reserved +} + +unsafe fn slot_bits(obj: *mut crate::ObjectHeader, slot: usize) -> u64 { + *((obj as usize + OBJECT_HEADER_SIZE + slot * 8) as *const u64) +} + +/// Reproduce #7834's bake **exactly**: a shape-keyed instance whose header +/// claims `GC_LAYOUT_POINTER_FREE | GC_OBJ_TYPED_LAYOUT_INTACT` and for which +/// `js_gc_declare_typed_shape_layout` was never called. Not calling it IS the +/// plant — that call is the thing #7834 removes. +/// +/// Three payload slots, because a payload below `layout_mask_min_slots()` makes +/// `layout_note_slot` decline the mask and take `GC_LAYOUT_UNKNOWN` instead; +/// this test wants the `SIDE_MASK` arm. +unsafe fn plant_baked_instance(shape_id: u32, packed_keys: &[u8]) -> *mut crate::ObjectHeader { + let obj = crate::object::js_object_alloc_with_shape( + shape_id, + 3, + packed_keys.as_ptr(), + packed_keys.len() as u32, + ); + let header = header_from_user_ptr(obj as *const u8); + (*header)._reserved = ((*header)._reserved & !GC_LAYOUT_STATE_MASK) + | GC_LAYOUT_POINTER_FREE + | GC_OBJ_TYPED_LAYOUT_INTACT; + assert!( + !layout_descriptor_reachable(obj as usize), + "premise: the bake installs NO descriptor — that is what makes it free" + ); + assert!( + inline_guard_raw_f64_arm_taken(reserved_of(obj)), + "premise: at birth the bake does claim the raw-f64 licence" + ); + obj +} + +/// The legitimate `SIDE_MASK | INTACT`: a real shared descriptor whose pointer +/// mask is non-empty, installed the way the runtime declare installs one. +unsafe fn plant_descriptor_backed_instance( + shape_id: u32, + packed_keys: &[u8], +) -> *mut crate::ObjectHeader { + let obj = crate::object::js_object_alloc_with_shape( + shape_id, + 3, + packed_keys.as_ptr(), + packed_keys.len() as u32, + ); + // slot 0 pointer, slots 1..2 raw f64 — the shape of `class C { s: string; + // x: number; y: number }`. + let raw_f64_words: [u64; 1] = [0b110]; + let pointer_words: [u64; 1] = [0b001]; + crate::gc::js_gc_declare_typed_shape_layout( + ptr_bits(obj as usize), + 3, + raw_f64_words.as_ptr(), + 1, + pointer_words.as_ptr(), + 1, + ); + assert!( + layout_descriptor_reachable(obj as usize), + "premise: the declare must install a reachable descriptor" + ); + obj +} + +// --------------------------------------------------------------------------- +// The residual +// --------------------------------------------------------------------------- + +/// The fix. A pointer stored into a statically-all-`number` slot is a runtime +/// type violation, which Perry permits by design (CLAUDE.md, "No runtime type +/// *validation*") — it is the one way to drive a baked instance off its +/// `POINTER_FREE` birth state. Doing so must retire the raw-f64 licence with it. +#[test] +fn a_descriptorless_bake_drops_its_intact_claim_on_the_generic_downgrade() { + unsafe { + let obj = plant_baked_instance(0x8115_0001, b"x\0y\0z\0"); + let child = string_bits(young_leaf()); + + crate::object::store_object_field_slot(obj, 0, child); + + // Subject-liveness: the generic pointer-mask branch must actually have + // run and minted a mask. Without this, a `layout_note_slot` that + // returned early would pass every assertion below vacuously. + let reserved = reserved_of(obj); + assert_eq!( + reserved & GC_LAYOUT_STATE_MASK, + GC_LAYOUT_SIDE_MASK, + "premise: the generic branch must have published SIDE_MASK" + ); + let mask = per_object_slot_mask(obj as usize) + .expect("premise: the generic branch must have minted a per-object mask"); + assert!( + mask.contains_slot(0), + "premise: the mask must record slot 0 as pointer-bearing" + ); + assert!( + !layout_descriptor_reachable(obj as usize), + "premise: nothing installed a descriptor — the state is SIDE_MASK \ + with no descriptor behind it, which is the #8115 state" + ); + + // The residual itself, in both consumers' terms. + assert!( + !inline_guard_raw_f64_arm_taken(reserved), + "#8115: a descriptor-less object must not license codegen's raw-f64 \ + arm — `_reserved` = {reserved:#06x}" + ); + assert!( + !conforming_note_elision_taken(reserved), + "#8115: a descriptor-less object must not license the conforming \ + layout-note elision — `_reserved` = {reserved:#06x}" + ); + } +} + +/// The conjunction the issue names, carried through to the value the program +/// sees: reach `SIDE_MASK` without a descriptor, then take the inline arm. +/// +/// The emitted fast path is a bare `load double` of the slot. Slot 0 holds a +/// NaN-boxed string, so taking that arm hands the program +/// `f64::from_bits(STRING_TAG | addr)` — a NaN — where the fallback hands it the +/// string. Sabotage check: restore the stale bit (the `red_control` arm) and +/// the same code reads the NaN, which is what an unfixed tree produces. +#[test] +fn the_inline_raw_f64_arm_must_not_read_a_pointer_slot_as_a_double() { + unsafe { + let obj = plant_baked_instance(0x8115_0002, b"a\0b\0c\0"); + let child = string_bits(young_leaf()); + crate::object::store_object_field_slot(obj, 0, child); + assert_eq!( + slot_bits(obj, 0), + child, + "premise: the slot holds the string" + ); + + // What the emitted guard would do, evaluated against the header the + // runtime actually produced. + let green = read_field_the_way_codegen_would(obj, 0); + assert_eq!( + green, + FieldRead::Boxed(child), + "#8115: the guard must fall back to the boxed read; taking the raw \ + arm here yields {:?}", + FieldRead::RawDouble(f64::from_bits(child)) + ); + + // Red control: the pre-#8115 header, reconstructed by putting the stale + // bit back. This is the sabotage, in-test — it proves the assertion + // above can fail, and that what it forbids is a real wrong answer. + let header = header_from_user_ptr(obj as *const u8); + (*header)._reserved |= GC_OBJ_TYPED_LAYOUT_INTACT; + let red = read_field_the_way_codegen_would(obj, 0); + match red { + FieldRead::RawDouble(v) => assert!( + v.is_nan(), + "red control: the raw arm must read the NaN-box as a double" + ), + FieldRead::Boxed(_) => panic!( + "red control did not reproduce the residual — the raw-f64 arm \ + was not taken with the stale intact bit restored" + ), + } + (*header)._reserved &= !GC_OBJ_TYPED_LAYOUT_INTACT; + } +} + +#[derive(Debug, PartialEq)] +enum FieldRead { + /// `class_field_inline_guard`'s fast arm: `load double, ptr %slot`. + RawDouble(f64), + /// The guard call / by-name fallback: the NaN-boxed slot word, unchanged. + Boxed(u64), +} + +/// Model of the emitted `this.f` read: take the raw-f64 arm iff the guard's +/// layout predicate holds. The guard's other conjuncts (class id, ShapeId, +/// `obj_type`, not-forwarded, no per-object descriptors) are all trivially true +/// for this fixture's single unmodified object, so the intact bit is the only +/// one that can decide the branch. +unsafe fn read_field_the_way_codegen_would( + obj: *mut crate::ObjectHeader, + slot: usize, +) -> FieldRead { + let bits = slot_bits(obj, slot); + if inline_guard_raw_f64_arm_taken(reserved_of(obj)) { + FieldRead::RawDouble(f64::from_bits(bits)) + } else { + FieldRead::Boxed(bits) + } +} + +// --------------------------------------------------------------------------- +// The legitimate case must survive +// --------------------------------------------------------------------------- + +/// `perry-codegen`'s `class_field_store_layout_note_is_conforming` elides the +/// layout note on exactly `SIDE_MASK | INTACT`, and its proof is "a descriptor +/// built from this class's mask globals is reachable". #8115 must not cost that +/// case its bit: the clear fires only where BOTH descriptor maps answered +/// `None`, and a descriptor-backed object returns from the `Some(verdict)` arm +/// long before it. +#[test] +fn a_descriptor_backed_side_mask_object_keeps_its_intact_claim() { + unsafe { + let obj = plant_descriptor_backed_instance(0x8115_0003, b"s\0x\0y\0"); + + let birth = reserved_of(obj); + assert_eq!( + birth & GC_LAYOUT_STATE_MASK, + GC_LAYOUT_SIDE_MASK, + "premise: a non-empty pointer mask installs SIDE_MASK" + ); + assert!( + conforming_note_elision_taken(birth), + "premise: this is the state expr/helpers.rs documents as legitimate" + ); + + // A conforming pointer store into the declared pointer slot: the very + // store the elision is for. + crate::object::store_object_field_slot(obj, 0, string_bits(young_leaf())); + // And a conforming raw-f64 store into a declared raw-f64 slot. + crate::object::store_object_field_slot(obj, 1, 1.5f64.to_bits()); + + let after = reserved_of(obj); + assert!( + layout_descriptor_reachable(obj as usize), + "the descriptor must survive two conforming stores" + ); + assert!( + inline_guard_raw_f64_arm_taken(after), + "#8115 must not clear the bit on a descriptor-backed object — \ + `_reserved` = {after:#06x}" + ); + assert!( + conforming_note_elision_taken(after), + "#8115 must not break the legitimate SIDE_MASK | INTACT case — \ + `_reserved` = {after:#06x}" + ); + } +} + +/// The other half of the legitimate case: a store that *contradicts* the +/// descriptor still downgrades through `layout_set_typed_unknown`, exactly as +/// before. #8115 adds a second clear; it must not displace the first. +#[test] +fn a_contradicting_store_still_downgrades_through_the_descriptor_path() { + unsafe { + let obj = plant_descriptor_backed_instance(0x8115_0004, b"p\0q\0r\0"); + + // Slot 1 is declared raw-f64; a NaN-boxed pointer there contradicts it. + crate::object::store_object_field_slot(obj, 1, string_bits(young_leaf())); + + let after = reserved_of(obj); + assert_eq!( + after & GC_LAYOUT_STATE_MASK, + GC_LAYOUT_UNKNOWN, + "a contradicting store must reach GC_LAYOUT_UNKNOWN" + ); + assert!( + !inline_guard_raw_f64_arm_taken(after), + "the intact bit must be gone with the descriptor it claimed" + ); + // The SHARED `SHAPE_LAYOUTS` entry deliberately survives — it still + // describes every sibling that has not diverged, and `layout.rs`'s + // `with_shape_shared_descriptor` doc calls the INTACT gate on that half + // load-bearing for exactly this reason. So `layout_descriptor_reachable` + // still answers `true` here; what changed is that THIS object no longer + // claims it. + assert!( + layout_descriptor_reachable(obj as usize), + "premise: the shared entry survives one object's divergence" + ); + } +} + +/// The bake's own fast path must stay free: a plain double into an in-range +/// slot of a baked instance returns at `layout_note_slot`'s raw-f64 arm, above +/// the #8115 clear, so the licence survives. Without this, "clear the bit" +/// could be implemented as "clear it on every store" and no other test here +/// would notice. +#[test] +fn a_conforming_raw_f64_store_leaves_the_bake_intact() { + unsafe { + let obj = plant_baked_instance(0x8115_0005, b"u\0v\0w\0"); + + crate::object::store_object_field_slot(obj, 0, 3.5f64.to_bits()); + crate::object::store_object_field_slot(obj, 1, (-7.25f64).to_bits()); + + let after = reserved_of(obj); + assert_eq!( + after & GC_LAYOUT_STATE_MASK, + GC_LAYOUT_POINTER_FREE, + "a pointer-free payload must stay POINTER_FREE" + ); + assert!( + inline_guard_raw_f64_arm_taken(after), + "#7834's bake must keep its licence while nothing contradicts it — \ + `_reserved` = {after:#06x}" + ); + assert_eq!( + read_field_the_way_codegen_would(obj, 0), + FieldRead::RawDouble(3.5), + "the fast arm must still be taken, and read the right double" + ); + } +} + +/// Why the residual never surfaced as a wrong answer from TypeScript, and why +/// that was not a defence worth keeping. +/// +/// Every by-name store — which is where a pointer into a statically-`number` +/// slot actually arrives, because the inline store arm refuses a non-finite +/// value for a raw-f64 field — calls `mark_object_dynamic_shape_unknown` first. +/// Its guard is +/// +/// ```text +/// state != GC_LAYOUT_SIDE_MASK && !layout_has_typed_descriptor(obj) -> return +/// ``` +/// +/// and #8115's issue read the second conjunct as true for a baked instance +/// ("no typed descriptor"). It is **false**: `layout_has_typed_descriptor` +/// answers by reading `GC_OBJ_TYPED_LAYOUT_INTACT` — the bit the bake set — so +/// the guard does not fire and `layout_mark_unknown` heals the object. The +/// stale claim was its own antidote. +/// +/// This test pins that coupling so it cannot be dissolved silently: making +/// `layout_has_typed_descriptor` honest (probing the maps) would turn the +/// early return back on. After #8115 nothing depends on it — +/// `a_descriptorless_bake_drops_its_intact_claim_on_the_generic_downgrade` +/// above heals through `layout_note_slot` alone, with +/// `mark_object_dynamic_shape_unknown` never called. +#[test] +fn the_bake_healed_itself_only_because_the_descriptor_probe_reads_the_same_bit() { + unsafe { + let obj = plant_baked_instance(0x8115_0006, b"m\0n\0o\0"); + + // The two questions disagree. That disagreement IS the #8115 state. + assert!( + crate::gc::layout_has_typed_descriptor(obj as usize), + "premise: the O(1) probe answers from the bit, so the bake makes it \ + say yes" + ); + assert!( + !layout_descriptor_reachable(obj as usize), + "premise: no descriptor is actually reachable" + ); + + crate::object::mark_object_dynamic_shape_unknown(obj); + + let after = reserved_of(obj); + assert_eq!( + after & GC_LAYOUT_STATE_MASK, + GC_LAYOUT_UNKNOWN, + "the guard must NOT have early-returned — if it did, a baked \ + instance would keep a licence no store had earned" + ); + assert!( + !inline_guard_raw_f64_arm_taken(after), + "`layout_mark_unknown` clears the bit on the way through" + ); + } +} diff --git a/docs/engine-plan.md b/docs/engine-plan.md index e7b24b8246..65b037a741 100644 --- a/docs/engine-plan.md +++ b/docs/engine-plan.md @@ -174,12 +174,14 @@ normalising two `(pointer, length)` pairs into slices only ever compared as integers, ~11 of `words_intersect` setup over two immutable globals, ~6 recomputing the slot kind. -### ⛔ Two remedies that look obvious and are wrong. Do not rebuild them. +### One remedy that looks obvious and is wrong, and one that shipped An earlier revision of this section proposed inlining the hit path at the `new` site, reasoning that every argument but the object pointer is a compile-time -constant and that this is the shape #7566 won 1.81× on. **Both halves of that -were tested in #7586 and both are wrong.** +constant and that this is the shape #7566 won 1.81× on. **The first half of that +was tested in #7586 and is wrong. The second half shipped in #7834** — four days +after this section was written to forbid it (`d2dca5823` 2026-08-07, +`d04cc3f29` 2026-08-11), and the ⛔ stood over `main` until #8115. 1. **Outlining/inlining the frame is not the lever — it is a regression.** The prologue does look like #7566's shape (`sub sp, sp, #0x150`, six `stp` pairs @@ -192,27 +194,67 @@ were tested in #7586 and both are wrong.** more in register moves than the prologue saves. **This function is bound by instruction count, not frame size.** -2. **⛔ Having codegen OR `GC_OBJ_TYPED_LAYOUT_INTACT` into the inline `new`'s - header word is a use-after-free factory.** It is seductive because it is - genuinely free — `declare`-path classes must have an empty pointer mask, so - since #7566 the inline `new` already writes its `GcHeader` as one i64 - constant, and OR-ing one more bit costs +0 instructions and +0 bytes. - - It breaks the unwritten invariant **"intact ⟹ a descriptor is reachable"**, - which `layout_note_slot` silently depends on. On a contradicting store to an - object that is intact but descriptor-less, the probe resolves `None`; - `layout_set_typed_unknown` — the only thing that clears the intact bit — is - reached **only from the `Some(verdict)` arm** (`gc/layout.rs:782–788`), so - control falls through to the pointer-mask path and **the bit is never - cleared**. The object is thereafter `SIDE_MASK` to the collector and *intact* - to the class-field inline guard, which consults no map by design. The raw-store - fast path then writes a double over a pointer slot with no barrier and no - layout note, and the next collection walks it as a heap pointer. - - Note the comment at `layout.rs:742` says a `None` verdict "can only cost an - extra fall-through, never mis-track a slot". That is true **only while the - invariant holds** — it is a consequence of it, not an independent guarantee, - and it reads like reassurance to anyone implementing this. +2. **✅ Having codegen OR `GC_OBJ_TYPED_LAYOUT_INTACT` into the inline `new`'s + header word SHIPPED in #7834 (`d04cc3f29`), and is sound as of #8115.** It is + free — `declare`-path classes must have an empty pointer mask, so since #7566 + the inline `new` already writes its `GcHeader` as one i64 constant, and OR-ing + one more bit costs +0 instructions and +0 bytes. It removed a per-instance + `js_gc_declare_typed_shape_layout` that was ~30% of `churn_alloc`/`push_cls`, + and it is covered by `lower_call/typed_shape_bake_tests.rs` — an IR census + with a negative control (a pointer-BEARING shape must still emit the declare). + + **The hazard this entry used to forbid was real, and was live from #7834 + until #8115.** It is kept in full, because the shape of it recurs. The bake + breaks the invariant **“intact ⟹ a descriptor is reachable”**, and nothing in + `layout_note_slot` used to repair it. On a contradicting store to an object + that is intact but descriptor-less the probe resolves `None`; + `layout_set_typed_unknown` — then the only thing that cleared the intact bit + — is reached **only from the `Some(verdict)` arm**, so control fell through to + the generic pointer-mask branch, whose `set_layout_state` masks + `!(GC_LAYOUT_STATE_MASK | GC_LAYOUT_ALL_POINTERS)` = `!0xE000` and therefore + **cannot** clear a `0x1000` bit. The object was thereafter `SIDE_MASK` to the + collector and *intact* to three codegen consumers that consult no map by + design: `class_field_inline_guard` (which tests this bit and no layout state + at all — there is no state test anywhere in that file), + `element_shape_guard`'s packed `0x1800_80FF` header compare, and + `class_field_store_layout_note_is_conforming`, which elides the layout note + outright on `_reserved & 0xD000 == 0x9000`. + + **What #8115 changed.** `layout_note_slot` clears the bit at the one point + where the broken state is observable — the fall-through past the descriptor + probe, reached only when BOTH maps answered `None`. The invariant therefore + holds for every *reader* again, with the bake as a birth-time exception that + self-heals on its first contradicting store. Pre-#7834 the descriptor existed + and any such store evicted it, bit included; this restores that outcome + without paying for the descriptor. Acceptance: + `perry-runtime/src/gc/tests/typed_layout_intact_residual.rs`, which reaches + `SIDE_MASK | INTACT` without a descriptor and then evaluates the codegen + predicate over the header the runtime actually produced — asserting the state + transition alone passes on an unfixed tree. + + **Three things to carry forward.** + + *The old severity was over-stated for what was reachable.* The lost-child + use-after-free needs the note elision, and its precondition (the class's + compile-time mask declares slot *k* a pointer) is mutually exclusive with the + bake's (`layout_pointer_free_at_allocation` requires the pointer mask to be + statically **empty**), so no single class satisfied both. What WAS reachable + is the raw-f64 read arm: once a pointer lands in a statically-all-`number` + slot — a runtime type violation, which Perry permits by design — the guard + read that slot as a bare `double` and handed the program a NaN where the boxed + fallback hands it the value. A wrong answer, not a collector fault. + + *The reassuring comment was a consequence, not a guarantee.* + `layout_note_slot`'s probe used to say a `None` verdict “can only cost an + extra fall-through, never mis-track a slot”. True only while the invariant + held. It now points at the clear that makes it true. + + *What the bake still rests on, unchanged.* Between allocation and its first + field store a baked instance claims raw-f64 slots that hold the allocator's + `TAG_UNDEFINED` fill. That is not new and is not #8115's: it is + `TypedShapeProof::FreshlyAllocated`, the same codegen-side proof the pre-#7834 + `js_gc_declare_typed_shape_layout` path rested on (#7510/#7512). If you change + who may declare a layout at allocation, that is the contract to re-check. **A declared class is no longer slower than an object literal.** #7512 is **closed**: `churn_alloc` and `push_cls` both measure 0.75 s. The cause was not From b0a8dfd1002c19a512512d2a4d65cb33a28f4f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 16 Aug 2026 00:18:00 +0200 Subject: [PATCH 2/3] docs(changelog): add the #8115 intact-bit fragment Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj --- .../8182-typed-layout-intact-residual.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 changelog.d/8182-typed-layout-intact-residual.md diff --git a/changelog.d/8182-typed-layout-intact-residual.md b/changelog.d/8182-typed-layout-intact-residual.md new file mode 100644 index 0000000000..91f275e593 --- /dev/null +++ b/changelog.d/8182-typed-layout-intact-residual.md @@ -0,0 +1,45 @@ +**GC: `GC_OBJ_TYPED_LAYOUT_INTACT` no longer outlives the descriptor it claims (#8115).** +The bit is documented to mean "a canonical `TypedLayoutDescriptor` is reachable +for this object", and four readers rely on it. #7834's at-allocation bake sets it +with no descriptor behind it, and `set_layout_state` masks +`!(GC_LAYOUT_STATE_MASK | GC_LAYOUT_ALL_POINTERS)` = `!0xE000` — it cannot clear a +`0x1000` bit — so `layout_note_slot`'s generic pointer-mask branch could publish +`GC_LAYOUT_SIDE_MASK | GC_OBJ_TYPED_LAYOUT_INTACT` **without** a descriptor. Three +codegen consumers read that state as a licence to skip a map: +`class_field_inline_guard` (which tests the bit and no layout state at all), +`element_shape_guard`'s packed `0x1800_80FF` header compare, and +`class_field_store_layout_note_is_conforming`, which elides the layout note +outright on `_reserved & 0xD000 == 0x9000`. + +`layout_note_slot` now clears the bit at the fall-through past its descriptor +probe — the one point where the broken state is observable, reached only when +*both* maps answered `None`. A descriptor-backed object returns from the +`Some(verdict)` arm above it, so the legitimate `SIDE_MASK | INTACT` case is +untouched; pre-#7834 the descriptor existed and any contradicting store evicted +it, bit included, so this restores that outcome without paying for the descriptor. +Cost is one 16-bit store on a path a baked object only reaches for a store the +descriptor path would have downgraded on. + +Acceptance: `perry-runtime/src/gc/tests/typed_layout_intact_residual.rs` (7 +tests). It reaches `SIDE_MASK` without a descriptor through the real +`layout_note_slot`, from the exact header the inline `new` bakes, and then +evaluates the codegen predicate over the header the runtime produced — asserting +the state transition alone passes on an unfixed tree. Premises come from the maps +(a new `#[cfg(test)] layout_descriptor_reachable`), never from the bit under test. +Sabotage-verified: with the clear disabled, exactly the two residual tests fail at +`_reserved = 0x9000` and the four controls still pass. + +Two findings recorded rather than fixed. `mark_object_dynamic_shape_unknown` is +**not** inert on baked instances as #8115 assumed — `layout_has_typed_descriptor` +answers from the same stale bit, so the guard does not early-return and +`layout_mark_unknown` heals the object; the stale claim was its own antidote, and +a test now pins that coupling. And the lost-child use-after-free the old +`docs/engine-plan.md` ⛔ posited needs the note elision, whose precondition (the +class declares a pointer slot) is mutually exclusive with the bake's (the pointer +mask is statically empty), so the reachable consequence was a wrong-value raw-f64 +read, not a collector fault. + +`docs/engine-plan.md` item 2 is rewritten: it forbade code that shipped four days +after it was written and is covered by an IR census with a negative control. It +now records that the bake shipped, what #8115 changed, and what the design still +rests on (`TypedShapeProof::FreshlyAllocated`). From ffcd2c6ccfe373493d02e21f2652529f3acf20ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 16 Aug 2026 00:21:00 +0200 Subject: [PATCH 3/3] docs(engine-plan): record why the residual never reproduced from TypeScript Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj --- docs/engine-plan.md | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/engine-plan.md b/docs/engine-plan.md index 65b037a741..d6c6123f3d 100644 --- a/docs/engine-plan.md +++ b/docs/engine-plan.md @@ -205,7 +205,7 @@ after this section was written to forbid it (`d2dca5823` 2026-08-07, **The hazard this entry used to forbid was real, and was live from #7834 until #8115.** It is kept in full, because the shape of it recurs. The bake - breaks the invariant **“intact ⟹ a descriptor is reachable”**, and nothing in + breaks the invariant **"intact ⟹ a descriptor is reachable"**, and nothing in `layout_note_slot` used to repair it. On a contradicting store to an object that is intact but descriptor-less the probe resolves `None`; `layout_set_typed_unknown` — then the only thing that cleared the intact bit @@ -244,9 +244,22 @@ after this section was written to forbid it (`d2dca5823` 2026-08-07, read that slot as a bare `double` and handed the program a NaN where the boxed fallback hands it the value. A wrong answer, not a collector fault. + Even that did not reproduce from TypeScript, and the reason is worth knowing: + the pointer arrives by name (the inline store arm refuses a non-finite value + for a raw-f64 field), and every by-name store calls + `mark_object_dynamic_shape_unknown` first. #8115's issue read its early-return + guard — `state != SIDE_MASK && !layout_has_typed_descriptor(obj)` — as firing + on a baked instance. It does not: `layout_has_typed_descriptor` answers by + reading `GC_OBJ_TYPED_LAYOUT_INTACT`, the bit the bake set, so the guard is + skipped and `layout_mark_unknown` heals the object. **The stale claim was its + own antidote** — one inaccuracy masking another, which is not a property to + ship on. `the_bake_healed_itself_only_because_the_descriptor_probe_reads_the_same_bit` + pins the coupling; after #8115 nothing depends on it, because healing happens + in `layout_note_slot` itself. + *The reassuring comment was a consequence, not a guarantee.* - `layout_note_slot`'s probe used to say a `None` verdict “can only cost an - extra fall-through, never mis-track a slot”. True only while the invariant + `layout_note_slot`'s probe used to say a `None` verdict "can only cost an + extra fall-through, never mis-track a slot". True only while the invariant held. It now points at the clear that makes it true. *What the bake still rests on, unchanged.* Between allocation and its first