From fb9d659e257d718b0f21bf179c0786b7ef6fc2a1 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Sat, 3 Jul 2021 02:12:26 -0500
Subject: [PATCH] Fix 10326: Regression: ValueFlow; Wrong Uninit value after do
 while (#3312)

---
 lib/forwardanalyzer.cpp  | 31 +++++++++++++++++++++++++------
 test/testnullpointer.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
 test/testuninitvar.cpp   | 11 +++++++++++
 3 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp
index dbdb63901..aa54f245d 100644
--- a/lib/forwardanalyzer.cpp
+++ b/lib/forwardanalyzer.cpp
@@ -374,10 +374,23 @@ struct ForwardTraversal {
                         Token* endBlock,
                         Token* condTok,
                         Token* initTok = nullptr,
-                        Token* stepTok = nullptr) {
+                        Token* stepTok = nullptr,
+                        bool exit = false) {
         if (initTok && updateRecursive(initTok) == Progress::Break)
             return Break();
         const bool isDoWhile = precedes(endBlock, condTok);
+        bool checkThen = true;
+        bool checkElse = false;
+        if (condTok && !Token::simpleMatch(condTok, ":"))
+            std::tie(checkThen, checkElse) = evalCond(condTok, isDoWhile ? endBlock->link()->previous() : nullptr);
+        if (checkElse && exit)
+            return Progress::Continue;
+        // do while(false) is not really a loop
+        if (checkElse && isDoWhile) {
+            if (updateRange(endBlock->link(), endBlock) == Progress::Break)
+                return Break();
+            return updateRecursive(condTok);
+        }
         Analyzer::Action bodyAnalysis = analyzeScope(endBlock);
         Analyzer::Action allAnalysis = bodyAnalysis;
         if (condTok)
@@ -392,14 +405,11 @@ struct ForwardTraversal {
             if (!analyzer->lowerToPossible())
                 return Break(Analyzer::Terminate::Bail);
         }
-        bool checkThen = true;
-        bool checkElse = false;
+
         if (condTok && !Token::simpleMatch(condTok, ":")) {
             if (!isDoWhile || (!bodyAnalysis.isModified() && !bodyAnalysis.isIdempotent()))
                 if (updateRecursive(condTok) == Progress::Break)
                     return Break();
-            // TODO: Check cond first before lowering
-            std::tie(checkThen, checkElse) = evalCond(condTok, isDoWhile ? endBlock->link()->previous() : nullptr);
         }
         // condition is false, we don't enter the loop
         if (checkElse)
@@ -442,6 +452,15 @@ struct ForwardTraversal {
         return Progress::Continue;
     }
 
+    Progress updateLoopExit(const Token* endToken,
+                            Token* endBlock,
+                            Token* condTok,
+                            Token* initTok = nullptr,
+                            Token* stepTok = nullptr)
+    {
+        return updateLoop(endToken, endBlock, condTok, initTok, stepTok, true);
+    }
+
     Progress updateScope(Token* endBlock) {
         if (forked)
             analyzer->forkScope(endBlock);
@@ -528,7 +547,7 @@ struct ForwardTraversal {
                 } else if (scope->type == Scope::eLambda) {
                     return Break();
                 } else if (scope->type == Scope::eDo && Token::simpleMatch(tok, "} while (")) {
-                    if (updateLoop(end, tok, tok->tokAt(2)->astOperand2()) == Progress::Break)
+                    if (updateLoopExit(end, tok, tok->tokAt(2)->astOperand2()) == Progress::Break)
                         return Break();
                     tok = tok->linkAt(2);
                 } else if (Token::simpleMatch(tok->next(), "else {")) {
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
index 1df8354da..dbfb28acd 100644
--- a/test/testnullpointer.cpp
+++ b/test/testnullpointer.cpp
@@ -114,6 +114,7 @@ private:
         TEST_CASE(nullpointer71); // #10178
         TEST_CASE(nullpointer72); // #10215
         TEST_CASE(nullpointer73); // #10321
+        TEST_CASE(nullpointer74);
         TEST_CASE(nullpointer_addressOf); // address of
         TEST_CASE(nullpointerSwitch); // #2626
         TEST_CASE(nullpointer_cast); // #4692
@@ -2289,6 +2290,45 @@ private:
         ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:10]: (warning) Either the condition 'ptr!=nullptr' is redundant or there is possible null pointer dereference: ptr.\n", errout.str());
     }
 
+    void nullpointer74() {
+        check("struct d {\n"
+              "  d* e();\n"
+              "};\n"
+              "void g(d* f) {\n"
+              "  do {\n"
+              "    f = f->e();\n"
+              "    if (f) {}\n"
+              "  } while (0);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("struct d {\n"
+              "  d* e();\n"
+              "};\n"
+              "void g(d* f, int i) {\n"
+              "  do {\n"
+              "    i--;\n"
+              "    f = f->e();\n"
+              "    if (f) {}\n"
+              "  } while (i > 0);\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:8] -> [test.cpp:7]: (warning) Either the condition 'f' is redundant or there is possible null pointer dereference: f.\n",
+            errout.str());
+
+        check("struct d {\n"
+              "  d* e();\n"
+              "};\n"
+              "void g(d* f, int i) {\n"
+              "  do {\n"
+              "    i--;\n"
+              "    f = f->e();\n"
+              "    if (f) {}\n"
+              "  } while (f && i > 0);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void nullpointer_addressOf() { // address of
         check("void f() {\n"
               "  struct X *x = 0;\n"
diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp
index 137dd5f4e..09c22a281 100644
--- a/test/testuninitvar.cpp
+++ b/test/testuninitvar.cpp
@@ -4845,6 +4845,17 @@ private:
                         "}\n");
         ASSERT_EQUALS("", errout.str());
 
+        // #10326
+        valueFlowUninit("void foo() {\n"
+                        "    int cnt;\n"
+                        "    do {\n"
+                        "        cnt = 32 ;\n"
+                        "    }\n"
+                        "    while ( 0 ) ;\n"
+                        "    if (cnt != 0) {}\n"
+                        "}\n");
+        ASSERT_EQUALS("", errout.str());
+
         // #10327 - avoid extra warnings for uninitialized variable
         valueFlowUninit("void dowork( int me ) {\n"
                         "    if ( me == 0 ) {}\n" // <- not uninitialized