From c1bd259472a6beefe2eea6dcc4a4505916c266a2 Mon Sep 17 00:00:00 2001 From: Tyson Nottingham Date: Wed, 25 Sep 2019 22:03:25 -0700 Subject: [PATCH 1/2] Fix false negatives in checkBitwiseOnBoolean Use AST-based tests in favor of token-based tests for greater coverage. --- lib/checkbool.cpp | 19 ++++++------------- lib/checkbool.h | 4 ++-- test/testbool.cpp | 25 +++++++++++++++---------- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/lib/checkbool.cpp b/lib/checkbool.cpp index dc38193ec0d..c87fef98ec9 100644 --- a/lib/checkbool.cpp +++ b/lib/checkbool.cpp @@ -93,27 +93,20 @@ void CheckBool::checkBitwiseOnBoolean() const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); for (const Scope * scope : symbolDatabase->functionScopes) { for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) { - if (Token::Match(tok, "(|.|return|&&|%oror%|throw|, %var% [&|]")) { - const Variable *var = tok->next()->variable(); - if (isBool(var)) { - bitwiseOnBooleanError(tok->next(), var->name(), tok->strAt(2) == "&" ? "&&" : "||"); - tok = tok->tokAt(2); - } - } else if (Token::Match(tok, "[&|] %var% )|.|return|&&|%oror%|throw|,") && (!tok->previous() || !tok->previous()->isExtendedOp() || tok->strAt(-1) == ")" || tok->strAt(-1) == "]")) { - const Variable *var = tok->next()->variable(); - if (isBool(var)) { - bitwiseOnBooleanError(tok->next(), var->name(), tok->str() == "&" ? "&&" : "||"); - tok = tok->tokAt(2); + if (tok->isBinaryOp() && (tok->str() == "&" || tok->str() == "|")) { + if (astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2())) { + const std::string expression = astIsBool(tok->astOperand1()) ? tok->astOperand1()->expressionString() : tok->astOperand2()->expressionString(); + bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||"); } } } } } -void CheckBool::bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op) +void CheckBool::bitwiseOnBooleanError(const Token *tok, const std::string &expression, const std::string &op) { reportError(tok, Severity::style, "bitwiseOnBoolean", - "Boolean variable '" + varname + "' is used in bitwise operation. Did you mean '" + op + "'?", + "Boolean expression '" + expression + "' is used in bitwise operation. Did you mean '" + op + "'?", CWE398, true); } diff --git a/lib/checkbool.h b/lib/checkbool.h index 177e9312597..335fe027eb8 100644 --- a/lib/checkbool.h +++ b/lib/checkbool.h @@ -106,7 +106,7 @@ class CPPCHECKLIB CheckBool : public Check { void comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression); void assignBoolToPointerError(const Token *tok); void assignBoolToFloatError(const Token *tok); - void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op); + void bitwiseOnBooleanError(const Token *tok, const std::string &expression, const std::string &op); void comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1); void pointerArithBoolError(const Token *tok); void returnValueBoolError(const Token *tok); @@ -120,7 +120,7 @@ class CPPCHECKLIB CheckBool : public Check { c.comparisonOfTwoFuncsReturningBoolError(nullptr, "func_name1", "func_name2"); c.comparisonOfBoolWithBoolError(nullptr, "var_name"); c.incrementBooleanError(nullptr); - c.bitwiseOnBooleanError(nullptr, "varname", "&&"); + c.bitwiseOnBooleanError(nullptr, "expression", "&&"); c.comparisonOfBoolExpressionWithIntError(nullptr, true); c.pointerArithBoolError(nullptr); c.comparisonOfBoolWithInvalidComparator(nullptr, "expression"); diff --git a/test/testbool.cpp b/test/testbool.cpp index 385a984c597..5b2c0e11839 100644 --- a/test/testbool.cpp +++ b/test/testbool.cpp @@ -785,56 +785,61 @@ class TestBool : public TestFixture { check("void f(_Bool a, _Bool b) {\n" " if(a & b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); check("void f(_Bool a, _Bool b) {\n" " if(a | b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); check("void f(bool a, bool b) {\n" " if(a & !b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); check("void f(bool a, bool b) {\n" " if(a | !b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); check("bool a, b;\n" "void f() {\n" " if(a & b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); check("bool a, b;\n" "void f() {\n" " if(a & !b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); check("bool a, b;\n" "void f() {\n" " if(a | b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); check("bool a, b;\n" "void f() {\n" " if(a | !b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str()); check("void f(bool a, int b) {\n" " if(a & b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); check("void f(int a, bool b) {\n" " if(a & b) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'b' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'b' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); + + check("void f(int a, int b) {\n" + " if((a > 0) & (b < 0)) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a>0' is used in bitwise operation. Did you mean '&&'?\n", errout.str()); check("void f(int a, int b) {\n" " if(a & b) {}\n" From 80732fea8c9d5e3f7064fd1976713b4317160abf Mon Sep 17 00:00:00 2001 From: Tyson Nottingham Date: Fri, 4 Oct 2019 23:54:46 -0700 Subject: [PATCH 2/2] Travis: add suppressions for bitwiseOnBool --- .travis_suppressions | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis_suppressions b/.travis_suppressions index 65d307bd32d..23c9413da37 100644 --- a/.travis_suppressions +++ b/.travis_suppressions @@ -6,6 +6,7 @@ noValidConfiguration shadowFunction functionConst functionStatic +bitwiseOnBoolean # temporary suppressions - fix the warnings! duplicateBranch:lib/checkunusedvar.cpp