walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fixed #5807 (non virtual dtor in virtual class)

This commit is contained in:
Robert Reif 2014-06-14 12:55:20 +02:00 committed by Daniel Marjamäki
parent 17f1841fba
commit 1f09cb0c30
3 changed files with 75 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
lib/checkclass.cpp
View File

@ -1381,14 +1381,31 @@ void CheckClass::virtualDestructor()
// * base class doesn't have virtual destructor
// * derived class has non-empty destructor
// * base class is deleted
// unless inconclusive in which case:
// * base class has virtual members but doesn't have virtual destructor
std::list<const Function *> inconclusive_errors;
const std::size_t classes = symbolDatabase->classAndStructScopes.size();
for (std::size_t i = 0; i < classes; ++i) {
const Scope * scope = symbolDatabase->classAndStructScopes[i];
// Skip base classes
if (scope->definedType->derivedFrom.empty())
// Skip base classes (unless inconclusive)
if (scope->definedType->derivedFrom.empty()) {
if (_settings->inconclusive) {
const Function *destructor = scope->getDestructor();
if (destructor && !destructor->isVirtual) {
std::list<Function>::const_iterator func;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->isVirtual) {
inconclusive_errors.push_back(destructor);
break;
}
}
}
}
continue;
}
// Find the destructor
const Function *destructor = scope->getDestructor();
@ -1466,8 +1483,13 @@ void CheckClass::virtualDestructor()
// Check that there is a destructor..
if (!base_destructor) {
if (derivedFrom->derivedFrom.empty())
virtualDestructorError(derivedFrom->classDef, derivedFrom->name(), derivedClass->str());
if (derivedFrom->derivedFrom.empty()) {
virtualDestructorError(derivedFrom->classDef, derivedFrom->name(), derivedClass->str(), false);
// check for duplicate error and remove if if found
std::list<const Function *>::iterator found = find(inconclusive_errors.begin(), inconclusive_errors.end(), base_destructor);
if (found != inconclusive_errors.end())
inconclusive_errors.erase(found);
}
} else if (!base_destructor->isVirtual) {
// TODO: This is just a temporary fix, better solution is needed.
// Skip situations where base class has base classes of its own, because
@ -1479,22 +1501,33 @@ void CheckClass::virtualDestructor()
// Make sure that the destructor is public (protected or private
// would not compile if inheritance is used in a way that would
// cause the bug we are trying to find here.)
if (base_destructor->access == Public)
virtualDestructorError(base, derivedFrom->name(), derivedClass->str());
if (base_destructor->access == Public) {
virtualDestructorError(base, derivedFrom->name(), derivedClass->str(), false);
// check for duplicate error and remove if if found
std::list<const Function *>::iterator found = find(inconclusive_errors.begin(), inconclusive_errors.end(), base_destructor);
if (found != inconclusive_errors.end())
inconclusive_errors.erase(found);
}
}
}
}
}
}
for (std::list<const Function *>::const_iterator i = inconclusive_errors.begin(); i != inconclusive_errors.end(); ++i)
virtualDestructorError((*i)->tokenDef, (*i)->name(), "", true);
}
void CheckClass::virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived)
void CheckClass::virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived, bool inconclusive)
{
reportError(tok, Severity::error, "virtualDestructor", "Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor.\n"
"Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor. "
"If you destroy instances of the derived class by deleting a pointer that points to the base class, only "
"the destructor of the base class is executed. Thus, dynamic memory that is managed by the derived class "
"could leak. This can be avoided by adding a virtual destructor to the base class.");
if (inconclusive)
reportError(tok, Severity::warning, "virtualDestructor", "Class '" + Base + "' which has virtual members does not have a virtual destructor.", true);
else
reportError(tok, Severity::error, "virtualDestructor", "Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor.\n"
"Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor. "
"If you destroy instances of the derived class by deleting a pointer that points to the base class, only "
"the destructor of the base class is executed. Thus, dynamic memory that is managed by the derived class "
"could leak. This can be avoided by adding a virtual destructor to the base class.");
}
//---------------------------------------------------------------------------

4
lib/checkclass.h
View File

@ -142,7 +142,7 @@ private:
void mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname);
void mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok);
void operatorEqReturnError(const Token *tok, const std::string &className);
void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived);
void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived, bool inconclusive);
void thisSubtractionError(const Token *tok);
void operatorEqRetRefThisError(const Token *tok);
void operatorEqToSelfError(const Token *tok);
@ -166,7 +166,7 @@ private:
c.mallocOnClassWarning(0, "malloc", 0);
c.mallocOnClassError(0, "malloc", 0, "std::string");
c.operatorEqReturnError(0, "class");
c.virtualDestructorError(0, "Base", "Derived");
c.virtualDestructorError(0, "Base", "Derived", false);
c.thisSubtractionError(0);
c.operatorEqRetRefThisError(0);
c.operatorEqToSelfError(0);

29
test/testclass.cpp
View File

@ -43,6 +43,8 @@ private:
TEST_CASE(virtualDestructorInherited);
TEST_CASE(virtualDestructorTemplate);
TEST_CASE(virtualDestructorInconclusive); // ticket # 5807
TEST_CASE(copyConstructor1);
TEST_CASE(copyConstructor2); // ticket #4458
@ -1737,11 +1739,12 @@ private:
}
// Check that base classes have virtual destructors
void checkVirtualDestructor(const char code[]) {
void checkVirtualDestructor(const char code[], bool inconclusive = false) {
// Clear the error log
errout.str("");
Settings settings;
settings.inconclusive = inconclusive;
// Tokenize..
Tokenizer tokenizer(&settings, this);
@ -1991,6 +1994,30 @@ private:
ASSERT_EQUALS("[test.cpp:9]: (error) Class 'AA<double>' which is inherited by class 'B' does not have a virtual destructor.\n", errout.str());
}
void virtualDestructorInconclusive() {
checkVirtualDestructor("class Base {\n"
"public:\n"
" ~Base(){}\n"
" virtual void foo(){}\n"
"};\n", true);
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Class 'Base' which has virtual members does not have a virtual destructor.\n", errout.str());
checkVirtualDestructor("class Base {\n"
"public:\n"
" ~Base(){}\n"
" virtual void foo(){}\n"
"};\n"
"class Derived : public Base {\n"
"public:\n"
" ~Derived() { bar(); }\n"
"};\n"
"void foo() {\n"
" Base * base = new Derived();\n"
" delete base;\n"
"}\n", true);
ASSERT_EQUALS("[test.cpp:3]: (error) Class 'Base' which is inherited by class 'Derived' does not have a virtual destructor.\n", errout.str());
}
void checkNoConstructor(const char code[], const char* level="style") {
// Clear the error log
errout.str("");