Author: Balázs Kéri Date: 2026-02-18T10:11:38+01:00 New Revision: 8f378ea7e6fa31179266c69368be56a866b631e1
URL: https://github.com/llvm/llvm-project/commit/8f378ea7e6fa31179266c69368be56a866b631e1 DIFF: https://github.com/llvm/llvm-project/commit/8f378ea7e6fa31179266c69368be56a866b631e1.diff LOG: [clang-tidy] Add check 'bugprone-unsafe-to-allow-exceptions' (#176430) `ExceptionEscapeCheck` does not warn if a function is `noexcept(false)` (may throw) but is one of the functions where throwing exceptions is unsafe. This check can be used to find such cases. Added: clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.cpp clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.h clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-to-allow-exceptions.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions-precpp17.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions.cpp Modified: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 4150442c25d61..310184037afbd 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -106,6 +106,7 @@ #include "UnintendedCharOstreamOutputCheck.h" #include "UniquePtrArrayMismatchCheck.h" #include "UnsafeFunctionsCheck.h" +#include "UnsafeToAllowExceptionsCheck.h" #include "UnusedLocalNonTrivialVariableCheck.h" #include "UnusedRaiiCheck.h" #include "UnusedReturnValueCheck.h" @@ -308,6 +309,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-crtp-constructor-accessibility"); CheckFactories.registerCheck<UnsafeFunctionsCheck>( "bugprone-unsafe-functions"); + CheckFactories.registerCheck<UnsafeToAllowExceptionsCheck>( + "bugprone-unsafe-to-allow-exceptions"); CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>( "bugprone-unused-local-non-trivial-variable"); CheckFactories.registerCheck<UnusedRaiiCheck>("bugprone-unused-raii"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index db1256d91d311..96ad671d03b39 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -108,6 +108,7 @@ add_clang_library(clangTidyBugproneModule STATIC UnhandledSelfAssignmentCheck.cpp UniquePtrArrayMismatchCheck.cpp UnsafeFunctionsCheck.cpp + UnsafeToAllowExceptionsCheck.cpp UnusedLocalNonTrivialVariableCheck.cpp UnusedRaiiCheck.cpp UnusedReturnValueCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.cpp new file mode 100644 index 0000000000000..f9a236aba4459 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.cpp @@ -0,0 +1,61 @@ +//===----------------------------------------------------------------------===// +// +// 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 "UnsafeToAllowExceptionsCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { +namespace { + +AST_MATCHER(FunctionDecl, isExplicitThrow) { + return isExplicitThrowExceptionSpec(Node.getExceptionSpecType()) && + Node.getExceptionSpecSourceRange().isValid(); +} + +} // namespace + +UnsafeToAllowExceptionsCheck::UnsafeToAllowExceptionsCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CheckedSwapFunctions(utils::options::parseStringList( + Options.get("CheckedSwapFunctions", "swap;iter_swap;iter_move"))) {} + +void UnsafeToAllowExceptionsCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckedSwapFunctions", + utils::options::serializeStringList(CheckedSwapFunctions)); +} + +void UnsafeToAllowExceptionsCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(isDefinition(), isExplicitThrow(), + anyOf(cxxDestructorDecl(), + cxxConstructorDecl(isMoveConstructor()), + cxxMethodDecl(isMoveAssignmentOperator()), + allOf(hasAnyName(CheckedSwapFunctions), + unless(parameterCountIs(0))), + isMain())) + .bind("f"), + this); +} + +void UnsafeToAllowExceptionsCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("f"); + assert(MatchedDecl); + + diag(MatchedDecl->getLocation(), + "function %0 should not throw exceptions but " + "it is still marked as potentially throwing") + << MatchedDecl; +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.h new file mode 100644 index 0000000000000..d91d48427ce4a --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.h @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// +// 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_UNSAFETOALLOWEXCEPTIONSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFETOALLOWEXCEPTIONSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Finds functions where throwing exceptions is unsafe but the function is +/// still marked as potentially throwing. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-to-allow-exceptions.html +class UnsafeToAllowExceptionsCheck : public ClangTidyCheck { +public: + UnsafeToAllowExceptionsCheck(StringRef Name, ClangTidyContext *Context); + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus && LangOpts.CXXExceptions; + } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::vector<llvm::StringRef> CheckedSwapFunctions; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFETOALLOWEXCEPTIONSCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2bb4800df47c9..68bab88146241 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -102,6 +102,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-unsafe-to-allow-exceptions + <clang-tidy/checks/bugprone/unsafe-to-allow-exceptions>` check. + + Finds functions where throwing exceptions is unsafe but the function is still + marked as potentially throwing. + - New :doc:`llvm-type-switch-case-types <clang-tidy/checks/llvm/type-switch-case-types>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst index a655661c57871..0a0c21f62c7a0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst @@ -28,7 +28,10 @@ Functions declared explicitly with ``noexcept(false)`` or ``throw(exception)`` will be excluded from the analysis, as even though it is not recommended for functions like ``swap()``, ``main()``, move constructors, move assignment operators and destructors, it is a clear indication of the developer's -intention and should be respected. +intention and should be respected. To check if these special functions are +marked as potentially throwing, the check +:doc:`bugprone-unsafe-to-allow-exceptions <unsafe-to-allow-exceptions>` can be +used. WARNING! This check may be expensive on large source files. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-to-allow-exceptions.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-to-allow-exceptions.rst new file mode 100644 index 0000000000000..894b1415e83dd --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-to-allow-exceptions.rst @@ -0,0 +1,41 @@ +.. title:: clang-tidy - bugprone-unsafe-to-allow-exceptions + +bugprone-unsafe-to-allow-exceptions +=================================== + +Finds functions where throwing exceptions is unsafe but the function is still +marked as potentially throwing. Throwing exceptions from the following +functions can be problematic: + +* Destructors +* Move constructors +* Move assignment operators +* The ``main()`` functions +* ``swap()`` functions +* ``iter_swap()`` functions +* ``iter_move()`` functions + +A destructor throwing an exception may result in undefined behavior, resource +leaks or unexpected termination of the program. Throwing move constructor or +move assignment also may result in undefined behavior or resource leak. The +``swap()`` operations expected to be non throwing most of the cases and they +are always possible to implement in a non throwing way. Non throwing ``swap()`` +operations are also used to create move operations. A throwing ``main()`` +function also results in unexpected termination. + +The check finds any of these functions if it is marked with ``noexcept(false)`` +or ``throw(exception)``. This would indicate that the function is expected to +throw exceptions. Only the presence of these keywords is checked, not if the +function actually throws any exception. To check if the function actually +throws exception, the check :doc:`bugprone-exception-escape <exception-escape>` +can be used (but it does not warn if a function is explicitly marked as +throwing). + +Options +------- + +.. option:: CheckedSwapFunctions + + Semicolon-separated list of checked swap function names (where throwing + exceptions is unsafe). These functions are checked if the parameter count is + at least 1. Default value is `swap;iter_swap;iter_move`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 0eabd9929dc39..c475870ed7b31 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -176,6 +176,7 @@ Clang-Tidy Checks :doc:`bugprone-unintended-char-ostream-output <bugprone/unintended-char-ostream-output>`, "Yes" :doc:`bugprone-unique-ptr-array-mismatch <bugprone/unique-ptr-array-mismatch>`, "Yes" :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`, + :doc:`bugprone-unsafe-to-allow-exceptions <bugprone/unsafe-to-allow-exceptions>`, :doc:`bugprone-unused-local-non-trivial-variable <bugprone/unused-local-non-trivial-variable>`, :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes" :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions-precpp17.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions-precpp17.cpp new file mode 100644 index 0000000000000..4f36557743cbd --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions-precpp17.cpp @@ -0,0 +1,49 @@ +// RUN: %check_clang_tidy -std=c++11,c++14 %s bugprone-unsafe-to-allow-exceptions %t -- \ +// RUN: -config="{CheckOptions: { \ +// RUN: bugprone-unsafe-to-allow-exceptions.CheckedSwapFunctions: 'swap', \ +// RUN: }}" \ +// RUN: -- -fexceptions + +class Exception {}; + +struct may_throw { + may_throw(may_throw&&) throw(int) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'may_throw' should not throw exceptions but it is still marked as potentially throwing [bugprone-unsafe-to-allow-exceptions] + } + may_throw& operator=(may_throw&&) throw(Exception) { + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: function 'operator=' should not throw exceptions but it is still marked as potentially throwing + } + ~may_throw() throw(char, Exception) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~may_throw' should not throw exceptions but it is still marked as potentially throwing + } + + void f() noexcept(false) { + } +}; + +struct no_throw { + no_throw(no_throw&&) throw() { + } + no_throw& operator=(no_throw&&) noexcept(true) { + } + ~no_throw() noexcept(true) { + } +}; + +int main() throw(char) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'main' should not throw exceptions but it is still marked as potentially throwing + return 0; +} + +void swap(no_throw&, no_throw&) throw(bool) { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'swap' should not throw exceptions but it is still marked as potentially throwing +} + +void iter_swap(int&, int&) throw(bool) { +} + +void iter_move(int&) throw(bool) { +} + +void swap(double&, double&) { +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions.cpp new file mode 100644 index 0000000000000..c6242881f295a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions.cpp @@ -0,0 +1,54 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-unsafe-to-allow-exceptions %t -- -- -fexceptions +// RUN: %check_clang_tidy -std=c++17-or-later -check-suffix=,CUSTOMSWAP %s bugprone-unsafe-to-allow-exceptions %t -- \ +// RUN: -config="{CheckOptions: { \ +// RUN: bugprone-unsafe-to-allow-exceptions.CheckedSwapFunctions: 'swap;iter_swap;iter_move;swap1', \ +// RUN: }}" \ +// RUN: -- -fexceptions + +struct may_throw { + may_throw(may_throw&&) noexcept(false) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'may_throw' should not throw exceptions but it is still marked as potentially throwing [bugprone-unsafe-to-allow-exceptions] + } + may_throw& operator=(may_throw&&) noexcept(false) { + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: function 'operator=' should not throw exceptions but it is still marked as potentially throwing + } + ~may_throw() noexcept(false) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~may_throw' should not throw exceptions but it is still marked as potentially throwing + } + + void f() noexcept(false) { + } +}; + +struct no_throw { + no_throw(no_throw&&) throw() { + } + no_throw& operator=(no_throw&&) noexcept(true) { + } + ~no_throw() noexcept(true) { + } +}; + +int main() noexcept(false) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'main' should not throw exceptions but it is still marked as potentially throwing + return 0; +} + +void swap(int&, int&) noexcept(false) { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'swap' should not throw exceptions but it is still marked as potentially throwing +} + +void iter_swap(int&, int&) noexcept(false) { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'iter_swap' should not throw exceptions but it is still marked as potentially throwing +} + +void iter_move(int&) noexcept(false) { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'iter_move' should not throw exceptions but it is still marked as potentially throwing +} + +void swap(double&, double&) { +} + +void swap1(long&) noexcept(false) { + // CHECK-MESSAGES-CUSTOMSWAP: :[[@LINE-1]]:6: warning: function 'swap1' should not throw exceptions but it is still marked as potentially throwing +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
