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

Fix FP memory leak with unknown function call in condition (#2012) 

* Fix FP memory leak with unknown function call in condition

This was introduced in 8513fb81d2 when
fixing memory leaks for global variables allocated in condition. The
refactored code had an inconsistency where c and c++ code behaved
slightly differently when `var` is NULL. This seemed to not have an
impact as the code was written prior to 8513fb81d2,
but when the same code was used for conditions, FPs were introduced.

The introduced FPs were memleak warnings when there should have been an
information message about missing configurations for code like

	void f() {
		char *p = malloc(10);
		if (set_data(p)) {}
	}

Fix this by always returning true if varTok->Variable() is NULL for
both c and c++ code.

* Improve function name
This commit is contained in:
Rikard Falkeborn 2019-07-18 15:23:19 +02:00 committed by Daniel Marjamäki
parent 8cd8a2671c
commit abea580b78
2 changed files with 23 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
lib/checkleakautovar.cpp
View File

@ -229,21 +229,21 @@ static bool isPointerReleased(const Token *startToken, const Token *endToken, no
return false; return false;
} }
static bool checkVariable(const Token *varTok, const bool isCpp) static bool isLocalVarNoAutoDealloc(const Token *varTok, const bool isCpp)
{ {
// not a local variable nor argument? // not a local variable nor argument?
const Variable *var = varTok->variable(); const Variable *var = varTok->variable();
if (var && !var->isArgument() && (!var->isLocal() || var->isStatic())) if (!var)
return true;
if (!var->isArgument() && (!var->isLocal() || var->isStatic()))
return false; return false;
// Don't check reference variables // Don't check reference variables
if (var && var->isReference()) if (var->isReference())
return false; return false;
// non-pod variable // non-pod variable
if (isCpp) { if (isCpp) {
if (!var)
return false;
// Possibly automatically deallocated memory // Possibly automatically deallocated memory
if (isAutoDealloc(var) && Token::Match(varTok, "%var% = new")) if (isAutoDealloc(var) && Token::Match(varTok, "%var% = new"))
return false; return false;
@ -365,7 +365,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
leakIfAllocated(varTok, *varInfo); leakIfAllocated(varTok, *varInfo);
varInfo->erase(varTok->varId()); varInfo->erase(varTok->varId());
if (!checkVariable(varTok, mTokenizer->isCPP())) if (!isLocalVarNoAutoDealloc(varTok, mTokenizer->isCPP()))
continue; continue;
// allocation? // allocation?
@ -405,7 +405,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
for (const Token *innerTok = tok->tokAt(2); innerTok && innerTok != closingParenthesis; innerTok = innerTok->next()) { for (const Token *innerTok = tok->tokAt(2); innerTok && innerTok != closingParenthesis; innerTok = innerTok->next()) {
// TODO: replace with checkTokenInsideExpression() // TODO: replace with checkTokenInsideExpression()
if (!checkVariable(innerTok, mTokenizer->isCPP())) if (!isLocalVarNoAutoDealloc(innerTok, mTokenizer->isCPP()))
continue; continue;
if (Token::Match(innerTok, "%var% =") && innerTok->astParent() == innerTok->next()) { if (Token::Match(innerTok, "%var% =") && innerTok->astParent() == innerTok->next()) {

16
test/testleakautovar.cpp
View File

@ -1767,19 +1767,27 @@ private:
} }
void configuration3() { void configuration3() {
check("void f() {\n" const char * code = "void f() {\n"
" char *p = malloc(10);\n" " char *p = malloc(10);\n"
" if (set_data(p)) { }\n" " if (set_data(p)) { }\n"
"}"); "}";
check(code);
ASSERT_EQUALS("[test.c:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n", errout.str()); ASSERT_EQUALS("[test.c:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n", errout.str());
check(code, true);
ASSERT_EQUALS("[test.cpp:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n", errout.str());
check("void f() {\n" code = "void f() {\n"
" char *p = malloc(10);\n" " char *p = malloc(10);\n"
" if (set_data(p)) { return; }\n" " if (set_data(p)) { return; }\n"
"}"); "}";
check(code);
ASSERT_EQUALS("[test.c:3]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n" ASSERT_EQUALS("[test.c:3]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
"[test.c:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n" "[test.c:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
, errout.str()); , errout.str());
check(code, true);
ASSERT_EQUALS("[test.cpp:3]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
"[test.cpp:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
, errout.str());
} }
void configuration4() { void configuration4() {