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 <[email protected]> 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 <[email protected]> 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@-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 <[email protected]> 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; } }; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
