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.