Author: Ryosuke Niwa Date: 2024-09-10T22:25:03-07:00 New Revision: 203a2ca8cd6af505e11a38aebceeaf864271042c
URL: https://github.com/llvm/llvm-project/commit/203a2ca8cd6af505e11a38aebceeaf864271042c DIFF: https://github.com/llvm/llvm-project/commit/203a2ca8cd6af505e11a38aebceeaf864271042c.diff LOG: [webkit.RefCntblBaseVirtualDtor] Make ThreadSafeRefCounted not generate warnings (#107676) 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. Added: clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp index 9df108e28ecdbb..ecba5f9aa23ee3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp @@ -67,6 +67,32 @@ class DerefFuncDeleteExprVisitor const Decl *D = CE->getCalleeDecl(); if (D && D->hasBody()) return VisitBody(D->getBody()); + else { + auto name = safeGetName(D); + if (name == "ensureOnMainThread" || name == "ensureOnMainRunLoop") { + for (unsigned i = 0; i < CE->getNumArgs(); ++i) { + auto *Arg = CE->getArg(i); + if (VisitLabmdaArgument(Arg)) + return true; + } + } + } + return false; + } + + bool VisitLabmdaArgument(const Expr *E) { + E = E->IgnoreParenCasts(); + if (auto *TempE = dyn_cast<CXXBindTemporaryExpr>(E)) + E = TempE->getSubExpr(); + if (auto *ConstructE = dyn_cast<CXXConstructExpr>(E)) { + for (unsigned i = 0; i < ConstructE->getNumArgs(); ++i) { + auto *Arg = ConstructE->getArg(i); + if (auto *Lambda = dyn_cast<LambdaExpr>(Arg)) { + if (VisitBody(Lambda->getBody())) + return true; + } + } + } return false; } 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..01527addb52992 --- /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; + ensureOnMainRunLoop([&] { + 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
