[clang-tools-extra] [clang-tidy] Added bugprone-exception-rethrow check (PR #86448)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

<details>
<summary>Changes</summary>

Identifies problematic exception rethrowing, especially with caught exception variables or empty throw statements outside catch blocks.

Closes #<!-- -->71292

---
Full diff: https://github.com/llvm/llvm-project/pull/86448.diff


8 Files Affected:

- (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3) 
- (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1) 
- (added) clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp (+50) 
- (added) clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h (+37) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) 
- (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst (+68) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) 
- (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp (+60) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 2931325d8b5798..adaa77fdbfbde0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -26,6 +26,7 @@
 #include "EasilySwappableParametersCheck.h"
 #include "EmptyCatchCheck.h"
 #include "ExceptionEscapeCheck.h"
+#include "ExceptionRethrowCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
 #include "ForwardingReferenceOverloadCheck.h"
@@ -126,6 +127,8 @@ class BugproneModule : public ClangTidyModule {
     CheckFactories.registerCheck<EmptyCatchCheck>("bugprone-empty-catch");
     CheckFactories.registerCheck<ExceptionEscapeCheck>(
         "bugprone-exception-escape");
+    CheckFactories.registerCheck<ExceptionRethrowCheck>(
+        "bugprone-exception-rethrow");
     CheckFactories.registerCheck<FoldInitTypeCheck>("bugprone-fold-init-type");
     CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
         "bugprone-forward-declaration-namespace");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 081ba67efe1538..7c7288f75e1888 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -20,6 +20,7 @@ add_clang_library(clangTidyBugproneModule
   EasilySwappableParametersCheck.cpp
   EmptyCatchCheck.cpp
   ExceptionEscapeCheck.cpp
+  ExceptionRethrowCheck.cpp
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   ForwardingReferenceOverloadCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
new file mode 100644
index 00000000000000..4855ccc2724a92
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
@@ -0,0 +1,50 @@
+//===--- ExceptionRethrowCheck.cpp - clang-tidy ---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ExceptionRethrowCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(VarDecl, isExceptionVariable) { return Node.isExceptionVariable(); }
+} // namespace
+
+void ExceptionRethrowCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxThrowExpr(unless(isExpansionInSystemHeader()),
+                   anyOf(unless(has(expr())),
+                         has(declRefExpr(to(varDecl(isExceptionVariable()))))),
+                   optionally(hasAncestor(cxxCatchStmt().bind("catch"))))
+          .bind("throw"),
+      this);
+}
+
+void ExceptionRethrowCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedThrow = Result.Nodes.getNodeAs<CXXThrowExpr>("throw");
+
+  if (const Expr *ThrownObject = MatchedThrow->getSubExpr()) {
+    diag(MatchedThrow->getThrowLoc(),
+         "throwing a copy of the caught %0 exception, remove the argument to "
+         "throw the original exception object")
+        << ThrownObject->getType().getNonReferenceType();
+    return;
+  }
+
+  const bool HasCatchAnsestor =
+      Result.Nodes.getNodeAs<Stmt>("catch") != nullptr;
+  if (!HasCatchAnsestor) {
+    diag(MatchedThrow->getThrowLoc(),
+         "empty 'throw' outside a catch block without an exception can trigger "
+         "'std::terminate'");
+  }
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
new file mode 100644
index 00000000000000..bbb30f779cf258
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
@@ -0,0 +1,37 @@
+//===--- ExceptionRethrowCheck.h - clang-tidy -------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTIONRETHROWCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTIONRETHROWCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Identifies problematic exception rethrowing, especially with caught
+/// exception variables or empty throw statements outside catch blocks.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/exception-rethrow.html
+class ExceptionRethrowCheck : public ClangTidyCheck {
+public:
+  ExceptionRethrowCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus && LangOpts.CXXExceptions;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTIONRETHROWCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a604e9276668ae..cc05cefc03b5a7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -110,6 +110,12 @@ New checks
   Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
   can be constructed outside itself and the derived class.
 
+- New :doc:`bugprone-exception-rethrow
+  <clang-tidy/checks/bugprone/exception-rethrow>` check.
+
+  Identifies problematic exception rethrowing, especially with caught exception
+  variables or empty throw statements outside catch blocks.
+
 - New :doc:`bugprone-suspicious-stringview-data-usage
   <clang-tidy/checks/bugprone/suspicious-stringview-data-usage>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
new file mode 100644
index 00000000000000..981dfff852d585
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -0,0 +1,68 @@
+.. title:: clang-tidy - bugprone-exception-rethrow
+
+bugprone-exception-rethrow
+==========================
+
+Identifies problematic exception rethrowing, especially with caught exception
+variables or empty throw statements outside catch blocks.
+
+In C++ exception handling, a common pitfall occurs when developers rethrow
+caught exceptions within catch blocks by directly passing the caught exception
+variable to the ``throw`` statement. While this approach can propagate
+exceptions to higher levels of the program, it often leads to code that is less
+clear and more error-prone. Rethrowing caught exceptions with the same exception
+object within catch blocks can obscure the original context of the exception and
+make it challenging to trace program flow. Additionally, this method can
+introduce issues such as exception object slicing and performance overhead due
+to the invocation of the copy constructor.
+
+.. code-block:: c++
+
+  try {
+    // Code that may throw an exception
+  } catch (const std::exception& e) {
+    throw e; // Bad
+  }
+
+To prevent these issues, it is advisable to utilize ``throw;`` statements to
+rethrow the original exception object for currently handled exceptions.
+
+.. code-block:: c++
+
+  try {
+    // Code that may throw an exception
+  } catch (const std::exception& e) {
+    throw; // Good
+  }
+
+However, when empty throw statement is used outside of a catch block, it
+will result in a call to ``std::terminate()``, which abruptly terminates the
+application. This behavior can lead to abnormal termination of the program and
+is often unintended. Such occurrences may indicate errors or oversights in the
+exception handling logic, and it is essential to avoid empty throw statements
+outside catch blocks to prevent unintended program termination.
+
+.. code-block:: c++
+
+  void foo() {
+    // std::terminate will be called because there is no exception to rethrow
+    throw;
+  }
+
+  int main() {
+    try {
+      foo();
+    } catch(...) {
+      return 1;
+    }
+    return 0;
+  }
+
+Above program will be terminated with:
+
+.. code:: text
+
+  terminate called without an active exception
+  Aborted (core dumped)
+
+
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 79e81dd174e4f3..481b1cdb02e64f 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -92,6 +92,7 @@ Clang-Tidy Checks
    :doc:`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters>`,
    :doc:`bugprone-empty-catch <bugprone/empty-catch>`,
    :doc:`bugprone-exception-escape <bugprone/exception-escape>`,
+   :doc:`bugprone-exception-rethrow <bugprone/exception-rethrow>`,
    :doc:`bugprone-fold-init-type <bugprone/fold-init-type>`,
    :doc:`bugprone-forward-declaration-namespace <bugprone/forward-declaration-namespace>`,
    :doc:`bugprone-forwarding-reference-overload <bugprone/forwarding-reference-overload>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
new file mode 100644
index 00000000000000..5e7ba036523933
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
@@ -0,0 +1,60 @@
+// RUN: %check_clang_tidy %s bugprone-exception-rethrow %t -- -- -fexceptions
+
+struct exception {};
+
+void correct() {
+  try {
+      throw exception();
+  } catch(const exception &) {
+      throw;
+  }
+}
+
+void correct2() {
+  try {
+      throw exception();
+  } catch(const exception &e) {
+      try {
+        throw exception();
+      } catch(...) {}
+  }
+}
+
+void not_correct() {
+  try {
+      throw exception();
+  } catch(const exception &e) {
+      throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'exception' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+  }
+}
+
+void not_correct2() {
+  try {
+      throw 5;
+  } catch(const int &e) {
+      throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'int' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+  }
+}
+
+void rethrow_not_correct() {
+  throw;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+}
+
+void rethrow_not_correct2() {
+  try {
+    throw;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+  } catch(...) {
+  }
+}
+
+void rethrow_correct() {
+  try {
+    throw 5;
+  } catch(...) {
+    throw;
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/86448