diff --git a/lib/checkexceptionsafety.cpp b/lib/checkexceptionsafety.cpp index f8cc28470..365878d9f 100644 --- a/lib/checkexceptionsafety.cpp +++ b/lib/checkexceptionsafety.cpp @@ -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); +} diff --git a/lib/checkexceptionsafety.h b/lib/checkexceptionsafety.h index 72b5ffa07..f316e4683 100644 --- a/lib/checkexceptionsafety.h +++ b/lib/checkexceptionsafety.h @@ -36,6 +36,7 @@ class ErrorLogger; // CWE ID used: 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 CWE480(480U); // Use of Incorrect Operator /// @addtogroup Checks @@ -72,6 +73,7 @@ public: checkExceptionSafety.checkCatchExceptionByValue(); checkExceptionSafety.nothrowThrows(); checkExceptionSafety.unhandledExceptionSpecification(); + checkExceptionSafety.rethrowNoCurrentException(); } /** Don't throw exceptions in destructors */ @@ -92,6 +94,9 @@ public: /** @brief %Check for unhandled exception specification */ void unhandledExceptionSpecification(); + /** @brief %Check for rethrow not from catch scope */ + void rethrowNoCurrentException(); + private: /** Don't throw exceptions in destructors */ 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); } + /** Rethrow without currently handled exception */ + void rethrowNoCurrentExceptionError(const Token *tok); + /** Generate all possible errors (for --errorlist) */ void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE { CheckExceptionSafety c(nullptr, settings, errorLogger); @@ -144,6 +152,7 @@ private: c.catchExceptionByValueError(nullptr); c.noexceptThrowError(nullptr); c.unhandledExceptionSpecificationError(nullptr, nullptr, "funcname"); + c.rethrowNoCurrentExceptionError(nullptr); } /** 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" "- Exception caught by value instead of by reference\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"; } }; /// @} diff --git a/test/testexceptionsafety.cpp b/test/testexceptionsafety.cpp index 42c734665..263bace8f 100644 --- a/test/testexceptionsafety.cpp +++ b/test/testexceptionsafety.cpp @@ -51,6 +51,9 @@ private: TEST_CASE(nothrowAttributeThrow); TEST_CASE(nothrowAttributeThrow2); // #5703 TEST_CASE(nothrowDeclspecThrow); + TEST_CASE(rethrowNoCurrentException1); + TEST_CASE(rethrowNoCurrentException2); + TEST_CASE(rethrowNoCurrentException3); } void check(const char code[], bool inconclusive = false) { @@ -407,6 +410,25 @@ private: check("const char *func() __attribute((nothrow)); void func1() { return 0; }"); 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)