uninitialized variables: added check

lib/checkother.cpp

@@ -1160,6 +1160,131 @@ void CheckOther::nullPointer()
     nullPointerConditionalAssignment();
 }
 
+static const Token *uninitvar_checkscope(const Token *tok, const unsigned int varid, bool &init)
+{
+    /* limit the checking in conditional code..
+     * int x;
+     * if (y)
+     *     x = 33;
+     * if (y)
+     *     return x;
+     */
+    bool limit = false;
+
+    for (; tok; tok = tok->next())
+    {
+        if (tok->str() == "}")
+            return 0;
+
+        // todo: handle for/while
+        if (Token::Match(tok, "for|while"))
+        {
+            init = true;
+            return 0;
+        }
+
+        if (tok->str() == "if")
+        {
+            bool canInit = false;
+            bool ifInit = true;
+            while (tok->str() == "if")
+            {
+                // goto "("
+                tok = tok->next();
+
+                // goto ")"
+                tok = tok ? tok->link() : 0;
+
+                // goto "{"
+                tok = tok ? tok->next() : 0;
+
+                if (!Token::simpleMatch(tok, "{"))
+                    return 0;
+
+                // Recursively check into the if ..
+                bool init2 = false;
+                const Token *tokerr = uninitvar_checkscope(tok->next(), varid, init2);
+                if (!limit && tokerr)
+                    return tokerr;
+
+                // if the scope didn't initialize then this whole if-chain is treated as non-initializing
+                ifInit &= init2;
+                canInit |= init2;
+
+                // goto "}"
+                tok = tok->link();
+
+                // there is no else => not initialized
+                if (Token::Match(tok, "} !!else"))
+                {
+                    ifInit = false;
+                    break;
+                }
+
+                // parse next "if"..
+                tok = tok->tokAt(2);
+                if (tok->str() == "if")
+                    continue;
+
+                // there is no "if"..
+                init2 = false;
+                tokerr = uninitvar_checkscope(tok->next(), varid, init2);
+                if (!limit && tokerr)
+                    return tokerr;
+
+                ifInit &= init2;
+                canInit |= init2;
+
+                tok = tok->link();
+                if (!tok)
+                    return 0;
+            }
+
+            if (ifInit)
+            {
+                init = true;
+                return 0;
+            }
+
+            if (canInit)
+            {
+                limit = true;
+            }
+        }
+
+        if (Token::Match(tok, "%varid% =", varid))
+        {
+            init = true;
+            return 0;
+        }
+
+        if (Token::Match(tok, "return| %varid% .|", varid))
+            return tok;
+    }
+    return 0;
+}
+
+void CheckOther::uninitvar()
+{
+    const Token *tok = _tokenizer->tokens();
+    while (0 != (tok = Token::findmatch(tok, "[{};] %type% *| %var% ;")))
+    {
+        // goto the variable
+        tok = tok->tokAt(2);
+        if (tok->str() == "*")
+            tok = tok->next();
+
+        // check that the variable id is non-zero
+        if (tok->varId() == 0)
+            continue;
+
+        bool init = false;
+        const Token *tokerr = uninitvar_checkscope(tok->next(), tok->varId(), init);
+        if (tokerr)
+            uninitvarError(tokerr, tok->str());
+    }
+}
+
 void CheckOther::checkZeroDivision()
 {
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
@@ -1301,6 +1426,11 @@ void CheckOther::nullPointerError(const Token *tok, const std::string &varname,
     reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname + " - otherwise it is redundant to check if " + varname + " is null at line " + MathLib::toString<long>(line));
 }
 
+void CheckOther::uninitvarError(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::error, "uninitvar", "Uninitialized variable: " + varname);
+}
+
 void CheckOther::zerodivError(const Token *tok)
 {
     reportError(tok, Severity::error, "zerodiv", "Division by zero");

lib/checkother.h

@@ -76,6 +76,7 @@ public:
         checkOther.strPlusChar();
         checkOther.invalidFunctionUsage();
         checkOther.checkZeroDivision();
+        checkOther.uninitvar();
     }
 
     // Casting
@@ -111,6 +112,9 @@ public:
     /** possible null pointer dereference */
     void nullPointer();
 
+    /** reading uninitialized var */
+    void uninitvar();
+
     /** Check zero division*/
     void checkZeroDivision();
 
@@ -124,6 +128,7 @@ public:
     //    haystack.remove(needle);
     void redundantCondition2();
 
+
     // Error messages..
     void cstyleCastError(const Token *tok);
     void redundantIfDelete0Error(const Token *tok);
@@ -142,6 +147,7 @@ public:
     void strPlusChar(const Token *tok);
     void nullPointerError(const Token *tok, const std::string &varname);
     void nullPointerError(const Token *tok, const std::string &varname, const int line);
+    void uninitvarError(const Token *tok, const std::string &varname);
     void zerodivError(const Token *tok);
     void postIncrementError(const Token *tok, const std::string &var_name, const bool isIncrement);
 
@@ -151,6 +157,7 @@ public:
         sprintfOverlappingDataError(0, "varname");
         udivError(0);
         nullPointerError(0, "pointer");
+        uninitvarError(0, "varname");
         zerodivError(0);
 
         // style
@@ -186,7 +193,7 @@ public:
                " * division with zero\n"
                " * null pointer dereferencing\n"
 
-               // warning
+               // style
                " * C-style pointer cast in cpp file\n"
                " * redundant if\n"
                " * bad usage of the function 'strtol'\n"
@@ -198,6 +205,7 @@ public:
                " * variable scope can be limited\n"
                " * condition that is always true/false\n"
                " * unusal pointer arithmetic. For example: \"abc\" + 'd'\n"
+               " * uninitialized variables\n"
 
                // optimisations
                " * optimisation: detect post increment/decrement\n";

test/testother.cpp

@@ -68,6 +68,8 @@ private:
         TEST_CASE(nullpointer5);    // References should not be checked
         TEST_CASE(nullpointer6);
 
+        TEST_CASE(uninitvar1);
+
         TEST_CASE(oldStylePointerCast);
 
         TEST_CASE(postIncrementDecrementStl);
@@ -910,6 +912,59 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+
+
+    void checkUninitVar(const char code[])
+    {
+        // Tokenize..
+        Tokenizer tokenizer;
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        // Clear the error buffer..
+        errout.str("");
+
+        // Check for redundant code..
+        Settings settings;
+        settings._checkCodingStyle = true;
+        CheckOther checkOther(&tokenizer, &settings, this);
+        checkOther.uninitvar();
+    }
+
+    void uninitvar1()
+    {
+        checkUninitVar("static void foo()\n"
+                       "{\n"
+                       "    Foo *p;\n"
+                       "    if (x)\n"
+                       "        p = new Foo;\n"
+                       "    p->abcd();\n"
+                       "}\n");
+        ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: p\n", errout.str());
+
+        checkUninitVar("int foo()\n"
+                       "{\n"
+                       "    int i;\n"
+                       "    if (x)\n"
+                       "        i = 22;\n"
+                       "    else\n"
+                       "        i = 33;\n"
+                       "    return i;\n"
+                       "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkUninitVar("static void foo()\n"
+                       "{\n"
+                       "    Foo *p;\n"
+                       "    if (x)\n"
+                       "        p = new Foo;\n"
+                       "    if (x)\n"
+                       "        p->abcd();\n"
+                       "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+
     void checkOldStylePointerCast(const char code[])
     {
         // Tokenize..