Exception safety: Removed the noisy checks and keep the useful checks
This commit is contained in:
Daniel Marjamäki
parent
b2a775f3e0
commit
3a8e7b4bf0
|
|
@ -73,216 +73,6 @@ void CheckExceptionSafety::destructors()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
void CheckExceptionSafety::unsafeNew()
|
|
||||||
{
|
|
||||||
if (!_settings->isEnabled("exceptNew"))
|
|
||||||
return;
|
|
||||||
|
|
||||||
// Inspect initializer lists..
|
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
|
|
||||||
{
|
|
||||||
if (tok->str() != ")")
|
|
||||||
continue;
|
|
||||||
tok = tok->next();
|
|
||||||
if (!tok)
|
|
||||||
break;
|
|
||||||
if (tok->str() != ":")
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// multiple "new" in an initializer list..
|
|
||||||
std::string varname;
|
|
||||||
for (tok = tok->next(); tok; tok = tok->next())
|
|
||||||
{
|
|
||||||
if (!Token::Match(tok, "%var% ("))
|
|
||||||
break;
|
|
||||||
tok = tok->next();
|
|
||||||
if (Token::Match(tok->next(), "new %type%"))
|
|
||||||
{
|
|
||||||
if (!varname.empty())
|
|
||||||
{
|
|
||||||
unsafeNewError(tok->previous(), varname);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
/*
|
|
||||||
// TODO: check if class is autodeallocated+might throw exception in constructor
|
|
||||||
if (!_settings->isAutoDealloc(tok->strAt(2)))
|
|
||||||
{
|
|
||||||
varname = tok->strAt(-1);
|
|
||||||
}
|
|
||||||
*/
|
|
||||||
}
|
|
||||||
tok = tok->link();
|
|
||||||
tok = tok ? tok->next() : 0;
|
|
||||||
if (!tok)
|
|
||||||
break;
|
|
||||||
if (tok->str() != ",")
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
if (!tok)
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
// Inspect constructors..
|
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
|
|
||||||
{
|
|
||||||
// match this pattern.. "C :: C ( .. ) {"
|
|
||||||
if (!tok->next() || tok->next()->str() != "::")
|
|
||||||
continue;
|
|
||||||
if (!Token::Match(tok, "%var% :: %var% ("))
|
|
||||||
continue;
|
|
||||||
if (tok->str() != tok->strAt(2))
|
|
||||||
continue;
|
|
||||||
if (!Token::simpleMatch(tok->tokAt(3)->link(), ") {"))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// inspect the constructor..
|
|
||||||
std::string varname;
|
|
||||||
for (tok = tok->tokAt(3)->link()->tokAt(2); tok; tok = tok->next())
|
|
||||||
{
|
|
||||||
if (tok->str() == "{" || tok->str() == "}")
|
|
||||||
break;
|
|
||||||
// some variable declaration..
|
|
||||||
if (Token::Match(tok->previous(), "[{;] %type% * %var% ;"))
|
|
||||||
break;
|
|
||||||
// allocating with new..
|
|
||||||
if (Token::Match(tok, "%var% = new %type%"))
|
|
||||||
{
|
|
||||||
if (!varname.empty())
|
|
||||||
{
|
|
||||||
unsafeNewError(tok, varname);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
/*
|
|
||||||
TODO: check is class is autodeallocated + might throw exceptions in the constructor
|
|
||||||
if (!_settings->isAutoDealloc(tok->strAt(3)))
|
|
||||||
varname = tok->str();
|
|
||||||
*/
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// allocating multiple local variables..
|
|
||||||
std::set<unsigned int> localVars;
|
|
||||||
std::string varname;
|
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
|
|
||||||
{
|
|
||||||
if (tok->str() == "{" || tok->str() == "}")
|
|
||||||
{
|
|
||||||
localVars.clear();
|
|
||||||
varname = "";
|
|
||||||
}
|
|
||||||
|
|
||||||
if (Token::Match(tok, "[;{}] %type% * %var% ;") &&
|
|
||||||
tok->tokAt(1)->isStandardType())
|
|
||||||
{
|
|
||||||
tok = tok->tokAt(3);
|
|
||||||
if (tok->varId())
|
|
||||||
localVars.insert(tok->varId());
|
|
||||||
}
|
|
||||||
|
|
||||||
if (Token::Match(tok, "[;{}] %var% = new %type%"))
|
|
||||||
{
|
|
||||||
if (!varname.empty())
|
|
||||||
{
|
|
||||||
unsafeNewError(tok->next(), varname);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
if (tok->next()->varId() && localVars.find(tok->next()->varId()) != localVars.end())
|
|
||||||
varname = tok->strAt(1);
|
|
||||||
}
|
|
||||||
|
|
||||||
else if (Token::Match(tok, "[;{}] %var% ("))
|
|
||||||
{
|
|
||||||
// False negatives: we don't handle switch cases properly so we just bail out.
|
|
||||||
if (tok->strAt(1) == "switch")
|
|
||||||
break;
|
|
||||||
|
|
||||||
for (tok = tok->next(); tok && !Token::Match(tok, "[;{}]"); tok = tok->next())
|
|
||||||
{
|
|
||||||
if (tok->str() == varname)
|
|
||||||
{
|
|
||||||
varname = "";
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (!tok)
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
else if (tok->str() == "delete")
|
|
||||||
{
|
|
||||||
if (Token::simpleMatch(tok->next(), varname.c_str()) ||
|
|
||||||
Token::simpleMatch(tok->next(), ("[ ] " + varname).c_str()))
|
|
||||||
{
|
|
||||||
varname = "";
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
void CheckExceptionSafety::realloc()
|
|
||||||
{
|
|
||||||
if (!_settings->isEnabled("exceptRealloc"))
|
|
||||||
return;
|
|
||||||
|
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
|
|
||||||
{
|
|
||||||
if (Token::simpleMatch(tok, "try {"))
|
|
||||||
{
|
|
||||||
tok = tok->next()->link();
|
|
||||||
if (!tok)
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!Token::Match(tok, "[{};] delete"))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
tok = tok->tokAt(2);
|
|
||||||
if (Token::simpleMatch(tok, "[ ]"))
|
|
||||||
tok = tok->tokAt(2);
|
|
||||||
if (!tok)
|
|
||||||
break;
|
|
||||||
|
|
||||||
// reallocating..
|
|
||||||
if (!Token::Match(tok, "%var% ; %var% = new %type%"))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// variable id of deallocated pointer..
|
|
||||||
const unsigned int varid(tok->varId());
|
|
||||||
if (varid == 0)
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// variable id of allocated pointer must match..
|
|
||||||
if (varid != tok->tokAt(2)->varId())
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// is is a class member variable..
|
|
||||||
const Token *tok1 = Token::findmatch(_tokenizer->tokens(), "%varid%", varid);
|
|
||||||
while (0 != (tok1 = tok1 ? tok1->previous() : 0))
|
|
||||||
{
|
|
||||||
if (tok1->str() == "}")
|
|
||||||
tok1 = tok1->link();
|
|
||||||
else if (tok1->str() == "{")
|
|
||||||
{
|
|
||||||
if (tok1->previous() && tok1->previous()->isName())
|
|
||||||
{
|
|
||||||
while (0 != (tok1 = tok1 ? tok1->previous() : 0))
|
|
||||||
{
|
|
||||||
if (!tok1->isName() && tok1->str() != ":" && tok1->str() != ",")
|
|
||||||
break;
|
|
||||||
if (tok1->str() == "class")
|
|
||||||
{
|
|
||||||
reallocError(tok->tokAt(2), tok->str());
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
void CheckExceptionSafety::deallocThrow()
|
void CheckExceptionSafety::deallocThrow()
|
||||||
|
|
|
||||||
|
|
@ -55,8 +55,6 @@ public:
|
||||||
{
|
{
|
||||||
CheckExceptionSafety checkExceptionSafety(tokenizer, settings, errorLogger);
|
CheckExceptionSafety checkExceptionSafety(tokenizer, settings, errorLogger);
|
||||||
checkExceptionSafety.destructors();
|
checkExceptionSafety.destructors();
|
||||||
checkExceptionSafety.unsafeNew();
|
|
||||||
checkExceptionSafety.realloc();
|
|
||||||
checkExceptionSafety.deallocThrow();
|
checkExceptionSafety.deallocThrow();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -64,12 +62,6 @@ public:
|
||||||
/** Don't throw exceptions in destructors */
|
/** Don't throw exceptions in destructors */
|
||||||
void destructors();
|
void destructors();
|
||||||
|
|
||||||
/** unsafe use of "new" */
|
|
||||||
void unsafeNew();
|
|
||||||
|
|
||||||
/** Unsafe realloc */
|
|
||||||
void realloc();
|
|
||||||
|
|
||||||
/** deallocating memory and then throw */
|
/** deallocating memory and then throw */
|
||||||
void deallocThrow();
|
void deallocThrow();
|
||||||
|
|
||||||
|
|
@ -80,18 +72,6 @@ private:
|
||||||
reportError(tok, Severity::style, "exceptThrowInDestructor", "Throwing exception in destructor");
|
reportError(tok, Severity::style, "exceptThrowInDestructor", "Throwing exception in destructor");
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Unsafe use of new */
|
|
||||||
void unsafeNewError(const Token * const tok, const std::string &varname)
|
|
||||||
{
|
|
||||||
reportError(tok, Severity::style, "exceptNew", "Upon exception there is memory leak: " + varname);
|
|
||||||
}
|
|
||||||
|
|
||||||
/** Unsafe reallocation */
|
|
||||||
void reallocError(const Token * const tok, const std::string &varname)
|
|
||||||
{
|
|
||||||
reportError(tok, Severity::style, "exceptRealloc", "Upon exception " + varname + " becomes a dead pointer");
|
|
||||||
}
|
|
||||||
|
|
||||||
void deallocThrowError(const Token * const tok, const std::string &varname)
|
void deallocThrowError(const Token * const tok, const std::string &varname)
|
||||||
{
|
{
|
||||||
reportError(tok, Severity::error, "exceptDeallocThrow", "Throwing exception in invalid state, " + varname + " points at deallocated memory");
|
reportError(tok, Severity::error, "exceptDeallocThrow", "Throwing exception in invalid state, " + varname + " points at deallocated memory");
|
||||||
|
|
@ -101,8 +81,6 @@ private:
|
||||||
void getErrorMessages()
|
void getErrorMessages()
|
||||||
{
|
{
|
||||||
destructorsError(0);
|
destructorsError(0);
|
||||||
unsafeNewError(0, "p");
|
|
||||||
reallocError(0, "p");
|
|
||||||
deallocThrowError(0, "p");
|
deallocThrowError(0, "p");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -117,8 +95,6 @@ private:
|
||||||
{
|
{
|
||||||
return "Checking exception safety\n"
|
return "Checking exception safety\n"
|
||||||
"* Throwing exceptions in destructors\n"
|
"* Throwing exceptions in destructors\n"
|
||||||
"* Unsafe use of 'new'\n"
|
|
||||||
"* Unsafe reallocation\n"
|
|
||||||
"* Throwing exception during invalid state";
|
"* Throwing exception during invalid state";
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -602,8 +602,6 @@ bool CppCheck::parseFromArgs(int argc, const char* const argv[])
|
||||||
" Example: -DDEBUG=1 -D__cplusplus\n"
|
" Example: -DDEBUG=1 -D__cplusplus\n"
|
||||||
" --enable=id Enable additional checks. The available ids are:\n"
|
" --enable=id Enable additional checks. The available ids are:\n"
|
||||||
" * all - enable all checks\n"
|
" * all - enable all checks\n"
|
||||||
" * exceptNew - exception safety when using new\n"
|
|
||||||
" * exceptRealloc - exception safety when reallocating\n"
|
|
||||||
" * style - Check coding style\n"
|
" * style - Check coding style\n"
|
||||||
" * unusedFunctions - check for unused functions\n"
|
" * unusedFunctions - check for unused functions\n"
|
||||||
" Several ids can be given if you separate them with commas\n"
|
" Several ids can be given if you separate them with commas\n"
|
||||||
|
|
|
||||||
|
|
@ -188,8 +188,6 @@ std::string Settings::addEnabled(const std::string &str)
|
||||||
handled = _checkCodingStyle = true;
|
handled = _checkCodingStyle = true;
|
||||||
|
|
||||||
std::set<std::string> id;
|
std::set<std::string> id;
|
||||||
id.insert("exceptNew");
|
|
||||||
id.insert("exceptRealloc");
|
|
||||||
id.insert("unusedFunctions");
|
id.insert("unusedFunctions");
|
||||||
|
|
||||||
if (str == "all")
|
if (str == "all")
|
||||||
|
|
|
||||||
|
|
@ -35,9 +35,6 @@ private:
|
||||||
void run()
|
void run()
|
||||||
{
|
{
|
||||||
TEST_CASE(destructors);
|
TEST_CASE(destructors);
|
||||||
TEST_CASE(newnew);
|
|
||||||
TEST_CASE(switchnewnew);
|
|
||||||
TEST_CASE(realloc);
|
|
||||||
TEST_CASE(deallocThrow);
|
TEST_CASE(deallocThrow);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -68,118 +65,6 @@ private:
|
||||||
ASSERT_EQUALS("[test.cpp:3]: (style) Throwing exception in destructor\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:3]: (style) Throwing exception in destructor\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
void newnew()
|
|
||||||
{
|
|
||||||
check("C::C() : a(new A), b(new B) { }");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
TODO_ASSERT_EQUALS("[test.cpp:1]: (style) Upon exception there is memory leak: a\n", errout.str());
|
|
||||||
|
|
||||||
check("C::C()\n"
|
|
||||||
"{\n"
|
|
||||||
" a = new A;\n"
|
|
||||||
" b = new B;\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
TODO_ASSERT_EQUALS("[test.cpp:4]: (style) Upon exception there is memory leak: a\n", errout.str());
|
|
||||||
|
|
||||||
check("void a()\n"
|
|
||||||
"{\n"
|
|
||||||
" A *a1 = new A;\n"
|
|
||||||
" A *a2 = new A;\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
TODO_ASSERT_EQUALS("[test.cpp:4]: (style) Upon exception there is memory leak: a1\n", errout.str());
|
|
||||||
|
|
||||||
check("void a()\n"
|
|
||||||
"{\n"
|
|
||||||
" A *a1 = new A;\n"
|
|
||||||
" A *a2 = new (std::nothrow) A;\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("void a()\n"
|
|
||||||
"{\n"
|
|
||||||
" A *a1 = new A;\n"
|
|
||||||
" delete a1;\n"
|
|
||||||
" A *a2 = new A;\n"
|
|
||||||
" delete a2;\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("void a()\n"
|
|
||||||
"{\n"
|
|
||||||
" A *a = new A;\n"
|
|
||||||
" B *b = new B;\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
// passing pointer to unknown function.. the pointer may be added to some list etc..
|
|
||||||
check("void f()\n"
|
|
||||||
"{\n"
|
|
||||||
" A *a1 = new A;\n"
|
|
||||||
" add(a1);\n"
|
|
||||||
" A *a2 = new A;\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
}
|
|
||||||
|
|
||||||
void switchnewnew()
|
|
||||||
{
|
|
||||||
check("int *f(int x)\n"
|
|
||||||
"{\n"
|
|
||||||
" int *p = 0;\n"
|
|
||||||
" switch(x)\n"
|
|
||||||
" {\n"
|
|
||||||
" case 1:\n"
|
|
||||||
" p = new int(10);\n"
|
|
||||||
" break;\n"
|
|
||||||
" case 2:\n"
|
|
||||||
" p = new int(100);\n"
|
|
||||||
" break;\n"
|
|
||||||
" };\n"
|
|
||||||
" return p;\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
}
|
|
||||||
|
|
||||||
void realloc()
|
|
||||||
{
|
|
||||||
check("class A\n"
|
|
||||||
"{\n"
|
|
||||||
" int *p;\n"
|
|
||||||
" void a()\n"
|
|
||||||
" {\n"
|
|
||||||
" delete p;\n"
|
|
||||||
" p = new int[123];\n"
|
|
||||||
" }\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("[test.cpp:7]: (style) Upon exception p becomes a dead pointer\n", errout.str());
|
|
||||||
|
|
||||||
check("class A\n"
|
|
||||||
"{\n"
|
|
||||||
" int *p;\n"
|
|
||||||
" void a()\n"
|
|
||||||
" {\n"
|
|
||||||
" delete p;\n"
|
|
||||||
" p = new (std::nothrow) int[123];\n"
|
|
||||||
" }\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
|
|
||||||
check("class A\n"
|
|
||||||
"{\n"
|
|
||||||
" int *p;\n"
|
|
||||||
" void a()\n"
|
|
||||||
" {\n"
|
|
||||||
" try {\n"
|
|
||||||
" delete p;\n"
|
|
||||||
" p = new int[123];\n"
|
|
||||||
" } catch (...) { p = 0; }\n"
|
|
||||||
" }\n"
|
|
||||||
"}\n");
|
|
||||||
ASSERT_EQUALS("", errout.str());
|
|
||||||
}
|
|
||||||
|
|
||||||
void deallocThrow()
|
void deallocThrow()
|
||||||
{
|
{
|
||||||
check("int * p;\n"
|
check("int * p;\n"
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue