Skip to content

Fix false negatives in checkBitwiseOnBoolean #2220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danmar merged 2 commits into from
Oct 6, 2019
Merged

Fix false negatives in checkBitwiseOnBoolean #2220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1bd259
Fix false negatives in checkBitwiseOnBoolean
tgnottingham Sep 26, 2019
80732fe
Travis: add suppressions for bitwiseOnBool
tgnottingham Oct 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .travis_suppressions
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ noValidConfiguration
shadowFunction
functionConst
functionStatic
bitwiseOnBoolean

# temporary suppressions - fix the warnings!
duplicateBranch:lib/checkunusedvar.cpp
Expand Down
19 changes: 6 additions & 13 deletions lib/checkbool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/checkbool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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");
Expand Down
25 changes: 15 additions & 10 deletions test/testbool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down