fix #13227: False positive: legacy uninitvar (danmar#6936)

Co-authored-by: Ludvig Gunne Lindström <[email protected]>

Author: ludviggunne

lib/checkuninitvar.cpp

@@ -916,7 +916,7 @@ bool CheckUninitVar::checkIfForWhileHead(const Token *startparentheses, const Va
 }
 
 /** recursively check loop, return error token */
-const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Variable& var, const Alloc alloc, const std::string &membervar, bool &bailout) const
+const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Variable& var, const Alloc alloc, const std::string &membervar, bool &bailout, bool &alwaysReturns) const
 {
     assert(start->str() == "{");
 
@@ -941,9 +941,10 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
             if (!Token::simpleMatch(top->previous(), "for (") || !Token::simpleMatch(top->link(), ") {"))
                 continue;
             const Token *bodyStart = top->link()->next();
-            const Token *errorToken1 = checkLoopBodyRecursive(bodyStart, var, alloc, membervar, bailout);
+            const Token *errorToken1 = checkLoopBodyRecursive(bodyStart, var, alloc, membervar, bailout, alwaysReturns);
             if (!errorToken)
                 errorToken = errorToken1;
+            bailout |= alwaysReturns;
             if (bailout)
                 return nullptr;
         }
@@ -962,11 +963,12 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
                 return nullptr;
             }
 
-            const Token *errorToken1 = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout);
+            bool alwaysReturnsUnused;
+            const Token *errorToken1 = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout, alwaysReturnsUnused);
             tok = tok->link();
             if (Token::simpleMatch(tok, "} else {")) {
                 const Token *elseBody = tok->tokAt(2);
-                const Token *errorToken2 = checkLoopBodyRecursive(elseBody, var, alloc, membervar, bailout);
+                const Token *errorToken2 = checkLoopBodyRecursive(elseBody, var, alloc, membervar, bailout, alwaysReturnsUnused);
                 tok = elseBody->link();
                 if (errorToken1 && errorToken2)
                     return errorToken1;
@@ -979,6 +981,23 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
                 errorToken = errorToken1;
         }
 
+        if (Token::simpleMatch(tok, "return")) {
+            bool returnWithoutVar = true;
+            while (tok && !Token::simpleMatch(tok, ";")) {
+                if (tok->isVariable() && tok->variable() == &var) {
+                    returnWithoutVar = false;
+                    break;
+                }
+                tok = tok->next();
+            }
+            if (returnWithoutVar) {
+                alwaysReturns = true;
+                return nullptr;
+            }
+        }
+        if (!tok)
+            break;
+
         if (tok->varId() != var.declarationId())
             continue;
 
@@ -1068,17 +1087,18 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
 bool CheckUninitVar::checkLoopBody(const Token *tok, const Variable& var, const Alloc alloc, const std::string &membervar, const bool suppressErrors)
 {
     bool bailout = false;
-    const Token *errorToken = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout);
+    bool alwaysReturns = false;
+    const Token *errorToken = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout, alwaysReturns);
 
-    if (!suppressErrors && !bailout && errorToken) {
+    if (!suppressErrors && !bailout && !alwaysReturns && errorToken) {
         if (membervar.empty())
             uninitvarError(errorToken, errorToken->expressionString(), alloc);
         else
             uninitStructMemberError(errorToken, errorToken->expressionString() + "." + membervar);
         return true;
     }
 
-    return bailout;
+    return bailout || alwaysReturns;
 }
 
 void CheckUninitVar::checkRhs(const Token *tok, const Variable &var, Alloc alloc, nonneg int number_of_if, const std::string &membervar)

lib/checkuninitvar.h

@@ -96,7 +96,7 @@ class CPPCHECKLIB CheckUninitVar : public Check {
     const Token* checkExpr(const Token* tok, const Variable& var, Alloc alloc, bool known, bool* bailout = nullptr) const;
     bool checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, Alloc alloc, const std::string &membervar);
     bool checkLoopBody(const Token *tok, const Variable& var, Alloc alloc, const std::string &membervar, bool suppressErrors);
-    const Token* checkLoopBodyRecursive(const Token *start, const Variable& var, Alloc alloc, const std::string &membervar, bool &bailout) const;
+    const Token* checkLoopBodyRecursive(const Token *start, const Variable& var, Alloc alloc, const std::string &membervar, bool &bailout, bool &alwaysReturns) const;
     void checkRhs(const Token *tok, const Variable &var, Alloc alloc, nonneg int number_of_if, const std::string &membervar);
     static int isFunctionParUsage(const Token *vartok, const Library &library, bool pointer, Alloc alloc, int indirect = 0);
     int isFunctionParUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect = 0) const;

test/testuninitvar.cpp

@@ -57,6 +57,7 @@ class TestUninitVar : public TestFixture {
         TEST_CASE(uninitvar3);          // #3844
         TEST_CASE(uninitvar4);          // #3869 (reference)
         TEST_CASE(uninitvar5);          // #3861
+        TEST_CASE(uninitvar6);          // #13227
         TEST_CASE(uninitvar2_func);     // function calls
         TEST_CASE(uninitvar2_value);    // value flow
         TEST_CASE(valueFlowUninit2_value);
@@ -3071,6 +3072,22 @@ class TestUninitVar : public TestFixture {
         ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: c\n", errout_str());
     }
 
+    // #13227
+    void uninitvar6() {
+        checkUninitVar("int foo(int x) {\n"
+                       "    int i;\n"
+                       "    if (x == 1) {\n"
+                       "        i = 3;\n"
+                       "    } else {\n"
+                       "        do {\n"
+                       "            return 2;\n"
+                       "        } while (0);\n"
+                       "    }\n"
+                       "    return i;\n"
+                       "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void uninitvar8() {
         const char code[] = "struct Fred {\n"
                             "    void Sync(dsmp_t& type, int& len, int limit = 123);\n"