From ddaea3244dff485b5f683ca54d5ef90aee2b4943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 19 Jul 2009 16:51:31 +0200 Subject: [PATCH] memleak: implemented simple checking for leaking struct members --- src/checkmemoryleak.cpp | 134 +++++++++++++++++++++++++++++++++++++++- src/checkmemoryleak.h | 18 ++++-- test/testmemleak.cpp | 123 ++++++++++++++++++++++++++++++++++-- 3 files changed, 265 insertions(+), 10 deletions(-) diff --git a/src/checkmemoryleak.cpp b/src/checkmemoryleak.cpp index 3ba113228..b6fc64a2c 100644 --- a/src/checkmemoryleak.cpp +++ b/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; + } + } + } + } + } +} diff --git a/src/checkmemoryleak.h b/src/checkmemoryleak.h index d972a3ff1..9f6921bdb 100644 --- a/src/checkmemoryleak.h +++ b/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() - { - /** @todo implement this */ - } + void check(); private: diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index dd65a507d..8a81fe521 100644 --- a/test/testmemleak.cpp +++ b/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" - " abc.a = malloc(10);\n" + " struct ABC *abc = malloc(sizeof(struct ABC));\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()); } };