From 626a814241966c9c8b726ba293153e2ccd4df50c Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Sun, 5 Dec 2010 20:26:52 +0100 Subject: [PATCH] Symbol database: better handling of inline functions. Ticket: #2219 --- lib/checkmemoryleak.cpp | 8 +- test/testmemleak.cpp | 295 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 293 insertions(+), 10 deletions(-) diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index d3107ad74..61391c3bb 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -2786,17 +2786,15 @@ void CheckMemoryLeakInClass::checkPublicFunctions(const SymbolDatabase::SpaceInf // If they allocate member variables, they should also deallocate std::list::const_iterator func; - // TODO: parse into any function scope that is not a constructor for (func = spaceinfo->functionList.begin(); func != spaceinfo->functionList.end(); ++func) { - /** @todo false negative: why do we only check inline functions? */ - if (func->access == SymbolDatabase::Public && func->hasBody && func->isInline) + if (func->type != SymbolDatabase::Func::Constructor && + func->access == SymbolDatabase::Public && func->hasBody) { const Token *tok2 = func->token; while (tok2->str() != "{") tok2 = tok2->next(); - /** @todo false negative: why do we only check for this specific case? */ - if (Token::Match(tok2, "{ %varid% =", varid)) + if (Token::Match(tok2, "{|}|; %varid% =", varid)) { const CheckMemoryLeak::AllocType alloc = getAllocationType(tok2->tokAt(3), varid); if (alloc != CheckMemoryLeak::No) diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 46016970a..bbe01d27f 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -3209,8 +3209,25 @@ private: " delete [] str2;\n" "}\n"); ASSERT_EQUALS("[test.cpp:4]: (error) Memory leak: Fred::str1\n", errout.str()); - } + check("class Fred\n" + "{\n" + "private:\n" + " char *str1;\n" + " char *str2;\n" + "public:\n" + " Fred()\n" + " {\n" + " str1 = new char[10];\n" + " str2 = new char[10];\n" + " }\n" + " ~Fred()\n" + " {\n" + " delete [] str2;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory leak: Fred::str1\n", errout.str()); + } void class2() { @@ -3233,6 +3250,22 @@ private: " free(str1);\n" "}\n"); ASSERT_EQUALS("[test.cpp:17]: (error) Mismatching allocation and deallocation: Fred::str1\n", errout.str()); + + check("class Fred\n" + "{\n" + "private:\n" + " char *str1;\n" + "public:\n" + " Fred()\n" + " {\n" + " str1 = new char[10];\n" + " }\n" + " ~Fred()\n" + " {\n" + " free(str1);\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:12]: (error) Mismatching allocation and deallocation: Fred::str1\n", errout.str()); } void class3() @@ -3271,6 +3304,35 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + + check("class Token;\n" + "\n" + "class Tokenizer\n" + "{\n" + "private:\n" + " Token *_tokens;\n" + "\n" + "public:\n" + " Tokenizer()\n" + " {\n" + " _tokens = new Token;\n" + " }\n" + " ~Tokenizer()\n" + " {\n" + " deleteTokens(_tokens);\n" + " }\n" + " void deleteTokens(Token *tok)\n" + " {\n" + " while (tok)\n" + " {\n" + " Token *next = tok->next();\n" + " delete tok;\n" + " tok = next;\n" + " }\n" + " }\n" + "};\n"); + + ASSERT_EQUALS("", errout.str()); } void class4() @@ -3295,6 +3357,23 @@ private: " addAbc( p );\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("struct ABC;\n" + "class Fred\n" + "{\n" + "private:\n" + " void addAbc(ABC* abc)\n" + " {\n" + " AbcPosts->Add(abc);\n" + " }\n" + "public:\n" + " void click()\n" + " {\n" + " ABC *p = new ABC;\n" + " addAbc( p );\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void class6() @@ -3312,6 +3391,18 @@ private: " hello();\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("class Fred\n" + "{\n" + "public:\n" + " void foo()\n" + " { \n" + " char *str = new char[100];\n" + " delete [] str;\n" + " hello();\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void class7() @@ -3333,6 +3424,21 @@ private: " delete this->i;\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("class Fred\n" + "{\n" + "public:\n" + " int *i;\n" + " Fred()\n" + " {\n" + " this->i = new int;\n" + " }\n" + " ~Fred()\n" + " {\n" + " delete this->i;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void class8() @@ -3351,6 +3457,19 @@ private: " doNothing(c);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("class A\n" + "{\n" + "public:\n" + " void a()\n" + " {\n" + " int* c = new int(1);\n" + " delete c;\n" + " doNothing(c);\n" + " }\n" + " void doNothing() { }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void class9() @@ -3369,10 +3488,31 @@ private: "A::~A()\n" "{ delete (p); }\n"); ASSERT_EQUALS("", errout.str()); + + check("class A\n" + "{\n" + "public:\n" + " int * p;\n" + " A()\n" + " { p = new int; }\n" + " ~A()\n" + " { delete (p); }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void class10() { + check("class A\n" + "{\n" + "public:\n" + " int * p;\n" + " A();\n" + "};\n" + "A::A()\n" + "{ p = new int; }\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory leak: A::p\n", errout.str()); + check("class A\n" "{\n" "public:\n" @@ -3384,6 +3524,15 @@ private: void class11() { + check("class A\n" + "{\n" + "public:\n" + " int * p;\n" + " A() : p(new int[10])\n" + " { }" + "};\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory leak: A::p\n", errout.str()); + check("class A\n" "{\n" "public:\n" @@ -3416,6 +3565,20 @@ private: "void A::cleanup()\n" "{ delete [] p; }\n"); ASSERT_EQUALS("[test.cpp:4]: (error) Memory leak: A::p\n", errout.str()); + + check("class A\n" + "{\n" + "private:\n" + " int *p;\n" + "public:\n" + " A()\n" + " { p = new int[10]; }\n" + " ~A()\n" + " { }\n" + " void cleanup()\n" + " { delete [] p; }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory leak: A::p\n", errout.str()); } void class13() @@ -3438,7 +3601,21 @@ private: "\n" "void A::foo()\n" "{ p = new int[10]; delete [] p; }\n"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:17]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n", errout.str()); + + check("class A\n" + "{\n" + "private:\n" + " int *p;\n" + "public:\n" + " A()\n" + " { }\n" + " ~A()\n" + " { }\n" + " void foo()\n" + " { p = new int[10]; delete [] p; }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:11]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n", errout.str()); } void class14() @@ -3452,7 +3629,19 @@ private: "\n" "void A::init()\n" "{ p = new int[10]; }\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n" + "[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + + check("class A\n" + "{\n" + " int *p;\n" + "public:\n" + " void init()\n" + " { p = new int[10]; }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n" + "[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + check("class A\n" "{\n" @@ -3463,7 +3652,19 @@ private: "\n" "void A::init()\n" "{ p = new int; }\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n" + "[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + + check("class A\n" + "{\n" + " int *p;\n" + "public:\n" + " void init()\n" + " { p = new int; }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n" + "[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + check("class A\n" "{\n" @@ -3474,7 +3675,18 @@ private: "\n" "void A::init()\n" "{ p = malloc(sizeof(int)*10); }\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n" + "[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); + + check("class A\n" + "{\n" + " int *p;\n" + "public:\n" + " void init()\n" + " { p = malloc(sizeof(int)*10); }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Possible leak in public function. The pointer 'p' is not deallocated before it is allocated.\n" + "[test.cpp:3]: (error) Memory leak: A::p\n", errout.str()); } void class15() @@ -3490,6 +3702,17 @@ private: "{ p = new int[10]; }"); ASSERT_EQUALS("", errout.str()); + check("class A\n" + "{\n" + " int *p;\n" + "public:\n" + " A()\n" + " { p = new int[10]; }\n" + " ~A() { delete [] p; }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("class A\n" "{\n" " int *p;\n" @@ -3501,6 +3724,17 @@ private: "{ p = new int; }"); ASSERT_EQUALS("", errout.str()); + check("class A\n" + "{\n" + " int *p;\n" + "public:\n" + " A()\n" + " { p = new int; }\n" + " ~A() { delete p; }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("class A\n" "{\n" " int *p;\n" @@ -3511,6 +3745,16 @@ private: "A::A()\n" "{ p = malloc(sizeof(int)*10); }"); ASSERT_EQUALS("", errout.str()); + + check("class A\n" + "{\n" + " int *p;\n" + "public:\n" + " A()\n" + " { p = malloc(sizeof(int)*10); }\n" + " ~A() { free(p); }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void class16() @@ -3543,6 +3787,33 @@ private: " delete [] A::pd;\n" "}\n"); ASSERT_EQUALS("", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:9]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); + + check("class A {\n" + "private:\n" + " char *pd;\n" + "public:\n" + " void foo()\n" + " {\n" + " pd = new char[12];\n" + " delete [] pd;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); + + check("class A {\n" + "private:\n" + " char *pd;\n" + "public:\n" + " void foo();\n" + "};\n" + "\n" + "void A::foo()\n" + "{\n" + " pd = new char[12];\n" + " delete [] pd;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:9]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); } void class18() @@ -3560,6 +3831,20 @@ private: " char *a;\n" "};\n"); ASSERT_EQUALS("", errout.str()); + + check("class A : public x\n" + "{\n" + "public:\n" + " A();\n" + "private:\n" + " char *a;\n" + "};\n" + "A::A()\n" + "{\n" + " a = new char[10];\n" + " foo(a);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void class19()