Read-PIC keyed on raw keys-array address: ABA staleness + defineProperty-after-prime bypass

[proggeramlug]

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_SHARED keys 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 at class_field_inline_guard.rs:152.

Confidence: high (emitted IR shows no descriptor/epoch check on the hit path).

[proggeramlug]

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 in generic_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_miss primes cache[0] = keys as i64 for any regular object (can_cache && is_regular), not just shape-shared ones. So the cacheable population is not GC_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_array is a plain GC_TYPE_ARRAY. GC_FLAG_PINNED does give the property we need — cycle.rs treats 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:271 only pushes to the worklist when flags & (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 in ic_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:

• widen the per-site global from [2 x i64] to [3 x i64] (emitted in 4 places: codegen/function.rs, codegen/closure.rs, codegen/entry.rs ×2);
• runtime exports @js_ic_epoch (start at 1, so a zeroinitializer cache — epoch 0 — can never falsely hit);
• bump it at the end of any cycle that swept or evacuated;
• ic_miss stores the current epoch in cache[2]; the hit predicate ANDs cache[2] == js_ic_epoch into the existing hit cond_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.

[proggeramlug]

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:

1. Shape-cache-resident keys arrays are rooted but not address-stable. move_young copies GC_FLAG_SHAPE_SHARED arrays 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_N globals 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.

2. 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) in lower_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.

[proggeramlug]

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 #6080 comment): OBJ_FLAG_HAS_DESCRIPTORS is folded into the is_object predicate via has_desc/no_desc, forcing a cache miss whenever Object.defineProperty has touched the receiver.
• ABA staleness on the raw keys-array address (~lines 380-397, #6080a): cache_epoch_ptr/live_epoch/epoch_eq/epoch_ok require cache[2] == PERRY_IC_EPOCH on 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.