diff --git a/lib/checkbool.cpp b/lib/checkbool.cpp index a069cc452..79b5993a7 100644 --- a/lib/checkbool.cpp +++ b/lib/checkbool.cpp @@ -99,30 +99,43 @@ 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 (tok->isBinaryOp() && (tok->str() == "&" || tok->str() == "|")) { - if (!(astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2()))) + if (tok->isBinaryOp()) { + bool isCompound{}; + if (tok->str() == "&" || tok->str() == "|") + isCompound = false; + else if (tok->str() == "&=" || tok->str() == "|=") + isCompound = true; + else continue; - if (tok->str() == "|" && !isConvertedToBool(tok) && !(astIsBool(tok->astOperand1()) && astIsBool(tok->astOperand2()))) + const bool isBoolOp1 = astIsBool(tok->astOperand1()); + const bool isBoolOp2 = astIsBool(tok->astOperand2()); + if (!(isBoolOp1 || isBoolOp2)) + continue; + if (isCompound && !isBoolOp1) + continue; + if (tok->str() == "|" && !isConvertedToBool(tok) && !(isBoolOp1 && isBoolOp2)) continue; // first operand will always be evaluated if (!isConstExpression(tok->astOperand2(), mSettings->library, true, mTokenizer->isCPP())) continue; if (tok->astOperand2()->variable() && tok->astOperand2()->variable()->nameToken() == tok->astOperand2()) continue; - const std::string expression = astIsBool(tok->astOperand1()) ? tok->astOperand1()->expressionString() - : tok->astOperand2()->expressionString(); - bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||"); + const std::string expression = (isBoolOp1 ? tok->astOperand1() : tok->astOperand2())->expressionString(); + bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||", isCompound); } } } } -void CheckBool::bitwiseOnBooleanError(const Token* tok, const std::string& expression, const std::string& op) +void CheckBool::bitwiseOnBooleanError(const Token* tok, const std::string& expression, const std::string& op, bool isCompound) { + std::string msg = "Boolean expression '" + expression + "' is used in bitwise operation."; + if (!isCompound) + msg += " Did you mean '" + op + "'?"; reportError(tok, Severity::style, "bitwiseOnBoolean", - "Boolean expression '" + expression + "' is used in bitwise operation. Did you mean '" + op + "'?", + msg, CWE398, Certainty::inconclusive); } diff --git a/lib/checkbool.h b/lib/checkbool.h index d3aeeecc2..9b843a5cd 100644 --- a/lib/checkbool.h +++ b/lib/checkbool.h @@ -104,7 +104,7 @@ private: 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& expression, const std::string& op); + void bitwiseOnBooleanError(const Token* tok, const std::string& expression, const std::string& op, bool isCompound = false); void comparisonOfBoolExpressionWithIntError(const Token *tok, bool not0or1); void pointerArithBoolError(const Token *tok); void returnValueBoolError(const Token *tok); diff --git a/test/testbool.cpp b/test/testbool.cpp index 9a69d6c95..f9320f6b9 100644 --- a/test/testbool.cpp +++ b/test/testbool.cpp @@ -932,6 +932,23 @@ private: " return b | g() | c;\n" "}\n"); ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'c' is used in bitwise operation. Did you mean '||'?\n", errout.str()); + + check("void f(int i) {\n" // #4233 + " bool b = true, c = false;\n" + " b &= i;\n" + " c |= i;\n" + " if (b || c) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Boolean expression 'b' is used in bitwise operation.\n" + "[test.cpp:4]: (style, inconclusive) Boolean expression 'c' is used in bitwise operation.\n", + errout.str()); + + check("void f(int i, int j, bool b) {\n" + " i &= b;\n" + " j |= b;\n" + " if (b || c) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void incrementBoolean() {