Fix 11996: FP: knownConditionTrueFalse (danmar#6357)

chrchr-github · May 4, 2024 · 665cfce · 665cfce
1 parent 0f42d81
commit 665cfce
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 17 deletions.
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -815,6 +815,11 @@ static T* getCondTokImpl(T* tok)
         return nullptr;
     if (Token::simpleMatch(tok, "("))
         return getCondTok(tok->previous());
+    if (Token::simpleMatch(tok, "do {")) {
+        T* endTok = tok->linkAt(1);
+        if (Token::simpleMatch(endTok, "} while ("))
+            return endTok->tokAt(2)->astOperand2();
+    }
     if (Token::simpleMatch(tok, "for") && Token::simpleMatch(tok->next()->astOperand2(), ";") &&
         tok->next()->astOperand2()->astOperand2())
         return tok->next()->astOperand2()->astOperand2()->astOperand1();
@@ -831,6 +836,8 @@ static T* getCondTokFromEndImpl(T* endBlock)
     T* startBlock = endBlock->link();
     if (!Token::simpleMatch(startBlock, "{"))
         return nullptr;
+    if (Token::simpleMatch(startBlock->previous(), "do"))
+        return getCondTok(startBlock->previous());
     if (Token::simpleMatch(startBlock->previous(), ")"))
         return getCondTok(startBlock->previous()->link());
     if (Token::simpleMatch(startBlock->tokAt(-2), "} else {"))

diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp
@@ -605,10 +605,10 @@ namespace {
                     const Scope* scope = tok->scope();
                     if (!scope)
                         return Break();
-                    if (Token::Match(tok->link()->previous(), ")|else {")) {
-                        const Token* tok2 = tok->link()->previous();
-                        const bool inElse = Token::simpleMatch(tok2, "else {");
-                        const bool inLoop = inElse ? false : Token::Match(tok2->link()->previous(), "while|for (");
+                    if (contains({Scope::eDo, Scope::eFor, Scope::eWhile, Scope::eIf, Scope::eElse}, scope->type)) {
+                        const bool inElse = scope->type == Scope::eElse;
+                        const bool inDoWhile = scope->type == Scope::eDo;
+                        const bool inLoop = contains({Scope::eDo, Scope::eFor, Scope::eWhile}, scope->type);
                         Token* condTok = getCondTokFromEnd(tok);
                         if (!condTok)
                             return Break();
@@ -637,19 +637,14 @@ namespace {
                             }
                         }
                         analyzer->assume(condTok, !inElse, Analyzer::Assume::Quiet);
-                        if (Token::simpleMatch(tok, "} else {"))
+                        assert(!inDoWhile || Token::simpleMatch(tok, "} while ("));
+                        if (Token::simpleMatch(tok, "} else {") || inDoWhile)
                             tok = tok->linkAt(2);
-                    } else if (scope->type == Scope::eTry) {
+                    } else if (contains({Scope::eTry, Scope::eCatch}, scope->type)) {
                         if (!analyzer->lowerToPossible())
                             return Break(Analyzer::Terminate::Bail);
                     } else if (scope->type == Scope::eLambda) {
                         return Break();
-                    } else if (scope->type == Scope::eDo && Token::simpleMatch(tok, "} while (")) {
-                        if (updateLoopExit(end, tok, tok->tokAt(2)->astOperand2()) == Progress::Break)
-                            return Break();
-                        tok = tok->linkAt(2);
-                    } else if (Token::simpleMatch(tok->next(), "else {")) {
-                        tok = tok->linkAt(2);
                     }
                 } else if (tok->isControlFlowKeyword() && Token::Match(tok, "if|while|for (") &&
                            Token::simpleMatch(tok->next()->link(), ") {")) {

diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp
@@ -106,6 +106,17 @@ namespace {
             return top;
         }
 
+        static Token* jumpToStart(Token* tok)
+        {
+            if (Token::simpleMatch(tok->tokAt(-2), "} else {"))
+                tok = tok->linkAt(-2);
+            if (Token::simpleMatch(tok->previous(), ") {"))
+                return tok->previous()->link();
+            if (Token::simpleMatch(tok->previous(), "do {"))
+                return tok->previous();
+            return tok;
+        }
+
         bool updateRecursive(Token* start) {
             bool continueB = true;
             visitAstNodes(start, [&](Token* tok) {
@@ -326,7 +337,7 @@ namespace {
                     // If the condition modifies the variable then bail
                     if (condAction.isModified())
                         break;
-                    tok = condTok->astTop()->previous();
+                    tok = jumpToStart(tok->link());
                     continue;
                 }
                 if (tok->str() == "{") {
@@ -343,10 +354,7 @@ namespace {
                         if (r.action.isModified())
                             break;
                     }
-                    if (Token::simpleMatch(tok->tokAt(-2), "} else {"))
-                        tok = tok->linkAt(-2);
-                    if (Token::simpleMatch(tok->previous(), ") {"))
-                        tok = tok->previous()->link();
+                    tok = jumpToStart(tok);
                     continue;
                 }
                 if (Token* next = isUnevaluated(tok)) {

diff --git a/test/testcondition.cpp b/test/testcondition.cpp
@@ -5263,6 +5263,17 @@ class TestCondition : public TestFixture {
               "    for (int i = 1000; i < 20; ++i) {}\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'i<20' is always false\n", errout_str());
+
+        check("int foo(int foo, int bar, bool baz, bool flag) {\n"
+              "    if (baz && flag) {\n"
+              "        do {\n"
+              "            if (bar==42)\n"
+              "                return 0;\n"
+              "        } while (flag);\n"
+              "    }\n"
+              "    return foo;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:6]: (style) Condition 'flag' is always true\n", errout_str());
     }
 
     void alwaysTrueTryCatch()