[clang] [webkit.RefCntblBaseVirtualDtor] Ignore WTF::RefCounted<T> and its variants missing virtual destructor (PR #91009)

[Ryosuke Niwa via cfe-commits]

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

>From 2b5782f9a7f6473174ccefa005268debb79aa744 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 3 May 2024 13:35:29 -0700
Subject: [PATCH 1/2] [webkit.RefCntblBaseVirtualDtor] Ignore
 WTF::RefCounted<T> and its variants missing virtual destructor

---
 .../WebKit/RefCntblBaseVirtualDtorChecker.cpp | 17 ++++++
 .../ref-cntbl-base-virtual-dtor-templates.cpp | 60 +++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index d879c110b75d33..5d3a20315feda1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DiagOutputUtils.h"
+#include "ASTUtils.h"
 #include "PtrTypesSemantics.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -90,6 +91,9 @@ class RefCntblBaseVirtualDtorChecker
           const CXXRecordDecl *C = T->getAsCXXRecordDecl();
           if (!C)
             return false;
+          if (isRefCountedClass(C))
+            return false;
+
           bool AnyInconclusiveBase = false;
           const auto hasPublicRefInBase =
               [&AnyInconclusiveBase](const CXXBaseSpecifier *Base,
@@ -164,6 +168,19 @@ class RefCntblBaseVirtualDtorChecker
     return false;
   }
 
+  static bool isRefCountedClass(const CXXRecordDecl* D) {
+    if (!D->getTemplateInstantiationPattern())
+      return false;
+    auto *NsDecl = D->getParent();
+    if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
+      return false;
+    auto NamespaceName = safeGetName(NsDecl);
+    auto ClsNameStr = safeGetName(D);
+    StringRef ClsName = ClsNameStr; // FIXME: Make safeGetName return StringRef.
+    return NamespaceName == "WTF" && (ClsName.ends_with("RefCounted") ||
+        ClsName == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr");
+  }
+
   void reportBug(const CXXRecordDecl *DerivedClass,
                  const CXXBaseSpecifier *BaseSpec,
                  const CXXRecordDecl *ProblematicBaseClass) const {
diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
index 3338fa9368e4b5..eeb62d5d89ec41 100644
--- a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
@@ -28,3 +28,63 @@ struct DerivedClassTmpl3 : T { };
 
 typedef DerivedClassTmpl3<RefCntblBase> Foo;
 Foo c;
+
+
+namespace WTF {
+
+class RefCountedBase {
+public:
+  void ref() const { ++count; }
+
+protected:
+  bool derefBase() const
+  {
+    return !--count;
+  }
+
+private:
+  mutable unsigned count;
+};
+
+template <typename T>
+class RefCounted : public RefCountedBase {
+public:
+  void deref() const {
+    if (derefBase())
+      delete const_cast<T*>(static_cast<const T*>(this));
+  }
+
+protected:
+  RefCounted() { }
+};
+
+template <typename T>
+class ThreadSafeRefCounted {
+public:
+  void ref() const;
+  bool deref() const;
+};
+
+template <typename T>
+class ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr {
+public:
+  void ref() const;
+  bool deref() const;
+};
+
+} // namespace WTF
+
+class DerivedClass4 : public WTF::RefCounted<DerivedClass4> { };
+
+class DerivedClass5 : public DerivedClass4 { };
+// expected-warning at -1{{Class 'DerivedClass4' is used as a base of class 'DerivedClass5' but doesn't have virtual destructor}}
+
+class DerivedClass6 : public WTF::ThreadSafeRefCounted<DerivedClass6> { };
+
+class DerivedClass7 : public DerivedClass6 { };
+// expected-warning at -1{{Class 'DerivedClass6' is used as a base of class 'DerivedClass7' but doesn't have virtual destructor}}
+
+class DerivedClass8 : public WTF::ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<DerivedClass8> { };
+
+class DerivedClass9 : public DerivedClass8 { };
+// expected-warning at -1{{Class 'DerivedClass8' is used as a base of class 'DerivedClass9' but doesn't have virtual destructor}}

>From a9eb73de2ee7d2eadb742498bc0efb651e0b4d9a Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 3 May 2024 13:46:16 -0700
Subject: [PATCH 2/2] Fix formatting.

---
 .../Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp     | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 5d3a20315feda1..e4d311851da22a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -168,7 +168,7 @@ class RefCntblBaseVirtualDtorChecker
     return false;
   }
 
-  static bool isRefCountedClass(const CXXRecordDecl* D) {
+  static bool isRefCountedClass(const CXXRecordDecl *D) {
     if (!D->getTemplateInstantiationPattern())
       return false;
     auto *NsDecl = D->getParent();
@@ -177,8 +177,9 @@ class RefCntblBaseVirtualDtorChecker
     auto NamespaceName = safeGetName(NsDecl);
     auto ClsNameStr = safeGetName(D);
     StringRef ClsName = ClsNameStr; // FIXME: Make safeGetName return StringRef.
-    return NamespaceName == "WTF" && (ClsName.ends_with("RefCounted") ||
-        ClsName == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr");
+    return NamespaceName == "WTF" &&
+           (ClsName.ends_with("RefCounted") ||
+            ClsName == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr");
   }
 
   void reportBug(const CXXRecordDecl *DerivedClass,