https://github.com/rniwa updated
https://github.com/llvm/llvm-project/pull/107676
>From 9a8c60355f88d7d4cee6d9f75312bb87d13b74a9 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Sat, 7 Sep 2024 01:45:05 -0700
Subject: [PATCH] [webkit.RefCntblBaseVirtualDtor] Make ThreadSafeRefCounted
not generate warnings
This PR makes WebKit's RefCntblBaseVirtualDtor checker not generate a warning
for
ThreadSafeRefCounted when the destruction thread is a specific thread.
Prior to this PR, we only allowed CRTP classes without a virtual destructor
if its deref function had an explicit cast to the derived type, skipping any
lambda declarations which aren't invoked. This ends up generating a warning for
ThreadSafeRefCounted when a specific thread is used to destruct the object
because there is no inline body / definition for ensureOnMainThread and
ensureOnMainRunLoop and DerefFuncDeleteExprVisitor concludes that there is no
explicit delete of the derived type.
This PR relaxes the condition DerefFuncDeleteExprVisitor checks by allowing
a delete expression to appear within a lambda declaration if it's an argument
to an "opaque" function; i.e. a function without definition / body.
---
.../WebKit/RefCntblBaseVirtualDtorChecker.cpp | 22 +-
.../ref-cntbl-crtp-base-no-virtual-dtor.cpp | 232 ++++++++++++++++++
2 files changed, 253 insertions(+), 1 deletion(-)
create mode 100644
clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 26879f2f87c8be..8e97efc1ab8e8e 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
#include <optional>
@@ -67,6 +68,15 @@ class DerefFuncDeleteExprVisitor
const Decl *D = CE->getCalleeDecl();
if (D && D->hasBody())
return VisitBody(D->getBody());
+ else if (!VisitLambdaBody) {
+ for (unsigned i = 0; i < CE->getNumArgs(); ++i) {
+ auto *Arg = CE->getArg(i);
+ VisitLambdaBody = true;
+ auto Restore = llvm::make_scope_exit([&] { VisitLambdaBody = false; });
+ if (VisitChildren(Arg))
+ return true;
+ }
+ }
return false;
}
@@ -113,12 +123,22 @@ class DerefFuncDeleteExprVisitor
// Return false since the contents of lambda isn't necessarily executed.
// If it is executed, VisitCallExpr above will visit its body.
- bool VisitLambdaExpr(const LambdaExpr *) { return false; }
+ // Allow returning true for a lambda if it's a function argument to another
+ // function without body / definition.
+ bool VisitLambdaExpr(const LambdaExpr *E) {
+ if (VisitLambdaBody) {
+ VisitLambdaBody = false;
+ auto Restore = llvm::make_scope_exit([&] { VisitLambdaBody = true; });
+ return VisitChildren(E);
+ }
+ return false;
+ }
private:
const TemplateArgumentList *ArgList{nullptr};
const CXXRecordDecl *ClassDecl;
llvm::DenseSet<const Stmt *> VisitedBody;
+ bool VisitLambdaBody{false};
};
class RefCntblBaseVirtualDtorChecker
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
new file mode 100644
index 00000000000000..e149c1d3d3575f
--- /dev/null
+++
b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
@@ -0,0 +1,232 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor
-verify %s
+
+#include "mock-types.h"
+
+namespace Detail {
+
+template<typename Out, typename... In>
+class CallableWrapperBase {
+public:
+ virtual ~CallableWrapperBase() { }
+ virtual Out call(In...) = 0;
+};
+
+template<typename, typename, typename...> class CallableWrapper;
+
+template<typename CallableType, typename Out, typename... In>
+class CallableWrapper : public CallableWrapperBase<Out, In...> {
+public:
+ explicit CallableWrapper(CallableType&& callable)
+ : m_callable(WTFMove(callable)) { }
+ CallableWrapper(const CallableWrapper&) = delete;
+ CallableWrapper& operator=(const CallableWrapper&) = delete;
+ Out call(In... in) final;
+private:
+ CallableType m_callable;
+};
+
+} // namespace Detail
+
+template<typename> class Function;
+
+template<typename Out, typename... In> Function<Out(In...)>
adopt(Detail::CallableWrapperBase<Out, In...>*);
+
+template <typename Out, typename... In>
+class Function<Out(In...)> {
+public:
+ using Impl = Detail::CallableWrapperBase<Out, In...>;
+
+ Function() = default;
+
+ template<typename FunctionType>
+ Function(FunctionType f);
+
+ Out operator()(In... in) const;
+ explicit operator bool() const { return !!m_callableWrapper; }
+
+private:
+ enum AdoptTag { Adopt };
+ Function(Impl* impl, AdoptTag)
+ : m_callableWrapper(impl)
+ {
+ }
+
+ friend Function adopt<Out, In...>(Impl*);
+
+ Impl* m_callableWrapper;
+};
+
+template<typename Out, typename... In> Function<Out(In...)>
adopt(Detail::CallableWrapperBase<Out, In...>* impl)
+{
+ 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.
+
+class ThreadSafeRefCountedBase {
+public:
+ ThreadSafeRefCountedBase() = default;
+
+ void ref() const
+ {
+ ++m_refCount;
+ }
+
+ bool hasOneRef() const
+ {
+ return refCount() == 1;
+ }
+
+ unsigned refCount() const
+ {
+ return m_refCount;
+ }
+
+protected:
+ bool derefBase() const
+ {
+ if (!--m_refCount) {
+ m_refCount = 1;
+ return true;
+ }
+ return false;
+ }
+
+private:
+ mutable unsigned m_refCount { 1 };
+};
+
+template<class T, DestructionThread destructionThread =
DestructionThread::Any> class ThreadSafeRefCounted : public
ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+
+ if constexpr (destructionThread == DestructionThread::Any) {
+ delete static_cast<const T*>(this);
+ } else if constexpr (destructionThread == DestructionThread::Main) {
+ ensureOnMainThread([this] {
+ delete static_cast<const T*>(this);
+ });
+ }
+ }
+
+protected:
+ ThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass final : public
ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main> {
+public:
+ static Ref<FancyRefCountedClass> create()
+ {
+ return adoptRef(*new FancyRefCountedClass());
+ }
+
+ virtual ~FancyRefCountedClass();
+
+private:
+ FancyRefCountedClass();
+};
+
+template<class T, DestructionThread destructionThread =
DestructionThread::Any> class BadThreadSafeRefCounted : public
ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+
+ [this] {
+ delete static_cast<const T*>(this);
+ };
+ }
+
+protected:
+ BadThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass2 final : public
ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main> {
+// expected-warning@-1{{Class 'ThreadSafeRefCounted<FancyRefCountedClass,
DestructionThread::Main>' is used as a base of class 'FancyRefCountedClass2'
but doesn't have virtual destructor}}
+public:
+ static Ref<FancyRefCountedClass2> create()
+ {
+ return adoptRef(*new FancyRefCountedClass2());
+ }
+
+ virtual ~FancyRefCountedClass2();
+
+private:
+ FancyRefCountedClass2();
+};
+
+template<class T, DestructionThread destructionThread =
DestructionThread::Any> class NestedThreadSafeRefCounted : public
ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+ ensureOnMainThread([&] {
+ auto destroyThis = [&] {
+ delete static_cast<const T*>(this);
+ };
+ destroyThis();
+ });
+ }
+
+protected:
+ NestedThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass3 final : public
NestedThreadSafeRefCounted<FancyRefCountedClass3, DestructionThread::Main> {
+public:
+ static Ref<FancyRefCountedClass3> create()
+ {
+ return adoptRef(*new FancyRefCountedClass3());
+ }
+
+ virtual ~FancyRefCountedClass3();
+
+private:
+ FancyRefCountedClass3();
+};
+
+template<class T, DestructionThread destructionThread =
DestructionThread::Any> class BadNestedThreadSafeRefCounted : public
ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+ ensureOnMainThread([&] {
+ auto destroyThis = [&] {
+ delete static_cast<const T*>(this);
+ };
+ });
+ }
+
+protected:
+ BadNestedThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass4 final : public
BadNestedThreadSafeRefCounted<FancyRefCountedClass4, DestructionThread::Main> {
+// expected-warning@-1{{Class
'BadNestedThreadSafeRefCounted<FancyRefCountedClass4, DestructionThread::Main>'
is used as a base of class 'FancyRefCountedClass4' but doesn't have virtual
destructor}}
+public:
+ static Ref<FancyRefCountedClass4> create()
+ {
+ return adoptRef(*new FancyRefCountedClass4());
+ }
+
+ virtual ~FancyRefCountedClass4();
+
+private:
+ FancyRefCountedClass4();
+};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
