Skip to content

Commit c736fe8

Browse files
Fix #11008 FP doubleFree with pointer in struct (#4294)
1 parent 30b20d1 commit c736fe8

3 files changed

Lines changed: 33 additions & 12 deletions

File tree

lib/astutils.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,11 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
356356
}
357357
while (ret && ret->str() == ".")
358358
ret = ret->astOperand2();
359-
if (ret && ret->str() == "=" && ret->astOperand1() && ret->astOperand1()->varId())
360-
ret = ret->astOperand1();
359+
const Token* varTok = ret ? ret->astOperand1() : nullptr;
360+
while (varTok && varTok->str() == ".")
361+
varTok = varTok->astOperand2();
362+
if (ret && ret->str() == "=" && varTok && varTok->varId())
363+
ret = varTok;
361364
else if (ret && ret->varId() == 0U)
362365
ret = nullptr;
363366
if (vartok)

lib/checkleakautovar.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -442,33 +442,36 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
442442
continue;
443443

444444
// Check assignments in the if-statement. Skip multiple assignments since we don't track those
445-
if (Token::Match(innerTok, "%var% =") && innerTok->astParent() == innerTok->next() &&
446-
!(innerTok->next()->astParent() && innerTok->next()->astParent()->isAssignmentOp())) {
445+
const Token* const eqTok = innerTok->astParent();
446+
if (Token::Match(innerTok, ".| %var% =") && Token::simpleMatch(eqTok, "=") &&
447+
!(eqTok->astParent() && eqTok->astParent()->isAssignmentOp())) {
447448
// allocation?
448449
// right ast part (after `=` operator)
449-
const Token* tokRightAstOperand = innerTok->next()->astOperand2();
450+
const Token* tokRightAstOperand = eqTok->astOperand2();
450451
while (tokRightAstOperand && tokRightAstOperand->isCast())
451452
tokRightAstOperand = tokRightAstOperand->astOperand2() ? tokRightAstOperand->astOperand2() : tokRightAstOperand->astOperand1();
453+
const Token* const allocTok = Token::simpleMatch(eqTok->astOperand2(), "(") ? eqTok->astOperand2()->astOperand1() : eqTok->astOperand2();
454+
const Token* const ptrTok = innerTok->astOperand2() ? innerTok->astOperand2() : innerTok;
452455
if (tokRightAstOperand && Token::Match(tokRightAstOperand->previous(), "%type% (")) {
453456
const Library::AllocFunc* f = mSettings->library.getAllocFuncInfo(tokRightAstOperand->previous());
454457
if (f && f->arg == -1) {
455-
VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()];
458+
VarInfo::AllocInfo& varAlloc = alloctype[ptrTok->varId()];
456459
varAlloc.type = f->groupId;
457460
varAlloc.status = VarInfo::ALLOC;
458461
varAlloc.allocTok = tokRightAstOperand->previous();
459462
} else {
460463
// Fixme: warn about leak
461-
alloctype.erase(innerTok->varId());
464+
alloctype.erase(ptrTok->varId());
462465
}
463466

464-
changeAllocStatusIfRealloc(alloctype, innerTok->tokAt(2), varTok);
465-
} else if (mTokenizer->isCPP() && Token::Match(innerTok->tokAt(2), "new !!(")) {
466-
const Token* tok2 = innerTok->tokAt(2)->astOperand1();
467+
changeAllocStatusIfRealloc(alloctype, allocTok, varTok);
468+
} else if (mTokenizer->isCPP() && Token::Match(allocTok, "new !!(")) {
469+
const Token* tok2 = allocTok->astOperand1();
467470
const bool arrayNew = (tok2 && (tok2->str() == "[" || (tok2->str() == "(" && tok2->astOperand1() && tok2->astOperand1()->str() == "[")));
468-
VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()];
471+
VarInfo::AllocInfo& varAlloc = alloctype[ptrTok->varId()];
469472
varAlloc.type = arrayNew ? NEW_ARRAY : NEW;
470473
varAlloc.status = VarInfo::ALLOC;
471-
varAlloc.allocTok = innerTok->tokAt(2);
474+
varAlloc.allocTok = allocTok;
472475
}
473476
}
474477

test/testleakautovar.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ class TestLeakAutoVar : public TestFixture {
127127
TEST_CASE(doublefree10); // #8706
128128
TEST_CASE(doublefree11);
129129
TEST_CASE(doublefree12); // #10502
130+
TEST_CASE(doublefree13); // #11008
130131

131132
// exit
132133
TEST_CASE(exit1);
@@ -1396,6 +1397,20 @@ class TestLeakAutoVar : public TestFixture {
13961397
ASSERT_EQUALS("", errout.str());
13971398
}
13981399

1400+
void doublefree13() { // #11008
1401+
check("struct buf_t { void* ptr; };\n"
1402+
"void f() {\n"
1403+
" struct buf_t buf;\n"
1404+
" if ((buf.ptr = malloc(10)) == NULL)\n"
1405+
" return;\n"
1406+
" free(buf.ptr);\n"
1407+
" if ((buf.ptr = malloc(10)) == NULL)\n"
1408+
" return;\n"
1409+
" free(buf.ptr);\n"
1410+
"}\n");
1411+
ASSERT_EQUALS("", errout.str());
1412+
}
1413+
13991414
void exit1() {
14001415
check("void f() {\n"
14011416
" char *p = malloc(10);\n"

0 commit comments

Comments
 (0)