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

ntfshard · web-flow · commit 06c4542ac2b4 · 2021-05-31T10:39:24.000+02:00
diff --git 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
@@ -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 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
         checkExceptionSafety.checkCatchExceptionByValue();
         checkExceptionSafety.nothrowThrows();
         checkExceptionSafety.unhandledExceptionSpecification();
+        checkExceptionSafety.rethrowNoCurrentException();
     }
 
     /** Don't throw exceptions in destructors */
@@ -92,6 +94,9 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
     /** @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 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
                     "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 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
         c.catchExceptionByValueError(nullptr);
         c.noexceptThrowError(nullptr);
         c.unhandledExceptionSpecificationError(nullptr, nullptr, "funcname");
+        c.rethrowNoCurrentExceptionError(nullptr);
     }
 
     /** Short description of class (for --doc) */
@@ -159,7 +168,8 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
                "- 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
@@ -51,6 +51,9 @@ class TestExceptionSafety : public TestFixture {
         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 @@ class TestExceptionSafety : public TestFixture {
         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)
