Bug 1433837 - Cleanup JSObject slots_ initialization r=jandem

js::Allocate<JSObject> now only sets slots_ if nDynamicSlots is
non-zero. This avoids spurious writes for other types and is now
consistent with JIT code.

MozReview-Commit-ID: 3spPMFj7Fxz

--HG--
extra : histedit_source : f01b0e2c54c85ec6b21f42ed564e714d0366cae0

js/src/gc/Allocator.cpp

@@ -119,10 +119,12 @@ GCRuntime::tryNewTenuredObject(JSContext* cx, AllocKind kind, size_t thingSize,
 
     JSObject* obj = tryNewTenuredThing<JSObject, allowGC>(cx, kind, thingSize);
 
-    if (obj)
-        obj->setInitialSlotsMaybeNonNative(slots);
-    else
+    if (obj) {
+        if (nDynamicSlots)
+            static_cast<NativeObject*>(obj)->initSlots(slots);
+    } else {
         js_free(slots);
+    }
 
     return obj;
 }

js/src/gc/Allocator.h

@@ -21,7 +21,8 @@ struct Class;
 //
 // Note that JSObject allocation must use the longer signature below that
 // includes slot, heap, and finalizer information in support of various
-// object-specific optimizations.
+// object-specific optimizations. If dynamic slots are requested they will be
+// allocated and the pointer stored directly in |NativeObject::slots_|.
 template <typename T, AllowGC allowGC = CanGC>
 T*
 Allocate(JSContext* cx);

js/src/gc/Nursery.cpp

@@ -293,8 +293,11 @@ js::Nursery::allocateObject(JSContext* cx, size_t size, size_t nDynamicSlots, co
         }
     }
 
-    /* Always initialize the slots field to match the JIT behavior. */
-    obj->setInitialSlotsMaybeNonNative(slots);
+    /* Store slots pointer directly in new object. If no dynamic slots were
+     * requested, caller must initialize slots_ field itself as needed. We
+     * don't know if the caller was a native object or not. */
+    if (nDynamicSlots)
+        static_cast<NativeObject*>(obj)->initSlots(slots);
 
     TraceNurseryAlloc(obj, size);
     return obj;

js/src/jit/MacroAssembler.cpp

@@ -875,6 +875,8 @@ MacroAssembler::allocateObject(Register result, Register temp, gc::AllocKind all
     if (!nDynamicSlots)
         return freeListAllocate(result, temp, allocKind, fail);
 
+    // Only NativeObject can have nDynamicSlots > 0 and reach here.
+
     callMallocStub(nDynamicSlots * sizeof(GCPtrValue), temp, fail);
 
     Label failAlloc;

js/src/jsobj.h

@@ -154,12 +154,6 @@ class JSObject : public js::gc::Cell
     inline js::Shape* maybeShape() const;
     inline js::Shape* ensureShape(JSContext* cx);
 
-    // Set the initial slots and elements of an object. These pointers are only
-    // valid for native objects, but during initialization are set for all
-    // objects. For non-native objects, these must not be dynamically allocated
-    // pointers which leak when the non-native object finishes initialization.
-    inline void setInitialSlotsMaybeNonNative(js::HeapSlot* slots);
-
     enum GenerateShape {
         GENERATE_NONE,
         GENERATE_SHAPE

js/src/jsobjinlines.h

@@ -397,12 +397,6 @@ SetNewObjectMetadata(JSContext* cx, T* obj)
 
 } // namespace js
 
-inline void
-JSObject::setInitialSlotsMaybeNonNative(js::HeapSlot* slots)
-{
-    static_cast<js::NativeObject*>(this)->slots_ = slots;
-}
-
 inline js::GlobalObject&
 JSObject::global() const
 {

js/src/vm/ArrayObject-inl.h

@@ -57,7 +57,9 @@ ArrayObject::createArrayInternal(JSContext* cx, gc::AllocKind kind, gc::InitialH
     ArrayObject* aobj = static_cast<ArrayObject*>(obj);
     aobj->initGroup(group);
     aobj->initShape(shape);
-    // NOTE: Slots are created and assigned internally by Allocate<JSObject>.
+    // NOTE: Dynamic slots are created internally by Allocate<JSObject>.
+    if (!nDynamicSlots)
+        aobj->initSlots(nullptr);
 
     MOZ_ASSERT(clasp->shouldDelayMetadataBuilder());
     cx->compartment()->setObjectPendingMetadata(cx, aobj);

js/src/vm/NativeObject-inl.h

@@ -544,7 +544,9 @@ NativeObject::create(JSContext* cx, js::gc::AllocKind kind, js::gc::InitialHeap
     NativeObject* nobj = static_cast<NativeObject*>(obj);
     nobj->initGroup(group);
     nobj->initShape(shape);
-    // NOTE: Slots are created and assigned internally by Allocate<JSObject>.
+    // NOTE: Dynamic slots are created internally by Allocate<JSObject>.
+    if (!nDynamicSlots)
+        nobj->initSlots(nullptr);
     nobj->setEmptyElements();
 
     if (clasp->hasPrivate())

js/src/vm/NativeObject.h

@@ -680,6 +680,12 @@ class NativeObject : public ShapedObject
     }
 
   public:
+
+    /* Object allocation may directly initialize slots so this is public. */
+    void initSlots(HeapSlot* slots) {
+        slots_ = slots;
+    }
+
     static MOZ_MUST_USE bool generateOwnShape(JSContext* cx, HandleNativeObject obj,
                                               Shape* newShape = nullptr)
     {