[clang] [alpha.webkit.ForwardDeclChecker] Ignore unary operator when detecting a parameter (PR #160994)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Sat Sep 27 10:58:31 PDT 2025
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/160994
>From bc6988f7e9dbd4880d2d3ed530847c74bf5d92af Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 27 Sep 2025 01:55:50 -0700
Subject: [PATCH 1/3] [alpha.webkit.ForwardDeclChecker] Ignore unary operator
when detecting a parameter
This PR updates the forward declaration checker so that unary operator & and * will
be ignored for the purpose of determining if a given function argument is also a
function argument of the caller / call-site.
---
.../Checkers/WebKit/ForwardDeclChecker.cpp | 26 ++++++++++++++-----
.../Checkers/WebKit/forward-decl-checker.mm | 8 ++++++
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp
index d8539eaaac49f..d8a796c7847d4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp
@@ -263,18 +263,30 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
void visitCallArg(const Expr *Arg, const ParmVarDecl *Param,
const Decl *DeclWithIssue) const {
auto *ArgExpr = Arg->IgnoreParenCasts();
- if (auto *InnerCE = dyn_cast<CallExpr>(Arg)) {
- auto *InnerCallee = InnerCE->getDirectCallee();
- if (InnerCallee && InnerCallee->isInStdNamespace() &&
- safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) {
- ArgExpr = InnerCE->getArg(0);
- if (ArgExpr)
- ArgExpr = ArgExpr->IgnoreParenCasts();
+ while (ArgExpr) {
+ ArgExpr = ArgExpr->IgnoreParenCasts();
+ if (auto *InnerCE = dyn_cast<CallExpr>(ArgExpr)) {
+ auto *InnerCallee = InnerCE->getDirectCallee();
+ if (InnerCallee && InnerCallee->isInStdNamespace() &&
+ safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) {
+ ArgExpr = InnerCE->getArg(0);
+ continue;
+ }
+ }
+ if (auto *UO = dyn_cast<UnaryOperator>(ArgExpr)) {
+ auto OpCode = UO->getOpcode();
+ if (OpCode == UO_Deref || OpCode == UO_AddrOf) {
+ ArgExpr = UO->getSubExpr();
+ continue;
+ }
}
+ break;
}
+
if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
isa<CXXDefaultArgExpr>(ArgExpr))
return;
+
if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) {
if (auto *ValDecl = DRE->getDecl()) {
if (isa<ParmVarDecl>(ValDecl))
diff --git a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
index 104b555c1c41d..50411ea9026b5 100644
--- a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
+++ b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
@@ -11,6 +11,8 @@
Obj* provide_obj_ptr();
void receive_obj_ptr(Obj* p = nullptr);
+void receive_obj_ref(Obj&);
+void receive_obj_rref(Obj&&);
sqlite3* open_db();
void close_db(sqlite3*);
@@ -38,6 +40,12 @@
return obj;
}
+void opaque_call_arg(Obj* obj, Obj&& otherObj) {
+ receive_obj_ref(*obj);
+ receive_obj_ptr(&*obj);
+ receive_obj_rref(std::move(otherObj));
+}
+
Obj&& provide_obj_rval();
void receive_obj_rval(Obj&& p);
>From 74b16c1fa2680c33e549bcaa4838c643da1b49f0 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 27 Sep 2025 03:25:22 -0700
Subject: [PATCH 2/3] Fix the checker for WeakPtr and unique_ptr
---
.../Checkers/WebKit/ForwardDeclChecker.cpp | 13 +++
.../Checkers/WebKit/PtrTypesSemantics.cpp | 10 ++-
.../Checkers/WebKit/PtrTypesSemantics.h | 4 +
.../Checkers/WebKit/forward-decl-checker.mm | 6 +-
.../Analysis/Checkers/WebKit/mock-types.h | 84 +++++++++++++++++--
5 files changed, 107 insertions(+), 10 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp
index d8a796c7847d4..1d4e6dd572749 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp
@@ -283,6 +283,19 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
break;
}
+ if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(ArgExpr)) {
+ if (isOwnerPtrType(MemberCallExpr->getObjectType()))
+ return;
+ }
+
+ if (auto *OpCE = dyn_cast<CXXOperatorCallExpr>(ArgExpr)) {
+ auto *Method = dyn_cast_or_null<CXXMethodDecl>(OpCE->getDirectCallee());
+ if (Method && isOwnerPtr(safeGetName(Method->getParent()))) {
+ if (OpCE->getOperator() == OO_Star && OpCE->getNumArgs() == 1)
+ return;
+ }
+ }
+
if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
isa<CXXDefaultArgExpr>(ArgExpr))
return;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index e5c74bbaf3d6b..17aed16ee54cf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -138,6 +138,11 @@ bool isCheckedPtr(const std::string &Name) {
return Name == "CheckedPtr" || Name == "CheckedRef";
}
+bool isOwnerPtr(const std::string &Name) {
+ return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
+ Name == "UniqueRef" || Name == "LazyUniqueRef";
+}
+
bool isSmartPtrClass(const std::string &Name) {
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) ||
Name == "WeakPtr" || Name == "WeakPtrFactory" ||
@@ -206,10 +211,7 @@ bool isRetainPtrOrOSPtrType(const clang::QualType T) {
}
bool isOwnerPtrType(const clang::QualType T) {
- return isPtrOfType(T, [](auto Name) {
- return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
- Name == "UniqueRef" || Name == "LazyUniqueRef";
- });
+ return isPtrOfType(T, [](auto Name) { return isOwnerPtr(Name); });
}
std::optional<bool> isUncounted(const QualType T) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 8300a6c051f3e..12e2e2d75b75d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -143,6 +143,10 @@ 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,
+/// and unique_ptr.
+bool isOwnerPtr(const std::string &Name);
+
/// \returns true if \p Name is a smart pointer type name, false if not.
bool isSmartPtrClass(const std::string &Name);
diff --git a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
index 50411ea9026b5..15cccdd550b7f 100644
--- a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
+++ b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
@@ -40,10 +40,14 @@
return obj;
}
-void opaque_call_arg(Obj* obj, Obj&& otherObj) {
+void opaque_call_arg(Obj* obj, Obj&& otherObj, const RefPtr<Obj>& safeObj, WeakPtr<Obj> weakObj, std::unique_ptr<Obj> uniqObj) {
receive_obj_ref(*obj);
receive_obj_ptr(&*obj);
receive_obj_rref(std::move(otherObj));
+ receive_obj_ref(*safeObj.get());
+ receive_obj_ptr(weakObj.get());
+ // expected-warning at -1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}}
+ receive_obj_ref(*uniqObj);
}
Obj&& provide_obj_rval();
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index a49faa1d25336..d110cd11142dd 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -25,23 +25,23 @@ namespace std {
template <typename T>
class unique_ptr {
private:
- T *t;
+ void *t;
public:
unique_ptr() : t(nullptr) { }
unique_ptr(T *t) : t(t) { }
~unique_ptr() {
if (t)
- delete t;
+ delete static_cast<T*>(t);
}
template <typename U> unique_ptr(unique_ptr<U>&& u)
: t(u.t)
{
u.t = nullptr;
}
- T *get() const { return t; }
- T *operator->() const { return t; }
- T &operator*() const { return *t; }
+ T *get() const { return static_cast<T*>(t); }
+ T *operator->() const { return get(); }
+ T &operator*() const { return *get(); }
unique_ptr &operator=(T *) { return *this; }
explicit operator bool() const { return !!t; }
};
@@ -313,4 +313,78 @@ class UniqueRef {
UniqueRef &operator=(T &) { return *this; }
};
+template <typename T>
+class WeakPtrImpl {
+private:
+ void* ptr;
+
+ template <typename U> friend class CanMakeWeakPtr;
+ template <typename U> friend class WeakPtr;
+
+ Ref<WeakPtrImpl<T>> create(T& t)
+ {
+ return adoptNS(new WeakPtrImpl<T>(t));
+ }
+
+ T* get() { return static_cast<T*>(ptr); }
+ operator bool() const { return !!ptr; }
+ void clear() { ptr = nullptr; }
+
+ WeakPtrImpl(T& t)
+ : ptr(static_cast<void*>(t))
+ { }
+};
+
+template <typename T>
+class CanMakeWeakPtr {
+private:
+ RefPtr<WeakPtrImpl<T>> impl;
+
+ template <typename U> friend class CanMakeWeakPtr;
+ template <typename U> friend class WeakPtr;
+
+ Ref<WeakPtrImpl<T>> createWeakPtrImpl() {
+ if (!impl)
+ impl = WeakPtrImpl<T>::create(static_cast<T>(*this));
+ return *impl;
+ }
+
+public:
+ ~CanMakeWeakPtr() {
+ if (!impl)
+ return;
+ impl->ptr = nullptr;
+ impl = nullptr;
+ }
+};
+
+template <typename T>
+class WeakPtr {
+private:
+ RefPtr<WeakPtrImpl<T>> impl;
+
+public:
+ WeakPtr(T& t) {
+ *this = t;
+ }
+ WeakPtr(T* t) {
+ *this = t;
+ }
+
+ template <typename U>
+ WeakPtr<T> operator=(U& obj) {
+ impl = obj.createWeakPtrImpl();
+ }
+
+ template <typename U>
+ WeakPtr<T> operator=(U* obj) {
+ impl = obj ? obj->createWeakPtrImpl() : nullptr;
+ }
+
+ T* get() {
+ return impl ? impl->get() : nullptr;
+ }
+
+};
+
#endif
>From 7e74953aeb5a0ee8d6f227b5eb0f79b2c4b52a55 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 27 Sep 2025 10:58:16 -0700
Subject: [PATCH 3/3] Test fix attempt
---
.../Analysis/Checkers/WebKit/mock-types.h | 34 +++++++++++++------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index d110cd11142dd..7055a94753a37 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -313,24 +313,36 @@ class UniqueRef {
UniqueRef &operator=(T &) { return *this; }
};
-template <typename T>
class WeakPtrImpl {
private:
- void* ptr;
+ void* ptr { nullptr };
+ mutable unsigned m_refCount { 0 };
template <typename U> friend class CanMakeWeakPtr;
template <typename U> friend class WeakPtr;
- Ref<WeakPtrImpl<T>> create(T& t)
+public:
+ template <typename T>
+ static Ref<WeakPtrImpl> create(T& t)
{
- return adoptNS(new WeakPtrImpl<T>(t));
+ return adoptRef(*new WeakPtrImpl(t));
}
+ void ref() const { m_refCount++; }
+ void deref() const {
+ m_refCount--;
+ if (!m_refCount)
+ delete const_cast<WeakPtrImpl*>(this);
+ }
+
+ template <typename T>
T* get() { return static_cast<T*>(ptr); }
operator bool() const { return !!ptr; }
void clear() { ptr = nullptr; }
- WeakPtrImpl(T& t)
+private:
+ template <typename T>
+ WeakPtrImpl(T* t)
: ptr(static_cast<void*>(t))
{ }
};
@@ -338,14 +350,14 @@ class WeakPtrImpl {
template <typename T>
class CanMakeWeakPtr {
private:
- RefPtr<WeakPtrImpl<T>> impl;
+ RefPtr<WeakPtrImpl> impl;
template <typename U> friend class CanMakeWeakPtr;
template <typename U> friend class WeakPtr;
- Ref<WeakPtrImpl<T>> createWeakPtrImpl() {
+ Ref<WeakPtrImpl> createWeakPtrImpl() {
if (!impl)
- impl = WeakPtrImpl<T>::create(static_cast<T>(*this));
+ impl = WeakPtrImpl::create(static_cast<T>(*this));
return *impl;
}
@@ -353,7 +365,7 @@ class CanMakeWeakPtr {
~CanMakeWeakPtr() {
if (!impl)
return;
- impl->ptr = nullptr;
+ impl->clear();
impl = nullptr;
}
};
@@ -361,7 +373,7 @@ class CanMakeWeakPtr {
template <typename T>
class WeakPtr {
private:
- RefPtr<WeakPtrImpl<T>> impl;
+ RefPtr<WeakPtrImpl> impl;
public:
WeakPtr(T& t) {
@@ -382,7 +394,7 @@ class WeakPtr {
}
T* get() {
- return impl ? impl->get() : nullptr;
+ return impl ? impl->get<T>() : nullptr;
}
};
More information about the cfe-commits
mailing list