[clang] [webkit.RefCntblBaseVirtualDtor] ThreadSafeRefCounted still generates warnings (PR #108656)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 17 19:13:56 PDT 2024


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

>From e86f101c5cd87db597c5fc8ab017b20adba98284 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 13 Sep 2024 15:19:45 -0700
Subject: [PATCH 1/2] [webkit.RefCntblBaseVirtualDtor] ThreadSafeRefCounted
 still generates warnings

Improve the fix in 203a2ca8cd6af505e11a38aebceeaf864271042c by allowing variable references
and more ignoring of parentheses.
---
 .../WebKit/RefCntblBaseVirtualDtorChecker.cpp | 19 ++++++++++++++-----
 .../ref-cntbl-crtp-base-no-virtual-dtor.cpp   | 18 ++++++++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index ecba5f9aa23ee3..5ea19233bb65c2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -84,13 +84,22 @@ class DerefFuncDeleteExprVisitor
     E = E->IgnoreParenCasts();
     if (auto *TempE = dyn_cast<CXXBindTemporaryExpr>(E))
       E = TempE->getSubExpr();
+    E = E->IgnoreParenCasts();
+    if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
+      if (auto *Decl = Ref->getDecl()) {
+        if (auto *VD = dyn_cast<VarDecl>(Decl))
+          return VisitLabmdaArgument(VD->getInit());
+      }
+      return false;
+    }
+    if (auto *Lambda = dyn_cast<LambdaExpr>(E)) {
+      if (VisitBody(Lambda->getBody()))
+        return true;
+    }
     if (auto *ConstructE = dyn_cast<CXXConstructExpr>(E)) {
       for (unsigned i = 0; i < ConstructE->getNumArgs(); ++i) {
-        auto *Arg = ConstructE->getArg(i);
-        if (auto *Lambda = dyn_cast<LambdaExpr>(Arg)) {
-          if (VisitBody(Lambda->getBody()))
-            return true;
-        }
+        if (VisitLabmdaArgument(ConstructE->getArg(i)))
+          return true;
       }
     }
     return false;
diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
index 01527addb52992..33c60ea8ca64d1 100644
--- a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
@@ -119,6 +119,11 @@ template<class T, DestructionThread destructionThread = DestructionThread::Any>
             ensureOnMainThread([this] {
                 delete static_cast<const T*>(this);
             });
+        } else if constexpr (destructionThread == DestructionThread::MainRunLoop) {
+            auto deleteThis = [this] {
+                delete static_cast<const T*>(this);
+            };
+            ensureOnMainThread(deleteThis);
         }
     }
 
@@ -230,3 +235,16 @@ class FancyRefCountedClass4 final : public BadNestedThreadSafeRefCounted<FancyRe
 private:
     FancyRefCountedClass4();
 };
+
+class FancyRefCountedClass5 final : public ThreadSafeRefCounted<FancyRefCountedClass5, DestructionThread::MainRunLoop> {
+public:
+    static Ref<FancyRefCountedClass5> create()
+    {
+        return adoptRef(*new FancyRefCountedClass5());
+    }
+
+    virtual ~FancyRefCountedClass5();
+
+private:
+    FancyRefCountedClass5();
+};

>From 1b2c46b0ff21f3a1d87e1610452b73926dd36fdc Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 17 Sep 2024 19:13:31 -0700
Subject: [PATCH 2/2] Address review comments.

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

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 5ea19233bb65c2..e80246f49a3100 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -72,7 +72,7 @@ class DerefFuncDeleteExprVisitor
       if (name == "ensureOnMainThread" || name == "ensureOnMainRunLoop") {
         for (unsigned i = 0; i < CE->getNumArgs(); ++i) {
           auto *Arg = CE->getArg(i);
-          if (VisitLabmdaArgument(Arg))
+          if (VisitLambdaArgument(Arg))
             return true;
         }
       }
@@ -80,16 +80,14 @@ class DerefFuncDeleteExprVisitor
     return false;
   }
 
-  bool VisitLabmdaArgument(const Expr *E) {
+  bool VisitLambdaArgument(const Expr *E) {
     E = E->IgnoreParenCasts();
     if (auto *TempE = dyn_cast<CXXBindTemporaryExpr>(E))
       E = TempE->getSubExpr();
     E = E->IgnoreParenCasts();
     if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
-      if (auto *Decl = Ref->getDecl()) {
-        if (auto *VD = dyn_cast<VarDecl>(Decl))
-          return VisitLabmdaArgument(VD->getInit());
-      }
+      if (auto *VD = dyn_cast_or_null<VarDecl>(Ref->getDecl()))
+        return VisitLambdaArgument(VD->getInit());
       return false;
     }
     if (auto *Lambda = dyn_cast<LambdaExpr>(E)) {
@@ -98,7 +96,7 @@ class DerefFuncDeleteExprVisitor
     }
     if (auto *ConstructE = dyn_cast<CXXConstructExpr>(E)) {
       for (unsigned i = 0; i < ConstructE->getNumArgs(); ++i) {
-        if (VisitLabmdaArgument(ConstructE->getArg(i)))
+        if (VisitLambdaArgument(ConstructE->getArg(i)))
           return true;
       }
     }



More information about the cfe-commits mailing list