Repository navigation
Read-PIC keyed on raw keys-array address: ABA staleness + defineProperty-after-prime bypass #6080
Description
Severity: P1 (silent wrong property value; getter bypass)
Found by: audit fable-audit-perry-2.md (code-verified against emitted IR).
Two coherence holes in the inline property-get PIC (@perry_ic_N caches), both mirroring bug classes already fixed elsewhere.
(a) ABA staleness — cache keyed on a raw keys-array address, never GC-registered/invalidated
The inline hit predicate is obj->keys_array == cache[0] && keys != 0 (+ gc/object-type checks); ic_miss primes cache[0] = keys as i64 (crates/perry-codegen/src/expr/property_get/generic_dispatch.rs:319-340, object/field_get_set/ic_miss.rs:385-386). The @perry_ic_N globals are zeroinitializer, never registered with the GC, never rewritten on evacuation, never cleared on sweep. Dynamically-built objects' keys arrays are ordinary collectable allocations; when one dies and its address is recycled by a different-shape keys array, the pointer-match falsely hits and loads a wrong slot — silently. The sibling transition cache had this exact production bug and got content-validation in #6006 (object/mod.rs:824-837, "silently mis-places property values at bundle scale"); the read PIC has no equivalent.
- Fix: pin/immortalize
GC_FLAG_SHAPE_SHAREDkeys arrays (they are exactly the cacheable population), or add a global GC-generation epoch checked beside the cache.
(b) defineProperty accessor installed after priming is bypassed
Converting an existing data prop to an accessor flips ACCESSORS_IN_USE and the per-object OBJ_FLAG_HAS_DESCRIPTORS, but the generic PIC hit path (generic_dispatch.rs:351-371) checks neither the disable flag nor the descriptor bit (only ic_miss's can_cache stops future priming), and converting the prop leaves keys_array untouched — so a previously-primed site keeps returning the stale raw slot instead of invoking the getter.
function f(o){ return o.x; } const a = { x: 1 };
f(a); Object.defineProperty(a, "x", { get(){ return 42; } });
console.log(f(a)); // node: 42 perry: 1- Fix: fold
OBJ_FLAG_HAS_DESCRIPTORS(already in the GcHeader being dereferenced) into the hit predicate — one load+and+icmp, mirroring the class-field guard atclass_field_inline_guard.rs:152.
Confidence: high (emitted IR shows no descriptor/epoch check on the hit path).
Activity
fix(codegen): read-PIC honors descriptors installed after priming (#6080
proggeramlug commentedon Jul 11, 2026
Status: (b) is fixed, (a) is still open.
(b) defineProperty-after-prime bypass — fixed by #6254. The generic PIC hit path now folds
OBJ_FLAG_HAS_DESCRIPTORS(GcHeader_reserved, offset -6) into the hit predicate, so a site primed on a data property forces a miss once that key is converted to an accessor. Verified ingeneric_dispatch.rs.(a) ABA staleness — still open, and I want to record what I found while scoping it, because the two obvious fixes are both wrong in a non-obvious way.
ic_missprimescache[0] = keys as i64for any regular object (can_cache && is_regular), not just shape-shared ones. So the cacheable population is notGC_FLAG_SHAPE_SHARED— the issue's first suggestion ("pin/immortalize the SHAPE_SHARED keys arrays, they are exactly the cacheable population") does not actually cover it.The deeper problem with pinning:
keys_arrayis a plainGC_TYPE_ARRAY.GC_FLAG_PINNEDdoes give the property we need —cycle.rstreats pinned like marked, so a pinned allocation is neither swept nor moved, which makes its address permanently unique and kills the ABA. But pinned objects are not traced:cycle.rs:271only pushes to the worklist whenflags & (MARKED|PINNED) == 0. So pinning a keys array whose owner later dies leaves it un-swept while its key strings are collected underneath it — trading a silent wrong-value for a dangling-pointer walk inic_miss. A pin-based fix therefore also has to register the pinned arrays as GC roots (a side-table + scanner, like the ~55 existing ones) and cap the set, or it just moves the bug.That makes the GC epoch the cheaper sound option:
[2 x i64]to[3 x i64](emitted in 4 places:codegen/function.rs,codegen/closure.rs,codegen/entry.rs×2);@js_ic_epoch(start at 1, so azeroinitializercache — epoch 0 — can never falsely hit);ic_missstores the current epoch incache[2]; the hit predicate ANDscache[2] == js_ic_epochinto the existinghitcond_br (one extra load + icmp).Cost is the ICs re-priming after each GC. That is a real perf hit on GC-heavy workloads and should be measured — but a silently wrong property value is not a trade worth keeping.
I stopped short of implementing it rather than land a half-verified change to the GC/IC boundary. It needs a Linux run (this bug class is Linux-shaped) plus GC stress, and ideally a deterministic repro that forces keys-array address reuse. Flagging clearly rather than guessing.
fix(gc): prune CryptoKey/secret-key side tables when their buffer is …
fix(crypto): KeyObject.from() must handle asymmetric and unregistered…
fix(gc): prune DATA_VIEW_REGISTRY and SHARED_ARRAY_BUFFER_REGISTRY on…
proggeramlug commentedon Aug 5, 2026
PR #7434 implements the epoch design for (a), narrowed to where it is still needed after #6803/#6807: pointer tokens only. Two findings from scoping it worth recording here:
Shape-cache-resident keys arrays are rooted but not address-stable.
move_youngcopiesGC_FLAG_SHAPE_SHAREDarrays like anything else (the flag is merely preserved), every rooted reference is rewritten, and the from-space address is recycled at the flip. So the C3c gate (keys_cacheable_for_pic) narrows the exposed population to class instances / unstamped receivers but does not close the ABA — the@perry_ic_Nglobals are the one reference holder no scanner rewrites. The epoch guard in fix(gc/codegen): epoch-gate read-PIC pointer tokens against GC address recycling (#6080) #7434 closes exactly that, and shape-ID tokens skip it (ids are never reused), so the hot stamped population never re-primes after GC.The perf: generalize guarded object-write fast paths beyond the #6811 two-field loop #6812 3-way dynamic-key WRITE IC has the same exposure and is NOT covered by fix(gc/codegen): epoch-gate read-PIC pointer tokens against GC address recycling (#6080) #7434. Its inline hit path compares the same discriminated shape token (
select(is_stamp, id_token, keys_ptr)inlower_put_value_dyn_ic_inline), and its pointer-token ways can go stale the same way — a stale WRITE hit mis-places a store, which is worse than a stale read. It packs 4(token, slot)ways into the full[8 x i64]global, so there is no free epoch slot; it needs either per-way epoch packing (e.g. steal high bits of the slot word — slots are < 64), a widened global, or a keys-content check like the Large-bundle: object property values read undefined past a leading prefix (value/keys_array desync at scale) #6006 transition validation. Should probably be its own issue once fix(gc/codegen): epoch-gate read-PIC pointer tokens against GC address recycling (#6080) #7434 lands.fix(gc/codegen): epoch-gate read-PIC pointer tokens against GC addres…
proggeramlug commentedon Aug 9, 2026
Closing — both named holes are fixed and live on HEAD in
crates/perry-codegen/src/expr/property_get/generic_dispatch.rs:defineProperty-after-prime bypass (~lines 312-326, carrying a#6080comment):OBJ_FLAG_HAS_DESCRIPTORSis folded into theis_objectpredicate viahas_desc/no_desc, forcing a cache miss wheneverObject.definePropertyhas touched the receiver.#6080a):cache_epoch_ptr/live_epoch/epoch_eq/epoch_okrequirecache[2] == PERRY_IC_EPOCHon the hit path, and the epoch is bumped by every completed GC collection — so a recycled keys-array address cannot be mistaken for the primed one.The epoch half is documented in
changelog.d/7434-read-pic-gc-epoch.md.The second one is the interesting fix in hindsight: keying a cache on a raw heap address is only safe if something invalidates it when the allocator can hand that address back, and tying the epoch to collection completion is the general form of that. Same family as the runtime-side pointer caches enumerated in #7231.
fix(gc): prune the buffer own-property table when its buffer dies (#8136
fix(runtime): module-global Uint8Array receivers, with the view-cache…