[clang] [WebKit checkers] Recognize adoptRef as a safe function (PR #119846)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 19 00:24:54 PST 2024
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/119846
>From 6b4a6b832f61efc26396f60309744c2e7264156d Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 13 Dec 2024 01:49:21 -0800
Subject: [PATCH 1/4] [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 797f3e1f3fba5a..5487fea1b956c8 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 94efddeaf66cd8..574e3aa6ef476a 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<Obj> 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 fb1ee51c7ec1de..17c449b6c2ec26 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -46,7 +46,10 @@ template<typename T> struct DefaultRefDerefTraits {
template <typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTraits = DefaultRefDerefTraits<T>> 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 T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTra
T* leakRef() { return PtrTraits::exchange(t, nullptr); }
};
+template <typename T> Ref<T> adoptRef(T& t) {
+ using Ref = Ref<T>;
+ return Ref(t, Ref::Adopt);
+}
+
+template<typename T> class RefPtr;
+template<typename T> RefPtr<T> adoptRef(T*);
+
template <typename T> struct RefPtr {
T *t;
- RefPtr() : t(new T) {}
+ RefPtr() : t(nullptr) { }
+
RefPtr(T *t)
: t(t) {
if (t)
@@ -85,6 +97,9 @@ template <typename T> struct RefPtr {
RefPtr(Ref<T>&& o)
: t(o.leakRef())
{ }
+ RefPtr(RefPtr&& o)
+ : t(o.leakRef())
+ { }
~RefPtr() {
if (t)
t->deref();
@@ -110,8 +125,19 @@ template <typename T> struct RefPtr {
return *this;
}
operator bool() const { return t; }
+
+private:
+ friend RefPtr adoptRef<T>(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 <typename T> RefPtr<T> adoptRef(T* t) {
+ return RefPtr<T>(t, RefPtr<T>::Adopt);
+}
+
template <typename T> bool operator==(const RefPtr<T> &, const RefPtr<T> &) {
return false;
}
>From 0cdeb5676251a3c8d832baf1de800fe8a535b600 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 13 Dec 2024 01:58:00 -0800
Subject: [PATCH 2/4] 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 17c449b6c2ec26..9625ae6128883c 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -95,11 +95,13 @@ template <typename T> struct RefPtr {
t->ref();
}
RefPtr(Ref<T>&& 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 <rniwa at webkit.org>
Date: Wed, 18 Dec 2024 10:31:41 -0800
Subject: [PATCH 3/4] 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 574e3aa6ef476a..f1eee216cdffa1 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<Obj> any;
};
>From 89004f9f0d784d165033eb92883f5bc88ae38e22 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 19 Dec 2024 00:24:35 -0800
Subject: [PATCH 4/4] 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 9625ae6128883c..f87f1ea5b89abb 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -95,13 +95,19 @@ template <typename T> struct RefPtr {
t->ref();
}
RefPtr(Ref<T>&& 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 33c60ea8ca64d1..4209db14eaa52b 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<typename Out, typename... In> Function<Out(In...)> adopt(Detail::Callab
return Function<Out(In...)>(impl, Function<Out(In...)>::Adopt);
}
-template<typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTraits = DefaultRefDerefTraits<T>> Ref<T, PtrTraits, RefDerefTraits> adoptRef(T&);
-
-template<typename T, typename _PtrTraits, typename RefDerefTraits>
-inline Ref<T, _PtrTraits, RefDerefTraits> adoptRef(T& reference)
-{
- return Ref<T, _PtrTraits, RefDerefTraits>(reference);
-}
-
enum class DestructionThread : unsigned char { Any, Main, MainRunLoop };
void ensureOnMainThread(Function<void()>&&); // Sync if called on main thread, async otherwise.
void ensureOnMainRunLoop(Function<void()>&&); // Sync if called on main run loop, async otherwise.
More information about the cfe-commits
mailing list