From 10773301cfa84ced5d11e2fd22d188297b59ab43 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Tue, 21 Sep 2021 11:37:12 +0300 Subject: [PATCH] Backed out 7 changesets (bug 1731218) for causing spidermonkey build bustages in RootingAPI. CLOSED TREE Backed out changeset 0dfcae852026 (bug 1731218) Backed out changeset e2c59b5af7ea (bug 1731218) Backed out changeset 3e98b832dcc6 (bug 1731218) Backed out changeset 57e60277e4ca (bug 1731218) Backed out changeset 3b264a4bc67e (bug 1731218) Backed out changeset 304f27af6b95 (bug 1731218) Backed out changeset 06e7a1992de8 (bug 1731218) --- js/public/RootingAPI.h | 196 ++++++++++++---------- js/public/Value.h | 2 +- js/src/gc/MaybeRooted.h | 4 +- js/src/gc/RootMarking.cpp | 96 ++++++----- js/src/jsapi-tests/testGCExactRooting.cpp | 4 +- js/src/vm/Interpreter.cpp | 2 +- js/src/vm/JSObject.h | 4 +- js/src/vm/Runtime.h | 6 +- 8 files changed, 179 insertions(+), 135 deletions(-) diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index 29331891e0ca..62997491f3a2 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -125,17 +125,16 @@ class MutableWrappedPtrOperations : public WrappedPtrOperations {}; template -class RootedOperations : public MutableWrappedPtrOperations {}; +class RootedBase : public MutableWrappedPtrOperations {}; template -class HandleOperations : public WrappedPtrOperations {}; +class HandleBase : public WrappedPtrOperations {}; template -class MutableHandleOperations : public MutableWrappedPtrOperations { -}; +class MutableHandleBase : public MutableWrappedPtrOperations {}; template -class HeapOperations : public MutableWrappedPtrOperations {}; +class HeapBase : public MutableWrappedPtrOperations {}; // Cannot use FOR_EACH_HEAP_ABLE_GC_POINTER_TYPE, as this would import too many // macros into scope @@ -290,7 +289,7 @@ inline void AssertGCThingIsNotNurseryAllocable(js::gc::Cell* cell) {} * Type T must be a public GC pointer type. */ template -class MOZ_NON_MEMMOVABLE Heap : public js::HeapOperations> { +class MOZ_NON_MEMMOVABLE Heap : public js::HeapBase> { // Please note: this can actually also be used by nsXBLMaybeCompiled, for // legacy reasons. static_assert(js::IsHeapConstructibleType::value, @@ -450,7 +449,7 @@ inline void AssertObjectIsNotGray(const JS::Heap& obj) {} * - It is not possible to store flag bits in a Heap. */ template -class TenuredHeap : public js::HeapOperations> { +class TenuredHeap : public js::HeapBase> { public: using ElementType = T; @@ -570,10 +569,10 @@ class PersistentRooted; * rooted. See "Move GC Stack Rooting" above. * * If you want to add additional methods to Handle for a specific - * specialization, define a HandleOperations specialization containing them. + * specialization, define a HandleBase specialization containing them. */ template -class MOZ_NONHEAP_CLASS Handle : public js::HandleOperations> { +class MOZ_NONHEAP_CLASS Handle : public js::HandleBase> { friend class MutableHandle; public: @@ -668,12 +667,12 @@ struct DefineComparisonOps> : std::true_type { * useful for outparams. * * If you want to add additional methods to MutableHandle for a specific - * specialization, define a MutableHandleOperations specialization containing + * specialization, define a MutableHandleBase specialization containing * them. */ template class MOZ_STACK_CLASS MutableHandle - : public js::MutableHandleOperations> { + : public js::MutableHandleBase> { public: using ElementType = T; @@ -878,64 +877,40 @@ struct VirtualTraceable { virtual void trace(JSTracer* trc, const char* name) = 0; }; -class StackRootedBase { - public: - StackRootedBase* previous() { return prev; } +template +struct RootedTraceable final : public VirtualTraceable { + static_assert(JS::MapTypeToRootKind::kind == JS::RootKind::Traceable, + "RootedTraceable is intended only for usage with a Traceable"); - protected: - StackRootedBase** stack; - StackRootedBase* prev; + T ptr; - template - auto* derived() { - return static_cast*>(this); - } -}; + template + explicit RootedTraceable(std::in_place_t, CtorArgs... args) + : ptr(std::forward(args)...) {} -class PersistentRootedBase - : protected mozilla::LinkedListElement { - protected: - friend class mozilla::LinkedList; - friend class mozilla::LinkedListElement; + template ::type> + MOZ_IMPLICIT RootedTraceable(U&& initial) : ptr(std::forward(initial)) {} - template - auto* derived() { - return static_cast*>(this); - } -}; + operator T&() { return ptr; } + operator const T&() const { return ptr; } -struct StackRootedTraceableBase : public StackRootedBase, - public VirtualTraceable {}; - -class PersistentRootedTraceableBase : public PersistentRootedBase, - public VirtualTraceable {}; - -template -class TypedRootedGCThingBase : public Base { - public: - void trace(JSTracer* trc, const char* name); -}; - -template -class TypedRootedTraceableBase : public Base { - public: void trace(JSTracer* trc, const char* name) override { - auto* self = this->template derived(); - JS::GCPolicy::trace(trc, self->address(), name); + JS::GCPolicy::trace(trc, &ptr, name); } }; template struct RootedTraceableTraits { - using StackRootedBase = TypedRootedTraceableBase; - using PersistentRootedBase = - TypedRootedTraceableBase; + static T* address(RootedTraceable& self) { return &self.ptr; } + static const T* address(const RootedTraceable& self) { return &self.ptr; } + static void trace(JSTracer* trc, VirtualTraceable* thingp, const char* name); }; template struct RootedGCThingTraits { - using StackRootedBase = TypedRootedGCThingBase; - using PersistentRootedBase = TypedRootedGCThingBase; + static T* address(T& self) { return &self; } + static const T* address(const T& self) { return &self; } + static void trace(JSTracer* trc, T* thingp, const char* name); }; } /* namespace js */ @@ -952,8 +927,29 @@ enum class AutoGCRooterKind : uint8_t { Limit }; +namespace detail { +// Dummy type to store root list entry pointers as. This code does not just use +// the actual type, because then eg JSObject* and JSFunction* would be assumed +// to never alias but they do (they are stored in the same list). Also, do not +// use `void*` so that `Rooted` is a compile error. +struct RootListEntry; +} // namespace detail + +template <> +struct MapTypeToRootKind { + static const RootKind kind = RootKind::Traceable; +}; + +// Workaround MSVC issue where GCPolicy is needed even though this dummy type is +// never instantiated. Ideally, RootListEntry is removed in the future and an +// appropriate class hierarchy for the Rooted types. +template <> +struct GCPolicy + : public IgnoreGCPolicy {}; + using RootedListHeads = - mozilla::EnumeratedArray; + mozilla::EnumeratedArray*>; using AutoRooterListHeads = mozilla::EnumeratedArray constexpr bool IsTraceable_v = MapTypeToRootKind::kind == JS::RootKind::Traceable; +template +using RootedPtr = + std::conditional_t, js::RootedTraceable, T>; + template using RootedPtrTraits = std::conditional_t, js::RootedTraceableTraits, @@ -1103,17 +1103,17 @@ using RootedPtrTraits = * function that requires a handle, e.g. Foo(Root(cx, x)). * * If you want to add additional methods to Rooted for a specific - * specialization, define a RootedOperations specialization containing them. + * specialization, define a RootedBase specialization containing them. */ template -class MOZ_RAII Rooted : public detail::RootedPtrTraits::StackRootedBase, - public js::RootedOperations> { +class MOZ_RAII Rooted : public js::RootedBase> { + using Ptr = detail::RootedPtr; using PtrTraits = detail::RootedPtrTraits; inline void registerWithRootLists(RootedListHeads& roots) { this->stack = &roots[JS::MapTypeToRootKind::kind]; - this->prev = *this->stack; - *this->stack = this; + this->prev = *stack; + *stack = reinterpret_cast*>(this); } inline RootedListHeads& rootLists(RootingContext* cx) { @@ -1141,7 +1141,8 @@ class MOZ_RAII Rooted : public detail::RootedPtrTraits::StackRootedBase, // Provide an initial value. Requires T to be constructible from the given // argument. - template + template ::type> Rooted(const RootingContext& cx, S&& initial) : ptr(std::forward(initial)) { MOZ_ASSERT(GCPolicy::isValid(ptr)); @@ -1160,16 +1161,19 @@ class MOZ_RAII Rooted : public detail::RootedPtrTraits::StackRootedBase, typename RootingContext, typename... CtorArgs, typename = std::enable_if_t, RootingContext>> explicit Rooted(const RootingContext& cx, CtorArgs... args) - : ptr(std::forward(args)...) { + : ptr(std::in_place, std::forward(args)...) { MOZ_ASSERT(GCPolicy::isValid(ptr)); registerWithRootLists(rootLists(cx)); } ~Rooted() { - MOZ_ASSERT(*this->stack == this); - *this->stack = this->prev; + MOZ_ASSERT(*stack == + reinterpret_cast*>(this)); + *stack = prev; } + Rooted* previous() { return reinterpret_cast*>(prev); } + /* * This method is public for Rooted so that Codegen.py can use a Rooted * interchangeably with a MutableHandleValue. @@ -1189,11 +1193,21 @@ class MOZ_RAII Rooted : public detail::RootedPtrTraits::StackRootedBase, T& get() { return ptr; } const T& get() const { return ptr; } - T* address() { return &ptr; } - const T* address() const { return &ptr; } + T* address() { return PtrTraits::address(ptr); } + const T* address() const { return PtrTraits::address(ptr); } + + void trace(JSTracer* trc, const char* name); private: - T ptr; + /* + * These need to be templated on RootListEntry* to avoid aliasing issues + * between, for example, Rooted and Rooted, which use + * the same stack head pointer for different classes. + */ + Rooted** stack; + Rooted* prev; + + Ptr ptr; Rooted(const Rooted&) = delete; } JS_HAZ_ROOTED; @@ -1253,7 +1267,7 @@ inline ProfilingStack* GetContextProfilingStackIfEnabled(JSContext* cx) { * Handle h = rooted; */ template -class RootedOperations +class RootedBase : public MutableWrappedPtrOperations { public: template @@ -1271,7 +1285,7 @@ class RootedOperations * Handle h = rooted; */ template -class HandleOperations +class HandleBase : public WrappedPtrOperations { public: template @@ -1320,11 +1334,13 @@ inline MutableHandle::MutableHandle(PersistentRooted* root) { ptr = root->address(); } -JS_PUBLIC_API void AddPersistentRoot(RootingContext* cx, RootKind kind, - js::PersistentRootedBase* root); +JS_PUBLIC_API void AddPersistentRoot( + RootingContext* cx, RootKind kind, + PersistentRooted* root); -JS_PUBLIC_API void AddPersistentRoot(JSRuntime* rt, RootKind kind, - js::PersistentRootedBase* root); +JS_PUBLIC_API void AddPersistentRoot( + JSRuntime* rt, RootKind kind, + PersistentRooted* root); /** * A copyable, assignable global GC root type with arbitrary lifetime, an @@ -1361,20 +1377,29 @@ JS_PUBLIC_API void AddPersistentRoot(JSRuntime* rt, RootKind kind, */ template class PersistentRooted - : public detail::RootedPtrTraits::PersistentRootedBase, - public js::RootedOperations> { + : public js::RootedBase>, + private mozilla::LinkedListElement> { + using ListBase = mozilla::LinkedListElement>; + using Ptr = detail::RootedPtr; using PtrTraits = detail::RootedPtrTraits; + friend class mozilla::LinkedList; + friend class mozilla::LinkedListElement; + void registerWithRootLists(RootingContext* cx) { MOZ_ASSERT(!initialized()); JS::RootKind kind = JS::MapTypeToRootKind::kind; - AddPersistentRoot(cx, kind, this); + AddPersistentRoot( + cx, kind, + reinterpret_cast*>(this)); } void registerWithRootLists(JSRuntime* rt) { MOZ_ASSERT(!initialized()); JS::RootKind kind = JS::MapTypeToRootKind::kind; - AddPersistentRoot(rt, kind, this); + AddPersistentRoot( + rt, kind, + reinterpret_cast*>(this)); } // Used when JSContext type is incomplete and so it is not known to inherit @@ -1406,11 +1431,12 @@ class PersistentRooted template , RootHolder>> explicit PersistentRooted(const RootHolder& cx, CtorArgs... args) - : ptr(std::forward(args)...) { + : ptr(std::in_place, std::forward(args)...) { registerWithRootLists(cx); } - PersistentRooted(const PersistentRooted& rhs) : ptr(rhs.ptr) { + PersistentRooted(const PersistentRooted& rhs) + : mozilla::LinkedListElement>(), ptr(rhs.ptr) { /* * Copy construction takes advantage of the fact that the original * is already inserted, and simply adds itself to whatever list the @@ -1422,7 +1448,7 @@ class PersistentRooted const_cast(rhs).setNext(this); } - bool initialized() const { return this->isInList(); } + bool initialized() const { return ListBase::isInList(); } void init(RootingContext* cx) { init(cx, SafelyInitialized()); } void init(JSContext* cx) { init(RootingContext::get(cx)); } @@ -1441,7 +1467,7 @@ class PersistentRooted void reset() { if (initialized()) { set(SafelyInitialized()); - this->remove(); + ListBase::remove(); } } @@ -1453,9 +1479,9 @@ class PersistentRooted T* address() { MOZ_ASSERT(initialized()); - return &ptr; + return PtrTraits::address(ptr); } - const T* address() const { return &ptr; } + const T* address() const { return PtrTraits::address(ptr); } template void set(U&& value) { @@ -1463,8 +1489,10 @@ class PersistentRooted ptr = std::forward(value); } + void trace(JSTracer* trc, const char* name); + private: - T ptr; + Ptr ptr; } JS_HAZ_ROOTED; namespace detail { diff --git a/js/public/Value.h b/js/public/Value.h index fa6d8f6123e2..da31c42afbd1 100644 --- a/js/public/Value.h +++ b/js/public/Value.h @@ -1217,7 +1217,7 @@ class MutableWrappedPtrOperations * type-querying, value-extracting, and mutating operations. */ template -class HeapOperations +class HeapBase : public MutableWrappedPtrOperations {}; MOZ_HAVE_NORETURN MOZ_COLD MOZ_NEVER_INLINE void ReportBadValueTypeAndCrash( diff --git a/js/src/gc/MaybeRooted.h b/js/src/gc/MaybeRooted.h index a6518159f424..7fd149877714 100644 --- a/js/src/gc/MaybeRooted.h +++ b/js/src/gc/MaybeRooted.h @@ -28,7 +28,7 @@ namespace js { * memory. */ template -class MOZ_RAII FakeRooted : public RootedOperations> { +class MOZ_RAII FakeRooted : public RootedBase> { public: using ElementType = T; @@ -72,7 +72,7 @@ namespace js { */ template class FakeMutableHandle - : public js::MutableHandleOperations> { + : public js::MutableHandleBase> { public: using ElementType = T; diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index bcb6826b3b9e..68a93718b722 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -40,30 +40,47 @@ using RootRange = RootedValueMap::Range; using RootEntry = RootedValueMap::Entry; using RootEnum = RootedValueMap::Enum; -template -inline void TypedRootedGCThingBase::trace(JSTracer* trc, - const char* name) { - auto* self = this->template derived(); - TraceNullableRoot(trc, self->address(), name); +// For more detail see JS::Rooted::root and js::RootedTraceable. +// +// The JS::RootKind::Traceable list contains a bunch of totally disparate types, +// but to refer to this list we need /something/ in the type field. We use the +// following type as a compatible stand-in. No actual methods from +// ConcreteTraceable type are actually used at runtime. +struct ConcreteTraceable { + ConcreteTraceable() = delete; + void trace(JSTracer*) = delete; +}; + +template +inline void RootedGCThingTraits::trace(JSTracer* trc, T* thingp, + const char* name) { + TraceNullableRoot(trc, thingp, name); } template -static inline void TraceExactStackRootList(JSTracer* trc, - StackRootedBase* listHead, - const char* name) { - // Check size of Rooted does not increase. - static_assert(sizeof(Rooted) == sizeof(T) + 2 * sizeof(uintptr_t)); - - for (StackRootedBase* root = listHead; root; root = root->previous()) { - static_cast*>(root)->trace(trc, name); - } +inline void RootedTraceableTraits::trace(JSTracer* trc, + VirtualTraceable* thingp, + const char* name) { + thingp->trace(trc, name); } -static inline void TraceExactStackRootTraceableList(JSTracer* trc, - StackRootedBase* listHead, - const char* name) { - for (StackRootedBase* root = listHead; root; root = root->previous()) { - static_cast(root)->trace(trc, name); +template +inline void JS::Rooted::trace(JSTracer* trc, const char* name) { + PtrTraits::trace(trc, &ptr, name); +} + +template +inline void JS::PersistentRooted::trace(JSTracer* trc, const char* name) { + PtrTraits::trace(trc, &ptr, name); +} + +template +static inline void TraceExactStackRootList( + JSTracer* trc, JS::Rooted* listHead, + const char* name) { + auto* typedList = reinterpret_cast*>(listHead); + for (JS::Rooted* root = typedList; root; root = root->previous()) { + root->trace(trc, name); } } @@ -81,8 +98,8 @@ static inline void TraceStackRoots(JSTracer* trc, // RootedTraceable uses virtual dispatch. JS::AutoSuppressGCAnalysis nogc; - TraceExactStackRootTraceableList(trc, stackRoots[JS::RootKind::Traceable], - "Traceable"); + TraceExactStackRootList( + trc, stackRoots[JS::RootKind::Traceable], "Traceable"); } void JS::RootingContext::traceStackRoots(JSTracer* trc) { @@ -95,16 +112,12 @@ static void TraceExactStackRoots(JSContext* cx, JSTracer* trc) { template static inline void TracePersistentRootedList( - JSTracer* trc, LinkedList& list, const char* name) { - for (PersistentRootedBase* root : list) { - static_cast*>(root)->trace(trc, name); - } -} - -static inline void TracePersistentRootedTraceableList( - JSTracer* trc, LinkedList& list, const char* name) { - for (PersistentRootedBase* root : list) { - static_cast(root)->trace(trc, name); + JSTracer* trc, + LinkedList>& list, + const char* name) { + auto& typedList = reinterpret_cast>&>(list); + for (PersistentRooted* root : typedList) { + root->trace(trc, name); } } @@ -122,7 +135,7 @@ void JSRuntime::tracePersistentRoots(JSTracer* trc) { // RootedTraceable uses virtual dispatch. JS::AutoSuppressGCAnalysis nogc; - TracePersistentRootedTraceableList( + TracePersistentRootedList( trc, heapRoots.ref()[JS::RootKind::Traceable], "persistent-traceable"); } @@ -132,9 +145,10 @@ static void TracePersistentRooted(JSRuntime* rt, JSTracer* trc) { template static void FinishPersistentRootedChain( - LinkedList& list) { + LinkedList>& listArg) { + auto& list = reinterpret_cast>&>(listArg); while (!list.isEmpty()) { - static_cast*>(list.getFirst())->reset(); + list.getFirst()->reset(); } } @@ -454,13 +468,15 @@ void js::gc::GCRuntime::checkNoRuntimeRoots(AutoGCSession& session) { #endif // DEBUG } -JS_PUBLIC_API void JS::AddPersistentRoot(JS::RootingContext* cx, RootKind kind, - PersistentRootedBase* root) { - JSRuntime* rt = static_cast(cx)->runtime(); - rt->heapRoots.ref()[kind].insertBack(root); +JS_PUBLIC_API void JS::AddPersistentRoot( + JS::RootingContext* cx, RootKind kind, + PersistentRooted* root) { + static_cast(cx)->runtime()->heapRoots.ref()[kind].insertBack( + root); } -JS_PUBLIC_API void JS::AddPersistentRoot(JSRuntime* rt, RootKind kind, - PersistentRootedBase* root) { +JS_PUBLIC_API void JS::AddPersistentRoot( + JSRuntime* rt, RootKind kind, + PersistentRooted* root) { rt->heapRoots.ref()[kind].insertBack(root); } diff --git a/js/src/jsapi-tests/testGCExactRooting.cpp b/js/src/jsapi-tests/testGCExactRooting.cpp index 7215b697f98d..9623099d2b1e 100644 --- a/js/src/jsapi-tests/testGCExactRooting.cpp +++ b/js/src/jsapi-tests/testGCExactRooting.cpp @@ -123,7 +123,7 @@ BEGIN_TEST(testGCRootedStaticStructInternalStackStorageAugmented) { JS::Rooted rv(cx); - CHECK_EQUAL(r1.constructor(), 1); // direct SafelyInitialized + CHECK_EQUAL(r1.constructor(), 101); // copy of SafelyInitialized CHECK_EQUAL(r2.constructor(), 2); // direct MyContainer(3.4) CHECK_EQUAL(r3.constructor(), 103); // copy of MyContainer(cx) CHECK_EQUAL(r4.constructor(), 3); // direct MyContainer(cx) @@ -169,7 +169,7 @@ BEGIN_TEST(testGCRootedStaticStructInternalStackStorageAugmented) { JS::PersistentRooted cp3(cx, cx); JS::PersistentRooted cp4(cx, cx, cx, cx); - CHECK_EQUAL(cp1.constructor(), 1); // direct SafelyInitialized + CHECK_EQUAL(cp1.constructor(), 101); // copy of SafelyInitialized CHECK_EQUAL(cp2.constructor(), 2); // direct MyContainer(double) CHECK_EQUAL(cp3.constructor(), 3); // direct MyContainer(cx) CHECK_EQUAL(cp4.constructor(), 4); // direct MyContainer(cx, cx, cx) diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index b8da3b6714b3..4e9b2bd5e07e 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1793,7 +1793,7 @@ static MOZ_ALWAYS_INLINE void InitElemArrayOperation(JSContext* cx, */ template -class ReservedRooted : public RootedOperations> { +class ReservedRooted : public RootedBase> { Rooted* savedRoot; public: diff --git a/js/src/vm/JSObject.h b/js/src/vm/JSObject.h index 83c5bfc92937..fa79b203c2fb 100644 --- a/js/src/vm/JSObject.h +++ b/js/src/vm/JSObject.h @@ -564,7 +564,7 @@ inline bool JSObject::is() const { template template -MOZ_ALWAYS_INLINE JS::Handle js::RootedOperations::as() +MOZ_ALWAYS_INLINE JS::Handle js::RootedBase::as() const { const Wrapper& self = *static_cast(this); MOZ_ASSERT(self->template is()); @@ -574,7 +574,7 @@ MOZ_ALWAYS_INLINE JS::Handle js::RootedOperations::as() template template -MOZ_ALWAYS_INLINE JS::Handle js::HandleOperations::as() +MOZ_ALWAYS_INLINE JS::Handle js::HandleBase::as() const { const JS::Handle& self = *static_cast*>(this); diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index e8f320ac409c..6ddc629d0f67 100644 --- a/js/src/vm/Runtime.h +++ b/js/src/vm/Runtime.h @@ -432,9 +432,9 @@ struct JSRuntime { js::GeckoProfilerRuntime& geckoProfiler() { return geckoProfiler_.ref(); } // Heap GC roots for PersistentRooted pointers. - js::MainThreadData< - mozilla::EnumeratedArray>> + js::MainThreadData>>> heapRoots; void tracePersistentRoots(JSTracer* trc);