walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Use library for memleak checks (#2002) 

* Use library for memleak checks

Change memleakOnRealloc and leakReturnValNotUsed to use library
configuration instead of hardcoding "realloc".

In order to do so, some care needs to be taken when matching for a
reallocation function, since it can no longer be assumed that the input
to be allocated is the first argument of the function. This complicates
getReallocationType() and checkReallocUsage() but is necessary in order
to handle for example freopen() properly.

Also, refactor memleakOnRealloc check to reduce duplicated code when
checking "a" and "*a". When doing so, extending the check to look for
arbitrary number of "*" can be done for free (just change an if
statement to a while statement). Most likely, this is an unusual case in
real world code.

* Remove redundant whitespace in Token::Match()

* Run on simplified checks

* Fix cppcheck warning
This commit is contained in:
Rikard Falkeborn 2019-07-22 10:37:36 +02:00 committed by Daniel Marjamäki
parent 06337cedf5
commit 8cd1d5a47d
5 changed files with 137 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
lib/checkmemoryleak.cpp
View File

@ -97,8 +97,9 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2,
if (!Token::Match(tok2, "%name% ::|. %type%")) {
// Using realloc..
if (varid && Token::Match(tok2, "realloc ( %any% ,") && tok2->tokAt(2)->varId() != varid)
return Malloc;
AllocType reallocType = getReallocationType(tok2, varid);
if (reallocType != No)
return reallocType;
if (mTokenizer_->isCPP() && tok2->str() == "new") {
if (tok2->strAt(1) == "(" && !Token::Match(tok2->next(),"( std| ::| nothrow )"))
@ -166,7 +167,7 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2,
}
CheckMemoryLeak::AllocType CheckMemoryLeak::getReallocationType(const Token *tok2, nonneg int varid)
CheckMemoryLeak::AllocType CheckMemoryLeak::getReallocationType(const Token *tok2, nonneg int varid) const
{
// What we may have...
// * var = (char *)realloc(..;
@ -177,12 +178,28 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getReallocationType(const Token *tok
if (! tok2)
return No;
if (varid > 0 && ! Token::Match(tok2, "%name% ( %varid% [,)]", varid))
if (!Token::Match(tok2, "%name% ("))
return No;
if (tok2->str() == "realloc")
return Malloc;
const Library::AllocFunc *f = mSettings_->library.getReallocFuncInfo(tok2);
if (!(f && f->reallocArg > 0 && f->reallocArg <= numberOfArguments(tok2)))
return No;
const Token* arg = getArguments(tok2).at(f->reallocArg - 1);
while (arg && arg->isCast())
arg = arg->astOperand1();
while (arg && arg->isUnaryOp("*"))
arg = arg->astOperand1();
if (varid > 0 && !Token::Match(arg, "%varid% [,)]", varid))
return No;
const int realloctype = mSettings_->library.getReallocId(tok2, -1);
if (realloctype > 0) {
if (realloctype == mSettings_->library.deallocId("free"))
return Malloc;
if (realloctype == mSettings_->library.deallocId("fclose"))
return File;
return Library::ismemory(realloctype) ? OtherMem : OtherRes;
}
return No;
}
@ -283,9 +300,9 @@ void CheckMemoryLeak::memleakError(const Token *tok, const std::string &varname)
reportErr(tok, Severity::error, "memleak", "$symbol:" + varname + "\nMemory leak: $symbol", CWE(401U));
}
void CheckMemoryLeak::memleakUponReallocFailureError(const Token *tok, const std::string &varname) const
void CheckMemoryLeak::memleakUponReallocFailureError(const Token *tok, const std::string &reallocfunction, const std::string &varname) const
{
reportErr(tok, Severity::error, "memleakOnRealloc", "$symbol:" + varname + "\nCommon realloc mistake: \'$symbol\' nulled but not freed upon failure", CWE(401U));
reportErr(tok, Severity::error, "memleakOnRealloc", "$symbol:" + varname + "\nCommon " + reallocfunction + " mistake: \'$symbol\' nulled but not freed upon failure", CWE(401U));
}
void CheckMemoryLeak::resourceLeakError(const Token *tok, const std::string &varname) const
@ -416,9 +433,6 @@ const char *CheckMemoryLeak::functionArgAlloc(const Function *func, nonneg int t
allocType = No;
} else if (Token::Match(tok->previous(), "* %name% =")) {
allocType = getAllocationType(tok->tokAt(2), arg->declarationId());
if (allocType == No) {
allocType = getReallocationType(tok->tokAt(2), arg->declarationId());
}
if (allocType != No) {
if (realloc)
return "realloc";
@ -488,16 +502,46 @@ void CheckMemoryLeakInFunction::checkReallocUsage()
// Search for the "var = realloc(var, 100" pattern within this function
for (const Token *tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
if (tok->varId() > 0 &&
Token::Match(tok, "%name% = realloc|g_try_realloc ( %name% ,") &&
tok->varId() == tok->tokAt(4)->varId() &&
isNoArgument(symbolDatabase, tok->varId())) {
if (tok->varId() > 0 && Token::Match(tok, "%name% =")) {
// Get the parenthesis in "realloc("
const Token* parTok = tok->next()->astOperand2();
// Skip casts
while (parTok && parTok->isCast())
parTok = parTok->astOperand1();
if (!parTok)
continue;
const Token *const reallocTok = parTok->astOperand1();
if (!reallocTok)
continue;
const Library::AllocFunc* f = mSettings->library.getReallocFuncInfo(reallocTok);
if (!(f && f->arg == -1 && mSettings->library.isnotnoreturn(reallocTok)))
continue;
const AllocType allocType = getReallocationType(reallocTok, tok->varId());
if (!((allocType == Malloc || allocType == OtherMem)))
continue;
const Token* arg = getArguments(reallocTok).at(f->reallocArg - 1);
while (arg && arg->isCast())
arg = arg->astOperand1();
const Token* tok2 = tok;
while (arg && arg->isUnaryOp("*") && tok2 && tok2->astParent() && tok2->astParent()->isUnaryOp("*")) {
arg = arg->astOperand1();
tok2 = tok2->astParent();
}
if (!arg || !tok2)
continue;
if (!((tok->varId() == arg->varId()) && isNoArgument(symbolDatabase, tok->varId())))
continue;
// Check that another copy of the pointer wasn't saved earlier in the function
if (Token::findmatch(scope->bodyStart, "%name% = %varid% ;", tok, tok->varId()) ||
Token::findmatch(scope->bodyStart, "[{};] %varid% = %name% [;=]", tok, tok->varId()))
continue;
const Token* tokEndRealloc = tok->linkAt(3);
const Token* tokEndRealloc = reallocTok->linkAt(1);
// Check that the allocation isn't followed immediately by an 'if (!var) { error(); }' that might handle failure
if (Token::simpleMatch(tokEndRealloc->next(), "; if (") &&
notvar(tokEndRealloc->tokAt(3)->astOperand2(), tok->varId())) {
@ -506,25 +550,7 @@ void CheckMemoryLeakInFunction::checkReallocUsage()
continue;
}
memleakUponReallocFailureError(tok, tok->str());
} else if (tok->next()->varId() > 0 &&
(Token::Match(tok, "* %name% = realloc|g_try_realloc ( * %name% ,") &&
tok->next()->varId() == tok->tokAt(6)->varId()) &&
isNoArgument(symbolDatabase, tok->next()->varId())) {
// Check that another copy of the pointer wasn't saved earlier in the function
if (Token::findmatch(scope->bodyStart, "%name% = * %varid% ;", tok, tok->next()->varId()) ||
Token::findmatch(scope->bodyStart, "[{};] * %varid% = %name% [;=]", tok, tok->next()->varId()))
continue;
const Token* tokEndRealloc = tok->linkAt(4);
// Check that the allocation isn't followed immediately by an 'if (!var) { error(); }' that might handle failure
if (Token::Match(tokEndRealloc->next(), "; if ( ! * %varid% ) {", tok->next()->varId())) {
const Token* tokEndBrace = tokEndRealloc->linkAt(8);
if (tokEndBrace && Token::simpleMatch(tokEndBrace->tokAt(-2), ") ;") &&
Token::Match(tokEndBrace->linkAt(-2)->tokAt(-2), "{|}|; %name% ("))
continue;
}
memleakUponReallocFailureError(tok->next(), tok->strAt(1));
memleakUponReallocFailureError(tok, reallocTok->str(), tok->str());
}
}
}

6
lib/checkmemoryleak.h
View File

@ -112,7 +112,7 @@ public:
/**
* @brief Get type of reallocation at given position
*/
static AllocType getReallocationType(const Token *tok2, nonneg int varid);
AllocType getReallocationType(const Token *tok2, nonneg int varid) const;
/**
* Report that there is a memory leak (new/malloc/etc)
@ -137,7 +137,7 @@ public:
void deallocuseError(const Token *tok, const std::string &varname) const;
void mismatchSizeError(const Token *tok, const std::string &sz) const;
void mismatchAllocDealloc(const std::list<const Token *> &callstack, const std::string &varname) const;
void memleakUponReallocFailureError(const Token *tok, const std::string &varname) const;
void memleakUponReallocFailureError(const Token *tok, const std::string &reallocfunction, const std::string &varname) const;
/** What type of allocated memory does the given function return? */
AllocType functionReturnType(const Function* func, std::list<const Function*> *callstack = nullptr) const;
@ -201,7 +201,7 @@ private:
c.mismatchSizeError(nullptr, "sz");
const std::list<const Token *> callstack;
c.mismatchAllocDealloc(callstack, "varname");
c.memleakUponReallocFailureError(nullptr, "varname");
c.memleakUponReallocFailureError(nullptr, "realloc", "varname");
}
/**

16
test/cfg/gtk.c
View File

@ -162,7 +162,7 @@ void g_try_malloc0_n_test()
void g_realloc_test()
{
// cppcheck-suppress ignoredReturnValue
// TODO cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress leakReturnValNotUsed
g_realloc(NULL, 1);
gpointer gpt = g_malloc(1);
@ -175,7 +175,7 @@ void g_realloc_test()
void g_realloc_n_test()
{
// cppcheck-suppress ignoredReturnValue
// TODO cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress leakReturnValNotUsed
g_realloc_n(NULL, 1, 2);
gpointer gpt = g_malloc_n(1, 2);
@ -188,7 +188,7 @@ void g_realloc_n_test()
void g_try_realloc_test()
{
// cppcheck-suppress ignoredReturnValue
// TODO cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress leakReturnValNotUsed
g_try_realloc(NULL, 1);
gpointer gpt = g_try_malloc(1);
@ -202,11 +202,11 @@ void g_try_realloc_test()
void g_try_realloc_n_test()
{
// cppcheck-suppress ignoredReturnValue
// TODO cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress leakReturnValNotUsed
g_try_realloc_n(NULL, 1, 2);
gpointer gpt = g_try_malloc_n(1, 2);
// TODO cppcheck-suppress memleakOnRealloc
// cppcheck-suppress memleakOnRealloc
gpt = g_try_realloc_n(gpt, 2, 3);
printf("%p", gpt);
@ -325,7 +325,7 @@ void g_renew_test()
struct a {
int b;
};
// TODO cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress leakReturnValNotUsed
g_renew(struct a, NULL, 1);
struct a * pNew = g_new(struct a, 1);
@ -340,11 +340,11 @@ void g_try_renew_test()
struct a {
int b;
};
// TODO cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress leakReturnValNotUsed
g_try_renew(struct a, NULL, 1);
struct a * pNew = g_try_new(struct a, 1);
// TODO cppcheck-suppress memleakOnRealloc
// cppcheck-suppress memleakOnRealloc
pNew = g_try_renew(struct a, pNew, 2);
printf("%p", pNew);

2
test/cfg/std.c
View File

@ -125,7 +125,7 @@ void arrayIndexOutOfBounds()
pAlloc3[5] = 'a';
// cppcheck-suppress arrayIndexOutOfBounds
pAlloc3[6] = 1;
// TODO cppcheck-suppress memleakOnRealloc
// cppcheck-suppress memleakOnRealloc
pAlloc3 = reallocarray(pAlloc3, 3,3);
pAlloc3[8] = 'a';
// cppcheck-suppress arrayIndexOutOfBounds

67
test/testmemleak.cpp
View File

@ -141,7 +141,6 @@ private:
Tokenizer tokenizer(settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
tokenizer.simplifyTokenList2();
// Check for memory leaks..
CheckMemoryLeakInFunction checkMemoryLeak(&tokenizer, settings, this);
@ -169,6 +168,11 @@ private:
TEST_CASE(realloc14);
TEST_CASE(realloc15);
TEST_CASE(realloc16);
TEST_CASE(realloc17);
TEST_CASE(realloc18);
TEST_CASE(realloc19);
TEST_CASE(realloc20);
TEST_CASE(reallocarray1);
}
void realloc1() {
@ -200,7 +204,7 @@ private:
" free(a);\n"
"}");
ASSERT_EQUALS("", errout.str());
TODO_ASSERT_EQUALS("", "[test.cpp:4]: (error) Common realloc mistake: 'a' nulled but not freed upon failure\n", errout.str());
}
void realloc4() {
@ -295,7 +299,7 @@ private:
" return;\n"
" free(a);\n"
"}");
ASSERT_EQUALS("", errout.str());
TODO_ASSERT_EQUALS("", "[test.cpp:4]: (error) Common realloc mistake: 'a' nulled but not freed upon failure\n", errout.str());
}
void realloc13() {
@ -339,6 +343,51 @@ private:
"}\n");
ASSERT_EQUALS("", errout.str());
}
void realloc17() {
check("void foo()\n"
"{\n"
" void ***a = malloc(sizeof(a));\n"
" ***a = realloc(***(a), sizeof(a) * 2);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Common realloc mistake: \'a\' nulled but not freed upon failure\n", errout.str());
}
void realloc18() {
check("void foo()\n"
"{\n"
" void *a = malloc(sizeof(a));\n"
" a = realloc((void*)a, sizeof(a) * 2);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Common realloc mistake: \'a\' nulled but not freed upon failure\n", errout.str());
}
void realloc19() {
check("void foo()\n"
"{\n"
" void *a = malloc(sizeof(a));\n"
" a = (realloc((void*)((a)), sizeof(a) * 2));\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Common realloc mistake: \'a\' nulled but not freed upon failure\n", errout.str());
}
void realloc20() {
check("void foo()\n"
"{\n"
" void *a = malloc(sizeof(a));\n"
" a = realloc((a) + 1, sizeof(a) * 2);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void reallocarray1() {
check("void foo()\n"
"{\n"
" char *a = (char *)malloc(10);\n"
" a = reallocarray(a, 100, 2);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Common reallocarray mistake: \'a\' nulled but not freed upon failure\n", errout.str());
}
};
REGISTER_TEST(TestMemleakInFunction)
@ -2111,6 +2160,12 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Return value of allocation function 'strdup' is not stored.\n", errout.str());
check("void x()\n"
"{\n"
" reallocarray(NULL, 10, 10);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Return value of allocation function 'reallocarray' is not stored.\n", errout.str());
check("void x()\n"
"{\n"
" (char*) malloc(10);\n"
@ -2268,6 +2323,12 @@ private:
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (error) Return value of allocation function 'fopen' is not stored.\n", errout.str());
check("void foo() {\n"
" FILE f* = fopen(\"file.txt\", \"r\");\n"
" freopen(\"file.txt\", \"r\", f);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Return value of allocation function 'freopen' is not stored.\n", errout.str());
check("struct Holder {\n"
" Holder(FILE* f) : file(f) {}\n"
" ~Holder() { fclose(file); }\n"