Bug 1719457 - Remove support for JSObject private slots. r=jonco

Initially the only way to attach data to an object was with a private slot.
More than 21 years ago (bug 73843) support for reserved slots was added
to the engine. An object can have multiple reserved slots, which makes
reserved slots more powerful and useful than the old-style private slot.

After converting the remaining private slots to reserved slots, we can now finally
remove support for private slots.

There's a bit more optimization work we can do to reserved slot accesses after
this lands.

Differential Revision: https://phabricator.services.mozilla.com/D121200

js/public/Class.h

@@ -490,8 +490,7 @@ static constexpr const js::ObjectOps* JS_NULL_OBJECT_OPS = nullptr;
 
 // Classes, objects, and properties.
 
-// Objects have private slot.
+// (1 << 0 is unused)
-static const uint32_t JSCLASS_HAS_PRIVATE = 1 << 0;
 
 // Class's initialization code will call `SetNewObjectMetadata` itself.
 static const uint32_t JSCLASS_DELAY_METADATA_BUILDER = 1 << 1;
@@ -685,8 +684,6 @@ struct alignas(js::gc::JSClassAlignBytes) JSClass {
   bool isNativeObject() const { return !(flags & NON_NATIVE); }
   bool isProxyObject() const { return flags & JSCLASS_IS_PROXY; }
 
-  bool hasPrivate() const { return !!(flags & JSCLASS_HAS_PRIVATE); }
-
   bool emulatesUndefined() const { return flags & JSCLASS_EMULATES_UNDEFINED; }
 
   bool isJSFunction() const { return this == js::FunctionClassPtr; }

js/public/Object.h

@@ -16,7 +16,7 @@
 
 #include "jstypes.h"  // JS_PUBLIC_API
 
-#include "js/Class.h"  // js::ESClass, JSCLASS_RESERVED_SLOTS, JSCLASS_HAS_PRIVATE
+#include "js/Class.h"       // js::ESClass, JSCLASS_RESERVED_SLOTS
 #include "js/Realm.h"       // JS::GetCompartmentForRealm
 #include "js/RootingAPI.h"  // JS::{,Mutable}Handle
 #include "js/Value.h"       // JS::Value
@@ -59,17 +59,6 @@ static MOZ_ALWAYS_INLINE Compartment* GetCompartment(JSObject* obj) {
   return GetCompartmentForRealm(realm);
 }
 
-/**
- * Get the private value stored for an object whose class has a private.
- *
- * It is safe to call this function within |obj|'s finalize hook.
- */
-inline void* GetPrivate(JSObject* obj) {
-  MOZ_ASSERT(GetClass(obj)->flags & JSCLASS_HAS_PRIVATE);
-  const auto* nobj = reinterpret_cast<const shadow::Object*>(obj);
-  return nobj->fixedSlots()[nobj->numFixedSlots()].toPrivate();
-}
-
 /**
  * Get the value stored in a reserved slot in an object.
  *

js/src/builtin/streams/ReadableStream.cpp

@@ -23,8 +23,8 @@
 #include "builtin/streams/ReadableStreamReader.h"  // js::CreateReadableStream{BYOB,Default}Reader, js::ForAuthorCodeBool
 #include "builtin/streams/WritableStream.h"  // js::WritableStream
 #include "js/CallArgs.h"                     // JS::CallArgs{,FromVp}
-#include "js/Class.h"  // JSCLASS_SLOT0_IS_NSISUPPORTS, JSCLASS_HAS_PRIVATE, JS_NULL_CLASS_OPS
+#include "js/Class.h"        // JSCLASS_SLOT0_IS_NSISUPPORTS, JS_NULL_CLASS_OPS
-#include "js/Conversions.h"           // JS::ToBoolean
+#include "js/Conversions.h"  // JS::ToBoolean
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/PropertySpec.h"  // JS{Function,Property}Spec, JS_FN, JS_PSG, JS_{FS,PS}_END
 #include "js/RootingAPI.h"        // JS::Handle, JS::Rooted, js::CanGC

js/src/builtin/streams/WritableStream.cpp

@@ -19,7 +19,7 @@
 #include "builtin/streams/WritableStreamDefaultWriter.h"  // js::CreateWritableStreamDefaultWriter
 #include "builtin/streams/WritableStreamOperations.h"  // js::WritableStream{Abort,Close{,QueuedOrInFlight}}
 #include "js/CallArgs.h"                               // JS::CallArgs{,FromVp}
-#include "js/Class.h"  // JS{Function,Property}Spec, JS_{FS,PS}_END, JSCLASS_SLOT0_IS_NSISUPPORTS, JSCLASS_HAS_PRIVATE, JS_NULL_CLASS_OPS
+#include "js/Class.h"  // JS{Function,Property}Spec, JS_{FS,PS}_END, JSCLASS_SLOT0_IS_NSISUPPORTS, JS_NULL_CLASS_OPS
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/RealmOptions.h"          // JS::RealmCreationOptions
 #include "js/RootingAPI.h"            // JS::Handle, JS::Rooted

js/src/gc/ObjectKind-inl.h

@@ -40,9 +40,6 @@ static inline AllocKind GetGCObjectKind(const JSClass* clasp) {
              "Proxies should use GetProxyGCObjectKind");
 
   uint32_t nslots = JSCLASS_RESERVED_SLOTS(clasp);
-  if (clasp->flags & JSCLASS_HAS_PRIVATE) {
-    nslots++;
-  }
   return GetGCObjectKind(nslots);
 }
 
@@ -120,23 +117,15 @@ static inline size_t GetGCKindSlots(AllocKind thingKind) {
 }
 
 static inline size_t GetGCKindSlots(AllocKind thingKind, const JSClass* clasp) {
-  size_t nslots = GetGCKindSlots(thingKind);
-
-  /* An object's private data uses the space taken by its last fixed slot. */
-  if (clasp->flags & JSCLASS_HAS_PRIVATE) {
-    MOZ_ASSERT(nslots > 0);
-    nslots--;
-  }
-
   /*
    * Functions have a larger alloc kind than AllocKind::OBJECT to reserve
    * space for the extra fields in JSFunction, but have no fixed slots.
    */
   if (clasp == FunctionClassPtr) {
-    nslots = 0;
+    return 0;
   }
 
-  return nslots;
+  return GetGCKindSlots(thingKind);
 }
 
 static inline size_t GetGCKindBytes(AllocKind thingKind) {

js/src/gc/Tracer.cpp

@@ -224,10 +224,8 @@ void js::gc::GetTraceThingInfo(char* buf, size_t bufsize, void* thing,
             bufsize--;
             PutEscapedString(buf, bufsize, fun->displayAtom(), 0);
           }
-        } else if (obj->getClass()->flags & JSCLASS_HAS_PRIVATE) {
-          snprintf(buf, bufsize, " %p", obj->as<NativeObject>().getPrivate());
         } else {
-          snprintf(buf, bufsize, " <no private>");
+          snprintf(buf, bufsize, " <unknown object>");
         }
         break;
       }

js/src/jit/CacheIR.cpp

@@ -11034,7 +11034,6 @@ AttachDecision NewArrayIRGenerator::tryAttachArrayObject() {
 
   MOZ_ASSERT(arrayObj->numUsedFixedSlots() == 0);
   MOZ_ASSERT(arrayObj->numDynamicSlots() == 0);
-  MOZ_ASSERT(!arrayObj->hasPrivate());
   MOZ_ASSERT(!arrayObj->isSharedMemory());
 
   // The macro assembler only supports creating arrays with fixed elements.
@@ -11113,7 +11112,6 @@ AttachDecision NewObjectIRGenerator::tryAttachPlainObject() {
     return AttachDecision::NoAction;
   }
 
-  MOZ_ASSERT(!nativeObj->hasPrivate());
   MOZ_ASSERT(!nativeObj->hasDynamicElements());
   MOZ_ASSERT(!nativeObj->isSharedMemory());
 

js/src/jit/MacroAssembler.cpp

@@ -923,7 +923,6 @@ void MacroAssembler::initGCThing(Register obj, Register temp,
     const TemplateNativeObject& ntemplate =
         templateObj.asTemplateNativeObject();
     MOZ_ASSERT(!ntemplate.hasDynamicElements());
-    MOZ_ASSERT(!ntemplate.hasPrivate());
 
     // If the object has dynamic slots, the slots member has already been
     // filled in.
@@ -949,11 +948,9 @@ void MacroAssembler::initGCThing(Register obj, Register temp,
               Address(obj, elementsOffset + ObjectElements::offsetOfLength()));
       store32(Imm32(0),
               Address(obj, elementsOffset + ObjectElements::offsetOfFlags()));
-      MOZ_ASSERT(!ntemplate.hasPrivate());
     } else if (ntemplate.isArgumentsObject()) {
       // The caller will initialize the reserved slots.
       MOZ_ASSERT(!initContents);
-      MOZ_ASSERT(!ntemplate.hasPrivate());
       storePtr(ImmPtr(emptyObjectElements),
                Address(obj, NativeObject::offsetOfElements()));
     } else {

js/src/jit/TemplateObject-inl.h

@@ -109,10 +109,6 @@ inline bool TemplateNativeObject::hasDynamicElements() const {
   return asNativeObject().hasDynamicElements();
 }
 
-inline bool TemplateNativeObject::hasPrivate() const {
-  return asNativeObject().hasPrivate();
-}
-
 inline gc::Cell* TemplateNativeObject::regExpShared() const {
   RegExpObject* regexp = &obj_->as<RegExpObject>();
   MOZ_ASSERT(regexp->hasShared());

js/src/jit/TemplateObject.h

@@ -67,7 +67,6 @@ class TemplateNativeObject : public TemplateObject {
   inline bool hasDynamicElements() const;
   inline const Value* getDenseElements() const;
 
-  inline bool hasPrivate() const;
   inline gc::Cell* regExpShared() const;
 };
 

js/src/jsfriendapi.cpp

@@ -568,11 +568,6 @@ JS_PUBLIC_API JSObject* JS_CloneObject(JSContext* cx, HandleObject obj,
                                 JSMSG_CANT_CLONE_OBJECT);
       return nullptr;
     }
-
-    if (obj->as<NativeObject>().hasPrivate()) {
-      clone->as<NativeObject>().setPrivate(
-          obj->as<NativeObject>().getPrivate());
-    }
   } else {
     auto* handler = GetProxyHandler(obj);
 

js/src/vm/ArrayBufferObject.cpp

@@ -1232,7 +1232,6 @@ ArrayBufferObject* ArrayBufferObject::createForContents(
     }
   }
 
-  MOZ_ASSERT(!(class_.flags & JSCLASS_HAS_PRIVATE));
   gc::AllocKind allocKind = GetArrayBufferGCObjectKind(nslots);
 
   AutoSetNewObjectMetadata metadata(cx);
@@ -1283,7 +1282,6 @@ ArrayBufferObject::createBufferAndData(
     }
   }
 
-  MOZ_ASSERT(!(class_.flags & JSCLASS_HAS_PRIVATE));
   gc::AllocKind allocKind = GetArrayBufferGCObjectKind(nslots);
 
   ArrayBufferObject* buffer = NewObjectWithClassProto<ArrayBufferObject>(

js/src/vm/GlobalObject.cpp

@@ -639,12 +639,6 @@ GlobalObject* GlobalObject::createInternal(JSContext* cx,
   Rooted<GlobalObject*> global(cx, &obj->as<GlobalObject>());
   MOZ_ASSERT(global->isUnqualifiedVarObj());
 
-  // Initialize the private slot to null if present, as GC can call class
-  // hooks before the caller gets to set this to a non-garbage value.
-  if (clasp->flags & JSCLASS_HAS_PRIVATE) {
-    global->setPrivate(nullptr);
-  }
-
   Rooted<GlobalLexicalEnvironmentObject*> lexical(
       cx, GlobalLexicalEnvironmentObject::create(cx, global));
   if (!lexical) {

js/src/vm/JSFunction-inl.h

@@ -71,7 +71,6 @@ inline JSFunction* CloneFunctionObject(JSContext* cx, HandleFunction fun,
   nobj->initEmptyDynamicSlots();
   nobj->setEmptyElements();
 
-  MOZ_ASSERT(!clasp->hasPrivate());
   MOZ_ASSERT(shape->slotSpan() == 0);
 
   JSFunction* fun = static_cast<JSFunction*>(nobj);

js/src/vm/JSObject.cpp

@@ -1151,7 +1151,6 @@ static bool InitializePropertiesFromCompatibleNativeObject(
     return true;
   }
 
-  MOZ_ASSERT(!src->hasPrivate());
   RootedShape shape(cx);
   if (src->staticPrototype() == dst->staticPrototype()) {
     shape = src->shape();
@@ -1295,8 +1294,8 @@ template XDRResult js::XDRObjectLiteral(XDRState<XDR_DECODE>* xdr,
 
 /* static */
 bool NativeObject::fillInAfterSwap(JSContext* cx, HandleNativeObject obj,
-                                   NativeObject* old, HandleValueVector values,
+                                   NativeObject* old,
-                                   void* priv) {
+                                   HandleValueVector values) {
   // This object has just been swapped with some other object, and its shape
   // no longer reflects its allocated size. Correct this information and
   // fill the slots in with the specified values.
@@ -1313,12 +1312,6 @@ bool NativeObject::fillInAfterSwap(JSContext* cx, HandleNativeObject obj,
     MOZ_ASSERT(obj->shape()->numFixedSlots() == nfixed);
   }
 
-  if (obj->hasPrivate()) {
-    obj->setPrivate(priv);
-  } else {
-    MOZ_ASSERT(!priv);
-  }
-
   uint32_t oldDictionarySlotSpan =
       obj->inDictionaryMode() ? obj->dictionaryModeSlotSpan() : 0;
 
@@ -1528,9 +1521,7 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b,
 
     // Remember the original values from the objects.
     RootedValueVector avals(cx);
-    void* apriv = nullptr;
     if (na) {
-      apriv = na->hasPrivate() ? na->getPrivate() : nullptr;
       for (size_t i = 0; i < na->slotSpan(); i++) {
         if (!avals.append(na->getSlot(i))) {
           oomUnsafe.crash("JSObject::swap");
@@ -1538,9 +1529,7 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b,
       }
     }
     RootedValueVector bvals(cx);
-    void* bpriv = nullptr;
     if (nb) {
-      bpriv = nb->hasPrivate() ? nb->getPrivate() : nullptr;
       for (size_t i = 0; i < nb->slotSpan(); i++) {
         if (!bvals.append(nb->getSlot(i))) {
           oomUnsafe.crash("JSObject::swap");
@@ -1573,14 +1562,12 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b,
     js_memcpy(b, &tmp, sizeof tmp);
 
     if (na) {
-      if (!NativeObject::fillInAfterSwap(cx, b.as<NativeObject>(), na, avals,
+      if (!NativeObject::fillInAfterSwap(cx, b.as<NativeObject>(), na, avals)) {
-                                         apriv)) {
         oomUnsafe.crash("fillInAfterSwap");
       }
     }
     if (nb) {
-      if (!NativeObject::fillInAfterSwap(cx, a.as<NativeObject>(), nb, bvals,
+      if (!NativeObject::fillInAfterSwap(cx, a.as<NativeObject>(), nb, bvals)) {
-                                         bpriv)) {
         oomUnsafe.crash("fillInAfterSwap");
       }
     }
@@ -3271,10 +3258,6 @@ void JSObject::dump(js::GenericPrinter& out) const {
   out.putChar('\n');
 
   if (nobj) {
-    if (clasp->flags & JSCLASS_HAS_PRIVATE) {
-      out.printf("  private %p\n", nobj->getPrivate());
-    }
-
     uint32_t reserved = JSCLASS_RESERVED_SLOTS(clasp);
     if (reserved) {
       out.printf("  reserved slots:\n");
@@ -3825,7 +3808,6 @@ void JSObject::debugCheckNewObject(Shape* shape, js::gc::AllocKind allocKind,
   // included in numFixedSlots.
   if (!clasp->isNativeObject()) {
     MOZ_ASSERT_IF(!clasp->isProxyObject(), JSCLASS_RESERVED_SLOTS(clasp) == 0);
-    MOZ_ASSERT(!clasp->hasPrivate());
     MOZ_ASSERT_IF(shape, shape->numFixedSlots() == 0);
   }
 }

js/src/vm/NativeObject-inl.h

@@ -38,11 +38,8 @@ inline uint32_t NativeObject::numFixedSlotsMaybeForwarded() const {
 }
 
 inline uint8_t* NativeObject::fixedData(size_t nslots) const {
-  mozilla::DebugOnly<const JSClass*> clasp =
+  MOZ_ASSERT(ClassCanHaveFixedData(gc::MaybeForwardedObjectClass(this)));
-      gc::MaybeForwardedObjectClass(this);
+  MOZ_ASSERT(nslots == numFixedSlotsMaybeForwarded());
-  MOZ_ASSERT(ClassCanHaveFixedData(clasp));
-  MOZ_ASSERT(nslots ==
-             numFixedSlotsMaybeForwarded() + (clasp->hasPrivate() ? 1 : 0));
   return reinterpret_cast<uint8_t*>(&fixedSlots()[nslots]);
 }
 
@@ -448,10 +445,6 @@ inline bool NativeObject::isInWholeCellBuffer() const {
   }
   nobj->setEmptyElements();
 
-  if (clasp->hasPrivate()) {
-    nobj->initPrivate(nullptr);
-  }
-
   if (size_t span = shape->slotSpan()) {
     nobj->initializeSlotRange(0, span);
   }

js/src/vm/NativeObject.h

@@ -960,8 +960,7 @@ class NativeObject : public JSObject {
   [[nodiscard]] static bool fillInAfterSwap(JSContext* cx,
                                             HandleNativeObject obj,
                                             NativeObject* old,
-                                            HandleValueVector values,
+                                            HandleValueVector values);
-                                            void* priv);
 
  public:
   // Return true if this object has been converted from shared-immutable
@@ -1468,61 +1467,6 @@ class NativeObject : public JSObject {
 
   inline void privatePreWriteBarrier(HeapSlot* pprivate);
 
-  /* Private data accessors. */
-
- private:
-  HeapSlot& privateRef(uint32_t nfixed) const {
-    /*
-     * The private field of an object is used to hold a pointer by storing it as
-     * a PrivateValue(). Private fields are stored immediately after the last
-     * fixed slot of the object.
-     */
-    MOZ_ASSERT(isNumFixedSlots(nfixed));
-    MOZ_ASSERT(hasPrivate());
-    return fixedSlots()[nfixed];
-  }
-
- public:
-  bool hasPrivate() const { return getClass()->hasPrivate(); }
-
-  void* getPrivate() const { return getPrivate(numFixedSlots()); }
-  void* getPrivate(uint32_t nfixed) const {
-    return privateRef(nfixed).toPrivate();
-  }
-
-  void initPrivate(void* data) {
-    uint32_t nfixed = numFixedSlots();
-    privateRef(nfixed).unbarrieredSet(PrivateValue(data));
-  }
-
-  void setPrivate(void* data) { setPrivate(numFixedSlots(), data); }
-  void setPrivate(uint32_t nfixed, void* data) {
-    HeapSlot* pprivate = &privateRef(nfixed);
-    privatePreWriteBarrier(pprivate);
-    setPrivateUnbarriered(nfixed, data);
-  }
-
-  void setPrivateGCThing(gc::Cell* cell) {
-#ifdef DEBUG
-    if (IsMarkedBlack(this)) {
-      JS::AssertCellIsNotGray(cell);
-    }
-#endif
-    uint32_t nfixed = numFixedSlots();
-    HeapSlot* pprivate = &privateRef(nfixed);
-    Cell* prev = static_cast<gc::Cell*>(pprivate->toPrivate());
-    privatePreWriteBarrier(pprivate);
-    setPrivateUnbarriered(nfixed, cell);
-    gc::PostWriteBarrierCell(this, prev, cell);
-  }
-
-  void setPrivateUnbarriered(void* data) {
-    setPrivateUnbarriered(numFixedSlots(), data);
-  }
-  void setPrivateUnbarriered(uint32_t nfixed, void* data) {
-    privateRef(nfixed).unbarrieredSet(PrivateValue(data));
-  }
-
   // The methods below are used to store GC things in a reserved slot as
   // PrivateValues. This is done to bypass the normal tracing code (debugger
   // objects use this to store cross-compartment pointers).
@@ -1747,22 +1691,6 @@ inline void InitReservedSlot(NativeObject* obj, uint32_t slot, T* ptr,
   InitReservedSlot(obj, slot, ptr, sizeof(T), use);
 }
 
-// Initialize an object's private slot with a pointer to malloc-allocated memory
-// and associate the memory with the object.
-//
-// This call should be matched with a call to JSFreeOp::free_/delete_ in the
-// object's finalizer to free the memory and update the memory accounting.
-
-inline void InitObjectPrivate(NativeObject* obj, void* ptr, size_t nbytes,
-                              MemoryUse use) {
-  AddCellMemory(obj, nbytes, use);
-  obj->initPrivate(ptr);
-}
-template <typename T>
-inline void InitObjectPrivate(NativeObject* obj, T* ptr, MemoryUse use) {
-  InitObjectPrivate(obj, ptr, sizeof(T), use);
-}
-
 }  // namespace js
 
 #endif /* vm_NativeObject_h */