Fix false negatives in checkBitwiseOnBoolean (#2220)

* Fix false negatives in checkBitwiseOnBoolean

Use AST-based tests in favor of token-based tests for greater coverage.

* Travis: add suppressions for bitwiseOnBool
This commit is contained in:
Tyson Nottingham 2019-10-06 00:57:31 -07:00 committed by Daniel Marjamäki
parent b97436e8f8
commit 0950a97df2
4 changed files with 24 additions and 25 deletions

View File

@ -6,6 +6,7 @@ noValidConfiguration
shadowFunction shadowFunction
functionConst functionConst
functionStatic functionStatic
bitwiseOnBoolean
# temporary suppressions - fix the warnings! # temporary suppressions - fix the warnings!
duplicateBranch:lib/checkunusedvar.cpp duplicateBranch:lib/checkunusedvar.cpp

View File

@ -93,27 +93,20 @@ void CheckBool::checkBitwiseOnBoolean()
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * scope : symbolDatabase->functionScopes) { for (const Scope * scope : symbolDatabase->functionScopes) {
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) { for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
if (Token::Match(tok, "(|.|return|&&|%oror%|throw|, %var% [&|]")) { if (tok->isBinaryOp() && (tok->str() == "&" || tok->str() == "|")) {
const Variable *var = tok->next()->variable(); if (astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2())) {
if (isBool(var)) { const std::string expression = astIsBool(tok->astOperand1()) ? tok->astOperand1()->expressionString() : tok->astOperand2()->expressionString();
bitwiseOnBooleanError(tok->next(), var->name(), tok->strAt(2) == "&" ? "&&" : "||"); bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||");
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);
} }
} }
} }
} }
} }
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", 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, CWE398,
true); true);
} }

View File

@ -106,7 +106,7 @@ private:
void comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression); void comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression);
void assignBoolToPointerError(const Token *tok); void assignBoolToPointerError(const Token *tok);
void assignBoolToFloatError(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 comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1);
void pointerArithBoolError(const Token *tok); void pointerArithBoolError(const Token *tok);
void returnValueBoolError(const Token *tok); void returnValueBoolError(const Token *tok);
@ -120,7 +120,7 @@ private:
c.comparisonOfTwoFuncsReturningBoolError(nullptr, "func_name1", "func_name2"); c.comparisonOfTwoFuncsReturningBoolError(nullptr, "func_name1", "func_name2");
c.comparisonOfBoolWithBoolError(nullptr, "var_name"); c.comparisonOfBoolWithBoolError(nullptr, "var_name");
c.incrementBooleanError(nullptr); c.incrementBooleanError(nullptr);
c.bitwiseOnBooleanError(nullptr, "varname", "&&"); c.bitwiseOnBooleanError(nullptr, "expression", "&&");
c.comparisonOfBoolExpressionWithIntError(nullptr, true); c.comparisonOfBoolExpressionWithIntError(nullptr, true);
c.pointerArithBoolError(nullptr); c.pointerArithBoolError(nullptr);
c.comparisonOfBoolWithInvalidComparator(nullptr, "expression"); c.comparisonOfBoolWithInvalidComparator(nullptr, "expression");

View File

@ -785,56 +785,61 @@ private:
check("void f(_Bool a, _Bool b) {\n" check("void f(_Bool a, _Bool b) {\n"
" if(a & 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" check("void f(_Bool a, _Bool b) {\n"
" if(a | 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" check("void f(bool a, bool b) {\n"
" if(a & !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" check("void f(bool a, bool b) {\n"
" if(a | !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" check("bool a, b;\n"
"void f() {\n" "void f() {\n"
" if(a & b) {}\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" check("bool a, b;\n"
"void f() {\n" "void f() {\n"
" if(a & !b) {}\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" check("bool a, b;\n"
"void f() {\n" "void f() {\n"
" if(a | b) {}\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" check("bool a, b;\n"
"void f() {\n" "void f() {\n"
" if(a | !b) {}\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" check("void f(bool a, int b) {\n"
" if(a & 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" check("void f(int a, bool b) {\n"
" if(a & 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" check("void f(int a, int b) {\n"
" if(a & b) {}\n" " if(a & b) {}\n"