diff --git a/dom/plugins/base/nsJSNPRuntime.cpp b/dom/plugins/base/nsJSNPRuntime.cpp index 1d42c18d64..7a118cae35 100644 --- a/dom/plugins/base/nsJSNPRuntime.cpp +++ b/dom/plugins/base/nsJSNPRuntime.cpp @@ -1747,7 +1747,7 @@ NPObjWrapper_ObjectMoved(JSObject *obj, const JSObject *old) auto entry = static_cast(sNPObjWrappers->Search(npobj)); MOZ_ASSERT(entry && entry->mJSObj); - MOZ_ASSERT(entry->mJSObj.unbarrieredGetPtr() == old); + MOZ_ASSERT(entry->mJSObj == old); entry->mJSObj = obj; } diff --git a/js/ipc/JavaScriptShared.cpp b/js/ipc/JavaScriptShared.cpp index 6ec339e493..6dfb00bc5e 100644 --- a/js/ipc/JavaScriptShared.cpp +++ b/js/ipc/JavaScriptShared.cpp @@ -100,7 +100,7 @@ IdToObjectMap::has(const ObjectId& id, const JSObject* obj) const auto p = table_.lookup(id); if (!p) return false; - return p->value().unbarrieredGet() == obj; + return p->value() == obj; } #endif diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index 120e1fcde0..aec25b0ffe 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -149,10 +149,6 @@ template struct PersistentRootedMarker; } /* namespace gc */ -#define DECLARE_POINTER_COMPARISON_OPS(T) \ - bool operator==(const T& other) const { return get() == other; } \ - bool operator!=(const T& other) const { return get() != other; } - // Important: Return a reference so passing a Rooted, etc. to // something that takes a |const T&| is not a GC hazard. #define DECLARE_POINTER_CONSTREF_OPS(T) \ @@ -242,6 +238,8 @@ class MOZ_NON_MEMMOVABLE Heap : public js::HeapBase> static_assert(js::IsHeapConstructibleType::value, "Type T must be a public GC pointer type"); public: + using ElementType = T; + Heap() { static_assert(sizeof(T) == sizeof(Heap), "Heap must be binary compatible with T."); @@ -376,6 +374,8 @@ template class TenuredHeap : public js::HeapBase> { public: + using ElementType = T; + TenuredHeap() : bits(0) { static_assert(sizeof(T) == sizeof(TenuredHeap), "TenuredHeap must be binary compatible with T."); @@ -383,9 +383,6 @@ class TenuredHeap : public js::HeapBase> explicit TenuredHeap(T p) : bits(0) { setPtr(p); } explicit TenuredHeap(const TenuredHeap& p) : bits(0) { setPtr(p.getPtr()); } - bool operator==(const TenuredHeap& other) { return bits == other.bits; } - bool operator!=(const TenuredHeap& other) { return bits != other.bits; } - void setPtr(T newPtr) { MOZ_ASSERT((reinterpret_cast(newPtr) & flagsMask) == 0); if (newPtr) @@ -462,6 +459,8 @@ class MOZ_NONHEAP_CLASS Handle : public js::HandleBase> friend class JS::MutableHandle; public: + using ElementType = T; + /* Creates a handle from a handle of a type convertible to T. */ template MOZ_IMPLICIT Handle(Handle handle, @@ -522,7 +521,6 @@ class MOZ_NONHEAP_CLASS Handle : public js::HandleBase> MOZ_IMPLICIT Handle(MutableHandle& root, typename mozilla::EnableIf::value, int>::Type dummy = 0); - DECLARE_POINTER_COMPARISON_OPS(T); DECLARE_POINTER_CONSTREF_OPS(T); DECLARE_NONPOINTER_ACCESSOR_METHODS(*ptr); @@ -549,6 +547,8 @@ template class MOZ_STACK_CLASS MutableHandle : public js::MutableHandleBase> { public: + using ElementType = T; + inline MOZ_IMPLICIT MutableHandle(Rooted* root); inline MOZ_IMPLICIT MutableHandle(PersistentRooted* root); @@ -781,6 +781,8 @@ class MOZ_RAII Rooted : public js::RootedBase> } public: + using ElementType = T; + template explicit Rooted(const RootingContext& cx) : ptr(GCPolicy::initial()) @@ -813,7 +815,6 @@ class MOZ_RAII Rooted : public js::RootedBase> ptr = mozilla::Move(value); } - DECLARE_POINTER_COMPARISON_OPS(T); DECLARE_POINTER_CONSTREF_OPS(T); DECLARE_POINTER_ASSIGN_OPS(Rooted, T); DECLARE_NONPOINTER_ACCESSOR_METHODS(ptr); @@ -890,13 +891,14 @@ template class MOZ_RAII FakeRooted : public RootedBase> { public: + using ElementType = T; + template explicit FakeRooted(CX* cx) : ptr(JS::GCPolicy::initial()) {} template FakeRooted(CX* cx, T initial) : ptr(initial) {} - DECLARE_POINTER_COMPARISON_OPS(T); DECLARE_POINTER_CONSTREF_OPS(T); DECLARE_POINTER_ASSIGN_OPS(FakeRooted, T); DECLARE_NONPOINTER_ACCESSOR_METHODS(ptr); @@ -917,6 +919,8 @@ template class FakeMutableHandle : public js::MutableHandleBase> { public: + using ElementType = T; + MOZ_IMPLICIT FakeMutableHandle(T* t) { ptr = t; } @@ -1107,6 +1111,8 @@ class PersistentRooted : public js::RootedBase>, js::RootLists& rootLists(js::ContextFriendFields* cx) = delete; public: + using ElementType = T; + PersistentRooted() : ptr(GCPolicy::initial()) {} template @@ -1160,7 +1166,6 @@ class PersistentRooted : public js::RootedBase>, } } - DECLARE_POINTER_COMPARISON_OPS(T); DECLARE_POINTER_CONSTREF_OPS(T); DECLARE_POINTER_ASSIGN_OPS(PersistentRooted, T); DECLARE_NONPOINTER_ACCESSOR_METHODS(ptr); @@ -1197,6 +1202,8 @@ class JS_PUBLIC_API(ObjectPtr) Heap value; public: + using ElementType = JSObject*; + ObjectPtr() : value(nullptr) {} explicit ObjectPtr(JSObject* obj) : value(obj) {} @@ -1311,6 +1318,177 @@ Swap(JS::TenuredHeap& aX, JS::TenuredHeap& aY) } /* namespace mozilla */ +namespace js { +namespace detail { + +// DefineComparisonOps is a trait which selects which wrapper classes to define +// operator== and operator!= for. It supplies a getter function to extract the +// value to compare. This is used to avoid triggering the automatic read +// barriers where appropriate. +// +// If DefineComparisonOps is not specialized for a particular wrapper you may +// get errors such as 'invalid operands to binary expression' or 'no match for +// operator==' when trying to compare against instances of the wrapper. + +template +struct DefineComparisonOps : mozilla::FalseType {}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const JS::Heap& v) { return v.unbarrieredGet(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T get(const JS::TenuredHeap& v) { return v.unbarrieredGetPtr(); } +}; + +template <> +struct DefineComparisonOps : mozilla::TrueType { + static const JSObject* get(const JS::ObjectPtr& v) { return v.unbarrieredGet(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const JS::Rooted& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const JS::Handle& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const JS::MutableHandle& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const JS::PersistentRooted& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const js::FakeRooted& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const js::FakeMutableHandle& v) { return v.get(); } +}; + +} /* namespace detail */ +} /* namespace js */ + +// Overload operator== and operator!= for all types with the DefineComparisonOps +// trait using the supplied getter. +// +// There are four cases: + +// Case 1: comparison between two wrapper objects. + +template +typename mozilla::EnableIf::value && + js::detail::DefineComparisonOps::value, bool>::Type +operator==(const T& a, const U& b) { + return js::detail::DefineComparisonOps::get(a) == js::detail::DefineComparisonOps::get(b); +} + +template +typename mozilla::EnableIf::value && + js::detail::DefineComparisonOps::value, bool>::Type +operator!=(const T& a, const U& b) { + return !(a == b); +} + +// Case 2: comparison between a wrapper object and its unwrapped element type. + +template +typename mozilla::EnableIf::value, bool>::Type +operator==(const T& a, const typename T::ElementType& b) { + return js::detail::DefineComparisonOps::get(a) == b; +} + +template +typename mozilla::EnableIf::value, bool>::Type +operator!=(const T& a, const typename T::ElementType& b) { + return !(a == b); +} + +template +typename mozilla::EnableIf::value, bool>::Type +operator==(const typename T::ElementType& a, const T& b) { + return a == js::detail::DefineComparisonOps::get(b); +} + +template +typename mozilla::EnableIf::value, bool>::Type +operator!=(const typename T::ElementType& a, const T& b) { + return !(a == b); +} + +// Case 3: For pointer wrappers, comparison between the wrapper and a const +// element pointer. + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator==(const typename mozilla::RemovePointer::Type* a, const T& b) { + return a == js::detail::DefineComparisonOps::get(b); +} + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator!=(const typename mozilla::RemovePointer::Type* a, const T& b) { + return !(a == b); +} + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator==(const T& a, const typename mozilla::RemovePointer::Type* b) { + return js::detail::DefineComparisonOps::get(a) == b; +} + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator!=(const T& a, const typename mozilla::RemovePointer::Type* b) { + return !(a == b); +} + +// Case 4: For pointer wrappers, comparison between the wrapper and nullptr. + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator==(std::nullptr_t a, const T& b) { + return a == js::detail::DefineComparisonOps::get(b); +} + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator!=(std::nullptr_t a, const T& b) { + return !(a == b); +} + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator==(const T& a, std::nullptr_t b) { + return js::detail::DefineComparisonOps::get(a) == b; +} + +template +typename mozilla::EnableIf::value && + mozilla::IsPointer::value, bool>::Type +operator!=(const T& a, std::nullptr_t b) { + return !(a == b); +} + #undef DELETE_ASSIGNMENT_OPS #endif /* js_RootingAPI_h */ diff --git a/js/src/gc/Barrier.h b/js/src/gc/Barrier.h index 25f153207f..1ec269abf3 100644 --- a/js/src/gc/Barrier.h +++ b/js/src/gc/Barrier.h @@ -356,7 +356,8 @@ class WriteBarrieredBase : public BarrieredBase, explicit WriteBarrieredBase(const T& v) : BarrieredBase(v) {} public: - DECLARE_POINTER_COMPARISON_OPS(T); + using ElementType = T; + DECLARE_POINTER_CONSTREF_OPS(T); // Use this if the automatic coercion to T isn't working. @@ -611,14 +612,13 @@ class ReadBarriered : public ReadBarrieredBase, return *this; } - const T get() const { - if (!InternalBarrierMethods::isMarkable(this->value)) - return JS::GCPolicy::initial(); - this->read(); + const T& get() const { + if (InternalBarrierMethods::isMarkable(this->value)) + this->read(); return this->value; } - const T unbarrieredGet() const { + const T& unbarrieredGet() const { return this->value; } @@ -626,9 +626,9 @@ class ReadBarriered : public ReadBarrieredBase, return bool(this->value); } - operator const T() const { return get(); } + operator const T&() const { return get(); } - const T operator->() const { return get(); } + const T& operator->() const { return get(); } T* unsafeGet() { return &this->value; } T const* unsafeGet() const { return &this->value; } @@ -934,6 +934,36 @@ typedef ReadBarriered ReadBarrieredWasmTableObject; typedef ReadBarriered ReadBarrieredValue; +namespace detail { + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const PreBarriered& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const GCPtr& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const HeapPtr& v) { return v.get(); } +}; + +template +struct DefineComparisonOps> : mozilla::TrueType { + static const T& get(const ReadBarriered& v) { return v.unbarrieredGet(); } +}; + +template <> +struct DefineComparisonOps : mozilla::TrueType { + static const Value& get(const HeapSlot& v) { return v.get(); } +}; + +} /* namespace detail */ + + } /* namespace js */ #endif /* gc_Barrier_h */ diff --git a/js/src/jsapi-tests/testGCHeapPostBarriers.cpp b/js/src/jsapi-tests/testGCHeapPostBarriers.cpp index ccc9869724..2353f9ae46 100644 --- a/js/src/jsapi-tests/testGCHeapPostBarriers.cpp +++ b/js/src/jsapi-tests/testGCHeapPostBarriers.cpp @@ -4,6 +4,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "mozilla/TypeTraits.h" #include "mozilla/UniquePtr.h" #include "js/RootingAPI.h" @@ -146,3 +147,110 @@ TestHeapPostBarrierInitFailure() } END_TEST(testGCHeapPostBarriers) + +BEGIN_TEST(testUnbarrieredEquality) +{ + // Use ArrayBuffers because they have finalizers, which allows using them + // in ObjectPtr without awkward conversations about nursery allocatability. + JS::RootedObject robj(cx, JS_NewArrayBuffer(cx, 20)); + JS::RootedObject robj2(cx, JS_NewArrayBuffer(cx, 30)); + cx->gc.evictNursery(); // Need tenured objects + + // Need some bare pointers to compare against. + JSObject* obj = robj; + JSObject* obj2 = robj2; + const JSObject* constobj = robj; + const JSObject* constobj2 = robj2; + + // Make them gray. We will make sure they stay gray. (For most reads, the + // barrier will unmark gray.) + using namespace js::gc; + TenuredCell* cell = &obj->asTenured(); + TenuredCell* cell2 = &obj2->asTenured(); + cell->markIfUnmarked(GRAY); + cell2->markIfUnmarked(GRAY); + MOZ_ASSERT(cell->isMarked(GRAY)); + MOZ_ASSERT(cell2->isMarked(GRAY)); + + { + JS::Heap heap(obj); + JS::Heap heap2(obj2); + CHECK(TestWrapper(obj, obj2, heap, heap2)); + CHECK(TestWrapper(constobj, constobj2, heap, heap2)); + } + + { + JS::TenuredHeap heap(obj); + JS::TenuredHeap heap2(obj2); + CHECK(TestWrapper(obj, obj2, heap, heap2)); + CHECK(TestWrapper(constobj, constobj2, heap, heap2)); + } + + { + JS::ObjectPtr objptr(obj); + JS::ObjectPtr objptr2(obj2); + CHECK(TestWrapper(obj, obj2, objptr, objptr2)); + CHECK(TestWrapper(constobj, constobj2, objptr, objptr2)); + objptr.finalize(cx); + objptr2.finalize(cx); + } + // Sanity check that the barriers normally mark things black. + { + JS::Heap heap(obj); + JS::Heap heap2(obj2); + heap.get(); + heap2.get(); + CHECK(cell->isMarked(BLACK)); + CHECK(cell2->isMarked(BLACK)); + } + + return true; +} + +template +bool +TestWrapper(ObjectT obj, ObjectT obj2, WrapperT& wrapper, WrapperT& wrapper2) +{ + using namespace js::gc; + + const TenuredCell& cell = obj->asTenured(); + const TenuredCell& cell2 = obj2->asTenured(); + + int x = 0; + + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + x += obj == obj2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + x += obj == wrapper2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + x += wrapper == obj2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + x += wrapper == wrapper2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + + CHECK(x == 0); + + x += obj != obj2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + x += obj != wrapper2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + x += wrapper != obj2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + x += wrapper != wrapper2; + CHECK(cell.isMarked(GRAY)); + CHECK(cell2.isMarked(GRAY)); + + CHECK(x == 4); + + return true; +} + +END_TEST(testUnbarrieredEquality) diff --git a/js/src/vm/SharedMem.h b/js/src/vm/SharedMem.h index fe8dad304d..8e5bae492a 100644 --- a/js/src/vm/SharedMem.h +++ b/js/src/vm/SharedMem.h @@ -11,8 +11,8 @@ template class SharedMem { - static_assert(mozilla::IsPointer::value, - "SharedMem encapsulates pointer types"); + // static_assert(mozilla::IsPointer::value, + // "SharedMem encapsulates pointer types"); enum Sharedness { IsUnshared, diff --git a/js/xpconnect/src/XPCInlines.h b/js/xpconnect/src/XPCInlines.h index ad3d0c7849..e60fac824e 100644 --- a/js/xpconnect/src/XPCInlines.h +++ b/js/xpconnect/src/XPCInlines.h @@ -470,7 +470,7 @@ inline void XPCWrappedNativeTearOff::JSObjectMoved(JSObject* obj, const JSObject* old) { MOZ_ASSERT(!IsMarked()); - MOZ_ASSERT(mJSObject.unbarrieredGetPtr() == old); + MOZ_ASSERT(mJSObject == old); mJSObject = obj; } diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp index 96f825f05b..2e3419f28a 100644 --- a/js/xpconnect/src/XPCWrappedNative.cpp +++ b/js/xpconnect/src/XPCWrappedNative.cpp @@ -935,7 +935,7 @@ void XPCWrappedNative::FlatJSObjectMoved(JSObject* obj, const JSObject* old) { JS::AutoAssertGCCallback inCallback(obj); - MOZ_ASSERT(mFlatJSObject.unbarrieredGetPtr() == old); + MOZ_ASSERT(mFlatJSObject == old); nsWrapperCache* cache = nullptr; CallQueryInterface(mIdentity, &cache); diff --git a/js/xpconnect/src/XPCWrappedNativeProto.cpp b/js/xpconnect/src/XPCWrappedNativeProto.cpp index 6aa023432e..7515ce3f04 100644 --- a/js/xpconnect/src/XPCWrappedNativeProto.cpp +++ b/js/xpconnect/src/XPCWrappedNativeProto.cpp @@ -114,7 +114,7 @@ XPCWrappedNativeProto::CallPostCreatePrototype() void XPCWrappedNativeProto::JSProtoObjectFinalized(js::FreeOp* fop, JSObject* obj) { - MOZ_ASSERT(obj == mJSProtoObject.unbarrieredGet(), "huh?"); + MOZ_ASSERT(obj == mJSProtoObject, "huh?"); // Only remove this proto from the map if it is the one in the map. ClassInfo2WrappedNativeProtoMap* map = GetScope()->GetWrappedNativeProtoMap(); @@ -129,7 +129,7 @@ XPCWrappedNativeProto::JSProtoObjectFinalized(js::FreeOp* fop, JSObject* obj) void XPCWrappedNativeProto::JSProtoObjectMoved(JSObject* obj, const JSObject* old) { - MOZ_ASSERT(mJSProtoObject.unbarrieredGet() == old); + MOZ_ASSERT(mJSProtoObject == old); mJSProtoObject.init(obj); // Update without triggering barriers. }