From 740becbddfd13a121ab1135b88f49b57d66ec4e7 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Thu, 26 Aug 2021 22:46:57 -0500
Subject: [PATCH] Fix 10348: FP knownConditionTrueFalse with condition variable
 in do ... while loop (#3422)

---
 lib/forwardanalyzer.cpp | 25 ++++++++++++++++---------
 lib/programmemory.cpp   |  9 ++++++---
 lib/valueflow.cpp       |  2 ++
 test/testcondition.cpp  | 36 ++++++++++++++++++++++++++++++++++++
 test/testvalueflow.cpp  |  1 +
 5 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp
index 25634f2c1..3dc141508 100644
--- a/lib/forwardanalyzer.cpp
+++ b/lib/forwardanalyzer.cpp
@@ -385,23 +385,27 @@ struct ForwardTraversal {
             std::tie(checkThen, checkElse) = evalCond(condTok, isDoWhile ? endBlock->previous() : nullptr);
         if (checkElse && exit)
             return Progress::Continue;
+        Analyzer::Action bodyAnalysis = analyzeScope(endBlock);
+        Analyzer::Action allAnalysis = bodyAnalysis;
+        Analyzer::Action condAnalysis;
+        if (condTok) {
+            condAnalysis = analyzeRecursive(condTok);
+            allAnalysis |= condAnalysis;
+        }
+        if (stepTok)
+            allAnalysis |= analyzeRecursive(stepTok);
+        actions |= allAnalysis;
         // do while(false) is not really a loop
-        if (checkElse && isDoWhile) {
+        if (checkElse && isDoWhile &&
+            (condTok->hasKnownIntValue() || (!bodyAnalysis.isModified() && condAnalysis.isRead()))) {
             if (updateRange(endBlock->link(), endBlock) == Progress::Break)
                 return Break();
             return updateRecursive(condTok);
         }
-        Analyzer::Action bodyAnalysis = analyzeScope(endBlock);
-        Analyzer::Action allAnalysis = bodyAnalysis;
-        if (condTok)
-            allAnalysis |= analyzeRecursive(condTok);
-        if (stepTok)
-            allAnalysis |= analyzeRecursive(stepTok);
-        actions |= allAnalysis;
         if (allAnalysis.isInconclusive()) {
             if (!analyzer->lowerToInconclusive())
                 return Break(Analyzer::Terminate::Bail);
-        } else if (allAnalysis.isModified()) {
+        } else if (allAnalysis.isModified() || (exit && allAnalysis.isIdempotent())) {
             if (!analyzer->lowerToPossible())
                 return Break(Analyzer::Terminate::Bail);
         }
@@ -417,6 +421,9 @@ struct ForwardTraversal {
         if (checkElse)
             return Progress::Continue;
         if (checkThen || isDoWhile) {
+            // Since we are re-entering the loop then assume the condition is true to update the state
+            if (exit)
+                analyzer->assume(condTok, true, Analyzer::Assume::Quiet | Analyzer::Assume::Absolute);
             if (updateInnerLoop(endBlock, stepTok, condTok) == Progress::Break)
                 return Break();
             // If loop re-enters then it could be modified again
diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp
index b3eb27c74..37141a8e1 100644
--- a/lib/programmemory.cpp
+++ b/lib/programmemory.cpp
@@ -284,9 +284,12 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok
         }
 
         if (tok2->str() == "{") {
-            if (indentlevel <= 0)
-                break;
-            --indentlevel;
+            if (indentlevel <= 0) {
+                // Keep progressing with anonymous/do scopes
+                if (!Token::Match(tok2->previous(), "do|; {"))
+                    break;
+            } else
+                --indentlevel;
             if (Token::simpleMatch(tok2->previous(), "else {"))
                 tok2 = tok2->linkAt(-2)->previous();
         }
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 247398c2f..b39cd4410 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -2383,6 +2383,8 @@ struct ValueFlowAnalyzer : Analyzer {
 
         if (isCondBlock) {
             const Token* startBlock = parent->link()->next();
+            if (Token::simpleMatch(startBlock, ";") && Token::simpleMatch(parent->tokAt(-2), "} while ("))
+                startBlock = parent->linkAt(-2);
             const Token* endBlock = startBlock->link();
             pms.removeModifiedVars(endBlock);
             if (state)
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index 45973a28f..7c2c1bb05 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -117,6 +117,8 @@ private:
         TEST_CASE(alwaysTrue);
         TEST_CASE(alwaysTrueSymbolic);
         TEST_CASE(alwaysTrueInfer);
+        TEST_CASE(alwaysTrueContainer);
+        TEST_CASE(alwaysTrueLoop);
         TEST_CASE(multiConditionAlwaysTrue);
         TEST_CASE(duplicateCondition);
 
@@ -4032,6 +4034,40 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void alwaysTrueLoop()
+    {
+        check("long foo() {\n"
+              "  bool bUpdated = false;\n"
+              "  long Ret{};\n"
+              "  do {\n"
+              "    Ret = bar();\n"
+              "    if (Ret == 0) {\n"
+              "      if (bUpdated)\n"
+              "        return 1;\n"
+              "      bUpdated = true;\n"
+              "    }\n"
+              "    else\n"
+              "      bUpdated = false;\n"
+              "  }\n"
+              "  while (bUpdated);\n"
+              "  return Ret;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("bool foo() {\n"
+              "  bool bFirst = true;\n"
+              "  do {\n"
+              "    if (bFirst)\n"
+              "      bar();\n"
+              "    if (baz())\n"
+              "      break;       \n"
+              "    bFirst = false;\n"
+              "  } while (true);\n"
+              "  return bFirst;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void multiConditionAlwaysTrue() {
         check("void f() {\n"
               "  int val = 0;\n"
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index f5923a272..4c75f55d5 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -2172,6 +2172,7 @@ private:
                "  } while (1);\n"
                "}";
         ASSERT_EQUALS(false, testValueOfX(code, 4U, 0));
+        ASSERT_EQUALS(true, testValueOfX(code, 4U, 3));
 
         // pointer/reference to x
         code = "int f(void) {\n"