Author: Baranov Victor Date: 2025-04-07T09:57:55+08:00 New Revision: da6e2454fff3fbc86861e31b60f18d3467354375
URL: https://github.com/llvm/llvm-project/commit/da6e2454fff3fbc86861e31b60f18d3467354375 DIFF: https://github.com/llvm/llvm-project/commit/da6e2454fff3fbc86861e31b60f18d3467354375.diff LOG: [clang-tidy] Improve `bugprone-capturing-this-in-member-variable` check: add support of `bind` functions. (#132635) Improve `bugprone-capturing-this-in-member-variable` check: Added support of `bind`-like functions that capture and store `this` pointer in class member. Closes https://github.com/llvm/llvm-project/issues/131220. Added: Modified: clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp index add0576a42c33..1bfe384258ddb 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp @@ -64,16 +64,23 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) { constexpr const char *DefaultFunctionWrapperTypes = "::std::function;::std::move_only_function;::boost::function"; +constexpr const char *DefaultBindFunctions = + "::std::bind;::boost::bind;::std::bind_front;::std::bind_back;" + "::boost::compat::bind_front;::boost::compat::bind_back"; CapturingThisInMemberVariableCheck::CapturingThisInMemberVariableCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), FunctionWrapperTypes(utils::options::parseStringList( - Options.get("FunctionWrapperTypes", DefaultFunctionWrapperTypes))) {} + Options.get("FunctionWrapperTypes", DefaultFunctionWrapperTypes))), + BindFunctions(utils::options::parseStringList( + Options.get("BindFunctions", DefaultBindFunctions))) {} void CapturingThisInMemberVariableCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "FunctionWrapperTypes", utils::options::serializeStringList(FunctionWrapperTypes)); + Options.store(Opts, "BindFunctions", + utils::options::serializeStringList(BindFunctions)); } void CapturingThisInMemberVariableCheck::registerMatchers(MatchFinder *Finder) { @@ -87,33 +94,52 @@ void CapturingThisInMemberVariableCheck::registerMatchers(MatchFinder *Finder) { // [self = this] capturesVar(varDecl(hasInitializer(cxxThisExpr()))))); auto IsLambdaCapturingThis = - lambdaExpr(hasAnyCapture(CaptureThis.bind("capture"))).bind("lambda"); - auto IsInitWithLambda = - anyOf(IsLambdaCapturingThis, - cxxConstructExpr(hasArgument(0, IsLambdaCapturingThis))); + lambdaExpr(hasAnyCapture(CaptureThis)).bind("lambda"); + + auto IsBindCapturingThis = + callExpr( + callee(functionDecl(matchers::matchesAnyListedName(BindFunctions)) + .bind("callee")), + hasAnyArgument(cxxThisExpr())) + .bind("bind"); + + auto IsInitWithLambdaOrBind = + anyOf(IsLambdaCapturingThis, IsBindCapturingThis, + cxxConstructExpr(hasArgument( + 0, anyOf(IsLambdaCapturingThis, IsBindCapturingThis)))); + Finder->addMatcher( cxxRecordDecl( anyOf(has(cxxConstructorDecl( unless(isCopyConstructor()), unless(isMoveConstructor()), hasAnyConstructorInitializer(cxxCtorInitializer( isMemberInitializer(), forField(IsStdFunctionField), - withInitializer(IsInitWithLambda))))), + withInitializer(IsInitWithLambdaOrBind))))), has(fieldDecl(IsStdFunctionField, - hasInClassInitializer(IsInitWithLambda)))), + hasInClassInitializer(IsInitWithLambdaOrBind)))), unless(correctHandleCaptureThisLambda())), this); } - void CapturingThisInMemberVariableCheck::check( const MatchFinder::MatchResult &Result) { - const auto *Capture = Result.Nodes.getNodeAs<LambdaCapture>("capture"); - const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda"); + if (const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda")) { + diag(Lambda->getBeginLoc(), + "'this' captured by a lambda and stored in a class member variable; " + "disable implicit class copying/moving to prevent potential " + "use-after-free"); + } else if (const auto *Bind = Result.Nodes.getNodeAs<CallExpr>("bind")) { + const auto *Callee = Result.Nodes.getNodeAs<FunctionDecl>("callee"); + assert(Callee); + diag(Bind->getBeginLoc(), + "'this' captured by a '%0' call and stored in a class member " + "variable; disable implicit class copying/moving to prevent potential " + "use-after-free") + << Callee->getQualifiedNameAsString(); + } + const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field"); - diag(Lambda->getBeginLoc(), - "'this' captured by a lambda and stored in a class member variable; " - "disable implicit class copying/moving to prevent potential " - "use-after-free") - << Capture->getLocation(); + assert(Field); + diag(Field->getLocation(), "class member of type '%0' that stores captured 'this'", DiagnosticIDs::Note) diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h index fe0b0aa10f108..934f99cd35797 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h @@ -37,6 +37,7 @@ class CapturingThisInMemberVariableCheck : public ClangTidyCheck { private: ///< store the function wrapper types const std::vector<StringRef> FunctionWrapperTypes; + const std::vector<StringRef> BindFunctions; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6c1f05009df98..fefb085409b44 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -103,8 +103,9 @@ New checks - New :doc:`bugprone-capturing-this-in-member-variable <clang-tidy/checks/bugprone/capturing-this-in-member-variable>` check. - Finds lambda captures that capture the ``this`` pointer and store it as class - members without handle the copy and move constructors and the assignments. + Finds lambda captures and ``bind`` function calls that capture the ``this`` + pointer and store it as class members without handle the copy and move + constructors and the assignments. - New :doc:`bugprone-unintended-char-ostream-output <clang-tidy/checks/bugprone/unintended-char-ostream-output>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst index b09d7d5fce959..dfc2ca1bbc7dd 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst @@ -40,3 +40,10 @@ Options A semicolon-separated list of names of types. Used to specify function wrapper that can hold lambda expressions. Default is `::std::function;::std::move_only_function;::boost::function`. + +.. option:: BindFunctions + + A semicolon-separated list of fully qualified names of functions that can + capture ``this`` pointer. + Default is `::std::bind;::boost::bind;::std::bind_front;::std::bind_back; + ::boost::compat::bind_front;::boost::compat::bind_back`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp index f5ebebfe4b058..4c90a8aa8944a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capturing-this-in-member-variable %t -- -config="{CheckOptions: {bugprone-capturing-this-in-member-variable.FunctionWrapperTypes: '::std::function;::Fn'}}" -- +// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capturing-this-in-member-variable %t -- -config="{CheckOptions: {bugprone-capturing-this-in-member-variable.FunctionWrapperTypes: '::std::function;::Fn', bugprone-capturing-this-in-member-variable.BindFunctions: '::std::bind;::Bind'}}" -- namespace std { @@ -12,12 +12,22 @@ class function<R(Args...)> { template<class F> function(F &&); }; +template <typename F, typename... Args> +function<F(Args...)> bind(F&&, Args&&...) { + return {}; +} + } // namespace std struct Fn { template<class F> Fn(F &&); }; +template <typename F, typename... Args> +std::function<F(Args...)> Bind(F&&, Args&&...) { + return {}; +} + struct BasicConstructor { BasicConstructor() : Captured([this]() { static_cast<void>(this); }) {} // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'this' captured by a lambda and stored in a class member variable; @@ -208,3 +218,49 @@ struct CustomFunctionWrapper { Fn Captured; // CHECK-MESSAGES: :[[@LINE-1]]:6: note: class member of type 'Fn' that stores captured 'this' }; + +struct BindConstructor { + BindConstructor() : Captured(std::bind(&BindConstructor::method, this)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'this' captured by a 'std::bind' call and stored in a class member variable; + void method() {} + std::function<void()> Captured; + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' +}; + +struct BindField1 { + void method() {} + std::function<void()> Captured = std::bind(&BindField1::method, this); + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 'this' captured by a 'std::bind' call and stored in a class member variable; + // CHECK-MESSAGES: :[[@LINE-2]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' +}; + +struct BindField2 { + void method() {} + std::function<void()> Captured{std::bind(&BindField2::method, this)}; + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'this' captured by a 'std::bind' call and stored in a class member variable; + // CHECK-MESSAGES: :[[@LINE-2]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' +}; + +struct BindCustom { + BindCustom() : Captured(Bind(&BindCustom::method, this)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'this' captured by a 'Bind' call and stored in a class member variable; + void method() {} + std::function<void()> Captured; + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' +}; + +struct BindNotCapturingThis { + void method(int) {} + BindNotCapturingThis(int V) : Captured(std::bind(&BindNotCapturingThis::method, V)) {} + std::function<void()> Captured; +}; + +struct DeletedCopyMoveWithBind { + DeletedCopyMoveWithBind() : Captured(std::bind(&DeletedCopyMoveWithBind::method, this)) {} + DeletedCopyMoveWithBind(DeletedCopyMoveWithBind const&) = delete; + DeletedCopyMoveWithBind(DeletedCopyMoveWithBind &&) = delete; + DeletedCopyMoveWithBind& operator=(DeletedCopyMoveWithBind const&) = delete; + DeletedCopyMoveWithBind& operator=(DeletedCopyMoveWithBind &&) = delete; + void method() {} + std::function<void()> Captured; +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
