Skip to content

Commit 0950a97

Browse files
tgnottinghamdanmar
tgnottingham
authored and
danmar
committed
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
1 parent b97436e commit 0950a97

4 files changed

Lines changed: 24 additions & 25 deletions

File tree

‎.travis_suppressions‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ noValidConfiguration
66
shadowFunction
77
functionConst
88
functionStatic
9+
bitwiseOnBoolean
910

1011
# temporary suppressions - fix the warnings!
1112
duplicateBranch:lib/checkunusedvar.cpp

‎lib/checkbool.cpp‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -93,27 +93,20 @@ void CheckBool::checkBitwiseOnBoolean()
9393
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
9494
for (const Scope * scope : symbolDatabase->functionScopes) {
9595
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
96-
if (Token::Match(tok, "(|.|return|&&|%oror%|throw|, %var% [&|]")) {
97-
const Variable *var = tok->next()->variable();
98-
if (isBool(var)) {
99-
bitwiseOnBooleanError(tok->next(), var->name(), tok->strAt(2) == "&" ? "&&" : "||");
100-
tok = tok->tokAt(2);
101-
}
102-
} else if (Token::Match(tok, "[&|] %var% )|.|return|&&|%oror%|throw|,") && (!tok->previous() || !tok->previous()->isExtendedOp() || tok->strAt(-1) == ")" || tok->strAt(-1) == "]")) {
103-
const Variable *var = tok->next()->variable();
104-
if (isBool(var)) {
105-
bitwiseOnBooleanError(tok->next(), var->name(), tok->str() == "&" ? "&&" : "||");
106-
tok = tok->tokAt(2);
96+
if (tok->isBinaryOp() && (tok->str() == "&" || tok->str() == "|")) {
97+
if (astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2())) {
98+
const std::string expression = astIsBool(tok->astOperand1()) ? tok->astOperand1()->expressionString() : tok->astOperand2()->expressionString();
99+
bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||");
107100
}
108101
}
109102
}
110103
}
111104
}
112105

113-
void CheckBool::bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op)
106+
void CheckBool::bitwiseOnBooleanError(const Token *tok, const std::string &expression, const std::string &op)
114107
{
115108
reportError(tok, Severity::style, "bitwiseOnBoolean",
116-
"Boolean variable '" + varname + "' is used in bitwise operation. Did you mean '" + op + "'?",
109+
"Boolean expression '" + expression + "' is used in bitwise operation. Did you mean '" + op + "'?",
117110
CWE398,
118111
true);
119112
}

‎lib/checkbool.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class CPPCHECKLIB CheckBool : public Check {
106106
void comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression);
107107
void assignBoolToPointerError(const Token *tok);
108108
void assignBoolToFloatError(const Token *tok);
109-
void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op);
109+
void bitwiseOnBooleanError(const Token *tok, const std::string &expression, const std::string &op);
110110
void comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1);
111111
void pointerArithBoolError(const Token *tok);
112112
void returnValueBoolError(const Token *tok);
@@ -120,7 +120,7 @@ class CPPCHECKLIB CheckBool : public Check {
120120
c.comparisonOfTwoFuncsReturningBoolError(nullptr, "func_name1", "func_name2");
121121
c.comparisonOfBoolWithBoolError(nullptr, "var_name");
122122
c.incrementBooleanError(nullptr);
123-
c.bitwiseOnBooleanError(nullptr, "varname", "&&");
123+
c.bitwiseOnBooleanError(nullptr, "expression", "&&");
124124
c.comparisonOfBoolExpressionWithIntError(nullptr, true);
125125
c.pointerArithBoolError(nullptr);
126126
c.comparisonOfBoolWithInvalidComparator(nullptr, "expression");

‎test/testbool.cpp‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -785,56 +785,61 @@ class TestBool : public TestFixture {
785785
check("void f(_Bool a, _Bool b) {\n"
786786
" if(a & b) {}\n"
787787
"}");
788-
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
788+
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
789789

790790
check("void f(_Bool a, _Bool b) {\n"
791791
" if(a | b) {}\n"
792792
"}");
793-
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
793+
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
794794

795795
check("void f(bool a, bool b) {\n"
796796
" if(a & !b) {}\n"
797797
"}");
798-
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
798+
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
799799

800800
check("void f(bool a, bool b) {\n"
801801
" if(a | !b) {}\n"
802802
"}");
803-
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
803+
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
804804

805805
check("bool a, b;\n"
806806
"void f() {\n"
807807
" if(a & b) {}\n"
808808
"}");
809-
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
809+
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
810810

811811
check("bool a, b;\n"
812812
"void f() {\n"
813813
" if(a & !b) {}\n"
814814
"}");
815-
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
815+
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
816816

817817
check("bool a, b;\n"
818818
"void f() {\n"
819819
" if(a | b) {}\n"
820820
"}");
821-
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
821+
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
822822

823823
check("bool a, b;\n"
824824
"void f() {\n"
825825
" if(a | !b) {}\n"
826826
"}");
827-
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
827+
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
828828

829829
check("void f(bool a, int b) {\n"
830830
" if(a & b) {}\n"
831831
"}");
832-
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
832+
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
833833

834834
check("void f(int a, bool b) {\n"
835835
" if(a & b) {}\n"
836836
"}");
837-
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean variable 'b' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
837+
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'b' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
838+
839+
check("void f(int a, int b) {\n"
840+
" if((a > 0) & (b < 0)) {}\n"
841+
"}");
842+
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a>0' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
838843

839844
check("void f(int a, int b) {\n"
840845
" if(a & b) {}\n"

0 commit comments

Comments
 (0)