https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/92501
>From 87cfc8234e1294dedc103b9bcd2b7d9d31874c4a Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@apple.com>
Date: Thu, 16 May 2024 23:24:13 -0700
Subject: [PATCH 1/5] [webkit.RefCntblBaseVirtualDtor] Ignore a base class
which has a specialized deref function for a given derived class.
When a base class B has a deref function which calls delete operator on a
derived class D,
don't emit a warning for B even if it did not have a virtual destructor.
---
.../WebKit/RefCntblBaseVirtualDtorChecker.cpp | 140 ++++++++++++-
.../ref-cntbl-base-virtual-dtor-templates.cpp | 196 +++++++++++++++++-
2 files changed, 330 insertions(+), 6 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 7f4c3a7b787e8..36ab581f1edbd 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -11,16 +11,128 @@
#include "PtrTypesSemantics.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/StmtVisitor.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "llvm/ADT/DenseSet.h"
#include <optional>
using namespace clang;
using namespace ento;
namespace {
+
+class DerefAnalysisVisitor
+ : public ConstStmtVisitor<DerefAnalysisVisitor, bool> {
+ // Returns true if any of child statements return true.
+ bool VisitChildren(const Stmt *S) {
+ for (const Stmt *Child : S->children()) {
+ if (Child && Visit(Child))
+ return true;
+ }
+ return false;
+ }
+
+ bool VisitBody(const Stmt *Body) {
+ if (!Body)
+ return false;
+
+ auto [It, IsNew] = VisitedBody.insert(Body);
+ if (!IsNew) // This body is recursive
+ return false;
+
+ return Visit(Body);
+ }
+
+public:
+ DerefAnalysisVisitor(const TemplateArgumentList &ArgList,
+ const CXXRecordDecl *ClassDecl)
+ : ArgList(&ArgList)
+ , ClassDecl(ClassDecl)
+ { }
+
+ DerefAnalysisVisitor(const CXXRecordDecl *ClassDecl)
+ : ClassDecl(ClassDecl)
+ { }
+
+ bool HasSpecializedDelete(CXXMethodDecl *Decl) {
+ if (auto *Tmpl = Decl->getTemplateInstantiationPattern())
+ return VisitBody(Tmpl->getBody());
+ return VisitBody(Decl->getBody());
+ }
+
+ bool VisitCallExpr(const CallExpr *CE) {
+ auto *Callee = CE->getCallee();
+ while (auto *Expr = dyn_cast<CastExpr>(Callee))
+ Callee = Expr->getSubExpr();
+ if (auto *DeclRef = dyn_cast<DeclRefExpr>(Callee)) {
+ auto *Decl = DeclRef->getDecl();
+ if (auto *VD = dyn_cast<VarDecl>(Decl)) {
+ if (auto *Init = VD->getInit()) {
+ if (auto *Lambda = dyn_cast<LambdaExpr>(Init))
+ return VisitBody(Lambda->getBody());
+ }
+ } else if (auto *FD = dyn_cast<FunctionDecl>(Decl))
+ return VisitBody(FD->getBody());
+ }
+ return false;
+ }
+
+ bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
+ auto *Callee = MCE->getMethodDecl();
+ if (!Callee)
+ return false;
+ return VisitBody(Callee->getBody());
+ }
+
+ bool VisitCXXDeleteExpr(const CXXDeleteExpr *E) {
+ auto *Arg = E->getArgument();
+ while (Arg) {
+ if (auto *Paren = dyn_cast<ParenExpr>(Arg))
+ Arg = Paren->getSubExpr();
+ else if (auto *Cast = dyn_cast<ExplicitCastExpr>(Arg)) {
+ Arg = Cast->getSubExpr();
+ auto CastType = Cast->getType();
+ if (auto *PtrType = dyn_cast<PointerType>(CastType)) {
+ auto PointeeType = PtrType->getPointeeType();
+ while (auto *ET = dyn_cast<ElaboratedType>(PointeeType)) {
+ if (ET->isSugared())
+ PointeeType = ET->desugar();
+ }
+ if (auto *ParmType = dyn_cast<TemplateTypeParmType>(PointeeType)) {
+ if (ArgList) {
+ auto ParmIndex = ParmType->getIndex();
+ auto Type = ArgList->get(ParmIndex).getAsType();
+ if (auto *RD = dyn_cast<RecordType>(Type)) {
+ if (RD->getDecl() == ClassDecl)
+ return true;
+ }
+ }
+ } else if (auto *RD = dyn_cast<RecordType>(PointeeType)) {
+ if (RD->getDecl() == ClassDecl)
+ return true;
+ }
+ }
+ } else
+ break;
+ }
+ return false;
+ }
+
+ bool VisitStmt(const Stmt *S) { return VisitChildren(S); }
+
+ // Return false since the contents of lambda isn't necessarily executed.
+ // If it is executed, VisitCallExpr above will visit its body.
+ bool VisitLambdaExpr(const LambdaExpr *) { return false; }
+
+private:
+ const TemplateArgumentList* ArgList { nullptr };
+ const CXXRecordDecl* ClassDecl;
+ llvm::DenseSet<const Stmt *> VisitedBody;
+};
+
class RefCntblBaseVirtualDtorChecker
: public Checker<check::ASTDecl<TranslationUnitDecl>> {
private:
@@ -91,8 +203,6 @@ class RefCntblBaseVirtualDtorChecker
const CXXRecordDecl *C = T->getAsCXXRecordDecl();
if (!C)
return false;
- if (isRefCountedClass(C))
- return false;
bool AnyInconclusiveBase = false;
const auto hasPublicRefInBase =
@@ -124,6 +234,9 @@ class RefCntblBaseVirtualDtorChecker
if (AnyInconclusiveBase || !hasRef || !hasDeref)
return false;
+ if (isClassWithSpecializedDelete(C, RD))
+ return false;
+
const auto *Dtor = C->getDestructor();
if (!Dtor || !Dtor->isVirtual()) {
ProblematicBaseSpecifier = Base;
@@ -182,6 +295,29 @@ class RefCntblBaseVirtualDtorChecker
ClsName == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr");
}
+ static bool isClassWithSpecializedDelete(const CXXRecordDecl *C,
+ const CXXRecordDecl *DerivedClass) {
+ if (auto *ClsTmplSpDecl = dyn_cast<ClassTemplateSpecializationDecl>(C)) {
+ for (auto *MethodDecl : C->methods()) {
+ if (safeGetName(MethodDecl) == "deref") {
+ DerefAnalysisVisitor DerefAnalysis(ClsTmplSpDecl->getTemplateArgs(),
+ DerivedClass);
+ if (DerefAnalysis.HasSpecializedDelete(MethodDecl))
+ return true;
+ }
+ }
+ return false;
+ }
+ for (auto *MethodDecl : C->methods()) {
+ if (safeGetName(MethodDecl) == "deref") {
+ DerefAnalysisVisitor DerefAnalysis(DerivedClass);
+ if (DerefAnalysis.HasSpecializedDelete(MethodDecl))
+ return true;
+ }
+ }
+ return false;
+ }
+
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 eeb62d5d89ec4..37f5d46110896 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
@@ -58,24 +58,101 @@ class RefCounted : public RefCountedBase {
RefCounted() { }
};
+template <typename X, typename T>
+class ExoticRefCounted : RefCountedBase {
+public:
+ void deref() const {
+ if (derefBase())
+ delete (const_cast<T*>(static_cast<const T*>(this)));
+ }
+};
+
+template <typename X, typename T>
+class BadBase : RefCountedBase {
+public:
+ void deref() const {
+ if (derefBase())
+ delete (const_cast<X*>(static_cast<const X*>(this)));
+ }
+};
+
+template <typename T>
+class FancyDeref {
+public:
+ void ref() const
+ {
+ ++refCount;
+ }
+
+ void deref() const
+ {
+ --refCount;
+ if (refCount)
+ return;
+ auto deleteThis = [this] {
+ delete static_cast<const T*>(this);
+ };
+ deleteThis();
+ }
+private:
+ mutable unsigned refCount { 0 };
+};
+
+template <typename T>
+class BadFancyDeref {
+public:
+ void ref() const
+ {
+ ++refCount;
+ }
+
+ void deref() const
+ {
+ --refCount;
+ if (refCount)
+ return;
+ auto deleteThis = [this] {
+ delete static_cast<const T*>(this);
+ };
+ delete this;
+ }
+private:
+ mutable unsigned refCount { 0 };
+};
+
template <typename T>
class ThreadSafeRefCounted {
public:
- void ref() const;
- bool deref() const;
+ void ref() const { ++refCount; }
+ void deref() const {
+ if (!--refCount)
+ delete const_cast<T*>(static_cast<const T*>(this));
+ }
+private:
+ mutable unsigned refCount { 0 };
};
template <typename T>
class ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr {
public:
- void ref() const;
- bool deref() const;
+ void ref() const { ++refCount; }
+ void deref() const {
+ if (!--refCount)
+ delete const_cast<T*>(static_cast<const T*>(this));
+ }
+private:
+ mutable unsigned refCount { 0 };
};
} // namespace WTF
class DerivedClass4 : public WTF::RefCounted<DerivedClass4> { };
+class DerivedClass4b : public WTF::ExoticRefCounted<int, DerivedClass4b> { };
+
+class DerivedClass4c : public WTF::BadBase<int, DerivedClass4c> { };
+// expected-warning@-1{{Class 'WTF::BadBase<int, DerivedClass4c>' is used as a
base of class 'DerivedClass4c' but doesn't have virtual destructor}}
+
class DerivedClass5 : public DerivedClass4 { };
// expected-warning@-1{{Class 'DerivedClass4' is used as a base of class
'DerivedClass5' but doesn't have virtual destructor}}
@@ -88,3 +165,114 @@ class DerivedClass8 : public
WTF::ThreadSafeRefCountedAndCanMakeThreadSafeWeakPt
class DerivedClass9 : public DerivedClass8 { };
// expected-warning@-1{{Class 'DerivedClass8' is used as a base of class
'DerivedClass9' but doesn't have virtual destructor}}
+
+class DerivedClass10 : public WTF::FancyDeref<DerivedClass10> { };
+
+class DerivedClass10b : public WTF::BadFancyDeref<DerivedClass10b> { };
+// expected-warning@-1{{Class 'WTF::BadFancyDeref<DerivedClass10b>' is used as
a base of class 'DerivedClass10b' but doesn't have virtual destructor}}
+
+class BaseClass1 {
+public:
+ void ref() const { ++refCount; }
+ void deref() const;
+private:
+ enum class Type { Base, Derived } type { Type::Base };
+ mutable unsigned refCount { 0 };
+};
+
+class DerivedClass11 : public BaseClass1 { };
+
+void BaseClass1::deref() const
+{
+ --refCount;
+ if (refCount)
+ return;
+ switch (type) {
+ case Type::Base:
+ delete const_cast<BaseClass1*>(this);
+ break;
+ case Type::Derived:
+ delete const_cast<DerivedClass11*>(static_cast<const
DerivedClass11*>(this));
+ break;
+ }
+}
+
+class BaseClass2;
+static void deleteBase2(BaseClass2*);
+
+class BaseClass2 {
+public:
+ void ref() const { ++refCount; }
+ void deref() const
+ {
+ if (!--refCount)
+ deleteBase2(const_cast<BaseClass2*>(this));
+ }
+ virtual bool isDerived() { return false; }
+private:
+ mutable unsigned refCount { 0 };
+};
+
+class DerivedClass12 : public BaseClass2 {
+ bool isDerived() final { return true; }
+};
+
+void deleteBase2(BaseClass2* obj) {
+ if (obj->isDerived())
+ delete static_cast<DerivedClass12*>(obj);
+ else
+ delete obj;
+}
+
+class BaseClass3 {
+public:
+ void ref() const { ++refCount; }
+ void deref() const
+ {
+ if (!--refCount)
+ const_cast<BaseClass3*>(this)->destory();
+ }
+ virtual bool isDerived() { return false; }
+
+private:
+ void destory();
+
+ mutable unsigned refCount { 0 };
+};
+
+class DerivedClass13 : public BaseClass3 {
+ bool isDerived() final { return true; }
+};
+
+void BaseClass3::destory() {
+ if (isDerived())
+ delete static_cast<DerivedClass13*>(this);
+ else
+ delete this;
+}
+
+class RecursiveBaseClass {
+public:
+ void ref() const {
+ if (otherObject)
+ otherObject->ref();
+ else
+ ++refCount;
+ }
+ void deref() const {
+ if (otherObject)
+ otherObject->deref();
+ else {
+ --refCount;
+ if (refCount)
+ return;
+ delete this;
+ }
+ }
+private:
+ RecursiveBaseClass* otherObject { nullptr };
+ mutable unsigned refCount { 0 };
+};
+
+class RecursiveDerivedClass : public RecursiveBaseClass { };
+// expected-warning@-1{{Class 'RecursiveBaseClass' is used as a base of class
'RecursiveDerivedClass' but doesn't have virtual destructor}}
>From c538cd4b291e550b830c2c450e095638f6785452 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@apple.com>
Date: Thu, 16 May 2024 23:37:40 -0700
Subject: [PATCH 2/5] Fix formatting.
---
.../WebKit/RefCntblBaseVirtualDtorChecker.cpp | 22 ++++++++-----------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 36ab581f1edbd..92c1c6ea75263 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -49,13 +49,9 @@ class DerefAnalysisVisitor
public:
DerefAnalysisVisitor(const TemplateArgumentList &ArgList,
const CXXRecordDecl *ClassDecl)
- : ArgList(&ArgList)
- , ClassDecl(ClassDecl)
- { }
+ : ArgList(&ArgList) , ClassDecl(ClassDecl) {}
- DerefAnalysisVisitor(const CXXRecordDecl *ClassDecl)
- : ClassDecl(ClassDecl)
- { }
+ DerefAnalysisVisitor(const CXXRecordDecl *ClassDecl) : ClassDecl(ClassDecl)
{}
bool HasSpecializedDelete(CXXMethodDecl *Decl) {
if (auto *Tmpl = Decl->getTemplateInstantiationPattern())
@@ -128,8 +124,8 @@ class DerefAnalysisVisitor
bool VisitLambdaExpr(const LambdaExpr *) { return false; }
private:
- const TemplateArgumentList* ArgList { nullptr };
- const CXXRecordDecl* ClassDecl;
+ const TemplateArgumentList *ArgList{nullptr};
+ const CXXRecordDecl *ClassDecl;
llvm::DenseSet<const Stmt *> VisitedBody;
};
@@ -309,11 +305,11 @@ class RefCntblBaseVirtualDtorChecker
return false;
}
for (auto *MethodDecl : C->methods()) {
- if (safeGetName(MethodDecl) == "deref") {
- DerefAnalysisVisitor DerefAnalysis(DerivedClass);
- if (DerefAnalysis.HasSpecializedDelete(MethodDecl))
- return true;
- }
+ if (safeGetName(MethodDecl) == "deref") {
+ DerefAnalysisVisitor DerefAnalysis(DerivedClass);
+ if (DerefAnalysis.HasSpecializedDelete(MethodDecl))
+ return true;
+ }
}
return false;
}
>From 97dfd40cf89901411ea6f21a0f9b7492f184ac39 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@apple.com>
Date: Thu, 16 May 2024 23:50:58 -0700
Subject: [PATCH 3/5] Fix formatting 2.
---
.../Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 92c1c6ea75263..17e5514850b21 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -49,7 +49,7 @@ class DerefAnalysisVisitor
public:
DerefAnalysisVisitor(const TemplateArgumentList &ArgList,
const CXXRecordDecl *ClassDecl)
- : ArgList(&ArgList) , ClassDecl(ClassDecl) {}
+ : ArgList(&ArgList), ClassDecl(ClassDecl) {}
DerefAnalysisVisitor(const CXXRecordDecl *ClassDecl) : ClassDecl(ClassDecl)
{}
>From 3e4d9495a1e52c04620036da026727655c65c86f Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@apple.com>
Date: Fri, 17 May 2024 01:54:21 -0700
Subject: [PATCH 4/5] [webkit.RefCntblBaseVirtualDtor] Only check immediate
base class
This PR changes RefCntblBaseVirtualDtor checker so that the checker will only
check
if a given class's immediate ref-counted base class has a virtual destructor,
not every ref-counted ancestor class.
This is sufficient because the checker will eventually see every class
declaration,
and transitively proves every ref-counted base class has a virtual destructor or
deref function which casts this pointer back to the derived class before
deleting.
---
.../WebKit/RefCntblBaseVirtualDtorChecker.cpp | 119 ++++++++----------
.../ref-cntbl-base-virtual-dtor-templates.cpp | 6 +
2 files changed, 58 insertions(+), 67 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 17e5514850b21..452d2b4a14a7e 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -172,80 +172,65 @@ class RefCntblBaseVirtualDtorChecker
if (shouldSkipDecl(RD))
return;
- CXXBasePaths Paths;
- Paths.setOrigin(RD);
-
- const CXXBaseSpecifier *ProblematicBaseSpecifier = nullptr;
- const CXXRecordDecl *ProblematicBaseClass = nullptr;
-
- const auto IsPublicBaseRefCntblWOVirtualDtor =
- [RD, &ProblematicBaseSpecifier,
- &ProblematicBaseClass](const CXXBaseSpecifier *Base, CXXBasePath &) {
- const auto AccSpec = Base->getAccessSpecifier();
- if (AccSpec == AS_protected || AccSpec == AS_private ||
- (AccSpec == AS_none && RD->isClass()))
- return false;
-
- auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
- auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
-
- bool hasRef = hasRefInBase && *hasRefInBase != nullptr;
- bool hasDeref = hasDerefInBase && *hasDerefInBase != nullptr;
-
- QualType T = Base->getType();
- if (T.isNull())
- return false;
-
- const CXXRecordDecl *C = T->getAsCXXRecordDecl();
- if (!C)
- return false;
-
- bool AnyInconclusiveBase = false;
- const auto hasPublicRefInBase =
- [&AnyInconclusiveBase](const CXXBaseSpecifier *Base,
- CXXBasePath &) {
- auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
- if (!hasRefInBase) {
- AnyInconclusiveBase = true;
- return false;
- }
- return (*hasRefInBase) != nullptr;
- };
- const auto hasPublicDerefInBase = [&AnyInconclusiveBase](
- const CXXBaseSpecifier *Base,
- CXXBasePath &) {
- auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
- if (!hasDerefInBase) {
+ for (auto& Base : RD->bases()) {
+ const auto AccSpec = Base.getAccessSpecifier();
+ if (AccSpec == AS_protected || AccSpec == AS_private ||
+ (AccSpec == AS_none && RD->isClass()))
+ continue;
+
+ auto hasRefInBase = clang::hasPublicMethodInBase(&Base, "ref");
+ auto hasDerefInBase = clang::hasPublicMethodInBase(&Base, "deref");
+
+ bool hasRef = hasRefInBase && *hasRefInBase != nullptr;
+ bool hasDeref = hasDerefInBase && *hasDerefInBase != nullptr;
+
+ QualType T = Base.getType();
+ if (T.isNull())
+ continue;
+
+ const CXXRecordDecl *C = T->getAsCXXRecordDecl();
+ if (!C)
+ continue;
+
+ bool AnyInconclusiveBase = false;
+ const auto hasPublicRefInBase =
+ [&AnyInconclusiveBase](const CXXBaseSpecifier *Base,
+ CXXBasePath &) {
+ auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
+ if (!hasRefInBase) {
AnyInconclusiveBase = true;
return false;
}
- return (*hasDerefInBase) != nullptr;
+ return (*hasRefInBase) != nullptr;
};
- CXXBasePaths Paths;
- Paths.setOrigin(C);
- hasRef = hasRef || C->lookupInBases(hasPublicRefInBase, Paths,
+ const auto hasPublicDerefInBase = [&AnyInconclusiveBase](
+ const CXXBaseSpecifier *Base,
+ CXXBasePath &) {
+ auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
+ if (!hasDerefInBase) {
+ AnyInconclusiveBase = true;
+ return false;
+ }
+ return (*hasDerefInBase) != nullptr;
+ };
+ CXXBasePaths Paths;
+ Paths.setOrigin(C);
+ hasRef = hasRef || C->lookupInBases(hasPublicRefInBase, Paths,
+ /*LookupInDependent =*/true);
+ hasDeref = hasDeref || C->lookupInBases(hasPublicDerefInBase, Paths,
/*LookupInDependent =*/true);
- hasDeref = hasDeref || C->lookupInBases(hasPublicDerefInBase, Paths,
- /*LookupInDependent =*/true);
- if (AnyInconclusiveBase || !hasRef || !hasDeref)
- return false;
-
- if (isClassWithSpecializedDelete(C, RD))
- return false;
-
- const auto *Dtor = C->getDestructor();
- if (!Dtor || !Dtor->isVirtual()) {
- ProblematicBaseSpecifier = Base;
- ProblematicBaseClass = C;
- return true;
- }
+ if (AnyInconclusiveBase || !hasRef || !hasDeref)
+ continue;
- return false;
- };
+ if (isClassWithSpecializedDelete(C, RD))
+ continue;
- if (RD->lookupInBases(IsPublicBaseRefCntblWOVirtualDtor, Paths,
- /*LookupInDependent =*/true)) {
- reportBug(RD, ProblematicBaseSpecifier, ProblematicBaseClass);
+ const auto *Dtor = C->getDestructor();
+ if (!Dtor || !Dtor->isVirtual()) {
+ auto* ProblematicBaseSpecifier = &Base;
+ auto* ProblematicBaseClass = C;
+ reportBug(RD, ProblematicBaseSpecifier, ProblematicBaseClass);
+ }
}
}
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 37f5d46110896..c42d8520b5f05 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
@@ -276,3 +276,9 @@ class RecursiveBaseClass {
class RecursiveDerivedClass : public RecursiveBaseClass { };
// expected-warning@-1{{Class 'RecursiveBaseClass' is used as a base of class
'RecursiveDerivedClass' but doesn't have virtual destructor}}
+
+class DerivedClass14 : public WTF::RefCounted<DerivedClass14> {
+public:
+ virtual ~DerivedClass14() { }
+};
+class DerivedClass15 : public DerivedClass14 { };
>From cb956371721c2b573363fb2de00d59535877ca4f Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@apple.com>
Date: Fri, 17 May 2024 02:06:32 -0700
Subject: [PATCH 5/5] Fix formatting.
---
.../WebKit/RefCntblBaseVirtualDtorChecker.cpp | 28 +++++++++----------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 452d2b4a14a7e..e851d1f5d5df9 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -172,7 +172,7 @@ class RefCntblBaseVirtualDtorChecker
if (shouldSkipDecl(RD))
return;
- for (auto& Base : RD->bases()) {
+ for (auto &Base : RD->bases()) {
const auto AccSpec = Base.getAccessSpecifier();
if (AccSpec == AS_protected || AccSpec == AS_private ||
(AccSpec == AS_none && RD->isClass()))
@@ -194,8 +194,7 @@ class RefCntblBaseVirtualDtorChecker
bool AnyInconclusiveBase = false;
const auto hasPublicRefInBase =
- [&AnyInconclusiveBase](const CXXBaseSpecifier *Base,
- CXXBasePath &) {
+ [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
if (!hasRefInBase) {
AnyInconclusiveBase = true;
@@ -203,16 +202,15 @@ class RefCntblBaseVirtualDtorChecker
}
return (*hasRefInBase) != nullptr;
};
- const auto hasPublicDerefInBase = [&AnyInconclusiveBase](
- const CXXBaseSpecifier *Base,
- CXXBasePath &) {
- auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
- if (!hasDerefInBase) {
- AnyInconclusiveBase = true;
- return false;
- }
- return (*hasDerefInBase) != nullptr;
- };
+ const auto hasPublicDerefInBase =
+ [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
+ auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
+ if (!hasDerefInBase) {
+ AnyInconclusiveBase = true;
+ return false;
+ }
+ return (*hasDerefInBase) != nullptr;
+ };
CXXBasePaths Paths;
Paths.setOrigin(C);
hasRef = hasRef || C->lookupInBases(hasPublicRefInBase, Paths,
@@ -227,8 +225,8 @@ class RefCntblBaseVirtualDtorChecker
const auto *Dtor = C->getDestructor();
if (!Dtor || !Dtor->isVirtual()) {
- auto* ProblematicBaseSpecifier = &Base;
- auto* ProblematicBaseClass = C;
+ auto *ProblematicBaseSpecifier = &Base;
+ auto *ProblematicBaseClass = C;
reportBug(RD, ProblematicBaseSpecifier, ProblematicBaseClass);
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
