From 303652c69c3285a62a0435174f769915225f30c0 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Thu, 18 Jan 2024 20:13:16 +0100 Subject: [PATCH] Fix #12327 FP memleak when storing pointer in object / #12332 FN memleak for allocation functions with std:: prefix (#5861) --- cfg/bsd.cfg | 21 +++++++++++++++++++ cfg/cppcheck-cfg.rng | 8 ++++---- cfg/std.cfg | 44 ++++++++++++---------------------------- lib/checkleakautovar.cpp | 8 +++++--- lib/library.cpp | 27 +++++++++++++++--------- test/cfg/bsd.c | 27 ++++++++++++++++++++++++ test/cfg/std.c | 5 ----- test/cfg/std.cpp | 29 ++++++++++++++++++++++++-- test/testleakautovar.cpp | 33 ++++++++++++++++++++++++++++++ test/testmemleak.cpp | 16 --------------- test/testvalueflow.cpp | 1 + 11 files changed, 148 insertions(+), 71 deletions(-) diff --git a/cfg/bsd.cfg b/cfg/bsd.cfg index 751bb03c00e..158d7a8a885 100644 --- a/cfg/bsd.cfg +++ b/cfg/bsd.cfg @@ -518,6 +518,27 @@ + + + + + false + + + + + + 0: + + + + 0: + + + + reallocarray + free + diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng index f3a3c1f7be2..5dd446c1778 100644 --- a/cfg/cppcheck-cfg.rng +++ b/cfg/cppcheck-cfg.rng @@ -35,7 +35,7 @@ - + @@ -50,7 +50,7 @@ - + @@ -75,7 +75,7 @@ - + @@ -87,7 +87,7 @@ - + diff --git a/cfg/std.cfg b/cfg/std.cfg index 753cd006ae1..7fb77addfd3 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -965,7 +965,7 @@ - + false @@ -3172,7 +3172,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun - + false @@ -4572,23 +4572,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun 0: - - - - - false - - - - - - 0: - - - - 0: - - @@ -8599,22 +8582,21 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init - malloc - calloc - aligned_alloc - realloc - reallocarray - free + malloc,std::malloc + calloc,std::calloc + aligned_alloc,std::aligned_alloc + realloc,std::realloc + free,std::free - strdup - free + strdup,std::strdup + free,std::free - fopen - tmpfile - freopen - fclose + fopen,std::fopen + tmpfile,std::tmpfile + freopen,std::freopen + fclose,std::fclose diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index f58e05d66c1..7869ebb52fd 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -181,7 +181,9 @@ void CheckLeakAutoVar::configurationInfo(const Token* tok, const std::paircheckLibrary && functionUsage.second == VarInfo::USED && (!functionUsage.first || !functionUsage.first->function() || !functionUsage.first->function()->hasBody())) { - const std::string funcStr = functionUsage.first ? mSettings->library.getFunctionName(functionUsage.first) : "f"; + std::string funcStr = functionUsage.first ? mSettings->library.getFunctionName(functionUsage.first) : "f"; + if (funcStr.empty()) + funcStr = "unknown::" + functionUsage.first->str(); reportError(tok, Severity::information, "checkLibraryUseIgnore", @@ -329,7 +331,7 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken, // check each token { - const bool isInit = Token::Match(tok, "%var% {|(") && tok->variable() && tok == tok->variable()->nameToken(); + const bool isInit = Token::Match(tok, "%var% {|(") && tok->variable() && tok == tok->variable()->nameToken() && tok->variable()->isPointer(); const Token * nextTok = isInit ? nullptr : checkTokenInsideExpression(tok, varInfo); if (nextTok) { tok = nextTok; @@ -1184,7 +1186,7 @@ void CheckLeakAutoVar::ret(const Token *tok, VarInfo &varInfo, const bool isEndO const auto use = possibleUsage.find(varid); if (use == possibleUsage.end()) { leakError(tok, var->name(), it->second.type); - } else { + } else if (!use->second.first->variable()) { // TODO: handle constructors configurationInfo(tok, use->second); } } diff --git a/lib/library.cpp b/lib/library.cpp index 57eb192b39a..3caf7e70a69 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -216,11 +216,16 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) int allocationId = 0; for (const tinyxml2::XMLElement *memorynode = node->FirstChildElement(); memorynode; memorynode = memorynode->NextSiblingElement()) { if (strcmp(memorynode->Name(),"dealloc")==0) { - const std::map::const_iterator it = mDealloc.find(memorynode->GetText()); - if (it != mDealloc.end()) { - allocationId = it->second.groupId; - break; + const auto names = getnames(memorynode->GetText()); + for (const auto& n : names) { + const std::map::const_iterator it = mDealloc.find(n); + if (it != mDealloc.end()) { + allocationId = it->second.groupId; + break; + } } + if (allocationId != 0) + break; } } if (allocationId == 0) { @@ -234,6 +239,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) // add alloc/dealloc/use functions.. for (const tinyxml2::XMLElement *memorynode = node->FirstChildElement(); memorynode; memorynode = memorynode->NextSiblingElement()) { const std::string memorynodename = memorynode->Name(); + const auto names = getnames(memorynode->GetText()); if (memorynodename == "alloc" || memorynodename == "realloc") { AllocFunc temp = {0}; temp.groupId = allocationId; @@ -268,17 +274,18 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) if (memorynodename == "realloc") temp.reallocArg = memorynode->IntAttribute("realloc-arg", 1); - if (memorynodename != "realloc") - mAlloc[memorynode->GetText()] = temp; - else - mRealloc[memorynode->GetText()] = temp; + auto& map = (memorynodename == "realloc") ? mRealloc : mAlloc; + for (const auto& n : names) + map[n] = temp; } else if (memorynodename == "dealloc") { AllocFunc temp = {0}; temp.groupId = allocationId; temp.arg = memorynode->IntAttribute("arg", 1); - mDealloc[memorynode->GetText()] = temp; + for (const auto& n : names) + mDealloc[n] = temp; } else if (memorynodename == "use") - functions[memorynode->GetText()].use = true; + for (const auto& n : names) + functions[n].use = true; else unknown_elements.insert(memorynodename); } diff --git a/test/cfg/bsd.c b/test/cfg/bsd.c index 227e327c982..734a97fe4aa 100644 --- a/test/cfg/bsd.c +++ b/test/cfg/bsd.c @@ -144,3 +144,30 @@ void uninitvar(void) // cppcheck-suppress uninitvar (void) arc4random_uniform(uint32Uninit); } + +void arrayIndexOutOfBounds(void) +{ + char * pAlloc = calloc(2, 3); + pAlloc[5] = 'a'; + // cppcheck-suppress arrayIndexOutOfBounds + pAlloc[6] = 1; + // cppcheck-suppress memleakOnRealloc + pAlloc = reallocarray(pAlloc, 3, 3); + pAlloc[8] = 'a'; + // cppcheck-suppress arrayIndexOutOfBounds + pAlloc[9] = 1; + free(pAlloc); +} + +void reallocarray_memleak(void) { + char *a = (char *)malloc(10); + // cppcheck-suppress [memleakOnRealloc, unreadVariable] + a = reallocarray(a, 100, 2); + // cppcheck-suppress memleak +} + +void reallocarray_notused(void) +{ + // cppcheck-suppress [leakReturnValNotUsed, ignoredReturnValue] + reallocarray(NULL, 10, 10); +} diff --git a/test/cfg/std.c b/test/cfg/std.c index ec5725d09be..9f2a0d213cf 100644 --- a/test/cfg/std.c +++ b/test/cfg/std.c @@ -348,11 +348,6 @@ void arrayIndexOutOfBounds() pAlloc3[5] = 'a'; // cppcheck-suppress arrayIndexOutOfBounds pAlloc3[6] = 1; - // cppcheck-suppress memleakOnRealloc - pAlloc3 = reallocarray(pAlloc3, 3,3); - pAlloc3[8] = 'a'; - // cppcheck-suppress arrayIndexOutOfBounds - pAlloc3[9] = 1; free(pAlloc3); } diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index 07f427e6fc1..c779bc43610 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -1615,6 +1615,7 @@ void uninitvar_fdim(void) void uninitvar_fclose(void) { + // cppcheck-suppress unassignedVariable FILE *stream; // cppcheck-suppress uninitvar (void)std::fclose(stream); @@ -1847,6 +1848,7 @@ void uninitvar_fread(void) void uninitvar_free(void) { + // cppcheck-suppress unassignedVariable void *block; // cppcheck-suppress uninitvar std::free(block); @@ -1859,7 +1861,7 @@ void uninitvar_freopen(void) FILE *stream; // cppcheck-suppress uninitvar FILE * p = std::freopen(filename,mode,stream); - free(p); + std::fclose(p); } void uninitvar_frexp(void) @@ -2557,6 +2559,8 @@ void uninitvar_srand(void) unsigned int seed; // cppcheck-suppress uninitvar (void)std::srand(seed); + // cppcheck-suppress ignoredReturnValue + std::rand(); } void uninitvar_ldiv(void) @@ -4893,8 +4897,29 @@ void std_vector_data_arithmetic() memcpy(buf.data() + 0, "", 1); } +void memleak_std_malloc() // #12332 +{ + //cppcheck-suppress [unreadVariable, constVariablePointer] + void* p = std::malloc(1); + //cppcheck-suppress memleak +} + +void memleak_std_realloc(void* block, size_t newsize) +{ + //cppcheck-suppress [unreadVariable, constVariablePointer] + void* p = std::realloc(block, newsize); + //cppcheck-suppress memleak +} + +void unusedAllocatedMemory_std_free() +{ + //cppcheck-suppress unusedAllocatedMemory + void* p = std::malloc(1); + std::free(p); +} + std::string global_scope_std() // #12355 { ::std::stringstream ss; return ss.str(); -} \ No newline at end of file +} diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp index 5d3ab7b3bba..62fb509be9c 100644 --- a/test/testleakautovar.cpp +++ b/test/testleakautovar.cpp @@ -597,6 +597,33 @@ class TestLeakAutoVar : public TestFixture { " fd->exec();\n" "}\n", true); ASSERT_EQUALS("", errout.str()); + + check("struct C {\n" // #12327 + " char* m_p;\n" + " C(char* p) : m_p(p) {}\n" + "};\n" + "std::list gli;\n" + "void f() {\n" + " std::list li;\n" + " char* p = new char[1];\n" + " C c(p);\n" + " li.push_back(c);\n" + " C c2(li.front());\n" + " delete[] c2.m_p;\n" + "}\n" + "void g() {\n" + " char* p = new char[1];\n" + " C c(p);\n" + " gli.push_back(c);\n" + "}\n" + "void h() {\n" + " std::list li;\n" + " char* p = new char[1];\n" + " C c(p);\n" + " li.push_back(c);\n" + " delete[] li.front().m_p;\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); } void isAutoDealloc() { @@ -2976,6 +3003,12 @@ class TestLeakAutoVar : public TestFixture { " bar(p);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("void f(int n) {\n" + " char* p = new char[n];\n" + " v.push_back(p);\n" + "}\n", /*cpp*/ true); + ASSERT_EQUALS("[test.cpp:4]: (information) --check-library: Function unknown::push_back() should have / configuration\n", errout.str()); } }; diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 6564db78656..eada4036faf 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -166,7 +166,6 @@ class TestMemleakInFunction : public TestFixture { TEST_CASE(realloc22); TEST_CASE(realloc23); TEST_CASE(realloc24); // #9228 - TEST_CASE(reallocarray1); } void realloc1() { @@ -435,15 +434,6 @@ class TestMemleakInFunction : public TestFixture { "}"); 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) @@ -2521,12 +2511,6 @@ class TestMemleakNoVar : public TestFixture { "}"); 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" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index e5b4499e2a9..bf97ca1ced3 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -6828,6 +6828,7 @@ class TestValueFlow : public TestFixture { LOAD_LIB_2(settings.library, "std.cfg"); LOAD_LIB_2(settings.library, "posix.cfg"); + LOAD_LIB_2(settings.library, "bsd.cfg"); code = "void* f() {\n" " void* x = malloc(10);\n"