[clang] [alpha.webkit.UncountedCallArgsChecker] Emit a warning for a WeakPtr argument. (PR #184563)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 4 00:08:02 PST 2026
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/184563
>From 45db3884aa5a83eb63bf7d3a81e2a2c15e04dc9f Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 3 Mar 2026 23:55:08 -0800
Subject: [PATCH 1/2] [alpha.webkit.UncountedCallArgsChecker] Emit a warning
for a WeakPtr argument.
This PR fixes a bug in UncountedCallArgsChecker that it would not emit a warning
when a function is called with a WeakPtr local variable as an argument.
We normally don't generate a warning for a local variable passed to a function
argument in UncountedCallArgsChecker as the variable may have a guardian in
an outer scope but only UncountedLocalVarsChecker is capable of locating one.
So rather than generating a warning in UncountedCallArgsChecker directly,
we rely on UncountedLocalVarsChecker to generate a warning for the local variable.
This all falls apart in the case of a WeakPtr local variable because
a WeakPtr is explicitly allowed as a local variable by UncountedLocalVarsChecker.
So, this PR fixes the bug by detecting this exact scenario (a WeakPtr local
variable used as a function argument), and generate a warning directly in
UncountedCallArgsChecker.
Also added the support for recognizing more weak smart pointer types in WebKit.
---
.../Checkers/WebKit/ASTUtils.cpp | 18 ++++++++++++++----
.../Checkers/WebKit/PtrTypesSemantics.cpp | 17 +++++++++++++++--
.../Checkers/WebKit/PtrTypesSemantics.h | 6 +++++-
.../Analysis/Checkers/WebKit/call-args.cpp | 18 ++++++++++++++++++
.../test/Analysis/Checkers/WebKit/mock-types.h | 3 +++
5 files changed, 55 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 4a9a3be2b6614..f2ee9c742f4b8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -239,10 +239,19 @@ bool tryToFindPtrOrigin(
bool isASafeCallArg(const Expr *E) {
assert(E);
+ auto IsCheckedLocalVarOrParam = [](const VarDecl *Decl) {
+ if (auto *Type = Decl->getType().getTypePtrOrNull()) {
+ if (auto *CXXRD = Type->getAsCXXRecordDecl()) {
+ if (isWeakPtr(CXXRD))
+ return false;
+ }
+ }
+ return Decl->isLocalVarDeclOrParm();
+ };
if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
auto *FoundDecl = Ref->getFoundDecl();
if (auto *D = dyn_cast_or_null<VarDecl>(FoundDecl)) {
- if (isa<ParmVarDecl>(D) || D->isLocalVarDecl())
+ if (IsCheckedLocalVarOrParam(D))
return true;
if (auto *ImplicitP = dyn_cast<ImplicitParamDecl>(D)) {
auto Kind = ImplicitP->getParameterKind();
@@ -253,9 +262,10 @@ bool isASafeCallArg(const Expr *E) {
return true;
}
} else if (auto *BD = dyn_cast_or_null<BindingDecl>(FoundDecl)) {
- VarDecl *VD = BD->getHoldingVar();
- if (VD && (isa<ParmVarDecl>(VD) || VD->isLocalVarDecl()))
- return true;
+ if (VarDecl *VD = BD->getHoldingVar()) {
+ if (IsCheckedLocalVarOrParam(VD))
+ return true;
+ }
}
}
if (isa<CXXTemporaryObjectExpr>(E))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 7a7e22ead74a8..a95bf6faaab52 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -143,11 +143,17 @@ bool isOwnerPtr(const std::string &Name) {
Name == "UniqueRef" || Name == "LazyUniqueRef";
}
+static bool isWeakPtrClass(const std::string &Name) {
+ return Name == "WeakPtr" || Name == "SingleThreadPackedWeakPtr" ||
+ Name == "SingleThreadWeakPtr" || Name == "ThreadSafeWeakPtr" ||
+ Name == "ThreadSafeWeakOrStrongPtr" || Name == "InlineWeakPtr";
+}
+
bool isSmartPtrClass(const std::string &Name) {
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) ||
- Name == "WeakPtr" || Name == "WeakPtrFactory" ||
+ isWeakPtrClass(Name) || Name == "WeakPtrFactory" ||
Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" ||
- Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" ||
+ Name == "WeakPtrImplBaseSingleThread" ||
Name == "ThreadSafeWeakOrStrongPtr" ||
Name == "ThreadSafeWeakPtrControlBlock" ||
Name == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr";
@@ -399,6 +405,13 @@ bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) {
return false;
}
+bool isWeakPtr(const CXXRecordDecl *R) {
+ assert(R);
+ if (auto *TmplR = R->getTemplateInstantiationPattern())
+ return isWeakPtrClass(safeGetName(TmplR));
+ return false;
+}
+
bool isSmartPtr(const CXXRecordDecl *R) {
assert(R);
if (auto *TmplR = R->getTemplateInstantiationPattern())
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index bcb4eee16bd2c..d26a18c456c77 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -60,6 +60,10 @@ bool isCheckedPtr(const clang::CXXRecordDecl *Class);
/// \returns true if \p Class is a RetainPtr, false if not.
bool isRetainPtrOrOSPtr(const clang::CXXRecordDecl *Class);
+/// \returns true if \p Class is a weak smart pointer (WeakPtr, InlineWeakPtr,
+/// etc...), false if not.
+bool isWeakPtr(const clang::CXXRecordDecl *Class);
+
/// \returns true if \p Class is a smart pointer (RefPtr, WeakPtr, etc...),
/// false if not.
bool isSmartPtr(const clang::CXXRecordDecl *Class);
@@ -139,7 +143,7 @@ bool isCheckedPtr(const std::string &Name);
/// \returns true if \p Name is RetainPtr or its variant, false if not.
bool isRetainPtrOrOSPtr(const std::string &Name);
-/// \returns true if \p Name is an owning smar pointer such as Ref, CheckedPtr,
+/// \returns true if \p Name is an owning smart pointer such as Ref, CheckedPtr,
/// and unique_ptr.
bool isOwnerPtr(const std::string &Name);
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
index 1a8bde29080ac..c74e7ab7b9d7e 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -440,3 +440,21 @@ namespace call_with_adopt_ref {
adoptRef(new Obj)->method();
}
}
+
+namespace call_with_weak_ptr {
+
+ class RefCountableWithWeakPtr : public RefCountable, public CanMakeWeakPtr<RefCountableWithWeakPtr> {
+ };
+
+ RefCountableWithWeakPtr* provide();
+ void consume(RefCountableWithWeakPtr*);
+
+ void foo() {
+ WeakPtr weakPtr = provide();
+ consume(weakPtr);
+ // expected-warning at -1{{Call argument is uncounted and unsafe}}
+ weakPtr->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ }
+
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index c139a5cb13de7..a5d9ed5ae174d 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -415,6 +415,9 @@ class WeakPtr {
return *this;
}
+ T* operator->() { return get(); }
+ operator T*() { return get(); }
+
T* get() {
return impl ? impl->get<T>() : nullptr;
}
>From cb64aa72918dc2d7d79de6f1efc1d102c3f49df5 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 4 Mar 2026 00:07:40 -0800
Subject: [PATCH 2/2] Fix formatting.
---
.../lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index f2ee9c742f4b8..ef2ae63ff95fa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -240,13 +240,13 @@ bool tryToFindPtrOrigin(
bool isASafeCallArg(const Expr *E) {
assert(E);
auto IsCheckedLocalVarOrParam = [](const VarDecl *Decl) {
- if (auto *Type = Decl->getType().getTypePtrOrNull()) {
- if (auto *CXXRD = Type->getAsCXXRecordDecl()) {
- if (isWeakPtr(CXXRD))
- return false;
- }
+ if (auto *Type = Decl->getType().getTypePtrOrNull()) {
+ if (auto *CXXRD = Type->getAsCXXRecordDecl()) {
+ if (isWeakPtr(CXXRD))
+ return false;
}
- return Decl->isLocalVarDeclOrParm();
+ }
+ return Decl->isLocalVarDeclOrParm();
};
if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
auto *FoundDecl = Ref->getFoundDecl();
More information about the cfe-commits
mailing list