Skip to content

Root-cause fix for CFG-builder stack overflow: convert recursive-descent builders to iterative walks #160

Description

@stephenc222

Context

PR #159 fixed a SIGABRT (stack overflow) crash hit during nightly hotspots-cloud crawl runs analyzing rust-lang/rust with --per-function-touches. The crash: a rayon worker thread overflowed its default 2MB stack building a CFG for a deeply nested/generated source file (e.g. under tests/ui/).

PR #159's fix — configuring the global rayon thread pool with a 32MB worker stack (hotspots-cli/src/cmd/analyze.rs) — is a mitigation, not a fix. It raises the ceiling but doesn't remove it: a sufficiently deep or adversarially generated file can still overflow any fixed stack size.

A follow-up (branch fix/rust-cfg-depth-guard) adds a depth guard (MAX_CFG_DEPTH) to the Rust CFG builder (hotspots-core/src/language/rust/cfg_builder.rs) so that builder now bails out to a minimal entry->exit CFG past 500 levels of nesting instead of recursing unboundedly. That closes the crash for that builder, but:

  1. It's still a depth limit, not a structural fix — legitimate (if unusual) deeply-nested code past the threshold gets silently downgraded to a minimal CFG rather than analyzed.
  2. It only covers the Rust builder. The same mutually-recursive, unbounded-depth pattern exists in every other language's CFG builder: the shared JS/TS builder (hotspots-core/src/cfg/builder.rs, ~14 mutually-recursive visit_* methods) plus separate builders for Go, Java, Python, C, and C# (hotspots-core/src/language/{go,java,python,c,csharp}/cfg_builder.rs). None of them have a depth guard today.
  3. Testing the Rust depth guard surfaced a related, currently-unguarded gap: syn::parse_str itself (the Rust parser, not our CFG builder) is also a recursive-descent implementation with no depth bound, and overflows a small stack at a nesting depth well below MAX_CFG_DEPTH. The same is presumably true of the other languages' parsers (swc for JS/TS, tree-sitter grammars for Go/Java/Python/C/C#) to varying degrees. A depth guard in our own CFG-building code does not protect against overflow happening one layer earlier, during parsing.

What a proper root-cause fix looks like

Convert the recursive-descent CFG builders from direct function-call recursion to an iterative walk over an explicit, heap-allocated work stack (i.e. a manual CPS-style transform of the current visitor logic). Concretely, for a construct like visit_if/build_if_cfg:

  • Today: visit the then-branch (recursing into nested statements), come back up the call stack, create the join node, then visit the else-branch (recursing again), then wire the join.
  • Iteratively: push explicit "frames" onto a heap Vec/stack describing "what to do when this branch's traversal completes" (e.g. AfterThenBranch { join_node, else_expr }), and pop/drive them in a loop instead of relying on the native call stack to hold that continuation.

This removes the failure mode entirely — depth becomes bounded by available heap/memory instead of a fixed thread stack — rather than just raising or gating the limit.

Scope

This touches at least:

  • hotspots-core/src/cfg/builder.rs (JS/TS, shared, ~14 mutually-recursive visit_* variants)
  • hotspots-core/src/language/go/cfg_builder.rs
  • hotspots-core/src/language/java/cfg_builder.rs
  • hotspots-core/src/language/python/cfg_builder.rs
  • hotspots-core/src/language/rust/cfg_builder.rs
  • hotspots-core/src/language/c/cfg_builder.rs
  • hotspots-core/src/language/csharp/cfg_builder.rs

All of tests/golden/*.json would need re-verification, since edge/node construction order could shift even where final CC values are unchanged — several of these builders were just reworked in v1.35.3 (Java ternary/&&/||/lambda edges, Python match-case branching, Go statement_list/break/continue routing), so this needs to land after those are stable, not concurrently.

Also worth scoping separately

The parser-level recursion gap (point 3 above) is a distinct problem from the CFG-builder recursion and would need its own investigation per parser (syn for Rust, swc for JS/TS, tree-sitter grammars for the rest) — tree-sitter's C API in particular may already have depth/error-recovery guards worth checking before assuming this needs custom handling.

Do not

  • Do not treat "raise the stack size further" or "lower MAX_CFG_DEPTH further" as a substitute for this — both are the same class of mitigation already shipped/in-review.
  • Do not attempt this as a single PR across all 7 builders — split per language to keep golden-fixture review tractable.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions