Fix defaulted and deleted functions (#4540)

* Fix 9392, but for destructors: out-of-line defaulted destructors skipped everything after

Context:
```
struct S {
    ~S();
};

S::~S() = default;

void g() {
    int j;
    ++j;
}
```
Everything after `S::~S() = default;` was skipped, so the uninitialized variables in g() weren't found.

Out-of-line destructors are useful e.g. when you have a forward declared unique_ptr in the .h,
and `= default` the destructor in the .cpp, so only the cpp needs to know the header for destructing
your unique_ptr (like in the pImpl-idiom)

* Fix unit test, by correctly fixing 10789

Previous commit broke this test, but also provided the tools for a cleaner fix

* Document current behaviour

* Rewrite control flow

* Fix deleted functions, which skipped everything after

`a::b f() = delete` triggered the final else in SymbolDatabase::addNewFunction,
which sets tok to nullptr, effectively skipping to the end of the stream.

* Remove troublesome nullptr, which skips every analysis afterwards

It was introduced in 0746c241 to fix a memory leak.
But setting tok to nullptr, effectively skipping to the end, seems not needed.

Previous commits fixes prevented some cases where you could enter the `else`.
This commit is more of a fall back.

* fixup! Fix deleted functions, which skipped everything after

`a::b f() = delete` triggered the final else in SymbolDatabase::addNewFunction,
which sets tok to nullptr, effectively skipping to the end of the stream.

* fixup! Fix deleted functions, which skipped everything after

`a::b f() = delete` triggered the final else in SymbolDatabase::addNewFunction,
which sets tok to nullptr, effectively skipping to the end of the stream.

* Make it heard when encountering unexpected syntax/tokens

Co-authored-by: Gerbo Engels <gerbo.engels@ortec-finance.com>
This commit is contained in:
gerboengels 2022-10-10 20:17:33 +02:00 committed by GitHub
parent 65cd6ea7b4
commit ed06e29f7a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 57 additions and 29 deletions

View File

@ -577,14 +577,12 @@ void CheckMemoryLeakInClass::variable(const Scope *scope, const Token *tokVarnam
const bool constructor = func.isConstructor(); const bool constructor = func.isConstructor();
const bool destructor = func.isDestructor(); const bool destructor = func.isDestructor();
if (!func.hasBody()) { if (!func.hasBody()) {
if (destructor) { // implementation for destructor is not seen => assume it deallocates all variables properly if (destructor && !func.isDefault()) { // implementation for destructor is not seen and not defaulted => assume it deallocates all variables properly
deallocInDestructor = true; deallocInDestructor = true;
memberDealloc = CheckMemoryLeak::Many; memberDealloc = CheckMemoryLeak::Many;
} }
continue; continue;
} }
if (!func.functionScope) // defaulted destructor
continue;
bool body = false; bool body = false;
const Token *end = func.functionScope->bodyEnd; const Token *end = func.functionScope->bodyEnd;
for (const Token *tok = func.arg->link(); tok != end; tok = tok->next()) { for (const Token *tok = func.arg->link(); tok != end; tok = tok->next()) {

View File

@ -3167,10 +3167,6 @@ void SymbolDatabase::addClassFunction(Scope **scope, const Token **tok, const To
Function * func = const_cast<Function *>(it->second); Function * func = const_cast<Function *>(it->second);
if (!func->hasBody()) { if (!func->hasBody()) {
if (func->argsMatch(scope1, func->argDef, (*tok)->next(), path, path_length)) { if (func->argsMatch(scope1, func->argDef, (*tok)->next(), path, path_length)) {
if (func->type == Function::eDestructor && destructor) {
func->hasBody(true);
} else if (func->type != Function::eDestructor && !destructor) {
// normal function?
const Token *closeParen = (*tok)->next()->link(); const Token *closeParen = (*tok)->next()->link();
if (closeParen) { if (closeParen) {
const Token *eq = mTokenizer->isFunctionHead(closeParen, ";"); const Token *eq = mTokenizer->isFunctionHead(closeParen, ";");
@ -3178,7 +3174,10 @@ void SymbolDatabase::addClassFunction(Scope **scope, const Token **tok, const To
func->isDefault(true); func->isDefault(true);
return; return;
} }
if (func->type == Function::eDestructor && destructor) {
func->hasBody(true);
} else if (func->type != Function::eDestructor && !destructor) {
// normal function?
const bool hasConstKeyword = closeParen->next()->str() == "const"; const bool hasConstKeyword = closeParen->next()->str() == "const";
if ((func->isConst() == hasConstKeyword) && if ((func->isConst() == hasConstKeyword) &&
(func->hasLvalRefQualifier() == lval) && (func->hasLvalRefQualifier() == lval) &&
@ -3234,22 +3233,17 @@ void SymbolDatabase::addNewFunction(Scope **scope, const Token **tok)
// syntax error? // syntax error?
if (!newScope->bodyEnd) { if (!newScope->bodyEnd) {
scopeList.pop_back(); mTokenizer->unmatchedToken(tok1);
while (tok1->next()) } else {
tok1 = tok1->next();
*scope = nullptr;
*tok = tok1;
return;
}
(*scope)->nestedList.push_back(newScope); (*scope)->nestedList.push_back(newScope);
*scope = newScope; *scope = newScope;
*tok = tok1;
} else {
scopeList.pop_back();
*scope = nullptr;
*tok = nullptr;
} }
} else if (tok1 && Token::Match(tok1->tokAt(-2), "= default|delete ;")) {
scopeList.pop_back();
} else {
throw InternalError(*tok, "Analysis failed (function not recognized). If the code is valid then please report this failure.");
}
*tok = tok1;
} }
bool Type::isClassType() const bool Type::isClassType() const

View File

@ -1613,7 +1613,7 @@ static Token * createAstAtToken(Token *tok, bool cpp)
Token::Match(typetok->previous(), "%name% ( !!*") && Token::Match(typetok->previous(), "%name% ( !!*") &&
typetok->previous()->varId() == 0 && typetok->previous()->varId() == 0 &&
!typetok->previous()->isKeyword() && !typetok->previous()->isKeyword() &&
Token::Match(typetok->link(), ") const|;|{")) (Token::Match(typetok->link(), ") const|;|{") || Token::Match(typetok->link(), ") const| = delete ;")))
return typetok; return typetok;
} }

View File

@ -363,6 +363,7 @@ private:
TEST_CASE(symboldatabase99); // #10864 TEST_CASE(symboldatabase99); // #10864
TEST_CASE(symboldatabase100); // #10174 TEST_CASE(symboldatabase100); // #10174
TEST_CASE(symboldatabase101); TEST_CASE(symboldatabase101);
TEST_CASE(symboldatabase102);
TEST_CASE(createSymbolDatabaseFindAllScopes1); TEST_CASE(createSymbolDatabaseFindAllScopes1);
TEST_CASE(createSymbolDatabaseFindAllScopes2); TEST_CASE(createSymbolDatabaseFindAllScopes2);
@ -4581,6 +4582,25 @@ private:
ASSERT(db->scopeList.back().functionList.size() == 1); ASSERT(db->scopeList.back().functionList.size() == 1);
ASSERT(db->scopeList.back().functionList.front().isDefault() == true); ASSERT(db->scopeList.back().functionList.front().isDefault() == true);
} }
{
GET_SYMBOL_DB("class C { ~C(); };\n"
"C::~C() = default;");
ASSERT(db->scopeList.size() == 2);
ASSERT(db->scopeList.back().functionList.size() == 1);
ASSERT(db->scopeList.back().functionList.front().isDefault() == true);
ASSERT(db->scopeList.back().functionList.front().isDestructor() == true);
}
{
GET_SYMBOL_DB("namespace ns {\n"
"class C { ~C(); };\n"
"}\n"
"using namespace ns;\n"
"C::~C() = default;");
ASSERT(db->scopeList.size() == 3);
ASSERT(db->scopeList.back().functionList.size() == 1);
ASSERT(db->scopeList.back().functionList.front().isDefault() == true);
ASSERT(db->scopeList.back().functionList.front().isDestructor() == true);
}
} }
void symboldatabase80() { // #9389 void symboldatabase80() { // #9389
@ -5013,6 +5033,15 @@ private:
ASSERT(it->tokAt(2)->variable()); ASSERT(it->tokAt(2)->variable());
} }
void symboldatabase102() {
GET_SYMBOL_DB("std::string f() = delete;\n"
"void g() {}");
ASSERT(db);
ASSERT(db->scopeList.size() == 2);
ASSERT(db->scopeList.front().type == Scope::eGlobal);
ASSERT(db->scopeList.back().className == "g");
}
void createSymbolDatabaseFindAllScopes1() { void createSymbolDatabaseFindAllScopes1() {
GET_SYMBOL_DB("void f() { union {int x; char *p;} a={0}; }"); GET_SYMBOL_DB("void f() { union {int x; char *p;} a={0}; }");
ASSERT(db->scopeList.size() == 3); ASSERT(db->scopeList.size() == 3);

View File

@ -6321,6 +6321,13 @@ private:
ASSERT_EQUALS("1f23,(+4+", testAst("1+f(2,3)+4")); ASSERT_EQUALS("1f23,(+4+", testAst("1+f(2,3)+4"));
ASSERT_EQUALS("1f2a&,(+", testAst("1+f(2,&a)")); ASSERT_EQUALS("1f2a&,(+", testAst("1+f(2,&a)"));
ASSERT_EQUALS("argv[", testAst("int f(char argv[]);")); ASSERT_EQUALS("argv[", testAst("int f(char argv[]);"));
ASSERT_EQUALS("", testAst("void f();"));
ASSERT_EQUALS("", testAst("void f() {}"));
ASSERT_EQUALS("", testAst("int f() = delete;"));
ASSERT_EQUALS("", testAst("a::b f();"));
ASSERT_EQUALS("", testAst("a::b f() {}"));
ASSERT_EQUALS("", testAst("a::b f() = delete;"));
ASSERT_EQUALS("constdelete=", testAst("int f() const = delete;"));
ASSERT_EQUALS("", testAst("extern unsigned f(const char *);")); ASSERT_EQUALS("", testAst("extern unsigned f(const char *);"));
ASSERT_EQUALS("charformat*...,", testAst("extern void f(const char *format, ...);")); ASSERT_EQUALS("charformat*...,", testAst("extern void f(const char *format, ...);"));
ASSERT_EQUALS("int((void,", testAst("extern int for_each_commit_graft(int (*)(int*), void *);")); ASSERT_EQUALS("int((void,", testAst("extern int for_each_commit_graft(int (*)(int*), void *);"));