From f9624368266db78a98c4d1e132b3eef3715061e7 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sat, 4 Jun 2022 17:25:10 +0200 Subject: [PATCH] Fix #11082 FN badBitmaskCheck for binary or with 0 (#4170) * Fix #11082 FN badBitmaskCheck for binary or with 0 * Add test for #10703 --- lib/checkcondition.cpp | 13 +++++++++++-- lib/checkcondition.h | 2 +- test/testcondition.cpp | 6 ++++++ test/testunusedvar.cpp | 10 ++++++++++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 7292cd0d6..ca7ea708f 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -310,13 +310,22 @@ void CheckCondition::checkBadBitmaskCheck() if (isBoolean && isTrue) badBitmaskCheckError(tok); + + const bool isNoOp = (tok->astOperand1()->hasKnownIntValue() && tok->astOperand1()->values().front().intvalue == 0) || + (tok->astOperand2()->hasKnownIntValue() && tok->astOperand2()->values().front().intvalue == 0); + + if (isNoOp) + badBitmaskCheckError(tok, isNoOp); } } } -void CheckCondition::badBitmaskCheckError(const Token *tok) +void CheckCondition::badBitmaskCheckError(const Token *tok, bool isNoOp) { - reportError(tok, Severity::warning, "badBitmaskCheck", "Result of operator '|' is always true if one operand is non-zero. Did you intend to use '&'?", CWE571, Certainty::normal); + if (isNoOp) + reportError(tok, Severity::warning, "badBitmaskCheck", "Operator '|' with one operand equal to zero is redundant.", CWE571, Certainty::normal); + else + reportError(tok, Severity::warning, "badBitmaskCheck", "Result of operator '|' is always true if one operand is non-zero. Did you intend to use '&'?", CWE571, Certainty::normal); } void CheckCondition::comparison() diff --git a/lib/checkcondition.h b/lib/checkcondition.h index 1e225593e..b1273f8f1 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -136,7 +136,7 @@ private: bool isOverlappingCond(const Token * const cond1, const Token * const cond2, bool pure) const; void assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result); void mismatchingBitAndError(const Token *tok1, const MathLib::bigint num1, const Token *tok2, const MathLib::bigint num2); - void badBitmaskCheckError(const Token *tok); + void badBitmaskCheckError(const Token *tok, bool isNoOp = false); void comparisonError(const Token *tok, const std::string &bitop, MathLib::bigint value1, diff --git a/test/testcondition.cpp b/test/testcondition.cpp index aea3f33f7..4c11d5bdb 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -887,6 +887,12 @@ private: " }\n" "}"); ASSERT_EQUALS("", errout.str()); + + check("void f(int i) {\n" // #11082 + " int j = 0;\n" + " if (i | j) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Operator '|' with one operand equal to zero is redundant.\n", errout.str()); } diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 66a52bb48..3a6679534 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -5918,6 +5918,16 @@ private: " g();\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + functionVariableUsage("int f(int *p) {\n" // #10703 + " std::unique_ptr up(p);\n" + " return *p;\n" + "}\n" + "int g(int* p) {\n" + " auto up = std::unique_ptr(p);\n" + " return *p;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } // ticket #3104 - false positive when variable is read with "if (NOT var)"