walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fixed #7567 ("(a | 7) > 6U" is always true)

This commit is contained in:
Bartlomiej Grzeskowiak 2016-07-31 22:09:47 +02:00 committed by Daniel Marjamäki
parent f337838917
commit 09a83f2cc8
2 changed files with 107 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
lib/checkcondition.cpp
View File

@ -281,36 +281,55 @@ void CheckCondition::comparison()
return;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "==|!=|>")) {
const Token *expr1 = tok->astOperand1();
const Token *expr2 = tok->astOperand2();
if (!expr1 || !expr2)
if (!tok->isComparisonOp())
continue;
const Token *expr1 = tok->astOperand1();
const Token *expr2 = tok->astOperand2();
if (!expr1 || !expr2)
continue;
if (expr1->isNumber())
std::swap(expr1,expr2);
if (!expr2->isNumber())
continue;
const MathLib::bigint num2 = MathLib::toLongNumber(expr2->str());
if (num2 < 0)
continue;
if (!Token::Match(expr1,"[&|]"))
continue;
std::list<MathLib::bigint> numbers;
getnumchildren(expr1, numbers);
for (std::list<MathLib::bigint>::const_iterator num = numbers.begin(); num != numbers.end(); ++num) {
const MathLib::bigint num1 = *num;
if (num1 < 0)
continue;
if (expr1->isNumber())
std::swap(expr1,expr2);
if (!expr2->isNumber())
continue;
const MathLib::bigint num2 = MathLib::toLongNumber(expr2->str());
if (num2 < 0)
continue;
if (!Token::Match(expr1,"[&|]"))
continue;
std::list<MathLib::bigint> numbers;
getnumchildren(expr1, numbers);
for (std::list<MathLib::bigint>::const_iterator num = numbers.begin(); num != numbers.end(); ++num) {
const MathLib::bigint num1 = *num;
if (num1 < 0)
continue;
if (Token::Match(tok, "==|!=")) {
if ((expr1->str() == "&" && (num1 & num2) != num2) ||
(expr1->str() == "|" && (num1 | num2) != num2)) {
const std::string& op(tok->str());
comparisonError(expr1, expr1->str(), num1, op, num2, op=="==" ? false : true);
}
} else if (Token::simpleMatch(tok, ">")) {
if ((expr1->str() == "&" && (num1 <= num2))) {
const std::string& op(tok->str());
comparisonError(expr1, expr1->str(), num1, op, num2, false);
if (Token::Match(tok, "==|!=")) {
if ((expr1->str() == "&" && (num1 & num2) != num2) ||
(expr1->str() == "|" && (num1 | num2) != num2)) {
const std::string& op(tok->str());
comparisonError(expr1, expr1->str(), num1, op, num2, op=="==" ? false : true);
}
} else if (expr1->str() == "&") {
const bool or_equal = Token::Match(tok, ">=|<=");
const std::string& op(tok->str());
if ((Token::Match(tok, ">=|<")) && (num1 < num2)) {
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? false : true);
} else if ((Token::Match(tok, "<=|>")) && (num1 <= num2)) {
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? true : false);
}
} else if (expr1->str() == "|") {
if ((expr1->astOperand1()->valueType()) &&
(expr1->astOperand1()->valueType()->sign == ValueType::Sign::UNSIGNED)) {
const bool or_equal = Token::Match(tok, ">=|<=");
const std::string& op(tok->str());
if ((Token::Match(tok, ">=|<")) && (num1 >= num2)) {
//"(a | 0x07) >= 7U" is always true for unsigned a
//"(a | 0x07) < 7U" is always false for unsigned a
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? true : false);
} else if ((Token::Match(tok, "<=|>")) && (num1 > num2)) {
//"(a | 0x08) <= 7U" is always false for unsigned a
//"(a | 0x07) > 6U" is always true for unsigned a
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? false : true);
}
}
}

153
test/testcondition.cpp
View File

@ -47,7 +47,7 @@ private:
TEST_CASE(assignAndCompare); // assignment and comparison don't match
TEST_CASE(mismatchingBitAnd); // overlapping bitmasks
TEST_CASE(compare); // mismatching LHS/RHS in comparison
TEST_CASE(comparison); // CheckCondition::comparison test cases
TEST_CASE(multicompare); // mismatching comparisons
TEST_CASE(duplicateIf); // duplicate conditions in if and else-if
@ -318,30 +318,64 @@ private:
ASSERT_EQUALS("", errout.str());
}
void compare() {
check("void foo(int x)\n"
"{\n"
" if ((x & 4) == 3);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X & 0x4) == 0x3' is always false.\n", errout.str());
check("void foo(int x)\n"
"{\n"
" if ((x | 4) == 3);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X | 0x4) == 0x3' is always false.\n", errout.str());
check("void foo(int x)\n"
"{\n"
" if ((x | 4) != 3);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X | 0x4) != 0x3' is always true.\n", errout.str());
check("void foo(int x)\n"
"{\n"
" if ((x & y & 4 & z ) == 3);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X & 0x4) == 0x3' is always false.\n", errout.str());
void comparison() {
// CheckCondition::comparison test cases
// '=='
check("void f(int a) {\n assert( (a & 0x07) == 8U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) == 0x8' is always false.\n",errout.str());
check("void f(int a) {\n assert( (a & b & 4 & c ) == 3 );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x4) == 0x3' is always false.\n", errout.str());
check("void f(int a) {\n assert( (a | 0x07) == 8U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x7) == 0x8' is always false.\n",errout.str());
check("void f(int a) {\n assert( (a & 0x07) == 7U );\n}");
ASSERT_EQUALS("", errout.str());
check("void f(int a) {\n assert( (a | 0x01) == -15 );\n}");
ASSERT_EQUALS("", errout.str());
// '!='
check("void f(int a) {\n assert( (a & 0x07) != 8U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) != 0x8' is always true.\n",errout.str());
check("void f(int a) {\n assert( (a | 0x07) != 8U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x7) != 0x8' is always true.\n",errout.str());
check("void f(int a) {\n assert( (a & 0x07) != 7U );\n}");
ASSERT_EQUALS("", errout.str());
check("void f(int a) {\n assert( (a | 0x07) != 7U );\n}");
ASSERT_EQUALS("", errout.str());
// '>='
check("void f(int a) {\n assert( (a & 0x07) >= 8U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) >= 0x8' is always false.\n",errout.str());
check("void f(unsigned int a) {\n assert( (a | 0x7) >= 7U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x7) >= 0x7' is always true.\n",errout.str());
check("void f(int a) {\n assert( (a & 0x07) >= 7U );\n}");
ASSERT_EQUALS("",errout.str());
check("void f(int a) {\n assert( (a | 0x07) >= 8U );\n}");
ASSERT_EQUALS("",errout.str()); //correct for negative 'a'
// '>'
check("void f(int a) {\n assert( (a & 0x07) > 7U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) > 0x7' is always false.\n",errout.str());
check("void f(unsigned int a) {\n assert( (a | 0x7) > 6U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x7) > 0x6' is always true.\n",errout.str());
check("void f(int a) {\n assert( (a & 0x07) > 6U );\n}");
ASSERT_EQUALS("",errout.str());
check("void f(int a) {\n assert( (a | 0x07) > 7U );\n}");
ASSERT_EQUALS("",errout.str()); //correct for negative 'a'
// '<='
check("void f(int a) {\n assert( (a & 0x07) <= 7U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) <= 0x7' is always true.\n",errout.str());
check("void f(unsigned int a) {\n assert( (a | 0x08) <= 7U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x8) <= 0x7' is always false.\n",errout.str());
check("void f(int a) {\n assert( (a & 0x07) <= 6U );\n}");
ASSERT_EQUALS("",errout.str());
check("void f(int a) {\n assert( (a | 0x08) <= 7U );\n}");
ASSERT_EQUALS("",errout.str()); //correct for negative 'a'
// '<'
check("void f(int a) {\n assert( (a & 0x07) < 8U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) < 0x8' is always true.\n",errout.str());
check("void f(unsigned int a) {\n assert( (a | 0x07) < 7U );\n}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x7) < 0x7' is always false.\n",errout.str());
check("void f(int a) {\n assert( (a & 0x07) < 3U );\n}");
ASSERT_EQUALS("",errout.str());
check("void f(int a) {\n assert( (a | 0x07) < 7U );\n}");
ASSERT_EQUALS("",errout.str()); //correct for negative 'a'
}
void multicompare() {
@ -592,75 +626,6 @@ private:
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
//Various bitmask comparison checks
//#7428 false negative: condition '(a&7)>7U' is always false
//#7522 false positive: condition '(X|7)>=6' is correct
check("void f() {\n"
"assert( (a & 0x07) == 8U );\n" // statement always false
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) == 0x8' is always false.\n",
errout.str());
check("void f() {\n"
"assert( (a & 0x07) == 7U );\n" // statement correct
"assert( (a & 0x07) == 6U );\n" // statement correct
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
"assert( (a | 0x07) == 8U );\n" // statement always false
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x7) == 0x8' is always false.\n",
errout.str());
check("void f() {\n"
"assert( (a | 0x07) == 7U );\n" // statement correct
"assert( (a | 0x07) == 23U );\n" // statement correct
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
"assert( (a & 0x07) != 8U );\n" // statement always true
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) != 0x8' is always true.\n",
errout.str());
check("void f() {\n"
"assert( (a & 0x07) != 7U );\n" // statement correct
"assert( (a & 0x07) != 0U );\n" // statement correct
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
"assert( (a | 0x07) != 8U );\n" // statement always true
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X | 0x7) != 0x8' is always true.\n",
errout.str());
check("void f() {\n"
"assert( (a | 0x07) != 7U );\n" // statement correct
"}");
ASSERT_EQUALS("", errout.str());
//TRAC #7428 false negative: condition '(X&7)>7' is always false
check("void f() {\n"
"assert( (a & 0x07) > 7U );\n" // statement always false
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) > 0x7' is always false.\n",
errout.str());
check("void f() {\n"
"assert( (a & 0x07) > 6U );\n" // statement correct
"}");
ASSERT_EQUALS("", errout.str());
//TRAC #7522 false positive: condition '(X|7)>=6' is correct (X can be negative)
check("void f() {\n"
"assert( (a | 0x07) >= 6U );\n" // statement correct (X can be negative)
"}");
ASSERT_EQUALS("",errout.str());
}