diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/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" @@ -102,6 +103,7 @@ "bugprone-dynamic-static-initializers"); CheckFactories.registerCheck( "bugprone-easily-swappable-parameters"); + CheckFactories.registerCheck("bugprone-empty-catch"); CheckFactories.registerCheck( "bugprone-exception-escape"); CheckFactories.registerCheck("bugprone-fold-init-type"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -15,6 +15,7 @@ DanglingHandleCheck.cpp DynamicStaticInitializersCheck.cpp EasilySwappableParametersCheck.cpp + EmptyCatchCheck.cpp ExceptionEscapeCheck.cpp FoldInitTypeCheck.cpp ForwardDeclarationNamespaceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h @@ -0,0 +1,36 @@ +//===--- 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 + +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; + +private: + std::vector IgnoreCatchWithKeywords; + std::vector AllowEmptyCatchForExceptions; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EMPTYCATCHCHECK_H diff --git a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp @@ -0,0 +1,108 @@ +//===--- 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 + +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, InnerMatcher) { + Stmt *Handler = Node.getHandlerBlock(); + if (!Handler) + return false; + return InnerMatcher.matches(*Handler, Finder, Builder); +} + +AST_MATCHER_P(CXXCatchStmt, hasCaughtType, Matcher, InnerMatcher) { + return InnerMatcher.matches(Node.getCaughtType(), Finder, Builder); +} + +AST_MATCHER_P(CompoundStmt, hasAnyTextFromList, std::vector, + 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; +} + +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( + traverse(TK_IgnoreUnlessSpelledInSource, + 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("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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -101,6 +101,11 @@ New checks ^^^^^^^^^^ +- New :doc:`bugprone-empty-catch + ` check. + + Detects and suggests addressing issues with empty catch statements. + - New :doc:`bugprone-unsafe-functions ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst @@ -0,0 +1,170 @@ +.. 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. + +**Say goodbye to lurking exceptions in your codebase with this Clang-tidy check +- keeping your code safe and robust!** + +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. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -86,6 +86,7 @@ `bugprone-dangling-handle `_, `bugprone-dynamic-static-initializers `_, `bugprone-easily-swappable-parameters `_, + `bugprone-empty-catch `_, `bugprone-exception-escape `_, `bugprone-fold-init-type `_, `bugprone-forward-declaration-namespace `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp @@ -0,0 +1,101 @@ +// 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'}]}" + +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 + } +}