From 6b4a6b832f61efc26396f60309744c2e7264156d Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Fri, 13 Dec 2024 01:49:21 -0800 Subject: [PATCH 1/5] [WebKit checkers] Recognize adoptRef as a safe function adoptRef in WebKit constructs Ref/RefPtr so treat it as such in isCtorOfRefCounted. Also removed the support for makeRef and makeRefPtr as they don't exist any more. --- .../Checkers/WebKit/PtrTypesSemantics.cpp | 5 ++-- .../Analysis/Checkers/WebKit/call-args.cpp | 17 +++++++++++ .../Analysis/Checkers/WebKit/mock-types.h | 28 ++++++++++++++++++- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 797f3e1f3fba5..5487fea1b956c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -125,9 +125,8 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) { assert(F); const std::string &FunctionName = safeGetName(F); - return isRefType(FunctionName) || FunctionName == "makeRef" || - FunctionName == "makeRefPtr" || FunctionName == "UniqueRef" || - FunctionName == "makeUniqueRef" || + return isRefType(FunctionName) || FunctionName == "adoptRef" || + FunctionName == "UniqueRef" || FunctionName == "makeUniqueRef" || FunctionName == "makeUniqueRefWithoutFastMallocCheck" || FunctionName == "String" || FunctionName == "AtomString" || diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp index 94efddeaf66cd..574e3aa6ef476 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp @@ -365,3 +365,20 @@ namespace call_with_explicit_temporary_obj { RefPtr { provide() }->method(); } } + +namespace call_with_adopt_ref { + class Obj { + public: + void ref() const; + void deref() const; + void method(); + }; + + struct dummy { + RefPtr any; + }; + + void foo() { + adoptRef(new Obj)->method(); + } +} diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index fb1ee51c7ec1d..17c449b6c2ec2 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -46,7 +46,10 @@ template struct DefaultRefDerefTraits { template , typename RefDerefTraits = DefaultRefDerefTraits> struct Ref { typename PtrTraits::StorageType t; + enum AdoptTag { Adopt }; + Ref() : t{} {}; + Ref(T &t, AdoptTag) : t(&t) { } Ref(T &t) : t(&RefDerefTraits::ref(t)) { } Ref(const Ref& o) : t(RefDerefTraits::refIfNotNull(PtrTraits::unwrap(o.t))) { } Ref(Ref&& o) : t(o.leakRef()) { } @@ -73,10 +76,19 @@ template , typename RefDerefTra T* leakRef() { return PtrTraits::exchange(t, nullptr); } }; +template Ref adoptRef(T& t) { + using Ref = Ref; + return Ref(t, Ref::Adopt); +} + +template class RefPtr; +template RefPtr adoptRef(T*); + template struct RefPtr { T *t; - RefPtr() : t(new T) {} + RefPtr() : t(nullptr) { } + RefPtr(T *t) : t(t) { if (t) @@ -85,6 +97,9 @@ template struct RefPtr { RefPtr(Ref&& o) : t(o.leakRef()) { } + RefPtr(RefPtr&& o) + : t(o.leakRef()) + { } ~RefPtr() { if (t) t->deref(); @@ -110,8 +125,19 @@ template struct RefPtr { return *this; } operator bool() const { return t; } + +private: + friend RefPtr adoptRef(T*); + + // call_with_adopt_ref in call-args.cpp requires this method to be private. + enum AdoptTag { Adopt }; + RefPtr(T *t, AdoptTag) : t(t) { } }; +template RefPtr adoptRef(T* t) { + return RefPtr(t, RefPtr::Adopt); +} + template bool operator==(const RefPtr &, const RefPtr &) { return false; } From 0cdeb5676251a3c8d832baf1de800fe8a535b600 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Fri, 13 Dec 2024 01:58:00 -0800 Subject: [PATCH 2/5] Fix the mock RefPtr --- clang/test/Analysis/Checkers/WebKit/mock-types.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 17c449b6c2ec2..9625ae6128883 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -95,11 +95,13 @@ template struct RefPtr { t->ref(); } RefPtr(Ref&& o) - : t(o.leakRef()) + : t(o.leafkRef()) { } RefPtr(RefPtr&& o) - : t(o.leakRef()) - { } + : t(o.t) + { + o.t = nullptr; + } ~RefPtr() { if (t) t->deref(); From bc318db7b2a4c48882988d1ca26ab54cb351757a Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Wed, 18 Dec 2024 10:31:41 -0800 Subject: [PATCH 3/5] Added a comment regarding dummy struct for rdar://141692212 --- clang/test/Analysis/Checkers/WebKit/call-args.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp index 574e3aa6ef476..f1eee216cdffa 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp @@ -374,6 +374,7 @@ namespace call_with_adopt_ref { void method(); }; + // This is needed due to rdar://141692212. struct dummy { RefPtr any; }; From 89004f9f0d784d165033eb92883f5bc88ae38e22 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 19 Dec 2024 00:24:35 -0800 Subject: [PATCH 4/5] Fix the tests --- clang/test/Analysis/Checkers/WebKit/mock-types.h | 8 +++++++- .../WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp | 8 -------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 9625ae6128883..f87f1ea5b89ab 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -95,13 +95,19 @@ template struct RefPtr { t->ref(); } RefPtr(Ref&& o) - : t(o.leafkRef()) + : t(o.leakRef()) { } RefPtr(RefPtr&& o) : t(o.t) { o.t = nullptr; } + RefPtr(const RefPtr& o) + : t(o.t) + { + if (t) + t->ref(); + } ~RefPtr() { if (t) t->deref(); diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp index 33c60ea8ca64d..4209db14eaa52 100644 --- a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp +++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp @@ -61,14 +61,6 @@ template Function adopt(Detail::Callab return Function(impl, Function::Adopt); } -template, typename RefDerefTraits = DefaultRefDerefTraits> Ref adoptRef(T&); - -template -inline Ref adoptRef(T& reference) -{ - return Ref(reference); -} - enum class DestructionThread : unsigned char { Any, Main, MainRunLoop }; void ensureOnMainThread(Function&&); // Sync if called on main thread, async otherwise. void ensureOnMainRunLoop(Function&&); // Sync if called on main run loop, async otherwise. From fa77995633581b4eb56b867ea767cd6f328a85b8 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 19 Dec 2024 11:48:41 -0800 Subject: [PATCH 5/5] Fix local-vars-counted-const-member.cpp by adding a copy assignment to mock RefPtr. --- clang/test/Analysis/Checkers/WebKit/mock-types.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index f87f1ea5b89ab..0dc4259a14e29 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -105,8 +105,17 @@ template struct RefPtr { RefPtr(const RefPtr& o) : t(o.t) { - if (t) - t->ref(); + if (t) + t->ref(); + } + RefPtr operator=(const RefPtr& o) + { + if (t) + t->deref(); + t = o.t; + if (t) + t->ref(); + return *this; } ~RefPtr() { if (t)