[clang] [webkit.UncountedLambdaCapturesChecker] Add a fallback for checking lambda captures (PR #119800)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 12 18:25:28 PST 2024


https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/119800

This PR reintroduces VisitLambdaExpr in LocalVisitor so that lambdas used to constrcut WTF::Function, for example, would get checked for its lambda captures.

We explicitly ignore lambda used in initializing a VarDecl, and lambdas we evaluated with other means via newly introduced DenseSet, LambdasToIgnore.

>From 8133e89e58cf3568ea01010e437e741f981797c5 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 12 Dec 2024 18:18:18 -0800
Subject: [PATCH] [webkit.UncountedLambdaCapturesChecker] Add a fallback for
 checking lambda captures

This PR reintroduces VisitLambdaExpr in LocalVisitor so that lambdas used to constrcut
WTF::Function, for example, would get checked for its lambda captures.

We explicitly ignore lambda used in initializing a VarDecl, and lambdas we evaluated
with other means via newly introduced DenseSet, LambdasToIgnore.
---
 .../WebKit/UncountedLambdaCapturesChecker.cpp | 31 ++++++-
 .../WebKit/uncounted-lambda-captures.cpp      | 92 +++++++++++++++++--
 2 files changed, 111 insertions(+), 12 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 599c2179db0f0e..ac5cf3d899d55a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -40,6 +40,7 @@ class UncountedLambdaCapturesChecker
     struct LocalVisitor : DynamicRecursiveASTVisitor {
       const UncountedLambdaCapturesChecker *Checker;
       llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
+      llvm::DenseSet<const LambdaExpr *> LambdasToIgnore;
       QualType ClsType;
 
       explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
@@ -61,6 +62,24 @@ class UncountedLambdaCapturesChecker
         return result && *result;
       }
 
+      bool VisitLambdaExpr(LambdaExpr *L) override {
+        if (LambdasToIgnore.contains(L))
+          return true;
+        Checker->visitLambdaExpr(L, shouldCheckThis());
+        return true;
+      }
+
+      bool VisitVarDecl(VarDecl *VD) override {
+        auto *Init = VD->getInit();
+        if (!Init)
+          return true;
+        auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
+        if (!L)
+          return true;
+        LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
+        return true;
+      }
+
       bool VisitDeclRefExpr(DeclRefExpr *DRE) override {
         if (DeclRefExprsToIgnore.contains(DRE))
           return true;
@@ -73,6 +92,7 @@ class UncountedLambdaCapturesChecker
         auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
         if (!L)
           return true;
+        LambdasToIgnore.insert(L);
         Checker->visitLambdaExpr(L, shouldCheckThis());
         return true;
       }
@@ -95,10 +115,10 @@ class UncountedLambdaCapturesChecker
             if (ArgIndex >= CE->getNumArgs())
               return true;
             auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
-            if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
-              if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
+            if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
+              LambdasToIgnore.insert(L);
+              if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape)
                 Checker->visitLambdaExpr(L, shouldCheckThis());
-              }
             }
             ++ArgIndex;
           }
@@ -117,6 +137,10 @@ class UncountedLambdaCapturesChecker
         if (!MD || CE->getNumArgs() < 1)
           return;
         auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+        if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
+          LambdasToIgnore.insert(L); // Calling a lambda upon creation is safe.
+          return;
+        }
         auto *ArgRef = dyn_cast<DeclRefExpr>(Arg);
         if (!ArgRef)
           return;
@@ -130,6 +154,7 @@ class UncountedLambdaCapturesChecker
         if (!L)
           return;
         DeclRefExprsToIgnore.insert(ArgRef);
+        LambdasToIgnore.insert(L);
         Checker->visitLambdaExpr(L, shouldCheckThis(),
                                  /* ignoreParamVarDecl */ true);
       }
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 65eee9d49106df..daff32e9940c83 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -1,16 +1,72 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
 
-struct A {
-  static void b();
+#include "mock-types.h"
+
+namespace WTF {
+
+namespace Detail {
+
+template<typename Out, typename... In>
+class CallableWrapperBase {
+public:
+    virtual ~CallableWrapperBase() { }
+    virtual Out call(In...) = 0;
+};
+
+template<typename, typename, typename...> class CallableWrapper;
+
+template<typename CallableType, typename Out, typename... In>
+class CallableWrapper : public CallableWrapperBase<Out, In...> {
+public:
+    explicit CallableWrapper(CallableType& callable)
+        : m_callable(callable) { }
+    Out call(In... in) final { return m_callable(in...); }
+
+private:
+    CallableType m_callable;
+};
+
+} // namespace Detail
+
+template<typename> class Function;
+
+template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>*);
+
+template <typename Out, typename... In>
+class Function<Out(In...)> {
+public:
+    using Impl = Detail::CallableWrapperBase<Out, In...>;
+
+    Function() = default;
+
+    template<typename FunctionType>
+    Function(FunctionType f)
+        : m_callableWrapper(new Detail::CallableWrapper<FunctionType, Out, In...>(f)) { }
+
+    Out operator()(In... in) const { return m_callableWrapper->call(in...); }
+    explicit operator bool() const { return !!m_callableWrapper; }
+
+private:
+    enum AdoptTag { Adopt };
+    Function(Impl* impl, AdoptTag)
+        : m_callableWrapper(impl)
+    {
+    }
+
+    friend Function adopt<Out, In...>(Impl*);
+
+    std::unique_ptr<Impl> m_callableWrapper;
 };
 
-struct RefCountable {
-  void ref() {}
-  void deref() {}
-  void method();
-  void constMethod() const;
-  int trivial() { return 123; }
-  RefCountable* next();
+template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>* impl)
+{
+    return Function<Out(In...)>(impl, Function<Out(In...)>::Adopt);
+}
+
+} // namespace WTF
+
+struct A {
+  static void b();
 };
 
 RefCountable* make_obj();
@@ -185,3 +241,21 @@ void lambda_with_args(RefCountable* obj) {
   };
   trivial_lambda(1);
 }
+
+void callFunctionOpaque(WTF::Function<void()>&&);
+void callFunction(WTF::Function<void()>&& function) {
+  someFunction();
+  function();
+}
+
+void lambda_converted_to_function(RefCountable* obj)
+{
+  callFunction([&]() {
+    obj->method();
+    // expected-warning at -1{{Implicitly captured raw-pointer 'obj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+  callFunctionOpaque([&]() {
+    obj->method();
+    // expected-warning at -1{{Implicitly captured raw-pointer 'obj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+  });
+}



More information about the cfe-commits mailing list