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

Improve check: Warn about virtual function calls in constructor/destructor

This commit is contained in:
Daniel Marjamäki 2018-04-02 15:31:47 +02:00
parent e492932f19
commit 1046ca2120
3 changed files with 279 additions and 248 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

136
lib/checkclass.cpp
View File

@ -64,11 +64,6 @@ static const char * getFunctionTypeName(Function::Type type)
return "";
}
static bool isPureWithoutBody(Function const & func)
{
return func.isPure() && !func.hasBody();
}
//---------------------------------------------------------------------------
CheckClass::CheckClass(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
@ -2200,15 +2195,15 @@ void CheckClass::selfInitializationError(const Token* tok, const std::string& va
//---------------------------------------------------------------------------
// Check for pure virtual function calls
// Check for virtual function calls in constructor/destructor
//---------------------------------------------------------------------------
void CheckClass::checkPureVirtualFunctionCall()
void CheckClass::checkVirtualFunctionCallInConstructor()
{
if (! _settings->isEnabled(Settings::WARNING))
return;
const std::size_t functions = symbolDatabase->functionScopes.size();
std::map<const Function *, std::list<const Token *> > callsPureVirtualFunctionMap;
std::map<const Function *, std::list<const Token *> > virtualFunctionCallsMap;
for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i];
if (scope->function == nullptr || !scope->function->hasBody() ||
@ -2216,40 +2211,42 @@ void CheckClass::checkPureVirtualFunctionCall()
scope->function->isDestructor()))
continue;
const std::list<const Token *> & pureVirtualFunctionCalls=callsPureVirtualFunction(*scope->function,callsPureVirtualFunctionMap);
for (std::list<const Token *>::const_iterator pureCallIter=pureVirtualFunctionCalls.begin();
pureCallIter!=pureVirtualFunctionCalls.end();
++pureCallIter) {
const Token & pureCall=**pureCallIter;
std::list<const Token *> pureFuncStack;
pureFuncStack.push_back(&pureCall);
getFirstPureVirtualFunctionCallStack(callsPureVirtualFunctionMap, pureCall, pureFuncStack);
if (!pureFuncStack.empty())
callsPureVirtualFunctionError(*scope->function, pureFuncStack, pureFuncStack.back()->str());
const std::list<const Token *> & virtualFunctionCalls = getVirtualFunctionCalls(*scope->function, virtualFunctionCallsMap);
for (std::list<const Token *>::const_iterator it = virtualFunctionCalls.begin(); it != virtualFunctionCalls.end(); ++it) {
const Token * callToken = *it;
std::list<const Token *> callstack;
callstack.push_back(callToken);
getFirstVirtualFunctionCallStack(virtualFunctionCallsMap, callToken, callstack);
if (callstack.empty())
continue;
if (callstack.back()->function()->isPure())
pureVirtualFunctionCallInConstructorError(scope->function, callstack, callstack.back()->str());
else
virtualFunctionCallInConstructorError(scope->function, callstack, callstack.back()->str());
}
}
}
const std::list<const Token *> & CheckClass::callsPureVirtualFunction(const Function & function,
std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap)
const std::list<const Token *> & CheckClass::getVirtualFunctionCalls(const Function & function,
std::map<const Function *, std::list<const Token *> > & virtualFunctionCallsMap)
{
std::pair<std::map<const Function *, std::list<const Token *> >::iterator, bool > found =
callsPureVirtualFunctionMap.insert(std::pair<const Function *, std::list< const Token *> >(&function, std::list<const Token *>()));
std::list<const Token *> & pureFunctionCalls = found.first->second;
if (found.second) {
if (function.hasBody()) {
for (const Token *tok = function.arg->link();
tok && tok != function.functionScope->classEnd;
tok = tok->next()) {
const std::map<const Function *, std::list<const Token *> >::const_iterator found = virtualFunctionCallsMap.find(&function);
if (found != virtualFunctionCallsMap.end())
return found->second;
virtualFunctionCallsMap[&function] = std::list<const Token *>();
std::list<const Token *> & virtualFunctionCalls = virtualFunctionCallsMap.find(&function)->second;
if (!function.hasBody())
return virtualFunctionCalls;
for (const Token *tok = function.arg->link(); tok != function.functionScope->classEnd; tok = tok->next()) {
if (function.type != Function::eConstructor &&
function.type != Function::eCopyConstructor &&
function.type != Function::eMoveConstructor &&
function.type != Function::eDestructor) {
if ((Token::simpleMatch(tok, ") {") &&
tok->link() &&
Token::Match(tok->link()->previous(), "if|switch")) ||
Token::simpleMatch(tok, "else {")
) {
if ((Token::simpleMatch(tok, ") {") && tok->link() && Token::Match(tok->link()->previous(), "if|switch")) ||
Token::simpleMatch(tok, "else {")) {
// Assume pure virtual function call is prevented by "if|else|switch" condition
tok = tok->linkAt(1);
continue;
@ -2273,49 +2270,68 @@ const std::list<const Token *> & CheckClass::callsPureVirtualFunction(const Func
continue;
}
if (isPureWithoutBody(*callFunction)) {
pureFunctionCalls.push_back(tok);
if (callFunction->isVirtual()) {
virtualFunctionCalls.push_back(tok);
continue;
}
const std::list<const Token *> & pureFunctionCallsOfTok = callsPureVirtualFunction(*callFunction,
callsPureVirtualFunctionMap);
if (!pureFunctionCallsOfTok.empty()) {
pureFunctionCalls.push_back(tok);
continue;
const std::list<const Token *> & virtualFunctionCallsOfTok = getVirtualFunctionCalls(*callFunction, virtualFunctionCallsMap);
if (!virtualFunctionCallsOfTok.empty())
virtualFunctionCalls.push_back(tok);
}
}
}
}
return pureFunctionCalls;
return virtualFunctionCalls;
}
void CheckClass::getFirstPureVirtualFunctionCallStack(
std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap,
const Token & pureCall,
std::list<const Token *> & pureFuncStack)
void CheckClass::getFirstVirtualFunctionCallStack(
std::map<const Function *, std::list<const Token *> > & virtualFunctionCallsMap,
const Token * callToken,
std::list<const Token *> & callstack)
{
if (isPureWithoutBody(*pureCall.function())) {
pureFuncStack.push_back(pureCall.function()->token);
const Function *callFunction = callToken->function();
if (callFunction->isVirtual() && (!callFunction->isPure() || !callFunction->hasBody())) {
callstack.push_back(callFunction->token);
return;
}
std::map<const Function *, std::list<const Token *> >::const_iterator found = callsPureVirtualFunctionMap.find(pureCall.function());
if (found == callsPureVirtualFunctionMap.end() ||
found->second.empty()) {
pureFuncStack.clear();
std::map<const Function *, std::list<const Token *> >::const_iterator found = virtualFunctionCallsMap.find(callFunction);
if (found == virtualFunctionCallsMap.end() || found->second.empty()) {
callstack.clear();
return;
}
const Token & firstPureCall = **found->second.begin();
pureFuncStack.push_back(&firstPureCall);
getFirstPureVirtualFunctionCallStack(callsPureVirtualFunctionMap, firstPureCall, pureFuncStack);
const Token * firstCall = *found->second.begin();
callstack.push_back(firstCall);
getFirstVirtualFunctionCallStack(virtualFunctionCallsMap, firstCall, callstack);
}
void CheckClass::callsPureVirtualFunctionError(
const Function & scopeFunction,
void CheckClass::virtualFunctionCallInConstructorError(
const Function * scopeFunction,
const std::list<const Token *> & tokStack,
const std::string &funcname)
{
const char * scopeFunctionTypeName = scopeFunction ? getFunctionTypeName(scopeFunction->type) : "constructor";
ErrorPath errorPath;
for (std::list<const Token *>::const_iterator it = tokStack.begin(); it != tokStack.end(); ++it)
errorPath.push_back(ErrorPathItem(*it, "Calling " + (*it)->str()));
if (!errorPath.empty())
errorPath.back().second = funcname + " is a virtual method";
reportError(errorPath, Severity::warning, "virtualCallInConstructor", "Call of virtual function '" + funcname + "' in " + scopeFunctionTypeName + ".\n"
"Call of pure virtual function '" + funcname + "' in " + scopeFunctionTypeName + ". Dynamic binding is not used.", CWE(0U), false);
}
void CheckClass::pureVirtualFunctionCallInConstructorError(
const Function * scopeFunction,
const std::list<const Token *> & tokStack,
const std::string &purefuncname)
{
const char * scopeFunctionTypeName = getFunctionTypeName(scopeFunction.type);
const char * scopeFunctionTypeName = scopeFunction ? getFunctionTypeName(scopeFunction->type) : "constructor";
ErrorPath errorPath;
for (std::list<const Token *>::const_iterator it = tokStack.begin(); it != tokStack.end(); ++it)
errorPath.push_back(ErrorPathItem(*it, "Calling " + (*it)->str()));
if (!errorPath.empty())
errorPath.back().second = purefuncname + " is a pure virtual method without body";
reportError(tokStack, Severity::warning, "pureVirtualCall", "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ".\n"
"Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ". The call will fail during runtime.", CWE(0U), false);
}

27
lib/checkclass.h
View File

@ -85,7 +85,7 @@ public:
checkClass.virtualDestructor();
checkClass.checkConst();
checkClass.copyconstructors();
checkClass.checkPureVirtualFunctionCall();
checkClass.checkVirtualFunctionCallInConstructor();
checkClass.checkDuplInheritedMembers();
checkClass.checkExplicitConstructors();
@ -143,8 +143,8 @@ public:
void copyconstructors();
/** @brief call of pure virtual function */
void checkPureVirtualFunctionCall();
/** @brief call of virtual function in constructor/destructor */
void checkVirtualFunctionCallInConstructor();
/** @brief Check duplicated inherited members */
void checkDuplInheritedMembers();
@ -184,7 +184,8 @@ private:
void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname);
void suggestInitializationList(const Token *tok, const std::string& varname);
void selfInitializationError(const Token* tok, const std::string& varname);
void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
void pureVirtualFunctionCallInConstructorError(const Function * scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
void virtualFunctionCallInConstructorError(const Function * scopeFunction, const std::list<const Token *> & tokStack, const std::string &funcname);
void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedname, const std::string &basename, const std::string &variablename, bool derivedIsStruct, bool baseIsStruct);
void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor);
void unsafeClassDivZeroError(const Token *tok, const std::string &className, const std::string &methodName, const std::string &varName);
@ -219,6 +220,8 @@ private:
c.duplInheritedMembersError(nullptr, nullptr, "class", "class", "variable", false, false);
c.copyCtorAndEqOperatorError(nullptr, "class", false, false);
c.unsafeClassDivZeroError(nullptr, "Class", "dostuff", "x");
c.pureVirtualFunctionCallInConstructorError(nullptr, std::list<const Token *>(), "f");
c.virtualFunctionCallInConstructorError(nullptr, std::list<const Token *>(), "f");
}
static std::string myName() {
@ -317,22 +320,22 @@ private:
/**
* @brief gives a list of tokens where pure virtual functions are called directly or indirectly
* @param function function to be checked
* @param callsPureVirtualFunctionMap map of results for already checked functions
* @param virtualFunctionCallsMap map of results for already checked functions
* @return list of tokens where pure virtual functions are called
*/
const std::list<const Token *> & callsPureVirtualFunction(
const std::list<const Token *> & getVirtualFunctionCalls(
const Function & function,
std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap);
std::map<const Function *, std::list<const Token *> > & virtualFunctionCallsMap);
/**
* @brief looks for the first pure virtual function call stack
* @param callsPureVirtualFunctionMap map of results obtained from callsPureVirtualFunction
* @param pureCall token where pure virtual function is called directly or indirectly
* @param virtualFunctionCallsMap map of results obtained from getVirtualFunctionCalls
* @param callToken token where pure virtual function is called directly or indirectly
* @param[in,out] pureFuncStack list to append the stack
*/
void getFirstPureVirtualFunctionCallStack(
std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap,
const Token & pureCall,
void getFirstVirtualFunctionCallStack(
std::map<const Function *, std::list<const Token *> > & virtualFunctionCallsMap,
const Token *callToken,
std::list<const Token *> & pureFuncStack);
static bool canNotCopy(const Scope *scope);

50
test/testclass.cpp
View File

@ -180,6 +180,7 @@ private:
TEST_CASE(initializerListUsage);
TEST_CASE(selfInitialization);
TEST_CASE(virtualFunctionCallInConstructor);
TEST_CASE(pureVirtualFunctionCall);
TEST_CASE(pureVirtualFunctionCallOtherClass);
TEST_CASE(pureVirtualFunctionCallWithBody);
@ -6297,7 +6298,7 @@ private:
ASSERT_EQUALS("", errout.str());
}
void checkPureVirtualFunctionCall(const char code[], Settings *s = 0, bool inconclusive = true) {
void checkVirtualFunctionCall(const char code[], Settings *s = 0, bool inconclusive = true) {
// Clear the error log
errout.str("");
@ -6316,11 +6317,22 @@ private:
tokenizer.simplifyTokenList2();
CheckClass checkClass(&tokenizer, s, this);
checkClass.checkPureVirtualFunctionCall();
checkClass.checkVirtualFunctionCallInConstructor();
}
void virtualFunctionCallInConstructor() {
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual int f() { return 1; }\n"
" A();\n"
"};\n"
"A::A()\n"
"{f();}\n");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:3]: (warning) Call of virtual function 'f' in constructor.\n", errout.str());
}
void pureVirtualFunctionCall() {
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual void pure()=0;\n"
" A();\n"
@ -6329,7 +6341,7 @@ private:
"{pure();}\n");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual int pure()=0;\n"
" A();\n"
@ -6339,7 +6351,7 @@ private:
"{}\n");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual void pure()=0; \n"
" virtual ~A(); \n"
@ -6349,7 +6361,7 @@ private:
"{pure();}\n");
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in destructor.\n", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual void pure()=0;\n"
" void nonpure()\n"
@ -6360,7 +6372,7 @@ private:
"{nonpure();}\n");
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual int pure()=0;\n"
" int nonpure()\n"
@ -6372,7 +6384,7 @@ private:
"{}\n");
TODO_ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", "", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual void pure()=0; \n"
" void nonpure()\n"
@ -6384,7 +6396,7 @@ private:
"{nonpure();}\n");
ASSERT_EQUALS("[test.cpp:10] -> [test.cpp:5] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in destructor.\n", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual void pure()=0;\n"
" A(bool b);\n"
@ -6393,7 +6405,7 @@ private:
"{if (b) pure();}\n");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual void pure()=0;\n"
" virtual ~A();\n"
@ -6404,7 +6416,7 @@ private:
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in destructor.\n", errout.str());
// #5831
checkPureVirtualFunctionCall("class abc {\n"
checkVirtualFunctionCall("class abc {\n"
"public:\n"
" virtual ~abc() throw() {}\n"
" virtual void def(void* g) throw () = 0;\n"
@ -6412,7 +6424,7 @@ private:
ASSERT_EQUALS("", errout.str());
// #4992
checkPureVirtualFunctionCall("class CMyClass {\n"
checkVirtualFunctionCall("class CMyClass {\n"
" std::function< void(void) > m_callback;\n"
"public:\n"
" CMyClass() {\n"
@ -6424,7 +6436,7 @@ private:
}
void pureVirtualFunctionCallOtherClass() {
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual void pure()=0;\n"
" A(const A & a);\n"
@ -6433,7 +6445,7 @@ private:
"{a.pure();}\n");
ASSERT_EQUALS("", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual void pure()=0;\n"
" A();\n"
@ -6448,7 +6460,7 @@ private:
}
void pureVirtualFunctionCallWithBody() {
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
"{\n"
" virtual void pureWithBody()=0;\n"
" A();\n"
@ -6459,7 +6471,7 @@ private:
"{}\n");
ASSERT_EQUALS("", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual void pureWithBody()=0;\n"
" void nonpure()\n"
@ -6475,7 +6487,7 @@ private:
}
void pureVirtualFunctionCallPrevented() {
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual void pure()=0;\n"
" void nonpure(bool bCallPure)\n"
@ -6486,7 +6498,7 @@ private:
"{nonpure(false);}\n");
ASSERT_EQUALS("", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual void pure()=0;\n"
" void nonpure(bool bCallPure)\n"
@ -6497,7 +6509,7 @@ private:
"{nonpure(false);}\n");
ASSERT_EQUALS("", errout.str());
checkPureVirtualFunctionCall("class A\n"
checkVirtualFunctionCall("class A\n"
" {\n"
" virtual void pure()=0;\n"
" void nonpure(bool bCallPure)\n"