From a5cfa2b12c034e59b5805ac51b53e893c8bc2d1b Mon Sep 17 00:00:00 2001 From: Bartlomiej Grzeskowiak Date: Fri, 17 Jun 2016 12:12:53 +0200 Subject: [PATCH] - #7522 and #7428 revisited. ((a&7)>7U) is always false and ((X|7)>=6) is correct (X can be negative). --- lib/checkcondition.cpp | 11 ++--- test/testcondition.cpp | 92 +++++++++++++++++++++++++++++++----------- 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index f9a256eb2..c6a8d590b 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -281,7 +281,7 @@ void CheckCondition::comparison() return; for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (Token::Match(tok, "==|!=|>|>=|<|<=")) { + if (Token::Match(tok, "==|!=|>")) { const Token *expr1 = tok->astOperand1(); const Token *expr2 = tok->astOperand2(); if (!expr1 || !expr2) @@ -301,22 +301,17 @@ void CheckCondition::comparison() const MathLib::bigint num1 = *num; if (num1 < 0) continue; - if (Token::Match(tok, "==|!=|>=|<=")) { + 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); + 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); } - } else if (Token::simpleMatch(tok, "<")) { - if ((expr1->str() == "|" && (num1 >= num2))) { - const std::string& op(tok->str()); - comparisonError(expr1, expr1->str(), num1, op, num2, false); - } } } } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 2b21a152d..e3b887a08 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -591,6 +591,75 @@ 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()); } @@ -1651,29 +1720,6 @@ private: " assert(x == 0);\n" "}"); ASSERT_EQUALS("", errout.str()); - - //TRAC #7428 false negative: Statement is always false - check("void f() {\n" - "assert( (a & 0x07) == 8U );\n" // statement always false, because 7 == 8 is false - "assert( (a & 0x07) > 7U );\n" // statement always false, because 7 > 7 is false - "assert( (a | 0x07) < 7U );\n" // statement always false, because 7 < 7 is false - "assert( (a & 0x07) > 8U );\n" // statement always false, because 7 > 8 is false - "assert( (a | 0x07) < 6U );\n" // statement always false, because 7 < 6 is false - "assert( (a & 0x07) >= 7U );\n" // statement correct - "assert( (a | 0x07) <= 7U );\n" // statement correct - "assert( (a & 0x07) >= 8U );\n" // statement always false, because 7 >= 8 is false - "assert( (a | 0x07) <= 6U );\n" // statement always false, because 7 <= 6 is false - "assert( (a & 0x07) > 3U );\n" // statement correct - "assert( (a | 0x07) < 9U );\n" // statement correct - "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) Expression '(X & 0x7) == 0x8' is always false.\n" - "[test.cpp:3]: (style) Expression '(X & 0x7) > 0x7' is always false.\n" - "[test.cpp:4]: (style) Expression '(X | 0x7) < 0x7' is always false.\n" - "[test.cpp:5]: (style) Expression '(X & 0x7) > 0x8' is always false.\n" - "[test.cpp:6]: (style) Expression '(X | 0x7) < 0x6' is always false.\n" - "[test.cpp:9]: (style) Expression '(X & 0x7) >= 0x8' is always false.\n" - "[test.cpp:10]: (style) Expression '(X | 0x7) <= 0x6' is always false.\n", - errout.str()); } void checkInvalidTestForOverflow() {