Fix issue 8825: ValueFlow: uninitialized struct member (#2087)

* Pass uninit value across pointers

* Add more testing

lib/astutils.cpp

@@ -995,20 +995,10 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
     return arg && !arg->isConst() && arg->isReference();
 }
 
-bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth)
+bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int depth)
 {
-    if (!precedes(start, end))
+    if (!tok)
         return false;
-    if (depth < 0)
-        return true;
-    for (const Token *tok = start; tok != end; tok = tok->next()) {
-        if (tok->varId() != varid) {
-            if (globalvar && Token::Match(tok, "%name% ("))
-                // TODO: Is global variable really changed by function call?
-                return true;
-            continue;
-        }
-
     const Token *tok2 = tok;
     while (Token::simpleMatch(tok2->astParent(), "*") || (Token::simpleMatch(tok2->astParent(), ".") && !Token::simpleMatch(tok2->astParent()->astParent(), "(")) ||
            (Token::simpleMatch(tok2->astParent(), "[") && tok2 == tok2->astParent()->astOperand1()))
@@ -1048,7 +1038,7 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va
         if (!isConst && (!fun || !fun->isConst()))
             return true;
         else
-                continue;
+            return false;
     }
 
     const Token *ftok = tok2;
@@ -1075,14 +1065,32 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va
     if (Token::simpleMatch(tok2->astParent(), ":") && tok2->astParent()->astParent() && Token::simpleMatch(tok2->astParent()->astParent()->previous(), "for (")) {
         const Token * varTok = tok2->astParent()->previous();
         if (!varTok)
-                continue;
+            return false;
         const Variable * loopVar = varTok->variable();
         if (!loopVar)
-                continue;
+            return false;
         if (!loopVar->isConst() && loopVar->isReference() && isVariableChanged(loopVar, settings, cpp, depth - 1))
             return true;
+        return false;
+    }
+    return false;
+}
+
+bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth)
+{
+    if (!precedes(start, end))
+        return false;
+    if (depth < 0)
+        return true;
+    for (const Token *tok = start; tok != end; tok = tok->next()) {
+        if (tok->varId() != varid) {
+            if (globalvar && Token::Match(tok, "%name% ("))
+                // TODO: Is global variable really changed by function call?
+                return true;
             continue;
         }
+        if (isVariableChanged(tok, settings, cpp, depth))
+            return true;
     }
     return false;
 }

lib/astutils.h

@@ -135,6 +135,8 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
 /** Is variable changed in block of code? */
 bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
 
+bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int depth = 20);
+
 bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, int depth = 20);
 
 bool isAliased(const Variable *var);

lib/checkuninitvar.cpp

@@ -1269,9 +1269,11 @@ void CheckUninitVar::uninitdataError(const Token *tok, const std::string &varnam
     reportError(tok, Severity::error, "uninitdata", "$symbol:" + varname + "\nMemory is allocated but not initialized: $symbol", CWE908, false);
 }
 
-void CheckUninitVar::uninitvarError(const Token *tok, const std::string &varname)
+void CheckUninitVar::uninitvarError(const Token *tok, const std::string &varname, ErrorPath errorPath)
 {
-    reportError(tok, Severity::error, "uninitvar", "$symbol:" + varname + "\nUninitialized variable: $symbol", CWE908, false);
+    errorPath.emplace_back(tok, "");
+    reportError(errorPath, Severity::error, "uninitvar", "$symbol:" + varname + "\nUninitialized variable: $symbol", CWE908, false);
+    // reportError(tok, Severity::error, "uninitvar", "$symbol:" + varname + "\nUninitialized variable: $symbol", CWE908, false);
 }
 
 void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string &membername)
@@ -1295,14 +1297,20 @@ void CheckUninitVar::valueFlowUninit()
                 tok = tok->linkAt(1);
                 continue;
             }
-            if (!tok->variable() || tok->values().size() != 1U)
+            if (!tok->variable())
                 continue;
-            const ValueFlow::Value &v = tok->values().front();
+            if (Token::simpleMatch(tok->astParent(), ".") && tok->astParent()->astOperand1() == tok)
-            if (v.valueType != ValueFlow::Value::ValueType::UNINIT || v.isInconclusive())
                 continue;
-            if (!isVariableUsage(tok, tok->variable()->isPointer(), NO_ALLOC))
+            auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue));
+            if (v == tok->values().end())
                 continue;
-            uninitvarError(tok, tok->str());
+            if (v->isInconclusive())
+                continue;
+            if (!isVariableUsage(tok, tok->variable()->isPointer(), tok->variable()->isArray() ? ARRAY : NO_ALLOC))
+                continue;
+            if (isVariableChanged(tok, mSettings, mTokenizer->isCPP()))
+                continue;
+            uninitvarError(tok, tok->str(), v->errorPath);
         }
     }
 }

lib/checkuninitvar.h

@@ -109,7 +109,12 @@ public:
 
     void uninitstringError(const Token *tok, const std::string &varname, bool strncpy_);
     void uninitdataError(const Token *tok, const std::string &varname);
-    void uninitvarError(const Token *tok, const std::string &varname);
+    void uninitvarError(const Token *tok, const std::string &varname, ErrorPath errorPath);
+    void uninitvarError(const Token *tok, const std::string &varname)
+    {
+        ErrorPath errorPath;
+        uninitvarError(tok, varname, errorPath);
+    }
     void uninitvarError(const Token *tok, const std::string &varname, Alloc alloc) {
         if (alloc == NO_CTOR_CALL || alloc == CTOR_CALL)
             uninitdataError(tok, varname);

lib/valueflow.cpp

@@ -399,10 +399,6 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
     if (!tok->addValue(value))
         return;
 
-    // Don't set parent for uninitialized values
-    if (value.isUninitValue())
-        return;
-
     Token *parent = tok->astParent();
     if (!parent)
         return;
@@ -459,6 +455,14 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
         return;
     }
 
+    if (value.isUninitValue()) {
+        if (parent->isUnaryOp("&"))
+            setTokenValue(parent, value, settings);
+        else if (Token::Match(parent, ". %var%") && parent->astOperand1() == tok)
+            setTokenValue(parent->astOperand2(), value, settings);
+        return;
+    }
+
     // cast..
     if (const Token *castType = getCastTypeStartToken(parent)) {
         const ValueType &valueType = ValueType::parseDecl(castType, settings);
@@ -4987,8 +4991,8 @@ static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatab
             pointer |= vardecl->str() == "*";
             vardecl = vardecl->next();
         }
-        if (!stdtype && !pointer)
+        // if (!stdtype && !pointer)
-            continue;
+            // continue;
         if (!Token::Match(vardecl, "%var% ;"))
             continue;
         if (Token::Match(vardecl, "%varid% ; %varid% =", vardecl->varId()))
@@ -4999,6 +5003,8 @@ static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatab
         if ((!var->isPointer() && var->type() && var->type()->needInitialization != Type::NeedInitialization::True) ||
             !var->isLocal() || var->isStatic() || var->isExtern() || var->isReference() || var->isThrow())
             continue;
+        if (!var->type() && !stdtype && !pointer)
+            continue;
 
         ValueFlow::Value uninitValue;
         uninitValue.setKnown();

test/testuninitvar.cpp

@@ -81,6 +81,7 @@ private:
         TEST_CASE(syntax_error); // Ticket #5073
         TEST_CASE(trac_5970);
         TEST_CASE(valueFlowUninit);
+        TEST_CASE(uninitvar_ipa);
 
         TEST_CASE(isVariableUsageDeref); // *p
 
@@ -3978,6 +3979,57 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void uninitvar_ipa() {
+        // #8825
+        valueFlowUninit("typedef struct  {\n"
+                        "    int flags;\n"
+                        "} someType_t;\n"
+                        "void bar(const someType_t * const p)  {\n"
+                        "    if( (p->flags & 0xF000) == 0xF000){}\n"
+                        "}\n"
+                        "void f(void) {\n"
+                        "    someType_t gVar;\n"
+                        "    bar(&gVar);\n"
+                        "}\n");
+        ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5]: (error) Uninitialized variable: flags\n", errout.str());
+
+        valueFlowUninit("typedef struct \n"
+                        "{\n"
+                        "        int flags[3];\n"
+                        "} someType_t;\n"
+                        "void f(void) {\n"
+                        "        someType_t gVar;\n"
+                        "        if(gVar.flags[1] == 42){}\n"
+                        "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: flags\n", errout.str());
+
+        valueFlowUninit("void f(bool * x) {\n"
+                        "    *x = false;\n"
+                        "}\n"
+                        "void g() {\n"
+                        "    bool b;\n"
+                        "    f(&b);\n"
+                        "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("struct A { bool b; };"
+                        "void f(A * x) {\n"
+                        "    x->b = false;\n"
+                        "}\n"
+                        "void g() {\n"
+                        "    A b;\n"
+                        "    f(&b);\n"
+                        "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("std::string f() {\n"
+                        "    std::ostringstream ostr;\n"
+                        "    ostr << \"\";\n"
+                        "    return ostr.str();\n"
+                        "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void isVariableUsageDeref() {
         // *p
         checkUninitVar("void f() {\n"