From 8f378ea7e6fa31179266c69368be56a866b631e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= Date: Wed, 18 Feb 2026 10:11:38 +0100 Subject: [PATCH] [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. --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/UnsafeToAllowExceptionsCheck.cpp | 61 +++++++++++++++++++ .../bugprone/UnsafeToAllowExceptionsCheck.h | 37 +++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../checks/bugprone/exception-escape.rst | 5 +- .../bugprone/unsafe-to-allow-exceptions.rst | 41 +++++++++++++ .../docs/clang-tidy/checks/list.rst | 1 + .../unsafe-to-allow-exceptions-precpp17.cpp | 49 +++++++++++++++ .../bugprone/unsafe-to-allow-exceptions.cpp | 54 ++++++++++++++++ 10 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnsafeToAllowExceptionsCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-to-allow-exceptions.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions-precpp17.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-to-allow-exceptions.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 4150442c25d6..310184037afb 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 @@ public: "bugprone-crtp-constructor-accessibility"); CheckFactories.registerCheck( "bugprone-unsafe-functions"); + CheckFactories.registerCheck( + "bugprone-unsafe-to-allow-exceptions"); CheckFactories.registerCheck( "bugprone-unused-local-non-trivial-variable"); CheckFactories.registerCheck("bugprone-unused-raii"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index db1256d91d31..96ad671d03b3 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 000000000000..f9a236aba445 --- /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("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 000000000000..d91d48427ce4 --- /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 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 2bb4800df47c..68bab8814624 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 + ` check. + + Finds functions where throwing exceptions is unsafe but the function is still + marked as potentially throwing. + - New :doc:`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 a655661c5787..0a0c21f62c7a 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 ` 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 000000000000..894b1415e83d --- /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 ` +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 0eabd9929dc3..c475870ed7b3 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 `, "Yes" :doc:`bugprone-unique-ptr-array-mismatch `, "Yes" :doc:`bugprone-unsafe-functions `, + :doc:`bugprone-unsafe-to-allow-exceptions `, :doc:`bugprone-unused-local-non-trivial-variable `, :doc:`bugprone-unused-raii `, "Yes" :doc:`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 000000000000..4f36557743cb --- /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 000000000000..c6242881f295 --- /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 +}