Raise wasm C stack size so deep recursion no longer traps (fixes #47)#48
Open
paulmanoni wants to merge 1 commit into
Open
Raise wasm C stack size so deep recursion no longer traps (fixes #47)#48paulmanoni wants to merge 1 commit into
paulmanoni wants to merge 1 commit into
Conversation
QuickJS's parser/evaluator recurse in C. The qjswasm target linked with wasm-ld's small default stack, so deeply (but ordinarily) nested source overflowed it and surfaced as "wasm error: out of bounds memory access" — e.g. ~1000 nested parens via plain Eval, and as few as ~15-20 levels of source nesting when running @vue/compiler-sfc (which adds its own recursion on top). Fixes fastschema#47. Root cause: the `--stack-first` / `--initial-memory` add_link_options sit AFTER add_executable(qjswasm), so they never applied to that target. Put the stack sizing in the target_link_options(qjswasm ...) block that actually takes effect: -z stack-size=16777216 (16 MiB C stack; was wasm-ld's small default) --initial-memory=20971520 (20 MiB; with --stack-first the stack lives at the bottom of linear memory, so initial memory must exceed stack-size + data) Verified: nested-paren Eval now succeeds to depth 20000 (was trapping at 1000), and a real 128-component Vue app that previously failed on 10 files now compiles all 128. qjs.wasm regenerated via `make` + wasm-opt -O3.
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
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.
Problem
Evaluating deeply‑nested expressions traps with
wasm error: out of bounds memory accessinstead of completing (or returning a catchable error). It's a C call‑stack overflow: QuickJS's parser/evaluator recurse in C, and theqjswasmtarget linked with wasm‑ld's small default stack.The ceiling is low enough to hit real code:
Evalof ~1000 nested parens traps;@vue/compiler-sfc(which adds its own recursion on top of the source AST) it traps at only ~15–20 levels of ordinary source nesting — array/object config literals with arrow callbacks, ternaries, optional chaining, etc.Fixes #47.
Root cause
--stack-firstand--initial-memoryare added viaadd_link_options(...)afteradd_executable(qjswasm)inqjswasm.cmake, so they never apply to that target (add_link_optionsonly affects targets created after it). The effective link therefore used wasm‑ld's default stack size.Fix
Move the stack sizing into the
target_link_options(qjswasm PRIVATE ...)block that actually applies to the target:qjs.wasmregenerated viamake+wasm-opt -O3(WASI SDK 33).Verification
Evalnow succeeds to depth 20000 (previously trapped at 1000).@vue/compiler-sfcon qjs: 10/128 files failed before, 0/128 fail now.Notes / follow‑ups (not in this PR)
(*Runtime).callreturned anerrorinstead ofpanic-ing (today a single deepEvalcan crash the host and poison the runtime), and if QuickJS's ownJS_SetMaxStackSizeguard were coupled just under the linker stack so true runaway recursion throws a catchableRangeError. Glad to do those in a follow‑up if you're interested.