Skip to content

expr: prevent stack overflow on deeply nested parentheses - #13404

Closed
koopatroopa787 wants to merge 1 commit into
uutils:mainfrom
koopatroopa787:fix-expr-stack-overflow-deep-parens
Closed

koopatroopa787 wants to merge 1 commit into
uutils:mainfrom
koopatroopa787:fix-expr-stack-overflow-deep-parens

Conversation

@koopatroopa787

Copy link
Copy Markdown
Contributor

Summary

Deeply nested parentheses like (((...))) cause a stack overflow in expr because the recursive descent parser creates ~8 stack frames per paren level (one parse_simple_expression, one parse_expression, and six parse_precedence calls). At ~105 levels the default thread stack is exhausted, producing a process abort.

Fix

Added a depth: usize counter to the Parser struct. When parse_simple_expression encounters ( and self.depth >= MAX_RECURSION_DEPTH, it returns ExprError::RecursionLimit ("expression too deeply nested", exit code 2) instead of recursing further.

MAX_RECURSION_DEPTH = 86 caps the recursive stack at ~688 frames, providing comfortable headroom below the observed ~840-frame overflow threshold in debug builds.

Test

A new regression test test_deeply_nested_parens_do_not_crash passes 5000 levels of nesting to expr and verifies it exits with code 2 and prints the error message, rather than crashing.

Fixes #13146

Deeply nested parentheses like (((...))) recurse through
parse_simple_expression → parse_expression → parse_precedence (×6) for
each level — ~8 frames per paren. At ~105 levels in debug builds the
default thread stack is exhausted, producing a SIGSEGV.

Add a depth counter to the Parser struct and return a new
ExprError::RecursionLimit error ("expression too deeply nested", exit 2)
when the paren nesting exceeds MAX_RECURSION_DEPTH (86). The limit is
chosen to cap the recursive stack at ~688 frames, providing comfortable
headroom below the observed debug-build overflow threshold.

Fixes uutils#13146
Copilot AI review requested due to automatic review settings July 14, 2026 18:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codspeed

codspeed Bot commented Jul 14, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 3.29%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 2 regressed benchmarks
✅ 337 untouched benchmarks
⏩ 46 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation du_max_depth_balanced_tree[(6, 4, 10)] 25.5 ms 26.5 ms -3.48%
❌ Simulation du_all_wide_tree[(5000, 500)] 16.2 ms 16.8 ms -3.1%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing koopatroopa787:fix-expr-stack-overflow-deep-parens (639b0d6) with main (76d7a53)2

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (67cc7a1) during the generation of this report, so 76d7a53 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@HackingRepo

Copy link
Copy Markdown
Contributor

codspeed was actually saying invalid benchmarks, @sylvestre can we have a workaround a workflow that use hyperfine or similar, codspeed is reporting highly invalid results

@xtqqczze

Copy link
Copy Markdown
Collaborator

#13333 might be a better approach?

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/misc/io-errors (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/retry (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/tail-n0f (passes in this run but fails in the 'main' branch)

@xtqqczze

Copy link
Copy Markdown
Collaborator

codspeed was actually saying invalid benchmarks, @sylvestre can we have a workaround a workflow that use hyperfine or similar, codspeed is reporting highly invalid results

The discrepancy comes from the different runtime environments (AMD EPYC 7763 64-Core Processor vs. Intel(R) Xeon(R) 6973P-C). Since GitHub Actions doesn’t provide consistent hardware across runners, this isn’t something we can realistically work around. In any case, since there were no changes to du, the benchmark discrepancies can be safely ignored.

@RenjiSann

Copy link
Copy Markdown
Collaborator

Will close since #13333 was merged

@RenjiSann RenjiSann closed this Jul 16, 2026
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.

expr: deeply nested parentheses cause a stack overflow (recursive parser)

5 participants