https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/181576
>From 87c1e4b3f76d940610b984e7ee8bf649127e516c Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Sun, 15 Feb 2026 13:57:17 -0800 Subject: [PATCH 1/5] [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 | 65 +++++++++++++++++- .../Checkers/WebKit/PtrTypesSemantics.h | 5 ++ .../Analysis/Checkers/WebKit/mock-types.h | 18 +++-- .../Checkers/WebKit/nodelete-annotation.cpp | 67 +++++++++++++++++++ .../Checkers/WebKit/uncounted-local-vars.cpp | 5 ++ 6 files changed, 166 insertions(+), 7 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..0d52dd474d47d 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,45 @@ 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) { + if (isa<LValueReferenceType>(Type)) + return true; // T& does not run its destructor. + if (auto *RT = dyn_cast<RValueReferenceType>(Type)) { + // For T&&, we evaluate the destructor of T. + 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 +622,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 +782,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)) @@ -769,7 +824,7 @@ class TrivialFunctionAnalysisVisitor bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE) { if (auto *Temp = BTE->getTemporary()) { - if (!TrivialFunctionAnalysis::isTrivialImpl(Temp->getDestructor(), Cache)) + if (!IsFunctionTrivial(Temp->getDestructor())) return false; } return Visit(BTE->getSubExpr()); @@ -857,4 +912,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..3ed32d8b48caf 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..213414505929f 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(); @@ -28,7 +30,27 @@ void [[clang::annotate_type("webkit.nodelete")]] defWithNoDelete() { someFunction(); } +class WeakRefCountable : public CanMakeWeakPtr<WeakRefCountable> { +public: + static Ref<WeakRefCountable> create(); + + ~WeakRefCountable(); + + void ref() { m_refCount++; } + void deref() { + m_refCount--; + if (!m_refCount) + delete this; + } + +private: + WeakRefCountable(); + + unsigned m_refCount { 0 }; +}; + class SomeClass { +public: void [[clang::annotate_type("webkit.nodelete")]] someMethod(); void [[clang::annotate_type("webkit.nodelete")]] unsafeMethod() { // expected-warning@-1{{A function 'unsafeMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}} @@ -57,6 +79,51 @@ class SomeClass { } virtual void [[clang::annotate_type("webkit.nodelete")]] anotherVirtualMethod(); + + void [[clang::annotate_type("webkit.nodelete")]] setObj(RefCountable* obj) { + // expected-warning@-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@-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@-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(WeakRefCountable&& unused) { + // expected-warning@-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")]] deposeArgPtr(RefPtr<RefCountable>&& unused) { + // expected-warning@-1{{A function 'deposeArgPtr' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}} + } + + void [[clang::annotate_type("webkit.nodelete")]] deposeLocal() { + // expected-warning@-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); + } + + RefPtr<RefCountable> [[clang::annotate_type("webkit.nodelete")]] copyRefPtr() { + return m_obj; + } + + Ref<WeakRefCountable> [[clang::annotate_type("webkit.nodelete")]] copyRef() { + return *m_weakObj.get(); + } + + RefPtr<WeakRefCountable> [[clang::annotate_type("webkit.nodelete")]] getWeakPtr() { + return m_weakObj.get(); + } + +private: + RefPtr<RefCountable> m_obj; + Ref<RefCountable> m_ref; + WeakPtr<WeakRefCountable> m_weakObj; }; 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@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} bar->trivial(); + { + auto* baz = foo.get(); + baz->trivial(); + } } void foo6() { >From bb727810ed8aa0357c15ee3ce27316d84902a9a2 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Sun, 15 Feb 2026 14:08:18 -0800 Subject: [PATCH 2/5] Fix formatting. --- .../Checkers/WebKit/NoDeleteChecker.cpp | 2 +- .../Checkers/WebKit/PtrTypesSemantics.cpp | 18 +++++++++--------- .../Checkers/WebKit/PtrTypesSemantics.h | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp index ee163bdd516ef..c906fbdba1021 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp @@ -96,7 +96,7 @@ class NoDeleteChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { return; bool ParamHaveTrivialDtors = true; - for (auto* Param : FD->parameters()) { + for (auto *Param : FD->parameters()) { if (!TFA.hasTrivialDtor(Param)) { ParamHaveTrivialDtors = false; break; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 0d52dd474d47d..60cb64666c6c5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -527,7 +527,7 @@ class TrivialFunctionAnalysisVisitor return true; if (FnDecl->isVirtualAsWritten()) return false; - for (auto* Param : FnDecl->parameters()) { + for (auto *Param : FnDecl->parameters()) { if (!HasTrivialDestructor(Param)) return false; } @@ -545,7 +545,7 @@ class TrivialFunctionAnalysisVisitor return Visit(Body); }); } - + bool HasTrivialDestructor(const VarDecl *VD) { auto QT = VD->getType(); if (QT.isPODType(VD->getASTContext())) @@ -556,7 +556,7 @@ class TrivialFunctionAnalysisVisitor const CXXRecordDecl *R = Type->getAsCXXRecordDecl(); if (!R) { if (isa<LValueReferenceType>(Type)) - return true; // T& does not run its destructor. + return true; // T& does not run its destructor. if (auto *RT = dyn_cast<RValueReferenceType>(Type)) { // For T&&, we evaluate the destructor of T. QT = RT->getPointeeType(); @@ -572,8 +572,8 @@ class TrivialFunctionAnalysisVisitor Type = QT.getTypePtrOrNull(); if (!Type) return false; - if (isa<PointerType>(Type)) // An array of pointer does not have a destructor. - return true; + if (isa<PointerType>(Type)) + return true; // An array of pointers doesn't run a destructor. R = Type->getAsCXXRecordDecl(); } } @@ -623,7 +623,7 @@ class TrivialFunctionAnalysisVisitor } bool VisitDeclStmt(const DeclStmt *DS) { - for (auto& Decl : DS->decls()) { + for (auto &Decl : DS->decls()) { if (auto *VD = dyn_cast<VarDecl>(Decl)) { if (!HasTrivialDestructor(VD)) return false; @@ -782,7 +782,7 @@ class TrivialFunctionAnalysisVisitor return true; } - bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr* E) { + bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *E) { return Visit(E->getExpr()); } @@ -912,8 +912,8 @@ bool TrivialFunctionAnalysis::isTrivialImpl( return V.IsStatementTrivial(S); } -bool TrivialFunctionAnalysis::hasTrivialDtorImpl( - const VarDecl *VD, CacheTy &Cache) { +bool TrivialFunctionAnalysis::hasTrivialDtorImpl(const VarDecl *VD, + CacheTy &Cache) { TrivialFunctionAnalysisVisitor V(Cache); return V.HasTrivialDestructor(VD); } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index c76f3d47bc08e..8a696a789c65b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -170,7 +170,7 @@ 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 { + bool hasTrivialDtor(const VarDecl *VD) const { return hasTrivialDtorImpl(VD, TheCache); } >From f625740e78451666efca67c11f6c48f0891b4ff2 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Sun, 15 Feb 2026 20:50:24 -0800 Subject: [PATCH 3/5] Fix a bug that we were erroneously flagg r-value reference of an enum value. Also cache the result of checking destructor triviality using WithCachedResult. --- .../Checkers/WebKit/PtrTypesSemantics.cpp | 90 ++++++++++--------- .../Checkers/WebKit/nodelete-annotation.cpp | 6 ++ 2 files changed, 53 insertions(+), 43 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 60cb64666c6c5..b33633bb2c46d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -522,17 +522,17 @@ class TrivialFunctionAnalysisVisitor TrivialFunctionAnalysisVisitor(CacheTy &Cache) : Cache(Cache) {} bool IsFunctionTrivial(const Decl *D) { - if (auto *FnDecl = dyn_cast<FunctionDecl>(D)) { - if (isNoDeleteFunction(FnDecl)) - return true; - if (FnDecl->isVirtualAsWritten()) - return false; - for (auto *Param : FnDecl->parameters()) { - if (!HasTrivialDestructor(Param)) + return WithCachedResult(D, [&]() { + if (auto *FnDecl = dyn_cast<FunctionDecl>(D)) { + if (isNoDeleteFunction(FnDecl)) + 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)) { for (auto *CtorInit : CtorDecl->inits()) { if (!Visit(CtorInit->getInit())) @@ -547,42 +547,46 @@ class TrivialFunctionAnalysisVisitor } 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) { - if (isa<LValueReferenceType>(Type)) - return true; // T& does not run its destructor. - if (auto *RT = dyn_cast<RValueReferenceType>(Type)) { - // For T&&, we evaluate the destructor of T. - QT = RT->getPointeeType(); - Type = QT.getTypePtrOrNull(); - if (!Type) - return false; - R = Type->getAsCXXRecordDecl(); + return WithCachedResult(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) { + if (isa<LValueReferenceType>(Type)) + return true; // T& does not run its destructor. + if (auto *RT = dyn_cast<RValueReferenceType>(Type)) { + // For T&&, we evaluate the destructor of T. + 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)) - return true; // An array of pointers doesn't run a destructor. - 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)) + return true; // An array of pointers doesn't run a destructor. + R = Type->getAsCXXRecordDecl(); + } } - } - if (!R) - return false; - auto *Dtor = R->getDestructor(); - if (!Dtor || Dtor->isTrivial()) - return true; - return IsFunctionTrivial(Dtor); + if (Type->isIntegralOrEnumerationType()) + return true; + if (!R) + return false; + auto *Dtor = R->getDestructor(); + if (!Dtor || Dtor->isTrivial()) + return true; + return IsFunctionTrivial(Dtor); + }); } bool IsStatementTrivial(const Stmt *S) { diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp index 213414505929f..9c2d0dba149f1 100644 --- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp +++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp @@ -103,6 +103,12 @@ class SomeClass { // expected-warning@-1{{A function 'deposeArgPtr' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}} } + enum class E : unsigned char { V1, V2 }; + bool [[clang::annotate_type("webkit.nodelete")]] deposeArgEnum() { + E&& e = E::V1; + return e != E::V2; + } + void [[clang::annotate_type("webkit.nodelete")]] deposeLocal() { // expected-warning@-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); >From 2f6c40dfbffc957e0d17a054a4f719a55d7f5dcf Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Mon, 16 Feb 2026 11:42:51 -0800 Subject: [PATCH 4/5] Allow delete with trivial destructor in trival function analysis. And fix a bunch of bugs in the mock WeakPtr implementation. --- .../Checkers/WebKit/PtrTypesSemantics.cpp | 14 ++++++++++- .../Analysis/Checkers/WebKit/mock-types.h | 24 ++++++++++++------- .../Checkers/WebKit/nodelete-annotation.cpp | 6 +++++ .../Checkers/WebKit/uncounted-local-vars.cpp | 1 + 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index b33633bb2c46d..6cdeaf8d1aa72 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -578,7 +578,7 @@ class TrivialFunctionAnalysisVisitor R = Type->getAsCXXRecordDecl(); } } - if (Type->isIntegralOrEnumerationType()) + if (Type->isIntegralOrEnumerationType() || Type->isNullPtrType()) return true; if (!R) return false; @@ -808,6 +808,18 @@ class TrivialFunctionAnalysisVisitor return IsFunctionTrivial(CE->getConstructor()); } + bool VisitCXXDeleteExpr(const CXXDeleteExpr* DE) { + auto QT = DE->getDestroyedType(); + auto *Type = QT.getTypePtrOrNull(); + if (!Type) + return false; + const CXXRecordDecl *R = Type->getAsCXXRecordDecl(); + if (!R) + return false; + auto *Dtor = R->getDestructor(); + return !Dtor || Dtor->isTrivial(); + } + bool VisitCXXInheritedCtorInitExpr(const CXXInheritedCtorInitExpr *E) { return IsFunctionTrivial(E->getConstructor()); } diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 3ed32d8b48caf..c139a5cb13de7 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -233,6 +233,7 @@ struct RefCountable { if (!--m_refCount) delete this; } + ~RefCountable(); void method(); void constMethod() const; int trivial() { return 123; } @@ -358,8 +359,8 @@ class WeakPtrImpl { private: template <typename T> - WeakPtrImpl(T* t) - : ptr(static_cast<void*>(t)) + WeakPtrImpl(T& t) + : ptr(static_cast<void*>(&t)) { } }; @@ -371,9 +372,9 @@ class CanMakeWeakPtr { template <typename U> friend class CanMakeWeakPtr; template <typename U> friend class WeakPtr; - Ref<WeakPtrImpl> createWeakPtrImpl() { + WeakPtrImpl& createWeakPtrImpl() { if (!impl) - impl = WeakPtrImpl::create(static_cast<T>(*this)); + impl = WeakPtrImpl::create(static_cast<T&>(*this)); return *impl; } @@ -392,21 +393,26 @@ class WeakPtr { RefPtr<WeakPtrImpl> impl; public: - WeakPtr(T& t) { - *this = t; + WeakPtr(T& t) + : impl(t.createWeakPtrImpl()) { } - WeakPtr(T* t) { - *this = t; + WeakPtr(T* t) + : impl(t ? &t->createWeakPtrImpl() : nullptr) { } template <typename U> WeakPtr<T> operator=(U& obj) { impl = obj.createWeakPtrImpl(); + return *this; } template <typename U> WeakPtr<T> operator=(U* obj) { - impl = obj ? obj->createWeakPtrImpl() : nullptr; + if (obj) + impl = obj->createWeakPtrImpl(); + else + impl = nullptr; + return *this; } T* get() { diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp index 9c2d0dba149f1..ecf55696f9f21 100644 --- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp +++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp @@ -51,6 +51,7 @@ class WeakRefCountable : public CanMakeWeakPtr<WeakRefCountable> { class SomeClass { public: + void [[clang::annotate_type("webkit.nodelete")]] someMethod(); void [[clang::annotate_type("webkit.nodelete")]] unsafeMethod() { // expected-warning@-1{{A function 'unsafeMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}} @@ -126,6 +127,11 @@ class SomeClass { return m_weakObj.get(); } + WeakRefCountable* [[clang::annotate_type("webkit.nodelete")]] useWeakPtr() { + WeakPtr localWeak = m_weakObj.get(); + return localWeak.get(); + } + private: RefPtr<RefCountable> m_obj; Ref<RefCountable> m_ref; diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 10aaf041e19dd..ad90198d5ac8b 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -26,6 +26,7 @@ void foo_ref() { void foo_ref_trivial() { RefCountable automatic; RefCountable &bar = automatic; + // expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} } void bar_ref(RefCountable &) {} >From f1a1937b547fd6f8bd03c40997a832657affc4bf Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Mon, 16 Feb 2026 12:52:00 -0800 Subject: [PATCH 5/5] Address review comments by refactoring HasTrivialDestructor. Extract CanTriviallyDestruct which is shared in multiple branches of HasTrivialDestructor and VisitCXXDeleteExpr. --- .../Checkers/WebKit/NoDeleteChecker.cpp | 11 +--- .../Checkers/WebKit/PtrTypesSemantics.cpp | 63 +++++++------------ 2 files changed, 26 insertions(+), 48 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp index c906fbdba1021..abbdc2967e859 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp @@ -95,15 +95,8 @@ class NoDeleteChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { if (!Body) return; - bool ParamHaveTrivialDtors = true; - for (auto *Param : FD->parameters()) { - if (!TFA.hasTrivialDtor(Param)) { - ParamHaveTrivialDtors = false; - break; - } - } - - if (ParamHaveTrivialDtors && TFA.isTrivial(Body)) + auto hasTrivialDtor = [&](VarDecl *D) { return TFA.hasTrivialDtor(D); }; + if (llvm::all_of(FD->parameters(), hasTrivialDtor) && 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 6cdeaf8d1aa72..bb01a67304738 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -516,6 +516,18 @@ class TrivialFunctionAnalysisVisitor return Result; } + bool CanTriviallyDestruct(const Type* T) { + if (T->isIntegralOrEnumerationType()) + return true; + if (isa<PointerType>(T) || T->isNullPtrType()) + return true; + auto *R = T->getAsCXXRecordDecl(); + if (!R) + return false; + auto *Dtor = R->getDestructor(); + return !Dtor || Dtor->isTrivial() || IsFunctionTrivial(Dtor); + } + public: using CacheTy = TrivialFunctionAnalysis::CacheTy; @@ -554,38 +566,18 @@ class TrivialFunctionAnalysisVisitor auto *Type = QT.getTypePtrOrNull(); if (!Type) return false; - const CXXRecordDecl *R = Type->getAsCXXRecordDecl(); - if (!R) { - if (isa<LValueReferenceType>(Type)) - return true; // T& does not run its destructor. - if (auto *RT = dyn_cast<RValueReferenceType>(Type)) { - // For T&&, we evaluate the destructor of T. - QT = RT->getPointeeType(); - Type = QT.getTypePtrOrNull(); - if (!Type) - return false; - R = Type->getAsCXXRecordDecl(); - } + if (isa<LValueReferenceType>(Type)) + return true; // T& does not run its destructor. + if (auto *RT = dyn_cast<RValueReferenceType>(Type)) { + // For T&&, we evaluate the destructor of T. + auto *T = RT->getPointeeType().getTypePtrOrNull(); + return T && CanTriviallyDestruct(T); } - if (!R) { - if (auto *AT = dyn_cast<ConstantArrayType>(Type)) { - QT = AT->getElementType(); - Type = QT.getTypePtrOrNull(); - if (!Type) - return false; - if (isa<PointerType>(Type)) - return true; // An array of pointers doesn't run a destructor. - R = Type->getAsCXXRecordDecl(); - } + if (auto *AT = dyn_cast<ConstantArrayType>(Type)) { + auto *T = AT->getElementType().getTypePtrOrNull(); + return T && CanTriviallyDestruct(T); } - if (Type->isIntegralOrEnumerationType() || Type->isNullPtrType()) - return true; - if (!R) - return false; - auto *Dtor = R->getDestructor(); - if (!Dtor || Dtor->isTrivial()) - return true; - return IsFunctionTrivial(Dtor); + return CanTriviallyDestruct(Type); }); } @@ -809,15 +801,8 @@ class TrivialFunctionAnalysisVisitor } bool VisitCXXDeleteExpr(const CXXDeleteExpr* DE) { - auto QT = DE->getDestroyedType(); - auto *Type = QT.getTypePtrOrNull(); - if (!Type) - return false; - const CXXRecordDecl *R = Type->getAsCXXRecordDecl(); - if (!R) - return false; - auto *Dtor = R->getDestructor(); - return !Dtor || Dtor->isTrivial(); + auto *Type = DE->getDestroyedType().getTypePtrOrNull(); + return Type && CanTriviallyDestruct(Type); } bool VisitCXXInheritedCtorInitExpr(const CXXInheritedCtorInitExpr *E) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
