Rework fix for #11008 (#4296)

This commit is contained in:
chrchr-github 2022-07-24 12:15:04 +02:00 committed by GitHub
parent 1aceded300
commit 479533a80a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 36 additions and 19 deletions

View File

@ -356,11 +356,8 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
} }
while (ret && ret->str() == ".") while (ret && ret->str() == ".")
ret = ret->astOperand2(); ret = ret->astOperand2();
const Token* varTok = ret ? ret->astOperand1() : nullptr; if (ret && ret->str() == "=" && ret->astOperand1() && ret->astOperand1()->varId())
while (varTok && varTok->str() == ".") ret = ret->astOperand1();
varTok = varTok->astOperand2();
if (ret && ret->str() == "=" && varTok && varTok->varId())
ret = varTok;
else if (ret && ret->varId() == 0U) else if (ret && ret->varId() == 0U)
ret = nullptr; ret = nullptr;
if (vartok) if (vartok)

View File

@ -364,6 +364,8 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
// assignment.. // assignment..
if (const Token* const tokAssignOp = isAssignment(varTok)) { if (const Token* const tokAssignOp = isAssignment(varTok)) {
if (Token::simpleMatch(tokAssignOp->astOperand1(), "."))
continue;
// taking address of another variable.. // taking address of another variable..
if (Token::Match(tokAssignOp, "= %var% [+;]")) { if (Token::Match(tokAssignOp, "= %var% [+;]")) {
if (varTok->tokAt(2)->varId() != varTok->varId()) { if (varTok->tokAt(2)->varId() != varTok->varId()) {
@ -442,36 +444,33 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
continue; continue;
// Check assignments in the if-statement. Skip multiple assignments since we don't track those // Check assignments in the if-statement. Skip multiple assignments since we don't track those
const Token* const eqTok = innerTok->astParent(); if (Token::Match(innerTok, "%var% =") && innerTok->astParent() == innerTok->next() &&
if (Token::Match(innerTok, ".| %var% =") && Token::simpleMatch(eqTok, "=") && !(innerTok->next()->astParent() && innerTok->next()->astParent()->isAssignmentOp())) {
!(eqTok->astParent() && eqTok->astParent()->isAssignmentOp())) {
// allocation? // allocation?
// right ast part (after `=` operator) // right ast part (after `=` operator)
const Token* tokRightAstOperand = eqTok->astOperand2(); const Token* tokRightAstOperand = innerTok->next()->astOperand2();
while (tokRightAstOperand && tokRightAstOperand->isCast()) while (tokRightAstOperand && tokRightAstOperand->isCast())
tokRightAstOperand = tokRightAstOperand->astOperand2() ? tokRightAstOperand->astOperand2() : tokRightAstOperand->astOperand1(); 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% (")) { if (tokRightAstOperand && Token::Match(tokRightAstOperand->previous(), "%type% (")) {
const Library::AllocFunc* f = mSettings->library.getAllocFuncInfo(tokRightAstOperand->previous()); const Library::AllocFunc* f = mSettings->library.getAllocFuncInfo(tokRightAstOperand->previous());
if (f && f->arg == -1) { if (f && f->arg == -1) {
VarInfo::AllocInfo& varAlloc = alloctype[ptrTok->varId()]; VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()];
varAlloc.type = f->groupId; varAlloc.type = f->groupId;
varAlloc.status = VarInfo::ALLOC; varAlloc.status = VarInfo::ALLOC;
varAlloc.allocTok = tokRightAstOperand->previous(); varAlloc.allocTok = tokRightAstOperand->previous();
} else { } else {
// Fixme: warn about leak // Fixme: warn about leak
alloctype.erase(ptrTok->varId()); alloctype.erase(innerTok->varId());
} }
changeAllocStatusIfRealloc(alloctype, allocTok, varTok); changeAllocStatusIfRealloc(alloctype, innerTok->tokAt(2), varTok);
} else if (mTokenizer->isCPP() && Token::Match(allocTok, "new !!(")) { } else if (mTokenizer->isCPP() && Token::Match(innerTok->tokAt(2), "new !!(")) {
const Token* tok2 = allocTok->astOperand1(); const Token* tok2 = innerTok->tokAt(2)->astOperand1();
const bool arrayNew = (tok2 && (tok2->str() == "[" || (tok2->str() == "(" && tok2->astOperand1() && tok2->astOperand1()->str() == "["))); const bool arrayNew = (tok2 && (tok2->str() == "[" || (tok2->str() == "(" && tok2->astOperand1() && tok2->astOperand1()->str() == "[")));
VarInfo::AllocInfo& varAlloc = alloctype[ptrTok->varId()]; VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()];
varAlloc.type = arrayNew ? NEW_ARRAY : NEW; varAlloc.type = arrayNew ? NEW_ARRAY : NEW;
varAlloc.status = VarInfo::ALLOC; varAlloc.status = VarInfo::ALLOC;
varAlloc.allocTok = allocTok; varAlloc.allocTok = innerTok->tokAt(2);
} }
} }
@ -626,6 +625,8 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
// delete // delete
else if (mTokenizer->isCPP() && tok->str() == "delete") { else if (mTokenizer->isCPP() && tok->str() == "delete") {
const Token * delTok = tok; const Token * delTok = tok;
if (Token::simpleMatch(delTok->astOperand1(), "."))
continue;
const bool arrayDelete = Token::simpleMatch(tok->next(), "[ ]"); const bool arrayDelete = Token::simpleMatch(tok->next(), "[ ]");
if (arrayDelete) if (arrayDelete)
tok = tok->tokAt(3); tok = tok->tokAt(3);
@ -649,6 +650,8 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
VarInfo::AllocInfo allocation(af ? af->groupId : 0, VarInfo::DEALLOC, ftok); VarInfo::AllocInfo allocation(af ? af->groupId : 0, VarInfo::DEALLOC, ftok);
if (allocation.type == 0) if (allocation.type == 0)
allocation.status = VarInfo::NOALLOC; allocation.status = VarInfo::NOALLOC;
if (Token::simpleMatch(ftok->astParent(), "(") && Token::simpleMatch(ftok->astParent()->astOperand2(), "."))
continue;
functionCall(ftok, openingPar, varInfo, allocation, af); functionCall(ftok, openingPar, varInfo, allocation, af);
tok = ftok->next()->link(); tok = ftok->next()->link();

View File

@ -168,6 +168,7 @@ private:
TEST_CASE(ifelse24); // #1733 TEST_CASE(ifelse24); // #1733
TEST_CASE(ifelse25); // #9966 TEST_CASE(ifelse25); // #9966
TEST_CASE(ifelse26); TEST_CASE(ifelse26);
TEST_CASE(ifelse27);
// switch // switch
TEST_CASE(switch1); TEST_CASE(switch1);
@ -733,7 +734,7 @@ private:
" delete foo->ptr;\n" " delete foo->ptr;\n"
" x = *foo->ptr;\n" " x = *foo->ptr;\n"
"}", true); "}", true);
ASSERT_EQUALS("[test.cpp:4]: (error) Dereferencing 'ptr' after it is deallocated / released\n", errout.str()); TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Dereferencing 'ptr' after it is deallocated / released\n", "", errout.str());
check("void parse() {\n" check("void parse() {\n"
" struct Buf {\n" " struct Buf {\n"
@ -1885,6 +1886,22 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void ifelse27() {
check("struct key { void* p; };\n"
"int f(struct key** handle) {\n"
" struct key* key;\n"
" if (!(key = calloc(1, sizeof(*key))))\n"
" return 0;\n"
" if (!(key->p = malloc(4))) {\n"
" free(key);\n"
" return 0;\n"
" }\n"
" *handle = key;\n"
" return 1;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void switch1() { void switch1() {
check("void f() {\n" check("void f() {\n"
" char *p = 0;\n" " char *p = 0;\n"