fix(runtime,codegen): TDZ × class-capture-refresh boot regression + JSON.stringify replacer SIGBUS hardening (#5989) - #6055
Merged
Conversation
added 2 commits
July 6, 2026 05:02
…Z boxes — un-break SWC-CJS-interop module boot #6044 gave lexical let/const a real Temporal Dead Zone: forward-referenced bindings are seeded with TAG_TDZ and any read through js_box_get_bits throws "Cannot access ... before initialization". That interacted with the #6037 class-capture refresh strategy and boot-broke every module using the standard SWC CJS interop shape (Next.js dist files, most transpiled npm packages): _export(exports, { C: () => C, ... }); // getters forward-ref the class const _fs = _interop(require("fs")); // captured by C's methods const _path = _interop(require("path")); // captured by C's methods class C { m() { _fs...; _path... } } The getter closures forward-reference the class, so the class's captured module consts (_fs, _path) are TDZ-seeded via PreallocateTdzBoxes. The #6037 strategy emits a RegisterClassCaptures snapshot refresh after EACH captured var's assignment — so the refresh emitted after `const _fs` reads _path's box while it is still in its dead zone. Pre-#6044 that read snapshotted `undefined` and the next refresh fixed it up; post-#6044 it throws at module init ("Cannot access undefined before initialization" — the name-agnostic captured-box path) and the whole program dies at boot. Trigger needs >= 2 class-captured module consts plus the getter head; the Next.js standalone server hit it in dist/server/dev/browser-logs/file-logger.js and could no longer boot at all. Fix: the snapshot refresh loads are Perry-internal materialization, not user reads — bracket exactly them in a TDZ-suppression window: - box.rs: TDZ_SUPPRESS_DEPTH thread-local + js_tdz_suppress_begin/end (#[used] keepalives for the auto-opt link). Inside the window a TAG_TDZ box reads as `undefined` (the pre-#6044 behavior); outside, TDZ throws exactly as before. Only the cold TAG_TDZ branch consults the flag. - static_field_meta.rs: the RegisterClassCaptures arm wraps its capture loads in begin/end. The window contains only side-effect-free LocalGets — no user code can observe it. - runtime_decls/strings.rs: declare the two externs. Validated: the verbatim file-logger.js repro plus 6 bisect variants go from ReferenceError-at-boot to working; 5 control variants unchanged; genuine TDZ violations still throw ReferenceError byte-identical to node (read-before-let direct and via hoisted function); the closure-created-before-init-called-after legal shapes still work. The Next.js standalone app boots again.
…rupted/mis-classified pointers (SIGBUS -> null) The Next.js App Router `/plain` render feeds a value graph to `JSON.stringify(value, replacer, space)` in which a corrupted / mis-encoded value reaches the replacer walk. Three deref sites there guarded only `is_handle_band` (small-id proxy/stream handles), NOT general pointer validity, so a value whose extracted pointer was mis-aligned or out of the heap range fell through to `gc_obj_type`, which deref'd its `GcHeader` and SIGBUS'd the server on the first request (Bus error: 10). Mirrors the mature `is_object_pointer` pre-load sanity (magnitude + 8-byte alignment) at the replacer's deref sites: - new `ptr_derefable(ptr)` — top 16 bits <= 1, >= 0x10000, 8-byte aligned. - `apply_to_json`: a non-derefable pointer can't carry a `toJSON` method, so skip the probe (return the value) instead of a `gc_obj_type` deref. - `dispatch_pointer_with_replacer`: a non-derefable pointer is unserializable — emit "null" (matching the handle-band fallback just above) instead of deref. - the `GC_TYPE_ARRAY` arm: `gc_obj_type` can mis-read a corrupted structure as an array whose `length` reads as garbage (~2.7e9); the `0..len` walk then runs OOB. Sanity-cap the length at 10M (mirrors `is_object_pointer`'s field-count cap) and emit "null" beyond it. `JSON.stringify` must never SIGBUS on a bad value. This turns the `/plain` hard crash back into the pre-existing #5989 render hang — a separate, deeper issue where the render produces the corrupted value graph (garbage-length arrays + raw untagged values) UPSTREAM of stringify. Validated: boots + 5/8 byte-identical vs node v26 (/, /about, /counter, /api/hello GET+POST); the 3 dynamic routes no longer crash the server (they hang per #5989). No regression. Runtime-only.
📝 WalkthroughWalkthroughThis PR adds a thread-local TDZ-suppression window ( ChangesClass capture TDZ suppression
JSON replacer pointer hardening
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Codegen as Codegen (static_field_meta.rs)
participant Runtime as Runtime (js_box_get_bits)
participant Counter as TDZ_SUPPRESS_DEPTH
Codegen->>Runtime: js_tdz_suppress_begin()
Runtime->>Counter: increment
loop each captured expression
Codegen->>Runtime: lower_expr (read capture)
Runtime->>Counter: check depth > 0
alt suppression active
Runtime-->>Codegen: TAG_UNDEFINED
else not suppressed
Runtime-->>Codegen: throw ReferenceError
end
end
Codegen->>Runtime: js_tdz_suppress_end()
Runtime->>Counter: decrement
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/perry-runtime/src/json/replacer.rs (1)
264-274: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCentralize the 10_000_000 cap The array guard matches
is_object_pointertoday, but the duplicated literal is brittle; extract the shared limit so both paths stay in sync if the threshold changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/json/replacer.rs` around lines 264 - 274, Centralize the duplicated 10_000_000 array length cap used in replacer::stringify_array_with_replacer_pretty so it stays in sync with is_object_pointer; extract the threshold into a shared constant or helper and have both checks reference that symbol instead of hardcoding the literal. Use the existing gc_obj_type/ArrayHeader length guard and the is_object_pointer path as the places to update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/perry-runtime/src/json/replacer.rs`:
- Around line 264-274: Centralize the duplicated 10_000_000 array length cap
used in replacer::stringify_array_with_replacer_pretty so it stays in sync with
is_object_pointer; extract the threshold into a shared constant or helper and
have both checks reference that symbol instead of hardcoding the literal. Use
the existing gc_obj_type/ArrayHeader length guard and the is_object_pointer path
as the places to update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e9bc72a-56d7-4731-84da-8913e0cd8307
📒 Files selected for processing (4)
crates/perry-codegen/src/expr/static_field_meta.rscrates/perry-codegen/src/runtime_decls/strings.rscrates/perry-runtime/src/box.rscrates/perry-runtime/src/json/replacer.rs
This was referenced Jul 6, 2026
proggeramlug
added a commit
that referenced
this pull request
Jul 6, 2026
…scope the desugar's id rewrites (#6090) The #5951 one-element-array-box desugar (ce3eed1) regressed the compiled Next.js standalone server from 5/8 byte-identical routes to 0/8: every route returned a 21-byte 500 with `TypeError: Cannot convert undefined or null to object`. Bisected to ce3eed1 (parent + the #6055 boot fix is 5/8). Root cause — the declaring-`Let` array-wrap skipped a `let` with NO initializer: if let Some(e) = init.take() { *init = Some(Expr::Array(vec![e])); } but every use of the flagged id is still rewritten to `id[0]`. SWC emits exactly this shape for hoisted computed-property temps: let prop; class NodeNextRequest extends BaseNextRequest { static #_ = prop = _NEXT_REQUEST_META = ...; // mutates `prop` } (next/dist/server/base-http/node.js — the wrapper classes for EVERY request and response). `prop` is detected as a shared-mutable capture, its uses become `prop[0]`, but no array is ever allocated — so the static initializer's `prop[0] = ...` is an IndexSet on `undefined` and throws the observed TypeError the moment Next lazy-requires the module inside the first request, 500ing every route. Fix: wrap a None init as `[undefined]`, which preserves `let prop;` semantics through the box. Also hardens the pass's id handling (found while isolating the above, both real unsoundness even though neither caused this app's breakage): - detection results and rewrites are now scoped to the body that owns the ids instead of one module-global set applied to every body (LocalIds are not unique across function scopes — member params/lets restart their id space); - within a rewritten region, an id is only desugared when it is UNAMBIGUOUS (declared exactly once across deep `Let`s and nested-closure params — nested closures restart their id spaces, so a numeric rewrite is only sound for unique ids). An ambiguous id keeps the pre-#6054 split-cell behavior instead of risking corruption of an unrelated same-numbered local. - diagnostics: `PERRY_5951_SKIP_MODS=<substr,substr>` skips the desugar per module; `PERRY_5951_TRACE=1` prints the per-module desugar inventory and ambiguous-skip decisions (both compile-time, inert by default, matching the existing `PERRY_NO_5951` escape hatch). Validated: the #5951 suite passes unchanged (10/10); the Next.js standalone matrix goes 0/8 -> 5/8 byte-identical vs node v26 (/, /about, /counter, /api/hello GET+POST), server surviving the dynamic-route requests. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Jul 8, 2026
…ubs (#233) (#6138) When an array outgrows its capacity, `js_array_grow` allocates a larger copy and installs a `GC_FLAG_FORWARDED` stub at the old location — its first 8 bytes (length|capacity) become the forwarding pointer to the grown array. Every hot array accessor resolves this via `clean_arr_ptr`, and the plain-JSON path (`json/stringify.rs`) already did, but the JSON.stringify *replacer* paths — the array arm of `dispatch_pointer_with_replacer` and `stringify_array_with_array_replacer` — read `(*arr).length` directly on the stub, yielding a bogus multi-GB "length". A stale pre-grow pointer reaches these paths from the object graph: e.g. React's RSC flight stores a `[key, value]` pair, then grows it with `pair[i] = …`, while the serialized payload still holds the pre-grow reference. The garbage length was only kept from SIGBUS-ing by the 10M sanity cap (emit "null"), silently dropping the real data. Follow the forwarding chain before reading, so the CURRENT grown array is serialized. This is the shared root cause of the two length-cap band-aids (#6101 property-walk cap, #6055 JSON null-cap); the caps stay as defensive backstops for genuinely mis-classified pointers. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This was referenced Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two independent runtime fixes found while working the Next.js standalone matrix on current
main. Together they take the app from fails-to-boot back to 5/8 routes byte-identical vs node v26, with the 3 dynamic-RSC routes hanging per #5989 instead of killing the server.1.
mainboot regression: #6044 TDZ × #6037 class-capture refresh (commit 1)The #6044 TDZ made
js_box_get_bitsthrow on aTAG_TDZbox. That boot-broke every module using the standard SWC CJS interop shape (Next.js dist files, most transpiled npm packages):The getter closures forward-reference the class, so the class-captured module consts are TDZ-seeded (
PreallocateTdzBoxes([_fs, _path, ...])). The #6037 strategy emits aRegisterClassCapturessnapshot refresh after each captured var's assignment — the refresh emitted afterconst _fsreads_path's box while still in its dead zone → module init throwsCannot access undefined before initialization→ the process dies at boot. (Statement-order HIR dump:LET _fs → RegisterClassCaptures(reads _fs,_path) → LET _path.) Trigger needs ≥2 class-captured consts + the getter head. Repro: verbatimnext/dist/server/dev/browser-logs/file-logger.js.Fix: those refresh loads are Perry-internal materialization, not user reads. Bracket exactly them in a TDZ-suppression window (
js_tdz_suppress_begin/end, thread-local depth): inside it a dead box snapshots asundefined(the pre-#6044 behavior — a later refresh fixes the value up); outside it TDZ throws exactly as before. The window contains only side-effect-free captureLocalGets; only the coldTAG_TDZbranch consults the flag.Validated: verbatim file-logger repro + 6 bisect variants go boot-throw → working; 5 control variants unchanged; genuine TDZ violations still throw
ReferenceErrorbyte-identical to node (direct read-before-let, and via hoisted function); the legal closure-created-before-init-called-after shapes still work.Pre-existing #6044 gap noticed while validating (NOT addressed here, no regression):
typeof zbeforeconst zdoes not throw in Perry; node throws.2.
JSON.stringifyreplacer path SIGBUS → "null" (commit 2, #5989)The
/plaindynamic render feedsJSON.stringify(value, replacer, space)a value graph containing a corrupted/mis-encoded pointer. Three deref sites in the replacer walk guarded onlyis_handle_band, not general pointer validity, so the server died withBus error: 10on the first request:ptr_derefable(ptr)(top 16 bits ≤ 1, ≥ 0x10000, 8-byte aligned — mirrorsis_object_pointer's pre-load sanity) guardsapply_to_json's toJSON probe anddispatch_pointer_with_replacer(emit"null").GC_TYPE_ARRAYarm sanity-capslengthat 10M: a mis-classified structure read as an array with garbage length (~2.7e9, actually the low 32 bits of a neighboring heap pointer) walked OOB. Emit"null"instead.JSON.stringifymust never SIGBUS on a bad value. The upstream producer of the corrupted value graph is the remaining #5989 investigation; with this hardening the route hangs (per #5989) instead of killing the server.Validation (current
main, combined)/,/about,/counter,/api/helloGET+POST./plain,/posts/123,/fetcher: hang per nextjs: dynamic RSC render evaporates silently with a clean async graph after expected manifest ENOENTs (post-#5988 wall for 8/8) #5989, server stays up (was: SIGBUS, dead server).cargo fmt --checkclean; runtime+codegen build clean.Summary by CodeRabbit