memleak: implemented simple checking for leaking struct members

src/checkmemoryleak.cpp

@@ -33,6 +33,7 @@ namespace
 {
 CheckMemoryLeakInFunction instance1;
 CheckMemoryLeakInClass instance2;
+CheckMemoryLeakStructMember instance3;
 }
 
 //---------------------------------------------------------------------------
@@ -1921,7 +1922,138 @@ void CheckMemoryLeakInClass::variable(const char classname[], const Token *tokVa
 
 
 
-
+void CheckMemoryLeakStructMember::check()
+{
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        // Locate struct variables..
+        if (Token::Match(tok, "struct|;|{|} %type% * %var% [=;]"))
+        {
+            const Token *vartok = tok->tokAt(3);
+            if (vartok->varId() == 0)
+                continue;
+
+            // Check that struct is allocated..
+            {
+                const unsigned int varid(vartok->varId());
+                bool alloc = false;
+                unsigned int indentlevel2 = 0;
+                for (const Token *tok2 = vartok; tok2; tok2 = tok2->next())
+                {
+                    if (tok2->str() == "{")
+                        ++indentlevel2;
+                    else if (tok2->str() == "}")
+                    {
+                        if (indentlevel2 == 0)
+                            break;
+                        --indentlevel2;
+                    }
+                    else if (Token::Match(tok2, "= %varid% [;=]", varid))
+                    {
+                        alloc = false;
+                        break;
+                    }
+                    else if (Token::Match(tok2, "%varid% = malloc|kmalloc (", varid))
+                    {
+                        alloc = true;
+                    }
+                }
+                if (!alloc)
+                    continue;
+            }
+
+            // Check struct..
+            unsigned int indentlevel2 = 0;
+            for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
+            {
+                if (tok2->str() == "{")
+                    ++indentlevel2;
+
+                else if (tok2->str() == "}")
+                {
+                    if (indentlevel2 == 0)
+                        break;
+                    --indentlevel2;
+                }
+
+                // Struct member is allocated => check if it is also properly deallocated..
+                else if (Token::Match(tok2, "%varid% . %var% = malloc|strdup|kmalloc (", vartok->varId()))
+                {
+                    const unsigned int structid(vartok->varId());
+                    const unsigned int structmemberid(tok2->tokAt(2)->varId());
+
+                    // This struct member is allocated.. check that it is deallocated
+                    unsigned int indentlevel3 = indentlevel2;
+                    for (const Token *tok3 = tok2; tok3; tok3 = tok3->next())
+                    {
+                        if (tok3->str() == "{")
+                            ++indentlevel3;
+
+                        else if (tok3->str() == "}")
+                        {
+                            if (indentlevel3 == 0)
+                            {
+                                memoryLeak(tok3, (vartok->str() + "." + tok2->strAt(2)).c_str(), Malloc, false);
+                                break;
+                            }
+                            --indentlevel3;
+                        }
+
+                        // Deallocating the struct member..
+                        else if (Token::Match(tok3, "free|kfree ( %var% . %varid% )", structmemberid))
+                            break;
+
+                        // Deallocating the struct..
+                        else if (Token::Match(tok3, "free|kfree ( %varid% )", structid))
+                        {
+                            memoryLeak(tok3, (vartok->str() + "." + tok2->strAt(2)).c_str(), Malloc, false);
+                            break;
+                        }
+
+                        // failed allocation => skip code..
+                        else if (Token::Match(tok3, "if ( ! %var% . %varid% )", structmemberid))
+                        {
+                            // Goto the ")"
+                            while (tok3->str() != ")")
+                                tok3 = tok3->next();
+
+                            // Skip block..
+                            unsigned int indentlevel = 0;
+                            while (tok3)
+                            {
+                                if (tok3->str() == "{")
+                                    ++indentlevel;
+
+                                else if (tok3->str() == "}")
+                                {
+                                    if (indentlevel <= 1)
+                                        break;
+                                    --indentlevel;
+                                }
+                                tok3 = tok3->next();
+                            }
+                        }
+
+                        // Returning from function..
+                        else if (tok3->str() == "return")
+                        {
+                            // Returning from function without deallocating struct member?
+                            if (!Token::Match(tok3, "return %varid% ;", structid))
+                            {
+                                memoryLeak(tok3, (vartok->str() + "." + tok2->strAt(2)).c_str(), Malloc, false);
+                            }
+                            break;
+                        }
+
+                        // goto isn't handled well.. bail out even though there might be leaks
+                        else if (tok3->str() == "goto")
+                            break;
+                    }
+                }
+            }
+        }
+    }
+}
 
 
 

src/checkmemoryleak.h

@@ -85,9 +85,22 @@ public:
     enum AllocType { No, Malloc, gMalloc, New, NewArray, File, Fd, Pipe, Dir, Many };
 
     void memoryLeak(const Token *tok, const char varname[], AllocType alloctype, bool all);
+
+    /**
+     * Get type of deallocation at given position
+     */
     AllocType getDeallocationType(const Token *tok, const char *varnames[]);
+
+    /**
+     * Get type of allocation at given position
+     */
     AllocType getAllocationType(const Token *tok2) const;
+
+    /**
+     * Get type of reallocation at given position
+     */
     AllocType getReallocationType(const Token *tok2);
+
     bool isclass(const Tokenizer *_tokenizer, const Token *typestr) const;
 
     void memleakError(const Token *tok, const std::string &varname);
@@ -295,10 +308,7 @@ public:
         checkMemoryLeak.check();
     }
 
-    void check()
+    void check();
-    {
-        /** @todo implement this */
-    }
 
 private:
 

test/testmemleak.cpp

@@ -2828,17 +2828,130 @@ private:
 
     void run()
     {
-        TEST_CASE(test1);
+        // testing that errors are detected
+        TEST_CASE(err);
+
+        // handle / bail out when "goto" is found
+        TEST_CASE(goto_);
+
+        // Don't report errors if the struct is returned
+        TEST_CASE(ret);
+
+        // assignments
+        TEST_CASE(assign);
+
+        // Failed allocation
+        TEST_CASE(failedAllocation);
     }
 
-    void test1()
+    void err()
     {
         check("static void foo()\n"
               "{\n"
-              "    struct ABC abc;\n"
+              "    struct ABC *abc = malloc(sizeof(struct ABC));\n"
-              "    abc.a = malloc(10);\n"
+              "    abc->a = malloc(10);\n"
+              "    free(abc);\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Memory leak: abc.a\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5]: (error) Memory leak: abc.a\n", errout.str());
+
+        check("static void foo()\n"
+              "{\n"
+              "    struct ABC *abc = malloc(sizeof(struct ABC));\n"
+              "    abc->a = malloc(10);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Memory leak: abc.a\n", errout.str());
+
+        check("static ABC * foo()\n"
+              "{\n"
+              "    ABC *abc = malloc(sizeof(ABC));\n"
+              "    abc->a = malloc(10);\n"
+              "    abc->b = malloc(10);\n"
+              "    if (abc->b == 0)\n"
+              "    {\n"
+              "        return 0;\n"
+              "    }\n"
+              "    return abc;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:8]: (error) Memory leak: abc.a\n", errout.str());
+    }
+
+    void goto_()
+    {
+        check("static void foo()\n"
+              "{\n"
+              "    struct ABC *abc = malloc(sizeof(struct ABC));\n"
+              "    abc->a = malloc(10);\n"
+              "    if (abc->a)\n"
+              "    { goto out; }\n"
+              "    free(abc);\n"
+              "    return;\n"
+              "out:\n"
+              "    free(abc->a);\n"
+              "    free(abc);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void ret()
+    {
+        check("static ABC * foo()\n"
+              "{\n"
+              "    struct ABC *abc = malloc(sizeof(struct ABC));\n"
+              "    abc->a = malloc(10);\n"
+              "    return abc;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("static void foo(struct ABC *abc)\n"
+              "{\n"
+              "    abc->a = malloc(10);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void assign()
+    {
+        check("static void foo()\n"
+              "{\n"
+              "    struct ABC *abc = abc1;\n"
+              "    abc->a = malloc(10);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("static void foo()\n"
+              "{\n"
+              "    struct ABC *abc;\n"
+              "    abc1 = abc = malloc(sizeof(ABC));\n"
+              "    abc->a = malloc(10);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+
+        check("static void foo()\n"
+              "{\n"
+              " struct msn_entry *ptr;\n"
+              " ptr = malloc(sizeof(struct msn_entry));\n"
+              " ptr->msn = malloc(100);\n"
+              " back = ptr;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+    }
+
+    void failedAllocation()
+    {
+        check("static struct ABC * foo()\n"
+              "{\n"
+              "    struct ABC *abc = malloc(sizeof(struct ABC));\n"
+              "    abc->a = malloc(10);\n"
+              "    if (!abc->a)\n"
+              "    {\n"
+              "        free(abc);\n"
+              "        return 0;\n"
+              "    }\n"
+              "    return abc;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 };