[clang] [webkit.UncountedLambdaCapturesChecker] Ignore trivial functions and [[clang::noescape]]. (PR #113845)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 30 18:06:53 PDT 2024


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

>From 1524ca532c9c1ef015c162360d4e4688296bbc0d Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sun, 27 Oct 2024 16:30:48 -0700
Subject: [PATCH 1/5] [webkit.UncountedLambdaCapturesChecker] Ignore trivial
 functions and [[clang::noescape]].

This PR makes webkit.UncountedLambdaCapturesChecker ignore trivial functions as well as
the one being passed to an argument with [[clang::noescape]] attribute. This dramatically
reduces the false positive rate for this checker.

To do this, this PR replaces VisitLambdaExpr in favor of checking lambdas via VisitDeclRefExpr
and VisitCallExpr. The idea is that if a lambda is defined but never called or stored somewhere,
then capturing whatever variable in such a lambda is harmless.

VisitCallExpr explicitly looks for direct invocation of lambdas and registers its DeclRefExpr
to be ignored in VisitDeclRefExpr. If a lambda is being passed to a function, it checks whether
its argument is annotated with [[clang::noescape]]. If it's not annotated such, it checks
captures for their safety.

Because WTF::switchOn could not be annotated with [[clang::noescape]] as function type parameters
are variadic template function so we hard-code this function into the checker.

Finally, this PR also converts the accompanying test to use -verify and adds a bunch of tests.
---
 .../Checkers/WebKit/PtrTypesSemantics.h       |   4 +
 .../WebKit/UncountedLambdaCapturesChecker.cpp | 102 +++++++++--
 .../Analysis/Checkers/WebKit/mock-types.h     |   2 +
 .../WebKit/uncounted-lambda-captures.cpp      | 159 +++++++++++++++---
 4 files changed, 233 insertions(+), 34 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 4b41ca96e1df1d..814015c311d61e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -63,6 +63,10 @@ std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
 /// class, false if not, std::nullopt if inconclusive.
 std::optional<bool> isUncountedPtr(const clang::QualType T);
 
+/// \returns true if \p T is either a raw pointer or reference to an uncounted
+/// or unchecked class, false if not, std::nullopt if inconclusive.
+std::optional<bool> isUnsafePtr(const QualType T);
+
 /// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its
 /// variant, false if not.
 bool isSafePtrType(const clang::QualType T);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 998bd4ccee07db..02f68ac5e41317 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "ASTUtils.h"
 #include "DiagOutputUtils.h"
 #include "PtrTypesSemantics.h"
 #include "clang/AST/CXXInheritance.h"
@@ -26,6 +27,7 @@ class UncountedLambdaCapturesChecker
   BugType Bug{this, "Lambda capture of uncounted variable",
               "WebKit coding guidelines"};
   mutable BugReporter *BR = nullptr;
+  TrivialFunctionAnalysis TFA;
 
 public:
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -37,6 +39,8 @@ class UncountedLambdaCapturesChecker
     // want to visit those, so we make our own RecursiveASTVisitor.
     struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
       const UncountedLambdaCapturesChecker *Checker;
+      llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
+
       explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
           : Checker(Checker) {
         assert(Checker);
@@ -45,8 +49,61 @@ class UncountedLambdaCapturesChecker
       bool shouldVisitTemplateInstantiations() const { return true; }
       bool shouldVisitImplicitCode() const { return false; }
 
-      bool VisitLambdaExpr(LambdaExpr *L) {
-        Checker->visitLambdaExpr(L);
+      bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+        if (DeclRefExprsToIgnore.contains(DRE))
+          return true;
+        if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
+          auto *Init = VD->getInit()->IgnoreParenCasts();
+          if (auto *L = dyn_cast_or_null<LambdaExpr>(Init)) {
+            Checker->visitLambdaExpr(L);
+            return true;
+          }
+        }
+        return true;
+      }
+
+      // WTF::switchOn(T, F... f) is a variadic template function and couldn't be annotated with NOESCAPE.
+      // We hard code it here to workaround that.
+      bool shouldTreatAllArgAsNoEscape(FunctionDecl *Decl) {
+        auto *NsDecl = Decl->getParent();
+        if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
+          return false;
+        return safeGetName(NsDecl) == "WTF" && safeGetName(Decl) == "switchOn";
+      }
+
+      bool VisitCallExpr(CallExpr *CE) {
+        if (auto *Callee = CE->getCallee()) {
+          if (auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts())) {
+            if (auto *MD = dyn_cast_or_null<CXXMethodDecl>(DRE->getDecl())) {
+              if (CE->getNumArgs() == 1) {
+                auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+                if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+                  if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
+                    auto *Init = VD->getInit()->IgnoreParenCasts();
+                    if (auto *L = dyn_cast_or_null<LambdaExpr>(Init)) {
+                      DeclRefExprsToIgnore.insert(DRE);
+                      Checker->visitLambdaExpr(L, /* ignoreParamVarDecl */ true);
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+        if (auto *Callee = CE->getDirectCallee()) {
+          bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
+          unsigned ArgIndex = 0;
+          for (auto *Param : Callee->parameters()) {
+            if (ArgIndex >= CE->getNumArgs())
+              break;
+            auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
+            if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
+              if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg->IgnoreParenCasts()))
+                Checker->visitLambdaExpr(L);
+            }
+            ++ArgIndex;
+          }
+        }
         return true;
       }
     };
@@ -55,22 +112,28 @@ class UncountedLambdaCapturesChecker
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitLambdaExpr(LambdaExpr *L) const {
+  void visitLambdaExpr(LambdaExpr *L, bool ignoreParamVarDecl = false) const {
+    if (TFA.isTrivial(L->getBody()))
+      return;
     for (const LambdaCapture &C : L->captures()) {
       if (C.capturesVariable()) {
         ValueDecl *CapturedVar = C.getCapturedVar();
+        if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
+          continue;
         QualType CapturedVarQualType = CapturedVar->getType();
-        if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
-          auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
-          if (IsUncountedPtr && *IsUncountedPtr)
-            reportBug(C, CapturedVar, CapturedVarType);
-        }
+        auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
+        if (IsUncountedPtr && *IsUncountedPtr)
+          reportBug(C, CapturedVar, CapturedVarQualType);
+      } else if (C.capturesThis()) {
+        if (ignoreParamVarDecl) // this is always a parameter to this function.
+          continue;
+        reportBugOnThisPtr(C);
       }
     }
   }
 
   void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
-                 const Type *T) const {
+                 const QualType T) const {
     assert(CapturedVar);
 
     SmallString<100> Buf;
@@ -89,7 +152,26 @@ class UncountedLambdaCapturesChecker
     }
 
     printQuotedQualifiedName(Os, Capture.getCapturedVar());
-    Os << " to uncounted type is unsafe.";
+    Os << " to ref-counted / CheckedPtr capable type is unsafe.";
+
+    PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    BR->emitReport(std::move(Report));
+  }
+
+  void reportBugOnThisPtr(const LambdaCapture &Capture) const {
+    assert(CapturedVar);
+
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    if (Capture.isExplicit()) {
+      Os << "Captured ";
+    } else {
+      Os << "Implicitly captured ";
+    }
+
+    Os << "raw-pointer 'this' to ref-counted / CheckedPtr capable type is unsafe.";
 
     PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
     auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 8d8a90f0afae0e..1291227c1a772b 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -103,7 +103,9 @@ struct RefCountable {
   void ref() {}
   void deref() {}
   void method();
+  void constMethod() const;
   int trivial() { return 123; }
+  RefCountable* next();
 };
 
 template <typename T> T *downcast(T *t) { return t; }
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 27e0a74d583cd3..f676367fe74d0b 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -1,39 +1,73 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker %s 2>&1 | FileCheck %s --strict-whitespace
-#include "mock-types.h"
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
+//#include "mock-types.h"
+
+struct RefCountable {
+//  static Ref<RefCountable> create();
+  void ref() {}
+  void deref() {}
+  void method();
+  void constMethod() const;
+  int trivial() { return 123; }
+  RefCountable* next();
+};
+
+RefCountable* make_obj();
+
+void someFunction();
+template <typename Callback> void call(Callback callback) {
+  someFunction();
+  callback();
+}
 
 void raw_ptr() {
-  RefCountable* ref_countable = nullptr;
-  auto foo1 = [ref_countable](){};
-  // CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
-  // CHECK-NEXT:{{^   6 | }}  auto foo1 = [ref_countable](){};
-  // CHECK-NEXT:{{^     | }}               ^
-  auto foo2 = [&ref_countable](){};
-  // CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
-  auto foo3 = [&](){ ref_countable = nullptr; };
-  // CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
-  // CHECK-NEXT:{{^  12 | }}  auto foo3 = [&](){ ref_countable = nullptr; };
-  // CHECK-NEXT:{{^     | }}                     ^
-  auto foo4 = [=](){ (void) ref_countable; };
-  // CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  RefCountable* ref_countable = make_obj();
+  auto foo1 = [ref_countable](){
+    // expected-warning at -1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+    ref_countable->method();
+  };
+  auto foo2 = [&ref_countable](){
+    // expected-warning at -1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+    ref_countable->method();
+  };
+  auto foo3 = [&](){
+    ref_countable->method();
+    // expected-warning at -1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+    ref_countable = nullptr;
+  };
+
+  auto foo4 = [=](){
+    ref_countable->method();
+    // expected-warning at -1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  };
+
+  call(foo1);
+  call(foo2);
+  call(foo3);
+  call(foo4);
 
   // Confirm that the checker respects [[clang::suppress]].
   RefCountable* suppressed_ref_countable = nullptr;
   [[clang::suppress]] auto foo5 = [suppressed_ref_countable](){};
-  // CHECK-NOT: warning: Captured raw-pointer 'suppressed_ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  // no warning.
+  call(foo5);
 }
 
 void references() {
   RefCountable automatic;
   RefCountable& ref_countable_ref = automatic;
+  auto foo1 = [ref_countable_ref](){ ref_countable_ref.constMethod(); };
+  // expected-warning at -1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  auto foo2 = [&ref_countable_ref](){ ref_countable_ref.method(); };
+  // expected-warning at -1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  auto foo3 = [&](){ ref_countable_ref.method(); };
+  // expected-warning at -1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  auto foo4 = [=](){ ref_countable_ref.constMethod(); };
+  // expected-warning at -1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
 
-  auto foo1 = [ref_countable_ref](){};
-  // CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
-  auto foo2 = [&ref_countable_ref](){};
-  // CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
-  auto foo3 = [&](){ (void) ref_countable_ref; };
-  // CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
-  auto foo4 = [=](){ (void) ref_countable_ref; };
-  // CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+  call(foo1);
+  call(foo2);
+  call(foo3);
+  call(foo4);
 }
 
 void quiet() {
@@ -45,5 +79,82 @@ void quiet() {
 
   auto foo3 = [&]() {};
   auto foo4 = [=]() {};
+
+  call(foo3);
+  call(foo4);
+
   RefCountable *ref_countable = nullptr;
 }
+
+template <typename Callback>
+void map(RefCountable* start, [[clang::noescape]] Callback&& callback)
+{
+  while (start) {
+    callback(*start);
+    start = start->next();
+  }
+}
+
+template <typename Callback1, typename Callback2>
+void doubleMap(RefCountable* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2)
+{
+  while (start) {
+    callback1(*start);
+    callback2(*start);
+    start = start->next();
+  }
+}
+
+void noescape_lambda() {
+  RefCountable* someObj = make_obj();
+  RefCountable* otherObj = make_obj();
+  map(make_obj(), [&](RefCountable& obj) {
+    otherObj->method();
+  });
+  doubleMap(make_obj(), [&](RefCountable& obj) {
+    otherObj->method();
+  }, [&](RefCountable& obj) {
+    otherObj->method();
+    // expected-warning at -1{{Implicitly captured raw-pointer 'otherObj' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+  ([&] {
+    someObj->method();
+  })();
+}
+
+void lambda_capture_param(RefCountable* obj) {
+  auto someLambda = [&] {
+    obj->method();
+  };
+  someLambda();
+  someLambda();
+}
+
+struct RefCountableWithLambdaCapturingThis {
+  void ref() const;
+  void deref() const;
+  void nonTrivial();
+
+  void method_captures_this_safe() {
+    auto lambda = [&]() {
+      nonTrivial();
+    };
+    lambda();
+  }
+
+  void method_captures_this_unsafe() {
+    auto lambda = [&]() {
+      nonTrivial();
+      // expected-warning at -1{{Implicitly captured raw-pointer 'this' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+    };
+    call(lambda);
+  }
+};
+
+void trivial_lambda() {
+  RefCountable* ref_countable = make_obj();
+  auto trivial_lambda = [&]() {
+    return ref_countable->trivial();
+  };
+  trivial_lambda();
+}

>From a9e526e1a5281bcc6f00d9fa08a58a9d531c685d Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sun, 27 Oct 2024 16:40:30 -0700
Subject: [PATCH 2/5] Fix formatting.

---
 .../WebKit/UncountedLambdaCapturesChecker.cpp        | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 02f68ac5e41317..eb44db4f654aac 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -62,8 +62,8 @@ class UncountedLambdaCapturesChecker
         return true;
       }
 
-      // WTF::switchOn(T, F... f) is a variadic template function and couldn't be annotated with NOESCAPE.
-      // We hard code it here to workaround that.
+      // WTF::switchOn(T, F... f) is a variadic template function and couldn't
+      // be annotated with NOESCAPE. We hard code it here to workaround that.
       bool shouldTreatAllArgAsNoEscape(FunctionDecl *Decl) {
         auto *NsDecl = Decl->getParent();
         if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
@@ -82,7 +82,8 @@ class UncountedLambdaCapturesChecker
                     auto *Init = VD->getInit()->IgnoreParenCasts();
                     if (auto *L = dyn_cast_or_null<LambdaExpr>(Init)) {
                       DeclRefExprsToIgnore.insert(DRE);
-                      Checker->visitLambdaExpr(L, /* ignoreParamVarDecl */ true);
+                      Checker->visitLambdaExpr(L,
+                                               /* ignoreParamVarDecl */ true);
                     }
                   }
                 }
@@ -98,7 +99,7 @@ class UncountedLambdaCapturesChecker
               break;
             auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
             if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
-              if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg->IgnoreParenCasts()))
+              if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg))
                 Checker->visitLambdaExpr(L);
             }
             ++ArgIndex;
@@ -171,7 +172,8 @@ class UncountedLambdaCapturesChecker
       Os << "Implicitly captured ";
     }
 
-    Os << "raw-pointer 'this' to ref-counted / CheckedPtr capable type is unsafe.";
+    Os << "raw-pointer 'this' to ref-counted / CheckedPtr capable type is "
+          "unsafe.";
 
     PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
     auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);

>From 7806a15c47065b663dbc1a8969fbeeb9dd499813 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 30 Oct 2024 17:39:09 -0700
Subject: [PATCH 3/5] Use early returns instead of nested if's per review
 comment

---
 .../WebKit/UncountedLambdaCapturesChecker.cpp | 61 +++++++++++--------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index eb44db4f654aac..8807e8cb996f2e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -52,13 +52,14 @@ class UncountedLambdaCapturesChecker
       bool VisitDeclRefExpr(DeclRefExpr *DRE) {
         if (DeclRefExprsToIgnore.contains(DRE))
           return true;
-        if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
-          auto *Init = VD->getInit()->IgnoreParenCasts();
-          if (auto *L = dyn_cast_or_null<LambdaExpr>(Init)) {
-            Checker->visitLambdaExpr(L);
-            return true;
-          }
-        }
+        auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
+        if (!VD)
+          return true;
+        auto *Init = VD->getInit()->IgnoreParenCasts();
+        auto *L = dyn_cast_or_null<LambdaExpr>(Init);
+        if (!L)
+          return true;
+        Checker->visitLambdaExpr(L);
         return true;
       }
 
@@ -72,25 +73,7 @@ class UncountedLambdaCapturesChecker
       }
 
       bool VisitCallExpr(CallExpr *CE) {
-        if (auto *Callee = CE->getCallee()) {
-          if (auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts())) {
-            if (auto *MD = dyn_cast_or_null<CXXMethodDecl>(DRE->getDecl())) {
-              if (CE->getNumArgs() == 1) {
-                auto *Arg = CE->getArg(0)->IgnoreParenCasts();
-                if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
-                  if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
-                    auto *Init = VD->getInit()->IgnoreParenCasts();
-                    if (auto *L = dyn_cast_or_null<LambdaExpr>(Init)) {
-                      DeclRefExprsToIgnore.insert(DRE);
-                      Checker->visitLambdaExpr(L,
-                                               /* ignoreParamVarDecl */ true);
-                    }
-                  }
-                }
-              }
-            }
-          }
-        }
+        checkCalleeLambda(CE);
         if (auto *Callee = CE->getDirectCallee()) {
           bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
           unsigned ArgIndex = 0;
@@ -107,6 +90,32 @@ class UncountedLambdaCapturesChecker
         }
         return true;
       }
+
+      void checkCalleeLambda(CallExpr *CE) {
+        auto* Callee = CE->getCallee();
+        if (!Callee)
+          return;
+        auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts());
+        if (!DRE)
+          return;
+        auto *MD = dyn_cast_or_null<CXXMethodDecl>(DRE->getDecl());
+        if (!MD || CE->getNumArgs() != 1)
+          return;
+        auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+        auto *ArgRef = dyn_cast<DeclRefExpr>(Arg);
+        if (!ArgRef)
+          return;
+        auto *VD = dyn_cast_or_null<VarDecl>(ArgRef->getDecl());
+        if (!VD)
+          return;
+        auto *Init = VD->getInit()->IgnoreParenCasts();
+        auto *L = dyn_cast_or_null<LambdaExpr>(Init);
+        if (!L)
+          return;
+        DeclRefExprsToIgnore.insert(ArgRef);
+        Checker->visitLambdaExpr(L, /* ignoreParamVarDecl */ true);
+      }
+
     };
 
     LocalVisitor visitor(this);

>From 9006cd9abaaa66d33f99839123f74824b2e1a80b Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 30 Oct 2024 17:44:49 -0700
Subject: [PATCH 4/5] Fix formatting

---
 .../Checkers/WebKit/UncountedLambdaCapturesChecker.cpp          | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 8807e8cb996f2e..e689f1a60dcb44 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -92,7 +92,7 @@ class UncountedLambdaCapturesChecker
       }
 
       void checkCalleeLambda(CallExpr *CE) {
-        auto* Callee = CE->getCallee();
+        auto *Callee = CE->getCallee();
         if (!Callee)
           return;
         auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts());

>From 00c36d079510f9cdba881382de7327d8f58cf553 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 30 Oct 2024 18:06:14 -0700
Subject: [PATCH 5/5] Fix formatting 2

---
 .../Checkers/WebKit/UncountedLambdaCapturesChecker.cpp           | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index e689f1a60dcb44..f3db2d1987f328 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -115,7 +115,6 @@ class UncountedLambdaCapturesChecker
         DeclRefExprsToIgnore.insert(ArgRef);
         Checker->visitLambdaExpr(L, /* ignoreParamVarDecl */ true);
       }
-
     };
 
     LocalVisitor visitor(this);



More information about the cfe-commits mailing list