Bug 1394831 part 15 - Pass ShapeTable* to add*Property. r=bhackett
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 13 Nov 2017 10:51:10 +0100
changeset 436019 ef44b38b852a0b67b16a265ba29c8dacfb37cdf7
parent 436004 d7cb7c614fe1cf336683a9b80e9706f097cc8aec
child 436020 ce448d8a91b3b4014346e9142ddb23fe0e0606ab
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersbhackett
bugs1394831
milestone59.0a1
Bug 1394831 part 15 - Pass ShapeTable* to add*Property. r=bhackett
js/src/vm/NativeObject.cpp
js/src/vm/NativeObject.h
js/src/vm/Shape-inl.h
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -479,29 +479,30 @@ NativeObject::sparsifyDenseElement(JSCon
     RootedValue value(cx, obj->getDenseElement(index));
     MOZ_ASSERT(!value.isMagic(JS_ELEMENTS_HOLE));
 
     removeDenseElementForSparseIndex(cx, obj, index);
 
     RootedId id(cx, INT_TO_JSID(index));
 
     AutoKeepShapeTables keep(cx);
+    ShapeTable* table = nullptr;
     ShapeTable::Entry* entry = nullptr;
     if (obj->inDictionaryMode()) {
-        ShapeTable* table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
+        table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
         if (!table)
             return false;
         entry = &table->search<MaybeAdding::Adding>(id, keep);
     }
 
     // NOTE: We don't use addDataProperty because we don't want the
     // extensibility check if we're, for example, sparsifying frozen objects..
     Shape* shape = addDataPropertyInternal(cx, obj, id, SHAPE_INVALID_SLOT,
                                            obj->getElementsHeader()->elementAttributes(),
-                                           entry, keep);
+                                           table, entry, keep);
     if (!shape) {
         obj->setDenseElementUnchecked(index, value);
         return false;
     }
 
     obj->initSlot(shape->slot(), value);
 
     return true;
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -893,23 +893,24 @@ class NativeObject : public ShapedObject
      * Internal helper that adds a shape not yet mapped by this object.
      *
      * Notes:
      * 1. getter and setter must be normalized based on flags (see jsscope.cpp).
      * 2. Checks for non-extensibility must be done by callers.
      */
     static Shape*
     addDataPropertyInternal(JSContext* cx, HandleNativeObject obj, HandleId id,
-                            uint32_t slot, unsigned attrs, ShapeTable::Entry* entry,
-                            const AutoKeepShapeTables& keep);
+                            uint32_t slot, unsigned attrs, ShapeTable* table,
+                            ShapeTable::Entry* entry, const AutoKeepShapeTables& keep);
 
     static Shape*
     addAccessorPropertyInternal(JSContext* cx, HandleNativeObject obj, HandleId id,
                                 JSGetterOp getter, JSSetterOp setter, unsigned attrs,
-                                ShapeTable::Entry* entry, const AutoKeepShapeTables& keep);
+                                ShapeTable* table, ShapeTable::Entry* entry,
+                                const AutoKeepShapeTables& keep);
 
     static MOZ_MUST_USE bool fillInAfterSwap(JSContext* cx, HandleNativeObject obj,
                                              const Vector<Value>& values, void* priv);
 
   public:
     // Return true if this object has been converted from shared-immutable
     // prototype-rooted shape storage to dictionary-shapes in a doubly-linked
     // list.
--- a/js/src/vm/Shape-inl.h
+++ b/js/src/vm/Shape-inl.h
@@ -64,27 +64,29 @@ Shape::maybeCreateTableForLookup(JSConte
         return true;
 
     return Shape::hashify(cx, this);
 }
 
 template<MaybeAdding Adding>
 /* static */ inline bool
 Shape::search(JSContext* cx, Shape* start, jsid id, const AutoKeepShapeTables& keep,
-              Shape** pshape, ShapeTable::Entry** pentry)
+              Shape** pshape, ShapeTable** ptable, ShapeTable::Entry** pentry)
 {
     if (start->inDictionary()) {
         ShapeTable* table = start->ensureTableForDictionary(cx, keep);
         if (!table)
             return false;
+        *ptable = table;
         *pentry = &table->search<Adding>(id, keep);
         *pshape = (*pentry)->shape();
         return true;
     }
 
+    *ptable = nullptr;
     *pentry = nullptr;
     *pshape = Shape::search<Adding>(cx, start, id);
     return true;
 }
 
 template<MaybeAdding Adding>
 /* static */ MOZ_ALWAYS_INLINE Shape*
 Shape::search(JSContext* cx, Shape* start, jsid id)
@@ -394,42 +396,44 @@ Shape::searchNoHashify(Shape* start, jsi
 NativeObject::addDataProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                               uint32_t slot, unsigned attrs)
 {
     MOZ_ASSERT(!JSID_IS_VOID(id));
     MOZ_ASSERT(obj->uninlinedNonProxyIsExtensible());
     MOZ_ASSERT(!obj->containsPure(id));
 
     AutoKeepShapeTables keep(cx);
+    ShapeTable* table = nullptr;
     ShapeTable::Entry* entry = nullptr;
     if (obj->inDictionaryMode()) {
-        ShapeTable* table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
+        table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
         if (!table)
             return nullptr;
         entry = &table->search<MaybeAdding::Adding>(id, keep);
     }
 
-    return addDataPropertyInternal(cx, obj, id, slot, attrs, entry, keep);
+    return addDataPropertyInternal(cx, obj, id, slot, attrs, table, entry, keep);
 }
 
 /* static */ MOZ_ALWAYS_INLINE Shape*
 NativeObject::addAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                                   GetterOp getter, SetterOp setter, unsigned attrs)
 {
     MOZ_ASSERT(!JSID_IS_VOID(id));
     MOZ_ASSERT(obj->uninlinedNonProxyIsExtensible());
     MOZ_ASSERT(!obj->containsPure(id));
 
     AutoKeepShapeTables keep(cx);
+    ShapeTable* table = nullptr;
     ShapeTable::Entry* entry = nullptr;
     if (obj->inDictionaryMode()) {
-        ShapeTable* table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
+        table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
         if (!table)
             return nullptr;
         entry = &table->search<MaybeAdding::Adding>(id, keep);
     }
 
-    return addAccessorPropertyInternal(cx, obj, id, getter, setter, attrs, entry, keep);
+    return addAccessorPropertyInternal(cx, obj, id, getter, setter, attrs, table, entry, keep);
 }
 
 } /* namespace js */
 
 #endif /* vm_Shape_inl_h */
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -497,37 +497,33 @@ class MOZ_RAII AutoCheckShapeConsistency
 #endif
 };
 
 } // namespace js
 
 /* static */ Shape*
 NativeObject::addAccessorPropertyInternal(JSContext* cx,
                                           HandleNativeObject obj, HandleId id,
-                                          GetterOp getter, SetterOp setter,
-                                          unsigned attrs, ShapeTable::Entry* entry,
+                                          GetterOp getter, SetterOp setter, unsigned attrs,
+                                          ShapeTable* table, ShapeTable::Entry* entry,
                                           const AutoKeepShapeTables& keep)
 {
     AutoCheckShapeConsistency check(obj);
     AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
 
     // The code below deals with either converting obj to dictionary mode or
     // growing an object that's already in dictionary mode.
-    ShapeTable* table = nullptr;
     if (!obj->inDictionaryMode()) {
         if (ShouldConvertToDictionary(obj)) {
             if (!toDictionaryMode(cx, obj))
                 return nullptr;
             table = obj->lastProperty()->maybeTable(keep);
             entry = &table->search<MaybeAdding::Adding>(id, keep);
         }
     } else {
-        table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
-        if (!table)
-            return nullptr;
         if (table->needsToGrow()) {
             if (!table->grow(cx))
                 return nullptr;
             entry = &table->search<MaybeAdding::Adding>(id, keep);
             MOZ_ASSERT(!entry->shape());
         }
     }
 
@@ -562,38 +558,36 @@ NativeObject::addAccessorPropertyInterna
 
     return shape;
 }
 
 /* static */ Shape*
 NativeObject::addDataPropertyInternal(JSContext* cx,
                                       HandleNativeObject obj, HandleId id,
                                       uint32_t slot, unsigned attrs,
-                                      ShapeTable::Entry* entry, const AutoKeepShapeTables& keep)
+                                      ShapeTable* table,
+                                      ShapeTable::Entry* entry,
+                                      const AutoKeepShapeTables& keep)
 {
     AutoCheckShapeConsistency check(obj);
 
     // The slot, if any, must be a reserved slot.
     MOZ_ASSERT(slot == SHAPE_INVALID_SLOT ||
                slot < JSCLASS_RESERVED_SLOTS(obj->getClass()));
 
     // The code below deals with either converting obj to dictionary mode or
     // growing an object that's already in dictionary mode.
-    ShapeTable* table = nullptr;
     if (!obj->inDictionaryMode()) {
         if (ShouldConvertToDictionary(obj)) {
             if (!toDictionaryMode(cx, obj))
                 return nullptr;
             table = obj->lastProperty()->maybeTable(keep);
             entry = &table->search<MaybeAdding::Adding>(id, keep);
         }
     } else {
-        table = obj->lastProperty()->ensureTableForDictionary(cx, keep);
-        if (!table)
-            return nullptr;
         if (table->needsToGrow()) {
             if (!table->grow(cx))
                 return nullptr;
             entry = &table->search<MaybeAdding::Adding>(id, keep);
             MOZ_ASSERT(!entry->shape());
         }
     }
 
@@ -800,28 +794,29 @@ NativeObject::putDataProperty(JSContext*
 {
     MOZ_ASSERT(!JSID_IS_VOID(id));
 
     AutoCheckShapeConsistency check(obj);
     AssertValidArrayIndex(obj, id);
 
     // Search for id in order to claim its entry if table has been allocated.
     AutoKeepShapeTables keep(cx);
+    ShapeTable* table;
     ShapeTable::Entry* entry;
     RootedShape shape(cx);
     if (!Shape::search<MaybeAdding::Adding>(cx, obj->lastProperty(), id, keep,
-                                            shape.address(), &entry))
+                                            shape.address(), &table, &entry))
     {
         return nullptr;
     }
 
     if (!shape) {
         MOZ_ASSERT(obj->nonProxyIsExtensible(),
                    "Can't add new property to non-extensible object");
-        return addDataPropertyInternal(cx, obj, id, SHAPE_INVALID_SLOT, attrs, entry, keep);
+        return addDataPropertyInternal(cx, obj, id, SHAPE_INVALID_SLOT, attrs, table, entry, keep);
     }
 
     // Property exists: search must have returned a valid entry.
     MOZ_ASSERT_IF(entry, !entry->isRemoved());
 
     AssertCanChangeAttrs(shape, attrs);
 
     // If the caller wants to allocate a slot, but doesn't care which slot,
@@ -915,28 +910,29 @@ NativeObject::putAccessorProperty(JSCont
 
     AutoCheckShapeConsistency check(obj);
     AssertValidArrayIndex(obj, id);
 
     AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
 
     // Search for id in order to claim its entry if table has been allocated.
     AutoKeepShapeTables keep(cx);
+    ShapeTable* table;
     ShapeTable::Entry* entry;
     RootedShape shape(cx);
     if (!Shape::search<MaybeAdding::Adding>(cx, obj->lastProperty(), id, keep,
-                                            shape.address(), &entry))
+                                            shape.address(), &table, &entry))
     {
         return nullptr;
     }
 
     if (!shape) {
         MOZ_ASSERT(obj->nonProxyIsExtensible(),
                    "Can't add new property to non-extensible object");
-        return addAccessorPropertyInternal(cx, obj, id, getter, setter, attrs, entry, keep);
+        return addAccessorPropertyInternal(cx, obj, id, getter, setter, attrs, table, entry, keep);
     }
 
     // Property exists: search must have returned a valid entry.
     MOZ_ASSERT_IF(entry, !entry->isRemoved());
 
     AssertCanChangeAttrs(shape, attrs);
 
     bool hadSlot = shape->isDataProperty();
@@ -956,17 +952,17 @@ NativeObject::putAccessorProperty(JSCont
         return shape;
 
     // Overwriting a non-last property requires switching to dictionary mode.
     // The shape tree is shared immutable, and we can't removeProperty and then
     // addAccessorPropertyInternal because a failure under add would lose data.
     if (shape != obj->lastProperty() && !obj->inDictionaryMode()) {
         if (!toDictionaryMode(cx, obj))
             return nullptr;
-        ShapeTable* table = obj->lastProperty()->maybeTable(keep);
+        table = obj->lastProperty()->maybeTable(keep);
         MOZ_ASSERT(table);
         entry = &table->search<MaybeAdding::NotAdding>(shape->propid(), keep);
         shape = entry->shape();
     }
 
     if (obj->inDictionaryMode()) {
         // Updating some property in a dictionary-mode object. Create a new
         // shape for the existing property, and also generate a new shape for
@@ -1050,32 +1046,33 @@ NativeObject::changeProperty(JSContext* 
 }
 
 /* static */ bool
 NativeObject::removeProperty(JSContext* cx, HandleNativeObject obj, jsid id_)
 {
     RootedId id(cx, id_);
 
     AutoKeepShapeTables keep(cx);
+    ShapeTable* table;
     ShapeTable::Entry* entry;
     RootedShape shape(cx);
-    if (!Shape::search(cx, obj->lastProperty(), id, keep, shape.address(), &entry))
+    if (!Shape::search(cx, obj->lastProperty(), id, keep, shape.address(), &table, &entry))
         return false;
 
     if (!shape)
         return true;
 
     /*
      * If shape is not the last property added, or the last property cannot
      * be removed, switch to dictionary mode.
      */
     if (!obj->inDictionaryMode() && (shape != obj->lastProperty() || !obj->canRemoveLastProperty())) {
         if (!toDictionaryMode(cx, obj))
             return false;
-        ShapeTable* table = obj->lastProperty()->maybeTable(keep);
+        table = obj->lastProperty()->maybeTable(keep);
         MOZ_ASSERT(table);
         entry = &table->search<MaybeAdding::NotAdding>(shape->propid(), keep);
         shape = entry->shape();
     }
 
     /*
      * If in dictionary mode, get a new shape for the last property after the
      * removal. We need a fresh shape for all dictionary deletions, even of
@@ -1111,18 +1108,17 @@ NativeObject::removeProperty(JSContext* 
         obj->freeSlot(cx, shape->slot());
 
     /*
      * A dictionary-mode object owns mutable, unique shapes on a non-circular
      * doubly linked list, hashed by lastProperty()->table. So we can edit the
      * list and hash in place.
      */
     if (obj->inDictionaryMode()) {
-        ShapeTable* table = obj->lastProperty()->maybeTable(keep);
-        MOZ_ASSERT(table);
+        MOZ_ASSERT(obj->lastProperty()->maybeTable(keep) == table);
 
         if (entry->hadCollision()) {
             entry->setRemoved();
             table->decEntryCount();
             table->incRemovedCount();
         } else {
             entry->setFree();
             table->decEntryCount();
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -756,17 +756,18 @@ class Shape : public gc::TenuredCell
     };
 
     template<MaybeAdding Adding = MaybeAdding::NotAdding>
     static MOZ_ALWAYS_INLINE Shape* search(JSContext* cx, Shape* start, jsid id);
 
     template<MaybeAdding Adding = MaybeAdding::NotAdding>
     static inline MOZ_MUST_USE bool search(JSContext* cx, Shape* start, jsid id,
                                            const AutoKeepShapeTables&,
-                                           Shape** pshape, ShapeTable::Entry** pentry);
+                                           Shape** pshape, ShapeTable** ptable,
+                                           ShapeTable::Entry** pentry);
 
     static inline Shape* searchNoHashify(Shape* start, jsid id);
 
     void removeFromDictionary(NativeObject* obj);
     void insertIntoDictionary(GCPtrShape* dictp);
 
     inline void initDictionaryShape(const StackShape& child, uint32_t nfixed,
                                     GCPtrShape* dictp);