From c736fe87877a98f42ac6c52ee7ee5dd817d2e9af Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Tue, 19 Jul 2022 20:41:08 +0200 Subject: [PATCH] Fix #11008 FP doubleFree with pointer in struct (#4294) --- lib/astutils.cpp | 7 +++++-- lib/checkleakautovar.cpp | 23 +++++++++++++---------- test/testleakautovar.cpp | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index e7c693f10..b3bc58dda 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -356,8 +356,11 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp, } while (ret && ret->str() == ".") ret = ret->astOperand2(); - if (ret && ret->str() == "=" && ret->astOperand1() && ret->astOperand1()->varId()) - ret = ret->astOperand1(); + const Token* varTok = ret ? ret->astOperand1() : nullptr; + while (varTok && varTok->str() == ".") + varTok = varTok->astOperand2(); + if (ret && ret->str() == "=" && varTok && varTok->varId()) + ret = varTok; else if (ret && ret->varId() == 0U) ret = nullptr; if (vartok) diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index 3ba2da1a5..af61b2bed 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -442,33 +442,36 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, continue; // Check assignments in the if-statement. Skip multiple assignments since we don't track those - if (Token::Match(innerTok, "%var% =") && innerTok->astParent() == innerTok->next() && - !(innerTok->next()->astParent() && innerTok->next()->astParent()->isAssignmentOp())) { + const Token* const eqTok = innerTok->astParent(); + if (Token::Match(innerTok, ".| %var% =") && Token::simpleMatch(eqTok, "=") && + !(eqTok->astParent() && eqTok->astParent()->isAssignmentOp())) { // allocation? // right ast part (after `=` operator) - const Token* tokRightAstOperand = innerTok->next()->astOperand2(); + const Token* tokRightAstOperand = eqTok->astOperand2(); while (tokRightAstOperand && tokRightAstOperand->isCast()) tokRightAstOperand = tokRightAstOperand->astOperand2() ? tokRightAstOperand->astOperand2() : tokRightAstOperand->astOperand1(); + const Token* const allocTok = Token::simpleMatch(eqTok->astOperand2(), "(") ? eqTok->astOperand2()->astOperand1() : eqTok->astOperand2(); + const Token* const ptrTok = innerTok->astOperand2() ? innerTok->astOperand2() : innerTok; if (tokRightAstOperand && Token::Match(tokRightAstOperand->previous(), "%type% (")) { const Library::AllocFunc* f = mSettings->library.getAllocFuncInfo(tokRightAstOperand->previous()); if (f && f->arg == -1) { - VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()]; + VarInfo::AllocInfo& varAlloc = alloctype[ptrTok->varId()]; varAlloc.type = f->groupId; varAlloc.status = VarInfo::ALLOC; varAlloc.allocTok = tokRightAstOperand->previous(); } else { // Fixme: warn about leak - alloctype.erase(innerTok->varId()); + alloctype.erase(ptrTok->varId()); } - changeAllocStatusIfRealloc(alloctype, innerTok->tokAt(2), varTok); - } else if (mTokenizer->isCPP() && Token::Match(innerTok->tokAt(2), "new !!(")) { - const Token* tok2 = innerTok->tokAt(2)->astOperand1(); + changeAllocStatusIfRealloc(alloctype, allocTok, varTok); + } else if (mTokenizer->isCPP() && Token::Match(allocTok, "new !!(")) { + const Token* tok2 = allocTok->astOperand1(); const bool arrayNew = (tok2 && (tok2->str() == "[" || (tok2->str() == "(" && tok2->astOperand1() && tok2->astOperand1()->str() == "["))); - VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()]; + VarInfo::AllocInfo& varAlloc = alloctype[ptrTok->varId()]; varAlloc.type = arrayNew ? NEW_ARRAY : NEW; varAlloc.status = VarInfo::ALLOC; - varAlloc.allocTok = innerTok->tokAt(2); + varAlloc.allocTok = allocTok; } } diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp index cd46b070b..363b3e0e5 100644 --- a/test/testleakautovar.cpp +++ b/test/testleakautovar.cpp @@ -127,6 +127,7 @@ private: TEST_CASE(doublefree10); // #8706 TEST_CASE(doublefree11); TEST_CASE(doublefree12); // #10502 + TEST_CASE(doublefree13); // #11008 // exit TEST_CASE(exit1); @@ -1396,6 +1397,20 @@ private: ASSERT_EQUALS("", errout.str()); } + void doublefree13() { // #11008 + check("struct buf_t { void* ptr; };\n" + "void f() {\n" + " struct buf_t buf;\n" + " if ((buf.ptr = malloc(10)) == NULL)\n" + " return;\n" + " free(buf.ptr);\n" + " if ((buf.ptr = malloc(10)) == NULL)\n" + " return;\n" + " free(buf.ptr);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void exit1() { check("void f() {\n" " char *p = malloc(10);\n"