Skip to content

fix(ts-runtime): bound GLR stack-merge recursion to prevent stack overflow#1002

Open
halindrome wants to merge 1 commit into
DeusData:mainfrom
halindrome:p913-glr-recursion
Open

fix(ts-runtime): bound GLR stack-merge recursion to prevent stack overflow#1002
halindrome wants to merge 1 commit into
DeusData:mainfrom
halindrome:p913-glr-recursion

Conversation

@halindrome

Copy link
Copy Markdown
Contributor

Summary

Bounds the recursive GLR stack-merge in the vendored tree-sitter runtime (internal/cbm/vendored/ts_runtime/src/stack.c) so pathologically nested, grammar-ambiguous input can no longer overflow the native stack during parsing. Split out of #461 at maintainer request — runtime patches warrant separate provenance/risk review.

Closes #913.

Problem

stack_node_add_link merges ambiguous parse-stack heads recursively — once per nesting level. Perl's paren-optional call grammar (f(f(f(...)))) makes every level ambiguous, so a deep chain drives that recursion until it overflows a small (~1 MB Windows) thread stack, and even an 8 MB POSIX stack at extreme depth — before any extraction runs. Unambiguous grammars (Java, C++) never hit the recursive merge.

Fix

  • Narrow recursion cap. A depth-tracked stack_node_add_link_inner caps the recursive merge at CBM_TS_STACK_MERGE_MAX_DEPTH (512). The public stack_node_add_link is a thin wrapper seeding depth 0, so every existing call site is unchanged.
  • Graceful degradation. Past the cap the ambiguity is left on the GLR stack instead of merged: a valid parse, never a wrong one. 512 frames sits well within a 1 MB stack while far exceeding any realistic source nesting.
  • No unrelated runtime behavior changes. Only the recursive-merge depth is bounded; normal parses are untouched.

Regression

perl_glr_deep_parse_recursion_capped (tests/test_stack_overflow.c) parses deep ambiguous Perl directly — a forked child on POSIX (crash isolation), in-process on Windows — bypassing cbm_extract_file's CBM_PERL_MAX_PARSE_NESTING pre-parse guard so the vendored cap itself is exercised rather than the guard. Without the cap this overflows the 1 MB Windows stack (Windows CLANG64 CI is the negative-case arbiter, as with #461); with it, the parse returns cleanly.

Scope

The higher-level Perl pre-parse guard added in #461 stays in place — this PR is scoped to the vendored runtime cap only. That guard can be retired in a follow-up once this cap is proven in the field.

Provenance

Self-contained depth guard in the vendored runtime; no upstream-grammar regeneration. Verified: scripts/build.sh clean under -Werror; scripts/test.sh green (the one unrelated red, cli_hook_gate_script_no_predictable_tmp_issue384, is pre-existing/environmental in the local sandbox and does not touch this code); clang-format applied to the test.

…rflow

tree-sitter's GLR parser merges ambiguous parse-stack heads recursively in
stack_node_add_link (internal/cbm/vendored/ts_runtime/src/stack.c), once per
nesting level. On pathologically nested input whose grammar is ambiguous
(Perl's paren-optional calls, f(f(f(...)))), that recursion overflows the
native stack during the parse — a small ~1 MB Windows thread stack, and even
an 8 MB POSIX stack at extreme depth — before any extraction runs. Grammars
that are unambiguous here (Java, C++) never trigger the recursive merge.

Cap the recursive merge at CBM_TS_STACK_MERGE_MAX_DEPTH (512) via a
depth-tracked inner worker; the public stack_node_add_link is a thin wrapper
seeding depth 0, so every existing call site is unchanged. Past the cap the
ambiguity is left on the GLR stack instead of merged — a valid parse, never a
wrong one. 512 frames is well within a 1 MB stack while far exceeding any
realistic source nesting.

Add perl_glr_deep_parse_recursion_capped: a crash-isolating regression that
parses deep ambiguous Perl directly (a forked child on POSIX, in-process on
Windows), bypassing cbm_extract_file's CBM_PERL_MAX_PARSE_NESTING pre-parse
guard so the vendored cap itself is exercised rather than the guard.

Narrow vendored-runtime change; no other parsing behavior is affected. The
higher-level Perl pre-parse guard (issue DeusData#913 context) stays in place and can
be retired in a follow-up once this cap is proven in the field.

Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
@halindrome

Copy link
Copy Markdown
Contributor Author

QA Round 1

Contract source: issue #913. Reviewer: Claude Code (claude-opus-4-8). Base main (upstream/main), head p913-glr-recursion.

Contract Verification

Criterion Status Evidence
Bound recursive GLR stack-merge in stack_node_add_link so it cannot overflow the native stack on pathologically nested ambiguous input ✅ satisfied stack.c — the sole recursive call (stack_node_add_link_inner) is guarded by if (depth < CBM_TS_STACK_MERGE_MAX_DEPTH); depth increments once per level, cap 512. No other recursion path exists.
Degrade gracefully past the cap: bounded merge still produces a VALID parse, never an incorrect one ✅ satisfied Past the cap the recursive sub-link merge is skipped but the node is still deduped and dynamic_precedence is still updated before return — a valid tree is still produced. Below 512, behavior is byte-identical to upstream, so realistic (<512-deep) ambiguous parses are unaffected.
Include a crash-isolating regression (fork harness) exercising the fix ✅ satisfied perl_glr_deep_parse_recursion_capped + so_parse_crashes (POSIX fork + waitpid + WIFSIGNALED; Windows in-process) parse deep ambiguous Perl directly via cbm_ts_language/ts_parser_parse_string, bypassing the extract-level guard. Ran locally: PASS.
Narrow vendored change only; all existing call sites unchanged; normal parses unaffected ✅ satisfied Only call site uses the unchanged 3-arg public stack_node_add_link wrapper. Diff touches only stack.c + test_stack_overflow.c.
Builds clean; existing test suite still green ✅ satisfied Build clean under -Werror. Suite: 5985 passed, 1 skipped, plus one unrelated failure (see below).

Findings

Contract: none. Regression: none.

Verified in depth: the recursion is genuinely bounded (only the depth-guarded call recurses); the non-recursive fall-through — dedup, dynamic_precedence update, link append, MAX_LINK_COUNT guard — is preserved exactly past the cap; the public wrapper keeps its original 3-arg signature so every call site is unchanged; unsigned depth < 512 has no signedness/overflow issue. Test harness verified: buffer sizing safe (DEPTH=30000, sz=90256, ~90045 bytes written), parser/tree freed in both child and Windows paths, NULL cbm_ts_language handled, #include "lang_specs.h" resolves cbm_ts_language while TSParser/ts_parser_* come transitively via cbm.h.

Schema change

None. Touches only a vendored C runtime file and a C test — no DDL, no migrations, no code reading/writing a DB column.

Pre-existing issue discovered (observation — non-blocking)

Notes (already disclosed in the PR description)

  • POSIX test strength: at DEPTH=30000 the recursion frames (~7.8 MB) may not overflow the 8 MB macOS/Linux main-thread stack even without the fix, so on POSIX the negative case can pass vacuously. Windows CLANG64 CI (1 MB stack, in-process harness path) is the designated negative-case arbiter, mirroring feat(perl-lsp): Perl LSP-tier semantic resolution (Closes #459) #461. The fork harness still isolates any crash that does occur on POSIX.
  • Graceful-degradation nuance: past the cap, on the mergeable branch the node is deduped and returns without merging link.node's sub-links (dropped from that head, still a valid tree) — matching the spirit of the existing MAX_LINK_COUNT bail-out. Only reachable beyond 512 nesting levels. Not a defect.

Summary

Severity Contract Regression
Critical 0 0
Major 0 0
Minor 0 0
Total 0 0

SAST: skipped — GitHub code scanning is not accessible for this fork-contributor context (API 403); CI CodeQL / security-static checks run on GitHub.

Verdict: clean round. The change minimally and correctly bounds the only recursive merge path, preserves the wrapper signature and sole call site, builds clean, and its new regression passes. The single suite failure is in an untouched file and unrelated.


QA performed by Claude Code (claude-opus-4-8)

@halindrome halindrome marked this pull request as ready for review July 10, 2026 13:34
@halindrome halindrome requested a review from DeusData as a code owner July 10, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ts_runtime: bound GLR stack-merge recursion to prevent stack overflow on pathological input

2 participants