Fixed #5845 (new check: same condition after noreturn conditional code => second condition is always false)

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 (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

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"

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"