Refactorized check for dead pointers after throw:

- Less bailouts for inconclusive checking
- Support for static variables
- Changed severity to warning (error is not certain)
This commit is contained in:
PKEuS 2012-01-21 19:11:06 +01:00
parent b0dac2fa2e
commit 87e19d2552
3 changed files with 60 additions and 15 deletions

View File

@ -58,6 +58,9 @@ void CheckExceptionSafety::destructors()
void CheckExceptionSafety::deallocThrow() void CheckExceptionSafety::deallocThrow()
{ {
if (!_settings->isEnabled("style"))
return;
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase(); const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
// Deallocate a global/member pointer and then throw exception // Deallocate a global/member pointer and then throw exception
@ -81,7 +84,7 @@ void CheckExceptionSafety::deallocThrow()
// we only look for global variables // we only look for global variables
const Variable* var = symbolDatabase->getVariableFromVarId(varid); const Variable* var = symbolDatabase->getVariableFromVarId(varid);
if (!var || !var->isGlobal()) if (!var || !(var->isGlobal() || var->isStatic()))
continue; continue;
// indentlevel.. // indentlevel..
@ -100,16 +103,24 @@ void CheckExceptionSafety::deallocThrow()
--indentlevel; --indentlevel;
} }
else if (tok2->str() == "throw") // Throw after delete -> Dead pointer
else if (tok2->str() == "throw") {
if (_settings->inconclusive) { // For inconclusive checking, throw directly.
deallocThrowError(tok2, tok->str());
break;
}
ThrowToken = tok2; ThrowToken = tok2;
}
// if the variable is not assigned after the throw then it // Variable is assigned -> Bail out
// is assumed that it is not the intention that it is a dead pointer.
else if (Token::Match(tok2, "%varid% =", varid)) { else if (Token::Match(tok2, "%varid% =", varid)) {
if (ThrowToken) if (ThrowToken) // For non-inconclusive checking, wait until we find an assignement to it. Otherwise we assume it is safe to leave a dead pointer.
deallocThrowError(ThrowToken, tok->str()); deallocThrowError(ThrowToken, tok2->str());
break; break;
} }
// Variable passed to function. Assume it becomes assigned -> Bail out
else if (Token::Match(tok2, "[,(] &| %varid% [,)]", varid)) // TODO: No bailout if passed by value or as const reference
break;
} }
} }
} }

View File

@ -74,7 +74,7 @@ private:
} }
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::warning, "exceptDeallocThrow", "Throwing exception in invalid state, " + varname + " points at deallocated memory");
} }
void rethrowCopyError(const Token * const tok, const std::string &varname) { void rethrowCopyError(const Token * const tok, const std::string &varname) {

View File

@ -35,24 +35,25 @@ private:
TEST_CASE(destructors); TEST_CASE(destructors);
TEST_CASE(deallocThrow1); TEST_CASE(deallocThrow1);
TEST_CASE(deallocThrow2); TEST_CASE(deallocThrow2);
TEST_CASE(deallocThrow3);
TEST_CASE(rethrowCopy1); TEST_CASE(rethrowCopy1);
TEST_CASE(rethrowCopy2); TEST_CASE(rethrowCopy2);
TEST_CASE(rethrowCopy3); TEST_CASE(rethrowCopy3);
TEST_CASE(rethrowCopy4); TEST_CASE(rethrowCopy4);
} }
void check(const std::string &code) { void check(const std::string &code, bool inconclusive = false) {
// Clear the error buffer.. // Clear the error buffer..
errout.str(""); errout.str("");
Settings settings; Settings settings;
settings.addEnabled("all"); settings.addEnabled("all");
settings.inconclusive = inconclusive;
// Tokenize.. // Tokenize..
Tokenizer tokenizer(&settings, this); Tokenizer tokenizer(&settings, this);
std::istringstream istr(code); std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp"); tokenizer.tokenize(istr, "test.cpp");
tokenizer.simplifyTokenList();
// Check char variable usage.. // Check char variable usage..
CheckExceptionSafety checkExceptionSafety(&tokenizer, &settings, this); CheckExceptionSafety checkExceptionSafety(&tokenizer, &settings, this);
@ -83,24 +84,57 @@ private:
void deallocThrow1() { void deallocThrow1() {
check("int * p;\n" check("int * p;\n"
"void f(int x)\n" "void f(int x) {\n"
"{\n"
" delete p;\n" " delete p;\n"
" if (x)\n" " if (x)\n"
" throw 123;\n" " throw 123;\n"
" p = 0;\n" " p = 0;\n"
"}\n"); "}");
ASSERT_EQUALS("[test.cpp:6]: (error) Throwing exception in invalid state, p points at deallocated memory\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (warning) Throwing exception in invalid state, p points at deallocated memory\n", errout.str());
check("void f() {\n"
" static int* p = foo;\n"
" delete p;\n"
" if (foo)\n"
" throw 1;\n"
" p = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (warning) Throwing exception in invalid state, p points at deallocated memory\n", errout.str());
} }
void deallocThrow2() { void deallocThrow2() {
check("void f() {\n" check("void f() {\n"
" int* p = 0;\n" " int* p = 0;\n"
" delete p;\n" " delete p;\n"
" throw 1;\n" " if (foo)\n"
" throw 1;\n"
" p = new int;\n" " p = new int;\n"
"}\n"); "}", true);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" static int* p = 0;\n"
" delete p;\n"
" reset(p);\n"
" throw 1;\n"
"}", true);
ASSERT_EQUALS("", errout.str());
}
void deallocThrow3() {
check("void f() {\n"
" static int* p = 0;\n"
" delete p;\n"
" throw 1;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" static int* p = 0;\n"
" delete p;\n"
" throw 1;\n"
"}", true);
ASSERT_EQUALS("[test.cpp:4]: (warning) Throwing exception in invalid state, p points at deallocated memory\n", errout.str());
} }
void rethrowCopy1() { void rethrowCopy1() {