Fix Uncontrolled Recursion in simplifyAddBracesToCommand

[emptyiscolor]

Related case: https://sourceforge.net/p/cppcheck/discussion/development/thread/eea4fd963a/

Applied Fix — Recursion Depth Limit

Added an int depth parameter (default 0) to both functions. Every recursive call passes depth + 1, and at the top of simplifyAddBracesToCommand we bail out when depth > 500:

Test

Both PoC vectors (100K else if chain and 50K nested if) no longer cause a stack overflow by running the PY code from the discussion. Normal code continues to be analyzed correctly.

Alternative Fix — Iterative Approach

The else if chain recursion (Path 1) can be eliminated by converting the tail-recursive call into a loop inside simplifyAddBracesToCommand.

This eliminates Path 1 (else-if chain) with zero stack growth regardless of chain length. Path 2 (mutual recursion with simplifyAddBracesPair) would still need a depth limit or its own iterative conversion using an explicit stack/worklist.

A full iterative conversion of both paths is a larger refactor but would make the code completely immune to stack overflow from any input shape.

[chrchr-github]

Thanks for your contribution. If we go with the depth limit, we should probably throw an error when the limit is exceeded. Also please add a test.

[firewave]

Thanks for your contribution. If we go with the depth limit, we should probably throw an error when the limit is exceeded. Also please add a test.

We have lots of depth bailouts in the code which report nothing so this should be fine.

I have a commit which flags all of those and I would like to introduce debug messages when they are hit - and in a perfect world also store all the limits in a single place so they could be tuned (at least being documented instead of hidden away in some default parameters).

[firewave]

Not having a testcase is not good though.

And I am wondering if the depth should be implemented the other way around. The limit is set in the default parameter and it is counted down - as we do in other cases. That would prevent unbounded calls being introduced in future changes.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[emptyiscolor]

Thanks for the feedback. The updated code switched to the count-down depth pattern used elsewhere in the codebase:
The default parameter is depth = 500, recursive calls pass depth - 1, bail out when depth < 0. This ensures new call sites are bound by default.
Added ifAddBracesDepthLimit test case to testtokenize.cpp : covering both recursion paths (1000-clause else-if chain and 1000-level nested if). Bailout is a silent return, consistent with existing depth limits (./testrunner TestTokenizer).

[danmar]

lgtm