Repository navigation
a[i]++ holds its receiver and index across three user-code-capable calls before js_dyn_index_set (#7154 shape) #7628
Description
Found while migrating expr/instance_misc1.rs onto the Layer 1 rooting API
(#7615 slice 2, #7627). The operand-to-operand half is fixed there; this is the
half that needs machinery the campaign does not yet have.
The window
Expr::IndexUpdate lowers a[i]++ / --a[i] as a read-modify-write over two
operands that are consumed by four different calls, with collection points
between them:
obj_box, idx_box = <re-read once, below the operand group>
old = js_dyn_index_get(obj_box, idx_box) ; a getter here is user code
old_num = js_to_numeric(old) ; a valueOf here is user code
new = js_numeric_step(old_num, step) ; ditto for a BigInt/object step
js_dyn_index_set(obj_box, idx_box, new) ; reads PRE-MOVE registers
obj_box and idx_box are NaN-boxed registers held across all three preceding
calls. If any of them drives an evacuating minor, js_dyn_index_set writes into
abandoned from-space memory: the element update silently does not appear on the
object the program keeps. That is #7154's shape, and the same one #7206 fixed on
the computed-read path.
Reproducible shape: a[i]++ where a's element is an object with a valueOf,
or where a is a Proxy / accessor-bearing object.
Why #7627 did not close it
rooting::with_operands_rooted re-reads its group at one point, at the end
of the operand list. That is the right shape when a single collection point
separates the group from its consumer, and it is what closed the
receiver-across-the-index window here. It is the wrong shape when the operands
are consumed by different instructions with collection points between them —
re-reading above js_dyn_index_get puts them straight back in the window the
roots exist to close.
The raw API already has the primitive: RootedOperands::reread_one, added for
#7154's dynamic-call lowering, whose doc describes exactly this situation. What
Layer 1 lacks is a combinator that exposes a rooted operand group as something
call_with_roots can consume per use — i.e. an Arg variant that carries the
group's per-operand Root / Reload / Reuse decision and materialises it at
the instant each call is emitted, rather than once.
Per the template's rule (#7617), a combinator arrives with its caller and with
a written argument for why the existing ones cannot serve, not ahead of one. This
issue is that argument; the caller is this arm.
Scope note
Expr::PropertyUpdate's generic tail has the same read-modify-write skeleton
(js_object_get_field_by_name_f64 → js_to_numeric → js_numeric_step →
js_object_set_field_by_name) over an obj_handle unboxed once above them. It
should be looked at in the same change; it lives in the same file and was left
alone for the same reason.
Activity
proggeramlug commentedon Aug 9, 2026
Fixed in #7699 — but not the half this issue names, and the difference is measured rather than argued.
The combinator already existed
This issue asked for "an
Argvariant that carries the group's per-operand decision and materialises it at the instant each call is emitted".#7615slice 6 had already built exactly that:RootedGroupis one temp-root scope whose contents are re-readable at any number of caller-chosen points and released once. Both arms use it, and no new primitive arrived with this caller.The operand half is not a live bug
Sabotage arm, run twice. Collapsing the per-use re-reads back to a single one — and, for
Expr::PropertyUpdate, removing the receiver's root entirely — leaves the emitted IR unchanged in the relevant respect.root_reload(#7280) rematerialises the slot load at every use a collection point can reach, and it does so through the handle derivation:That is worth recording against #7280's own taxonomy, which lists "a pointer already unboxed to raw
i64" as case (a), the class the pass cannot repair. That entry is about a raw handle a helper RETURNS, not one masked out of a NaN-boxed value the pass has spilled — the latter's chain roots in aload ptr addrspace(1)and is rematerialised whole.The per-use re-reads are kept anyway: they cost nothing (the pass emits them regardless) and they stop the arms depending on a pass carrying a documented side condition ("unless a store to that slot can also run on the way") and a corpus allowlist. But they are documented as belt-and-braces, and their two tests are named as pipeline assertions rather than lowering assertions — a test that cannot fail on the code it appears to cover should not be spelled like a gate.
What was actually repaired: the result
For a BigInt element
js_to_numeric/js_numeric_stephand back a heapBigIntHeader, and whichever value the expression yields —old_numfor postfix,newfor prefix — is live acrossjs_dyn_index_set/js_object_set_field_by_name, i.e. across a user setter, as a bare call result with no slot forroot_reloadto reload from. That is the taxonomy's case (d).RootedGroup::adopt_emittedcloses it, gated onis_provably_not_bigintso a typed-arrayta[i]++keeps the IR it had.Sabotaging that gate (
protectforcedfalse) turnsthe_result_is_rooted_only_when_the_element_may_be_a_bigintred; the typed-array arm is the measured counterfactual, with its returned register produced above the write.The scope note's
Expr::PropertyUpdatetail is included, with the same treatment.Acceptance
test-files/test_gap_7628_index_update_rooted.ts— both fixities, BigInt elements, avalueOfreceiver, the lodashcountByshape (#957), once-only index evaluation — matches node 26.5.1 byte-for-byte. IR-ordering assertions and both sabotage arms are recorded incrates/perry-codegen/src/expr/issue7628_rooting_tests.rs.The two arms moved to
crates/perry-codegen/src/expr/member_update.rs;instance_misc1.rswas four lines under the 2000-line cap.fix(codegen): root a[i]++ / o.f++'s result across the write (#7628)
fix(codegen): the lowering-window cluster — push order, member update…
proggeramlug commentedon Aug 9, 2026
Closed by
9965eb3f6(PR #7699), section "root a[i]++ / o.f++'s result across the write (#7628)" — the member read-modify-write arms now go throughRootedGroup.The interesting part is that the fix is not the one this issue asked for. On re-verification the operand half filed here is not a live bug —
root_reloadalready covered it. The real repair is rooting the result across a possible user setter, which matters for BigInt elements. Two named tests are cited as pipeline assertions.Worth recording as a pattern: this is the second issue today whose stated mechanism was wrong while its symptom was real (#6984 was the other — its bug turned out to be in an unrelated optimization, not in the kill switch it named). Re-deriving the mechanism before fixing is what caught both.