Fix 10348: FP knownConditionTrueFalse with condition variable in do ... while loop (#3422)

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

lib/programmemory.cpp

@@ -284,8 +284,11 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok
         }
 
         if (tok2->str() == "{") {
-            if (indentlevel <= 0)
+            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();

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)

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"

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"