From 87e19d2552eba470a2212811f659fde1d8b37405 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sat, 21 Jan 2012 19:11:06 +0100 Subject: [PATCH] Refactorized check for dead pointers after throw: - Less bailouts for inconclusive checking - Support for static variables - Changed severity to warning (error is not certain) --- lib/checkexceptionsafety.cpp | 23 ++++++++++++----- lib/checkexceptionsafety.h | 2 +- test/testexceptionsafety.cpp | 50 ++++++++++++++++++++++++++++++------ 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/lib/checkexceptionsafety.cpp b/lib/checkexceptionsafety.cpp index c9e2d45fa..465f36259 100644 --- a/lib/checkexceptionsafety.cpp +++ b/lib/checkexceptionsafety.cpp @@ -58,6 +58,9 @@ void CheckExceptionSafety::destructors() void CheckExceptionSafety::deallocThrow() { + if (!_settings->isEnabled("style")) + return; + const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase(); // Deallocate a global/member pointer and then throw exception @@ -81,7 +84,7 @@ void CheckExceptionSafety::deallocThrow() // we only look for global variables const Variable* var = symbolDatabase->getVariableFromVarId(varid); - if (!var || !var->isGlobal()) + if (!var || !(var->isGlobal() || var->isStatic())) continue; // indentlevel.. @@ -100,16 +103,24 @@ void CheckExceptionSafety::deallocThrow() --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; + } - // if the variable is not assigned after the throw then it - // is assumed that it is not the intention that it is a dead pointer. + // Variable is assigned -> Bail out else if (Token::Match(tok2, "%varid% =", varid)) { - if (ThrowToken) - deallocThrowError(ThrowToken, tok->str()); + 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, tok2->str()); 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; } } } diff --git a/lib/checkexceptionsafety.h b/lib/checkexceptionsafety.h index 42ce1a34e..9a20f5caf 100644 --- a/lib/checkexceptionsafety.h +++ b/lib/checkexceptionsafety.h @@ -74,7 +74,7 @@ private: } 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) { diff --git a/test/testexceptionsafety.cpp b/test/testexceptionsafety.cpp index bef687d43..4cb0a206f 100644 --- a/test/testexceptionsafety.cpp +++ b/test/testexceptionsafety.cpp @@ -35,24 +35,25 @@ private: TEST_CASE(destructors); TEST_CASE(deallocThrow1); TEST_CASE(deallocThrow2); + TEST_CASE(deallocThrow3); TEST_CASE(rethrowCopy1); TEST_CASE(rethrowCopy2); TEST_CASE(rethrowCopy3); TEST_CASE(rethrowCopy4); } - void check(const std::string &code) { + void check(const std::string &code, bool inconclusive = false) { // Clear the error buffer.. errout.str(""); Settings settings; settings.addEnabled("all"); + settings.inconclusive = inconclusive; // Tokenize.. Tokenizer tokenizer(&settings, this); std::istringstream istr(code); tokenizer.tokenize(istr, "test.cpp"); - tokenizer.simplifyTokenList(); // Check char variable usage.. CheckExceptionSafety checkExceptionSafety(&tokenizer, &settings, this); @@ -83,24 +84,57 @@ private: void deallocThrow1() { check("int * p;\n" - "void f(int x)\n" - "{\n" + "void f(int x) {\n" " delete p;\n" " if (x)\n" " throw 123;\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() { check("void f() {\n" " int* p = 0;\n" " delete p;\n" - " throw 1;\n" + " if (foo)\n" + " throw 1;\n" " p = new int;\n" - "}\n"); + "}", true); 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() {