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

Improve handling of realloc in memory leak checker (#3036) 

Mark realloced variables as realloced instead of freed. This allows
improved checking for code with error realloc handling.

If cppcheck finds an if-statement which checks the validity of the
allocated memory or resource, check if the memory/resource is
reallocated from another variable. If so, we can add checking of that
variable in the if-statement instead. This allows to check that variable
for memleaks and double frees.

This fixes #9292 and #9990 which both concern FPs with double frees
after correct error handling.
This commit is contained in:
Rikard Falkeborn 2021-01-11 07:55:05 +01:00 committed by GitHub
parent 707f1f2fbe
commit f018163551
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 120 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
lib/checkleakautovar.cpp
View File

@ -101,6 +101,9 @@ void VarInfo::print()
case NOALLOC:
status = "noalloc";
break;
case REALLOC:
status = "realloc";
break;
default:
status = "?";
break;
@ -111,6 +114,7 @@ void VarInfo::print()
<< "possibleUsage='" << strusage << "' "
<< "conditionalAlloc=" << (conditionalAlloc.find(it->first) != conditionalAlloc.end() ? "yes" : "no") << " "
<< "referenced=" << (referenced.find(it->first) != referenced.end() ? "yes" : "no") << " "
<< "reallocedFrom=" << it->second.reallocedFromType
<< std::endl;
}
}
@ -499,18 +503,24 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
const Token *vartok = nullptr;
if (astIsVariableComparison(tok3, "!=", "0", &vartok)) {
varInfo2.reallocToAlloc(vartok->varId());
varInfo2.erase(vartok->varId());
if (notzero.find(vartok->varId()) != notzero.end())
varInfo2.clear();
} else if (astIsVariableComparison(tok3, "==", "0", &vartok)) {
varInfo1.reallocToAlloc(vartok->varId());
varInfo1.erase(vartok->varId());
} else if (astIsVariableComparison(tok3, "<", "0", &vartok)) {
varInfo1.reallocToAlloc(vartok->varId());
varInfo1.erase(vartok->varId());
} else if (astIsVariableComparison(tok3, ">", "0", &vartok)) {
varInfo2.reallocToAlloc(vartok->varId());
varInfo2.erase(vartok->varId());
} else if (astIsVariableComparison(tok3, "==", "-1", &vartok)) {
varInfo1.reallocToAlloc(vartok->varId());
varInfo1.erase(vartok->varId());
} else if (astIsVariableComparison(tok3, "!=", "-1", &vartok)) {
varInfo2.reallocToAlloc(vartok->varId());
varInfo2.erase(vartok->varId());
}
return ChildrenToVisit::none;
@ -801,15 +811,18 @@ void CheckLeakAutoVar::changeAllocStatusIfRealloc(std::map<int, VarInfo::AllocIn
const Library::AllocFunc* f = mSettings->library.getReallocFuncInfo(fTok);
if (f && f->arg == -1 && f->reallocArg > 0 && f->reallocArg <= numberOfArguments(fTok)) {
const Token* argTok = getArguments(fTok).at(f->reallocArg - 1);
VarInfo::AllocInfo& argAlloc = alloctype[argTok->varId()];
if (alloctype.find(argTok->varId()) != alloctype.end()) {
VarInfo::AllocInfo& argAlloc = alloctype[argTok->varId()];
if (argAlloc.type != 0 && argAlloc.type != f->groupId)
mismatchError(fTok, argAlloc.allocTok, argTok->str());
argAlloc.status = VarInfo::REALLOC;
argAlloc.allocTok = fTok;
}
VarInfo::AllocInfo& retAlloc = alloctype[retTok->varId()];
if (argAlloc.type != 0 && argAlloc.type != f->groupId)
mismatchError(fTok, argAlloc.allocTok, argTok->str());
argAlloc.status = VarInfo::DEALLOC;
argAlloc.allocTok = fTok;
retAlloc.type = f->groupId;
retAlloc.status = VarInfo::ALLOC;
retAlloc.allocTok = fTok;
retAlloc.reallocedFromType = argTok->varId();
}
}
@ -826,7 +839,8 @@ void CheckLeakAutoVar::changeAllocStatus(VarInfo *varInfo, const VarInfo::AllocI
varInfo->erase(arg->varId());
} else if (var->second.managed()) {
doubleFreeError(tok, var->second.allocTok, arg->str(), allocation.type);
} else if (var->second.type != allocation.type) {
var->second.status = allocation.status;
} else if (var->second.type != allocation.type && var->second.type != 0) {
// mismatching allocation and deallocation
mismatchError(tok, var->second.allocTok, arg->str());
varInfo->erase(arg->varId());

13
lib/checkleakautovar.h
View File

@ -39,7 +39,7 @@ class Tokenizer;
class CPPCHECKLIB VarInfo {
public:
enum AllocStatus { OWNED = -2, DEALLOC = -1, NOALLOC = 0, ALLOC = 1 };
enum AllocStatus { REALLOC = -3, OWNED = -2, DEALLOC = -1, NOALLOC = 0, ALLOC = 1 };
struct AllocInfo {
AllocStatus status;
/** Allocation type. If it is a positive value then it corresponds to
@ -47,6 +47,7 @@ public:
* checkleakautovar allocation type.
*/
int type;
int reallocedFromType = -1;
const Token * allocTok;
AllocInfo(int type_ = 0, AllocStatus status_ = NOALLOC, const Token* allocTok_ = nullptr) : status(status_), type(type_), allocTok(allocTok_) {}
@ -80,6 +81,16 @@ public:
referenced.swap(other.referenced);
}
void reallocToAlloc(nonneg int varid) {
const AllocInfo& alloc = alloctype[varid];
if (alloc.reallocedFromType >= 0) {
const std::map<int, VarInfo::AllocInfo>::iterator it = alloctype.find(alloc.reallocedFromType);
if (it != alloctype.end() && it->second.status == REALLOC) {
it->second.status = ALLOC;
}
}
}
/** set possible usage for all variables */
void possibleUsageAll(const std::string &functionName);

88
test/testleakautovar.cpp
View File

@ -86,6 +86,8 @@ private:
TEST_CASE(realloc1);
TEST_CASE(realloc2);
TEST_CASE(realloc3);
TEST_CASE(realloc4);
TEST_CASE(realloc5); // #9292, #9990
TEST_CASE(freopen1);
TEST_CASE(freopen2);
@ -108,6 +110,7 @@ private:
TEST_CASE(doublefree8);
TEST_CASE(doublefree9);
TEST_CASE(doublefree10); // #8706
TEST_CASE(doublefree11);
// exit
TEST_CASE(exit1);
@ -139,6 +142,8 @@ private:
TEST_CASE(ifelse15); // #9206 - if (global_ptr = malloc(1))
TEST_CASE(ifelse16); // #9635 - if (p = malloc(4), p == NULL)
TEST_CASE(ifelse17); // if (!!(!p))
TEST_CASE(ifelse18);
TEST_CASE(ifelse19);
// switch
TEST_CASE(switch1);
@ -467,6 +472,40 @@ private:
ASSERT_EQUALS("[test.c:4]: (error) Memory leak: q\n", errout.str());
}
void realloc4() {
check("void f(void *p) {\n"
" void * q = realloc(p, 10);\n"
" if (q == NULL)\n"
" return;\n"
"}");
ASSERT_EQUALS("[test.c:5]: (error) Memory leak: q\n", errout.str());
}
void realloc5() {
// #9292
check("void * f(void * ptr, size_t size) {\n"
" void *datap = realloc(ptr, size);\n"
" if (size && !datap)\n"
" free(ptr);\n"
" return datap;\n"
"}");
ASSERT_EQUALS("", errout.str());
// #9990
check("void f() {\n"
" void * p1 = malloc(10);\n"
" if (!p1)\n"
" return;\n"
" void * p2 = realloc(p1, 42);\n"
" if (!p2) {\n"
" free(p1);\n"
" return;\n"
" }\n"
" free(p2);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void freopen1() {
check("void f() {\n"
" void *p = fopen(name,a);\n"
@ -1196,6 +1235,22 @@ private:
ASSERT_EQUALS("", errout.str());
}
void doublefree11() {
check("void f() {\n"
" void * p = malloc(5);\n"
" void * q = realloc(p, 10);\n"
" if (q == NULL) {\n"
" free(p);\n"
" return;\n"
" }\n"
" free(p);\n"
" if (q == NULL)\n"
" return;\n"
" free(q)\n"
"}");
ASSERT_EQUALS("[test.c:3] -> [test.c:8]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str());
}
void exit1() {
check("void f() {\n"
" char *p = malloc(10);\n"
@ -1508,6 +1563,39 @@ private:
ASSERT_EQUALS("", errout.str());
}
void ifelse18() {
check("void f() {\n"
" void * p = malloc(10);\n"
" void * q = realloc(p, 20);\n"
" if (q == 0)\n"
" return;\n"
" free(q);\n"
"}");
ASSERT_EQUALS("[test.c:5]: (error) Memory leak: p\n", errout.str());
check("void f() {\n"
" void * p = malloc(10);\n"
" void * q = realloc(p, 20);\n"
" if (q != 0) {\n"
" free(q);\n"
" return;\n"
" } else\n"
" return;\n"
"}");
ASSERT_EQUALS("[test.c:8]: (error) Memory leak: p\n", errout.str());
}
void ifelse19() {
check("void f() {\n"
" static char * a;\n"
" char * b = realloc(a, 10);\n"
" if (!b)\n"
" return;\n"
" a = b;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void switch1() {
check("void f() {\n"
" char *p = 0;\n"