[clang] [WebKit Checkers] Trivial analysis should check destructors of function parameters and local variables (PR #181576)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 15 13:52:45 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Ryosuke Niwa (rniwa)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/181576.diff
6 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp (+12-1)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+60-1)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+5)
- (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+14-4)
- (modified) clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp (+30)
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp (+5)
``````````diff
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() {
``````````
</details>
https://github.com/llvm/llvm-project/pull/181576
More information about the cfe-commits
mailing list