codegen(calls): pad under-applied same-module direct calls with undefined (#8770) - #8852
Closed
proggeramlug wants to merge 1 commit into
Closed
Conversation
…ined (PerryTS#8770) A same-module direct call with fewer arguments than the callee's declared parameter count lowered only the provided args, leaving the remaining FP argument registers holding caller-saved garbage — which the callee then read as JS values. The cross-module twin (extern_func.rs, the issue PerryTS#608 arm) has always padded missing trailing args with TAG_UNDEFINED; the same-module plain arm sat "one else away" (PerryTS#7154's own words) unpadded. On the Claude Code bundle — one giant module, so EVERY direct call resolves through the same-module arm — `aP([q])` for `function aP(q, K = !1, _)` handed K/_ whatever d1/d2 held after js_array_from_values: impossible-NaN bit patterns (0xffffffffffffffff) that flowed into truthiness tests and method receivers (`_.get(A)`) and faulted in shape_is_url_search_params / js_is_truthy (~60% of `cc -p` runs SEGV), or silently corrupted the async iteration ("Detected unsettled top-level await", most of the rest). With the padding, `cc -p` runs 30/30 clean: 0 SEGV, 0 hangs, 0 unsettled awaits, node-identical output on every run. The GC-knob correlations the long PerryTS#8770 investigation chased (scavenge pacing, conservative-scan "fixes") were register-content side effects of the missing padding, not collector bugs. Regression test: an under-applied direct call to a 3-param callee must emit all three double args, the omitted two as the TAG_UNDEFINED literal. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughSame-module direct calls now pad omitted trailing arguments with NaN-boxed ChangesSame-module under-application
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized code-generation change pads omitted direct-call arguments with undefined and includes regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the bug, impact, fix, and validation, but it does not follow the repository template. It omits the required Summary, Changes, Related issue, Test plan, Screenshots / output, and Checklist sections.
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
proggeramlug
added a commit
that referenced
this pull request
Aug 26, 2026
* perf: cache owning Uint32Array admissions * perf: fast-path Array subclass length misses * perf(codegen): route proven Array length writes * perf(runtime): bulk-truncate ordinary dense arrays * chore: add array truncation changelog * runtime: add Node-API host core * docs: add Node-API host changelog fragment * runtime: harden Node-API host contracts * runtime: bound Node-API UTF-16 encoding * codegen(calls): pad under-applied same-module direct calls with undefined (#8770) A same-module direct call with fewer arguments than the callee's declared parameter count lowered only the provided args, leaving the remaining FP argument registers holding caller-saved garbage — which the callee then read as JS values. The cross-module twin (extern_func.rs, the issue #608 arm) has always padded missing trailing args with TAG_UNDEFINED; the same-module plain arm sat "one else away" (#7154's own words) unpadded. On the Claude Code bundle — one giant module, so EVERY direct call resolves through the same-module arm — `aP([q])` for `function aP(q, K = !1, _)` handed K/_ whatever d1/d2 held after js_array_from_values: impossible-NaN bit patterns (0xffffffffffffffff) that flowed into truthiness tests and method receivers (`_.get(A)`) and faulted in shape_is_url_search_params / js_is_truthy (~60% of `cc -p` runs SEGV), or silently corrupted the async iteration ("Detected unsettled top-level await", most of the rest). With the padding, `cc -p` runs 30/30 clean: 0 SEGV, 0 hangs, 0 unsettled awaits, node-identical output on every run. The GC-knob correlations the long #8770 investigation chased (scavenge pacing, conservative-scan "fixes") were register-content side effects of the missing padding, not collector bugs. Regression test: an under-applied direct call to a 3-param callee must emit all three double args, the omitted two as the TAG_UNDEFINED literal. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP * fix(async_hooks): complete node suite parity * chore: batch-landing fixes (node-api scoped ptrs, header/ic_miss splits, fmt) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Contributor
Author
|
Landed on |
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.
The bug
A same-module direct call with fewer arguments than the callee's declared parameter count lowered only the provided args, leaving the remaining FP argument registers holding caller-saved garbage — which the callee then read as JS values.
The cross-module twin (
extern_func.rs, the issue #608 arm) has always padded missing trailing args withTAG_UNDEFINED. The same-module plain arm infunc_ref.rssat "oneelseaway" (#7154's own comment about this exact file-pair pattern) unpadded. The three rest-parameter arms pad vialower_rest_call_args_rooted; only the plain arm was missed.Impact (this is the #8770 Claude Code crash family)
The Claude Code bundle is one giant module, so every direct call resolves through the same-module arm — and minified JS under-applies constantly.
aP([q])forfunction aP(q, K = !1, _)handedK/_whateverd1/d2held afterjs_array_from_values: impossible-NaN bit patterns (0xffffffffffffffff) that flowed into truthiness tests (if (_)) and method receivers (_.get(A)) and faulted inshape_is_url_search_params/js_is_truthy(~60% ofcc -pruns SEGV), or silently corrupted the async iteration (Detected unsettled top-level await, most of the rest).The GC-knob correlations the long #8770 investigation chased (scavenge pacing,
PERRY_CONSERVATIVE_STACK_SCAN=Full"fixing" it) were register-content side effects of the missing padding — different collector paths leave different residue in the never-written argument registers — not collector bugs.The fix
In
try_lower_func_ref_call's plain (non-rest) arm, after lowering the provided args, padloweredwith theTAG_UNDEFINEDliteral up todeclared_count— mirroring the cross-module twin.declared_countis the fullf.params.len()from the function registry (includes default and trailing params, which is what the callee's compiled signature and its default-parameterundefinedtests expect).Validation
cli_2.1.112.js(68 MB bundle):cc -p30/30 clean — 0 SEGV, 0 hangs, 0 unsettled awaits, node-identical output on every run (baseline: ~60% SEGV, ~73% corrupted async).--version/--helpunchanged-clean.underapply_pad_tests.rs): an under-applied direct call to a 3-param callee must emit all three double args, the omitted two as theTAG_UNDEFINEDliteral.perry-codegensuite: 1223 passed, 0 failed.https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP
Summary by CodeRabbit
Bug Fixes
undefined, matching calls across modules.Tests
undefinedvalues.