Repository navigation
codegen: js_eq's left operand is a stale register across its sibling's allocating lowering (residual #7217 allocation-point witness) #7248
Description
Summary
test_gap_gc_assign_string_source_rooting is improved but not green by #7217's fix: bad char 3 count 3 → bad char 1 count 1 on the allocation-point arm, both 10/10 deterministic on the same build lineage. Two of the three bad iterations were the lazy globalThis bootstrap #7217 closed. The residual one is a codegen-side stale operand in the test's own assertion, not in Object.assign at all.
So the ten assign_string_source entries in test-parity/gc_repsel_triage.txt are retargeted here, not deleted — and their stated cause ("allocation-point relocation inside js_object_assign_one / object_assign_string_source") is now known to be wrong.
Reproduction
export PERRY_RUNTIME_DIR=<build>/perry-dev PERRY_NO_AUTO_OPTIMIZE=1
perry test-files/test_gap_gc_assign_string_source_rooting.ts -o /tmp/asrc # no compile-time env
PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off /tmp/asrc
# bad char 1 count 1 (oracle: bad char 0 count 0)
| build | result, 10/10 |
|---|---|
merge-base 8b024958f |
bad char 3 count 3 |
| with #7217's no-move bootstrap window | bad char 1 count 1 |
The site, named by #7196's quarantine
[gc-fromspace-protect] FAULT: signal 10 at 0x5484173ffac
block=0x54841640000 +1048492 retired_bytes=1048512 retired_by_minor=#2
last-known object: user_ptr=0x5484173ffa8 obj_type=3 size=32
2 js_jsvalue_equals + 332
3 js_eq + 20
4 main + 1028
obj_type=3 is a string; retired_by_minor=#2 (not #0), which is why it survived the bootstrap fix.
Why
The failing line is the test's own comparison:
if (got !== ALPHA[i % 26]) { badChar++; break; }got (a copied property read) is lowered first; ALPHA[i % 26] is a string index read, which allocates a fresh one-character string. So the left operand sits in a register across its sibling's allocating lowering and js_eq hands js_jsvalue_equals a from-space StringHeader*, which it then dereferences to compare bytes.
The emitted IR says it without inference (--trace llvm, no compile-time GC env):
%r136 = load double, ptr %r97 ; `got` -- loaded ABOVE
...
%r149 = call double @js_string_index_get_boxed(double %r137, double %r148) ; ALLOCATES
%r150 = bitcast double %r136 to i64 ; the STALE register
%r151 = bitcast double %r149 to i64
%r152 = call i64 @js_eq(i64 %r150, i64 %r151)%r97 may well be a shadow-bound alloca that the collector rewrites -- but %r136 is a register taken before the collection point and it is never re-read. That is the third property a root must buy (#7207's words: liveness, a rewritten location, and the value the call actually observed), and only the first two are present.
That is exactly the family #7206 (a call receiver and a computed-read base across their sibling operands) and #7214 (the callee, this and every argument of js_closure_callN) closed for other call shapes. js_eq / js_strict_eq / the relational operators take two lowered operands and were apparently not swept in either pass: guard_store_operand_across / RootedOperands::reread exist for precisely this and are not applied here.
Why it matters beyond the witness
a !== b where either side allocates is ordinary JS. The reason this is only visible on the allocation-point route is that on the safepoint route the sibling's allocation is not a collection point — which is the #7217 route argument, restated for a codegen site rather than a runtime one.
Suggested shape
Sweep the binary-operator lowerings (js_eq, js_strict_eq, js_lt/le/gt/ge, js_add where it can be a string concat) with the same guard_store_operand_across treatment #7206 gave the call receiver: root the first-lowered operand whenever the second's lowering collects, and re-read it below. A codegen regression test in the #7206/#7214 style (assert the emitted IR re-reads the left operand below the sibling's allocating call) is the gate.
Activity
fix(gc): run the lazy realm bootstraps in a no-move window (#7217)
fix(gc): run the globalThis bootstrap in a no-move window (#7217)
fix(gc): run the globalThis bootstrap in a no-move window (#7217)
fix(gc): run the globalThis bootstrap in a no-move window (#7217)
fix(gc): run the globalThis bootstrap in a no-move window (#7217) (#7249
proggeramlug commentedon Aug 2, 2026
Still open on
main=d4ac65f21— but the numbers in this issue are no longer correct, and the difference matters.Re-verified as part of the sweep in #7262. Same build recipe as this issue specifies (
--profile perry-dev,PERRY_RUNTIME_DIRpinned,PERRY_NO_AUTO_OPTIMIZE=1, compiled with no compile-time GC env), same arm:d4ac65f21bad char 1 count 1, 10/10 deterministicbad char 2 count 26/10,bad char 3 count 34/10Movement on the same binary and arm: 3 copying minors, 57 318 objects copied. Oracle (node 26.5.1) is
bad char 0 count 0.Two corrections
test-parity/gc_repsel_triage.txtboth describe a fixed1, 10/10. It is a distribution — 2 or 3 per run. Anyone bisecting against "did it go from 1 to 0" will read noise as signal; the acceptance criterion has to bebad char 0 count 0across N runs, not a count comparison.js_eqoperand" — implied a single remaining site. At 2–3 bad iterations post-fix(gc): run the globalThis bootstrap in a no-move window (#7217) #7249, either the stale-js_eq-operand site fires more than once, or there is a second site. Thejs_eqdiagnosis itself is well-evidenced (the--trace llvmextract showing%r136loaded above the allocatingjs_string_index_get_boxedand never re-read, plus thejs_jsvalue_equals + 332quarantine frame) and is not in question — but it should not be assumed to be the whole residual.The triage entries in
test-parity/gc_repsel_triage.txtquotingbad char 1 count 1for all ten arms are stale on the same grounds.Not fixing here — read-only sweep.
proggeramlug commentedon Aug 3, 2026
Heads-up before anyone invests here — this issue is in a class that a planned architectural change would make unrepresentable, and I don't want someone building new machinery that gets deleted.
See
docs/src/internals/rfc-rooting-by-construction.md("Where this sits in the one GC correctness plan", landed in #7292). Short version: this is layer 2 — a GC-managed pointer live in emitted machine code across a point where the collector can run. Statepoints/stack maps (#7108, #7174, enabled by #7241) would have LLVM track that liveness precisely, so the whole shape stops being expressible.This is not "don't fix it". The architectural change is weeks away, the bug is real now, and correctness comes first — please do fix it.
It is "fix the instance, don't build new mechanism." Specifically:
RuntimeHandleScope/GcSuppressScopeat the site: good, lands now, survives.Also worth knowing when you verify:
PERRY_GC_MOVING_LOOP_POLLS=1set at run time against a default-compiled binary is clean 10/10 and proves nothing — the poll codegen isn't in the binary. Compile-time and run-time env must both be set (#7280). And a green run that performed zero copying minors proves nothing either (#7255) — assertcopied_objects > 0.proggeramlug commentedon Aug 10, 2026
Does not reproduce on
main@db44b31b7, and the IR shows why: the stale register is gone — the left operand is re-read below the sibling's allocating call.The witness
Exactly the reproduction in the issue (
perry-dev,PERRY_NO_AUTO_OPTIMIZE=1, no compile-time GC env, thenPERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off):8b024958fbad char 3 count 3, 10/10bad char 1 count 1, 10/10maintodaybad char 0 count 0, 10/10 — the oracleChecked that the pass is not vacuous, because a GC witness that stops collecting is the failure mode this repo pays for most: the same run under
PERRY_GC_DIAG=1reports 3 collections, all 3 copying minors, freeing 1.3 / 3.5 / 5.0 MB. It collects, relocates, and still gets the right answer.The IR, which is the actual answer
The issue's diagnosis was a register taken above an allocating call and never re-read:
Emitted today for the same source, same flags:
The
.rs4p/.rs4isuffixes are the RS4GC retype pass: the operand is a relocatableptr addrspace(1)reloaded from its slot after the allocation point, sojs_eqobserves the post-collection value. That is the third property #7207 names — liveness, a rewritten location, and the value the call actually observed — and all three are now present at this site.What I did and did not do
I implemented the suggested fix first —
rooting::with_operands_rootedover[left, right]in both thejs_eq/js_strict_eqand thejs_rel_*fallback arms ofexpr/compare.rs— and it passes 10/10 too. Then I A/B'd it against an unmodified build and the baseline also passes 10/10, so the change fixes nothing that is currently broken. It is reverted rather than shipped: adding operand rooting where RS4GC already re-reads is pure IR cost, and this repo's own rule is that a rooting change has to be justified by a window that exists.Closing as fixed-elsewhere. If it returns, the diagnosis in this issue is still the right one and the fix shape is still
with_operands_rootedon those two arms — the thing to check first is whether the.rs4pre-read belowjs_string_index_get_boxedis still being emitted, since that is what closed it.The ten
assign_string_sourceentries intest-parity/gc_repsel_triage.txtthat were retargeted here can go.