diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index e8ac2d8e5..9533c8e5e 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -441,10 +441,12 @@ void CheckCondition::multiConditionError(const Token *tok, unsigned int line1) } //--------------------------------------------------------------------------- -// Detect oppositing inner and outer conditions +// - Opposite inner conditions => always false +// - (TODO) Same/Overlapping inner condition => always true +// - same condition after early exit => always false //--------------------------------------------------------------------------- -void CheckCondition::oppositeInnerCondition() +void CheckCondition::multiCondition2() { if (!_settings->isEnabled(Settings::WARNING)) return; @@ -496,9 +498,19 @@ void CheckCondition::oppositeInnerCondition() } } - // parse until inner condition is reached.. + // parse until second condition is reached.. + enum MULTICONDITIONTYPE { INNER, AFTER } type; + const Token *tok; + if (Token::Match(scope->classStart, "{ return|throw|continue|break")) { + tok = scope->classEnd->next(); + type = MULTICONDITIONTYPE::AFTER; + } else { + tok = scope->classStart; + type = MULTICONDITIONTYPE::INNER; + } + const Token * const endToken = tok->scope()->classEnd; const Token *ifToken = nullptr; - for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) { + for (; tok && tok != endToken; tok = tok->next()) { if (Token::simpleMatch(tok, "if (")) { ifToken = tok; break; @@ -539,8 +551,13 @@ void CheckCondition::oppositeInnerCondition() // Condition.. const Token *cond2 = ifToken->next()->astOperand2(); - if (isOppositeCond(false, _tokenizer->isCPP(), cond1, cond2, _settings->library, true)) - oppositeInnerConditionError(cond1, cond2); + if (type == MULTICONDITIONTYPE::INNER) { + if (isOppositeCond(false, _tokenizer->isCPP(), cond1, cond2, _settings->library, true)) + oppositeInnerConditionError(cond1, cond2); + } else { + if (isSameExpression(_tokenizer->isCPP(), true, cond1, cond2, _settings->library, true)) + sameConditionAfterEarlyExitError(cond1, cond2); + } } } @@ -552,6 +569,14 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* reportError(errorPath, Severity::warning, "oppositeInnerCondition", "Opposite inner 'if' condition leads to a dead code block.", CWE398, false); } +void CheckCondition::sameConditionAfterEarlyExitError(const Token *cond1, const Token* cond2) +{ + ErrorPath errorPath; + errorPath.push_back(ErrorPathItem(cond1, "first condition")); + errorPath.push_back(ErrorPathItem(cond2, "second condition")); + reportError(errorPath, Severity::warning, "sameConditionAfterEarlyExit", "Same condition, second condition is always false", CWE398, false); +} + //--------------------------------------------------------------------------- // if ((x != 1) || (x != 3)) // expression always true // if ((x == 1) && (x == 3)) // expression always false diff --git a/lib/checkcondition.h b/lib/checkcondition.h index 25c9cea0f..cf8a135bc 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -55,7 +55,7 @@ public: CheckCondition checkCondition(tokenizer, settings, errorLogger); checkCondition.multiCondition(); checkCondition.clarifyCondition(); // not simplified because ifAssign - checkCondition.oppositeInnerCondition(); + checkCondition.multiCondition2(); checkCondition.checkIncorrectLogicOperator(); checkCondition.checkInvalidTestForOverflow(); checkCondition.alwaysTrueFalse(); @@ -90,8 +90,13 @@ public: /** match 'if' and 'else if' conditions */ void multiCondition(); - /** To check the dead code in a program, which is inaccessible due to the counter-conditions check in nested-if statements **/ - void oppositeInnerCondition(); + /** + * multiconditions #2 + * - Opposite inner conditions => always false + * - (TODO) Same/Overlapping inner condition => always true + * - same condition after early exit => always false + **/ + void multiCondition2(); /** @brief %Check for testing for mutual exclusion over ||*/ void checkIncorrectLogicOperator(); @@ -124,6 +129,8 @@ private: void oppositeInnerConditionError(const Token *tok1, const Token* tok2); + void sameConditionAfterEarlyExitError(const Token *cond1, const Token *cond2); + void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive); void redundantConditionError(const Token *tok, const std::string &text, bool inconclusive); @@ -144,6 +151,7 @@ private: c.multiConditionError(nullptr,1); c.mismatchingBitAndError(nullptr, 0xf0, nullptr, 1); c.oppositeInnerConditionError(nullptr, nullptr); + c.sameConditionAfterEarlyExitError(nullptr, nullptr); c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true, false); c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false); c.moduloAlwaysTrueFalseError(nullptr, "1"); @@ -163,7 +171,8 @@ private: "- Detect usage of | where & should be used\n" "- Detect matching 'if' and 'else if' conditions\n" "- Mismatching bitand (a &= 0xf0; a &= 1; => a = 0)\n" - "- Find dead code which is inaccessible due to the counter-conditions check in nested if statements\n" + "- Opposite inner condition is always false\n" + "- Same condition after early exit is always false\n" "- Condition that is always true/false\n" "- Mutual exclusion over || always evaluating to true\n" "- Comparisons of modulo results that are always true/false.\n" diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 62fac42b2..807285cd7 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -79,6 +79,8 @@ private: TEST_CASE(oppositeInnerCondition2); TEST_CASE(oppositeInnerConditionAnd); + TEST_CASE(sameConditionAfterEarlyExit); + TEST_CASE(clarifyCondition1); // if (a = b() < 0) TEST_CASE(clarifyCondition2); // if (a & b == c) TEST_CASE(clarifyCondition3); // if (! a & b) @@ -1709,6 +1711,14 @@ private: ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); } + void sameConditionAfterEarlyExit() { + check("void f(int x) {\n" + " if (x > 100) { return; }\n" + " if (x > 100) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Same condition, second condition is always false\n", errout.str()); + } + // clarify conditions with = and comparison void clarifyCondition1() { check("void f() {\n"