From 80d7d43961c1a33752289248609a348c0eee489f Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 24 May 2022 06:51:11 +0000 Subject: [PATCH 01/13] Issue #1742 - Part 1: Refactor rooting base class templates Based on Bug 1325406 --- dom/bindings/RootedOwningNonNull.h | 15 +--- dom/bindings/RootedRefPtr.h | 13 +-- dom/xbl/nsXBLMaybeCompiled.h | 12 +-- js/public/GCHashTable.h | 76 ++++-------------- js/public/GCVariant.h | 36 ++------- js/public/GCVector.h | 36 ++------- js/public/RootingAPI.h | 83 +++++++++----------- js/public/SweepingAPI.h | 7 +- js/public/Value.h | 52 ++++-------- js/src/builtin/MapObject.h | 20 +++-- js/src/ds/TraceableFifo.h | 54 ++----------- js/src/gc/Barrier.h | 25 +++--- js/src/jsapi-tests/testGCExactRooting.cpp | 17 +--- js/src/jsapi.h | 44 ++++------- js/src/jscompartment.cpp | 5 +- js/src/jsobj.h | 14 ++-- js/src/vm/ArrayBufferObject.cpp | 4 +- js/src/vm/ArrayBufferObject.h | 17 +--- js/src/vm/Interpreter.cpp | 14 +--- js/src/vm/JSONParser.h | 9 ++- js/src/vm/SavedStacks.h | 51 ++++++------ js/src/vm/Scope.h | 96 +++++++---------------- js/src/vm/Shape.h | 33 +++----- js/src/vm/TaggedProto.h | 18 +---- 24 files changed, 239 insertions(+), 512 deletions(-) diff --git a/dom/bindings/RootedOwningNonNull.h b/dom/bindings/RootedOwningNonNull.h index 13424d1557..387a707344 100644 --- a/dom/bindings/RootedOwningNonNull.h +++ b/dom/bindings/RootedOwningNonNull.h @@ -50,21 +50,12 @@ struct GCPolicy> } // namespace JS namespace js { -template -struct RootedBase> +template +struct WrappedPtrOperations, Wrapper> { - typedef mozilla::OwningNonNull SmartPtrType; - - operator SmartPtrType& () const - { - auto& self = *static_cast*>(this); - return self.get(); - } - operator T& () const { - auto& self = *static_cast*>(this); - return self.get(); + return static_cast(this)->get(); } }; } // namespace js diff --git a/dom/bindings/RootedRefPtr.h b/dom/bindings/RootedRefPtr.h index 34f2da5c9f..2563d46493 100644 --- a/dom/bindings/RootedRefPtr.h +++ b/dom/bindings/RootedRefPtr.h @@ -38,19 +38,12 @@ struct GCPolicy> } // namespace JS namespace js { -template -struct RootedBase> +template +struct WrappedPtrOperations, Wrapper> { - operator RefPtr& () const - { - auto& self = *static_cast>*>(this); - return self.get(); - } - operator T*() const { - auto& self = *static_cast>*>(this); - return self.get(); + return static_cast(this)->get(); } }; } // namespace js diff --git a/dom/xbl/nsXBLMaybeCompiled.h b/dom/xbl/nsXBLMaybeCompiled.h index 4bd7f83960..d9d16fdfbc 100644 --- a/dom/xbl/nsXBLMaybeCompiled.h +++ b/dom/xbl/nsXBLMaybeCompiled.h @@ -126,15 +126,15 @@ struct IsHeapConstructibleType> static const bool value = true; }; -template -class HeapBase> +template +class HeapBase, Wrapper> { - const JS::Heap>& wrapper() const { - return *static_cast>*>(this); + const Wrapper& wrapper() const { + return *static_cast(this); } - JS::Heap>& wrapper() { - return *static_cast>*>(this); + Wrapper& wrapper() { + return *static_cast(this); } const nsXBLMaybeCompiled* extract() const { diff --git a/js/public/GCHashTable.h b/js/public/GCHashTable.h index 4de1c39a81..3e647ed100 100644 --- a/js/public/GCHashTable.h +++ b/js/public/GCHashTable.h @@ -133,13 +133,13 @@ class GCRekeyableHashMap : public JS::GCHashMap -class GCHashMapOperations +template +class WrappedPtrOperations, Wrapper> { using Map = JS::GCHashMap; using Lookup = typename Map::Lookup; - const Map& map() const { return static_cast(this)->get(); } + const Map& map() const { return static_cast(this)->get(); } public: using AddPtr = typename Map::AddPtr; @@ -162,18 +162,18 @@ class GCHashMapOperations } }; -template -class MutableGCHashMapOperations - : public GCHashMapOperations +template +class MutableWrappedPtrOperations, Wrapper> + : public WrappedPtrOperations, Wrapper> { using Map = JS::GCHashMap; using Lookup = typename Map::Lookup; - Map& map() { return static_cast(this)->get(); } + Map& map() { return static_cast(this)->get(); } public: using AddPtr = typename Map::AddPtr; - struct Enum : public Map::Enum { explicit Enum(Outer& o) : Map::Enum(o.map()) {} }; + struct Enum : public Map::Enum { explicit Enum(Wrapper& o) : Map::Enum(o.map()) {} }; using Ptr = typename Map::Ptr; using Range = typename Map::Range; @@ -210,26 +210,6 @@ class MutableGCHashMapOperations } }; -template -class RootedBase> - : public MutableGCHashMapOperations>, A,B,C,D,E> -{}; - -template -class MutableHandleBase> - : public MutableGCHashMapOperations>, A,B,C,D,E> -{}; - -template -class HandleBase> - : public GCHashMapOperations>, A,B,C,D,E> -{}; - -template -class WeakCacheBase> - : public MutableGCHashMapOperations>, A,B,C,D,E> -{}; - } // namespace js namespace JS { @@ -291,13 +271,13 @@ class GCHashSet : public js::HashSet namespace js { -template -class GCHashSetOperations +template +class WrappedPtrOperations, Wrapper> { using Set = JS::GCHashSet; using Lookup = typename Set::Lookup; - const Set& set() const { return static_cast(this)->get(); } + const Set& set() const { return static_cast(this)->get(); } public: using AddPtr = typename Set::AddPtr; @@ -321,19 +301,19 @@ class GCHashSetOperations } }; -template -class MutableGCHashSetOperations - : public GCHashSetOperations +template +class MutableWrappedPtrOperations, Wrapper> + : public WrappedPtrOperations, Wrapper> { using Set = JS::GCHashSet; using Lookup = typename Set::Lookup; - Set& set() { return static_cast(this)->get(); } + Set& set() { return static_cast(this)->get(); } public: using AddPtr = typename Set::AddPtr; using Entry = typename Set::Entry; - struct Enum : public Set::Enum { explicit Enum(Outer& o) : Set::Enum(o.set()) {} }; + struct Enum : public Set::Enum { explicit Enum(Wrapper& o) : Set::Enum(o.set()) {} }; using Ptr = typename Set::Ptr; using Range = typename Set::Range; @@ -369,30 +349,6 @@ class MutableGCHashSetOperations } }; -template -class RootedBase> - : public MutableGCHashSetOperations>, T, HP, AP> -{ -}; - -template -class MutableHandleBase> - : public MutableGCHashSetOperations>, T, HP, AP> -{ -}; - -template -class HandleBase> - : public GCHashSetOperations>, T, HP, AP> -{ -}; - -template -class WeakCacheBase> - : public MutableGCHashSetOperations>, T, HP, AP> -{ -}; - } /* namespace js */ #endif /* GCHashTable_h */ diff --git a/js/public/GCVariant.h b/js/public/GCVariant.h index 43f2957fee..3ec40df5ca 100644 --- a/js/public/GCVariant.h +++ b/js/public/GCVariant.h @@ -123,13 +123,13 @@ struct GCPolicy> namespace js { -template -class GCVariantOperations +template +class WrappedPtrOperations, Wrapper> { using Impl = JS::detail::GCVariantImplementation; using Variant = mozilla::Variant; - const Variant& variant() const { return static_cast(this)->get(); } + const Variant& variant() const { return static_cast(this)->get(); } public: template @@ -149,15 +149,15 @@ class GCVariantOperations } }; -template -class MutableGCVariantOperations - : public GCVariantOperations +template +class MutableWrappedPtrOperations, Wrapper> + : public WrappedPtrOperations, Wrapper> { using Impl = JS::detail::GCVariantImplementation; using Variant = mozilla::Variant; - const Variant& variant() const { return static_cast(this)->get(); } - Variant& variant() { return static_cast(this)->get(); } + const Variant& variant() const { return static_cast(this)->get(); } + Variant& variant() { return static_cast(this)->get(); } public: template @@ -172,26 +172,6 @@ class MutableGCVariantOperations } }; -template -class RootedBase> - : public MutableGCVariantOperations>, Ts...> -{ }; - -template -class MutableHandleBase> - : public MutableGCVariantOperations>, Ts...> -{ }; - -template -class HandleBase> - : public GCVariantOperations>, Ts...> -{ }; - -template -class PersistentRootedBase> - : public MutableGCVariantOperations>, Ts...> -{ }; - } // namespace js #endif // js_GCVariant_h diff --git a/js/public/GCVector.h b/js/public/GCVector.h index 1c99252617..4acf0d1fc5 100644 --- a/js/public/GCVector.h +++ b/js/public/GCVector.h @@ -134,11 +134,11 @@ class GCVector namespace js { -template -class GCVectorOperations +template +class WrappedPtrOperations, Wrapper> { using Vec = JS::GCVector; - const Vec& vec() const { return static_cast(this)->get(); } + const Vec& vec() const { return static_cast(this)->get(); } public: const AllocPolicy& allocPolicy() const { return vec().allocPolicy(); } @@ -154,13 +154,13 @@ class GCVectorOperations } }; -template -class MutableGCVectorOperations - : public GCVectorOperations +template +class MutableWrappedPtrOperations, Wrapper> + : public WrappedPtrOperations, Wrapper> { using Vec = JS::GCVector; - const Vec& vec() const { return static_cast(this)->get(); } - Vec& vec() { return static_cast(this)->get(); } + const Vec& vec() const { return static_cast(this)->get(); } + Vec& vec() { return static_cast(this)->get(); } public: const AllocPolicy& allocPolicy() const { return vec().allocPolicy(); } @@ -223,26 +223,6 @@ class MutableGCVectorOperations void erase(T* aBegin, T* aEnd) { vec().erase(aBegin, aEnd); } }; -template -class RootedBase> - : public MutableGCVectorOperations>, T,N,AP> -{}; - -template -class MutableHandleBase> - : public MutableGCVectorOperations>, T,N,AP> -{}; - -template -class HandleBase> - : public GCVectorOperations>, T,N,AP> -{}; - -template -class PersistentRootedBase> - : public MutableGCVectorOperations>, T,N,AP> -{}; - } // namespace js #endif // js_GCVector_h diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index 5f0740d53a..120e1fcde0 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -112,17 +112,23 @@ template struct BarrierMethods { }; -template -class RootedBase {}; +template +class WrappedPtrOperations {}; -template -class HandleBase {}; +template +class MutableWrappedPtrOperations : public WrappedPtrOperations {}; -template -class MutableHandleBase {}; +template +class RootedBase : public MutableWrappedPtrOperations {}; -template -class HeapBase {}; +template +class HandleBase : public WrappedPtrOperations {}; + +template +class MutableHandleBase : public MutableWrappedPtrOperations {}; + +template +class HeapBase : public MutableWrappedPtrOperations {}; // Cannot use FOR_EACH_HEAP_ABLE_GC_POINTER_TYPE, as this would import too many macros into scope template struct IsHeapConstructibleType { static constexpr bool value = false; }; @@ -132,8 +138,8 @@ FOR_EACH_PUBLIC_GC_POINTER_TYPE(DECLARE_IS_HEAP_CONSTRUCTIBLE_TYPE) FOR_EACH_PUBLIC_TAGGED_GC_POINTER_TYPE(DECLARE_IS_HEAP_CONSTRUCTIBLE_TYPE) #undef DECLARE_IS_HEAP_CONSTRUCTIBLE_TYPE -template -class PersistentRootedBase {}; +template +class PersistentRootedBase : public MutableWrappedPtrOperations {}; static void* const ConstNullValue = nullptr; @@ -230,7 +236,7 @@ AssertGCThingIsNotAnObjectSubclass(js::gc::Cell* cell) {} * Type T must be a public GC pointer type. */ template -class MOZ_NON_MEMMOVABLE Heap : public js::HeapBase +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, @@ -367,7 +373,7 @@ ScriptIsMarkedGray(const Heap& script) * - It is not possible to store flag bits in a Heap. */ template -class TenuredHeap : public js::HeapBase +class TenuredHeap : public js::HeapBase> { public: TenuredHeap() : bits(0) { @@ -451,7 +457,7 @@ class TenuredHeap : public js::HeapBase * specialization, define a HandleBase specialization containing them. */ template -class MOZ_NONHEAP_CLASS Handle : public js::HandleBase +class MOZ_NONHEAP_CLASS Handle : public js::HandleBase> { friend class JS::MutableHandle; @@ -540,7 +546,7 @@ class MOZ_NONHEAP_CLASS Handle : public js::HandleBase * them. */ template -class MOZ_STACK_CLASS MutableHandle : public js::MutableHandleBase +class MOZ_STACK_CLASS MutableHandle : public js::MutableHandleBase> { public: inline MOZ_IMPLICIT MutableHandle(Rooted* root); @@ -753,7 +759,7 @@ namespace JS { * specialization, define a RootedBase specialization containing them. */ template -class MOZ_RAII Rooted : public js::RootedBase +class MOZ_RAII Rooted : public js::RootedBase> { inline void registerWithRootLists(js::RootedListHeads& roots) { this->stack = &roots[JS::MapTypeToRootKind::kind]; @@ -853,8 +859,8 @@ namespace js { * Rooted rooted(cx, &obj->as()); * Handle h = rooted; */ -template <> -class RootedBase +template +class RootedBase : public MutableWrappedPtrOperations { public: template @@ -871,8 +877,8 @@ class RootedBase * Rooted rooted(cx, &obj->as()); * Handle h = rooted; */ -template <> -class HandleBase +template +class HandleBase : public WrappedPtrOperations { public: template @@ -881,7 +887,7 @@ class HandleBase /** Interface substitute for Rooted which does not root the variable's memory. */ template -class MOZ_RAII FakeRooted : public RootedBase +class MOZ_RAII FakeRooted : public RootedBase> { public: template @@ -908,7 +914,7 @@ class MOZ_RAII FakeRooted : public RootedBase /** Interface substitute for MutableHandle which is not required to point to rooted memory. */ template -class FakeMutableHandle : public js::MutableHandleBase +class FakeMutableHandle : public js::MutableHandleBase> { public: MOZ_IMPLICIT FakeMutableHandle(T* t) { @@ -1075,7 +1081,7 @@ MutableHandle::MutableHandle(PersistentRooted* root) * marked when the object itself is marked. */ template -class PersistentRooted : public js::PersistentRootedBase, +class PersistentRooted : public js::RootedBase>, private mozilla::LinkedListElement> { using ListBase = mozilla::LinkedListElement>; @@ -1240,10 +1246,10 @@ class JS_PUBLIC_API(ObjectPtr) namespace js { -template -class UniquePtrOperations +template +class WrappedPtrOperations, Container> { - const UniquePtr& uniquePtr() const { return static_cast(this)->get(); } + const UniquePtr& uniquePtr() const { return static_cast(this)->get(); } public: explicit operator bool() const { return !!uniquePtr(); } @@ -1252,36 +1258,17 @@ class UniquePtrOperations T& operator*() const { return *uniquePtr(); } }; -template -class MutableUniquePtrOperations : public UniquePtrOperations +template +class MutableWrappedPtrOperations, Container> + : public WrappedPtrOperations, Container> { - UniquePtr& uniquePtr() { return static_cast(this)->get(); } + UniquePtr& uniquePtr() { return static_cast(this)->get(); } public: MOZ_MUST_USE typename UniquePtr::Pointer release() { return uniquePtr().release(); } void reset(T* ptr = T()) { uniquePtr().reset(ptr); } }; -template -class RootedBase> - : public MutableUniquePtrOperations>, T, D> -{ }; - -template -class MutableHandleBase> - : public MutableUniquePtrOperations>, T, D> -{ }; - -template -class HandleBase> - : public UniquePtrOperations>, T, D> -{ }; - -template -class PersistentRootedBase> - : public MutableUniquePtrOperations>, T, D> -{ }; - namespace gc { template diff --git a/js/public/SweepingAPI.h b/js/public/SweepingAPI.h index 39abb0fc0e..bbc1ec279b 100644 --- a/js/public/SweepingAPI.h +++ b/js/public/SweepingAPI.h @@ -8,11 +8,6 @@ #include "js/HeapAPI.h" -namespace js { -template -class WeakCacheBase {}; -} // namespace js - namespace JS { template class WeakCache; @@ -24,7 +19,7 @@ RegisterWeakCache(JS::Zone* zone, JS::WeakCache* cachep); // A WeakCache stores the given Sweepable container and links itself into a // list of such caches that are swept during each GC. template -class WeakCache : public js::WeakCacheBase, +class WeakCache : public js::MutableWrappedPtrOperations>, private mozilla::LinkedListElement> { friend class mozilla::LinkedListElement>; diff --git a/js/public/Value.h b/js/public/Value.h index 087d178efd..c645f07733 100644 --- a/js/public/Value.h +++ b/js/public/Value.h @@ -1340,20 +1340,18 @@ struct BarrierMethods } }; -template class MutableValueOperations; +template class MutableValueOperations; /** * A class designed for CRTP use in implementing the non-mutating parts of the - * Value interface in Value-like classes. Outer must be a class inheriting - * ValueOperations with a visible get() method returning a const - * reference to the Value abstracted by Outer. + * Value interface in Value-like classes. Wrapper must be a class inheriting + * ValueOperations with a visible get() method returning a const + * reference to the Value abstracted by Wrapper. */ -template -class ValueOperations +template +class WrappedPtrOperations { - friend class MutableValueOperations; - - const JS::Value& value() const { return static_cast(this)->get(); } + const JS::Value& value() const { return static_cast(this)->get(); } public: bool isUndefined() const { return value().isUndefined(); } @@ -1398,17 +1396,17 @@ class ValueOperations /** * A class designed for CRTP use in implementing all the mutating parts of the - * Value interface in Value-like classes. Outer must be a class inheriting - * MutableValueOperations with visible get() methods returning const and - * non-const references to the Value abstracted by Outer. + * Value interface in Value-like classes. Wrapper must be a class inheriting + * MutableWrappedPtrOperations with visible get() methods returning const and + * non-const references to the Value abstracted by Wrapper. */ -template -class MutableValueOperations : public ValueOperations +template +class MutableWrappedPtrOperations : public WrappedPtrOperations { protected: void set(const JS::Value& v) { // Call Outer::set to trigger any barriers. - static_cast(this)->set(v); + static_cast(this)->set(v); } public: @@ -1434,13 +1432,9 @@ class MutableValueOperations : public ValueOperations * Augment the generic Heap interface when T = Value with * type-querying, value-extracting, and mutating operations. */ -template <> -class HeapBase : public MutableValueOperations > +template +class HeapBase : public MutableWrappedPtrOperations { - typedef JS::Heap Outer; - - friend class ValueOperations; - public: void setNumber(uint32_t ui) { if (ui > JSVAL_INT_MAX) { @@ -1460,22 +1454,6 @@ class HeapBase : public MutableValueOperations > } }; -template <> -class HandleBase : public ValueOperations > -{}; - -template <> -class MutableHandleBase : public MutableValueOperations > -{}; - -template <> -class RootedBase : public MutableValueOperations > -{}; - -template <> -class PersistentRootedBase : public MutableValueOperations> -{}; - /* * If the Value is a GC pointer type, convert to that type and call |f| with * the pointer. If the Value is not a GC type, calls F::defaultValue. diff --git a/js/src/builtin/MapObject.h b/js/src/builtin/MapObject.h index 5382adfd25..a9f685ea00 100644 --- a/js/src/builtin/MapObject.h +++ b/js/src/builtin/MapObject.h @@ -52,14 +52,22 @@ class HashableValue } }; -template <> -class RootedBase { +template +class WrappedPtrOperations +{ + public: + Value value() const { + return static_cast(this)->get().get(); + } +}; + +template +class MutableWrappedPtrOperations + : public WrappedPtrOperations +{ public: MOZ_MUST_USE bool setValue(JSContext* cx, HandleValue v) { - return static_cast*>(this)->get().setValue(cx, v); - } - Value value() const { - return static_cast*>(this)->get().get(); + return static_cast(this)->get().setValue(cx, v); } }; diff --git a/js/src/ds/TraceableFifo.h b/js/src/ds/TraceableFifo.h index 04f67da151..0f94b93cbd 100644 --- a/js/src/ds/TraceableFifo.h +++ b/js/src/ds/TraceableFifo.h @@ -51,11 +51,11 @@ class TraceableFifo : public js::Fifo } }; -template -class TraceableFifoOperations +template +class WrappedPtrOperations, Wrapper> { using TF = TraceableFifo; - const TF& fifo() const { return static_cast(this)->extract(); } + const TF& fifo() const { return static_cast(this)->get(); } public: size_t length() const { return fifo().length(); } @@ -63,12 +63,12 @@ class TraceableFifoOperations const T& front() const { return fifo().front(); } }; -template -class MutableTraceableFifoOperations - : public TraceableFifoOperations +template +class MutableWrappedPtrOperations, Wrapper> + : public WrappedPtrOperations, Wrapper> { using TF = TraceableFifo; - TF& fifo() { return static_cast(this)->extract(); } + TF& fifo() { return static_cast(this)->get(); } public: T& front() { return fifo().front(); } @@ -82,46 +82,6 @@ class MutableTraceableFifoOperations void clear() { fifo().clear(); } }; -template -class RootedBase> - : public MutableTraceableFifoOperations>, A,B,C> -{ - using TF = TraceableFifo; - - friend class TraceableFifoOperations, A,B,C>; - const TF& extract() const { return *static_cast*>(this)->address(); } - - friend class MutableTraceableFifoOperations, A,B,C>; - TF& extract() { return *static_cast*>(this)->address(); } -}; - -template -class MutableHandleBase> - : public MutableTraceableFifoOperations>, A,B,C> -{ - using TF = TraceableFifo; - - friend class TraceableFifoOperations, A,B,C>; - const TF& extract() const { - return *static_cast*>(this)->address(); - } - - friend class MutableTraceableFifoOperations, A,B,C>; - TF& extract() { return *static_cast*>(this)->address(); } -}; - -template -class HandleBase> - : public TraceableFifoOperations>, A,B,C> -{ - using TF = TraceableFifo; - - friend class TraceableFifoOperations, A,B,C>; - const TF& extract() const { - return *static_cast*>(this)->address(); - } -}; - } // namespace js #endif // js_TraceableFifo_h diff --git a/js/src/gc/Barrier.h b/js/src/gc/Barrier.h index 681ccc9c4c..d10fb4321c 100644 --- a/js/src/gc/Barrier.h +++ b/js/src/gc/Barrier.h @@ -324,18 +324,12 @@ struct InternalBarrierMethods static void postBarrier(jsid* idp, jsid prev, jsid next) {} }; -// Barrier classes can use Mixins to add methods to a set of barrier -// instantiations, to make the barriered thing look and feel more like the -// thing itself. -template -class BarrieredBaseMixins {}; - // Base class of all barrier types. // // This is marked non-memmovable since post barriers added by derived classes // can add pointers to class instances to the store buffer. template -class MOZ_NON_MEMMOVABLE BarrieredBase : public BarrieredBaseMixins +class MOZ_NON_MEMMOVABLE BarrieredBase { protected: // BarrieredBase is not directly instantiable. @@ -356,9 +350,12 @@ class MOZ_NON_MEMMOVABLE BarrieredBase : public BarrieredBaseMixins // Base class for barriered pointer types that intercept only writes. template -class WriteBarrieredBase : public BarrieredBase +class WriteBarrieredBase : public BarrieredBase, + public WrappedPtrOperations> { protected: + using BarrieredBase::value; + // WriteBarrieredBase is not directly instantiable. explicit WriteBarrieredBase(const T& v) : BarrieredBase(v) {} @@ -580,8 +577,12 @@ class ReadBarrieredBase : public BarrieredBase // insert manual post-barriers on the table for rekeying if the key is based in // any way on the address of the object. template -class ReadBarriered : public ReadBarrieredBase +class ReadBarriered : public ReadBarrieredBase, + public WrappedPtrOperations> { + protected: + using ReadBarrieredBase::value; + public: ReadBarriered() : ReadBarrieredBase(JS::GCPolicy::initial()) {} @@ -649,12 +650,6 @@ class ReadBarriered : public ReadBarrieredBase template using WeakRef = ReadBarriered; -// Add Value operations to all Barrier types. Note, this must be defined before -// HeapSlot for HeapSlot's base to get these operations. -template <> -class BarrieredBaseMixins : public ValueOperations> -{}; - // A pre- and post-barriered Value that is specialized to be aware that it // resides in a slots or elements vector. This allows it to be relocated in // memory, but with substantially less overhead than a HeapPtr. diff --git a/js/src/jsapi-tests/testGCExactRooting.cpp b/js/src/jsapi-tests/testGCExactRooting.cpp index aff65014a9..6d41d374b5 100644 --- a/js/src/jsapi-tests/testGCExactRooting.cpp +++ b/js/src/jsapi-tests/testGCExactRooting.cpp @@ -56,19 +56,10 @@ struct MyContainer }; namespace js { -template <> -struct RootedBase { - HeapPtr& obj() { return static_cast*>(this)->get().obj; } - HeapPtr& str() { return static_cast*>(this)->get().str; } -}; -template <> -struct PersistentRootedBase { - HeapPtr& obj() { - return static_cast*>(this)->get().obj; - } - HeapPtr& str() { - return static_cast*>(this)->get().str; - } +template +struct MutableWrappedPtrOperations { + HeapPtr& obj() { return static_cast(this)->get().obj; } + HeapPtr& str() { return static_cast(this)->get().str; } }; } // namespace js diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 1eecdbf749..8e70cc152b 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2546,10 +2546,14 @@ struct JS_PUBLIC_API(PropertyDescriptor) { void trace(JSTracer* trc); }; -template -class PropertyDescriptorOperations +} // namespace JS + +namespace js { + +template +class WrappedPtrOperations { - const PropertyDescriptor& desc() const { return static_cast(this)->get(); } + const JS::PropertyDescriptor& desc() const { return static_cast(this)->get(); } bool has(unsigned bit) const { MOZ_ASSERT(bit != 0); @@ -2678,10 +2682,11 @@ class PropertyDescriptorOperations } }; -template -class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations +template +class MutableWrappedPtrOperations + : public js::WrappedPtrOperations { - PropertyDescriptor& desc() { return static_cast(this)->get(); } + JS::PropertyDescriptor& desc() { return static_cast(this)->get(); } public: void clear() { @@ -2692,7 +2697,7 @@ class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations< value().setUndefined(); } - void initFields(HandleObject obj, HandleValue v, unsigned attrs, + void initFields(JS::HandleObject obj, JS::HandleValue v, unsigned attrs, JSGetterOp getterOp, JSSetterOp setterOp) { MOZ_ASSERT(getterOp != JS_PropertyStub); MOZ_ASSERT(setterOp != JS_StrictPropertyStub); @@ -2704,7 +2709,7 @@ class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations< setSetter(setterOp); } - void assign(PropertyDescriptor& other) { + void assign(JS::PropertyDescriptor& other) { object().set(other.obj); setAttributes(other.attrs); setGetter(other.getter); @@ -2712,7 +2717,7 @@ class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations< value().set(other.value); } - void setDataDescriptor(HandleValue v, unsigned attrs) { + void setDataDescriptor(JS::HandleValue v, unsigned attrs) { MOZ_ASSERT((attrs & ~(JSPROP_ENUMERATE | JSPROP_PERMANENT | JSPROP_READONLY | @@ -2787,26 +2792,7 @@ class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations< } }; -} /* namespace JS */ - -namespace js { - -template <> -class RootedBase - : public JS::MutablePropertyDescriptorOperations> -{}; - -template <> -class HandleBase - : public JS::PropertyDescriptorOperations> -{}; - -template <> -class MutableHandleBase - : public JS::MutablePropertyDescriptorOperations> -{}; - -} /* namespace js */ +} // namespace js namespace JS { diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index ebf2d178fc..5e33f27f7d 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -722,8 +722,9 @@ JSCompartment::sweepAfterMinorGC(JSTracer* trc) { globalWriteBarriered = 0; - if (innerViews.needsSweepAfterMinorGC()) - innerViews.sweepAfterMinorGC(); + InnerViewTable& table = innerViews.get(); + if (table.needsSweepAfterMinorGC()) + table.sweepAfterMinorGC(); crossCompartmentWrappers.sweepAfterMinorGC(trc); } diff --git a/js/src/jsobj.h b/js/src/jsobj.h index ca48f8de7c..52be7e8188 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -587,21 +587,23 @@ class JSObject : public js::gc::Cell void operator=(const JSObject& other) = delete; }; -template +template +template MOZ_ALWAYS_INLINE JS::Handle -js::RootedBase::as() const +js::RootedBase::as() const { - const JS::Rooted& self = *static_cast*>(this); - MOZ_ASSERT(self->is()); + const Wrapper& self = *static_cast(this); + MOZ_ASSERT(self->template is()); return Handle::fromMarkedLocation(reinterpret_cast(self.address())); } +template template MOZ_ALWAYS_INLINE JS::Handle -js::HandleBase::as() const +js::HandleBase::as() const { const JS::Handle& self = *static_cast*>(this); - MOZ_ASSERT(self->is()); + MOZ_ASSERT(self->template is()); return Handle::fromMarkedLocation(reinterpret_cast(self.address())); } diff --git a/js/src/vm/ArrayBufferObject.cpp b/js/src/vm/ArrayBufferObject.cpp index 1de3d3b1ec..5d355ada9d 100644 --- a/js/src/vm/ArrayBufferObject.cpp +++ b/js/src/vm/ArrayBufferObject.cpp @@ -352,7 +352,7 @@ ArrayBufferObject::detach(JSContext* cx, Handle buffer, // Update all views of the buffer to account for the buffer having been // detached, and clear the buffer's data and list of views. - auto& innerViews = cx->compartment()->innerViews; + auto& innerViews = cx->compartment()->innerViews.get(); if (InnerViewTable::ViewVector* views = innerViews.maybeViewsUnbarriered(buffer)) { for (size_t i = 0; i < views->length(); i++) NoteViewBufferWasDetached((*views)[i], newContents, cx); @@ -427,7 +427,7 @@ ArrayBufferObject::changeContents(JSContext* cx, BufferContents newContents, setNewData(cx->runtime()->defaultFreeOp(), newContents, ownsState); // Update all views. - auto& innerViews = cx->compartment()->innerViews; + auto& innerViews = cx->compartment()->innerViews.get(); if (InnerViewTable::ViewVector* views = innerViews.maybeViewsUnbarriered(this)) { for (size_t i = 0; i < views->length(); i++) changeViewContents(cx, (*views)[i], oldDataPointer, newContents); diff --git a/js/src/vm/ArrayBufferObject.h b/js/src/vm/ArrayBufferObject.h index e6dbd30961..87dce34ba1 100644 --- a/js/src/vm/ArrayBufferObject.h +++ b/js/src/vm/ArrayBufferObject.h @@ -541,7 +541,6 @@ class InnerViewTable typedef Vector ViewVector; friend class ArrayBufferObject; - friend class WeakCacheBase; private: struct MapGCPolicy { @@ -602,23 +601,15 @@ class InnerViewTable size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf); }; -template <> -class WeakCacheBase +template +class MutableWrappedPtrOperations + : public WrappedPtrOperations { InnerViewTable& table() { - return static_cast*>(this)->get(); - } - const InnerViewTable& table() const { - return static_cast*>(this)->get(); + return static_cast(this)->get(); } public: - InnerViewTable::ViewVector* maybeViewsUnbarriered(ArrayBufferObject* obj) { - return table().maybeViewsUnbarriered(obj); - } - void removeViews(ArrayBufferObject* obj) { table().removeViews(obj); } - void sweepAfterMinorGC() { table().sweepAfterMinorGC(); } - bool needsSweepAfterMinorGC() const { return table().needsSweepAfterMinorGC(); } size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) { return table().sizeOfExcludingThis(mallocSizeOf); } diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 56f715e2be..7e3f2a7f1b 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1613,11 +1613,7 @@ GetSuperEnvFunction(JSContext* cx, InterpreterRegs& regs) */ template -class ReservedRootedBase { -}; - -template -class ReservedRooted : public ReservedRootedBase +class ReservedRooted : public RootedBase> { Rooted* savedRoot; @@ -1645,14 +1641,6 @@ class ReservedRooted : public ReservedRootedBase DECLARE_POINTER_ASSIGN_OPS(ReservedRooted, T) }; -template <> -class ReservedRootedBase : public ValueOperations> -{}; - -template <> -class ReservedRootedBase : public ScopeCastOperation> -{}; - static MOZ_NEVER_INLINE bool Interpret(JSContext* cx, RunState& state) { diff --git a/js/src/vm/JSONParser.h b/js/src/vm/JSONParser.h index 6f91391db0..70ed86f58e 100644 --- a/js/src/vm/JSONParser.h +++ b/js/src/vm/JSONParser.h @@ -255,10 +255,13 @@ class MOZ_STACK_CLASS JSONParser : public JSONParserBase void operator=(const JSONParser& other) = delete; }; -template -struct RootedBase> { +template +class MutableWrappedPtrOperations, Wrapper> + : public WrappedPtrOperations, Wrapper> +{ + public: bool parse(MutableHandleValue vp) { - return static_cast>*>(this)->get().parse(vp); + return static_cast(this)->get().parse(vp); } }; diff --git a/js/src/vm/SavedStacks.h b/js/src/vm/SavedStacks.h index c6bda2831b..3ea6c40874 100644 --- a/js/src/vm/SavedStacks.h +++ b/js/src/vm/SavedStacks.h @@ -265,24 +265,6 @@ class SavedStacks { uint32_t column; }; - template - struct LocationValueOperations { - JSAtom* source() const { return loc().source; } - size_t line() const { return loc().line; } - uint32_t column() const { return loc().column; } - private: - const LocationValue& loc() const { return static_cast(this)->get(); } - }; - - template - struct MutableLocationValueOperations : public LocationValueOperations { - void setSource(JSAtom* v) { loc().source = v; } - void setLine(size_t v) { loc().line = v; } - void setColumn(uint32_t v) { loc().column = v; } - private: - LocationValue& loc() { return static_cast(this)->get(); } - }; - private: struct PCLocationHasher : public DefaultHasher { using ScriptPtrHasher = DefaultHasher; @@ -313,15 +295,32 @@ class SavedStacks { MutableHandle locationp); }; -template <> -class RootedBase - : public SavedStacks::MutableLocationValueOperations> -{}; +template +struct WrappedPtrOperations +{ + JSAtom* source() const { return loc().source; } + size_t line() const { return loc().line; } + uint32_t column() const { return loc().column; } -template <> -class MutableHandleBase - : public SavedStacks::MutableLocationValueOperations> -{}; + private: + const SavedStacks::LocationValue& loc() const { + return static_cast(this)->get(); + } +}; + +template +struct MutableWrappedPtrOperations + : public WrappedPtrOperations +{ + void setSource(JSAtom* v) { loc().source = v; } + void setLine(size_t v) { loc().line = v; } + void setColumn(uint32_t v) { loc().column = v; } + + private: + SavedStacks::LocationValue& loc() { + return static_cast(this)->get(); + } +}; UTF8CharsZ BuildUTF8StackString(JSContext* cx, HandleObject stack); diff --git a/js/src/vm/Scope.h b/js/src/vm/Scope.h index 4f0edcc133..fc1419bb89 100644 --- a/js/src/vm/Scope.h +++ b/js/src/vm/Scope.h @@ -22,6 +22,7 @@ namespace js { class ModuleObject; +class Scope; enum class BindingKind : uint8_t { @@ -223,6 +224,21 @@ class BindingLocation } }; +// +// Allow using is and as on Rooted and Handle. +// +template +class WrappedPtrOperations +{ + public: + template + JS::Handle as() const { + const Wrapper& self = *static_cast(this); + MOZ_ASSERT_IF(self, self->template is()); + return Handle::fromMarkedLocation(reinterpret_cast(self.address())); + } +}; + // // The base class of all Scopes. // @@ -1338,10 +1354,10 @@ class MOZ_STACK_CLASS ScopeIter // Specializations of Rooted containers for the iterators. // -template -class BindingIterOperations +template +class WrappedPtrOperations { - const BindingIter& iter() const { return static_cast(this)->get(); } + const BindingIter& iter() const { return static_cast(this)->get(); } public: bool done() const { return iter().done(); } @@ -1361,19 +1377,20 @@ class BindingIterOperations uint32_t nextEnvironmentSlot() const { return iter().nextEnvironmentSlot(); } }; -template -class MutableBindingIterOperations : public BindingIterOperations +template +class MutableWrappedPtrOperations + : public WrappedPtrOperations { - BindingIter& iter() { return static_cast(this)->get(); } + BindingIter& iter() { return static_cast(this)->get(); } public: void operator++(int) { iter().operator++(1); } }; -template -class ScopeIterOperations +template +class WrappedPtrOperations { - const ScopeIter& iter() const { return static_cast(this)->get(); } + const ScopeIter& iter() const { return static_cast(this)->get(); } public: bool done() const { return iter().done(); } @@ -1384,69 +1401,16 @@ class ScopeIterOperations bool hasSyntacticEnvironment() const { return iter().hasSyntacticEnvironment(); } }; -template -class MutableScopeIterOperations : public ScopeIterOperations +template +class MutableWrappedPtrOperations + : public WrappedPtrOperations { - ScopeIter& iter() { return static_cast(this)->get(); } + ScopeIter& iter() { return static_cast(this)->get(); } public: void operator++(int) { iter().operator++(1); } }; -#define SPECIALIZE_ROOTING_CONTAINERS(Iter, BaseIter) \ - template <> \ - class RootedBase \ - : public Mutable##BaseIter##Operations> \ - { }; \ - \ - template <> \ - class MutableHandleBase \ - : public Mutable##BaseIter##Operations> \ - { }; \ - \ - template <> \ - class HandleBase \ - : public BaseIter##Operations> \ - { }; \ - \ - template <> \ - class PersistentRootedBase \ - : public Mutable##BaseIter##Operations> \ - { } - -SPECIALIZE_ROOTING_CONTAINERS(BindingIter, BindingIter); -SPECIALIZE_ROOTING_CONTAINERS(PositionalFormalParameterIter, BindingIter); -SPECIALIZE_ROOTING_CONTAINERS(ScopeIter, ScopeIter); - -#undef SPECIALIZE_ROOTING_CONTAINERS - -// -// Allow using is and as on Rooted and Handle. -// - -template -struct ScopeCastOperation -{ - template - JS::Handle as() const { - const Outer& self = *static_cast(this); - MOZ_ASSERT_IF(self, self->template is()); - return Handle::fromMarkedLocation(reinterpret_cast(self.address())); - } -}; - -template <> -class RootedBase : public ScopeCastOperation> -{ }; - -template <> -class HandleBase : public ScopeCastOperation> -{ }; - -template <> -class MutableHandleBase : public ScopeCastOperation> -{ }; - } // namespace js namespace JS { diff --git a/js/src/vm/Shape.h b/js/src/vm/Shape.h index 52b9197da6..b292bd58f2 100644 --- a/js/src/vm/Shape.h +++ b/js/src/vm/Shape.h @@ -1264,9 +1264,10 @@ struct InitialShapeEntry bool needsSweep() { Shape* ushape = shape.unbarrieredGet(); - JSObject* protoObj = proto.proto().raw(); + TaggedProto uproto = proto.proto().unbarrieredGet(); + JSObject* protoObj = uproto.raw(); return (gc::IsAboutToBeFinalizedUnbarriered(&ushape) || - (proto.proto().isObject() && gc::IsAboutToBeFinalizedUnbarriered(&protoObj))); + (uproto.isObject() && gc::IsAboutToBeFinalizedUnbarriered(&protoObj))); } }; @@ -1356,9 +1357,10 @@ struct StackShape void trace(JSTracer* trc); }; -template -class StackShapeOperations { - const StackShape& ss() const { return static_cast(this)->get(); } +template +class WrappedPtrOperations +{ + const StackShape& ss() const { return static_cast(this)->get(); } public: bool hasSlot() const { return ss().hasSlot(); } @@ -1370,9 +1372,11 @@ class StackShapeOperations { uint8_t attrs() const { return ss().attrs; } }; -template -class MutableStackShapeOperations : public StackShapeOperations { - StackShape& ss() { return static_cast(this)->get(); } +template +class MutableWrappedPtrOperations + : public WrappedPtrOperations +{ + StackShape& ss() { return static_cast(this)->get(); } public: void updateGetterSetter(GetterOp rawGetter, SetterOp rawSetter) { @@ -1383,19 +1387,6 @@ class MutableStackShapeOperations : public StackShapeOperations { void setAttrs(uint8_t attrs) { ss().attrs = attrs; } }; -template <> -class RootedBase : public MutableStackShapeOperations> -{}; - -template <> -class HandleBase : public StackShapeOperations> -{}; - -template <> -class MutableHandleBase - : public MutableStackShapeOperations> -{}; - inline Shape::Shape(const StackShape& other, uint32_t nfixed) : base_(other.base), diff --git a/js/src/vm/TaggedProto.h b/js/src/vm/TaggedProto.h index de3e86a7f5..5b9fe5c7e8 100644 --- a/js/src/vm/TaggedProto.h +++ b/js/src/vm/TaggedProto.h @@ -77,11 +77,11 @@ struct InternalBarrierMethods } }; -template -class TaggedProtoOperations +template +class WrappedPtrOperations { const TaggedProto& value() const { - return static_cast(this)->get(); + return static_cast(this)->get(); } public: @@ -95,18 +95,6 @@ class TaggedProtoOperations uint64_t uniqueId() const { return value().uniqueId(); } }; -template <> -class HandleBase : public TaggedProtoOperations> -{}; - -template <> -class RootedBase : public TaggedProtoOperations> -{}; - -template <> -class BarrieredBaseMixins : public TaggedProtoOperations> -{}; - // If the TaggedProto is a JSObject pointer, convert to that type and call |f| // with the pointer. If the TaggedProto is lazy, calls F::defaultValue. template From c48b8e8932819fd05c4e4584008e904a5beacc61 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 24 May 2022 21:15:12 +0000 Subject: [PATCH 02/13] Issue #1742 - Part 2: Add a class to encapsulate the possible results of a property lookup --- js/public/Class.h | 111 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/js/public/Class.h b/js/public/Class.h index 6f1960a308..55da324e15 100644 --- a/js/public/Class.h +++ b/js/public/Class.h @@ -271,8 +271,119 @@ class ObjectOpResult } }; +class PropertyResult +{ + union { + js::Shape* shape_; + uintptr_t bits_; + }; + + static const uintptr_t NotFound = 0; + static const uintptr_t NonNativeProperty = 1; + static const uintptr_t DenseOrTypedArrayElement = 1; + + public: + PropertyResult() : bits_(NotFound) {} + + explicit PropertyResult(js::Shape* propertyShape) + : shape_(propertyShape) + { + MOZ_ASSERT(!isFound() || isNativeProperty()); + } + + explicit operator bool() const { + return isFound(); + } + + bool isFound() const { + return bits_ != NotFound; + } + + bool isNonNativeProperty() const { + return bits_ == NonNativeProperty; + } + + bool isDenseOrTypedArrayElement() const { + return bits_ == DenseOrTypedArrayElement; + } + + bool isNativeProperty() const { + return isFound() && !isNonNativeProperty(); + } + + js::Shape* maybeShape() const { + MOZ_ASSERT(!isNonNativeProperty()); + return isFound() ? shape_ : nullptr; + } + + js::Shape* shape() const { + MOZ_ASSERT(isNativeProperty()); + return shape_; + } + + void setNotFound() { + bits_ = NotFound; + } + + void setNativeProperty(js::Shape* propertyShape) { + shape_ = propertyShape; + MOZ_ASSERT(isNativeProperty()); + } + + void setNonNativeProperty() { + bits_ = NonNativeProperty; + } + + void setDenseOrTypedArrayElement() { + bits_ = DenseOrTypedArrayElement; + } + + void trace(JSTracer* trc); +}; + } // namespace JS +namespace js { + +template +class WrappedPtrOperations +{ + const JS::PropertyResult& value() const { return static_cast(this)->get(); } + + public: + bool isFound() const { return value().isFound(); } + explicit operator bool() const { return bool(value()); } + js::Shape* maybeShape() const { return value().maybeShape(); } + js::Shape* shape() const { return value().shape(); } + bool isNativeProperty() const { return value().isNativeProperty(); } + bool isNonNativeProperty() const { return value().isNonNativeProperty(); } + bool isDenseOrTypedArrayElement() const { return value().isDenseOrTypedArrayElement(); } + js::Shape* asTaggedShape() const { return value().asTaggedShape(); } +}; + +template +class MutableWrappedPtrOperations + : public WrappedPtrOperations +{ + JS::PropertyResult& value() { return static_cast(this)->get(); } + + public: + void setNotFound() { + value().setNotFound(); + } + void setNativeProperty(js::Shape* shape) { + value().setNativeProperty(shape); + } + void setNonNativeProperty() { + value().setNonNativeProperty(); + } + void setDenseOrTypedArrayElement() { + value().setDenseOrTypedArrayElement(); + } +}; + +} // namespace js + // JSClass operation signatures. /** From 574b295d15ff7cd5a5084928c3625313e111281e Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 24 May 2022 21:19:51 +0000 Subject: [PATCH 03/13] Issue #1742 - Part 3: use JS::PropertyResult instead of Shape* This is the meat of the issue and switches using raw shape pointers out for PropertyResult objects where feasible. --- js/public/Class.h | 2 +- js/src/NamespaceImports.h | 2 + js/src/builtin/Object.cpp | 14 ++-- js/src/builtin/TypedObject.cpp | 6 +- js/src/builtin/TypedObject.h | 2 +- js/src/jit/BaselineIC.cpp | 40 ++++++--- js/src/jit/CacheIR.cpp | 6 +- js/src/jit/IonCaches.cpp | 93 ++++++++++++--------- js/src/jit/IonCaches.h | 4 +- js/src/jit/SharedIC.cpp | 15 ++-- js/src/jit/SharedIC.h | 2 +- js/src/jit/VMFunctions.cpp | 6 +- js/src/jsapi.cpp | 4 +- js/src/jsfriendapi.h | 2 +- js/src/jsobj.cpp | 104 +++++++++++------------ js/src/jsobj.h | 12 +-- js/src/jsobjinlines.h | 6 +- js/src/json.cpp | 4 +- js/src/proxy/Proxy.cpp | 6 +- js/src/vm/Debugger.cpp | 8 +- js/src/vm/EnvironmentObject.cpp | 12 +-- js/src/vm/EnvironmentObject.h | 2 +- js/src/vm/GeneratorObject.cpp | 10 ++- js/src/vm/Interpreter-inl.h | 32 ++++--- js/src/vm/Interpreter.cpp | 43 +++++----- js/src/vm/NativeObject-inl.h | 53 ++++++------ js/src/vm/NativeObject.cpp | 142 +++++++++++++++++--------------- js/src/vm/NativeObject.h | 2 +- js/src/vm/Shape-inl.h | 6 +- js/src/vm/Shape.cpp | 7 ++ js/src/vm/Shape.h | 32 ------- js/src/vm/UnboxedObject.cpp | 12 +-- js/src/vm/UnboxedObject.h | 4 +- 33 files changed, 367 insertions(+), 328 deletions(-) diff --git a/js/public/Class.h b/js/public/Class.h index 55da324e15..1dd03a1821 100644 --- a/js/public/Class.h +++ b/js/public/Class.h @@ -539,7 +539,7 @@ namespace js { typedef bool (* LookupPropertyOp)(JSContext* cx, JS::HandleObject obj, JS::HandleId id, - JS::MutableHandleObject objp, JS::MutableHandle propp); + JS::MutableHandleObject objp, JS::MutableHandle propp); typedef bool (* DefinePropertyOp)(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::Handle desc, diff --git a/js/src/NamespaceImports.h b/js/src/NamespaceImports.h index de8e3a6f24..2b7a1f0e8b 100644 --- a/js/src/NamespaceImports.h +++ b/js/src/NamespaceImports.h @@ -45,6 +45,7 @@ class MOZ_STACK_CLASS SourceBufferHolder; class HandleValueArray; class ObjectOpResult; +class PropertyResult; class Symbol; enum class SymbolCode: uint32_t; @@ -150,6 +151,7 @@ using JS::FalseHandleValue; using JS::HandleValueArray; using JS::ObjectOpResult; +using JS::PropertyResult; using JS::Zone; diff --git a/js/src/builtin/Object.cpp b/js/src/builtin/Object.cpp index 1221d2daf1..d661a222e5 100644 --- a/js/src/builtin/Object.cpp +++ b/js/src/builtin/Object.cpp @@ -69,18 +69,18 @@ js::obj_propertyIsEnumerable(JSContext* cx, unsigned argc, Value* vp) JSObject* obj = &args.thisv().toObject(); /* Step 3. */ - Shape* shape; + PropertyResult prop; if (obj->isNative() && - NativeLookupOwnProperty(cx, &obj->as(), id, &shape)) + NativeLookupOwnProperty(cx, &obj->as(), id, &prop)) { /* Step 4. */ - if (!shape) { + if (!prop) { args.rval().setBoolean(false); return true; } /* Step 5. */ - unsigned attrs = GetShapeAttributes(obj, shape); + unsigned attrs = GetPropertyAttributes(obj, prop); args.rval().setBoolean((attrs & JSPROP_ENUMERATE) != 0); return true; } @@ -582,11 +582,11 @@ js::obj_hasOwnProperty(JSContext* cx, unsigned argc, Value* vp) jsid id; if (args.thisv().isObject() && ValueToId(cx, idValue, &id)) { JSObject* obj = &args.thisv().toObject(); - Shape* prop; + PropertyResult prop; if (obj->isNative() && NativeLookupOwnProperty(cx, &obj->as(), id, &prop)) { - args.rval().setBoolean(!!prop); + args.rval().setBoolean(prop.isFound()); return true; } } @@ -839,7 +839,7 @@ EnumerableOwnProperties(JSContext* cx, const JS::CallArgs& args, EnumerableOwnPr value = nobj->getDenseOrTypedArrayElement(JSID_TO_INT(id)); } else { shape = nobj->lookup(cx, id); - if (!shape || !(GetShapeAttributes(nobj, shape) & JSPROP_ENUMERATE)) + if (!shape || !(shape->attributes() & JSPROP_ENUMERATE)) continue; if (!shape->isAccessorShape()) { if (!NativeGetExistingProperty(cx, nobj, nobj, shape, &value)) diff --git a/js/src/builtin/TypedObject.cpp b/js/src/builtin/TypedObject.cpp index 4c938568f0..2796848c02 100644 --- a/js/src/builtin/TypedObject.cpp +++ b/js/src/builtin/TypedObject.cpp @@ -1671,10 +1671,10 @@ TypeDescr::hasProperty(const JSAtomState& names, jsid id) /* static */ bool TypedObject::obj_lookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject objp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandle propp) { if (obj->as().typeDescr().hasProperty(cx->names(), id)) { - MarkNonNativePropertyFound(propp); + propp.setNonNativeProperty(); objp.set(obj); return true; } @@ -1682,7 +1682,7 @@ TypedObject::obj_lookupProperty(JSContext* cx, HandleObject obj, HandleId id, RootedObject proto(cx, obj->staticPrototype()); if (!proto) { objp.set(nullptr); - propp.set(nullptr); + propp.setNotFound(); return true; } diff --git a/js/src/builtin/TypedObject.h b/js/src/builtin/TypedObject.h index 4fcd30cb0b..83700001d4 100644 --- a/js/src/builtin/TypedObject.h +++ b/js/src/builtin/TypedObject.h @@ -509,7 +509,7 @@ class TypedObject : public ShapedObject static MOZ_MUST_USE bool obj_lookupProperty(JSContext* cx, HandleObject obj, HandleId id, MutableHandleObject objp, - MutableHandleShape propp); + MutableHandle propp); static MOZ_MUST_USE bool obj_defineProperty(JSContext* cx, HandleObject obj, HandleId id, Handle desc, diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp index f5dcd2a106..7dbe239a7e 100644 --- a/js/src/jit/BaselineIC.cpp +++ b/js/src/jit/BaselineIC.cpp @@ -1145,9 +1145,9 @@ TryAttachNativeOrUnboxedGetValueElemStub(JSContext* cx, HandleScript script, jsb return true; bool needsAtomize = checkAtomize(keyVal); - RootedShape shape(cx); + Rooted prop(cx); RootedObject holder(cx); - if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &shape)) + if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &prop)) return false; if (!holder || (holder != obj && !holder->isNative())) return true; @@ -1214,6 +1214,8 @@ TryAttachNativeOrUnboxedGetValueElemStub(JSContext* cx, HandleScript script, jsb if (!holder->isNative()) return true; + RootedShape shape(cx, prop.shape()); + if (IsCacheableGetPropReadSlot(obj, holder, shape)) { bool isFixedSlot; uint32_t offset; @@ -1264,13 +1266,14 @@ TryAttachNativeGetAccessorElemStub(JSContext* cx, HandleScript script, jsbytecod return true; bool needsAtomize = checkAtomize(keyVal); - RootedShape shape(cx); + Rooted prop(cx); RootedObject baseHolder(cx); - if (!EffectlesslyLookupProperty(cx, obj, id, &baseHolder, &shape)) + if (!EffectlesslyLookupProperty(cx, obj, id, &baseHolder, &prop)) return false; if (!baseHolder || !baseHolder->isNative()) return true; + RootedShape shape(cx, prop.shape()); HandleNativeObject holder = baseHolder.as(); bool getterIsScripted = false; @@ -3348,11 +3351,17 @@ TryAttachNativeInStub(JSContext* cx, HandleScript outerScript, ICIn_Fallback* st return true; RootedPropertyName name(cx, JSID_TO_ATOM(id)->asPropertyName()); - RootedShape shape(cx); + Rooted prop(cx); RootedObject holder(cx); - if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &shape)) + if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &prop)) return false; + if (prop.isNonNativeProperty()) { + MOZ_ASSERT(!IsCacheableProtoChain(obj, holder, false)); + return true; + } + + RootedShape shape(cx, prop.maybeShape()); if (IsCacheableGetPropReadSlot(obj, holder, shape)) { ICStub::Kind kind = (obj == holder) ? ICStub::In_Native : ICStub::In_NativePrototype; @@ -4259,14 +4268,17 @@ TryAttachSetValuePropStub(JSContext* cx, HandleScript script, jsbytecode* pc, IC { MOZ_ASSERT(!*attached); - RootedShape shape(cx); + Rooted prop(cx); RootedObject holder(cx); - if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &shape)) + if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &prop)) return false; if (obj != holder) return true; - if (!obj->isNative()) { + RootedShape shape(cx); + if (obj->isNative()) { + shape = prop.shape(); + } else { if (obj->is()) { UnboxedExpandoObject* expando = obj->as().maybeExpando(); if (expando) { @@ -4365,11 +4377,17 @@ TryAttachSetAccessorPropStub(JSContext* cx, HandleScript script, jsbytecode* pc, MOZ_ASSERT(!*attached); MOZ_ASSERT(!*isTemporarilyUnoptimizable); - RootedShape shape(cx); + Rooted prop(cx); RootedObject holder(cx); - if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &shape)) + if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &prop)) return false; + if (prop.isNonNativeProperty()) { + MOZ_ASSERT(!IsCacheableProtoChain(obj, holder)); + return true; + } + + RootedShape shape(cx, prop.maybeShape()); bool isScripted = false; bool cacheableCall = IsCacheableSetPropCall(cx, obj, holder, shape, &isScripted, isTemporarilyUnoptimizable); diff --git a/js/src/jit/CacheIR.cpp b/js/src/jit/CacheIR.cpp index fc58bdb98e..9168a344e5 100644 --- a/js/src/jit/CacheIR.cpp +++ b/js/src/jit/CacheIR.cpp @@ -109,7 +109,8 @@ CanAttachNativeGetProp(JSContext* cx, HandleObject obj, HandleId id, // only miss out on shape hashification, which is only a temporary perf cost. // The limits were arbitrarily set, anyways. JSObject* baseHolder = nullptr; - if (!LookupPropertyPure(cx, obj, id, &baseHolder, shape.address())) + PropertyResult prop; + if (!LookupPropertyPure(cx, obj, id, &baseHolder, &prop)) return CanAttachNone; MOZ_ASSERT(!holder); @@ -118,8 +119,9 @@ CanAttachNativeGetProp(JSContext* cx, HandleObject obj, HandleId id, return CanAttachNone; holder.set(&baseHolder->as()); } + shape.set(prop.maybeShape()); - if (IsCacheableGetPropReadSlotForIonOrCacheIR(obj, holder, shape) || + if (IsCacheableGetPropReadSlotForIonOrCacheIR(obj, holder, prop) || IsCacheableNoProperty(cx, obj, holder, shape, id, pc)) { return CanAttachReadSlot; diff --git a/js/src/jit/IonCaches.cpp b/js/src/jit/IonCaches.cpp index 0a0c7ac22e..5a7e437285 100644 --- a/js/src/jit/IonCaches.cpp +++ b/js/src/jit/IonCaches.cpp @@ -468,11 +468,12 @@ jit::IsCacheableProtoChainForIonOrCacheIR(JSObject* obj, JSObject* holder) } bool -jit::IsCacheableGetPropReadSlotForIonOrCacheIR(JSObject* obj, JSObject* holder, Shape* shape) +jit::IsCacheableGetPropReadSlotForIonOrCacheIR(JSObject* obj, JSObject* holder, PropertyResult prop) { - if (!shape || !IsCacheableProtoChainForIonOrCacheIR(obj, holder)) + if (!prop || !IsCacheableProtoChainForIonOrCacheIR(obj, holder)) return false; + Shape* shape = prop.shape(); if (!shape->hasSlot() || !shape->hasDefaultGetter()) return false; @@ -480,10 +481,10 @@ jit::IsCacheableGetPropReadSlotForIonOrCacheIR(JSObject* obj, JSObject* holder, } static bool -IsCacheableNoProperty(JSObject* obj, JSObject* holder, Shape* shape, jsbytecode* pc, +IsCacheableNoProperty(JSObject* obj, JSObject* holder, PropertyResult prop, jsbytecode* pc, const TypedOrValueRegister& output) { - if (shape) + if (prop) return false; MOZ_ASSERT(!holder); @@ -751,7 +752,7 @@ CheckDOMProxyExpandoDoesNotShadow(JSContext* cx, MacroAssembler& masm, JSObject* static void GenerateReadSlot(JSContext* cx, IonScript* ion, MacroAssembler& masm, IonCache::StubAttacher& attacher, MaybeCheckTDZ checkTDZ, - JSObject* obj, JSObject* holder, Shape* shape, Register object, + JSObject* obj, JSObject* holder, PropertyResult prop, Register object, TypedOrValueRegister output, Label* failures = nullptr) { // If there's a single jump to |failures|, we can patch the shape guard @@ -778,7 +779,7 @@ GenerateReadSlot(JSContext* cx, IonScript* ion, MacroAssembler& masm, if (obj != holder || obj->is() || - !holder->as().isFixedSlot(shape->slot())) + !holder->as().isFixedSlot(prop.shape()->slot())) { if (output.hasValue()) { scratchReg = output.valueReg().scratchReg(); @@ -793,7 +794,7 @@ GenerateReadSlot(JSContext* cx, IonScript* ion, MacroAssembler& masm, // Fast path: single failure jump, no prototype guards. if (!multipleFailureJumps) { - EmitLoadSlot(masm, &holder->as(), shape, object, output, scratchReg); + EmitLoadSlot(masm, &holder->as(), prop.shape(), object, output, scratchReg); if (restoreScratch) masm.pop(scratchReg); attacher.jumpRejoin(masm); @@ -848,7 +849,8 @@ GenerateReadSlot(JSContext* cx, IonScript* ion, MacroAssembler& masm, // Slot access. if (holder) { - EmitLoadSlot(masm, &holder->as(), shape, holderReg, output, scratchReg); + EmitLoadSlot(masm, &holder->as(), prop.shape(), holderReg, output, + scratchReg); if (checkTDZ && output.hasValue()) masm.branchTestMagic(Assembler::Equal, output.valueReg(), failures); } else { @@ -1294,7 +1296,8 @@ CanAttachNativeGetProp(JSContext* cx, const GetPropCache& cache, // only miss out on shape hashification, which is only a temporary perf cost. // The limits were arbitrarily set, anyways. JSObject* baseHolder = nullptr; - if (!LookupPropertyPure(cx, obj, id, &baseHolder, shape.address())) + PropertyResult prop; + if (!LookupPropertyPure(cx, obj, id, &baseHolder, &prop)) return GetPropertyIC::CanAttachNone; MOZ_ASSERT(!holder); @@ -1303,12 +1306,13 @@ CanAttachNativeGetProp(JSContext* cx, const GetPropCache& cache, return GetPropertyIC::CanAttachNone; holder.set(&baseHolder->as()); } + shape.set(prop.maybeShape()); RootedScript script(cx); jsbytecode* pc; cache.getScriptedLocation(&script, &pc); - if (IsCacheableGetPropReadSlotForIonOrCacheIR(obj, holder, shape) || - IsCacheableNoProperty(obj, holder, shape, pc, cache.output())) + if (IsCacheableGetPropReadSlotForIonOrCacheIR(obj, holder, prop) || + IsCacheableNoProperty(obj, holder, prop, pc, cache.output())) { return GetPropertyIC::CanAttachReadSlot; } @@ -1505,7 +1509,7 @@ GetPropertyIC::tryAttachNative(JSContext* cx, HandleScript outerScript, IonScrip switch (type) { case CanAttachReadSlot: GenerateReadSlot(cx, ion, masm, attacher, DontCheckTDZ, obj, holder, - shape, object(), output(), maybeFailures); + PropertyResult(shape), object(), output(), maybeFailures); attachKind = idempotent() ? "idempotent reading" : "non idempotent reading"; outcome = JS::TrackedOutcome::ICGetPropStub_ReadSlot; @@ -1588,7 +1592,7 @@ GetPropertyIC::tryAttachUnboxedExpando(JSContext* cx, HandleScript outerScript, StubAttacher attacher(*this); GenerateReadSlot(cx, ion, masm, attacher, DontCheckTDZ, obj, obj, - shape, object(), output(), maybeFailures); + PropertyResult(shape), object(), output(), maybeFailures); return linkAndAttachStub(cx, masm, attacher, ion, "read unboxed expando", JS::TrackedOutcome::ICGetPropStub_UnboxedReadExpando); } @@ -2927,12 +2931,14 @@ IsCacheableDOMProxyUnshadowedSetterCall(JSContext* cx, HandleObject obj, HandleI if (!checkObj) return false; - if (!LookupPropertyPure(cx, obj, id, holder.address(), shape.address())) + PropertyResult prop; + if (!LookupPropertyPure(cx, obj, id, holder.address(), &prop)) return false; - if (!holder) + if (!holder || !holder->isNative()) return false; + shape.set(prop.shape()); return IsCacheableSetPropCallNative(checkObj, holder, shape) || IsCacheableSetPropCallPropertyOp(checkObj, holder, shape) || IsCacheableSetPropCallScripted(checkObj, holder, shape); @@ -3344,22 +3350,26 @@ CanAttachNativeSetProp(JSContext* cx, HandleObject obj, HandleId id, const Const // If we couldn't find the property on the object itself, do a full, but // still pure lookup for setters. - if (!LookupPropertyPure(cx, obj, id, holder.address(), shape.address())) + Rooted prop(cx); + if (!LookupPropertyPure(cx, obj, id, holder.address(), prop.address())) return SetPropertyIC::CanAttachNone; // If the object doesn't have the property, we don't know if we can attach // a stub to add the property until we do the VM call to add. If the // property exists as a data property on the prototype, we should add // a new, shadowing property. - if (obj->isNative() && (!shape || (obj != holder && holder->isNative() && - shape->hasDefaultSetter() && shape->hasSlot()))) + if (obj->isNative() && + (!prop || (obj != holder && holder->isNative() && + prop.shape()->hasDefaultSetter() && prop.shape()->hasSlot()))) { + shape.set(prop.maybeShape()); return SetPropertyIC::MaybeCanAttachAddSlot; } - if (IsImplicitNonNativeProperty(shape)) + if (prop.isNonNativeProperty()) return SetPropertyIC::CanAttachNone; + shape.set(prop.maybeShape()); if (IsCacheableSetPropCallPropertyOp(obj, holder, shape) || IsCacheableSetPropCallNative(obj, holder, shape) || IsCacheableSetPropCallScripted(obj, holder, shape)) @@ -4836,7 +4846,7 @@ BindNameIC::update(JSContext* cx, HandleScript outerScript, size_t cacheIndex, bool NameIC::attachReadSlot(JSContext* cx, HandleScript outerScript, IonScript* ion, HandleObject envChain, HandleObject holderBase, - HandleNativeObject holder, HandleShape shape) + HandleNativeObject holder, Handle prop) { MacroAssembler masm(cx, ion, outerScript, profilerLeavePc_); Label failures; @@ -4854,7 +4864,7 @@ NameIC::attachReadSlot(JSContext* cx, HandleScript outerScript, IonScript* ion, // doesn't generate the extra guard. // // NAME ops must do their own TDZ checks. - GenerateReadSlot(cx, ion, masm, attacher, CheckTDZ, holderBase, holder, shape, scratchReg, + GenerateReadSlot(cx, ion, masm, attacher, CheckTDZ, holderBase, holder, prop, scratchReg, outputReg(), failures.used() ? &failures : nullptr); return linkAndAttachStub(cx, masm, attacher, ion, "generic", @@ -4880,26 +4890,26 @@ IsCacheableEnvironmentChain(JSObject* envChain, JSObject* obj) } static bool -IsCacheableNameReadSlot(HandleObject envChain, HandleObject obj, - HandleObject holder, HandleShape shape, jsbytecode* pc, +IsCacheableNameReadSlot(JSContext* cx, HandleObject envChain, HandleObject obj, + HandleObject holder, Handle prop, jsbytecode* pc, const TypedOrValueRegister& output) { - if (!shape) + if (!prop) return false; if (!obj->isNative()) return false; if (obj->is()) { // Support only simple property lookups. - if (!IsCacheableGetPropReadSlotForIonOrCacheIR(obj, holder, shape) && - !IsCacheableNoProperty(obj, holder, shape, pc, output)) + if (!IsCacheableGetPropReadSlotForIonOrCacheIR(obj, holder, prop) && + !IsCacheableNoProperty(obj, holder, prop, pc, output)) return false; } else if (obj->is()) { // We don't yet support lookups in a module environment. return false; } else if (obj->is()) { MOZ_ASSERT(obj == holder); - if (!shape->hasDefaultGetter()) + if (!prop.shape()->hasDefaultGetter()) return false; } else { // We don't yet support lookups on Block or DeclEnv objects. @@ -4942,9 +4952,9 @@ NameIC::attachCallGetter(JSContext* cx, HandleScript outerScript, IonScript* ion static bool IsCacheableNameCallGetter(HandleObject envChain, HandleObject obj, HandleObject holder, - HandleShape shape) + Handle prop) { - if (!shape) + if (!prop) return false; if (!obj->is()) return false; @@ -4952,6 +4962,10 @@ IsCacheableNameCallGetter(HandleObject envChain, HandleObject obj, HandleObject if (!IsCacheableEnvironmentChain(envChain, obj)) return false; + if (!prop || !IsCacheableProtoChainForIonOrCacheIR(obj, holder)) + return false; + + Shape* shape = prop.shape(); return IsCacheableGetPropCallNative(obj, holder, shape) || IsCacheableGetPropCallPropertyOp(obj, holder, shape) || IsCacheableGetPropCallScripted(obj, holder, shape); @@ -4996,10 +5010,10 @@ NameIC::attachTypeOfNoProperty(JSContext* cx, HandleScript outerScript, IonScrip static bool IsCacheableNameNoProperty(HandleObject envChain, HandleObject obj, - HandleObject holder, HandleShape shape, jsbytecode* pc, + HandleObject holder, Handle prop, jsbytecode* pc, NameIC& cache) { - if (cache.isTypeOf() && !shape) { + if (cache.isTypeOf() && !prop) { MOZ_ASSERT(!obj); MOZ_ASSERT(!holder); MOZ_ASSERT(envChain); @@ -5029,34 +5043,35 @@ NameIC::update(JSContext* cx, HandleScript outerScript, size_t cacheIndex, Handl RootedObject obj(cx); RootedObject holder(cx); - RootedShape shape(cx); - if (!LookupName(cx, name, envChain, &obj, &holder, &shape)) + Rooted prop(cx); + if (!LookupName(cx, name, envChain, &obj, &holder, &prop)) return false; // Look first. Don't generate cache entries if the lookup fails. if (cache.isTypeOf()) { - if (!FetchName(cx, obj, holder, name, shape, vp)) + if (!FetchName(cx, obj, holder, name, prop, vp)) return false; } else { - if (!FetchName(cx, obj, holder, name, shape, vp)) + if (!FetchName(cx, obj, holder, name, prop, vp)) return false; } if (cache.canAttachStub()) { - if (IsCacheableNameReadSlot(envChain, obj, holder, shape, pc, cache.outputReg())) { + if (IsCacheableNameReadSlot(cx, envChain, obj, holder, prop, pc, cache.outputReg())) { if (!cache.attachReadSlot(cx, outerScript, ion, envChain, obj, - holder.as(), shape)) + holder.as(), prop)) { return false; } - } else if (IsCacheableNameCallGetter(envChain, obj, holder, shape)) { + } else if (IsCacheableNameCallGetter(envChain, obj, holder, prop)) { void* returnAddr = GetReturnAddressToIonCode(cx); + RootedShape shape(cx, prop.shape()); if (!cache.attachCallGetter(cx, outerScript, ion, envChain, obj, holder, shape, returnAddr)) { return false; } - } else if (IsCacheableNameNoProperty(envChain, obj, holder, shape, pc, cache)) { + } else if (IsCacheableNameNoProperty(envChain, obj, holder, prop, pc, cache)) { if (!cache.attachTypeOfNoProperty(cx, outerScript, ion, envChain)) return false; } diff --git a/js/src/jit/IonCaches.h b/js/src/jit/IonCaches.h index a7135000e5..914965055b 100644 --- a/js/src/jit/IonCaches.h +++ b/js/src/jit/IonCaches.h @@ -806,7 +806,7 @@ class NameIC : public IonCache MOZ_MUST_USE bool attachReadSlot(JSContext* cx, HandleScript outerScript, IonScript* ion, HandleObject envChain, HandleObject holderBase, - HandleNativeObject holder, HandleShape shape); + HandleNativeObject holder, Handle prop); MOZ_MUST_USE bool attachCallGetter(JSContext* cx, HandleScript outerScript, IonScript* ion, HandleObject envChain, HandleObject obj, @@ -839,7 +839,7 @@ IONCACHE_KIND_LIST(CACHE_CASTS) #undef OPCODE_CASTS bool IsCacheableProtoChainForIonOrCacheIR(JSObject* obj, JSObject* holder); -bool IsCacheableGetPropReadSlotForIonOrCacheIR(JSObject* obj, JSObject* holder, Shape* shape); +bool IsCacheableGetPropReadSlotForIonOrCacheIR(JSObject* obj, JSObject* holder, PropertyResult prop); } // namespace jit } // namespace js diff --git a/js/src/jit/SharedIC.cpp b/js/src/jit/SharedIC.cpp index 1434894193..f8f4433af7 100644 --- a/js/src/jit/SharedIC.cpp +++ b/js/src/jit/SharedIC.cpp @@ -2192,12 +2192,12 @@ GetDOMProxyProto(JSObject* obj) // existence of the property on the object. bool EffectlesslyLookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject holder, MutableHandleShape shape, + MutableHandleObject holder, MutableHandle prop, bool* checkDOMProxy, DOMProxyShadowsResult* shadowsResult, bool* domProxyHasGeneration) { - shape.set(nullptr); + prop.setNotFound(); holder.set(nullptr); if (checkDOMProxy) { @@ -2231,11 +2231,11 @@ EffectlesslyLookupProperty(JSContext* cx, HandleObject obj, HandleId id, return true; } - if (LookupPropertyPure(cx, checkObj, id, holder.address(), shape.address())) + if (LookupPropertyPure(cx, checkObj, id, holder.address(), prop.address())) return true; holder.set(nullptr); - shape.set(nullptr); + prop.setNotFound(); return true; } @@ -2421,15 +2421,16 @@ TryAttachNativeGetAccessorPropStub(JSContext* cx, SharedStubInfo* info, bool isDOMProxy; bool domProxyHasGeneration; DOMProxyShadowsResult domProxyShadowsResult; - RootedShape shape(cx); + Rooted prop(cx); RootedObject holder(cx); RootedId id(cx, NameToId(name)); - if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &shape, &isDOMProxy, + if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &prop, &isDOMProxy, &domProxyShadowsResult, &domProxyHasGeneration)) { return false; } + RootedShape shape(cx, prop.maybeShape()); ICStub* monitorStub = stub->fallbackMonitorStub()->firstMonitorStub(); bool isScripted = false; @@ -2492,7 +2493,7 @@ TryAttachNativeGetAccessorPropStub(JSContext* cx, SharedStubInfo* info, MOZ_ASSERT(ToWindowIfWindowProxy(obj) == cx->global()); obj = cx->global(); - if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &shape, &isDOMProxy, + if (!EffectlesslyLookupProperty(cx, obj, id, &holder, &prop, &isDOMProxy, &domProxyShadowsResult, &domProxyHasGeneration)) { return false; diff --git a/js/src/jit/SharedIC.h b/js/src/jit/SharedIC.h index 8ad8fd495b..d0038c9378 100644 --- a/js/src/jit/SharedIC.h +++ b/js/src/jit/SharedIC.h @@ -2249,7 +2249,7 @@ StripPreliminaryObjectStubs(JSContext* cx, ICFallbackStub* stub); MOZ_MUST_USE bool EffectlesslyLookupProperty(JSContext* cx, HandleObject obj, HandleId name, - MutableHandleObject holder, MutableHandleShape shape, + MutableHandleObject holder, MutableHandle prop, bool* checkDOMProxy=nullptr, DOMProxyShadowsResult* shadowsResult=nullptr, bool* domProxyHasGeneration=nullptr); diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp index fd41f6fbcb..fbe6977bf9 100644 --- a/js/src/jit/VMFunctions.cpp +++ b/js/src/jit/VMFunctions.cpp @@ -583,11 +583,11 @@ GetDynamicName(JSContext* cx, JSObject* envChain, JSString* str, Value* vp) return; } - Shape* shape = nullptr; + PropertyResult prop; JSObject* scope = nullptr; JSObject* pobj = nullptr; - if (LookupNameNoGC(cx, atom->asPropertyName(), envChain, &scope, &pobj, &shape)) { - if (FetchNameNoGC(pobj, shape, MutableHandleValue::fromMarkedLocation(vp))) + if (LookupNameNoGC(cx, atom->asPropertyName(), envChain, &scope, &pobj, &prop)) { + if (FetchNameNoGC(pobj, prop, MutableHandleValue::fromMarkedLocation(vp))) return; } diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index f4b3c98545..3901a0e63c 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -2947,9 +2947,9 @@ JS_AlreadyHasOwnPropertyById(JSContext* cx, HandleObject obj, HandleId id, bool* return js::HasOwnProperty(cx, obj, id, foundp); RootedNativeObject nativeObj(cx, &obj->as()); - RootedShape prop(cx); + Rooted prop(cx); NativeLookupOwnPropertyNoResolve(cx, nativeObj, id, &prop); - *foundp = !!prop; + *foundp = prop.isFound(); return true; } diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 95f49b35e3..494bcb042a 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -331,7 +331,7 @@ extern JS_FRIEND_DATA(const js::ObjectOps) ProxyObjectOps; extern JS_FRIEND_API(bool) proxy_LookupProperty(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::MutableHandleObject objp, - JS::MutableHandle propp); + JS::MutableHandle propp); extern JS_FRIEND_API(bool) proxy_DefineProperty(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::Handle desc, diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index a243f44681..a23ac63366 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -2116,7 +2116,7 @@ JSObject::constructHook() const bool js::LookupProperty(JSContext* cx, HandleObject obj, js::HandleId id, - MutableHandleObject objp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandle propp) { /* NB: The logic of lookupProperty is implicitly reflected in * BaselineIC.cpp's |EffectlesslyLookupProperty| logic. @@ -2129,7 +2129,7 @@ js::LookupProperty(JSContext* cx, HandleObject obj, js::HandleId id, bool js::LookupName(JSContext* cx, HandlePropertyName name, HandleObject envChain, - MutableHandleObject objp, MutableHandleObject pobjp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandleObject pobjp, MutableHandle propp) { RootedId id(cx, NameToId(name)); @@ -2144,13 +2144,13 @@ js::LookupName(JSContext* cx, HandlePropertyName name, HandleObject envChain, objp.set(nullptr); pobjp.set(nullptr); - propp.set(nullptr); + propp.setNotFound(); return true; } bool js::LookupNameNoGC(JSContext* cx, PropertyName* name, JSObject* envChain, - JSObject** objp, JSObject** pobjp, Shape** propp) + JSObject** objp, JSObject** pobjp, PropertyResult* propp) { AutoAssertNoException nogc(cx); @@ -2177,13 +2177,13 @@ js::LookupNameWithGlobalDefault(JSContext* cx, HandlePropertyName name, HandleOb RootedId id(cx, NameToId(name)); RootedObject pobj(cx); - RootedShape shape(cx); + Rooted prop(cx); RootedObject env(cx, envChain); for (; !env->is(); env = env->enclosingEnvironment()) { - if (!LookupProperty(cx, env, id, &pobj, &shape)) + if (!LookupProperty(cx, env, id, &pobj, &prop)) return false; - if (shape) + if (prop) break; } @@ -2198,20 +2198,20 @@ js::LookupNameUnqualified(JSContext* cx, HandlePropertyName name, HandleObject e RootedId id(cx, NameToId(name)); RootedObject pobj(cx); - RootedShape shape(cx); + Rooted prop(cx); RootedObject env(cx, envChain); for (; !env->isUnqualifiedVarObj(); env = env->enclosingEnvironment()) { - if (!LookupProperty(cx, env, id, &pobj, &shape)) + if (!LookupProperty(cx, env, id, &pobj, &prop)) return false; - if (shape) + if (prop) break; } // See note above RuntimeLexicalErrorObject. if (pobj == env) { bool isTDZ = false; - if (shape && name != cx->names().dotThis) { + if (prop && name != cx->names().dotThis) { // Treat Debugger environments specially for TDZ checks, as they // look like non-native environments but in fact wrap native // environments. @@ -2222,7 +2222,7 @@ js::LookupNameUnqualified(JSContext* cx, HandlePropertyName name, HandleObject e return false; isTDZ = IsUninitializedLexical(v); } else { - isTDZ = IsUninitializedLexicalSlot(env, shape); + isTDZ = IsUninitializedLexicalSlot(env, prop); } } @@ -2230,7 +2230,7 @@ js::LookupNameUnqualified(JSContext* cx, HandlePropertyName name, HandleObject e env = RuntimeLexicalErrorObject::create(cx, env, JSMSG_UNINITIALIZED_LEXICAL); if (!env) return false; - } else if (env->is() && !shape->writable()) { + } else if (env->is() && !prop.shape()->writable()) { // Assigning to a named lambda callee name is a no-op in sloppy mode. Rooted lexicalEnv(cx, &env->as()); if (lexicalEnv->isExtensible() || @@ -2262,16 +2262,16 @@ js::HasOwnProperty(JSContext* cx, HandleObject obj, HandleId id, bool* result) return true; } - RootedShape shape(cx); - if (!NativeLookupOwnProperty(cx, obj.as(), id, &shape)) + Rooted prop(cx); + if (!NativeLookupOwnProperty(cx, obj.as(), id, &prop)) return false; - *result = (shape != nullptr); + *result = prop.isFound(); return true; } bool js::LookupPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, JSObject** objp, - Shape** propp) + PropertyResult* propp) { bool isTypedArrayOutOfRange = false; do { @@ -2292,12 +2292,12 @@ js::LookupPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, JSObject** } while (obj); *objp = nullptr; - *propp = nullptr; + propp->setNotFound(); return true; } bool -js::LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Shape** propp, +js::LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, PropertyResult* propp, bool* isTypedArrayOutOfRange /* = nullptr */) { JS::AutoCheckCannotGC nogc; @@ -2308,7 +2308,7 @@ js::LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Shape** // Search for a native dense element, typed array element, or property. if (JSID_IS_INT(id) && obj->as().containsDenseElement(JSID_TO_INT(id))) { - MarkDenseOrTypedArrayElementFound(propp); + propp->setDenseOrTypedArrayElement(); return true; } @@ -2316,9 +2316,9 @@ js::LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Shape** uint64_t index; if (IsTypedArrayIndex(id, &index)) { if (index < obj->as().length()) { - MarkDenseOrTypedArrayElementFound(propp); + propp->setDenseOrTypedArrayElement(); } else { - *propp = nullptr; + propp->setNotFound(); if (isTypedArrayOutOfRange) *isTypedArrayOutOfRange = true; } @@ -2327,7 +2327,7 @@ js::LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Shape** } if (Shape* shape = obj->as().lookupPure(id)) { - *propp = shape; + propp->setNativeProperty(shape); return true; } @@ -2337,31 +2337,31 @@ js::LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Shape** return false; } else if (obj->is()) { if (obj->as().containsUnboxedOrExpandoProperty(cx, id)) { - MarkNonNativePropertyFound(propp); + propp->setNonNativeProperty(); return true; } } else if (obj->is()) { if (obj->as().containsProperty(cx, id)) { - MarkNonNativePropertyFound(propp); + propp->setNonNativeProperty(); return true; } } else if (obj->is()) { if (obj->as().typeDescr().hasProperty(cx->names(), id)) { - MarkNonNativePropertyFound(propp); + propp->setNonNativeProperty(); return true; } } else { return false; } - *propp = nullptr; + propp->setNotFound(); return true; } static inline bool -NativeGetPureInline(NativeObject* pobj, jsid id, Shape* shape, Value* vp) +NativeGetPureInline(NativeObject* pobj, jsid id, PropertyResult prop, Value* vp) { - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { // For simplicity we ignore the TypedArray with string index case. if (!JSID_IS_INT(id)) return false; @@ -2371,6 +2371,7 @@ NativeGetPureInline(NativeObject* pobj, jsid id, Shape* shape, Value* vp) } // Fail if we have a custom getter. + Shape* shape = prop.shape(); if (!shape->hasDefaultGetter()) return false; @@ -2388,22 +2389,23 @@ bool js::GetPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Value* vp) { JSObject* pobj; - Shape* shape; - if (!LookupPropertyPure(cx, obj, id, &pobj, &shape)) + PropertyResult prop; + if (!LookupPropertyPure(cx, obj, id, &pobj, &prop)) return false; - if (!shape) { + if (!prop) { vp->setUndefined(); return true; } - return pobj->isNative() && NativeGetPureInline(&pobj->as(), id, shape, vp); + return pobj->isNative() && NativeGetPureInline(&pobj->as(), id, prop, vp); } static inline bool -NativeGetGetterPureInline(Shape* shape, JSFunction** fp) +NativeGetGetterPureInline(PropertyResult prop, JSFunction** fp) { - if (!IsImplicitDenseOrTypedArrayElement(shape) && shape->hasGetterObject()) { + if (!prop.isDenseOrTypedArrayElement() && prop.shape()->hasGetterObject()) { + Shape* shape = prop.shape(); if (shape->getterObject()->is()) { *fp = &shape->getterObject()->as(); return true; @@ -2420,32 +2422,32 @@ js::GetGetterPure(ExclusiveContext* cx, JSObject* obj, jsid id, JSFunction** fp) /* Just like GetPropertyPure, but get getter function, without invoking * it. */ JSObject* pobj; - Shape* shape; - if (!LookupPropertyPure(cx, obj, id, &pobj, &shape)) + PropertyResult prop; + if (!LookupPropertyPure(cx, obj, id, &pobj, &prop)) return false; - if (!shape) { + if (!prop) { *fp = nullptr; return true; } - return pobj->isNative() && NativeGetGetterPureInline(shape, fp); + return prop.isNativeProperty() && NativeGetGetterPureInline(prop, fp); } bool js::GetOwnGetterPure(ExclusiveContext* cx, JSObject* obj, jsid id, JSFunction** fp) { JS::AutoCheckCannotGC nogc; - Shape* shape; - if (!LookupOwnPropertyPure(cx, obj, id, &shape)) + PropertyResult prop; + if (!LookupOwnPropertyPure(cx, obj, id, &prop)) return false; - if (!shape) { + if (!prop) { *fp = nullptr; return true; } - return NativeGetGetterPureInline(shape, fp); + return prop.isNativeProperty() && NativeGetGetterPureInline(prop, fp); } bool @@ -2453,14 +2455,14 @@ js::GetOwnNativeGetterPure(JSContext* cx, JSObject* obj, jsid id, JSNative* nati { JS::AutoCheckCannotGC nogc; *native = nullptr; - Shape* shape; - if (!LookupOwnPropertyPure(cx, obj, id, &shape)) + PropertyResult prop; + if (!LookupOwnPropertyPure(cx, obj, id, &prop)) return false; - if (!shape || IsImplicitDenseOrTypedArrayElement(shape) || !shape->hasGetterObject()) + if (!prop || prop.isDenseOrTypedArrayElement() || !prop.shape()->hasGetterObject()) return true; - JSObject* getterObj = shape->getterObject(); + JSObject* getterObj = prop.shape()->getterObject(); if (!getterObj->is()) return true; @@ -2475,12 +2477,12 @@ js::GetOwnNativeGetterPure(JSContext* cx, JSObject* obj, jsid id, JSNative* nati bool js::HasOwnDataPropertyPure(JSContext* cx, JSObject* obj, jsid id, bool* result) { - Shape* shape = nullptr; - if (!LookupOwnPropertyPure(cx, obj, id, &shape)) + PropertyResult prop; + if (!LookupOwnPropertyPure(cx, obj, id, &prop)) return false; - *result = shape && !IsImplicitDenseOrTypedArrayElement(shape) && shape->hasDefaultGetter() && - shape->hasSlot(); + *result = prop && !prop.isDenseOrTypedArrayElement() && prop.shape()->hasDefaultGetter() && + prop.shape()->hasSlot(); return true; } diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 52be7e8188..68491a1aa9 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -999,11 +999,11 @@ GetPropertyDescriptor(JSContext* cx, HandleObject obj, HandleId id, */ extern bool LookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject objp, MutableHandleShape propp); + MutableHandleObject objp, MutableHandle propp); inline bool LookupProperty(JSContext* cx, HandleObject obj, PropertyName* name, - MutableHandleObject objp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandle propp) { RootedId id(cx, NameToId(name)); return LookupProperty(cx, obj, id, objp, propp); @@ -1195,11 +1195,11 @@ ReadPropertyDescriptors(JSContext* cx, HandleObject props, bool checkAccessors, /* Read the name using a dynamic lookup on the scopeChain. */ extern bool LookupName(JSContext* cx, HandlePropertyName name, HandleObject scopeChain, - MutableHandleObject objp, MutableHandleObject pobjp, MutableHandleShape propp); + MutableHandleObject objp, MutableHandleObject pobjp, MutableHandle propp); extern bool LookupNameNoGC(JSContext* cx, PropertyName* name, JSObject* scopeChain, - JSObject** objp, JSObject** pobjp, Shape** propp); + JSObject** objp, JSObject** pobjp, PropertyResult* propp); /* * Like LookupName except returns the global object if 'name' is not found in @@ -1233,10 +1233,10 @@ FindVariableScope(JSContext* cx, JSFunction** funp); bool LookupPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, JSObject** objp, - Shape** propp); + PropertyResult* propp); bool -LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Shape** propp, +LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, PropertyResult* propp, bool* isTypedArrayOutOfRange = nullptr); bool diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 8a55cd435c..07eeff655e 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -585,11 +585,11 @@ HasNoToPrimitiveMethodPure(JSObject* obj, JSContext* cx) { jsid id = SYMBOL_TO_JSID(cx->wellKnownSymbols().toPrimitive); JSObject* pobj; - Shape* shape; - if (!LookupPropertyPure(cx, obj, id, &pobj, &shape)) + PropertyResult prop; + if (!LookupPropertyPure(cx, obj, id, &pobj, &prop)) return false; - return !shape; + return !prop; } /* ES6 draft rev 28 (2014 Oct 14) 7.1.14 */ diff --git a/js/src/json.cpp b/js/src/json.cpp index e32994e908..f3cf22dac9 100644 --- a/js/src/json.cpp +++ b/js/src/json.cpp @@ -445,9 +445,9 @@ JO(JSContext* cx, HandleObject obj, StringifyContext* scx) #ifdef DEBUG if (scx->maybeSafely) { RootedNativeObject nativeObj(cx, &obj->as()); - RootedShape prop(cx); + Rooted prop(cx); NativeLookupOwnPropertyNoResolve(cx, nativeObj, id, &prop); - MOZ_ASSERT(prop && prop->isDataDescriptor()); + MOZ_ASSERT(prop && prop.isNativeProperty() && prop.shape()->isDataDescriptor()); } #endif // DEBUG if (!GetProperty(cx, obj, obj, id, &outputValue)) diff --git a/js/src/proxy/Proxy.cpp b/js/src/proxy/Proxy.cpp index 376bbcdda7..6f91dfb106 100644 --- a/js/src/proxy/Proxy.cpp +++ b/js/src/proxy/Proxy.cpp @@ -530,18 +530,18 @@ Proxy::trace(JSTracer* trc, JSObject* proxy) bool js::proxy_LookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject objp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandle propp) { bool found; if (!Proxy::has(cx, obj, id, &found)) return false; if (found) { - MarkNonNativePropertyFound(propp); + propp.setNonNativeProperty(); objp.set(obj); } else { + propp.setNotFound(); objp.set(nullptr); - propp.set(nullptr); } return true; } diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 305555f414..f665dcef9b 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -10098,12 +10098,14 @@ DebuggerObject::forceLexicalInitializationByName(JSContext* cx, HandleDebuggerOb RootedObject globalLexical(cx, &referent->lexicalEnvironment()); RootedObject pobj(cx); - RootedShape shape(cx); - if (!LookupProperty(cx, globalLexical, id, &pobj, &shape)) + Rooted prop(cx); + if (!LookupProperty(cx, globalLexical, id, &pobj, &prop)) return false; result = false; - if (shape) { + if (prop) { + MOZ_ASSERT(prop.isNativeProperty()); + Shape* shape = prop.shape(); Value v = globalLexical->as().getSlot(shape->slot()); if (shape->hasSlot() && v.isMagic() && v.whyMagic() == JS_UNINITIALIZED_LEXICAL) { globalLexical->as().setSlot(shape->slot(), UndefinedValue()); diff --git a/js/src/vm/EnvironmentObject.cpp b/js/src/vm/EnvironmentObject.cpp index bffa32f6bf..4e74b95ab2 100644 --- a/js/src/vm/EnvironmentObject.cpp +++ b/js/src/vm/EnvironmentObject.cpp @@ -518,14 +518,14 @@ ModuleEnvironmentObject::fixEnclosingEnvironmentAfterCompartmentMerge(GlobalObje /* static */ bool ModuleEnvironmentObject::lookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject objp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandle propp) { const IndirectBindingMap& bindings = obj->as().importBindings(); Shape* shape; ModuleEnvironmentObject* env; if (bindings.lookup(id, &env, &shape)) { objp.set(env); - propp.set(shape); + propp.setNativeProperty(shape); return true; } @@ -688,13 +688,13 @@ CheckUnscopables(JSContext *cx, HandleObject obj, HandleId id, bool *scopable) static bool with_LookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject objp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandle propp) { // SpiderMonkey-specific: consider internal '.generator' and '.this' names // to be unscopable. if (IsUnscopableDotName(cx, id)) { objp.set(nullptr); - propp.set(nullptr); + propp.setNotFound(); return true; } @@ -708,7 +708,7 @@ with_LookupProperty(JSContext* cx, HandleObject obj, HandleId id, return false; if (!scopable) { objp.set(nullptr); - propp.set(nullptr); + propp.setNotFound(); } } return true; @@ -1104,7 +1104,7 @@ ReportRuntimeLexicalErrorId(JSContext* cx, unsigned errorNumber, HandleId id) static bool lexicalError_LookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject objp, MutableHandleShape propp) + MutableHandleObject objp, MutableHandle propp) { ReportRuntimeLexicalErrorId(cx, obj->as().errorNumber(), id); return false; diff --git a/js/src/vm/EnvironmentObject.h b/js/src/vm/EnvironmentObject.h index 0277e7c65c..752eb5c65c 100644 --- a/js/src/vm/EnvironmentObject.h +++ b/js/src/vm/EnvironmentObject.h @@ -407,7 +407,7 @@ class ModuleEnvironmentObject : public EnvironmentObject private: static bool lookupProperty(JSContext* cx, HandleObject obj, HandleId id, - MutableHandleObject objp, MutableHandleShape propp); + MutableHandleObject objp, MutableHandle propp); static bool hasProperty(JSContext* cx, HandleObject obj, HandleId id, bool* foundp); static bool getProperty(JSContext* cx, HandleObject obj, HandleValue receiver, HandleId id, MutableHandleValue vp); diff --git a/js/src/vm/GeneratorObject.cpp b/js/src/vm/GeneratorObject.cpp index 018e5a4819..9265a1b628 100644 --- a/js/src/vm/GeneratorObject.cpp +++ b/js/src/vm/GeneratorObject.cpp @@ -363,12 +363,14 @@ js::CheckStarGeneratorResumptionValue(JSContext* cx, HandleValue v) // It should have `value` data property, but the type doesn't matter JSObject* ignored; - Shape* shape; - if (!LookupPropertyPure(cx, obj, NameToId(cx->names().value), &ignored, &shape)) + PropertyResult prop; + if (!LookupPropertyPure(cx, obj, NameToId(cx->names().value), &ignored, &prop)) return false; - if (!shape) + if (!prop) return false; - if (!shape->hasDefaultGetter()) + if (!prop.isNativeProperty()) + return false; + if (!prop.shape()->hasDefaultGetter()) return false; return true; diff --git a/js/src/vm/Interpreter-inl.h b/js/src/vm/Interpreter-inl.h index 2e94a2ab22..5c2320d3fb 100644 --- a/js/src/vm/Interpreter-inl.h +++ b/js/src/vm/Interpreter-inl.h @@ -78,17 +78,18 @@ IsUninitializedLexical(const Value& val) } static inline bool -IsUninitializedLexicalSlot(HandleObject obj, HandleShape shape) +IsUninitializedLexicalSlot(HandleObject obj, Handle prop) { - MOZ_ASSERT(shape); + MOZ_ASSERT(prop); if (obj->is()) return false; - // We check for IsImplicitDenseOrTypedArrayElement even though the shape - // is always a non-indexed property because proxy hooks may return a - // "non-native property found" shape, which happens to be encoded in the - // same way as the "dense element" shape. See MarkNonNativePropertyFound. - if (IsImplicitDenseOrTypedArrayElement(shape) || - !shape->hasSlot() || + + // Proxy hooks may return a non-native property. + if (prop.isNonNativeProperty()) + return false; + + Shape* shape = prop.shape(); + if (!shape->hasSlot() || !shape->hasDefaultGetter() || !shape->hasDefaultSetter()) { @@ -174,9 +175,9 @@ GetLengthProperty(const Value& lval, MutableHandleValue vp) template inline bool FetchName(JSContext* cx, HandleObject obj, HandleObject obj2, HandlePropertyName name, - HandleShape shape, MutableHandleValue vp) + Handle prop, MutableHandleValue vp) { - if (!shape) { + if (!prop) { if (TypeOf) { vp.setUndefined(); return true; @@ -190,6 +191,7 @@ FetchName(JSContext* cx, HandleObject obj, HandleObject obj2, HandlePropertyName if (!GetProperty(cx, obj, obj, id, vp)) return false; } else { + RootedShape shape(cx, prop.shape()); RootedObject normalized(cx, obj); if (normalized->is() && !shape->hasDefaultGetter()) normalized = &normalized->as().object(); @@ -213,9 +215,13 @@ FetchName(JSContext* cx, HandleObject obj, HandleObject obj2, HandlePropertyName } inline bool -FetchNameNoGC(JSObject* pobj, Shape* shape, MutableHandleValue vp) +FetchNameNoGC(JSObject* pobj, PropertyResult prop, MutableHandleValue vp) { - if (!shape || !pobj->isNative() || !shape->isDataDescriptor() || !shape->hasDefaultGetter()) + if (!prop || !pobj->isNative()) + return false; + + Shape* shape = prop.shape(); + if (!shape->isDataDescriptor() || !shape->hasDefaultGetter()) return false; vp.set(pobj->as().getSlot(shape->slot())); @@ -361,7 +367,7 @@ DefVarOperation(JSContext* cx, HandleObject varobj, HandlePropertyName dn, unsig } #endif - RootedShape prop(cx); + Rooted prop(cx); RootedObject obj2(cx); if (!LookupProperty(cx, varobj, dn, &obj2, &prop)) return false; diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 7e3f2a7f1b..cf58e2d608 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -210,27 +210,27 @@ GetNameOperation(JSContext* cx, InterpreterFrame* fp, jsbytecode* pc, MutableHan if (IsGlobalOp(JSOp(*pc)) && !fp->script()->hasNonSyntacticScope()) obj = &obj->global().lexicalEnvironment(); - Shape* shape = nullptr; + PropertyResult prop; JSObject* env = nullptr; JSObject* pobj = nullptr; - if (LookupNameNoGC(cx, name, obj, &env, &pobj, &shape)) { - if (FetchNameNoGC(pobj, shape, vp)) + if (LookupNameNoGC(cx, name, obj, &env, &pobj, &prop)) { + if (FetchNameNoGC(pobj, prop, vp)) return true; } RootedObject objRoot(cx, obj), envRoot(cx), pobjRoot(cx); RootedPropertyName nameRoot(cx, name); - RootedShape shapeRoot(cx); + Rooted propRoot(cx); - if (!LookupName(cx, nameRoot, objRoot, &envRoot, &pobjRoot, &shapeRoot)) + if (!LookupName(cx, nameRoot, objRoot, &envRoot, &pobjRoot, &propRoot)) return false; /* Kludge to allow (typeof foo == "undefined") tests. */ JSOp op2 = JSOp(pc[JSOP_GETNAME_LENGTH]); if (op2 == JSOP_TYPEOF) - return FetchName(cx, envRoot, pobjRoot, nameRoot, shapeRoot, vp); + return FetchName(cx, envRoot, pobjRoot, nameRoot, propRoot, vp); - return FetchName(cx, envRoot, pobjRoot, nameRoot, shapeRoot, vp); + return FetchName(cx, envRoot, pobjRoot, nameRoot, propRoot, vp); } static inline bool @@ -238,12 +238,12 @@ GetImportOperation(JSContext* cx, InterpreterFrame* fp, jsbytecode* pc, MutableH { RootedObject obj(cx, fp->environmentChain()), env(cx), pobj(cx); RootedPropertyName name(cx, fp->script()->getName(pc)); - RootedShape shape(cx); + Rooted prop(cx); - MOZ_ALWAYS_TRUE(LookupName(cx, name, obj, &env, &pobj, &shape)); + MOZ_ALWAYS_TRUE(LookupName(cx, name, obj, &env, &pobj, &prop)); MOZ_ASSERT(env && env->is()); MOZ_ASSERT(env->as().hasImportBinding(name)); - return FetchName(cx, env, pobj, name, shape, vp); + return FetchName(cx, env, pobj, name, prop, vp); } static bool @@ -4388,12 +4388,12 @@ bool js::GetEnvironmentName(JSContext* cx, HandleObject envChain, HandlePropertyName name, MutableHandleValue vp) { - RootedShape shape(cx); + Rooted prop(cx); RootedObject obj(cx), pobj(cx); - if (!LookupName(cx, name, envChain, &obj, &pobj, &shape)) + if (!LookupName(cx, name, envChain, &obj, &pobj, &prop)) return false; - if (!shape) + if (!prop) return ReportIsNotDefined(cx, name); if (!GetProperty(cx, obj, obj, name, vp)) @@ -4415,12 +4415,12 @@ bool js::GetEnvironmentNameForTypeOf(JSContext* cx, HandleObject envChain, HandlePropertyName name, MutableHandleValue vp) { - RootedShape shape(cx); + Rooted prop(cx); RootedObject obj(cx), pobj(cx); - if (!LookupName(cx, name, envChain, &obj, &pobj, &shape)) + if (!LookupName(cx, name, envChain, &obj, &pobj, &prop)) return false; - if (!shape) { + if (!prop) { vp.set(UndefinedValue()); return true; } @@ -4478,9 +4478,9 @@ js::DefFunOperation(JSContext* cx, HandleScript script, HandleObject envChain, /* ES5 10.5 (NB: with subsequent errata). */ RootedPropertyName name(cx, fun->explicitName()->asPropertyName()); - RootedShape shape(cx); + Rooted prop(cx); RootedObject pobj(cx); - if (!LookupProperty(cx, parent, name, &pobj, &shape)) + if (!LookupProperty(cx, parent, name, &pobj, &prop)) return false; RootedValue rval(cx, ObjectValue(*fun)); @@ -4494,7 +4494,7 @@ js::DefFunOperation(JSContext* cx, HandleScript script, HandleObject envChain, : JSPROP_ENUMERATE | JSPROP_PERMANENT; /* Steps 5d, 5f. */ - if (!shape || pobj != parent) { + if (!prop || pobj != parent) { if (!DefineProperty(cx, parent, name, rval, nullptr, nullptr, attrs)) return false; @@ -4512,6 +4512,7 @@ js::DefFunOperation(JSContext* cx, HandleScript script, HandleObject envChain, */ MOZ_ASSERT(parent->isNative() || parent->is()); if (parent->is()) { + Shape* shape = prop.shape(); if (shape->configurable()) { if (!DefineProperty(cx, parent, name, rval, nullptr, nullptr, attrs)) return false; @@ -4716,8 +4717,8 @@ js::DeleteNameOperation(JSContext* cx, HandlePropertyName name, HandleObject sco MutableHandleValue res) { RootedObject scope(cx), pobj(cx); - RootedShape shape(cx); - if (!LookupName(cx, name, scopeObj, &scope, &pobj, &shape)) + Rooted prop(cx); + if (!LookupName(cx, name, scopeObj, &scope, &pobj, &prop)) return false; if (!scope) { diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index 2bb70b7d90..67dff24b7b 100644 --- a/js/src/vm/NativeObject-inl.h +++ b/js/src/vm/NativeObject-inl.h @@ -382,8 +382,8 @@ NewNativeObjectWithClassProto(ExclusiveContext* cx, const Class* clasp, HandleOb * *recursedp = false and return true. */ static MOZ_ALWAYS_INLINE bool -CallResolveOp(JSContext* cx, HandleNativeObject obj, HandleId id, MutableHandleShape propp, - bool* recursedp) +CallResolveOp(JSContext* cx, HandleNativeObject obj, HandleId id, + MutableHandle propp, bool* recursedp) { // Avoid recursion on (obj, id) already being resolved on cx. AutoResolving resolving(cx, obj, id); @@ -407,13 +407,18 @@ CallResolveOp(JSContext* cx, HandleNativeObject obj, HandleId id, MutableHandleS obj->getClass()->getMayResolve()(cx->names(), id, obj)); if (JSID_IS_INT(id) && obj->containsDenseElement(JSID_TO_INT(id))) { - MarkDenseOrTypedArrayElementFound(propp); + propp.setDenseOrTypedArrayElement(); return true; } MOZ_ASSERT(!obj->is()); - propp.set(obj->lookup(cx, id)); + RootedShape shape(cx, obj->lookup(cx, id)); + if (shape) + propp.setNativeProperty(shape); + else + propp.setNotFound(); + return true; } @@ -444,12 +449,12 @@ static MOZ_ALWAYS_INLINE bool LookupOwnPropertyInline(ExclusiveContext* cx, typename MaybeRooted::HandleType obj, typename MaybeRooted::HandleType id, - typename MaybeRooted::MutableHandleType propp, + typename MaybeRooted::MutableHandleType propp, bool* donep) { // Check for a native dense element. if (JSID_IS_INT(id) && obj->containsDenseElement(JSID_TO_INT(id))) { - MarkDenseOrTypedArrayElementFound(propp); + propp.setDenseOrTypedArrayElement(); *donep = true; return true; } @@ -460,11 +465,10 @@ LookupOwnPropertyInline(ExclusiveContext* cx, if (obj->template is()) { uint64_t index; if (IsTypedArrayIndex(id, &index)) { - if (index < obj->template as().length()) { - MarkDenseOrTypedArrayElementFound(propp); - } else { - propp.set(nullptr); - } + if (index < obj->template as().length()) + propp.setDenseOrTypedArrayElement(); + else + propp.setNotFound(); *donep = true; return true; } @@ -472,7 +476,7 @@ LookupOwnPropertyInline(ExclusiveContext* cx, // Check for a native property. if (Shape* shape = obj->lookup(cx, id)) { - propp.set(shape); + propp.setNativeProperty(shape); *donep = true; return true; } @@ -486,14 +490,14 @@ LookupOwnPropertyInline(ExclusiveContext* cx, if (!CallResolveOp(cx->asJSContext(), MaybeRooted::toHandle(obj), MaybeRooted::toHandle(id), - MaybeRooted::toMutableHandle(propp), + MaybeRooted::toMutableHandle(propp), &recursed)) { return false; } if (recursed) { - propp.set(nullptr); + propp.setNotFound(); *donep = true; return true; } @@ -504,7 +508,7 @@ LookupOwnPropertyInline(ExclusiveContext* cx, } } - propp.set(nullptr); + propp.setNotFound(); *donep = false; return true; } @@ -515,11 +519,11 @@ LookupOwnPropertyInline(ExclusiveContext* cx, */ static inline void NativeLookupOwnPropertyNoResolve(ExclusiveContext* cx, HandleNativeObject obj, HandleId id, - MutableHandleShape result) + MutableHandle result) { // Check for a native dense element. if (JSID_IS_INT(id) && obj->containsDenseElement(JSID_TO_INT(id))) { - MarkDenseOrTypedArrayElementFound(result); + result.setDenseOrTypedArrayElement(); return; } @@ -528,15 +532,18 @@ NativeLookupOwnPropertyNoResolve(ExclusiveContext* cx, HandleNativeObject obj, H uint64_t index; if (IsTypedArrayIndex(id, &index)) { if (index < obj->as().length()) - MarkDenseOrTypedArrayElementFound(result); + result.setDenseOrTypedArrayElement(); else - result.set(nullptr); + result.setNotFound(); return; } } // Check for a native property. - result.set(obj->lookup(cx, id)); + if (Shape* shape = obj->lookup(cx, id)) + result.setNativeProperty(shape); + else + result.setNotFound(); } template @@ -545,7 +552,7 @@ LookupPropertyInline(ExclusiveContext* cx, typename MaybeRooted::HandleType obj, typename MaybeRooted::HandleType id, typename MaybeRooted::MutableHandleType objp, - typename MaybeRooted::MutableHandleType propp) + typename MaybeRooted::MutableHandleType propp) { /* NB: The logic of this procedure is implicitly reflected in * BaselineIC.cpp's |EffectlesslyLookupProperty| logic. @@ -578,14 +585,14 @@ LookupPropertyInline(ExclusiveContext* cx, MaybeRooted::toHandle(proto), MaybeRooted::toHandle(id), MaybeRooted::toMutableHandle(objp), - MaybeRooted::toMutableHandle(propp)); + MaybeRooted::toMutableHandle(propp)); } current = &proto->template as(); } objp.set(nullptr); - propp.set(nullptr); + propp.setNotFound(); return true; } diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 3625d86f54..53f7c0bfac 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -1044,7 +1044,7 @@ bool js::NativeLookupOwnProperty(ExclusiveContext* cx, typename MaybeRooted::HandleType obj, typename MaybeRooted::HandleType id, - typename MaybeRooted::MutableHandleType propp) + typename MaybeRooted::MutableHandleType propp) { bool done; return LookupOwnPropertyInline(cx, obj, id, propp, &done); @@ -1052,11 +1052,11 @@ js::NativeLookupOwnProperty(ExclusiveContext* cx, template bool js::NativeLookupOwnProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId id, - MutableHandleShape propp); + MutableHandle propp); template bool js::NativeLookupOwnProperty(ExclusiveContext* cx, NativeObject* const& obj, const jsid& id, - FakeMutableHandle propp); + FakeMutableHandle propp); /*** [[DefineOwnProperty]] ***********************************************************************/ @@ -1279,19 +1279,20 @@ GetExistingProperty(JSContext* cx, static bool GetExistingPropertyValue(ExclusiveContext* cx, HandleNativeObject obj, HandleId id, - HandleShape shape, MutableHandleValue vp) + Handle prop, MutableHandleValue vp) { - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { vp.set(obj->getDenseOrTypedArrayElement(JSID_TO_INT(id))); return true; } if (!cx->shouldBeJSContext()) return false; - MOZ_ASSERT(shape->propid() == id); - MOZ_ASSERT(obj->contains(cx, shape)); + MOZ_ASSERT(prop.shape()->propid() == id); + MOZ_ASSERT(obj->contains(cx, prop.shape())); RootedValue receiver(cx, ObjectValue(*obj)); + RootedShape shape(cx, prop.shape()); return GetExistingProperty(cx->asJSContext(), receiver, obj, shape, vp); } @@ -1302,7 +1303,7 @@ GetExistingPropertyValue(ExclusiveContext* cx, HandleNativeObject obj, HandleId */ static bool DefinePropertyIsRedundant(ExclusiveContext* cx, HandleNativeObject obj, HandleId id, - HandleShape shape, unsigned shapeAttrs, + Handle prop, unsigned shapeAttrs, Handle desc, bool *redundant) { *redundant = false; @@ -1319,16 +1320,16 @@ DefinePropertyIsRedundant(ExclusiveContext* cx, HandleNativeObject obj, HandleId if (desc.hasValue()) { // Get the current value of the existing property. RootedValue currentValue(cx); - if (!IsImplicitDenseOrTypedArrayElement(shape) && - shape->hasSlot() && - shape->hasDefaultGetter()) + if (!prop.isDenseOrTypedArrayElement() && + prop.shape()->hasSlot() && + prop.shape()->hasDefaultGetter()) { // Inline GetExistingPropertyValue in order to omit a type // correctness assertion that's too strict for this particular // call site. For details, see bug 1125624 comments 13-16. - currentValue.set(obj->getSlot(shape->slot())); + currentValue.set(obj->getSlot(prop.shape()->slot())); } else { - if (!GetExistingPropertyValue(cx, obj, id, shape, ¤tValue)) + if (!GetExistingPropertyValue(cx, obj, id, prop, ¤tValue)) return false; } @@ -1339,22 +1340,24 @@ DefinePropertyIsRedundant(ExclusiveContext* cx, HandleNativeObject obj, HandleId } GetterOp existingGetterOp = - IsImplicitDenseOrTypedArrayElement(shape) ? nullptr : shape->getter(); + prop.isDenseOrTypedArrayElement() ? nullptr : prop.shape()->getter(); if (desc.getter() != existingGetterOp) return true; SetterOp existingSetterOp = - IsImplicitDenseOrTypedArrayElement(shape) ? nullptr : shape->setter(); + prop.isDenseOrTypedArrayElement() ? nullptr : prop.shape()->setter(); if (desc.setter() != existingSetterOp) return true; } else { - if (desc.hasGetterObject()) { - if (!(shapeAttrs & JSPROP_GETTER) || desc.getterObject() != shape->getterObject()) - return true; + if (desc.hasGetterObject() && + (!(shapeAttrs & JSPROP_GETTER) || desc.getterObject() != prop.shape()->getterObject())) + { + return true; } - if (desc.hasSetterObject()) { - if (!(shapeAttrs & JSPROP_SETTER) || desc.setterObject() != shape->setterObject()) - return true; + if (desc.hasSetterObject() && + (!(shapeAttrs & JSPROP_SETTER) || desc.setterObject() != prop.shape()->setterObject())) + { + return true; } } @@ -1421,14 +1424,14 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId } // 9.1.6.1 OrdinaryDefineOwnProperty steps 1-2. - RootedShape shape(cx); + Rooted prop(cx); if (desc_.attributes() & JSPROP_RESOLVING) { // We are being called from a resolve or enumerate hook to reify a // lazily-resolved property. To avoid reentering the resolve hook and // recursing forever, skip the resolve hook when doing this lookup. - NativeLookupOwnPropertyNoResolve(cx, obj, id, &shape); + NativeLookupOwnPropertyNoResolve(cx, obj, id, &prop); } else { - if (!NativeLookupOwnProperty(cx, obj, id, &shape)) + if (!NativeLookupOwnProperty(cx, obj, id, &prop)) return false; } @@ -1443,7 +1446,7 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId Rooted desc(cx, desc_); // Step 2. - if (!shape) { + if (!prop) { if (!obj->nonProxyIsExtensible()) return result.fail(JSMSG_CANT_DEFINE_PROP_OBJECT_NOT_EXTENSIBLE); @@ -1455,21 +1458,20 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId return result.succeed(); } - MOZ_ASSERT(shape); - // Steps 3-4. (Step 3 is a special case of step 4.) We use shapeAttrs as a // stand-in for shape in many places below, since shape might not be a // pointer to a real Shape (see IsImplicitDenseOrTypedArrayElement). - unsigned shapeAttrs = GetShapeAttributes(obj, shape); + unsigned shapeAttrs = GetPropertyAttributes(obj, prop); bool redundant; - if (!DefinePropertyIsRedundant(cx, obj, id, shape, shapeAttrs, desc, &redundant)) + if (!DefinePropertyIsRedundant(cx, obj, id, prop, shapeAttrs, desc, &redundant)) return false; if (redundant) { // In cases involving JSOP_NEWOBJECT and JSOP_INITPROP, obj can have a // type for this property that doesn't match the value in the slot. // Update the type here, even though this DefineProperty call is // otherwise a no-op. (See bug 1125624 comment 13.) - if (!IsImplicitDenseOrTypedArrayElement(shape) && desc.hasValue()) { + if (!prop.isDenseOrTypedArrayElement() && desc.hasValue()) { + RootedShape shape(cx, prop.shape()); if (!UpdateShapeTypeAndValue(cx, obj, shape, desc.value())) return false; } @@ -1512,24 +1514,24 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId MOZ_ASSERT(!desc.hasSetterObject()); if (IsDataDescriptor(shapeAttrs)) { RootedValue currentValue(cx); - if (!GetExistingPropertyValue(cx, obj, id, shape, ¤tValue)) + if (!GetExistingPropertyValue(cx, obj, id, prop, ¤tValue)) return false; desc.setValue(currentValue); desc.setWritable(IsWritable(shapeAttrs)); } else { - desc.setGetterObject(shape->getterObject()); - desc.setSetterObject(shape->setterObject()); + desc.setGetterObject(prop.shape()->getterObject()); + desc.setSetterObject(prop.shape()->setterObject()); } } else if (desc.isDataDescriptor() != IsDataDescriptor(shapeAttrs)) { // Step 7. if (!IsConfigurable(shapeAttrs) && !skipRedefineChecks) return result.fail(JSMSG_CANT_REDEFINE_PROP); - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { MOZ_ASSERT(!obj->is()); if (!NativeObject::sparsifyDenseElement(cx, obj, JSID_TO_INT(id))) return false; - shape = obj->lookup(cx, id); + prop.setNativeProperty(obj->lookup(cx, id)); } // Fill in desc fields with default values (steps 7.b.i and 7.c.i). @@ -1541,15 +1543,15 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId return result.fail(JSMSG_CANT_REDEFINE_PROP); if (frozen || !desc.hasValue()) { - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { MOZ_ASSERT(!obj->is()); if (!NativeObject::sparsifyDenseElement(cx, obj, JSID_TO_INT(id))) return false; - shape = obj->lookup(cx, id); + prop.setNativeProperty(obj->lookup(cx, id)); } RootedValue currentValue(cx); - if (!GetExistingPropertyValue(cx, obj, id, shape, ¤tValue)) + if (!GetExistingPropertyValue(cx, obj, id, prop, ¤tValue)) return false; if (!desc.hasValue()) { @@ -1571,32 +1573,32 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId desc.setWritable(IsWritable(shapeAttrs)); } else { // Step 9. - MOZ_ASSERT(shape->isAccessorDescriptor()); + MOZ_ASSERT(prop.shape()->isAccessorDescriptor()); MOZ_ASSERT(desc.isAccessorDescriptor()); // The spec says to use SameValue, but since the values in // question are objects, we can just compare pointers. if (desc.hasSetterObject()) { if (!IsConfigurable(shapeAttrs) && - desc.setterObject() != shape->setterObject() && + desc.setterObject() != prop.shape()->setterObject() && !skipRedefineChecks) { return result.fail(JSMSG_CANT_REDEFINE_PROP); } } else { // Fill in desc.[[Set]] from shape. - desc.setSetterObject(shape->setterObject()); + desc.setSetterObject(prop.shape()->setterObject()); } if (desc.hasGetterObject()) { if (!IsConfigurable(shapeAttrs) && - desc.getterObject() != shape->getterObject() && + desc.getterObject() != prop.shape()->getterObject() && !skipRedefineChecks) { return result.fail(JSMSG_CANT_REDEFINE_PROP); } } else { // Fill in desc.[[Get]] from shape. - desc.setGetterObject(shape->getterObject()); + desc.setGetterObject(prop.shape()->getterObject()); } } @@ -1681,18 +1683,18 @@ bool js::NativeHasProperty(JSContext* cx, HandleNativeObject obj, HandleId id, bool* foundp) { RootedNativeObject pobj(cx, obj); - RootedShape shape(cx); + Rooted prop(cx); // This loop isn't explicit in the spec algorithm. See the comment on step // 7.a. below. for (;;) { // Steps 2-3. ('done' is a SpiderMonkey-specific thing, used below.) bool done; - if (!LookupOwnPropertyInline(cx, pobj, id, &shape, &done)) + if (!LookupOwnPropertyInline(cx, pobj, id, &prop, &done)) return false; // Step 4. - if (shape) { + if (prop) { *foundp = true; return true; } @@ -1733,15 +1735,15 @@ bool js::NativeGetOwnPropertyDescriptor(JSContext* cx, HandleNativeObject obj, HandleId id, MutableHandle desc) { - RootedShape shape(cx); - if (!NativeLookupOwnProperty(cx, obj, id, &shape)) + Rooted prop(cx); + if (!NativeLookupOwnProperty(cx, obj, id, &prop)) return false; - if (!shape) { + if (!prop) { desc.object().set(nullptr); return true; } - desc.setAttributes(GetShapeAttributes(obj, shape)); + desc.setAttributes(GetPropertyAttributes(obj, prop)); if (desc.isAccessorDescriptor()) { MOZ_ASSERT(desc.isShared()); @@ -1754,13 +1756,13 @@ js::NativeGetOwnPropertyDescriptor(JSContext* cx, HandleNativeObject obj, Handle // than return true with desc incomplete, we fill out the missing // getter or setter with a null, following CompletePropertyDescriptor. if (desc.hasGetterObject()) { - desc.setGetterObject(shape->getterObject()); + desc.setGetterObject(prop.shape()->getterObject()); } else { desc.setGetterObject(nullptr); desc.attributesRef() |= JSPROP_GETTER; } if (desc.hasSetterObject()) { - desc.setSetterObject(shape->setterObject()); + desc.setSetterObject(prop.shape()->setterObject()); } else { desc.setSetterObject(nullptr); desc.attributesRef() |= JSPROP_SETTER; @@ -1776,9 +1778,10 @@ js::NativeGetOwnPropertyDescriptor(JSContext* cx, HandleNativeObject obj, Handle desc.setSetter(nullptr); desc.attributesRef() &= ~JSPROP_SHARED; - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { desc.value().set(obj->getDenseOrTypedArrayElement(JSID_TO_INT(id))); } else { + RootedShape shape(cx, prop.shape()); if (!NativeGetExistingProperty(cx, obj, obj, shape, desc.value())) return false; } @@ -2058,23 +2061,25 @@ NativeGetPropertyInline(JSContext* cx, typename MaybeRooted::MutableHandleType vp) { typename MaybeRooted::RootType pobj(cx, obj); - typename MaybeRooted::RootType shape(cx); + typename MaybeRooted::RootType prop(cx); // This loop isn't explicit in the spec algorithm. See the comment on step // 4.d below. for (;;) { // Steps 2-3. ('done' is a SpiderMonkey-specific thing, used below.) bool done; - if (!LookupOwnPropertyInline(cx, pobj, id, &shape, &done)) + if (!LookupOwnPropertyInline(cx, pobj, id, &prop, &done)) return false; - if (shape) { + if (prop) { // Steps 5-8. Special case for dense elements because // GetExistingProperty doesn't support those. - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { vp.set(pobj->getDenseOrTypedArrayElement(JSID_TO_INT(id))); return true; } + + typename MaybeRooted::RootType shape(cx, prop.shape()); return GetExistingProperty(cx, receiver, pobj, shape, vp); } @@ -2366,11 +2371,11 @@ SetDenseOrTypedArrayElement(JSContext* cx, HandleNativeObject obj, uint32_t inde */ static bool SetExistingProperty(JSContext* cx, HandleNativeObject obj, HandleId id, HandleValue v, - HandleValue receiver, HandleNativeObject pobj, HandleShape shape, + HandleValue receiver, HandleNativeObject pobj, Handle prop, ObjectOpResult& result) { // Step 5 for dense elements. - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { // Step 5.a. if (pobj->getElementsHeader()->isFrozen()) return result.fail(JSMSG_READ_ONLY); @@ -2384,6 +2389,7 @@ SetExistingProperty(JSContext* cx, HandleNativeObject obj, HandleId id, HandleVa } // Step 5 for all other properties. + RootedShape shape(cx, prop.shape()); if (shape->isDataDescriptor()) { // Step 5.a. if (!shape->writable()) @@ -2441,7 +2447,7 @@ js::NativeSetProperty(JSContext* cx, HandleNativeObject obj, HandleId id, Handle // Step numbers below reference ES6 rev 27 9.1.9, the [[Set]] internal // method for ordinary objects. We substitute our own names for these names // used in the spec: O -> pobj, P -> id, ownDesc -> shape. - RootedShape shape(cx); + Rooted prop(cx); RootedNativeObject pobj(cx, obj); // This loop isn't explicit in the spec algorithm. See the comment on step @@ -2450,12 +2456,12 @@ js::NativeSetProperty(JSContext* cx, HandleNativeObject obj, HandleId id, Handle for (;;) { // Steps 2-3. ('done' is a SpiderMonkey-specific thing, used below.) bool done; - if (!LookupOwnPropertyInline(cx, pobj, id, &shape, &done)) + if (!LookupOwnPropertyInline(cx, pobj, id, &prop, &done)) return false; - if (shape) { + if (prop) { // Steps 5-6. - return SetExistingProperty(cx, obj, id, v, receiver, pobj, shape, result); + return SetExistingProperty(cx, obj, id, v, receiver, pobj, prop, result); } // Steps 4.a-b. The check for 'done' on this next line is tricky. @@ -2513,12 +2519,12 @@ js::NativeDeleteProperty(JSContext* cx, HandleNativeObject obj, HandleId id, ObjectOpResult& result) { // Steps 2-3. - RootedShape shape(cx); - if (!NativeLookupOwnProperty(cx, obj, id, &shape)) + Rooted prop(cx); + if (!NativeLookupOwnProperty(cx, obj, id, &prop)) return false; // Step 4. - if (!shape) { + if (!prop) { // If no property call the class's delProperty hook, passing succeeded // as the result parameter. This always succeeds when there is no hook. return CallJSDeletePropertyOp(cx, obj->getClass()->getDelProperty(), obj, id, result); @@ -2527,7 +2533,7 @@ js::NativeDeleteProperty(JSContext* cx, HandleNativeObject obj, HandleId id, cx->runtime()->gc.poke(); // Step 6. Non-configurable property. - if (GetShapeAttributes(obj, shape) & JSPROP_PERMANENT) + if (GetPropertyAttributes(obj, prop) & JSPROP_PERMANENT) return result.failCantDelete(); if (!CallJSDeletePropertyOp(cx, obj->getClass()->getDelProperty(), obj, id, result)) @@ -2536,7 +2542,7 @@ js::NativeDeleteProperty(JSContext* cx, HandleNativeObject obj, HandleId id, return true; // Step 5. - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { // Typed array elements are non-configurable. MOZ_ASSERT(!obj->is()); diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index 657fc8d57c..abc84c9fd1 100644 --- a/js/src/vm/NativeObject.h +++ b/js/src/vm/NativeObject.h @@ -1459,7 +1459,7 @@ extern bool NativeLookupOwnProperty(ExclusiveContext* cx, typename MaybeRooted::HandleType obj, typename MaybeRooted::HandleType id, - typename MaybeRooted::MutableHandleType propp); + typename MaybeRooted::MutableHandleType propp); /* * Get a property from `receiver`, after having already done a lookup and found diff --git a/js/src/vm/Shape-inl.h b/js/src/vm/Shape-inl.h index 32754740a5..ab16bfb192 100644 --- a/js/src/vm/Shape-inl.h +++ b/js/src/vm/Shape-inl.h @@ -191,17 +191,17 @@ AutoRooterGetterSetter::AutoRooterGetterSetter(ExclusiveContext* cx, uint8_t att } static inline uint8_t -GetShapeAttributes(JSObject* obj, Shape* shape) +GetPropertyAttributes(JSObject* obj, PropertyResult prop) { MOZ_ASSERT(obj->isNative()); - if (IsImplicitDenseOrTypedArrayElement(shape)) { + if (prop.isDenseOrTypedArrayElement()) { if (obj->is()) return JSPROP_ENUMERATE | JSPROP_PERMANENT; return obj->as().getElementsHeader()->elementAttributes(); } - return shape->attributes(); + return prop.shape()->attributes(); } } /* namespace js */ diff --git a/js/src/vm/Shape.cpp b/js/src/vm/Shape.cpp index 7cdf382ed6..c71cef5a7e 100644 --- a/js/src/vm/Shape.cpp +++ b/js/src/vm/Shape.cpp @@ -1771,3 +1771,10 @@ JS::ubi::Concrete::size(mozilla::MallocSizeOf mallocSizeOf) const { return js::gc::Arena::thingSize(get().asTenured().getAllocKind()); } + +void +PropertyResult::trace(JSTracer* trc) +{ + if (isNativeProperty()) + TraceRoot(trc, &shape_, "PropertyResult::shape_"); +} diff --git a/js/src/vm/Shape.h b/js/src/vm/Shape.h index b292bd58f2..bb813997f0 100644 --- a/js/src/vm/Shape.h +++ b/js/src/vm/Shape.h @@ -1541,38 +1541,6 @@ Shape::matches(const StackShape& other) const other.rawGetter, other.rawSetter); } -// Property lookup hooks on objects are required to return a non-nullptr shape -// to signify that the property has been found. For cases where the property is -// not actually represented by a Shape, use a dummy value. This includes all -// properties of non-native objects, and dense elements for native objects. -// Use separate APIs for these two cases. - -template -static inline void -MarkNonNativePropertyFound(typename MaybeRooted::MutableHandleType propp) -{ - propp.set(reinterpret_cast(1)); -} - -template -static inline void -MarkDenseOrTypedArrayElementFound(typename MaybeRooted::MutableHandleType propp) -{ - propp.set(reinterpret_cast(1)); -} - -static inline bool -IsImplicitDenseOrTypedArrayElement(Shape* prop) -{ - return prop == reinterpret_cast(1); -} - -static inline bool -IsImplicitNonNativeProperty(Shape* prop) -{ - return prop == reinterpret_cast(1); -} - Shape* ReshapeForAllocKind(JSContext* cx, Shape* shape, TaggedProto proto, gc::AllocKind allocKind); diff --git a/js/src/vm/UnboxedObject.cpp b/js/src/vm/UnboxedObject.cpp index de6bdef9e3..2ed89e32e4 100644 --- a/js/src/vm/UnboxedObject.cpp +++ b/js/src/vm/UnboxedObject.cpp @@ -720,10 +720,10 @@ UnboxedPlainObject::createWithProperties(ExclusiveContext* cx, HandleObjectGroup /* static */ bool UnboxedPlainObject::obj_lookupProperty(JSContext* cx, HandleObject obj, HandleId id, MutableHandleObject objp, - MutableHandleShape propp) + MutableHandle propp) { if (obj->as().containsUnboxedOrExpandoProperty(cx, id)) { - MarkNonNativePropertyFound(propp); + propp.setNonNativeProperty(); objp.set(obj); return true; } @@ -731,7 +731,7 @@ UnboxedPlainObject::obj_lookupProperty(JSContext* cx, HandleObject obj, RootedObject proto(cx, obj->staticPrototype()); if (!proto) { objp.set(nullptr); - propp.set(nullptr); + propp.setNotFound(); return true; } @@ -1411,10 +1411,10 @@ UnboxedArrayObject::containsProperty(ExclusiveContext* cx, jsid id) /* static */ bool UnboxedArrayObject::obj_lookupProperty(JSContext* cx, HandleObject obj, HandleId id, MutableHandleObject objp, - MutableHandleShape propp) + MutableHandle propp) { if (obj->as().containsProperty(cx, id)) { - MarkNonNativePropertyFound(propp); + propp.setNonNativeProperty(); objp.set(obj); return true; } @@ -1422,7 +1422,7 @@ UnboxedArrayObject::obj_lookupProperty(JSContext* cx, HandleObject obj, RootedObject proto(cx, obj->staticPrototype()); if (!proto) { objp.set(nullptr); - propp.set(nullptr); + propp.setNotFound(); return true; } diff --git a/js/src/vm/UnboxedObject.h b/js/src/vm/UnboxedObject.h index 8622be8a70..6fc482ec71 100644 --- a/js/src/vm/UnboxedObject.h +++ b/js/src/vm/UnboxedObject.h @@ -242,7 +242,7 @@ class UnboxedPlainObject : public JSObject static bool obj_lookupProperty(JSContext* cx, HandleObject obj, HandleId id, MutableHandleObject objp, - MutableHandleShape propp); + MutableHandle propp); static bool obj_defineProperty(JSContext* cx, HandleObject obj, HandleId id, Handle desc, @@ -378,7 +378,7 @@ class UnboxedArrayObject : public JSObject static bool obj_lookupProperty(JSContext* cx, HandleObject obj, HandleId id, MutableHandleObject objp, - MutableHandleShape propp); + MutableHandle propp); static bool obj_defineProperty(JSContext* cx, HandleObject obj, HandleId id, Handle desc, From 30a3ec27a222140ff0902771049dc3212ccc8d33 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 25 May 2022 18:33:46 +0000 Subject: [PATCH 04/13] Issue #1742 - Remove the ambiguous portions of gc that dealt with tagged pointers for shapes. Nobody likes "isNullLike(obj)" when trying to garbage collect. It either is or is not. Master Yoda would be pleased with this change. --- js/src/gc/Barrier.h | 4 ---- js/src/gc/Heap.h | 7 +------ js/src/gc/Marking.cpp | 2 +- js/src/gc/Marking.h | 8 -------- js/src/jsobj.h | 5 ----- js/src/vm/String.h | 2 +- js/src/vm/TaggedProto.h | 4 ---- 7 files changed, 3 insertions(+), 29 deletions(-) diff --git a/js/src/gc/Barrier.h b/js/src/gc/Barrier.h index d10fb4321c..25f153207f 100644 --- a/js/src/gc/Barrier.h +++ b/js/src/gc/Barrier.h @@ -261,8 +261,6 @@ struct InternalBarrierMethods { static bool isMarkable(T* v) { return v != nullptr; } - static bool isMarkableTaggedPointer(T* v) { return !IsNullTaggedPointer(v); } - static void preBarrier(T* v) { T::writeBarrierPre(v); } static void postBarrier(T** vp, T* prev, T* next) { T::writeBarrierPost(vp, prev, next); } @@ -282,7 +280,6 @@ template <> struct InternalBarrierMethods { static bool isMarkable(const Value& v) { return v.isGCThing(); } - static bool isMarkableTaggedPointer(const Value& v) { return isMarkable(v); } static void preBarrier(const Value& v) { DispatchTyped(PreBarrierFunctor(), v); @@ -318,7 +315,6 @@ template <> struct InternalBarrierMethods { static bool isMarkable(jsid id) { return JSID_IS_GCTHING(id); } - static bool isMarkableTaggedPointer(jsid id) { return isMarkable(id); } static void preBarrier(jsid id) { DispatchTyped(PreBarrierFunctor(), id); } static void postBarrier(jsid* idp, jsid prev, jsid next) {} diff --git a/js/src/gc/Heap.h b/js/src/gc/Heap.h index 2f27702601..ce4a0dbd56 100644 --- a/js/src/gc/Heap.h +++ b/js/src/gc/Heap.h @@ -317,10 +317,6 @@ class TenuredCell : public Cell MOZ_ALWAYS_INLINE void unmark(uint32_t color) const; MOZ_ALWAYS_INLINE void copyMarkBitsFrom(const TenuredCell* src); - // Note: this is in TenuredCell because JSObject subclasses are sometimes - // used tagged. - static MOZ_ALWAYS_INLINE bool isNullLike(const Cell* thing) { return !thing; } - // Access to the arena. inline Arena* arena() const; inline AllocKind getAllocKind() const; @@ -1302,7 +1298,7 @@ TenuredCell::isInsideZone(JS::Zone* zone) const TenuredCell::readBarrier(TenuredCell* thing) { MOZ_ASSERT(!CurrentThreadIsIonCompiling()); - MOZ_ASSERT(!isNullLike(thing)); + MOZ_ASSERT(thing); // It would be good if barriers were never triggered during collection, but // at the moment this can happen e.g. when rekeying tables containing @@ -1335,7 +1331,6 @@ AssertSafeToSkipBarrier(TenuredCell* thing); TenuredCell::writeBarrierPre(TenuredCell* thing) { MOZ_ASSERT(!CurrentThreadIsIonCompiling()); - MOZ_ASSERT_IF(thing, !isNullLike(thing)); if (!thing) return; diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 241a1df20d..da8da5c6b5 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -501,7 +501,7 @@ void js::TraceNullableRoot(JSTracer* trc, T* thingp, const char* name) { AssertRootMarkingPhase(trc); - if (InternalBarrierMethods::isMarkableTaggedPointer(*thingp)) + if (InternalBarrierMethods::isMarkable(*thingp)) DispatchToTracer(trc, ConvertToBase(thingp), name); } diff --git a/js/src/gc/Marking.h b/js/src/gc/Marking.h index 2b9e7ace1f..40b331b311 100644 --- a/js/src/gc/Marking.h +++ b/js/src/gc/Marking.h @@ -414,14 +414,6 @@ ToMarkable(Cell* cell) return cell; } -// Return true if the pointer is nullptr, or if it is a tagged pointer to -// nullptr. -MOZ_ALWAYS_INLINE bool -IsNullTaggedPointer(void* p) -{ - return uintptr_t(p) <= LargestTaggedNullCellPointer; -} - // Wrap a GC thing pointer into a new Value or jsid. The type system enforces // that the thing pointer is a wrappable type. template diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 68491a1aa9..5e0cc347f9 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -299,7 +299,6 @@ class JSObject : public js::gc::Cell static const JS::TraceKind TraceKind = JS::TraceKind::Object; static const size_t MaxTagBits = 3; - static bool isNullLike(const JSObject* obj) { return uintptr_t(obj) < (1 << MaxTagBits); } MOZ_ALWAYS_INLINE JS::Zone* zone() const { return group_->zone(); @@ -635,7 +634,6 @@ struct JSObject_Slots16 : JSObject { void* data[3]; js::Value fslots[16]; }; /* static */ MOZ_ALWAYS_INLINE void JSObject::readBarrier(JSObject* obj) { - MOZ_ASSERT_IF(obj, !isNullLike(obj)); if (obj && obj->isTenured()) obj->asTenured().readBarrier(&obj->asTenured()); } @@ -643,7 +641,6 @@ JSObject::readBarrier(JSObject* obj) /* static */ MOZ_ALWAYS_INLINE void JSObject::writeBarrierPre(JSObject* obj) { - MOZ_ASSERT_IF(obj, !isNullLike(obj)); if (obj && obj->isTenured()) obj->asTenured().writeBarrierPre(&obj->asTenured()); } @@ -652,8 +649,6 @@ JSObject::writeBarrierPre(JSObject* obj) JSObject::writeBarrierPost(void* cellp, JSObject* prev, JSObject* next) { MOZ_ASSERT(cellp); - MOZ_ASSERT_IF(next, !IsNullTaggedPointer(next)); - MOZ_ASSERT_IF(prev, !IsNullTaggedPointer(prev)); // If the target needs an entry, add it. js::gc::StoreBuffer* buffer; diff --git a/js/src/vm/String.h b/js/src/vm/String.h index 0937ac24d2..4c43439cd9 100644 --- a/js/src/vm/String.h +++ b/js/src/vm/String.h @@ -521,7 +521,7 @@ class JSString : public js::gc::TenuredCell } static MOZ_ALWAYS_INLINE void writeBarrierPre(JSString* thing) { - if (isNullLike(thing) || thing->isPermanentAtom()) + if (!thing || thing->isPermanentAtom()) return; TenuredCell::writeBarrierPre(thing); diff --git a/js/src/vm/TaggedProto.h b/js/src/vm/TaggedProto.h index 5b9fe5c7e8..10b8c1171b 100644 --- a/js/src/vm/TaggedProto.h +++ b/js/src/vm/TaggedProto.h @@ -68,10 +68,6 @@ struct InternalBarrierMethods static void readBarrier(const TaggedProto& proto); - static bool isMarkableTaggedPointer(TaggedProto proto) { - return proto.isObject(); - } - static bool isMarkable(TaggedProto proto) { return proto.isObject(); } From e632fc1e8cfe267f33d4cdd281377a7b20e65618 Mon Sep 17 00:00:00 2001 From: Jeremy Andrews Date: Thu, 26 May 2022 18:57:18 -0500 Subject: [PATCH 05/13] Issue #1742 - Part 4: Don't trigger read barriers when comparing wrapped pointers types This is actually an undocumented dependency of Bug 1325406 for Linux and SunOS. Ref: BZ 1308236 --- dom/plugins/base/nsJSNPRuntime.cpp | 2 +- js/ipc/JavaScriptShared.cpp | 2 +- js/public/RootingAPI.h | 200 +++++++++++++++++- js/src/gc/Barrier.h | 46 +++- js/src/jsapi-tests/testGCHeapPostBarriers.cpp | 108 ++++++++++ js/src/vm/SharedMem.h | 4 +- js/xpconnect/src/XPCInlines.h | 2 +- js/xpconnect/src/XPCWrappedNative.cpp | 2 +- js/xpconnect/src/XPCWrappedNativeProto.cpp | 4 +- 9 files changed, 343 insertions(+), 27 deletions(-) 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. } From 1cdbf6eb6a9e4364d3c3df0a6e8f6dfa0c8deca8 Mon Sep 17 00:00:00 2001 From: Jeremy Andrews Date: Fri, 27 May 2022 07:25:48 -0500 Subject: [PATCH 06/13] Issue #1742 - Part 5: Remove unnecessary GCPtr::unbarrieredGet Ref: BZ 1323441 --- js/src/gc/Barrier.h | 4 ---- js/src/vm/NativeObject-inl.h | 2 +- js/src/vm/ObjectGroup.cpp | 2 +- js/src/vm/Shape-inl.h | 2 +- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/js/src/gc/Barrier.h b/js/src/gc/Barrier.h index 1ec269abf3..345131a296 100644 --- a/js/src/gc/Barrier.h +++ b/js/src/gc/Barrier.h @@ -454,10 +454,6 @@ class GCPtr : public WriteBarrieredBase DECLARE_POINTER_ASSIGN_OPS(GCPtr, T); - T unbarrieredGet() const { - return this->value; - } - private: void set(const T& v) { this->pre(); diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index 67dff24b7b..004b308f05 100644 --- a/js/src/vm/NativeObject-inl.h +++ b/js/src/vm/NativeObject-inl.h @@ -290,7 +290,7 @@ NativeObject::setSlotWithType(ExclusiveContext* cx, Shape* shape, inline void NativeObject::updateShapeAfterMovingGC() { - Shape* shape = shape_.unbarrieredGet(); + Shape* shape = shape_; if (IsForwarded(shape)) shape_.unsafeSet(Forwarded(shape)); } diff --git a/js/src/vm/ObjectGroup.cpp b/js/src/vm/ObjectGroup.cpp index f2a3305cfd..741531f015 100644 --- a/js/src/vm/ObjectGroup.cpp +++ b/js/src/vm/ObjectGroup.cpp @@ -421,7 +421,7 @@ struct ObjectGroupCompartment::NewEntry } static inline bool match(const ObjectGroupCompartment::NewEntry& key, const Lookup& lookup) { - TaggedProto proto = key.group.unbarrieredGet()->proto().unbarrieredGet(); + TaggedProto proto = key.group.unbarrieredGet()->proto(); JSObject* assoc = key.associated; MOZ_ASSERT(proto.hasUniqueId()); MOZ_ASSERT_IF(assoc, assoc->zone()->hasUniqueId(assoc)); diff --git a/js/src/vm/Shape-inl.h b/js/src/vm/Shape-inl.h index ab16bfb192..7b7561220e 100644 --- a/js/src/vm/Shape-inl.h +++ b/js/src/vm/Shape-inl.h @@ -123,7 +123,7 @@ Shape::new_(ExclusiveContext* cx, Handle other, uint32_t nfixed) inline void Shape::updateBaseShapeAfterMovingGC() { - BaseShape* base = base_.unbarrieredGet(); + BaseShape* base = base_; if (IsForwarded(base)) base_.unsafeSet(Forwarded(base)); } From c782076add034f2204ee7cd31a1e4e40d75038a4 Mon Sep 17 00:00:00 2001 From: trav90 Date: Sat, 28 May 2022 11:45:35 -0500 Subject: [PATCH 07/13] Issue #457 - Remove the constructor from gfxShapedText::CompressedGlyph and make it a trivial class Also provide a couple of convenience "factory" methods to create simple and complex glyph values. --- gfx/thebes/gfxCoreTextShaper.cpp | 15 +++---- gfx/thebes/gfxFT2Fonts.cpp | 17 ++++---- gfx/thebes/gfxFont.cpp | 6 +-- gfx/thebes/gfxFont.h | 67 ++++++++++++++++++++++---------- gfx/thebes/gfxGraphiteShaper.cpp | 21 +++++----- gfx/thebes/gfxHarfBuzzShaper.cpp | 22 ++++++----- gfx/thebes/gfxTextRun.cpp | 38 +++++++++--------- gfx/thebes/gfxTextRun.h | 31 ++++++++------- layout/mathml/nsMathMLChar.cpp | 22 +++++------ 9 files changed, 137 insertions(+), 102 deletions(-) diff --git a/gfx/thebes/gfxCoreTextShaper.cpp b/gfx/thebes/gfxCoreTextShaper.cpp index 08217b82f9..254b0b63ef 100644 --- a/gfx/thebes/gfxCoreTextShaper.cpp +++ b/gfx/thebes/gfxCoreTextShaper.cpp @@ -310,6 +310,8 @@ gfxCoreTextShaper::SetGlyphsFromRun(gfxShapedText *aShapedText, CTRunRef aCTRun, int32_t aStringOffset) { + typedef gfxShapedText::CompressedGlyph CompressedGlyph; + // The word has been bidi-wrapped; aStringOffset is the number // of chars at the beginning of the CTLine that we should skip. // aCTRun is a glyph run from the CoreText layout process. @@ -392,8 +394,7 @@ gfxCoreTextShaper::SetGlyphsFromRun(gfxShapedText *aShapedText, nullptr, nullptr, nullptr); AutoTArray detailedGlyphs; - gfxShapedText::CompressedGlyph *charGlyphs = - aShapedText->GetCharacterGlyphs() + aOffset; + CompressedGlyph* charGlyphs = aShapedText->GetCharacterGlyphs() + aOffset; // CoreText gives us the glyphindex-to-charindex mapping, which relates each glyph // to a source text character; we also need the charindex-to-glyphindex mapping to @@ -614,10 +615,10 @@ gfxCoreTextShaper::SetGlyphsFromRun(gfxShapedText *aShapedText, advance = int32_t(toNextGlyph * appUnitsPerDevUnit); } - gfxTextRun::CompressedGlyph textRunGlyph; - textRunGlyph.SetComplex(charGlyphs[baseCharIndex].IsClusterStart(), - true, detailedGlyphs.Length()); - aShapedText->SetGlyphs(aOffset + baseCharIndex, textRunGlyph, + bool isClusterStart = charGlyphs[baseCharIndex].IsClusterStart(); + aShapedText->SetGlyphs(aOffset + baseCharIndex, + CompressedGlyph::MakeComplex(isClusterStart, true, + detailedGlyphs.Length()), detailedGlyphs.Elements()); detailedGlyphs.Clear(); @@ -625,7 +626,7 @@ gfxCoreTextShaper::SetGlyphsFromRun(gfxShapedText *aShapedText, // the rest of the chars in the group are ligature continuations, no associated glyphs while (++baseCharIndex != endCharIndex && baseCharIndex < wordLength) { - gfxShapedText::CompressedGlyph &shapedTextGlyph = charGlyphs[baseCharIndex]; + CompressedGlyph &shapedTextGlyph = charGlyphs[baseCharIndex]; NS_ASSERTION(!shapedTextGlyph.IsSimpleGlyph(), "overwriting a simple glyph"); shapedTextGlyph.SetComplex(inOrder && shapedTextGlyph.IsClusterStart(), false, 0); } diff --git a/gfx/thebes/gfxFT2Fonts.cpp b/gfx/thebes/gfxFT2Fonts.cpp index 1902541911..dfb94bbb69 100644 --- a/gfx/thebes/gfxFT2Fonts.cpp +++ b/gfx/thebes/gfxFT2Fonts.cpp @@ -62,13 +62,14 @@ void gfxFT2Font::AddRange(const char16_t *aText, uint32_t aOffset, uint32_t aLength, gfxShapedText *aShapedText) { + typedef gfxShapedText::CompressedGlyph CompressedGlyph; + const uint32_t appUnitsPerDevUnit = aShapedText->GetAppUnitsPerDevUnit(); // we'll pass this in/figure it out dynamically, but at this point there can be only one face. gfxFT2LockedFace faceLock(this); FT_Face face = faceLock.get(); - gfxShapedText::CompressedGlyph *charGlyphs = - aShapedText->GetCharacterGlyphs(); + CompressedGlyph* charGlyphs = aShapedText->GetCharacterGlyphs(); const gfxFT2Font::CachedGlyphData *cgd = nullptr, *cgdNext = nullptr; @@ -135,8 +136,8 @@ gfxFT2Font::AddRange(const char16_t *aText, uint32_t aOffset, } if (advance >= 0 && - gfxShapedText::CompressedGlyph::IsSimpleAdvance(advance) && - gfxShapedText::CompressedGlyph::IsSimpleGlyphID(gid)) { + CompressedGlyph::IsSimpleAdvance(advance) && + CompressedGlyph::IsSimpleGlyphID(gid)) { charGlyphs[aOffset].SetSimpleGlyph(advance, gid); } else if (gid == 0) { // gid = 0 only happens when the glyph is missing from the font @@ -149,9 +150,11 @@ gfxFT2Font::AddRange(const char16_t *aText, uint32_t aOffset, details.mAdvance = advance; details.mXOffset = 0; details.mYOffset = 0; - gfxShapedText::CompressedGlyph g; - g.SetComplex(charGlyphs[aOffset].IsClusterStart(), true, 1); - aShapedText->SetGlyphs(aOffset, g, &details); + bool isClusterStart = charGlyphs[aOffset].IsClusterStart(); + aShapedText->SetGlyphs(aOffset, + CompressedGlyph::MakeComplex(isClusterStart, + true, 1), + &details); } } } diff --git a/gfx/thebes/gfxFont.cpp b/gfx/thebes/gfxFont.cpp index 9acfdeb95f..f79c5cbd72 100644 --- a/gfx/thebes/gfxFont.cpp +++ b/gfx/thebes/gfxFont.cpp @@ -639,10 +639,10 @@ gfxShapedText::SetupClusterBoundaries(uint32_t aOffset, const char16_t *aString, uint32_t aLength) { - CompressedGlyph *glyphs = GetCharacterGlyphs() + aOffset; + CompressedGlyph* glyphs = GetCharacterGlyphs() + aOffset; - gfxTextRun::CompressedGlyph extendCluster; - extendCluster.SetComplex(false, true, 0); + CompressedGlyph extendCluster = + CompressedGlyph::MakeComplex(false, true, 0); ClusterIterator iter(aString, aLength); diff --git a/gfx/thebes/gfxFont.h b/gfx/thebes/gfxFont.h index ab1677b9c5..36687585a9 100644 --- a/gfx/thebes/gfxFont.h +++ b/gfx/thebes/gfxFont.h @@ -245,7 +245,7 @@ struct gfxTextRange { /** * Font cache design: - * + * * The mFonts hashtable contains most fonts, indexed by (gfxFontEntry*, style). * It does not add a reference to the fonts it contains. * When a font's refcount decreases to zero, instead of deleting it we @@ -699,21 +699,19 @@ public: * This class records the information associated with a character in the * input string. It's optimized for the case where there is one glyph * representing that character alone. - * + * * A character can have zero or more associated glyphs. Each glyph * has an advance width and an x and y offset. * A character may be the start of a cluster. * A character may be the start of a ligature group. * A character can be "missing", indicating that the system is unable * to render the character. - * + * * All characters in a ligature group conceptually share all the glyphs * associated with the characters in a group. */ class CompressedGlyph { public: - CompressedGlyph() { mValue = 0; } - enum { // Indicates that a cluster and ligature group starts at this // character; this character has a single glyph with a reasonable @@ -843,25 +841,52 @@ public: return toggle; } - CompressedGlyph& SetSimpleGlyph(uint32_t aAdvanceAppUnits, uint32_t aGlyph) { + // Create a CompressedGlyph value representing a simple glyph with + // no extra flags (line-break or is_space) set. + static CompressedGlyph + MakeSimpleGlyph(uint32_t aAdvanceAppUnits, uint32_t aGlyph) { NS_ASSERTION(IsSimpleAdvance(aAdvanceAppUnits), "Advance overflow"); NS_ASSERTION(IsSimpleGlyphID(aGlyph), "Glyph overflow"); + CompressedGlyph g; + g.mValue = FLAG_IS_SIMPLE_GLYPH | + (aAdvanceAppUnits << ADVANCE_SHIFT) | + aGlyph; + return g; + } + + // Assign a simple glyph value to an existing CompressedGlyph record, + // preserving line-break/is-space flags if present. + CompressedGlyph& SetSimpleGlyph(uint32_t aAdvanceAppUnits, + uint32_t aGlyph) { NS_ASSERTION(!CharTypeFlags(), "Char type flags lost"); mValue = (mValue & (FLAGS_CAN_BREAK_BEFORE | FLAG_CHAR_IS_SPACE)) | - FLAG_IS_SIMPLE_GLYPH | - (aAdvanceAppUnits << ADVANCE_SHIFT) | aGlyph; + MakeSimpleGlyph(aAdvanceAppUnits, aGlyph).mValue; return *this; } + + // Create a CompressedGlyph value representing a complex glyph record, + // without any line-break or char-type flags. + static CompressedGlyph + MakeComplex(bool aClusterStart, bool aLigatureStart, + uint32_t aGlyphCount) { + CompressedGlyph g; + g.mValue = FLAG_NOT_MISSING | + (aClusterStart ? 0 : FLAG_NOT_CLUSTER_START) | + (aLigatureStart ? 0 : FLAG_NOT_LIGATURE_GROUP_START) | + (aGlyphCount << GLYPH_COUNT_SHIFT); + return g; + } + + // Assign a complex glyph value to an existing CompressedGlyph record, + // preserving line-break/char-type flags if present. CompressedGlyph& SetComplex(bool aClusterStart, bool aLigatureStart, - uint32_t aGlyphCount) { + uint32_t aGlyphCount) { mValue = (mValue & (FLAGS_CAN_BREAK_BEFORE | FLAG_CHAR_IS_SPACE)) | - FLAG_NOT_MISSING | CharTypeFlags() | - (aClusterStart ? 0 : FLAG_NOT_CLUSTER_START) | - (aLigatureStart ? 0 : FLAG_NOT_LIGATURE_GROUP_START) | - (aGlyphCount << GLYPH_COUNT_SHIFT); + MakeComplex(aClusterStart, aLigatureStart, aGlyphCount).mValue; return *this; } + /** * Missing glyphs are treated as ligature group starts; don't mess with * the cluster-start flag (see bugs 618870 and 619286). @@ -914,7 +939,7 @@ public: /** The advance, x-offset and y-offset of the glyph, in appunits * mAdvance is in the text direction (RTL or LTR) * mXOffset is always from left to right - * mYOffset is always from top to bottom */ + * mYOffset is always from top to bottom */ int32_t mAdvance; float mXOffset, mYOffset; }; @@ -1039,7 +1064,7 @@ protected: // For characters whose glyph data does not fit the "simple" glyph criteria // in CompressedGlyph, we use a sorted array to store the association - // between the source character offset and an index into an array + // between the source character offset and an index into an array // DetailedGlyphs. The CompressedGlyph record includes a count of // the number of DetailedGlyph records that belong to the character, // starting at the given index. @@ -1581,11 +1606,11 @@ public: // (offset1, length1) plus the advance width of (offset1 + length1, // length2) should be the advance width of (offset1, length1 + length2) gfxFloat mAdvanceWidth; - + // For zero-width substrings, these must be zero! gfxFloat mAscent; // always non-negative gfxFloat mDescent; // always non-negative - + // Bounding box that is guaranteed to include everything drawn. // If a tight boundingBox was requested when these metrics were // generated, this will tightly wrap the glyphs, otherwise it is @@ -1648,11 +1673,11 @@ public: * @param aSpacing spacing to insert before and after glyphs. The bounding box * need not include the spacing itself, but the spacing affects the glyph * positions. null if there is no spacing. - * + * * Callers guarantee: * -- aStart and aEnd are aligned to cluster and ligature boundaries * -- all glyphs use this font - * + * * The default implementation just uses font metrics and aTextRun's * advances, and assumes no characters fall outside the font box. In * general this is insufficient, because that assumption is not always true. @@ -1707,7 +1732,7 @@ public: (mUnicodeRangeMap && !mUnicodeRangeMap->test(ch))) { return false; } - return mFontEntry->HasCharacter(ch); + return mFontEntry->HasCharacter(ch); } const gfxCharacterMap* GetUnicodeRangeMap() const { @@ -1722,7 +1747,7 @@ public: if (!mIsValid) { return 0; } - return mFontEntry->GetUVSGlyph(aCh, aVS); + return mFontEntry->GetUVSGlyph(aCh, aVS); } template diff --git a/gfx/thebes/gfxGraphiteShaper.cpp b/gfx/thebes/gfxGraphiteShaper.cpp index aeebf30f28..fe93f376f4 100644 --- a/gfx/thebes/gfxGraphiteShaper.cpp +++ b/gfx/thebes/gfxGraphiteShaper.cpp @@ -215,6 +215,8 @@ gfxGraphiteShaper::SetGlyphsFromSegment(DrawTarget *aDrawTarget, const char16_t *aText, gr_segment *aSegment) { + typedef gfxShapedText::CompressedGlyph CompressedGlyph; + int32_t dev2appUnits = aShapedText->GetAppUnitsPerDevUnit(); bool rtl = aShapedText->IsRightToLeft(); @@ -291,8 +293,7 @@ gfxGraphiteShaper::SetGlyphsFromSegment(DrawTarget *aDrawTarget, bool roundX, roundY; GetRoundOffsetsToPixels(aDrawTarget, &roundX, &roundY); - gfxShapedText::CompressedGlyph *charGlyphs = - aShapedText->GetCharacterGlyphs() + aOffset; + CompressedGlyph* charGlyphs = aShapedText->GetCharacterGlyphs() + aOffset; // now put glyphs into the textrun, one cluster at a time for (uint32_t i = 0; i <= cIndex; ++i) { @@ -325,8 +326,8 @@ gfxGraphiteShaper::SetGlyphsFromSegment(DrawTarget *aDrawTarget, uint32_t appAdvance = roundX ? NSToIntRound(adv) * dev2appUnits : NSToIntRound(adv * dev2appUnits); if (c.nGlyphs == 1 && - gfxShapedText::CompressedGlyph::IsSimpleGlyphID(gids[c.baseGlyph]) && - gfxShapedText::CompressedGlyph::IsSimpleAdvance(appAdvance) && + CompressedGlyph::IsSimpleGlyphID(gids[c.baseGlyph]) && + CompressedGlyph::IsSimpleAdvance(appAdvance) && charGlyphs[offs].IsClusterStart() && yLocs[c.baseGlyph] == 0) { @@ -352,15 +353,17 @@ gfxGraphiteShaper::SetGlyphsFromSegment(DrawTarget *aDrawTarget, d->mAdvance = 0; } } - gfxShapedText::CompressedGlyph g; - g.SetComplex(charGlyphs[offs].IsClusterStart(), - true, details.Length()); - aShapedText->SetGlyphs(aOffset + offs, g, details.Elements()); + bool isClusterStart = charGlyphs[offs].IsClusterStart(); + aShapedText->SetGlyphs(aOffset + offs, + CompressedGlyph::MakeComplex(isClusterStart, + true, + details.Length()), + details.Elements()); } for (uint32_t j = c.baseChar + 1; j < c.baseChar + c.nChars; ++j) { NS_ASSERTION(j < aLength, "unexpected offset"); - gfxShapedText::CompressedGlyph &g = charGlyphs[j]; + CompressedGlyph &g = charGlyphs[j]; NS_ASSERTION(!g.IsSimpleGlyph(), "overwriting a simple glyph"); g.SetComplex(g.IsClusterStart(), false, 0); } diff --git a/gfx/thebes/gfxHarfBuzzShaper.cpp b/gfx/thebes/gfxHarfBuzzShaper.cpp index 4a563a95f9..7f05f9011f 100644 --- a/gfx/thebes/gfxHarfBuzzShaper.cpp +++ b/gfx/thebes/gfxHarfBuzzShaper.cpp @@ -817,7 +817,7 @@ GetKernValueVersion1Fmt3(const void* aSubtable, hdr->leftClassCount * hdr->rightClassCount > aSubtableLen) { return 0; } - + if (aFirstGlyph >= glyphCount || aSecondGlyph >= glyphCount) { // glyphs are out of range for the class tables return 0; @@ -1503,6 +1503,8 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(DrawTarget *aDrawTarget, hb_buffer_t *aBuffer, bool aVertical) { + typedef gfxShapedText::CompressedGlyph CompressedGlyph; + uint32_t numGlyphs; const hb_glyph_info_t *ginfo = hb_buffer_get_glyph_infos(aBuffer, &numGlyphs); if (numGlyphs == 0) { @@ -1541,8 +1543,7 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(DrawTarget *aDrawTarget, } int32_t appUnitsPerDevUnit = aShapedText->GetAppUnitsPerDevUnit(); - gfxShapedText::CompressedGlyph *charGlyphs = - aShapedText->GetCharacterGlyphs() + aOffset; + CompressedGlyph* charGlyphs = aShapedText->GetCharacterGlyphs() + aOffset; // factor to convert 16.16 fixed-point pixels to app units // (only used if not rounding) @@ -1688,8 +1689,8 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(DrawTarget *aDrawTarget, } // Check if it's a simple one-to-one mapping if (glyphsInClump == 1 && - gfxTextRun::CompressedGlyph::IsSimpleGlyphID(ginfo[glyphStart].codepoint) && - gfxTextRun::CompressedGlyph::IsSimpleAdvance(advance) && + CompressedGlyph::IsSimpleGlyphID(ginfo[glyphStart].codepoint) && + CompressedGlyph::IsSimpleAdvance(advance) && charGlyphs[baseCharIndex].IsClusterStart() && iOffset == 0 && b_offset == 0 && b_advance == 0 && bPos == 0) @@ -1760,11 +1761,12 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(DrawTarget *aDrawTarget, } } - gfxShapedText::CompressedGlyph g; - g.SetComplex(charGlyphs[baseCharIndex].IsClusterStart(), - true, detailedGlyphs.Length()); + bool isClusterStart = charGlyphs[baseCharIndex].IsClusterStart(); aShapedText->SetGlyphs(aOffset + baseCharIndex, - g, detailedGlyphs.Elements()); + CompressedGlyph::MakeComplex(isClusterStart, + true, + detailedGlyphs.Length()), + detailedGlyphs.Elements()); detailedGlyphs.Clear(); } @@ -1773,7 +1775,7 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(DrawTarget *aDrawTarget, // no associated glyphs while (++baseCharIndex != endCharIndex && baseCharIndex < int32_t(wordLength)) { - gfxShapedText::CompressedGlyph &g = charGlyphs[baseCharIndex]; + CompressedGlyph &g = charGlyphs[baseCharIndex]; NS_ASSERTION(!g.IsSimpleGlyph(), "overwriting a simple glyph"); g.SetComplex(g.IsClusterStart(), false, 0); } diff --git a/gfx/thebes/gfxTextRun.cpp b/gfx/thebes/gfxTextRun.cpp index db9fc346b4..fd97615366 100644 --- a/gfx/thebes/gfxTextRun.cpp +++ b/gfx/thebes/gfxTextRun.cpp @@ -244,7 +244,7 @@ gfxTextRun::ComputeLigatureData(Range aPartRange, NS_ASSERTION(aPartRange.start < aPartRange.end, "Computing ligature data for empty range"); NS_ASSERTION(aPartRange.end <= GetLength(), "Character length overflow"); - + LigatureData result; const CompressedGlyph *charGlyphs = mCharacterGlyphs; @@ -392,7 +392,7 @@ gfxTextRun::ShrinkToLigatureBoundaries(Range* aRange) const { if (aRange->start >= aRange->end) return; - + const CompressedGlyph *charGlyphs = mCharacterGlyphs; while (aRange->start < aRange->end && @@ -440,7 +440,7 @@ ClipPartialLigature(const gfxTextRun* aTextRun, } else { *aEnd = std::min(*aEnd, endEdge); } - } + } } void @@ -789,7 +789,7 @@ gfxTextRun::AccumulatePartialLigatureMetrics(gfxFont *aFont, Range aRange, // ligature. Shift it left. metrics.mBoundingBox.x -= IsRightToLeft() ? metrics.mAdvanceWidth - (data.mPartAdvance + data.mPartWidth) - : data.mPartAdvance; + : data.mPartAdvance; metrics.mAdvanceWidth = data.mPartWidth; aMetrics->CombineWith(metrics, IsRightToLeft()); @@ -905,7 +905,7 @@ gfxTextRun::BreakAndMeasureText(uint32_t aStart, uint32_t aMaxLength, } // There can't be a word-wrap break opportunity at the beginning of the - // line: if the width is too small for even one character to fit, it + // line: if the width is too small for even one character to fit, it // could be the first and last break opportunity on the line, and that // would trigger an infinite loop. if (aSuppressBreak != eSuppressAllBreaks && @@ -923,7 +923,7 @@ gfxTextRun::BreakAndMeasureText(uint32_t aStart, uint32_t aMaxLength, if (atHyphenationBreak) { hyphenatedAdvance += aProvider->GetHyphenWidth(); } - + if (lastBreak < 0 || width + hyphenatedAdvance - trimmableAdvance <= aWidth) { // We can break here. lastBreak = i; @@ -943,7 +943,7 @@ gfxTextRun::BreakAndMeasureText(uint32_t aStart, uint32_t aMaxLength, } } } - + gfxFloat charAdvance; if (i >= ligatureRange.start && i < ligatureRange.end) { charAdvance = GetAdvanceForGlyphs(Range(i, i + 1)); @@ -956,7 +956,7 @@ gfxTextRun::BreakAndMeasureText(uint32_t aStart, uint32_t aMaxLength, charAdvance = ComputePartialLigatureWidth(Range(i, i + 1), aProvider); } - + advance += charAdvance; if (aTrimWhitespace || aWhitespaceCanHang) { if (mCharacterGlyphs[i].CharIsSpace()) { @@ -1145,7 +1145,7 @@ gfxTextRun::AddGlyphRun(gfxFont *aFont, uint8_t aMatchType, "mixed orientation should have been resolved"); if (!aFont) { return NS_OK; - } + } uint32_t numGlyphRuns = mGlyphRuns.Length(); if (!aForceNewRun && numGlyphRuns > 0) { GlyphRun *lastGlyphRun = &mGlyphRuns[numGlyphRuns - 1]; @@ -1405,7 +1405,7 @@ gfxTextRun::SetSpaceGlyph(gfxFont* aFont, DrawTarget* aDrawTarget, (GetFlags() & gfxTextRunFactory::TEXT_ORIENT_VERTICAL_UPRIGHT) != 0; gfxShapedWord* sw = aFont->GetShapedWord(aDrawTarget, &space, 1, - gfxShapedWord::HashMix(0, ' '), + gfxShapedWord::HashMix(0, ' '), Script::LATIN, vertical, mAppUnitsPerDevUnit, @@ -1439,8 +1439,8 @@ gfxTextRun::SetSpaceGlyphIfSimple(gfxFont* aFont, uint32_t aCharIndex, AddGlyphRun(aFont, gfxTextRange::kFontGroup, aCharIndex, false, aOrientation); - CompressedGlyph g; - g.SetSimpleGlyph(spaceWidthAppUnits, spaceGlyph); + CompressedGlyph g = + CompressedGlyph::MakeSimpleGlyph(spaceWidthAppUnits, spaceGlyph); if (aSpaceChar == ' ') { g.SetIsSpace(); } @@ -1471,7 +1471,7 @@ gfxTextRun::FetchGlyphExtents(DrawTarget* aRefDrawTarget) bool fontIsSetup = false; uint32_t j; gfxGlyphExtents *extents = font->GetOrCreateGlyphExtents(mAppUnitsPerDevUnit); - + for (j = start; j < end; ++j) { const gfxTextRun::CompressedGlyph *glyphData = &charGlyphs[j]; if (glyphData->IsSimpleGlyph()) { @@ -1943,13 +1943,13 @@ gfxFontGroup::Copy(const gfxFontStyle *aStyle) return fg; } -bool +bool gfxFontGroup::IsInvalidChar(uint8_t ch) { return ((ch & 0x7f) < 0x20 || ch == 0x7f); } -bool +bool gfxFontGroup::IsInvalidChar(char16_t ch) { // All printable 7-bit ASCII values are OK @@ -2514,8 +2514,8 @@ gfxFontGroup::InitScriptRun(DrawTarget* aDrawTarget, detailedGlyph.mGlyphID = mainFont->GetSpaceGlyph(); detailedGlyph.mAdvance = advance; detailedGlyph.mXOffset = detailedGlyph.mYOffset = 0; - gfxShapedText::CompressedGlyph g; - g.SetComplex(true, true, 1); + CompressedGlyph g = + CompressedGlyph::MakeComplex(true, true, 1); aTextRun->SetGlyphs(aOffset + index, g, &detailedGlyph); } @@ -3128,7 +3128,7 @@ gfxFontGroup::WhichPrefFontSupportsChar(uint32_t aCh, uint32_t aNextCh) uint32_t unicodeRange = FindCharUnicodeRange(aCh); charLang = pfl->GetFontPrefLangFor(unicodeRange); } - + // if the last pref font was the first family in the pref list, no need to recheck through a list of families if (mLastPrefFont && charLang == mLastPrefLang && mLastPrefFirstFont && mLastPrefFont->HasCharacter(aCh)) { @@ -3190,7 +3190,7 @@ already_AddRefed gfxFontGroup::WhichSystemFontSupportsChar(uint32_t aCh, uint32_t aNextCh, Script aRunScript) { - gfxFontEntry *fe = + gfxFontEntry *fe = gfxPlatformFontList::PlatformFontList()-> SystemFindFontForChar(aCh, aNextCh, aRunScript, &mStyle); if (fe) { diff --git a/gfx/thebes/gfxTextRun.h b/gfx/thebes/gfxTextRun.h index 7217eca747..d61992527b 100644 --- a/gfx/thebes/gfxTextRun.h +++ b/gfx/thebes/gfxTextRun.h @@ -70,7 +70,7 @@ struct gfxTextRunDrawCallbacks { * of text. It stores runs of positioned glyph data, each run having a single * gfxFont. The glyphs are associated with a string of source text, and the * gfxTextRun APIs take parameters that are offsets into that source text. - * + * * gfxTextRuns are mostly immutable. The only things that can change are * inter-cluster spacing and line break placement. Spacing is always obtained * lazily by methods that need it, it is not cached. Line breaks are stored @@ -78,7 +78,7 @@ struct gfxTextRunDrawCallbacks { * not actually do anything to explicitly account for line breaks). Initially * there are no line breaks. The textrun can record line breaks before or after * any given cluster. (Line breaks specified inside clusters are ignored.) - * + * * It is important that zero-length substrings are handled correctly. This will * be on the test! */ @@ -163,11 +163,11 @@ public: * Set the potential linebreaks for a substring of the textrun. These are * the "allow break before" points. Initially, there are no potential * linebreaks. - * + * * This can change glyphs and/or geometry! Some textruns' shapes * depend on potential line breaks (e.g., title-case-converting textruns). * This function is virtual so that those textruns can reshape themselves. - * + * * @return true if this changed the linebreaks, false if the new line * breaks are the same as the old */ @@ -179,7 +179,7 @@ public: * potential line break points and computation of spacing. We pass the data * this way to allow lazy data acquisition; for example BreakAndMeasureText * will want to only ask for properties of text it's actually looking at. - * + * * NOTE that requested spacing may not actually be applied, if the textrun * is unable to apply it in some context. Exception: spacing around a * whitespace character MUST always be applied. @@ -238,7 +238,7 @@ public: * Draws a substring. Uses only GetSpacing from aBreakProvider. * The provided point is the baseline origin on the left of the string * for LTR, on the right of the string for RTL. - * + * * Drawing should respect advance widths in the sense that for LTR runs, * Draw(Range(start, middle), pt, ...) followed by * Draw(Range(middle, end), gfxPoint(pt.x + advance, pt.y), ...) @@ -250,7 +250,7 @@ public: * Draw(Range(start, middle), gfxPoint(pt.x + advance, pt.y), ...) * should have the same effect as * Draw(Range(start, end), pt, ...) - * + * * Glyphs should be drawn in logical content order, which can be significant * if they overlap (perhaps due to negative spacing). */ @@ -307,14 +307,14 @@ public: * Clear all stored line breaks for the given range (both before and after), * and then set the line-break state before aRange.start to aBreakBefore and * after the last cluster to aBreakAfter. - * + * * We require that before and after line breaks be consistent. For clusters * i and i+1, we require that if there is a break after cluster i, a break * will be specified before cluster i+1. This may be temporarily violated * (e.g. after reflowing line L and before reflowing line L+1); to handle * these temporary violations, we say that there is a break betwen i and i+1 * if a break is specified after i OR a break is specified before i+1. - * + * * This can change textrun geometry! The existence of a linebreak can affect * the advance width of the cluster before the break (when kerning) or the * geometry of one cluster before the break or any number of clusters @@ -322,11 +322,11 @@ public: * arbitrary; if some scripts require breaking it, then we need to * alter nsTextFrame::TrimTrailingWhitespace, perhaps drastically becase * it could affect the layout of frames before it...) - * + * * We return true if glyphs or geometry changed, false otherwise. This * function is virtual so that gfxTextRun subclasses can reshape * properly. - * + * * @param aAdvanceWidthDelta if non-null, returns the change in advance * width of the given range. */ @@ -393,7 +393,7 @@ public: * @param aBreakPriority in/out the priority of the break opportunity * saved in the line. If we are prioritizing break opportunities, we will * not set a break with a lower priority. @see gfxBreakPriority. - * + * * Note that negative advance widths are possible especially if negative * spacing is provided. */ @@ -596,11 +596,11 @@ public: // when the part is at the start of the ligature, and after-spacing // when the part is as the end of the ligature gfxFloat mPartWidth; - + bool mClipBeforePart; bool mClipAfterPart; }; - + // return storage used by this run, for memory reporter; // nsTransformedTextRun needs to override this as it holds additional data virtual size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) @@ -681,7 +681,7 @@ protected: CompressedGlyph *mCharacterGlyphs; private: - // **** general helpers **** + // **** general helpers **** // Get the total advance for a range of glyphs. int32_t GetAdvanceForGlyphs(Range aRange) const; @@ -765,6 +765,7 @@ private: class gfxFontGroup : public gfxTextRunFactory { public: typedef mozilla::unicode::Script Script; + typedef gfxShapedText::CompressedGlyph CompressedGlyph; static void Shutdown(); // platform must call this to release the languageAtomService diff --git a/layout/mathml/nsMathMLChar.cpp b/layout/mathml/nsMathMLChar.cpp index 4b02a52d8d..ad23c57bf5 100644 --- a/layout/mathml/nsMathMLChar.cpp +++ b/layout/mathml/nsMathMLChar.cpp @@ -581,9 +581,9 @@ nsOpenTypeTable::MakeTextRun(DrawTarget* aDrawTarget, aFontGroup->GetFirstValidFont()-> GetGlyphHAdvance(aDrawTarget, aGlyph.glyphID)); detailedGlyph.mXOffset = detailedGlyph.mYOffset = 0; - gfxShapedText::CompressedGlyph g; - g.SetComplex(true, true, 1); - textRun->SetGlyphs(0, g, &detailedGlyph); + textRun->SetGlyphs(0, + gfxShapedText::CompressedGlyph::MakeComplex(true, true, 1), + &detailedGlyph); return textRun.forget(); } @@ -1459,7 +1459,7 @@ nsMathMLChar::StretchEnumContext::EnumCallback(const FontFamilyName& aFamily, if (!openTypeTable) { if (context->mTablesTried.Contains(glyphTable)) return true; // already tried this one - + // Only try this table once. context->mTablesTried.AppendElement(glyphTable); } @@ -1626,7 +1626,7 @@ nsMathMLChar::StretchInternal(nsPresContext* aPresContext, if (!maxWidth && !largeop) { // Doing Stretch() not GetMaxWidth(), // and not a largeop in display mode; we're done if size fits - if ((targetSize <= 0) || + if ((targetSize <= 0) || ((isVertical && charSize >= targetSize) || IsSizeOK(charSize, targetSize, aStretchHint))) done = true; @@ -1678,7 +1678,7 @@ nsMathMLChar::StretchInternal(nsPresContext* aPresContext, // variables accordingly. mUnscaledAscent = aDesiredStretchSize.ascent; } - + if (glyphFound) { return NS_OK; } @@ -1695,7 +1695,7 @@ nsMathMLChar::StretchInternal(nsPresContext* aPresContext, if (!Preferences::GetBool("mathml.scale_stretchy_operators.enabled", true)) { return NS_OK; } - + // stretchy character if (stretchy) { if (isVertical) { @@ -1863,7 +1863,7 @@ public: nsDisplayMathMLCharForeground(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame, nsMathMLChar* aChar, uint32_t aIndex, bool aIsSelected) - : nsDisplayItem(aBuilder, aFrame), mChar(aChar), + : nsDisplayItem(aBuilder, aFrame), mChar(aChar), mIndex(aIndex), mIsSelected(aIsSelected) { MOZ_COUNT_CTOR(nsDisplayMathMLCharForeground); } @@ -1886,7 +1886,7 @@ public: temp.Inflate(mFrame->PresContext()->AppUnitsPerDevPixel()); return temp; } - + virtual void Paint(nsDisplayListBuilder* aBuilder, nsRenderingContext* aCtx) override { @@ -1901,7 +1901,7 @@ public: bool snap; return GetBounds(aBuilder, &snap); } - + virtual uint32_t GetPerFrameKey() override { return (mIndex << nsDisplayItem::TYPE_BITS) | nsDisplayItem::GetPerFrameKey(); @@ -2355,7 +2355,7 @@ nsMathMLChar::PaintHorizontally(nsPresContext* aPresContext, // _cairo_scaled_font_glyph_device_extents rounds outwards to the nearest // pixel, so the bm values can include 1 row of faint pixels on each edge. // Don't rely on this pixel as it can look like a gap. - if (bm.rightBearing - bm.leftBearing >= 2 * oneDevPixel) { + if (bm.rightBearing - bm.leftBearing >= 2 * oneDevPixel) { start[i] = dx + bm.leftBearing + oneDevPixel; // left join end[i] = dx + bm.rightBearing - oneDevPixel; // right join } else { From f2cfb9997e0a857db50a9339282d0158ddd6c9d1 Mon Sep 17 00:00:00 2001 From: trav90 Date: Sat, 28 May 2022 11:45:52 -0500 Subject: [PATCH 08/13] Issue #457 - Make GLContextSymbols a pure aggregate POD --- gfx/gl/GLContext.cpp | 5 +++-- gfx/gl/GLContextSymbols.h | 10 ++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp index 72a7bea58b..d07b925ad5 100644 --- a/gfx/gl/GLContext.cpp +++ b/gfx/gl/GLContext.cpp @@ -454,6 +454,7 @@ GLContext::GLContext(CreateContextFlags flags, const SurfaceCaps& caps, mTopError(LOCAL_GL_NO_ERROR), mDebugFlags(ChooseDebugFlags(flags)), mSharedContext(sharedContext), + mSymbols{}, mCaps(caps), mScreen(nullptr), mLockedSurface(nullptr), @@ -519,7 +520,7 @@ GLContext::InitWithPrefix(const char* prefix, bool trygl) if (!InitWithPrefixImpl(prefix, trygl)) { // If initialization fails, zero the symbols to avoid hard-to-understand bugs. - mSymbols.Zero(); + mSymbols = {}; NS_WARNING("GLContext::InitWithPrefix failed!"); return false; } @@ -2205,7 +2206,7 @@ GLContext::MarkDestroyed() mReadTexImageHelper = nullptr; mIsDestroyed = true; - mSymbols.Zero(); + mSymbols = {}; if (MakeCurrent(true)) { mTexGarbageBin->GLContextTeardown(); } diff --git a/gfx/gl/GLContextSymbols.h b/gfx/gl/GLContextSymbols.h index 61069e3f73..798e6cef5d 100644 --- a/gfx/gl/GLContextSymbols.h +++ b/gfx/gl/GLContextSymbols.h @@ -25,15 +25,9 @@ namespace mozilla { namespace gl { -struct GLContextSymbols +struct GLContextSymbols final { - GLContextSymbols() { - Zero(); - } - - void Zero() { - memset(this, 0, sizeof(GLContextSymbols)); - } + GLContextSymbols() = delete; // Initialize with {}. typedef void (GLAPIENTRY * PFNGLACTIVETEXTUREPROC) (GLenum texture); PFNGLACTIVETEXTUREPROC fActiveTexture; From 6ec59e79847133ced52aa66b4bdc9ca24f6c1e5b Mon Sep 17 00:00:00 2001 From: roytam1 Date: Tue, 31 May 2022 09:36:20 +0800 Subject: [PATCH 09/13] image: sync with https://repo.palemoon.org/MoonchildProductions/UXP/commit/8e42fccca66d06b664ecc3c7345dc787a26ef4d9 --- image/imgFrame.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/image/imgFrame.cpp b/image/imgFrame.cpp index 6fd1a69b61..88cef68bf9 100644 --- a/image/imgFrame.cpp +++ b/image/imgFrame.cpp @@ -123,7 +123,7 @@ AllowedImageSize(int32_t aWidth, int32_t aHeight) return false; } - // check to make sure we don't overflow a 32-bit + // check to make sure we don't overflow 32-bit size for RGBA CheckedInt32 requiredBytes = CheckedInt32(aWidth) * CheckedInt32(aHeight) * 4; if (MOZ_UNLIKELY(!requiredBytes.isValid())) { NS_WARNING("width or height too large"); From 7f3a7225afd0c92589259d5f58ca8dd050828c43 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 10 Feb 2022 15:10:39 +0000 Subject: [PATCH 10/13] [network] SocketTransport2 cleanup --- netwerk/base/nsSocketTransport2.cpp | 33 +++++++++++++++++------------ netwerk/base/nsSocketTransport2.h | 2 ++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/netwerk/base/nsSocketTransport2.cpp b/netwerk/base/nsSocketTransport2.cpp index 037b1a8829..39ea735f8c 100644 --- a/netwerk/base/nsSocketTransport2.cpp +++ b/netwerk/base/nsSocketTransport2.cpp @@ -931,7 +931,6 @@ nsresult nsSocketTransport::InitWithConnectedSocket(PRFileDesc *fd, const NetAddr *addr) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "wrong thread"); - NS_ASSERTION(!mFD.IsInitialized(), "already initialized"); char buf[kNetAddrMaxCStrBufSize]; NetAddrToString(addr, buf, sizeof(buf)); @@ -957,6 +956,7 @@ nsSocketTransport::InitWithConnectedSocket(PRFileDesc *fd, const NetAddr *addr) { MutexAutoLock lock(mLock); + NS_ASSERTION(!mFD.IsInitialized(), "already initialized"); mFD = fd; mFDref = 1; mFDconnected = 1; @@ -1320,11 +1320,14 @@ nsSocketTransport::InitiateSocket() // // if we already have a connected socket, then just attach and return. // - if (mFD.IsInitialized()) { + { + MutexAutoLock lock(mlock); + if (mFD.IsInitialized()) { rv = mSocketTransportService->AttachSocket(mFD, this); if (NS_SUCCEEDED(rv)) - mAttached = true; + mAttached = true; return rv; + } } // @@ -1390,18 +1393,18 @@ nsSocketTransport::InitiateSocket() PR_SetSocketOption(fd, &opt); #endif - // inform socket transport about this newly created socket... - rv = mSocketTransportService->AttachSocket(fd, this); - if (NS_FAILED(rv)) { - CloseSocket(fd); - return rv; - } - mAttached = true; - // assign mFD so that we can properly handle OnSocketDetached before we've // established a connection. { MutexAutoLock lock(mLock); + // inform socket transport about this newly created socket... + rv = mSocketTransportService->AttachSocket(fd, this); + if (NS_FAILED(rv)) { + CloseSocket(fd); + return rv; + } + mAttached = true; + mFD = fd; mFDref = 1; mFDconnected = false; @@ -1546,8 +1549,12 @@ nsSocketTransport::RecoverFromError() nsresult rv; - // OK to check this outside mLock - NS_ASSERTION(!mFDconnected, "socket should not be connected"); +#ifdef DEBUG + { + MutexAutoLock lock(mLock); + NS_ASSERTION(!mFDconnected, "socket should not be connected"); + } +#endif // all connection failures need to be reported to DNS so that the next // time we will use a different address if available. diff --git a/netwerk/base/nsSocketTransport2.h b/netwerk/base/nsSocketTransport2.h index cb107e6e79..89b75efa57 100644 --- a/netwerk/base/nsSocketTransport2.h +++ b/netwerk/base/nsSocketTransport2.h @@ -350,11 +350,13 @@ private: void OnMsgInputPending() { + MOZ_ASSERT(OnSocketThread(), "not on socket thread"); if (mState == STATE_TRANSFERRING) mPollFlags |= (PR_POLL_READ | PR_POLL_EXCEPT); } void OnMsgOutputPending() { + MOZ_ASSERT(OnSocketThread(), "not on socket thread"); if (mState == STATE_TRANSFERRING) mPollFlags |= (PR_POLL_WRITE | PR_POLL_EXCEPT); } From 3bcd2ee360e49b87b24fbb259ef5fd89bcba19b8 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 10 Feb 2022 22:21:40 +0000 Subject: [PATCH 11/13] [DOM] Don't allow internal MIME types to be assigned to DataTransfer We already blocked x-moz-file(-promise) and x-moz-place* but of course people would find ways to abuse other internal types. This change now blocks everything except x-moz-url types which are harmless. (i.e. whitelist instead of blacklist) --- dom/events/DataTransfer.cpp | 15 +++++---------- widget/nsITransferable.idl | 7 ++++++- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/dom/events/DataTransfer.cpp b/dom/events/DataTransfer.cpp index 5e7d477dfb..3a3f5464d2 100644 --- a/dom/events/DataTransfer.cpp +++ b/dom/events/DataTransfer.cpp @@ -639,16 +639,11 @@ DataTransfer::PrincipalMaySetData(const nsAString& aType, return false; } - if (aType.EqualsASCII(kFileMime) || - aType.EqualsASCII(kFilePromiseMime)) { - NS_WARNING("Disallowing adding x-moz-file or x-moz-file-promize types to DataTransfer"); - return false; - } - - // Disallow content from creating x-moz-place flavors, so that it cannot - // create fake Places smart queries exposing user data. - if (StringBeginsWith(aType, NS_LITERAL_STRING("text/x-moz-place"))) { - NS_WARNING("Disallowing adding moz-place types to DataTransfer"); + // Don't allow adding internal types of the form */x-moz-*, but + // special-case the url types as they are simple variations of urls. + if (FindInReadable(NS_LITERAL_STRING(kInternal_Mimetype_Prefix), aType) && + !StringBeginsWith(aType, NS_LITERAL_STRING("text/x-moz-url"))) { + NS_WARNING("Disallowing adding requested internal type to DataTransfer"); return false; } } diff --git a/widget/nsITransferable.idl b/widget/nsITransferable.idl index b128586dd1..e580673f5e 100644 --- a/widget/nsITransferable.idl +++ b/widget/nsITransferable.idl @@ -13,12 +13,17 @@ interface nsIPrincipal; %{ C++ +// Internal formats must have their second part starting with 'x-moz-', +// for example text/x-moz-internaltype. These cannot be assigned by +// unprivileged content but all other types can. +#define kInternal_Mimetype_Prefix "/x-moz-" + // these probably shouldn't live here, but in some central repository shared // by the entire app. #define kTextMime "text/plain" #define kRTFMime "text/rtf" #define kUnicodeMime "text/unicode" -#define kMozTextInternal "text/x-moz-text-internal" // text data which isn't suppoed to be parsed by other apps. +#define kMozTextInternal "text/x-moz-text-internal" // text data which isn't suppoed to be parsed by other apps. #define kHTMLMime "text/html" #define kAOLMailMime "AOLMAIL" #define kPNGImageMime "image/png" From 6b3ab70997d7c560c9041e27362538ee9967abd3 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 9 Apr 2022 00:33:41 +0200 Subject: [PATCH 12/13] [network] Fix typo in nsSocketTransport2.cpp Follow-up to 7322bc0e9df42bd43d84ec9be22b00c47a09f3d7 --- netwerk/base/nsSocketTransport2.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netwerk/base/nsSocketTransport2.cpp b/netwerk/base/nsSocketTransport2.cpp index 39ea735f8c..d6b713fe6e 100644 --- a/netwerk/base/nsSocketTransport2.cpp +++ b/netwerk/base/nsSocketTransport2.cpp @@ -1321,7 +1321,7 @@ nsSocketTransport::InitiateSocket() // if we already have a connected socket, then just attach and return. // { - MutexAutoLock lock(mlock); + MutexAutoLock lock(mLock); if (mFD.IsInitialized()) { rv = mSocketTransportService->AttachSocket(mFD, this); if (NS_SUCCEEDED(rv)) From 7b3f9fb7c08b2da84f69c9dcb0c99ccfaf65679e Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Mon, 23 May 2022 21:58:07 +0800 Subject: [PATCH 13/13] Issue #1909 - Guard against empty update manifest URL When installing an incompatible add-on, the add-ons manager checks first if a newer and compatible version of that add-on is available by sending a request either to the AUS or the provided update URL in the manifest. If there's no update URL in the manifest and if the application does not provide an add-on update URL via preferences, the add-ons manager will error out and fail to notify that the said add-on is incompatible. This commit addresses that by: (a) preventing substitutions on the update manifest URL - this throws an error if it's empty; and (b) failing early in the add-on update checker if the update manifest URL is empty and sends out an error notification --- .../mozapps/extensions/internal/AddonUpdateChecker.jsm | 8 ++++++++ toolkit/mozapps/extensions/internal/XPIProvider.jsm | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm b/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm index a475f7f1fa..6710c6a70f 100644 --- a/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm +++ b/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm @@ -611,6 +611,14 @@ function UpdateParser(aId, aUpdateKey, aUrl, aObserver) { let requireBuiltIn = Services.prefs.getBoolPref(PREF_UPDATE_REQUIREBUILTINCERTS, true); logger.debug("Requesting " + aUrl); + + if (!aUrl) { + logger.warn("Request failed: empty update manifest URL"); + this._doneAt = new Error("UP_emptyManifestURL"); + this.notifyError(AddonUpdateChecker.ERROR_DOWNLOAD_ERROR); + return; + } + try { this.request = new ServiceRequest(); this.request.open("GET", this.url, true); diff --git a/toolkit/mozapps/extensions/internal/XPIProvider.jsm b/toolkit/mozapps/extensions/internal/XPIProvider.jsm index 690351be1d..8eb140f0c6 100644 --- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm +++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm @@ -6135,7 +6135,11 @@ function UpdateChecker(aAddon, aListener, aReason, aAppVersion, aPlatformVersion if ("onUpdateAvailable" in this.listener) aReason |= UPDATE_TYPE_NEWVERSION; - let url = escapeAddonURI(aAddon, updateURL, aReason, aAppVersion); + // Don't perform substitutions on the update URL if we still don't + // have one at this point. + let url = updateURL ? + escapeAddonURI(aAddon, url, aReason, aAppVersion) : + updateURL; this._parser = AddonUpdateChecker.checkForUpdates(aAddon.id, aAddon.updateKey, url, this); }