Skip to content

Fix #15000: Propagate integral cast ranges with path-safe value flow - #8819

Open
jfdeverge wants to merge 1 commit into
cppcheck-opensource:mainfrom
jfdeverge:dev/fix_misra_14_3
Open

jfdeverge wants to merge 1 commit into
cppcheck-opensource:mainfrom
jfdeverge:dev/fix_misra_14_3

Conversation

@jfdeverge

@jfdeverge jfdeverge commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

There was missing detection of 'MISRA C:2012 Rule 14.3" on some specific code structure generated by Matlab Simulink coder.
We identified these issues by comparing its results with reports previously generated by our company’s internal tool on the same software.

The bug report would have looked like below.

Problem

Cppcheck did not report a MISRA C:2012 Rule 14.3 violation when a comparison
used an integer expression whose range was provably bounded by casts and
constant arithmetic. The existing check handled a variable compared with a
value outside the variable's declared type range, but did not evaluate this
equivalent expression form.

Minimal Reproducer

typedef unsigned int uint32;
typedef unsigned long long uint64;
typedef long long sint64;

void f(uint32 x)
{
    uint64 tmp = ((uint64)x) + 1ULL;
    if (tmp > 4294967295ULL)
        tmp = 4294967295ULL;

    if ((((sint64)((uint32)tmp)) - 1LL) > 4294967295LL) {
    }
}

After the clamp and casts, the left-hand expression cannot exceed
UINT32_MAX - 1. Therefore the final condition is always false and produces
the knownConditionTrueFalse diagnostic. The company's end-to-end MISRA
postprocessor mapping was not exercised here.

Observed Behavior

Before the fix, the final comparison produced no diagnostic. This allowed the
generated-code pattern to pass through the cppcheck-based MISRA pipeline
without the expected Rule 14.3 finding.

Comment thread lib/checkcondition.cpp Fixed
Comment thread test/testcondition.cpp Resolved
@chrchr-github

Copy link
Copy Markdown
Collaborator

I have created this ticket: https://trac.cppcheck.net/ticket/15000

@jfdeverge

Copy link
Copy Markdown
Contributor Author

I will update the commit message with Fix #15000 ...

Comment thread lib/checkcondition.cpp Resolved
Comment thread lib/checkcondition.cpp Resolved
@danmar

danmar commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

The PR title should be updated. The commit message will not be shown in the git log but the PR title will.

@jfdeverge jfdeverge changed the title Fix Rule 14.3 detection for bounded expressions Fix #15000 CheckCondition: detect compareValueOutOfTypeRange for bounded cast/arithmetic expressions Sep 6, 2026
@jfdeverge

Copy link
Copy Markdown
Contributor Author

The PR title should be updated. The commit message will not be shown in the git log but the PR title will.

Done.

PR title updated to "Fix #15000 CheckCondition: detect compareValueOutOfTypeRange for bounded cast/arithmetic expressions" ; let me know if better convention would apply here.

Comment thread lib/checkcondition.cpp Resolved
Comment thread lib/checkcondition.cpp Resolved
Comment thread lib/checkcondition.cpp Resolved
Comment thread test/testcondition.cpp Resolved
@danmar danmar changed the title Fix #15000 CheckCondition: detect compareValueOutOfTypeRange for bounded cast/arithmetic expressions Fix #15000 (CheckCondition: detect compareValueOutOfTypeRange for bounded cast/arithmetic expressions) Sep 6, 2026
@danmar

danmar commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

The PR title should be updated. The commit message will not be shown in the git log but the PR title will.

Done.

PR title updated to "Fix #15000 CheckCondition: detect compareValueOutOfTypeRange for bounded cast/arithmetic expressions" ; let me know if better convention would apply here.

Thanks! I don't have a strict pattern that must be followed but I typically put parentheses around the ticket title. Most important is that you indicated which ticket it solves..

Comment thread lib/checkcondition.cpp Resolved
Comment thread lib/checkcondition.cpp Resolved
Comment thread lib/checkcondition.cpp Resolved
Comment thread lib/checkcondition.cpp Resolved
@chrchr-github

Copy link
Copy Markdown
Collaborator

I have a feeling that some of the logic should be performed in ValueFlow and not in the check. Currently, ValueFlow only seems to handle bool value ranges. @danmar What do you think?

@danmar

danmar commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

I have a feeling that some of the logic should be performed in ValueFlow and not in the check. Currently, ValueFlow only seems to handle bool value ranges. @danmar What do you think?

I agree that checkers should have some nice way to determine min/max values for an expression.

If that is put in ValueFlow or a utility function I don't have a strong opinion it depends on what is more efficient.

@chrchr-github

Copy link
Copy Markdown
Collaborator

I have a feeling that some of the logic should be performed in ValueFlow and not in the check. Currently, ValueFlow only seems to handle bool value ranges. @danmar What do you think?

I agree that checkers should have some nice way to determine min/max values for an expression.

If that is put in ValueFlow or a utility function I don't have a strong opinion it depends on what is more efficient.

I meant that ValueFlow should determine that the condition is always true/false, instead of having complex parsing logic (in this case, for casts and arithmetic) in the checker.

@jfdeverge jfdeverge changed the title Fix #15000 (CheckCondition: detect compareValueOutOfTypeRange for bounded cast/arithmetic expressions) Fix #15000 Propagate integral cast ranges to condition analysis Sep 12, 2026
Comment thread lib/vf_common.cpp Resolved
Comment thread test/testcondition.cpp Resolved
@jfdeverge
jfdeverge force-pushed the dev/fix_misra_14_3 branch 2 times, most recently from 1f38959 to 1ebcbe5 Compare September 14, 2026 07:13
@jfdeverge

Copy link
Copy Markdown
Contributor Author

I rebased against main (no conflicts).

Comment thread lib/valueflow.cpp Resolved
@danmar

danmar commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

I doubt the CI-cygwin failure was caused by you, I restarted the pipeline..

@dmcppcheck

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved.

@jfdeverge
jfdeverge force-pushed the dev/fix_misra_14_3 branch 4 times, most recently from 8a8e98c to 76f6fb3 Compare October 28, 2026 11:08

@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

One new problem: the classic signed-char bug c == 0xFF is no longer reported on any platform. Base gives compareValueOutOfTypeRangeError for char c; if (c == 0xFF), c == 200, c > 127 and *s == 0xEF on unix64, win64 and native. The PR reports none of these.

The rewritten check treats plain char as having unknown signedness even when the platform defines it (defaultSign == 's' on every built-in platform except unspecified). It then uses the union of the signed and unsigned ranges. An earlier review suggested that union, but only for the case where the sign is really unknown. The PR also changes the expected result of the existing regression test for #11592 (char c; if (c == 128) on unix64) from the warning to "". That drops a result someone deliberately added, and it is inconsistent with the PR's own value-flow code, which does use platform.defaultSign for plain char conversions.

Apart from this, I found no new problems. Everything else I tested matched base or improved on it, and the problems I saw are already in comments.md.

Testing

I built cppcheck and testrunner in pr/, and cppcheck in base/.

Test suites: TestCondition, TestValueFlow, TestBufferOverrun, TestType, TestStl, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase all pass with 0 failures.

Regression: plain char compared with a value that only fits an unsigned char

ch.c:

void use(int);
void e1(char c) { if (c == 0xFF) use(1); }
void e2(char c) { if (c == 200) use(2); }
void e3(const char *s) { if (*s == 0xEF) use(3); }
void e4(char c) { if (c > 127) use(4); }

Command: cppcheck -q --enable=style,warning --platform=unix64 --template='{line}:{id}:{message}' ch.c

Base (the same with --platform=win64 and --platform=native):

2:compareValueOutOfTypeRangeError:Comparing expression of type 'char' against value 255. Condition is always false.
3:compareValueOutOfTypeRangeError:Comparing expression of type 'char' against value 200. Condition is always false.
4:compareValueOutOfTypeRangeError:Comparing expression of type 'const char' against value 239. Condition is always false.
5:compareValueOutOfTypeRangeError:Comparing expression of type 'char' against value 127. Condition is always false.

PR: no output on unix64, win64 or native.

Other checks (same as base, or better)

  • Out-of-range comparisons: I compared about 25 cases of compareValueOutOfTypeRange across signed, unsigned, bool and 64-bit types. Base reported some of them wrongly, and the PR fixes those:
    • signed char c == 0xFFFFFFFFu is no longer called always false.
    • i <= 0xFFFFFFFFu is now reported as always true.
    • int64_t x > 0x7FFF...LL is now reported as always false.
    • u > -1 now gets a correct message.
  • Casts and arithmetic: I tried about 25 cast and arithmetic cases and found no new false positives. The PR also removes base's wrong t = x + 1u; t == 0 "always false".
  • Float bounds: I tried about 18 cases (>=/<= conditions, sqrt/log/acos/1.0/x, float→int casts after a clamp). The only new result is if (x >= 2.0) {} acos(x) → invalidFunctionArg, which is correct.
  • for loops: I tried &&/|| loop conditions and negative init values. The results match base, except for the unsigned-index case that comments.md already reports.
  • Crash check: cppcheck -j8 on samples/ and test/cfg/std.c gave no internal errors or crashes.

Automated review of 76f6fb3 by Claude. The pull request was built and tested locally.

Comment thread lib/checkcondition.cpp Resolved
Comment thread test/testcondition.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

I found no new problems that change any warning. Many of my tests reproduced issues that are already in comments.md, or cases that the PR fixes. That includes base's wrong "always false" results for (uint64_t)x + 1 == 0, unsigned y = x - 6; y == 4294967295u and x - y > 10.

I found one small new detail. The bitfield-width leak that was reported for getCommonIntegralType still happens on the ++/-- path, which copies the operand's whole ValueType. For s.b++ on an unsigned b : 3 bitfield, the dump now has valueType-bits="3"; base has 0. I found no checker result that this changes. It only shows in dump files that addons read.

Testing

Build: cppcheck and testrunner in pr/, and cppcheck in base/. Both built without errors.

Unit tests: TestCondition, TestValueFlow, TestBufferOverrun, TestType, TestStl, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase all give Tests failed: 0. In test/cli/other_test.py, -k "15000 or missing_include or cast or floating or bounded or 6366" gives 7 passed.

Base vs PR comparisons

All runs use cppcheck -q --enable=style,warning --platform=unix64 unless noted.

  • #15000 reproducer and variations: these are now reported as always false, and they are correct:
    • (int64_t)(uint32_t)x - 1LL > 4294967295LL
    • (int)(int16_t)x - 1 < -32769
    • (int)(uint16_t)x + 1 > 65536
    • (uint16_t)x + 1 > 65536
    • (long long)(unsigned long)x > 4294967295LL on win64 and unix32. On unix64 it is correctly not reported.
  • Possible false positives I tried: about 25 C and C++ cases with casts and arithmetic, including static_cast, functional casts, enum class, wchar_t/char16_t, uintptr_t, uint64_t sign tricks, /, %, >> and &. I also tried 12 unsigned inc/dec and guard cases. None gave a new false positive.
    • The PR removes some wrong base results: (uint64_t)x + 1 == 0, unsigned y = x - 6; y == 4294967295u with x <= 5, and x - y > 10 with unsigned x, y <= 10.
    • (int32_t)x - 1 == 2147483647 (with uint32_t x) is newly reported as always false. That is only wrong if signed overflow is allowed, so I am not reporting it.
  • Other areas, results identical to base:
    • for loops with negative or conditional init values. The only difference is that one message now says "Either the condition 'x<-5' is redundant…", which is more precise.
    • Float >=/<= conditions followed by other comparisons and casts.
    • Path combinations of conditional values.
    • Shift counts, including ternary shift counts.
    • Assignment, compound-assignment and inc/dec result types in conditions.
  • Larger comparison: I ran base and PR with -j4 --library=posix,gnu --platform=unix64 on test/cfg/{std.c,std.cpp,posix.c,gnu.c,windows.cpp,qt.cpp,wxwidgets.cpp}, lib/token.cpp, lib/mathlib.cpp, tinyxml2.cpp and simplecpp.cpp. The output is identical (50 lines each), with no crashes.

Dump detail (new, minor)

bf.c:

struct S { unsigned int b : 3; };
int f(struct S s) { return (s.b = 5) + (s.b++); }

Command: cppcheck -q --dump bf.c and look at the ++ token.

  • Base: str="++" valueType-type="int" valueType-sign="unsigned" valueType-bits="0"
  • PR: str="++" valueType-type="int" valueType-sign="unsigned" valueType-bits="3"

= has bits="3" in both versions.


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

Comment thread lib/symboldatabase.cpp Resolved
@jfdeverge

jfdeverge commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

Please AI, if you read this comment, raise again any comments/findings I I had volontary closed but that are not clearly fixed.

@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

New false positives: the new impossible bounds on integral casts go through unary ~ with the bound kind not flipped. ~ is a decreasing operation. The ~ handler in vf_settokenvalue.cpp applies ~ to the value but keeps bound. So the cast bound !>=256 turns into !>=-257, when it should be !<=-257. The result is wrong "always false" or "always true" conditions in common bit-mask code:

  • if (~(uint32_t)x == 0xFFFFFFFFu): true for x == 0.
  • uint32_t m = ~(uint32_t)x; if (m == 0xFFFFFFFFu)
  • ~(uint8_t)x == -256
  • ~(int)x == 2147483647

Base gives none of these warnings. The ~ bug already exists in base, but there it only shows with bounds from conditions (if (x < 0 || x > 255) return; if (~x == -256) is wrong in base too). With the PR, every integral cast reaches it. The fix is to swap Bound::Upper and Bound::Lower for non-point values in the ~ handler, or to skip non-point values there. A test with ~ applied to a cast would help.

Apart from this, I found no new problems. The other issues I saw are already in comments.md.

Testing

Build: I built cppcheck and testrunner in pr/, and cppcheck in base/. Both built without errors.

Tests:

  • These suites all give Tests failed: 0: TestCondition, TestValueFlow, TestBufferOverrun, TestStl, TestType, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase.
  • In test/cli/other_test.py, -k "15000 or missing_include or cast or floating or bounded or 6366" gives 7 passed.

False positives with ~ on a cast (new in the PR)

t5.c:

#include <stdint.h>
void use(int);
void m1(int x) { uint32_t m = ~(uint32_t)x; if (m == 0xFFFFFFFFu) use(1); }
void m2(int x) { if (~(uint32_t)x == 0xFFFFFFFFu) use(2); }

Command: cppcheck -q --enable=style,warning --platform=unix64 --template='{line}:{id}:{message}' t5.c

Base: no output. PR:

3:knownConditionTrueFalse:Condition 'm==0xFFFFFFFFu' is always false
4:knownConditionTrueFalse:Condition '~(uint32_t)x==0xFFFFFFFFu' is always false

Both conditions are true for x == 0.

t3.c / t6.cpp (same command):

void c4(int x) { if (~(uint8_t)x == -256) use(22); }        // true for x==255
void c8(int x) { if (~(uint8_t)x < -255) use(26); }         // false for x==0
void c7(int x) { if (~(int8_t)x == 127) use(25); }          // true for x==-128
void n4(int x) { if (~static_cast<uint32_t>(x) == 0xFFFFFFFFu) use(4); }
void n6(int x) { if ((int)~(uint8_t)x == -1) use(6); }      // true for x==0
void n7(long long x) { if (~(int)x == 2147483647) use(7); } // true for (int)x==INT_MIN
  • The PR reports each of these as always false. It reports c8 as always true.
  • Base reports none of them.

--debug for c4:

~ always !>=-257
( always {!<=-1,!>=256}

The cast's !>=256 is mapped to !>=-257, which says ~(uint8_t)x < -257 always. The correct result is !<=-257.

Same bug in base without casts: void d1(int x) { if (x < 0 || x > 255) return; if (~x == -256) use(1); } is reported as always false by both base and PR. So the PR does not create the bug, but it makes it much easier to reach.

Other checks:

  • Cast and arithmetic cases gave no other new false positives. I tried %, /, &, |, <<, 100 - (uint8_t)x, -(uint8_t)x and -(int8_t)x. One of them, (int)(uint8_t)x - 300 > 0, is now correctly reported as always false.

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

Comment thread lib/valueflow.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

I found one new false positive: a value from an integral cast, decremented in place (y -= 1, y--), is reported as never wrapping. The PR adds the impossible bound !>=256 to (uint8_t)x. That bound reaches y through unsigned y = (uint8_t)x;. The forward analyzer (evalAssignment in vf_analyzers.cpp) then shifts the bound for -=/-- without the no-wrap check that the PR added for binary +/-/* in setTokenValue. So y -= 1; if (y > 255) is reported as always false, but it is true for x == 0. The plain form y = y - 1 is handled correctly.

The same analyzer bug already exists in base for bounds that come from conditions (if (y > 255) return; y -= 1; if (y > 255) is wrong in base too). This is the same situation as the earlier ~ finding: with the PR, every integral cast reaches the bug.

The tests I ran pass. Apart from this, the cast and arithmetic cases I tried gave no new false positives. The problems I saw that are already in comments.md (plain char, unsigned loop index, && loops) I did not report again.

Testing

Build: I built cppcheck and testrunner in pr/, and cppcheck in base/. Both built without errors.

Tests:

  • These suites all give Tests failed: 0: TestCondition, TestValueFlow, TestBufferOverrun, TestStl, TestType, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase.
  • In test/cli/other_test.py, -k "15000 or missing_include or cast or floating or bounded or 6366" gives 7 passed.

New false positive: compound decrement after a cast

c.c:

#include <stdint.h>
void use(int);
void g4(int x) { uint32_t y = (uint8_t)x; y -= 1; if (y > 255) use(1); }
void g5(int x) { unsigned y = (uint8_t)x; y -= 1; if (y > 255) use(2); }
void g6(int x) { unsigned y = (uint8_t)x; y = y - 1; if (y > 255) use(3); }
void g7(int x) { unsigned y = (uint8_t)x; y--; if (y > 255) use(4); }
void g8(int x) { unsigned y = (uint8_t)x; if (y - 1 > 255) use(5); }
void g9(unsigned char c) { unsigned y = c; y -= 1; if (y > 255) use(6); }

Command: cppcheck -q --enable=style,warning --platform=unix64 --template='{line}:{id}:{message}' c.c

  • Base: no output.
  • PR:
3:knownConditionTrueFalse:Condition 'y>255' is always false
4:knownConditionTrueFalse:Condition 'y>255' is always false
6:knownConditionTrueFalse:Condition 'y>255' is always false

All three conditions are true for x == 0. y = y - 1 (line 5) and y - 1 > 255 (line 7) are handled correctly.

--debug on the g5 line shows that the cast bound is shifted with no wrap check:

( always {!<=-1,!>=256}
y {symbolic=((uint8_t)x-1),!<=-1,!>=255}

A realistic case: size_t r = (uint16_t)n; r -= 1; if (r >= 65535) return 0;. The PR reports r>=65535 as always false; base does not.

The same analyzer bug is already in base for bounds from conditions, so this is not new code: void k1(unsigned y) { if (y > 255) return; y -= 1; if (y > 255) use(1); } is reported as always false by both base and PR.

Other cases that matched base or improved on it:

  • Arithmetic on casts, on unix64, win64 and unix32: *// by negative constants, 300 - (uint8_t)x, >>, <<, |, &, %, (uint8_t)x ± (uint8_t)y, and -(int8_t)x.
  • Other targets: enum casts with an underlying type, size_t, uintptr_t and unsigned long casts, (unsigned)(p - q), and assignments to int followed by +=/-=.
  • The new results I got were correct, for example 300-(uint8_t)x<45 and int y=(uint8_t)x; y-=256; y>=0 are always false. I found no other false positives.

Automated review of 04fb4da by Claude. The pull request was built and tested locally.

Comment thread lib/valueflow.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

One new false positive: when a value comes from a cast to a narrow signed type and is then incremented or decremented in place (i++, i += 1, i--), it is reported as never wrapping. The PR puts the bounds !<=-129,!>=128 on (signed char)x. These bounds reach i through signed char i = (signed char)x;. After i++, the bound is shifted to !<=-127. That happens because the new guard at lib/vf_analyzers.cpp:286 only stops this for UNSIGNED types. But char and short are promoted to int and then converted back, so 127 + 1 becomes -128 in practice (implementation-defined in C, defined modulo 2^N since C++20). The result is new "always false" warnings for common wrap checks like if (i == -128) and if (s == -32768). Base gives none of them.

The unsigned version of this problem (y -= 1 and y--) was raised before and is now fixed. This signed narrow case is new. Apart from it I found no new problems. The other results I saw match base or improve on it, or are already in comments.md.

Testing

Build: I built cppcheck and testrunner in pr/, and cppcheck in base/. Both built without errors.

Unit tests: These suites all give Tests failed: 0: TestCondition, TestValueFlow, TestBufferOverrun, TestType, TestStl, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase.

New false positive: in-place inc/dec on a narrow signed type after a cast

d.c:

void use(int);
void d1(int x) { signed char i = (signed char)x; i++; if (i == -128) use(1); }
void d2(int x) { short i = (short)x; i++; if (i == -32768) use(2); }
void d3(int x) { short i = (short)x; i += 1; if (i == -32768) use(3); }
void d4(int x) { short i = (short)x; i--; if (i == 32767) use(4); }
void d5(signed char i) { i++; if (i == -128) use(5); }
void d6(int x) { signed char i = (signed char)x; i = i + 1; if (i == -128) use(6); }

Command: cppcheck -q --enable=style,warning --platform=unix64 --template='{line}:{id}:{message}' d.c

Base: no output for these lines. PR:

2:knownConditionTrueFalse:Condition 'i==-128' is always false
3:knownConditionTrueFalse:Condition 'i==-32768' is always false
4:knownConditionTrueFalse:Condition 'i==-32768' is always false
5:knownConditionTrueFalse:Condition 'i==32767' is always false

Each condition is true for some x, for example x == 127, x == 32767 or x == -32768. i = i + 1 (d6) is handled correctly. So is a plain parameter (d5).

--debug for d1 shows the bound being shifted with no wrap check:

( always {!<=-129,!>=128}
++ {!<=-129,!>=128,symbolic=((char)x),127}
i {symbolic=((char)x+1),!<=-127}

The same happens with int8_t (int8_t i = (int8_t)x; i++; if (i == -128)).

Other checks (same as base, or better):

  • I tried about 25 cast and arithmetic cases, all with no false positives. They covered multiplying or dividing by negative constants, 0 - (int8_t)x, ^, |, %, >>, <<, !, mixed signed/unsigned cast operands, and *= / -= / unary - on int variables that hold a cast value.
  • I tried about 20 C++ cases: static_cast to an enum with an underlying type, enum class, bool, char16_t, wchar_t, functional casts and chained casts. They ran on unix64 and win64 with no new false positives.
  • The PR removes base false positives such as uint8_t i = (uint8_t)x; i++; if (i == 0), uint16_t i += 1, unsigned char c--; c == 255, int r = (uint8_t)x; r *= -1; r == -255, and (bool)x + 1 == 2.
  • Larger files: tinyxml2.cpp, simplecpp.cpp and test/cfg/std.c took the same time with base and the PR (21/22 s, 526/530 s, 5/5 s with the debug build). The warnings were identical. The only difference was one extra "Assuming that condition … is not redundant" note in tinyxml2, a change in error paths that comments.md already mentions.

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

Comment thread lib/vf_analyzers.cpp Outdated
// Only if its invertible
if (value->isImpossible() && !Token::Match(parent, "+=|-=|*=|++|--"))
return Action::None;
// Shifting an impossible range bound is unsound if the unsigned operation can wrap

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

This guard only covers UNSIGNED types. Narrow signed types (signed char/int8_t, short) wrap in the same way when they are incremented in place: they are promoted to int and converted back, which is implementation-defined in C and modulo 2^N since C++20. Together with the new cast bounds, this gives new "always false" warnings that base does not give:

void d1(int x) { signed char i = (signed char)x; i++; if (i == -128) use(1); }
void d2(int x) { short i = (short)x; i++;    if (i == -32768) use(2); }
void d3(int x) { short i = (short)x; i += 1; if (i == -32768) use(3); }
void d4(int x) { short i = (short)x; i--;    if (i == 32767) use(4); }

--debug for d1 shows i {symbolic=((char)x+1),!<=-127} after i++, but i is -128 for x == 127. i = i + 1 is handled correctly. The guard could also skip signed types whose rank is lower than int, which are the types that ValueType::isIntegral() reports as CHAR/SHORT. A test with short i = (short)x; i++; would help.

@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

New regression: an explicit cast to plain char now drops every integer value, even values that fit in any char. (char)0, (char)65 and static_cast<char>(x) with a known x get no value in the PR, on unix64, win64 and native. As a result, warnings that base gives are lost:

  • zerodiv on 10 / (char)0
  • zerodivcond
  • arrayIndexOutOfBounds / arrayIndexOutOfBoundsCond on a[(char)i]
  • knownConditionTrueFalse on (char)x == 'A'

The cause is that the new castValue() returns false for UNKNOWN_SIGN, and plain char always has that sign. setTokenValueCast passes valueType.sign to castValue() directly. The assignment path (truncateValues in valueflow.cpp) already resolves plain char through platform.defaultSign, but the explicit-cast path does not.

Earlier reviews about plain char covered compareValueOutOfTypeRange and the assignment path, not cast values, so this is new. Apart from it I found no new problems. The other cases I tried matched base, improved on it, or are already in comments.md.

Testing

Build: cppcheck and testrunner were already up to date in pr/, and cppcheck in base/. Both make runs finished without errors.

Unit tests: TestCondition, TestValueFlow, TestBufferOverrun, TestType, TestStl, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase all give Tests failed: 0. I did not run the Python CLI tests again; earlier reviews ran them.

Regression: values lost on casts to plain char (new)

ch2.c:

void use(int);
void j7(void) { int x = 65; if ((char)x == 'A') use(12); }
void j8(void) { int x = 0; char c = (char)x; if (c == 0) use(13); }
void j9(void) { char c = (char)0; if (c != 0) use(14); }
void j10(void) { int x = 300; if ((char)x == 44) use(15); }

Command: cppcheck -q --enable=style,warning --platform=unix64 --template='{line}:{id}:{message}' ch2.c (win64 and native give the same results)

Base:

2:knownConditionTrueFalse:Condition '(char)x=='A'' is always true
3:knownConditionTrueFalse:Condition 'c==0' is always true
4:knownConditionTrueFalse:Condition 'c!=0' is always false
5:knownConditionTrueFalse:Condition '(char)x==44' is always true

PR: only line 3.

ch3.c / ch4.cpp:

int z1(int x) { if (x == 0) {} return 10 / (char)x; }
int z2(void) { return 10 / (char)0; }
char a[10];
void z3(void) { int i = 20; a[(char)i] = 0; }
void z4(int x) { if (x == 10) {} a[(char)x] = 0; }
void s1() { int x = 65; if (static_cast<char>(x) == 'A') use(1); }

Base:

ch3.c:5:arrayIndexOutOfBounds:Array 'a[10]' accessed at index 20, which is out of bounds.
ch3.c:6:arrayIndexOutOfBoundsCond:Either the condition 'x==10' is redundant or the array 'a[10]' is accessed at index 10, which is out of bounds.
ch3.c:2:zerodivcond:Either the condition 'x==0' is redundant or there is division by zero at line 2.
ch3.c:3:zerodiv:Division by zero.
ch4.cpp:2:knownConditionTrueFalse:Condition 'static_cast<char>(x)=='A'' is always true

PR: no output.

--debug for j7: base has ( always 65 on the cast. The PR has no values on the cast.

Other checks (same as base, or better)

  • About 25 cast and arithmetic cases matched base or improved on it:
    • -(uint32_t)x, << on uint32_t casts, *// on unsigned casts, ^ -1, % on int8_t.
    • In-place <<=, *=, = -m, = ~m, += on cast values.
    • unsigned m = (uint8_t)x; m = ~m; if (m > 255u) is newly reported as always true, which is correct. The PR also removes base's wrong c += 200; c < 10 and u += 0xFFFFFF00u; u < 10 results.
  • Known-value casts to uint8_t, int8_t, uint16_t, _Bool, uint32_t and unsigned long long behave correctly. (_Bool)256 == 1 is newly reported, which is correct.
  • for loops with conditional or cast init values gave the same results as base. The only difference is a more precise "Either the condition…" message.
  • Synthetic stress files (300 chained conditions, 200 conditional loop updates) took the same time with base and the PR.

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

Comment thread lib/vf_settokenvalue.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

New false positive: on Windows platforms, compareValueOutOfTypeRangeError now treats wchar_t as a signed 16-bit type. On win64, win32A and win32W the PR reports BOM and character checks such as *s == 0xFEFF, w == 0xFFFF and w > 32767 as "always false", with the message "Comparing expression of type 'signed wchar_t'". On Windows, wchar_t is an unsigned 16-bit type, so all of these conditions can be true. Base gives no warning for them.

The cause is in the rewritten check. Base only handled bool, char, short, int, long and long long. The PR now derives the range of any integral type from getSizeOf() and ValueType::sign. For wchar_t, the symbol database falls back to SIGNED on every platform. The new test only covers unix64, where wchar_t really is signed 32-bit.

Apart from this I found no new problems. The other cases I tried either match base, are improvements, or are already raised in comments.md.

Testing

Build: I built cppcheck and testrunner in pr/, and cppcheck in base/. Both built without errors.

Unit tests: TestCondition, TestValueFlow, TestBufferOverrun, TestType, TestStl, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase all give Tests failed: 0. I did not rerun the Python CLI tests; earlier reviews ran them.

New false positive: wchar_t on Windows platforms

w.c (w.cpp is the same file and gives the same results):

#include <wchar.h>
void use(int);
void w1(long long x) { if ((wchar_t)x > 40000) use(1); }
void w2(wchar_t w) { if (w == 0xFFFF) use(2); }
void w3(wchar_t w) { if (w > 32767) use(3); }
void w4(const wchar_t *s) { if (*s == 0xFEFF) use(4); }

Command: cppcheck -q --enable=style,warning --platform=win64 --template='{file}:{line}:{id}:{message}' w.c w.cpp

  • Base: no output.
  • PR, the same with --platform=win32W:
w.c:3:compareValueOutOfTypeRangeError:Comparing expression of type 'signed wchar_t' against value 40000. Condition is always false.
w.c:4:compareValueOutOfTypeRangeError:Comparing expression of type 'signed wchar_t' against value 65535. Condition is always false.
w.c:5:compareValueOutOfTypeRangeError:Comparing expression of type 'signed wchar_t' against value 32767. Condition is always false.
w.c:6:compareValueOutOfTypeRangeError:Comparing expression of type 'const signed wchar_t' against value 65279. Condition is always false.
(and the same four for w.cpp)

With --platform=unix64 the PR gives no output, which is correct.

A version where the condition is plainly true, void v1(void) { int x = 0xFFFF; if ((wchar_t)x == 0xFFFF) use(1); }, gives Comparing expression of type 'signed wchar_t' against value 65535. Condition is always false. with the PR on win64. Base gives nothing.

Other checks (same as base, or better):

  • long and pointer-sized casts: (long)x, (unsigned long)x, (size_t)x, (uintptr_t)x, (intptr_t)x and (ptrdiff_t)x compared with 32-bit limits on unix64, win64, unix32 and win32A. Some warnings changed ID from compareValueOutOfTypeRangeError to knownConditionTrueFalse; both are correct.
  • (int)(short)x + 70000 > 102767: newly reported as always false, which is correct.
  • Mixed signed/unsigned on cast values: ?: with -1, u > -1, i < 5u, i + 0u, i * 1u, (unsigned)i, %, &, >> and << on (int8_t)/(int16_t) values. I found no new false positives. long long l = (int8_t)x; l + 0u > 300 is newly reported as always false, which is correct.

Automated review of 4a5cec6 by Claude. The pull request was built and tested locally.

Comment thread lib/checkcondition.cpp Resolved
@dmcppcheck

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved.

@dmcppcheck

Copy link
Copy Markdown
Contributor

Test results for commit 5c412fe (tools/test-my-pr.py, main compared to this PR):

Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8819/
packages: 1000
Differing warnings: 3942 (+1894 -2048)
AI review: 18 improvement, 28 neutral, 4 regression (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8819/2026-10-31_16-13-22-5c412fe0be60-review.txt)
Timing: +0.1% (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8819/2026-10-31_16-13-22-5c412fe0be60_timing.html)

Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong.

@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

New false positive: comparing an enum with its own enumerator is reported as "always false" when the enumerator's value is 0x80000000 or higher.

Flag enums often have a high bit like this (F_HIGH = 0x80000000). With enum Flags { F_NONE = 0, F_HIGH = 0x80000000 }; if (f == F_HIGH), the PR reports compareValueOutOfTypeRangeError: Comparing expression of type 'signed int' against value 2147483648. Condition is always false. It does this in C and C++, on unix64 and win64. Base gives no warning.

The cause:

  • getEnumType() still gives the enum the type signed int; it never works out the sign.
  • The PR now gives the enumerator token the value 2147483648 with valueType signed int. Base gave it -2147483648.
  • The rewritten check trusts both. It treats a known value of a signed int expression as lying outside the signed int range, which is not possible.

Apart from this I found no new problems. The other cases I tried (shift counts in ternaries, float bounds, per-path inference through function arguments, result types of assignment, ++/-- and ternaries) either match base, improve on it, or are already in comments.md.

Testing

Build: make -j8 cppcheck testrunner in pr/ and make -j8 cppcheck in base/ both built without errors.

Unit tests: TestCondition, TestValueFlow, TestBufferOverrun, TestStl, TestType, TestOther, TestFunctions, TestUninitVar and TestSymbolDatabase all give Tests failed: 0. I did not rerun the Python CLI tests.

New false positive: enum compared with its own enumerator (e3.cpp)

void use(int);
enum Flags { F_NONE = 0, F_LOW = 1, F_HIGH = 0x80000000 };
void a(Flags f) { if (f == F_HIGH) use(1); }
void b(unsigned m) { Flags f = (Flags)m; if (f == F_HIGH) use(2); }

Command: cppcheck -q --enable=style,warning --platform=unix64 --template='{file}:{line}:{id}:{message}' e3.cpp. The same happens with --platform=win64 and with a C version that uses enum Flags f.

  • Base: no output.
  • PR:
e3.cpp:3:compareValueOutOfTypeRangeError:Comparing expression of type 'signed int' against value 2147483648. Condition is always false.
e3.cpp:4:compareValueOutOfTypeRangeError:Comparing expression of type 'signed int' against value 2147483648. Condition is always false.

enum G { G0 = 0, G1 = 0xFFFFFFFFu }; void f6(enum G g) { if (g == G1) use(6); } gives the same warning with value 4294967295.

With --debug on line 3, base shows F_HIGH always -2147483648 and the PR shows F_HIGH always 2147483648. In the dump, both versions give f and F_HIGH the type int/signed.

Other checks (same as base, or better):

  • Shift counts in ternaries: x ? 0 : (1 << s) with s = x ? 40 : 3, and variants. Same results as base.
  • Float bounds: after >=/<= conditions, with sqrt/acos/log/1.0/x and float-to-int casts. No new false positives. Three correct new floatConversionOverflow warnings (e.g. if (d >= 1e30) {} return (int)d;).
  • Path-split inference through function arguments:
    • f(1,2); f(5,4); f(20,3) with a[x - y + 5] now gives the correct index 22.
    • m(1,2) with a[x - y] gives the correct negativeIndex.
    • z(3,3) with 100 / (x - y) gives the correct zerodiv.
    • I found no false "always" conditions.
  • Casts and arithmetic: about 25 common cast idioms such as (uint8_t)(x >> 8) == 0xFF, (int8_t)(x & 0xFF) < 0 and (uint8_t)s[0] >= 0x80 give the same results as base.
  • Result types: assignment and ++/--, and ternaries with bool/char operands, give the same results as base.
  • Timing: generated stress files (300 chained conditions, 40 loops, 200 cast reassignments) took about the same time with base and the PR.

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

Comment thread lib/checkcondition.cpp
if (knownValue == 0)
continue; // prevent overlap with TestOther::unsignedPositive/unsignedLessThanZero

const ValueType commonType = ValueType::getCommonIntegralType(type, valueType, mSettings);

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

This trusts that the known value fits the value token's own type. That is not true for enumerators: getEnumType() still types an enum as signed int, but with this PR an enumerator such as F_HIGH = 0x80000000 gets the value 2147483648 (base gave it -2147483648). So comparing an enum with its own enumerator is reported as always false:

enum Flags { F_NONE = 0, F_HIGH = 0x80000000 };
void a(Flags f) { if (f == F_HIGH) use(1); }

With --platform=unix64 (and win64, and in C), the PR reports compareValueOutOfTypeRangeError: Comparing expression of type 'signed int' against value 2147483648. Condition is always false. enum G { G1 = 0xFFFFFFFFu }; if (g == G1) gives the same. Base gives no warning.

Two possible fixes:

  • Skip the comparison when the known value does not fit valueTok's own type, because then that type is not reliable.
  • Skip types that come from an enum without a fixed underlying type.

Please also add a test for a high-bit flag enum.

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.

6 participants