[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 #131220.

Author: vbvictor

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)

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

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.

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`.

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;
+};