Skip to content

Fix #11592 FN compareValueOutOfTypeRangeError with char #8940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chrchr-github wants to merge 4 commits into
base: main
Choose a base branch
from
+12 −5
Open

Fix #11592 FN compareValueOutOfTypeRangeError with char #8940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc6a3b6
Update checkcondition.cpp
chrchr-github Oct 14, 2026
9ec3dfb
Update testcondition.cpp
chrchr-github Oct 14, 2026
ca3eda6
Update checkcondition.cpp
chrchr-github Oct 14, 2026
3a9288b
Update checkcondition.cpp
chrchr-github Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions lib/checkcondition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2009,14 +2009,15 @@ void CheckConditionImpl::checkCompareValueOutOfTypeRange()
for (int i = 0; i < 2; ++i) {
const Token * const valueTok = (i == 0) ? tok->astOperand1() : tok->astOperand2();
const Token * const typeTok = valueTok->astSibling();
if (!valueTok->hasKnownIntValue() || !typeTok->valueType() || typeTok->valueType()->pointer)
const ValueType* const typeVt = typeTok->valueType();
if (!valueTok->hasKnownIntValue() || !typeVt || typeVt->pointer)
continue;
if (valueTok->getKnownIntValue() < 0 && valueTok->valueType() && valueTok->valueType()->sign != ValueType::Sign::SIGNED)
continue;
if (typeTok->isLiteral())
continue;
std::uint8_t bits = 0;
switch (typeTok->valueType()->type) {
switch (typeVt->type) {
case ValueType::Type::BOOL:
bits = 1;
break;
Expand All @@ -2041,10 +2042,10 @@ void CheckConditionImpl::checkCompareValueOutOfTypeRange()
if (bits == 0 || bits >= 63)
continue;

const auto typeMinValue = (typeTok->valueType()->sign == ValueType::Sign::UNSIGNED) ? 0 : (-(1LL << (bits-1)));
const auto typeMinValue = (typeVt->sign == ValueType::Sign::UNSIGNED) ? 0 : (-(1LL << (bits-1)));
const auto unsignedTypeMaxValue = (1LL << bits) - 1LL;
long long typeMaxValue;
if (typeTok->valueType()->sign != ValueType::Sign::SIGNED)
if (typeVt->sign != ValueType::Sign::SIGNED && !(typeVt->sign == ValueType::Sign::UNKNOWN_SIGN && typeVt->type == ValueType::Type::CHAR && mSettings.platform.defaultSign == 's'))

@dmcppcheck dmcppcheck Oct 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI review

Plain char now gets the signed range, but the operand conversions are not considered. When valueTok is unsigned with a rank of at least int (127u, sizeof(...)), a negative c becomes a huge unsigned value, so c > 127u is true for c == -1. The PR still reports it as always false:

int main(){ char c=-1; return c > 127u; }   // exits with 1 at runtime
void f(char c){ if (c > sizeof(int) * 50) {} }

--platform=unix64 gives Comparing expression of type 'char' against value 127. Condition is always false. and ... against value 200 .... Base already has this false positive for signed char and short, but this change extends it to plain char, where c > sizeof(...) is common. == and != against an unsigned value are still correct. The relational operators should be skipped, or should allow for wraparound, when the value operand is unsigned with a rank of at least int.

@chrchr-github chrchr-github Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

See https://trac.cppcheck.net/ticket/15111

@dmcppcheck dmcppcheck Oct 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI review

Only lowercase 's' is checked here. SymbolDatabase (lib/symboldatabase.cpp:67) also accepts 'S', and a platform file with <default-sign>S</default-sign> does not get the new warning (I confirmed this with c == 128).

@dmcppcheck dmcppcheck Oct 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI review

Plain char now gets the signed range, but ValueFlow does not sign-extend a value cast to plain char. ValueFlow::truncateIntValue (lib/vf_common.cpp:107) sign-extends only when the sign is SIGNED, so (char)200 becomes the known value 200 instead of -56. Comparisons like these are now reported as always false, but they can be true:

#define MARK ((char)0xE2)
int f(char c) {
    if (c == (char)200) return 1;              // "against value 200. Condition is always false"
    if (c == static_cast<char>(0x80)) return 2; // "against value 128 ..."
    if (c == MARK) return 3;                   // "against value 226 ..."
    return 0;
}

f((char)200) returns 1 when compiled with g++ on x86_64. Base does not warn here. One fix is to sign-extend UNKNOWN_SIGN char in the cast truncation when defaultSign is signed. Another is to skip this new case when valueTok is itself of type char.

typeMaxValue = unsignedTypeMaxValue;
else if (bits >= mSettings.platform.int_bit && (!valueTok->valueType() || valueTok->valueType()->sign != ValueType::Sign::SIGNED))
typeMaxValue = unsignedTypeMaxValue;
Expand Down Expand Up @@ -2105,7 +2106,7 @@ void CheckConditionImpl::checkCompareValueOutOfTypeRange()
}
if (!error || diag(tok))
continue;
compareValueOutOfTypeRangeError(valueTok, typeTok->valueType()->str(), kiv, result);
compareValueOutOfTypeRangeError(valueTok, typeVt->str(), kiv, result);
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions test/testcondition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6652,6 +6652,12 @@ class TestCondition : public TestFixture {
"[test.cpp:2:28]: (style) Comparing expression of type 'signed int' against value 2147483647. Condition is always false. [compareValueOutOfTypeRangeError]\n"
"[test.cpp:7:17]: (style) Comparing expression of type 'signed int' against value 2147483647. Condition is always false. [compareValueOutOfTypeRangeError]\n",
errout_str());

check("void f(char c) {\n" // #11592
" if (c == 128) {}\n"
"}\n", settingsUnix64);
ASSERT_EQUALS("[test.cpp:2:14]: (style) Comparing expression of type 'char' against value 128. Condition is always false. [compareValueOutOfTypeRangeError]\n",
errout_str());
}

void knownConditionCast() {
Expand Down
Loading