https://github.com/vbvictor created https://github.com/llvm/llvm-project/pull/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. >From c91ad611e7a64b08a243a4a7f07a7f51e96b8ac0 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Sun, 23 Mar 2025 23:57:15 +0300 Subject: [PATCH] [clang-tidy] Add support of `bind` function calls that capture `this` --- .../CapturingThisInMemberVariableCheck.cpp | 56 +++++++++++++----- .../CapturingThisInMemberVariableCheck.h | 1 + clang-tools-extra/docs/ReleaseNotes.rst | 5 +- .../capturing-this-in-member-variable.rst | 8 +++ .../capturing-this-in-member-variable.cpp | 58 ++++++++++++++++++- 5 files changed, 110 insertions(+), 18 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp index add0576a42c33..c0bcd7698a6ae 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp @@ -64,16 +64,21 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) { constexpr const char *DefaultFunctionWrapperTypes = "::std::function;::std::move_only_function;::boost::function"; +constexpr const char *DefaultBindFunctions = "::std::bind;::boost::bind"; 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 +92,54 @@ 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"); + const auto EmitDiag = [this](const SourceLocation &Location, + const FunctionDecl *Bind) { + const std::string BindName = Bind ? Bind->getQualifiedNameAsString() : ""; + diag(Location, "'this' captured by a %select{lambda|'%1' call}0 and " + "stored in a class member variable; disable implicit class " + "copying/moving to prevent potential use-after-free") + << (Bind ? 1 : 0) << BindName; + }; + + if (const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda")) { + EmitDiag(Lambda->getBeginLoc(), nullptr); + } else if (const auto *Bind = Result.Nodes.getNodeAs<CallExpr>("bind")) { + const auto *Callee = Result.Nodes.getNodeAs<FunctionDecl>("callee"); + assert(Callee); + EmitDiag(Bind->getBeginLoc(), Callee); + } + 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 72aa05eb4dcd1..396e071098a18 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -106,8 +106,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 bb75e9239d9b5..de05bfe3a34bc 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 @@ -32,8 +32,16 @@ Possible fixes: object types. - passing ``this`` pointer as parameter +Options +------- + .. option:: FunctionWrapperTypes 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`. 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
