Skip to content

Conversation

@rniwa
Copy link
Contributor

@rniwa rniwa commented Mar 22, 2025

This PR adds the support for WTF::NoVirtualDestructorBase, which signifies to the checker that the class is exempt from having a virtual destructor.

…ase.

This PR adds the support for WTF::NoVirtualDestructorBase, which signifies to the checker that
the class is exempt from having a virtual destructor.
@rniwa rniwa requested a review from t-rasmud March 22, 2025 01:18
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Mar 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 22, 2025

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

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

This PR adds the support for WTF::NoVirtualDestructorBase, which signifies to the checker that the class is exempt from having a virtual destructor.


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

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp (+9-1)
  • (modified) clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp (+17)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 77520f1f731c1..98c587d62978b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -202,6 +202,13 @@ class RefCntblBaseVirtualDtorChecker
           if (!C)
             continue;
 
+          bool isExempt = T.getAsString() == "NoVirtualDestructorBase" &&
+                          safeGetName(C->getParent()) == "WTF";
+          if (isExempt || ExemptDecls.contains(C)) {
+            ExemptDecls.insert(RD);
+            continue;
+          }
+
           if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(C)) {
             for (auto &Arg : CTSD->getTemplateArgs().asArray()) {
               if (Arg.getKind() != TemplateArgument::Type)
@@ -223,12 +230,13 @@ class RefCntblBaseVirtualDtorChecker
 
       llvm::SetVector<const CXXRecordDecl *> Decls;
       llvm::DenseSet<const CXXRecordDecl *> CRTPs;
+      llvm::DenseSet<const CXXRecordDecl *> ExemptDecls;
     };
 
     LocalVisitor visitor(this);
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
     for (auto *RD : visitor.Decls) {
-      if (visitor.CRTPs.contains(RD))
+      if (visitor.CRTPs.contains(RD) || visitor.ExemptDecls.contains(RD))
         continue;
       visitCXXRecordDecl(RD);
     }
diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp
index 5cf7e7614d06e..fd4144d572e01 100644
--- a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp
@@ -1,5 +1,13 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s
 
+namespace WTF {
+
+class NoVirtualDestructorBase { };
+
+};
+
+using WTF::NoVirtualDestructorBase;
+
 struct RefCntblBase {
   void ref() {}
   void deref() {}
@@ -19,6 +27,15 @@ struct [[clang::suppress]] SuppressedDerivedWithVirtualDtor : RefCntblBase {
   virtual ~SuppressedDerivedWithVirtualDtor() {}
 };
 
+class ClassWithoutVirtualDestructor : public NoVirtualDestructorBase {
+public:
+  void ref() const;
+  void deref() const;
+};
+
+class DerivedClassWithoutVirtualDestructor : public ClassWithoutVirtualDestructor {
+};
+
 // FIXME: Support attributes on base specifiers? Currently clang
 // doesn't support such attributes at all, even though it knows
 // how to parse them.

Copy link
Contributor

@t-rasmud t-rasmud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rniwa
Copy link
Contributor Author

rniwa commented Mar 26, 2025

Thank you for the review!

@rniwa rniwa merged commit 2b43ecd into llvm:main Mar 26, 2025
14 checks passed
@rniwa rniwa deleted the add-no-virtual-destructor-base branch March 26, 2025 23:57
rniwa added a commit to rniwa/llvm-project that referenced this pull request Apr 22, 2025
…ase. (llvm#132497)

This PR adds the support for WTF::NoVirtualDestructorBase, which
signifies to the checker that the class is exempt from having a virtual
destructor.
rniwa added a commit to rniwa/llvm-project that referenced this pull request Aug 21, 2025
…ase. (llvm#132497)

This PR adds the support for WTF::NoVirtualDestructorBase, which
signifies to the checker that the class is exempt from having a virtual
destructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants