https://github.com/rniwa updated 
https://github.com/llvm/llvm-project/pull/119932

>From 5c032267f263fb6b7f10d25745d14e63b2f7af59 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 13 Dec 2024 14:34:39 -0800
Subject: [PATCH 1/2] [webkit.UncountedLambdaCapturesChecker] Detect
 protectedThis pattern.

In WebKit, we often capture this as Ref or RefPtr in addition to this itself so 
that
the object lives as long as a capturing lambda stays alive.

Detect this pattern and treat it as safe. This PR also makes the check for a 
lambda
being passed as a function argument more robust by handling 
CXXBindTemporaryExpr,
CXXConstructExpr, and DeclRefExpr referring to the lambda.
---
 .../WebKit/UncountedLambdaCapturesChecker.cpp | 79 ++++++++++++++++++-
 .../WebKit/uncounted-lambda-captures.cpp      | 47 ++++++++---
 2 files changed, 113 insertions(+), 13 deletions(-)

diff --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 599c2179db0f0e..299038262b8d36 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -96,9 +96,8 @@ class UncountedLambdaCapturesChecker
               return true;
             auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
             if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
-              if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
-                Checker->visitLambdaExpr(L, shouldCheckThis());
-              }
+              if (auto *Lambda = findLambdaInArg(Arg))
+                Checker->visitLambdaExpr(Lambda, shouldCheckThis());
             }
             ++ArgIndex;
           }
@@ -106,6 +105,38 @@ class UncountedLambdaCapturesChecker
         return true;
       }
 
+      LambdaExpr* findLambdaInArg(Expr* E) {
+        if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
+          return Lambda;
+        auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E);
+        if (!TempExpr)
+          return nullptr;
+        E = TempExpr->getSubExpr()->IgnoreParenCasts();
+        if (!E)
+          return nullptr;
+        if (auto *Lambda = dyn_cast<LambdaExpr>(E))
+          return Lambda;
+        auto *CE = dyn_cast_or_null<CXXConstructExpr>(E);
+        if (!CE || !CE->getNumArgs())
+          return nullptr;
+        auto *CtorArg = CE->getArg(0)->IgnoreParenCasts();
+        if (!CtorArg)
+          return nullptr;
+        if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg))
+          return Lambda;
+        auto *DRE = dyn_cast<DeclRefExpr>(CtorArg);
+        if (!DRE)
+          return nullptr;
+        auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
+        if (!VD)
+          return nullptr;
+        auto* Init = VD->getInit();
+        if (!Init)
+          return nullptr;
+        TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
+        return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr());
+      }
+
       void checkCalleeLambda(CallExpr *CE) {
         auto *Callee = CE->getCallee();
         if (!Callee)
@@ -155,11 +186,51 @@ class UncountedLambdaCapturesChecker
       } else if (C.capturesThis() && shouldCheckThis) {
         if (ignoreParamVarDecl) // this is always a parameter to this function.
           continue;
-        reportBugOnThisPtr(C);
+        bool hasProtectThis = false;
+        for (const LambdaCapture &OtherCapture : L->captures()) {
+          if (auto *ValueDecl = OtherCapture.getCapturedVar()) {
+            if (protectThis(ValueDecl)) {
+              hasProtectThis = true;
+              break;
+            }
+          }
+        }
+        if (!hasProtectThis)
+          reportBugOnThisPtr(C);
       }
     }
   }
 
+  bool protectThis(const ValueDecl *ValueDecl) const {
+    auto* VD = dyn_cast<VarDecl>(ValueDecl);
+    if (!VD)
+      return false;
+    auto *Init = VD->getInit()->IgnoreParenCasts();
+    if (!Init)
+      return false;
+    auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Init);
+    if (!BTE)
+      return false;
+    auto *CE = dyn_cast_or_null<CXXConstructExpr>(BTE->getSubExpr());
+    if (!CE)
+      return false;
+    auto* Ctor = CE->getConstructor();
+    if (!Ctor)
+      return false;
+    auto clsName = safeGetName(Ctor->getParent());
+    if (!isRefType(clsName) || !CE->getNumArgs())
+      return false;
+    auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+    while (auto *UO = dyn_cast<UnaryOperator>(Arg)) {
+      auto OpCode = UO->getOpcode();
+      if (OpCode == UO_Deref || OpCode == UO_AddrOf)
+        Arg = UO->getSubExpr();
+      else
+        break;
+    }
+    return isa<CXXThisExpr>(Arg);
+  }
+
   void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
                  const QualType T) const {
     assert(CapturedVar);
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp 
b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 65eee9d49106df..106fa02f4a89ef 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -1,18 +1,11 @@
 // RUN: %clang_analyze_cc1 
-analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
 
+#include "mock-types.h"
+
 struct A {
   static void b();
 };
 
-struct RefCountable {
-  void ref() {}
-  void deref() {}
-  void method();
-  void constMethod() const;
-  int trivial() { return 123; }
-  RefCountable* next();
-};
-
 RefCountable* make_obj();
 
 void someFunction();
@@ -151,6 +144,42 @@ struct RefCountableWithLambdaCapturingThis {
     };
     call(lambda);
   }
+
+  void method_captures_this_unsafe_capture_local_var_explicitly() {
+    RefCountable* x = make_obj();
+    call([this, protectedThis = RefPtr { this }, x]() {
+      // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or 
CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+      nonTrivial();
+      x->method();
+    });
+  }
+
+  void method_captures_this_unsafe_capture_local_var_explicitly_with_deref() {
+    RefCountable* x = make_obj();
+    call([this, protectedThis = Ref { *this }, x]() {
+      // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or 
CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+      nonTrivial();
+      x->method();
+    });
+  }
+
+  void method_captures_this_unsafe_local_var_via_vardecl() {
+    RefCountable* x = make_obj();
+    auto lambda = [this, protectedThis = Ref { *this }, x]() {
+      // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or 
CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+      nonTrivial();
+      x->method();
+    };
+    call(lambda);
+  }
+
+  void method_captures_this_with_guardian() {
+    auto lambda = [this, protectedThis = Ref { *this }]() {
+      nonTrivial();
+    };
+    call(lambda);
+  }
+
 };
 
 struct NonRefCountableWithLambdaCapturingThis {

>From 5b9e8fe00764cee42bdcb9e3031ba84ca4168690 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 13 Dec 2024 19:08:41 -0800
Subject: [PATCH 2/2] Fix formatting

---
 .../Checkers/WebKit/UncountedLambdaCapturesChecker.cpp    | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 299038262b8d36..20de89279ac620 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -105,7 +105,7 @@ class UncountedLambdaCapturesChecker
         return true;
       }
 
-      LambdaExpr* findLambdaInArg(Expr* E) {
+      LambdaExpr *findLambdaInArg(Expr* E) {
         if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
           return Lambda;
         auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E);
@@ -130,7 +130,7 @@ class UncountedLambdaCapturesChecker
         auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
         if (!VD)
           return nullptr;
-        auto* Init = VD->getInit();
+        auto *Init = VD->getInit();
         if (!Init)
           return nullptr;
         TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
@@ -202,7 +202,7 @@ class UncountedLambdaCapturesChecker
   }
 
   bool protectThis(const ValueDecl *ValueDecl) const {
-    auto* VD = dyn_cast<VarDecl>(ValueDecl);
+    auto *VD = dyn_cast<VarDecl>(ValueDecl);
     if (!VD)
       return false;
     auto *Init = VD->getInit()->IgnoreParenCasts();
@@ -214,7 +214,7 @@ class UncountedLambdaCapturesChecker
     auto *CE = dyn_cast_or_null<CXXConstructExpr>(BTE->getSubExpr());
     if (!CE)
       return false;
-    auto* Ctor = CE->getConstructor();
+    auto *Ctor = CE->getConstructor();
     if (!Ctor)
       return false;
     auto clsName = safeGetName(Ctor->getParent());

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to