Bug 650161 - Fix ObjectImpl::toDictionaryMode() to work with compacting GC r=terrence
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 03 Oct 2014 10:04:19 +0100
changeset 208559 3aade35d80936c4df6c75dac6838825d02dab0df
parent 208558 978df8aa78baf8bc8e73c30d8bf6619bc82dc929
child 208560 2424dd8f3da08094b568e2092f6b43b89a25edf1
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersterrence
bugs650161
milestone35.0a1
Bug 650161 - Fix ObjectImpl::toDictionaryMode() to work with compacting GC r=terrence
js/src/jspropertytree.cpp
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/src/jspropertytree.cpp
+++ b/js/src/jspropertytree.cpp
@@ -273,20 +273,20 @@ Shape::finalize(FreeOp *fop)
 
 void
 Shape::fixupDictionaryShapeAfterMovingGC()
 {
     if (!listp)
         return;
 
     // It's possible that this shape is unreachable and that listp points to the
-    // location of a dead object in the nursery. In this case we should never
-    // touch it again, so poison it for good measure.
+    // location of a dead object in the nursery, in which case we should never
+    // touch it again.
     if (IsInsideNursery(reinterpret_cast<Cell *>(listp))) {
-        JS_POISON(reinterpret_cast<void *>(this), JS_SWEPT_TENURED_PATTERN, sizeof(Shape));
+        listp = nullptr;
         return;
     }
 
     MOZ_ASSERT(!IsInsideNursery(reinterpret_cast<Cell *>(listp)));
     AllocKind kind = TenuredCell::fromPointer(listp)->getAllocKind();
     MOZ_ASSERT(kind == FINALIZE_SHAPE || kind <= FINALIZE_OBJECT_LAST);
     if (kind == FINALIZE_SHAPE) {
         // listp points to the parent field of the next shape.
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -438,72 +438,64 @@ NativeObject::lookupChildProperty(Thread
     return shape;
 }
 
 bool
 js::NativeObject::toDictionaryMode(ThreadSafeContext *cx)
 {
     MOZ_ASSERT(!inDictionaryMode());
 
-#ifdef JSGC_COMPACTING
-    // TODO: This crashes if we run a compacting GC here.
-    js::AutoDisableCompactingGC nogc(zone()->runtimeFromAnyThread());
-#endif
-
     /* We allocate the shapes from cx->compartment(), so make sure it's right. */
     MOZ_ASSERT(cx->isInsideCurrentCompartment(this));
 
     /*
      * This function is thread safe as long as the object is thread local. It
      * does not modify the shared shapes, and only allocates newly allocated
      * (and thus also thread local) shapes.
      */
     MOZ_ASSERT(cx->isThreadLocal(this));
 
     uint32_t span = slotSpan();
 
     Rooted<NativeObject*> self(cx, this);
 
-    /*
-     * Clone the shapes into a new dictionary list. Don't update the
-     * last property of this object until done, otherwise a GC
-     * triggered while creating the dictionary will get the wrong
-     * slot span for this object.
-     */
+    // Clone the shapes into a new dictionary list. Don't update the last
+    // property of this object until done, otherwise a GC triggered while
+    // creating the dictionary will get the wrong slot span for this object.
     RootedShape root(cx);
     RootedShape dictionaryShape(cx);
 
     RootedShape shape(cx, lastProperty());
     while (shape) {
         MOZ_ASSERT(!shape->inDictionary());
 
         Shape *dprop = js_NewGCShape(cx);
         if (!dprop) {
             js_ReportOutOfMemory(cx);
             return false;
         }
 
-        HeapPtrShape *listp = dictionaryShape
-                              ? &dictionaryShape->parent
-                              : (HeapPtrShape *) root.address();
-
+        HeapPtrShape *listp = dictionaryShape ? &dictionaryShape->parent : nullptr;
         StackShape child(shape);
         dprop->initDictionaryShape(child, self->numFixedSlots(), listp);
 
+        if (!dictionaryShape)
+            root = dprop;
+
         MOZ_ASSERT(!dprop->hasTable());
         dictionaryShape = dprop;
         shape = shape->previous();
     }
 
     if (!Shape::hashify(cx, root)) {
         js_ReportOutOfMemory(cx);
         return false;
     }
 
-    MOZ_ASSERT((Shape **) root->listp == root.address());
+    MOZ_ASSERT(root->listp == nullptr);
     root->listp = &self->shape_;
     self->shape_ = root;
 
     MOZ_ASSERT(self->inDictionaryMode());
     root->base()->setSlotSpan(span);
 
     return true;
 }
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -698,17 +698,18 @@ class Shape : public gc::TenuredCell
     void removeFromDictionary(NativeObject *obj);
     void insertIntoDictionary(HeapPtrShape *dictp);
 
     void initDictionaryShape(const StackShape &child, uint32_t nfixed, HeapPtrShape *dictp) {
         new (this) Shape(child, nfixed);
         this->flags |= IN_DICTIONARY;
 
         this->listp = nullptr;
-        insertIntoDictionary(dictp);
+        if (dictp)
+            insertIntoDictionary(dictp);
     }
 
     /* Replace the base shape of the last shape in a non-dictionary lineage with base. */
     static Shape *replaceLastProperty(ExclusiveContext *cx, StackBaseShape &base,
                                       TaggedProto proto, HandleShape shape);
 
     /*
      * This function is thread safe if every shape in the lineage of |shape|