Author: Fady Farag Date: 2026-09-16T01:38:39-07:00 New Revision: 59e842209b868ca110a285c92b8917a3e223995d
URL: https://github.com/llvm/llvm-project/commit/59e842209b868ca110a285c92b8917a3e223995d DIFF: https://github.com/llvm/llvm-project/commit/59e842209b868ca110a285c92b8917a3e223995d.diff LOG: [alpha.webkit.UncountedCallArgsChecker] Look through unique_ptr::get() when finding a pointer's origin (#223568) Previously, the checker only looked through `operator->` and `operator*` on `std::unique_ptr`, `UniqueRef` and `LazyUniqueRef` when walking a call argument back to its origin. It didn't include `get()` and `ptr()` on those types, which caused an unnecessary warning. This includes `get()` and `ptr()` on those types such that they are recognized and are consistent with ref-counted and checked pointers. Added: clang/test/Analysis/Checkers/WebKit/call-args-checked-unique-ptr.cpp clang/test/Analysis/Checkers/WebKit/call-args-counted-unique-ptr.cpp Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index e8f69f1aac757..06eaa0673143f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -122,6 +122,10 @@ bool tryToFindPtrOrigin( } continue; } + if (isGetterOfUniquePtr(decl)) { + E = memberCall->getImplicitObjectArgument(); + continue; + } } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 37347c51d4ca2..bdf6358f1ca15 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -138,9 +138,13 @@ bool isCheckedPtr(const std::string &Name) { return Name == "CheckedPtr" || Name == "CheckedRef"; } +bool isUniquePtr(const std::string &Name) { + return Name == "unique_ptr" || Name == "UniqueRef" || Name == "LazyUniqueRef"; +} + bool isOwnerPtr(const std::string &Name) { return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) || - Name == "unique_ptr" || Name == "UniqueRef" || Name == "LazyUniqueRef"; + isUniquePtr(Name); } static bool isWeakPtrClass(const std::string &Name) { @@ -405,6 +409,20 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) { return false; } +bool isGetterOfUniquePtr(const CXXMethodDecl *M) { + assert(M); + if (!isUniquePtr(safeGetName(M->getParent()))) + return false; + auto method = safeGetName(M); + if (method == "get" || method == "ptr") + return true; + if (auto *conversion = dyn_cast<CXXConversionDecl>(M)) { + const Type *T = conversion->getConversionType().getTypePtrOrNull(); + return T && (T->isPointerType() || T->isReferenceType()); + } + return false; +} + bool isRefCounted(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 4e548c44c6bb9..3a58b68f50a9a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -149,12 +149,19 @@ bool isRetainPtrOrOSPtr(const std::string &Name); /// and unique_ptr. bool isOwnerPtr(const std::string &Name); +/// \returns true if \p Name is unique_ptr, UniqueRef, or LazyUniqueRef. +bool isUniquePtr(const std::string &Name); + /// \returns true if \p Name is a smart pointer type name, false if not. bool isSmartPtrClass(const std::string &Name); /// \returns true if \p M is getter of a ref-counted class, false if not. std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method); +/// \returns true if \p M is a getter of unique_ptr, UniqueRef, or +/// LazyUniqueRef, false if not. +bool isGetterOfUniquePtr(const clang::CXXMethodDecl *Method); + /// \returns true if \p F is a conversion between ref-countable or ref-counted /// pointer types. bool isPtrConversion(const FunctionDecl *F); diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked-unique-ptr.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked-unique-ptr.cpp new file mode 100644 index 0000000000000..86833a7e852a2 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked-unique-ptr.cpp @@ -0,0 +1,62 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedCallArgsChecker -verify %s + +#include "mock-types.h" + +void consume(CheckedObj*); +void consumeRef(CheckedObj&); + +namespace local_unique_ptr { + +void foo() { + std::unique_ptr<CheckedObj> obj; + obj->method(); + (*obj).method(); + consumeRef(*obj); + consume(&*obj); + consume(obj.get()); + obj.get()->method(); +} + +void bar(std::unique_ptr<CheckedObj>& obj, const std::unique_ptr<CheckedObj>& constObj) { + obj->method(); + consume(obj.get()); + constObj->method(); + consume(constObj.get()); +} + +} // namespace local_unique_ptr + +namespace local_unique_ref { + +void foo(CheckedObj& target) { + UniqueRef<CheckedObj> obj(target); + obj->method(); + obj.get().method(); + consumeRef(obj.get()); + consumeRef(obj); + consume(&obj.get()); +} + +} // namespace local_unique_ref + +namespace member_unique_ptr { + +class Foo { +public: + void bar(); + +private: + const std::unique_ptr<CheckedObj> m_constObj; + std::unique_ptr<CheckedObj> m_obj; +}; + +void Foo::bar() { + consume(m_constObj.get()); + m_constObj.get()->method(); + consume(m_obj.get()); + // expected-warning@-1{{Function argument 'this->m_obj.get()' (to 'consume') is a raw pointer to CheckedPtr-capable type 'CheckedObj'}} + m_obj.get()->method(); + // expected-warning@-1{{Function argument 'this->m_obj.get()' (parameter 'this' to 'CheckedObj::method') is a raw pointer to CheckedPtr-capable type 'CheckedObj'}} +} + +} // namespace member_unique_ptr diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-counted-unique-ptr.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-counted-unique-ptr.cpp new file mode 100644 index 0000000000000..ae1230a15f921 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-counted-unique-ptr.cpp @@ -0,0 +1,62 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s + +#include "mock-types.h" + +void consume(RefCountable*); +void consumeRef(RefCountable&); + +namespace local_unique_ptr { + +void foo() { + std::unique_ptr<RefCountable> obj = RefCountable::makeUnique(); + obj->method(); + (*obj).method(); + consumeRef(*obj); + consume(&*obj); + consume(obj.get()); + obj.get()->method(); +} + +void bar(std::unique_ptr<RefCountable>& obj, const std::unique_ptr<RefCountable>& constObj) { + obj->method(); + consume(obj.get()); + constObj->method(); + consume(constObj.get()); +} + +} // namespace local_unique_ptr + +namespace local_unique_ref { + +void foo(RefCountable& target) { + UniqueRef<RefCountable> obj(target); + obj->method(); + obj.get().method(); + consumeRef(obj.get()); + consumeRef(obj); + consume(&obj.get()); +} + +} // namespace local_unique_ref + +namespace member_unique_ptr { + +class Foo { +public: + void bar(); + +private: + const std::unique_ptr<RefCountable> m_constObj; + std::unique_ptr<RefCountable> m_obj; +}; + +void Foo::bar() { + consume(m_constObj.get()); + m_constObj.get()->method(); + consume(m_obj.get()); + // expected-warning@-1{{Function argument 'this->m_obj.get()' (to 'consume') is a raw pointer to RefPtr-capable type 'RefCountable'}} + m_obj.get()->method(); + // expected-warning@-1{{Function argument 'this->m_obj.get()' (parameter 'this' to 'RefCountable::method') is a raw pointer to RefPtr-capable type 'RefCountable'}} +} + +} // namespace member_unique_ptr _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
