From 1ced94be43a5d698462651be137a47f5885f76c9 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 23 May 2022 23:21:36 +0200 Subject: [PATCH] Fix #11019 FN memleak with redundant pointer op / #7705 FN: Memory leak not detected on struct member (#4126) * Fix #11019 FN memleak with redundant pointer op * Style * Fix #7705 FN: Memory leak not detected on struct member --- lib/checkmemoryleak.cpp | 29 ++++++++++++++++++++++++----- test/testmemleak.cpp | 26 +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 348c9387a..a61ac12c3 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -725,7 +725,7 @@ void CheckMemoryLeakStructMember::check() const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase(); for (const Variable* var : symbolDatabase->variableList()) { - if (!var || !var->isLocal() || var->isStatic() || var->isReference()) + if (!var || (!var->isLocal() && !(var->isArgument() && var->scope())) || var->isStatic() || var->isReference()) continue; if (var->typeEndToken()->isStandardType()) continue; @@ -789,7 +789,26 @@ void CheckMemoryLeakStructMember::checkStructVariable(const Variable * const var return deallocated; }; - for (const Token *tok2 = variable->nameToken(); tok2 && tok2 != variable->scope()->bodyEnd; tok2 = tok2->next()) { + // return { memberTok, rhsTok } + auto isMemberAssignment = [](const Token* varTok, int varId) -> std::pair { + if (varTok->varId() != varId) + return {}; + const Token* top = varTok->astTop(); + if (!Token::simpleMatch(top, "=")) + return {}; + const Token* dot = top->astOperand1(); + while (dot && dot->str() != ".") + dot = dot->astOperand1(); + if (!dot) + return {}; + return { dot->astOperand2(), top->next() }; + }; + std::pair assignToks; + + const Token* tokStart = variable->nameToken(); + if (variable->isArgument() && variable->scope()) + tokStart = variable->scope()->bodyStart->next(); + for (const Token *tok2 = tokStart; tok2 && tok2 != variable->scope()->bodyEnd; tok2 = tok2->next()) { if (tok2->str() == "{") ++indentlevel2; @@ -805,12 +824,12 @@ void CheckMemoryLeakStructMember::checkStructVariable(const Variable * const var break; // Struct member is allocated => check if it is also properly deallocated.. - else if (Token::Match(tok2->previous(), "[;{}] %varid% . %var% =", variable->declarationId())) { - if (getAllocationType(tok2->tokAt(4), tok2->tokAt(2)->varId()) == AllocType::No) + else if ((assignToks = isMemberAssignment(tok2, variable->declarationId())).first) { + if (getAllocationType(assignToks.second, assignToks.first->varId()) == AllocType::No) continue; const int structid(variable->declarationId()); - const int structmemberid(tok2->tokAt(2)->varId()); + const int structmemberid(assignToks.first->varId()); // This struct member is allocated.. check that it is deallocated int indentlevel3 = indentlevel2; diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 4fe96c91e..fa77369a1 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -1708,6 +1708,7 @@ private: TEST_CASE(assign1); TEST_CASE(assign2); TEST_CASE(assign3); + TEST_CASE(assign4); // #11019 // Failed allocation TEST_CASE(failedAllocation); @@ -1887,6 +1888,29 @@ private: ASSERT_EQUALS("", errout.str()); } + void assign4() { + check("struct S { int a, b, c; };\n" // #11019 + "void f() {\n" + " struct S s;\n" + " *&s.a = open(\"xx.log\", O_RDONLY);\n" + " ((s).b) = open(\"xx.log\", O_RDONLY);\n" + " (&s)->c = open(\"xx.log\", O_RDONLY);\n" + "}\n", false); + ASSERT_EQUALS("[test.c:7]: (error) Memory leak: s.a\n" + "[test.c:7]: (error) Memory leak: s.b\n" + "[test.c:7]: (error) Memory leak: s.c\n", + errout.str()); + + check("struct S { int *p, *q; };\n" // #7705 + "void f(S s) {\n" + " s.p = new int[10];\n" + " s.q = malloc(40);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Memory leak: s.p\n" + "[test.cpp:5]: (error) Memory leak: s.q\n", + errout.str()); + } + void failedAllocation() { check("static struct ABC * foo()\n" "{\n" @@ -2137,7 +2161,7 @@ private: " ((f)->realm) = strdup(realm);\n" " if(f->realm == NULL) {}\n" "}", false); - TODO_ASSERT_EQUALS("[test.c:6]: (error) Memory leak: f.realm\n", "", errout.str()); + ASSERT_EQUALS("[test.c:6]: (error) Memory leak: f.realm\n", errout.str()); } void customAllocation() { // #4770