New check for rethrow without current handled exception (#3270)

This commit is contained in:
Maksim Derbasov 2021-05-31 11:39:24 +03:00 committed by GitHub
parent 04c9a8329a
commit 06c4542ac2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 74 additions and 1 deletions

View File

@ -304,3 +304,44 @@ void CheckExceptionSafety::unhandledExceptionSpecification()
} }
} }
} }
//--------------------------------------------------------------------------
// 7.6.18.4 If no exception is presently being handled, evaluating a throw-expression with no operand calls std::terminate().
//--------------------------------------------------------------------------
void CheckExceptionSafety::rethrowNoCurrentException()
{
const SymbolDatabase* const symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * scope : symbolDatabase->functionScopes) {
const Function* function = scope->function;
if (!function)
continue;
// Rethrow can be used in 'exception dispatcher' idiom which is FP in such case
// https://isocpp.org/wiki/faq/exceptions#throw-without-an-object
// We check the beggining of the function with idiom pattern
if (Token::simpleMatch(function->functionScope->bodyStart->next(), "try { throw ; } catch (" ))
continue;
for (const Token *tok = function->functionScope->bodyStart->next();
tok != function->functionScope->bodyEnd; tok = tok->next()) {
if (Token::simpleMatch(tok, "catch (")) {
tok = tok->linkAt(1); // skip catch argument
if (Token::simpleMatch(tok, ") {"))
tok = tok->linkAt(1); // skip catch scope
else
break;
}
if (Token::simpleMatch(tok, "throw ;")) {
rethrowNoCurrentExceptionError(tok);
}
}
}
}
void CheckExceptionSafety::rethrowNoCurrentExceptionError(const Token *tok) {
reportError(tok, Severity::error, "rethrowNoCurrentException",
"Rethrowing current exception with 'throw;', it seems there is no current exception to rethrow."
" If there is no current exception this calls std::terminate()."
" More: https://isocpp.org/wiki/faq/exceptions#throw-without-an-object",
CWE480, Certainty::normal);
}

View File

@ -36,6 +36,7 @@ class ErrorLogger;
// CWE ID used: // CWE ID used:
static const struct CWE CWE398(398U); // Indicator of Poor Code Quality static const struct CWE CWE398(398U); // Indicator of Poor Code Quality
static const struct CWE CWE703(703U); // Improper Check or Handling of Exceptional Conditions static const struct CWE CWE703(703U); // Improper Check or Handling of Exceptional Conditions
static const struct CWE CWE480(480U); // Use of Incorrect Operator
/// @addtogroup Checks /// @addtogroup Checks
@ -72,6 +73,7 @@ public:
checkExceptionSafety.checkCatchExceptionByValue(); checkExceptionSafety.checkCatchExceptionByValue();
checkExceptionSafety.nothrowThrows(); checkExceptionSafety.nothrowThrows();
checkExceptionSafety.unhandledExceptionSpecification(); checkExceptionSafety.unhandledExceptionSpecification();
checkExceptionSafety.rethrowNoCurrentException();
} }
/** Don't throw exceptions in destructors */ /** Don't throw exceptions in destructors */
@ -92,6 +94,9 @@ public:
/** @brief %Check for unhandled exception specification */ /** @brief %Check for unhandled exception specification */
void unhandledExceptionSpecification(); void unhandledExceptionSpecification();
/** @brief %Check for rethrow not from catch scope */
void rethrowNoCurrentException();
private: private:
/** Don't throw exceptions in destructors */ /** Don't throw exceptions in destructors */
void destructorsError(const Token * const tok, const std::string &className) { void destructorsError(const Token * const tok, const std::string &className) {
@ -135,6 +140,9 @@ private:
"Either use a try/catch around the function call, or add a exception specification for " + funcname + "() also.", CWE703, Certainty::inconclusive); "Either use a try/catch around the function call, or add a exception specification for " + funcname + "() also.", CWE703, Certainty::inconclusive);
} }
/** Rethrow without currently handled exception */
void rethrowNoCurrentExceptionError(const Token *tok);
/** Generate all possible errors (for --errorlist) */ /** Generate all possible errors (for --errorlist) */
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE { void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
CheckExceptionSafety c(nullptr, settings, errorLogger); CheckExceptionSafety c(nullptr, settings, errorLogger);
@ -144,6 +152,7 @@ private:
c.catchExceptionByValueError(nullptr); c.catchExceptionByValueError(nullptr);
c.noexceptThrowError(nullptr); c.noexceptThrowError(nullptr);
c.unhandledExceptionSpecificationError(nullptr, nullptr, "funcname"); c.unhandledExceptionSpecificationError(nullptr, nullptr, "funcname");
c.rethrowNoCurrentExceptionError(nullptr);
} }
/** Short description of class (for --doc) */ /** Short description of class (for --doc) */
@ -159,7 +168,8 @@ private:
"- Throwing a copy of a caught exception instead of rethrowing the original exception\n" "- Throwing a copy of a caught exception instead of rethrowing the original exception\n"
"- Exception caught by value instead of by reference\n" "- Exception caught by value instead of by reference\n"
"- Throwing exception in noexcept, nothrow(), __attribute__((nothrow)) or __declspec(nothrow) function\n" "- Throwing exception in noexcept, nothrow(), __attribute__((nothrow)) or __declspec(nothrow) function\n"
"- Unhandled exception specification when calling function foo()\n"; "- Unhandled exception specification when calling function foo()\n"
"- Rethrow without currently handled exception\n";
} }
}; };
/// @} /// @}

View File

@ -51,6 +51,9 @@ private:
TEST_CASE(nothrowAttributeThrow); TEST_CASE(nothrowAttributeThrow);
TEST_CASE(nothrowAttributeThrow2); // #5703 TEST_CASE(nothrowAttributeThrow2); // #5703
TEST_CASE(nothrowDeclspecThrow); TEST_CASE(nothrowDeclspecThrow);
TEST_CASE(rethrowNoCurrentException1);
TEST_CASE(rethrowNoCurrentException2);
TEST_CASE(rethrowNoCurrentException3);
} }
void check(const char code[], bool inconclusive = false) { void check(const char code[], bool inconclusive = false) {
@ -407,6 +410,25 @@ private:
check("const char *func() __attribute((nothrow)); void func1() { return 0; }"); check("const char *func() __attribute((nothrow)); void func1() { return 0; }");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void rethrowNoCurrentException1() {
check("void func1(const bool flag) { try{ if(!flag) throw; } catch (int&) { ; } }");
ASSERT_EQUALS("[test.cpp:1]: (error) Rethrowing current exception with 'throw;', it seems there is no current exception to rethrow."
" If there is no current exception this calls std::terminate(). More: https://isocpp.org/wiki/faq/exceptions#throw-without-an-object\n", errout.str());
}
void rethrowNoCurrentException2() {
check("void func1() { try{ ; } catch (...) { ; } throw; }");
ASSERT_EQUALS("[test.cpp:1]: (error) Rethrowing current exception with 'throw;', it seems there is no current exception to rethrow."
" If there is no current exception this calls std::terminate(). More: https://isocpp.org/wiki/faq/exceptions#throw-without-an-object\n", errout.str());
}
void rethrowNoCurrentException3() {
check("void on_error() { try { throw; } catch (const int &) { ; } catch (...) { ; } }\n" // exception dispatcher idiom
"void func2() { try{ ; } catch (const int&) { throw; } ; }\n"
"void func3() { throw 0; }");
ASSERT_EQUALS("", errout.str());
}
}; };
REGISTER_TEST(TestExceptionSafety) REGISTER_TEST(TestExceptionSafety)