[clang] [WebKit Checkers] Trivial analysis should check destructors of function parameters and local variables (PR #181576)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 15 13:52:18 PST 2026
https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/181576
This PR fixes the bug in TrivialFunctionAnalysisVisitor that it wasn't checking the triviality of destructors of function parameters and local variables. This meant that code calls a non-trivial desturctors such as RefPtr<T>::~RefPtr<T> which calls T::~T to be incorrectly treated as trivial, resulting in false negatives.
To do this, we manually visit every function parameter and local variable declaration and check the triviality of its destructor recursively.
>From 5ad449e38bdf33cb7c159152718e390f933c9e22 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sun, 15 Feb 2026 13:51:31 -0800
Subject: [PATCH] [WebKit Checkers] Trivial analysis should check destructors
of function parameters and local variables
This PR fixes the bug in TrivialFunctionAnalysisVisitor that it wasn't checking the triviality of
destructors of function parameters and local variables. This meant that code calls a non-trivial
desturctors such as RefPtr<T>::~RefPtr<T> which calls T::~T to be incorrectly treated as trivial,
resulting in false negatives.
To do this, we manually visit every function parameter and local variable declaration and check
the triviality of its destructor recursively.
---
.../Checkers/WebKit/NoDeleteChecker.cpp | 13 +++-
.../Checkers/WebKit/PtrTypesSemantics.cpp | 61 ++++++++++++++++++-
.../Checkers/WebKit/PtrTypesSemantics.h | 5 ++
.../Analysis/Checkers/WebKit/mock-types.h | 18 ++++--
.../Checkers/WebKit/nodelete-annotation.cpp | 30 +++++++++
.../Checkers/WebKit/uncounted-local-vars.cpp | 5 ++
6 files changed, 126 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
index 2740890704767..ee163bdd516ef 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
@@ -92,7 +92,18 @@ class NoDeleteChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
return;
auto Body = FD->getBody();
- if (!Body || TFA.isTrivial(Body))
+ if (!Body)
+ return;
+
+ bool ParamHaveTrivialDtors = true;
+ for (auto* Param : FD->parameters()) {
+ if (!TFA.hasTrivialDtor(Param)) {
+ ParamHaveTrivialDtors = false;
+ break;
+ }
+ }
+
+ if (ParamHaveTrivialDtors && TFA.isTrivial(Body))
return;
SmallString<100> Buf;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index c47dabf2ec5b0..bdea5e208a7d1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -527,6 +527,10 @@ class TrivialFunctionAnalysisVisitor
return true;
if (FnDecl->isVirtualAsWritten())
return false;
+ for (auto* Param : FnDecl->parameters()) {
+ if (!HasTrivialDestructor(Param))
+ return false;
+ }
}
return WithCachedResult(D, [&]() {
if (auto *CtorDecl = dyn_cast<CXXConstructorDecl>(D)) {
@@ -541,6 +545,43 @@ class TrivialFunctionAnalysisVisitor
return Visit(Body);
});
}
+
+ bool HasTrivialDestructor(const VarDecl *VD) {
+ auto QT = VD->getType();
+ if (QT.isPODType(VD->getASTContext()))
+ return true;
+ auto *Type = QT.getTypePtrOrNull();
+ if (!Type)
+ return false;
+ const CXXRecordDecl *R = Type->getAsCXXRecordDecl();
+ if (!R) {
+ // Get T out of const T& / T&&.
+ if (auto *RT = dyn_cast<ReferenceType>(Type)) {
+ QT = RT->getPointeeType();
+ Type = QT.getTypePtrOrNull();
+ if (!Type)
+ return false;
+ R = Type->getAsCXXRecordDecl();
+ }
+ }
+ if (!R) {
+ if (auto *AT = dyn_cast<ConstantArrayType>(Type)) {
+ QT = AT->getElementType();
+ Type = QT.getTypePtrOrNull();
+ if (!Type)
+ return false;
+ if (isa<PointerType>(Type)) // An array of pointer does not have a destructor.
+ return true;
+ R = Type->getAsCXXRecordDecl();
+ }
+ }
+ if (!R)
+ return false;
+ auto *Dtor = R->getDestructor();
+ if (!Dtor || Dtor->isTrivial())
+ return true;
+ return IsFunctionTrivial(Dtor);
+ }
bool IsStatementTrivial(const Stmt *S) {
auto CacheIt = Cache.find(S);
@@ -579,7 +620,15 @@ class TrivialFunctionAnalysisVisitor
return true;
}
- bool VisitDeclStmt(const DeclStmt *DS) { return VisitChildren(DS); }
+ bool VisitDeclStmt(const DeclStmt *DS) {
+ for (auto& Decl : DS->decls()) {
+ if (auto *VD = dyn_cast<VarDecl>(Decl)) {
+ if (!HasTrivialDestructor(VD))
+ return false;
+ }
+ }
+ return VisitChildren(DS);
+ }
bool VisitDoStmt(const DoStmt *DS) { return VisitChildren(DS); }
bool VisitIfStmt(const IfStmt *IS) {
return WithCachedResult(IS, [&]() { return VisitChildren(IS); });
@@ -731,6 +780,10 @@ class TrivialFunctionAnalysisVisitor
return true;
}
+ bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr* E) {
+ return Visit(E->getExpr());
+ }
+
bool checkArguments(const CallExpr *CE) {
for (const Expr *Arg : CE->arguments()) {
if (Arg && !Visit(Arg))
@@ -857,4 +910,10 @@ bool TrivialFunctionAnalysis::isTrivialImpl(
return V.IsStatementTrivial(S);
}
+bool TrivialFunctionAnalysis::hasTrivialDtorImpl(
+ const VarDecl *VD, CacheTy &Cache) {
+ TrivialFunctionAnalysisVisitor V(Cache);
+ return V.HasTrivialDestructor(VD);
+}
+
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 431357a2150be..c76f3d47bc08e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -28,6 +28,7 @@ class Stmt;
class TranslationUnitDecl;
class Type;
class TypedefDecl;
+class VarDecl;
// Ref-countability of a type is implicitly defined by Ref<T> and RefPtr<T>
// implementation. It can be modeled as: type T having public methods ref() and
@@ -169,6 +170,9 @@ class TrivialFunctionAnalysis {
/// \returns true if \p D is a "trivial" function.
bool isTrivial(const Decl *D) const { return isTrivialImpl(D, TheCache); }
bool isTrivial(const Stmt *S) const { return isTrivialImpl(S, TheCache); }
+ bool hasTrivialDtor(const VarDecl* VD) const {
+ return hasTrivialDtorImpl(VD, TheCache);
+ }
private:
friend class TrivialFunctionAnalysisVisitor;
@@ -179,6 +183,7 @@ class TrivialFunctionAnalysis {
static bool isTrivialImpl(const Decl *D, CacheTy &Cache);
static bool isTrivialImpl(const Stmt *S, CacheTy &Cache);
+ static bool hasTrivialDtorImpl(const VarDecl *VD, CacheTy &Cache);
};
} // namespace clang
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 8a24a3c64e0e4..99b07691f6e75 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -227,12 +227,19 @@ template <typename T> bool operator!=(const RefPtr<T> &, T &) { return false; }
struct RefCountable {
static Ref<RefCountable> create();
static std::unique_ptr<RefCountable> makeUnique();
- void ref() {}
- void deref() {}
+ void ref() { ++m_refCount; }
+ void deref() {
+ --m_refCount;
+ if (!--m_refCount)
+ delete this;
+ }
void method();
void constMethod() const;
int trivial() { return 123; }
RefCountable* next();
+
+private:
+ unsigned m_refCount { 0 };
};
template <typename T> T *downcast(T *t) { return t; }
@@ -280,11 +287,14 @@ template <typename T> struct CheckedPtr {
class CheckedObj {
public:
- void incrementCheckedPtrCount();
- void decrementCheckedPtrCount();
+ void incrementCheckedPtrCount() { ++m_ptrCount; }
+ void decrementCheckedPtrCount() { --m_ptrCount; }
void method();
int trivial() { return 123; }
CheckedObj* next();
+
+private:
+ unsigned m_ptrCount { 0 };
};
class RefCountableAndCheckable {
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index 82667a7916f42..0f4efadef4e6b 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -1,5 +1,7 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoDeleteChecker -verify %s
+#include "mock-types.h"
+
void someFunction();
void [[clang::annotate_type("webkit.nodelete")]] safeFunction();
@@ -29,6 +31,7 @@ void [[clang::annotate_type("webkit.nodelete")]] defWithNoDelete() {
}
class SomeClass {
+public:
void [[clang::annotate_type("webkit.nodelete")]] someMethod();
void [[clang::annotate_type("webkit.nodelete")]] unsafeMethod() {
// expected-warning at -1{{A function 'unsafeMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
@@ -57,6 +60,33 @@ class SomeClass {
}
virtual void [[clang::annotate_type("webkit.nodelete")]] anotherVirtualMethod();
+
+ void [[clang::annotate_type("webkit.nodelete")]] setObj(RefCountable* obj) {
+ // expected-warning at -1{{A function 'setObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ m_obj = obj;
+ }
+
+ void [[clang::annotate_type("webkit.nodelete")]] swapObj(RefPtr<RefCountable>&& obj) {
+ // expected-warning at -1{{A function 'swapObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ m_obj.swap(obj);
+ }
+
+ void [[clang::annotate_type("webkit.nodelete")]] clearObj(RefCountable* obj) {
+ // expected-warning at -1{{A function 'clearObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ m_obj = nullptr;
+ }
+
+ void [[clang::annotate_type("webkit.nodelete")]] deposeArg(RefPtr<RefCountable>&& unused) {
+ // expected-warning at -1{{A function 'deposeArg' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ }
+
+ void [[clang::annotate_type("webkit.nodelete")]] deposeLocal() {
+ // expected-warning at -1{{A function 'deposeLocal' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ RefPtr<RefCountable> obj = std::move(m_obj);
+ }
+
+private:
+ RefPtr<RefCountable> m_obj;
};
class IntermediateClass : public SomeClass {
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index e8022b7fe8ba0..10aaf041e19dd 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -63,7 +63,12 @@ void foo4() {
void foo5() {
RefPtr<RefCountable> foo;
auto* bar = foo.get();
+ // expected-warning at -1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
bar->trivial();
+ {
+ auto* baz = foo.get();
+ baz->trivial();
+ }
}
void foo6() {
More information about the cfe-commits
mailing list