[webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. #120528

rniwa · 2024-12-19T07:42:42Z

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.

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.

llvmbot · 2024-12-19T07:43:15Z

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/120528.diff

2 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp (+74-2)
(modified) clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp (+36)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index d786b02e2d7f3a..5c35a9a738c5ed 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -115,7 +115,7 @@ class UncountedLambdaCapturesChecker
             if (ArgIndex >= CE->getNumArgs())
               return true;
             auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
-            if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
+            if (auto *L = findLambdaInArg(Arg)) {
               LambdasToIgnore.insert(L);
               if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape)
                 Checker->visitLambdaExpr(L, shouldCheckThis());
@@ -126,6 +126,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)
@@ -180,11 +212,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 daff32e9940c83..b2cadbe50786e3 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -207,6 +207,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 {

t-rasmud · 2024-12-19T07:46:12Z

clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp

+    while (auto *UO = dyn_cast<UnaryOperator>(Arg)) {
+      auto OpCode = UO->getOpcode();
+      if (OpCode == UO_Deref || OpCode == UO_AddrOf)
+        Arg = UO->getSubExpr();


Can we add tests where the Arg isn't this and for UO_AddrOf?

Sure, & case is a bit silly because it's &*this but added anyway.

t-rasmud · 2024-12-19T08:41:56Z

clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp

+    };
+    call(lambda);
+  }
+
 };


@rniwa I'm running into the following crash when I run this test locally:

RUN: at line 1: /Users/admin/webkit-pr/build-xcode/bin/clang -cc1 -internal-isystem /Users/admin/webkit-pr/build-xcode/lib/clang/20/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify /Users/admin/webkit-pr/llvm-project/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp + /Users/admin/webkit-pr/build-xcode/bin/clang -cc1 -internal-isystem /Users/admin/webkit-pr/build-xcode/lib/clang/20/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify /Users/admin/webkit-pr/llvm-project/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp Assertion failed: (capturesVariable() && "No variable available for capture"), function getCapturedVar, file LambdaCapture.h, line 105. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump: 0. Program arguments: /Users/admin/webkit-pr/build-xcode/bin/clang -cc1 -internal-isystem /Users/admin/webkit-pr/build-xcode/lib/clang/20/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify /Users/admin/webkit-pr/llvm-project/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp 1. <eof> parser at end of file #0 0x000000010a558d5c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10a048d5c) #1 0x000000010a559258 PrintStackTraceSignalHandler(void*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10a049258) #2 0x000000010a5572b0 llvm::sys::RunSignalHandlers() (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10a0472b0) #3 0x000000010a559fbc SignalHandler(int) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10a049fbc) #4 0x00000001991dc184 (/usr/lib/system/libsystem_platform.dylib+0x180484184) #5 0x00000001991a6f70 (/usr/lib/system/libsystem_pthread.dylib+0x18044ef70) #6 0x00000001990b3908 (/usr/lib/system/libsystem_c.dylib+0x18035b908) #7 0x00000001990b2c1c (/usr/lib/system/libsystem_c.dylib+0x18035ac1c) #8 0x000000010217f778 clang::LambdaCapture::getCapturedVar() const (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x101c6f778) #9 0x0000000105979588 (anonymous namespace)::UncountedLambdaCapturesChecker::visitLambdaExpr(clang::LambdaExpr*, bool, bool) const (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x105469588) #10 0x00000001059775b8 (anonymous namespace)::UncountedLambdaCapturesChecker::checkASTDecl(clang::TranslationUnitDecl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) const::LocalVisitor::VisitDeclRefExpr(clang::DeclRefExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1054675b8) #11 0x0000000107eb5ec8 (anonymous namespace)::Impl::VisitDeclRefExpr(clang::DeclRefExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079a5ec8) #12 0x0000000107e62148 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::WalkUpFromDeclRefExpr(clang::DeclRefExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107952148) #13 0x0000000107e446c4 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseDeclRefExpr(clang::DeclRefExpr*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079346c4) #14 0x0000000107e44664 clang::DynamicRecursiveASTVisitor::TraverseDeclRefExpr(clang::DeclRefExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107934664) #15 0x0000000107ea5b78 (anonymous namespace)::Impl::TraverseDeclRefExpr(clang::DeclRefExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107995b78) #16 0x0000000107e24b68 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107914b68) #17 0x0000000107e2432c clang::DynamicRecursiveASTVisitor::dataTraverseNode(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10791432c) #18 0x0000000107ea4bb8 (anonymous namespace)::Impl::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107994bb8) #19 0x0000000107e20f04 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseStmt(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910f04) #20 0x0000000107e20cf8 clang::DynamicRecursiveASTVisitor::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910cf8) #21 0x0000000107e94d10 (anonymous namespace)::Impl::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107984d10) #22 0x0000000107e45af8 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseImplicitCastExpr(clang::ImplicitCastExpr*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107935af8) #23 0x0000000107e459d4 clang::DynamicRecursiveASTVisitor::TraverseImplicitCastExpr(clang::ImplicitCastExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079359d4) #24 0x0000000107ea5d38 (anonymous namespace)::Impl::TraverseImplicitCastExpr(clang::ImplicitCastExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107995d38) #25 0x0000000107e24c48 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107914c48) #26 0x0000000107e2432c clang::DynamicRecursiveASTVisitor::dataTraverseNode(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10791432c) #27 0x0000000107ea4bb8 (anonymous namespace)::Impl::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107994bb8) #28 0x0000000107e20f04 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseStmt(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910f04) #29 0x0000000107e20cf8 clang::DynamicRecursiveASTVisitor::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910cf8) #30 0x0000000107e94d10 (anonymous namespace)::Impl::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107984d10) #31 0x0000000107e4aacc clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseCXXConstructExpr(clang::CXXConstructExpr*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10793aacc) #32 0x0000000107e4a9a8 clang::DynamicRecursiveASTVisitor::TraverseCXXConstructExpr(clang::CXXConstructExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10793a9a8) #33 0x0000000107ea64a8 (anonymous namespace)::Impl::TraverseCXXConstructExpr(clang::CXXConstructExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079964a8) #34 0x0000000107e25000 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107915000) #35 0x0000000107e2432c clang::DynamicRecursiveASTVisitor::dataTraverseNode(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10791432c) #36 0x0000000107ea4bb8 (anonymous namespace)::Impl::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107994bb8) #37 0x0000000107e20f04 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseStmt(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910f04) #38 0x0000000107e20cf8 clang::DynamicRecursiveASTVisitor::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910cf8) #39 0x0000000107e94d10 (anonymous namespace)::Impl::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107984d10) #40 0x0000000107e47300 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseCallExpr(clang::CallExpr*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107937300) #41 0x0000000107e471dc clang::DynamicRecursiveASTVisitor::TraverseCallExpr(clang::CallExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079371dc) #42 0x0000000107ea5f68 (anonymous namespace)::Impl::TraverseCallExpr(clang::CallExpr*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107995f68) #43 0x0000000107e24d60 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107914d60) #44 0x0000000107e2432c clang::DynamicRecursiveASTVisitor::dataTraverseNode(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10791432c) #45 0x0000000107ea4bb8 (anonymous namespace)::Impl::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107994bb8) #46 0x0000000107e20f04 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseStmt(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910f04) #47 0x0000000107e20cf8 clang::DynamicRecursiveASTVisitor::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910cf8) #48 0x0000000107e94d10 (anonymous namespace)::Impl::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107984d10) #49 0x0000000107e5c844 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseCompoundStmt(clang::CompoundStmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10794c844) #50 0x0000000107e5c720 clang::DynamicRecursiveASTVisitor::TraverseCompoundStmt(clang::CompoundStmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10794c720) #51 0x0000000107ea8070 (anonymous namespace)::Impl::TraverseCompoundStmt(clang::CompoundStmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107998070) #52 0x0000000107e25de4 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107915de4) #53 0x0000000107e2432c clang::DynamicRecursiveASTVisitor::dataTraverseNode(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10791432c) #54 0x0000000107ea4bb8 (anonymous namespace)::Impl::dataTraverseNode(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107994bb8) #55 0x0000000107e20f04 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseStmt(clang::Stmt*, llvm::SmallVectorImpl<llvm::PointerIntPair<clang::Stmt*, 1u, bool, llvm::PointerLikeTypeTraits<clang::Stmt*>, llvm::PointerIntPairInfo<clang::Stmt*, 1u, llvm::PointerLikeTypeTraits<clang::Stmt*>>>>*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910f04) #56 0x0000000107e20cf8 clang::DynamicRecursiveASTVisitor::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910cf8) #57 0x0000000107e94d10 (anonymous namespace)::Impl::TraverseStmt(clang::Stmt*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107984d10) #58 0x0000000107eb2c10 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseFunctionHelper(clang::FunctionDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079a2c10) #59 0x0000000107e2db24 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseCXXMethodDecl(clang::CXXMethodDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10791db24) #60 0x0000000107e2da98 clang::DynamicRecursiveASTVisitor::TraverseCXXMethodDecl(clang::CXXMethodDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10791da98) #61 0x0000000105977308 (anonymous namespace)::UncountedLambdaCapturesChecker::checkASTDecl(clang::TranslationUnitDecl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) const::LocalVisitor::TraverseCXXMethodDecl(clang::CXXMethodDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x105467308) #62 0x0000000107e983a4 (anonymous namespace)::Impl::TraverseCXXMethodDecl(clang::CXXMethodDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079883a4) #63 0x0000000107e20008 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910008) #64 0x0000000107e1f8ac clang::DynamicRecursiveASTVisitor::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10790f8ac) #65 0x0000000107e83784 (anonymous namespace)::Impl::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107973784) #66 0x0000000107eaa5e4 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseDeclContextHelper(clang::DeclContext*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10799a5e4) #67 0x0000000107e32fdc clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseCXXRecordDecl(clang::CXXRecordDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107922fdc) #68 0x0000000107e32ef0 clang::DynamicRecursiveASTVisitor::TraverseCXXRecordDecl(clang::CXXRecordDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107922ef0) #69 0x0000000107e988e4 (anonymous namespace)::Impl::TraverseCXXRecordDecl(clang::CXXRecordDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079888e4) #70 0x0000000107e20488 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107910488) #71 0x0000000107e1f8ac clang::DynamicRecursiveASTVisitor::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10790f8ac) #72 0x0000000107e83784 (anonymous namespace)::Impl::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107973784) #73 0x0000000107eaa5e4 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseDeclContextHelper(clang::DeclContext*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10799a5e4) #74 0x0000000107e26a98 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseTranslationUnitDecl(clang::TranslationUnitDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107916a98) #75 0x0000000107e268a0 clang::DynamicRecursiveASTVisitor::TraverseTranslationUnitDecl(clang::TranslationUnitDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1079168a0) #76 0x0000000107e97ca4 (anonymous namespace)::Impl::TraverseTranslationUnitDecl(clang::TranslationUnitDecl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x107987ca4) #77 0x0000000107e1fa08 clang::RecursiveASTVisitor<(anonymous namespace)::Impl>::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10790fa08) #78 0x0000000107e1f8ac clang::DynamicRecursiveASTVisitor::TraverseDecl(clang::Decl*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10790f8ac) #79 0x00000001059770b8 (anonymous namespace)::UncountedLambdaCapturesChecker::checkASTDecl(clang::TranslationUnitDecl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) const (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x1054670b8) #80 0x000000010597705c void clang::ento::check::ASTDecl<clang::TranslationUnitDecl>::_checkDecl<(anonymous namespace)::UncountedLambdaCapturesChecker>(void*, clang::Decl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10546705c) #81 0x0000000105a18994 clang::ento::CheckerFn<void (clang::Decl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&)>::operator()(clang::Decl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) const (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x105508994) #82 0x0000000105a18578 clang::ento::CheckerManager::runCheckersOnASTDecl(clang::Decl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x105508578) #83 0x00000001054dd068 (anonymous namespace)::AnalysisConsumer::runAnalysisOnTranslationUnit(clang::ASTContext&) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x104fcd068) #84 0x00000001054d3138 (anonymous namespace)::AnalysisConsumer::HandleTranslationUnit(clang::ASTContext&) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x104fc3138) #85 0x0000000106169664 clang::ParseAST(clang::Sema&, bool, bool) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x105c59664) #86 0x0000000105e5689c clang::ASTFrontendAction::ExecuteAction() (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10594689c) #87 0x0000000105e56138 clang::FrontendAction::Execute() (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x105946138) #88 0x0000000105d6cbdc clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10585cbdc) #89 0x000000010125381c clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x100d4381c) #90 0x0000000100521398 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x100011398) #91 0x0000000100513154 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x100003154) #92 0x0000000100511e80 clang_main(int, char**, llvm::ToolContext const&) (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x100001e80) #93 0x000000010054ab94 main (/Users/admin/webkit-pr/build-xcode/bin/clang-20+0x10003ab94) #94 0x0000000198e24274 /Users/admin/webkit-pr/build-xcode/tools/clang/test/Analysis/Checkers/WebKit/Output/uncounted-lambda-captures.cpp.script: line 1: 33767 Abort trap: 6 /Users/admin/webkit-pr/build-xcode/bin/clang -cc1 -internal-isystem /Users/admin/webkit-pr/build-xcode/lib/clang/20/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify /Users/admin/webkit-pr/llvm-project/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp

Oh oops, fixed!

github-actions · 2024-12-19T20:55:37Z

✅ With the latest revision this PR passed the C/C++ code formatter.

t-rasmud

LGTM!

rniwa · 2024-12-20T02:10:12Z

Thanks for the review

…llvm#120528) 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.

rniwa requested a review from t-rasmud December 19, 2024 07:42

llvmbot added clang clang:static analyzer labels Dec 19, 2024

t-rasmud reviewed Dec 19, 2024

View reviewed changes

Added more test cases

Loading
Loading status checks…

3b5d22f

t-rasmud reviewed Dec 19, 2024

View reviewed changes

Fix debug assertion in visitLambdaExpr

Loading
Loading status checks…

6d56aea

Fix formatting

Loading
Loading status checks…

91dd080

t-rasmud approved these changes Dec 20, 2024

View reviewed changes

rniwa merged commit a71f9e6 into llvm:main Dec 20, 2024
8 checks passed

rniwa deleted the detect-protected-this-pattern-lambda-captures branch December 20, 2024 02:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. #120528

[webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. #120528

rniwa commented Dec 19, 2024

llvmbot commented Dec 19, 2024 •

edited

Loading

t-rasmud Dec 19, 2024

rniwa Dec 19, 2024

t-rasmud Dec 19, 2024

rniwa Dec 19, 2024

github-actions bot commented Dec 19, 2024 •

edited

Loading

t-rasmud left a comment

rniwa commented Dec 20, 2024

[webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. #120528

[webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. #120528

Conversation

rniwa commented Dec 19, 2024

llvmbot commented Dec 19, 2024 • edited Loading

t-rasmud Dec 19, 2024

Choose a reason for hiding this comment

rniwa Dec 19, 2024

Choose a reason for hiding this comment

t-rasmud Dec 19, 2024

Choose a reason for hiding this comment

rniwa Dec 19, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 19, 2024 • edited Loading

t-rasmud left a comment

Choose a reason for hiding this comment

rniwa commented Dec 20, 2024

llvmbot commented Dec 19, 2024 •

edited

Loading

github-actions bot commented Dec 19, 2024 •

edited

Loading