Skip to content

Commit 2bf9afa

Browse files
chrchr-githubweb-flow
authored andcommitted
Partial fix for #9078 FN compareValueOutOfTypeRangeError (x <= UINT32_MAX) (cppcheck-opensource#8379)
1 parent 951e179 commit 2bf9afa

5 files changed

Lines changed: 42 additions & 13 deletions

File tree

lib/checkcondition.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,8 +1975,6 @@ void CheckCondition::checkCompareValueOutOfTypeRange()
19751975
continue;
19761976
if (valueTok->getKnownIntValue() < 0 && valueTok->valueType() && valueTok->valueType()->sign != ValueType::Sign::SIGNED)
19771977
continue;
1978-
if (valueTok->valueType() && valueTok->valueType()->isTypeEqual(typeTok->valueType()))
1979-
continue;
19801978
std::uint8_t bits = 0;
19811979
switch (typeTok->valueType()->type) {
19821980
case ValueType::Type::BOOL:
@@ -2015,6 +2013,8 @@ void CheckCondition::checkCompareValueOutOfTypeRange()
20152013

20162014
bool result{};
20172015
const auto kiv = valueTok->getKnownIntValue();
2016+
if (kiv == 0)
2017+
continue; // prevent overlap with TestOther::unsignedPositive/unsignedLessThanZero
20182018
if (tok->str() == "==")
20192019
result = false;
20202020
else if (tok->str() == "!=")

lib/vf_settokenvalue.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ namespace ValueFlow
7272
return value;
7373
if (!parent->isBinaryOp())
7474
return value;
75-
if (!parent->isConstOp())
75+
if (!parent->isConstOp() && !parent->isAssignmentOp())
7676
return value;
7777
if (!astIsIntegral(parent->astOperand1(), false))
7878
return value;
@@ -88,7 +88,7 @@ namespace ValueFlow
8888
ValueType::Sign sign = ValueType::Sign::UNSIGNED;
8989
if (n1 < n2)
9090
sign = vt2->sign;
91-
else if (n1 > n2)
91+
else // (n1 >= n2)
9292
sign = vt1->sign;
9393
Value v = castValue(value, sign, std::max(n1, n2) * 8);
9494
v.wideintvalue = value.intvalue;

test/testcondition.cpp

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6324,28 +6324,24 @@ class TestCondition : public TestFixture {
63246324

63256325
check("void f(const unsigned char u) {\n"
63266326
" if (u > 0) {}\n"
6327-
" if (u < 0) {}\n" // warn
6328-
" if (u >= 0) {}\n" // warn
6327+
" if (u < 0) {}\n"
6328+
" if (u >= 0) {}\n"
63296329
" if (u <= 0) {}\n"
63306330
" if (u > 255) {}\n" // warn
63316331
" if (u < 255) {}\n"
63326332
" if (u >= 255) {}\n"
63336333
" if (u <= 255) {}\n" // warn
63346334
" if (0 < u) {}\n"
6335-
" if (0 > u) {}\n" // warn
6336-
" if (0 <= u) {}\n" // warn
6335+
" if (0 > u) {}\n"
6336+
" if (0 <= u) {}\n"
63376337
" if (0 >= u) {}\n"
63386338
" if (255 < u) {}\n" // warn
63396339
" if (255 > u) {}\n"
63406340
" if (255 <= u) {}\n"
63416341
" if (255 >= u) {}\n" // warn
63426342
"}\n", settingsUnix64);
6343-
ASSERT_EQUALS("[test.cpp:3:14]: (style) Comparing expression of type 'const unsigned char' against value 0. Condition is always false. [compareValueOutOfTypeRangeError]\n"
6344-
"[test.cpp:4:14]: (style) Comparing expression of type 'const unsigned char' against value 0. Condition is always true. [compareValueOutOfTypeRangeError]\n"
6345-
"[test.cpp:6:14]: (style) Comparing expression of type 'const unsigned char' against value 255. Condition is always false. [compareValueOutOfTypeRangeError]\n"
6343+
ASSERT_EQUALS("[test.cpp:6:14]: (style) Comparing expression of type 'const unsigned char' against value 255. Condition is always false. [compareValueOutOfTypeRangeError]\n"
63466344
"[test.cpp:9:14]: (style) Comparing expression of type 'const unsigned char' against value 255. Condition is always true. [compareValueOutOfTypeRangeError]\n"
6347-
"[test.cpp:11:9]: (style) Comparing expression of type 'const unsigned char' against value 0. Condition is always false. [compareValueOutOfTypeRangeError]\n"
6348-
"[test.cpp:12:9]: (style) Comparing expression of type 'const unsigned char' against value 0. Condition is always true. [compareValueOutOfTypeRangeError]\n"
63496345
"[test.cpp:14:9]: (style) Comparing expression of type 'const unsigned char' against value 255. Condition is always false. [compareValueOutOfTypeRangeError]\n"
63506346
"[test.cpp:17:9]: (style) Comparing expression of type 'const unsigned char' against value 255. Condition is always true. [compareValueOutOfTypeRangeError]\n",
63516347
errout_str());
@@ -6355,6 +6351,15 @@ class TestCondition : public TestFixture {
63556351
"}\n", settingsUnix64);
63566352
ASSERT_EQUALS("[test.cpp:2:14]: (style) Comparing expression of type 'bool' against value 2. Condition is always true. [compareValueOutOfTypeRangeError]\n",
63576353
errout_str());
6354+
6355+
check("void f(const std::uint32_t& u) {\n" // #9078
6356+
" if (u >= UINT32_MAX) {}\n"
6357+
" if (u <= UINT32_MAX) {}\n"
6358+
" if (u > UINT32_MAX) {}\n"
6359+
"}\n");
6360+
ASSERT_EQUALS("[test.cpp:3:14]: (style) Comparing expression of type 'const unsigned int &' against value 4294967295. Condition is always true. [compareValueOutOfTypeRangeError]\n"
6361+
"[test.cpp:4:13]: (style) Comparing expression of type 'const unsigned int &' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
6362+
errout_str());
63586363
}
63596364

63606365
void knownConditionCast() {

test/testother.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9313,6 +9313,24 @@ class TestOther : public TestFixture {
93139313
" for (unsigned p = 0; p < (sizeof(a) / sizeof((a)[0])); ++p) {}\n"
93149314
"}");
93159315
ASSERT_EQUALS("", errout_str());
9316+
9317+
check("void f(const unsigned char u) {\n"
9318+
" if (u > 0) {}\n"
9319+
" if (u < 0) {}\n"
9320+
" if (u >= 0) {}\n"
9321+
" if (u <= 0) {}\n"
9322+
" if (0 < u) {}\n"
9323+
" if (0 > u) {}\n"
9324+
" if (0 <= u) {}\n"
9325+
" if (0 >= u) {}\n"
9326+
"}");
9327+
ASSERT_EQUALS("[test.cpp:3:11]: (style) Checking if unsigned expression 'u' is less than zero. [unsignedLessThanZero]\n"
9328+
"[test.cpp:4:11]: (style) Unsigned expression 'u' can't be negative so it is unnecessary to test it. [unsignedPositive]\n"
9329+
"[test.cpp:5:11]: (style) Checking if unsigned expression 'u' is less than zero. [unsignedLessThanZero]\n"
9330+
"[test.cpp:7:11]: (style) Checking if unsigned expression 'u' is less than zero. [unsignedLessThanZero]\n"
9331+
"[test.cpp:8:11]: (style) Unsigned expression 'u' can't be negative so it is unnecessary to test it. [unsignedPositive]\n"
9332+
"[test.cpp:9:11]: (style) Checking if unsigned expression 'u' is less than zero. [unsignedLessThanZero]\n",
9333+
errout_str());
93169334
}
93179335

93189336
void checkSignOfPointer() {

test/testvalueflow.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3112,6 +3112,12 @@ class TestValueFlow : public TestFixture {
31123112
" return x;\n"
31133113
"}\n";
31143114
ASSERT_EQUALS(false, testValueOfXKnown(code, 9U, 1));
3115+
3116+
code = "int32_t f() {\n"
3117+
" const int32_t x = 0xffffffff;\n"
3118+
" return x;\n"
3119+
"}\n";
3120+
ASSERT_EQUALS(true, testValueOfXKnown(code, 3U, -1));
31153121
}
31163122

31173123
void valueFlowAfterSwap()

0 commit comments

Comments
 (0)