[clang-tidy] Add bugprone-empty-catch check

Detects and suggests addressing issues with empty catch statements.

Reviewed By: xgupta

Differential Revision: https://reviews.llvm.org/D144748

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -20,6 +20,7 @@
 #include "DanglingHandleCheck.h"
 #include "DynamicStaticInitializersCheck.h"
 #include "EasilySwappableParametersCheck.h"
+#include "EmptyCatchCheck.h"
 #include "ExceptionEscapeCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
@@ -106,6 +107,7 @@ public:
         "bugprone-dynamic-static-initializers");
     CheckFactories.registerCheck<EasilySwappableParametersCheck>(
         "bugprone-easily-swappable-parameters");
+    CheckFactories.registerCheck<EmptyCatchCheck>("bugprone-empty-catch");
     CheckFactories.registerCheck<ExceptionEscapeCheck>(
         "bugprone-exception-escape");
     CheckFactories.registerCheck<FoldInitTypeCheck>("bugprone-fold-init-type");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -15,6 +15,7 @@ add_clang_library(clangTidyBugproneModule
   DanglingHandleCheck.cpp
   DynamicStaticInitializersCheck.cpp
   EasilySwappableParametersCheck.cpp
+  EmptyCatchCheck.cpp
   ExceptionEscapeCheck.cpp
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp

clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp

@@ -0,0 +1,110 @@
+//===--- EmptyCatchCheck.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 "EmptyCatchCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+using ::clang::ast_matchers::internal::Matcher;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXCatchStmt, isInMacro) {
+  return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID() ||
+         Node.getCatchLoc().isMacroID();
+}
+
+AST_MATCHER_P(CXXCatchStmt, hasHandler, Matcher<Stmt>, InnerMatcher) {
+  Stmt *Handler = Node.getHandlerBlock();
+  if (!Handler)
+    return false;
+  return InnerMatcher.matches(*Handler, Finder, Builder);
+}
+
+AST_MATCHER_P(CXXCatchStmt, hasCaughtType, Matcher<QualType>, InnerMatcher) {
+  return InnerMatcher.matches(Node.getCaughtType(), Finder, Builder);
+}
+
+AST_MATCHER_P(CompoundStmt, hasAnyTextFromList, std::vector<llvm::StringRef>,
+              List) {
+  if (List.empty())
+    return false;
+
+  ASTContext &Context = Finder->getASTContext();
+  SourceManager &SM = Context.getSourceManager();
+  StringRef Text = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Node.getSourceRange()), SM,
+      Context.getLangOpts());
+  return std::any_of(List.begin(), List.end(), [&](const StringRef &Str) {
+    return Text.contains_insensitive(Str);
+  });
+}
+
+} // namespace
+
+EmptyCatchCheck::EmptyCatchCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IgnoreCatchWithKeywords(utils::options::parseStringList(
+          Options.get("IgnoreCatchWithKeywords", "@TODO;@FIXME"))),
+      AllowEmptyCatchForExceptions(utils::options::parseStringList(
+          Options.get("AllowEmptyCatchForExceptions", ""))) {}
+
+void EmptyCatchCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreCatchWithKeywords",
+                utils::options::serializeStringList(IgnoreCatchWithKeywords));
+  Options.store(
+      Opts, "AllowEmptyCatchForExceptions",
+      utils::options::serializeStringList(AllowEmptyCatchForExceptions));
+}
+
+bool EmptyCatchCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus;
+}
+
+std::optional<TraversalKind> EmptyCatchCheck::getCheckTraversalKind() const {
+  return TK_IgnoreUnlessSpelledInSource;
+}
+
+void EmptyCatchCheck::registerMatchers(MatchFinder *Finder) {
+  auto AllowedNamedExceptionDecl =
+      namedDecl(matchers::matchesAnyListedName(AllowEmptyCatchForExceptions));
+  auto AllowedNamedExceptionTypes =
+      qualType(anyOf(hasDeclaration(AllowedNamedExceptionDecl),
+                     references(AllowedNamedExceptionDecl),
+                     pointsTo(AllowedNamedExceptionDecl)));
+  auto IgnoredExceptionType =
+      qualType(anyOf(AllowedNamedExceptionTypes,
+                     hasCanonicalType(AllowedNamedExceptionTypes)));
+
+  Finder->addMatcher(
+      cxxCatchStmt(unless(isExpansionInSystemHeader()), unless(isInMacro()),
+                   unless(hasCaughtType(IgnoredExceptionType)),
+                   hasHandler(compoundStmt(
+                       statementCountIs(0),
+                       unless(hasAnyTextFromList(IgnoreCatchWithKeywords)))))
+          .bind("catch"),
+      this);
+}
+
+void EmptyCatchCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCatchStmt = Result.Nodes.getNodeAs<CXXCatchStmt>("catch");
+
+  diag(
+      MatchedCatchStmt->getCatchLoc(),
+      "empty catch statements hide issues; to handle exceptions appropriately, "
+      "consider re-throwing, handling, or avoiding catch altogether");
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h

@@ -0,0 +1,37 @@
+//===--- EmptyCatchCheck.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_EMPTYCATCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EMPTYCATCHCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <vector>
+
+namespace clang::tidy::bugprone {
+
+/// Detects and suggests addressing issues with empty catch statements.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/empty-catch.html
+class EmptyCatchCheck : public ClangTidyCheck {
+public:
+  EmptyCatchCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override;
+
+private:
+  std::vector<llvm::StringRef> IgnoreCatchWithKeywords;
+  std::vector<llvm::StringRef> AllowEmptyCatchForExceptions;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EMPTYCATCHCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -114,6 +114,11 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-empty-catch
+  <clang-tidy/checks/bugprone/empty-catch>` check.
+
+  Detects and suggests addressing issues with empty catch statements.
+
 - New :doc:`bugprone-multiple-new-in-one-expression
   <clang-tidy/checks/bugprone/multiple-new-in-one-expression>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst

@@ -0,0 +1,149 @@
+.. title:: clang-tidy - bugprone-empty-catch
+
+bugprone-empty-catch
+====================
+
+Detects and suggests addressing issues with empty catch statements.
+
+.. code-block:: c++
+
+  try {
+    // Some code that can throw an exception
+  } catch(const std::exception&) {
+  }
+
+Having empty catch statements in a codebase can be a serious problem that
+developers should be aware of. Catch statements are used to handle exceptions
+that are thrown during program execution. When an exception is thrown, the
+program jumps to the nearest catch statement that matches the type of the
+exception.
+
+Empty catch statements, also known as "swallowing" exceptions, catch the
+exception but do nothing with it. This means that the exception is not handled
+properly, and the program continues to run as if nothing happened. This can
+lead to several issues, such as:
+
+* *Hidden Bugs*: If an exception is caught and ignored, it can lead to hidden
+  bugs that are difficult to diagnose and fix. The root cause of the problem
+  may not be apparent, and the program may continue to behave in unexpected
+  ways.
+
+* *Security Issues*: Ignoring exceptions can lead to security issues, such as
+  buffer overflows or null pointer dereferences. Hackers can exploit these
+  vulnerabilities to gain access to sensitive data or execute malicious code.
+
+* *Poor Code Quality*: Empty catch statements can indicate poor code quality
+  and a lack of attention to detail. This can make the codebase difficult to
+  maintain and update, leading to longer development cycles and increased
+  costs.
+
+* *Unreliable Code*: Code that ignores exceptions is often unreliable and can
+  lead to unpredictable behavior. This can cause frustration for users and
+  erode trust in the software.
+
+To avoid these issues, developers should always handle exceptions properly.
+This means either fixing the underlying issue that caused the exception or
+propagating the exception up the call stack to a higher-level handler.
+If an exception is not important, it should still be logged or reported in
+some way so that it can be tracked and addressed later.
+
+If the exception is something that can be handled locally, then it should be
+handled within the catch block. This could involve logging the exception or
+taking other appropriate action to ensure that the exception is not ignored.
+
+Here is an example:
+
+.. code-block:: c++
+
+  try {
+    // Some code that can throw an exception
+  } catch (const std::exception& ex) {
+    // Properly handle the exception, e.g.:
+    std::cerr << "Exception caught: " << ex.what() << std::endl;
+  }
+
+If the exception cannot be handled locally and needs to be propagated up the
+call stack, it should be re-thrown or new exception should be thrown.
+
+Here is an example:
+
+.. code-block:: c++
+
+  try {
+    // Some code that can throw an exception
+  } catch (const std::exception& ex) {
+    // Re-throw the exception
+    throw;
+  }
+
+In some cases, catching the exception at this level may not be necessary, and
+it may be appropriate to let the exception propagate up the call stack.
+This can be done simply by not using ``try/catch`` block.
+
+Here is an example:
+
+.. code-block:: c++
+
+  void function() {
+    // Some code that can throw an exception
+  }
+
+  void callerFunction() {
+    try {
+      function();
+    } catch (const std::exception& ex) {
+      // Handling exception on higher level
+      std::cerr << "Exception caught: " << ex.what() << std::endl;
+    }
+  }
+
+Other potential solution to avoid empty catch statements is to modify the code
+to avoid throwing the exception in the first place. This can be achieved by
+using a different API, checking for error conditions beforehand, or handling
+errors in a different way that does not involve exceptions. By eliminating the
+need for try-catch blocks, the code becomes simpler and less error-prone.
+
+Here is an example:
+
+.. code-block:: c++
+
+  // Old code:
+  try {
+    mapContainer["Key"].callFunction();
+  } catch(const std::out_of_range&) {
+  }
+
+  // New code
+  if (auto it = mapContainer.find("Key"); it != mapContainer.end()) {
+    it->second.callFunction();
+  }
+
+In conclusion, empty catch statements are a bad practice that can lead to hidden
+bugs, security issues, poor code quality, and unreliable code. By handling
+exceptions properly, developers can ensure that their code is robust, secure,
+and maintainable.
+
+Options
+-------
+
+.. option:: IgnoreCatchWithKeywords
+
+  This option can be used to ignore specific catch statements containing
+  certain keywords. If a ``catch`` statement body contains (case-insensitive)
+  any of the keywords listed in this semicolon-separated option, then the
+  catch will be ignored, and no warning will be raised.
+  Default value: `@TODO;@FIXME`.
+
+.. option:: AllowEmptyCatchForExceptions
+
+  This option can be used to ignore empty catch statements for specific
+  exception types. By default, the check will raise a warning if an empty
+  catch statement is detected, regardless of the type of exception being
+  caught. However, in certain situations, such as when a developer wants to
+  intentionally ignore certain exceptions or handle them in a different way,
+  it may be desirable to allow empty catch statements for specific exception
+  types.
+  To configure this option, a semicolon-separated list of exception type names
+  should be provided. If an exception type name in the list is caught in an
+  empty catch statement, no warning will be raised.
+  Default value: empty string.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -86,6 +86,7 @@ Clang-Tidy Checks
    `bugprone-dangling-handle <bugprone/dangling-handle.html>`_,
    `bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers.html>`_,
    `bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters.html>`_,
+   `bugprone-empty-catch <bugprone/empty-catch.html>`_,
    `bugprone-exception-escape <bugprone/exception-escape.html>`_,
    `bugprone-fold-init-type <bugprone/fold-init-type.html>`_,
    `bugprone-forward-declaration-namespace <bugprone/forward-declaration-namespace.html>`_,

clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp

@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-empty-catch %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-empty-catch.AllowEmptyCatchForExceptions, value: '::SafeException;WarnException'}, \
+// RUN:        {key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: '@IGNORE;@TODO'}]}" -- -fexceptions
+
+struct Exception {};
+struct SafeException {};
+struct WarnException : Exception {};
+
+int functionWithThrow() {
+  try {
+    throw 5;
+  } catch (const Exception &) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  } catch (...) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  }
+  return 0;
+}
+
+int functionWithHandling() {
+  try {
+    throw 5;
+  } catch (const Exception &) {
+    return 2;
+  } catch (...) {
+    return 1;
+  }
+  return 0;
+}
+
+int functionWithReThrow() {
+  try {
+    throw 5;
+  } catch (...) {
+    throw;
+  }
+}
+
+int functionWithNewThrow() {
+  try {
+    throw 5;
+  } catch (...) {
+    throw Exception();
+  }
+}
+
+void functionWithAllowedException() {
+  try {
+
+  } catch (const SafeException &) {
+  } catch (WarnException) {
+  }
+}
+
+void functionWithComment() {
+  try {
+  } catch (const Exception &) {
+    // @todo: implement later, check case insensitive
+  }
+}
+
+void functionWithComment2() {
+  try {
+  } catch (const Exception &) {
+    // @IGNORE: relax its safe
+  }
+}

llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn

@@ -27,6 +27,7 @@ static_library("bugprone") {
     "DanglingHandleCheck.cpp",
     "DynamicStaticInitializersCheck.cpp",
     "EasilySwappableParametersCheck.cpp",
+    "EmptyCatchCheck.cpp",
     "ExceptionEscapeCheck.cpp",
     "FoldInitTypeCheck.cpp",
     "ForwardDeclarationNamespaceCheck.cpp",