Skip to content

Fix #722 (#if: &&, || and ?: do not short-circuit) - #723

Merged
danmar merged 3 commits into
masterfrom
fix-722
Nov 7, 2026
Merged

danmar merged 3 commits into
masterfrom
fix-722

Conversation

@danmar

@danmar danmar commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Replace the constFold* functions with a recursive-descent evaluator. Operands that are not evaluated are only parsed, so they can't cause division by zero errors.

@dmcppcheck dmcppcheck 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.

AI review

The new recursive evaluator crashes with a stack overflow on deeply nested #if expressions, and base handles the same input. The recursion depth has no limit. Every ( goes through about 12 stack frames (conditional → logicalOr → … → unary → primary → conditional), and a chain of unary !/~ goes one frame per operator. With the default 8 MB stack, about 20000 nested parentheses (debug build) or 50000 (-O2) give a SIGSEGV. Both gcc and clang accept this input, and so does base simplecpp, which evaluates the condition iteratively. Threads with smaller stacks will crash much sooner, for example cppcheck worker threads (on Windows the default thread stack is 1 MB). A fuzzer will also hit this. A nesting-depth counter that throws std::runtime_error past some limit would fix it.

Apart from that, the short-circuit behaviour is correct in every case I tried, including nested ?: and combinations of &&/||/?:. Results for the valid expressions I compared match base, except where the PR intends a change. I found no other new problems.

Testing

Built /work/pr with make -j8 testrunner simplecpp CXXOPTS=-Werror without warnings, and built base too. In pr/:

  • ./testrunner passes.
  • python3 run-tests.py: 176 tests, 0 failed (the same as base).
  • python3 -m pytest integration_test.py: 128 passed.
  • ./selfcheck.sh passes.

Stack overflow (regression):

cd /work/scratch
python3 -c "n=20000; open('d2.c','w').write('#if '+'('*n+'1'+')'*n+'\nyes\n#endif\n')"
/work/pr/simplecpp d2.c    # Segmentation fault (rc 139)
/work/base/simplecpp d2.c  # prints: yes
gcc -E d2.c; clang -E d2.c # both print: yes

A -O2 build (g++ -O2 -std=c++11 main.cpp simplecpp.cpp) still segfaults at n=50000. A unary chain such as '#if ' + '~'*200000 + '1' (or '! '*200000) also segfaults in pr.

Short-circuit comparison with gcc -E / g++ -E and base: I compared about 50 expressions. All of these now match gcc, where base reported division by zero:

  • 1 ? 1 : 0 || 1/0
  • (0 && 1/0) || 1
  • 1 && 0 ? 1/0 : 5
  • 0 ? (1/0) : (0 ? 1/0 : 1)
  • 1 ? 0 ? 1/0 : 2 : 3
  • defined(X) && X/0
  • (1) ? (0) : (1/0)

Precedence, associativity, alternative tokens and suffixed, hex and char literals give the same results as base. Some differences from gcc exist in base as well, so the PR does not cause them: -1 > 0u, 1 << 64, 1 and not 0 in C++, 0b101, 1 , 2, and invalid expressions being silently false.


Automated review of 79d0f4e by Claude. The pull request was built and tested locally.

Comment thread simplecpp.cpp Resolved
Comment thread simplecpp.cpp Resolved

@dmcppcheck dmcppcheck 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.

AI review

The nesting limit added in 5310bad rejects valid #if expressions that base, gcc and clang all accept. It fixes the stack overflow, but the limit of 256 is low, and one counter is shared by parentheses, every prefix unary operator and every ?:. Three kinds of input now fail with failed to evaluate #if condition, expression is nested too deeply:

  • a chain of more than 256 ?:, which is a common pattern in generated code and lookup macros;
  • macros defined in terms of each other, where each one wraps the previous one in parentheses (#define Xn (Xn-1 + 1)), once the chain is deeper than 256;
  • only about 128 levels of -( … ), because the unary operator and the parenthesis each count as one level.

Base evaluated all of these without trouble. Annex B's "256 nesting levels of parenthesized expressions" is a minimum that implementations must support, not a maximum. cppcheck will now skip or mis-evaluate such conditions. A much larger limit would fix this, and so would counting only real recursion through (: handle prefix operators in a loop and right-nested ?: iteratively.

I found no other new problems. Short-circuiting and the other tests behave correctly.

Testing

I built /work/pr with make -j8 testrunner simplecpp CXXOPTS=-Werror and got no warnings. I also built base.

  • ./testrunner: passes, including ifShortCircuit and ifNestingDepth.
  • python3 run-tests.py: 0 failed.
  • python3 -m pytest integration_test.py: 128 passed.
  • ./selfcheck.sh: passes.

Regression: valid deep expressions are rejected. The test files are generated in /work/scratch:

n=300
open('chain.c','w').write('#define X0 0\n'+''.join('#define X%d (X%d+1)\n'%(i,i-1) for i in range(1,n+1))+'#if X%d == %d\nyes\n#else\nno\n#endif\n'%(n,n))
open('tern.c','w').write('#define V 299\n#if '+''.join('V==%d ? %d : '%(i,i) for i in range(300))+'-1\nyes\n#endif\n')
open('neg.c','w').write('#if '+'-('*150+'1'+')'*150+' == 1\nyes\n#endif\n')

For example, tern.c contains #if V==0 ? 0 : V==1 ? 1 : ... : -1, and chain.c contains #define X1 (X0+1) … #if X300 == 300.

$ /work/pr/simplecpp chain.c
chain.c:302: syntax error: failed to evaluate #if condition, expression is nested too deeply
$ /work/pr/simplecpp tern.c
tern.c:2: syntax error: failed to evaluate #if condition, expression is nested too deeply
$ /work/pr/simplecpp neg.c
neg.c:1: syntax error: failed to evaluate #if condition, expression is nested too deeply

/work/base/simplecpp, gcc -E and clang -E print yes for all three files.


Automated review of 5310bad by Claude. The pull request was built and tested locally.

Comment thread simplecpp.cpp Resolved
Comment thread simplecpp.cpp Resolved
Comment thread simplecpp.cpp Resolved

@dmcppcheck dmcppcheck 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.

AI review

The new right-to-left ?: folding is quadratic in the length of a ?: chain. For every ? it folds, constFoldQuestionOp scans the whole range again from tok1 to find the last ?. A generated lookup chain such as #if V==0 ? 0 : V==1 ? 1 : ... : -1 with 20000 entries takes 10 s instead of 0.1 s in base, and 5000 entries take 0.56 s instead of 0.025 s. cppcheck preprocesses every file with this code, so the cost matters.

I found no other new problems. Short-circuiting through &&, || and ?: now matches gcc and clang in every case I tried, including nested and chained ?: and error values that pass through unary, arithmetic, shift, comparison and bitwise operators. A small side note: the PR description still says "Replace the constFold* functions with a recursive-descent evaluator", but the current code keeps the constFold* functions and uses error values instead.

Testing

I built /work/pr with make -j8 testrunner simplecpp CXXOPTS=-Werror and got no warnings. I also built base/.

  • ./testrunner: 0 failed.
  • python3 run-tests.py: 0 failed.
  • python3 -m pytest integration_test.py: 128 passed.
  • ./selfcheck.sh: passes.

Comparison with gcc -E, clang -E and base. I compared about 35 #if expressions, for example 0 && 1/0 || 1, 1 || 1/0 && 1, 0 ? 1 : 0 ? 1/0 : 1, 0 && ~(1/0), 0 && 1 << (1/0), 0 && (1/0 == 1/0), (0 && 1/0) ? 1/0 : 1, (1 ? 2 : 3) + (0 ? 1/0 : 4) == 6 and 0 && (-9223372036854775807-1)/-1. All of them match gcc and clang now, where base reported division by zero. Expressions in which the division is evaluated, such as 1 ? 1/0 : 0, -(1/0) and 1 && (0 || 1/0), still give an error. The remaining differences from gcc also exist in base (the comma operator, invalid expressions being silently false).

Performance regression with long ?: chains:

cd /work/scratch
python3 -c "
n=20000
open('tc.c','w').write('#define V 5\n#if '+''.join('V==%d ? %d : '%(i,i) for i in range(n))+'-1\nyes\n#endif\n')"
time /work/base/simplecpp tc.c   # yes, real 0m0.107s
time /work/pr/simplecpp tc.c     # yes, real 0m10.053s
?: entries in the chain base pr
1000 0.008 s 0.030 s
5000 0.025 s 0.564 s
20000 0.107 s 10.05 s

The time grows quadratically.

I also checked 20000 parenthesised (0 && 1) terms joined by +. Base and pr both take about 4.2 s, so that slowdown already exists in base and the PR does not cause it.


Automated review of 1d63864 by Claude. The pull request was built and tested locally.

Comment thread simplecpp.cpp Resolved
A division by zero no longer throws while folding. It gives an error
value, that is passed on by the other operators. && || and ?: discard
it when the operand is not evaluated. The error is thrown if the error
value is still there after folding.

Also fold chained ?: from the right, since ?: is right-associative.
After a ?: is folded, search backward from the result for the next ?
instead of rescanning the whole range.
…nd ^

isAlternativeBinaryOp() did not accept an error value as operand, so
"0 and (1/0)" was not converted to "0 && (1/0)" and the error was reported.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@danmar
danmar merged commit 45de9af into master Nov 7, 2026
47 checks passed
@danmar
danmar deleted the fix-722 branch November 7, 2026 10:50
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