Fix issue 8234: false negative: (warning) Opposite inner 'if' condition leads to a dead code block. (cppcheck-opensource#2781)

pfultz2 · web-flow · commit a4f43fc2ad44 · 2020-09-07T07:53:41.000+02:00
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -956,6 +956,17 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
     if (!cond1 || !cond2)
         return false;
 
+    if (cond1->str() == "&&" && cond2->str() == "&&") {
+        for(const Token* tok1:{cond1->astOperand1(), cond1->astOperand2()}) {
+            for(const Token* tok2:{cond2->astOperand1(), cond2->astOperand2()}) {
+                if (isSameExpression(cpp, true, tok1, tok2, library, pure, followVar, errors)) {
+                    if (isOppositeCond(isNot, cpp, tok1->astSibling(), tok2->astSibling(), library, pure, followVar, errors))
+                        return true;
+                }
+            }
+        }
+    }
+
     if (cond1->str() == "!") {
         if (cond2->str() == "!=") {
             if (cond2->astOperand1() && cond2->astOperand1()->str() == "0")
diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
@@ -701,8 +701,10 @@ void CheckCondition::multiCondition2()
                             if (!firstCondition)
                                 return ChildrenToVisit::none;
                             if (firstCondition->str() == "&&") {
-                                return ChildrenToVisit::op1_and_op2;
-                            } else if (!firstCondition->hasKnownIntValue()) {
+                                if (!isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, true))
+                                    return ChildrenToVisit::op1_and_op2;
+                            }
+                            if (!firstCondition->hasKnownIntValue()) {
                                 if (!isReturnVar && isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, true, &errorPath)) {
                                     if (!isAliased(vars))
                                         oppositeInnerConditionError(firstCondition, cond2, errorPath);
diff --git a/lib/token.h b/lib/token.h
@@ -1263,6 +1263,26 @@ class CPPCHECKLIB Token {
     const Token * astParent() const {
         return mImpl->mAstParent;
     }
+    Token * astSibling() {
+        if (!astParent())
+            return nullptr;
+        if (this == astParent()->astOperand1())
+            return astParent()->astOperand2();
+        else if (this == astParent()->astOperand2())
+            return astParent()->astOperand1();
+        return nullptr;
+        
+    }
+    const Token * astSibling() const {
+        if (!astParent())
+            return nullptr;
+        if (this == astParent()->astOperand1())
+            return astParent()->astOperand2();
+        else if (this == astParent()->astOperand2())
+            return astParent()->astOperand1();
+        return nullptr;
+        
+    }
     Token *astTop() {
         Token *ret = this;
         while (ret->mImpl->mAstParent)
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
@@ -2310,6 +2310,12 @@ class TestCondition : public TestFixture {
               "  }"
               "}");
         ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
+
+        check("void f(bool x, const int a, const int b) {\n"
+              "        if(x && a < b)\n"
+              "            if( x && a > b){}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
     }
 
     void oppositeInnerConditionEmpty() {
diff --git a/test/testother.cpp b/test/testother.cpp
@@ -5310,7 +5310,7 @@ class TestOther : public TestFixture {
         check("void f(int* x, bool b) {\n"
               "    if ((!x && b) || (x != 0 && b)) {}\n"
               "}\n");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (style) Opposite expression on both sides of '||'.\n", errout.str());
     }
 
     void oppositeExpression() {

Original file line number	Diff line number	Diff line change
`@@ -5310,7 +5310,7 @@ class TestOther : public TestFixture {`
`5310`	`5310`	`check("void f(int* x, bool b) {\n"`
`5311`	`5311`	`" if ((!x && b) \|\| (x != 0 && b)) {}\n"`
`5312`	`5312`	`"}\n");`
`5313`		`- ASSERT_EQUALS("", errout.str());`
	`5313`	`+ ASSERT_EQUALS("[test.cpp:2]: (style) Opposite expression on both sides of '\|\|'.\n", errout.str());`
`5314`	`5314`	`}`
`5315`	`5315`
`5316`	`5316`	`void oppositeExpression() {`