Skip to content

[naga wgsl-in] Restore accidentally removed recursion depth check.#9424

Merged
teoxoy merged 1 commit intogfx-rs:trunkfrom
jimblandy:wgsl-in-restore-recursion-check
Apr 14, 2026
Merged

[naga wgsl-in] Restore accidentally removed recursion depth check.#9424
teoxoy merged 1 commit intogfx-rs:trunkfrom
jimblandy:wgsl-in-restore-recursion-check

Conversation

@jimblandy
Copy link
Copy Markdown
Member

In naga::front::wgsl::parse, restore an incorrectly removed call to Parser::track_recursion, moving it from Parser::unary_expression to Parser::expression.

To prevent pathological shaders from overflowing the Rust stack, 34cfee6 introduced the Parser::track_recursion function to track how deeply the parser has recursed, and throw an uncategorized error if it has far exceeded the depth we expect to encounter while processing any reasonable input. These checks were added to a few strategically chosen functions, such that any recursive execution would need to go through at least one check.

When fca3c2e rewrote Parser::unary_expression to use a loop instead of recursing, it removed that function's call to track_recursion. While it was true that unary_expression itself no longer recursed, its call to track_recursion was also serving as a choke point for the rest of the expression parser; removing the check reintroduced the potential for unbounded recursion.

This commit reintroduces the recursion depth check, but this time places it in Parser::expression, a more natural location that is adequate now that unary_expression no longer recurses on its own.

In local testing, a nesting depth limit of 256 still allowed the Rust stack to overflow, so this patch also lowers the depth limit to 200.

@jimblandy
Copy link
Copy Markdown
Member Author

This bug was not our highest priority, but the process of just filing it was most of the work of fixing it, so I just went ahead and wrote a fix.

In `naga::front::wgsl::parse`, restore an incorrectly removed call to
`Parser::track_recursion`, moving it from `Parser::unary_expression`
to `Parser::expression`.

To prevent pathological shaders from overflowing the Rust stack, 34cfee6
introduced the `Parser::track_recursion` function to track how deeply the parser
has recursed, and throw an uncategorized error if it has far exceeded the depth
we expect to encounter while processing any reasonable input. These checks were
added to a few strategically chosen functions, such that any recursive execution
would need to go through at least one check.

When fca3c2e rewrote `Parser::unary_expression` to use a loop instead of
recursing, it removed that function's call to `track_recursion`. While it was
true that `unary_expression` itself no longer recursed, its call to
`track_recursion` was also serving as a choke point for the rest of the
expression parser; removing the check reintroduced the potential for unbounded
recursion.

This commit reintroduces the recursion depth check, but this time places it in
`Parser::expression`, a more natural location that is adequate now that
`unary_expression` no longer recurses on its own.

In local testing, a nesting depth limit of 256 still allowed the Rust
stack to overflow, so this patch also lowers the depth limit to 200.
@jimblandy jimblandy force-pushed the wgsl-in-restore-recursion-check branch from e5b4f4b to 0af9ecf Compare April 13, 2026 22:23
@jimblandy jimblandy requested a review from teoxoy April 13, 2026 22:23
@teoxoy
Copy link
Copy Markdown
Member

teoxoy commented Apr 14, 2026

The only functions that indirectly recurse:

  • enclosed_expression
  • template_elaborated_ident
  • arguments

@teoxoy teoxoy merged commit fccadd6 into gfx-rs:trunk Apr 14, 2026
58 checks passed
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.

2 participants