Bug 1127246 - Add a post barrier to the baseShapes table r=terrence a=abillings

js/public/HashTable.h

@@ -253,10 +253,13 @@ class HashMap
             rekeyAs(old_key, new_key, new_key);
     }
 
-    // Infallibly rekey one entry, if present.
+    // Infallibly rekey one entry if present, and return whether that happened.
-    void rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const Key &new_key) {
+    bool rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const Key &new_key) {
-        if (Ptr p = lookup(old_lookup))
+        if (Ptr p = lookup(old_lookup)) {
             impl.rekeyAndMaybeRehash(p, new_lookup, new_key);
+            return true;
+        }
+        return false;
     }
 
     // HashMap is movable
@@ -471,10 +474,13 @@ class HashSet
             rekeyAs(old_value, new_value, new_value);
     }
 
-    // Infallibly rekey one entry, if present.
+    // Infallibly rekey one entry if present, and return whether that happened.
-    void rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const T &new_value) {
+    bool rekeyAs(const Lookup &old_lookup, const Lookup &new_lookup, const T &new_value) {
-        if (Ptr p = lookup(old_lookup))
+        if (Ptr p = lookup(old_lookup)) {
             impl.rekeyAndMaybeRehash(p, new_lookup, new_value);
+            return true;
+        }
+        return false;
     }
 
     // Infallibly rekey one entry with a new key that is equivalent.

js/src/jscompartment.h

@@ -286,6 +286,7 @@ struct JSCompartment
     void checkTypeObjectTableAfterMovingGC(js::types::NewTypeObjectTable &table);
     void checkInitialShapesTableAfterMovingGC();
     void checkWrapperMapAfterMovingGC();
+    void checkBaseShapeTableAfterMovingGC();
 #endif
 
     /*

js/src/jsgc.cpp

@@ -6962,6 +6962,7 @@ js::gc::CheckHashTablesAfterMovingGC(JSRuntime *rt)
         c->checkTypeObjectTablesAfterMovingGC();
         c->checkInitialShapesTableAfterMovingGC();
         c->checkWrapperMapAfterMovingGC();
+        c->checkBaseShapeTableAfterMovingGC();
         if (c->debugScopes)
             c->debugScopes->checkHashTablesAfterMovingGC(rt);
     }

js/src/vm/Shape.cpp

@@ -1297,22 +1297,22 @@ Shape::setObjectFlag(ExclusiveContext *cx, BaseShape::Flag flag, TaggedProto pro
 }
 
 /* static */ inline HashNumber
-StackBaseShape::hash(const StackBaseShape *base)
+StackBaseShape::hash(const Lookup& lookup)
 {
-    HashNumber hash = base->flags;
+    HashNumber hash = lookup.flags;
-    hash = RotateLeft(hash, 4) ^ (uintptr_t(base->clasp) >> 3);
+    hash = RotateLeft(hash, 4) ^ (uintptr_t(lookup.clasp) >> 3);
-    hash = RotateLeft(hash, 4) ^ (uintptr_t(base->parent) >> 3);
+    hash = RotateLeft(hash, 4) ^ (uintptr_t(lookup.hashParent) >> 3);
-    hash = RotateLeft(hash, 4) ^ (uintptr_t(base->metadata) >> 3);
+    hash = RotateLeft(hash, 4) ^ (uintptr_t(lookup.hashMetadata) >> 3);
     return hash;
 }
 
 /* static */ inline bool
-StackBaseShape::match(UnownedBaseShape *key, const StackBaseShape *lookup)
+StackBaseShape::match(UnownedBaseShape *key, const Lookup& lookup)
 {
-    return key->flags == lookup->flags
+    return key->flags == lookup.flags
-        && key->clasp_ == lookup->clasp
+        && key->clasp_ == lookup.clasp
-        && key->parent == lookup->parent
+        && key->parent == lookup.matchParent
-        && key->metadata == lookup->metadata;
+        && key->metadata == lookup.matchMetadata;
 }
 
 void
@@ -1325,6 +1325,61 @@ StackBaseShape::trace(JSTracer *trc)
         gc::MarkObjectRoot(trc, (JSObject**)&metadata, "StackBaseShape metadata");
 }
 
+/*
+ * This class is used to add a post barrier on the baseShapes set, as the key is
+ * calculated based on objects which may be moved by generational GC.
+ */
+class BaseShapeSetRef : public BufferableRef
+{
+    BaseShapeSet *set;
+    UnownedBaseShape *base;
+    JSObject *parentPrior;
+    JSObject *metadataPrior;
+
+  public:
+    BaseShapeSetRef(BaseShapeSet *set, UnownedBaseShape *base)
+      : set(set),
+        base(base),
+        parentPrior(base->getObjectParent()),
+        metadataPrior(base->getObjectMetadata())
+    {
+        MOZ_ASSERT(!base->isOwned());
+    }
+
+    void mark(JSTracer *trc) {
+        JSObject *parent = parentPrior;
+        if (parent)
+            Mark(trc, &parent, "baseShapes set parent");
+        JSObject *metadata = metadataPrior;
+        if (metadata)
+            Mark(trc, &metadata, "baseShapes set metadata");
+        if (parent == parentPrior && metadata == metadataPrior)
+            return;
+
+        StackBaseShape::Lookup lookupPrior(base->getObjectFlags(),
+                                           base->clasp(),
+                                           parentPrior, parent,
+                                           metadataPrior, metadata);
+        ReadBarriered<UnownedBaseShape *> b(base);
+        MOZ_ALWAYS_TRUE(set->rekeyAs(lookupPrior, base, b));
+    }
+};
+
+static void
+BaseShapesTablePostBarrier(ExclusiveContext *cx, BaseShapeSet *table, UnownedBaseShape *base)
+{
+    if (!cx->isJSContext()) {
+        MOZ_ASSERT(!IsInsideNursery(base->getObjectParent()));
+        MOZ_ASSERT(!IsInsideNursery(base->getObjectMetadata()));
+        return;
+    }
+
+    if (IsInsideNursery(base->getObjectParent()) || IsInsideNursery(base->getObjectMetadata())) {
+        StoreBuffer &sb = cx->asJSContext()->runtime()->gc.storeBuffer;
+        sb.putGeneric(BaseShapeSetRef(table, base));
+    }
+}
+
 /* static */ UnownedBaseShape*
 BaseShape::getUnowned(ExclusiveContext *cx, StackBaseShape &base)
 {
@@ -1333,7 +1388,7 @@ BaseShape::getUnowned(ExclusiveContext *cx, StackBaseShape &base)
     if (!table.initialized() && !table.init())
         return nullptr;
 
-    DependentAddPtr<BaseShapeSet> p(cx, table, &base);
+    DependentAddPtr<BaseShapeSet> p(cx, table, base);
     if (p)
         return *p;
 
@@ -1343,13 +1398,15 @@ BaseShape::getUnowned(ExclusiveContext *cx, StackBaseShape &base)
     if (!nbase_)
         return nullptr;
 
-    new (nbase_) BaseShape(*root);
+    new (nbase_) BaseShape(base);
 
     UnownedBaseShape *nbase = static_cast<UnownedBaseShape *>(nbase_);
 
-    if (!p.add(cx, table, root, nbase))
+    if (!p.add(cx, table, base, nbase))
         return nullptr;
 
+    BaseShapesTablePostBarrier(cx, &table, nbase);
+
     return nbase;
 }
 
@@ -1390,9 +1447,8 @@ JSCompartment::fixupBaseShapeTable()
     for (BaseShapeSet::Enum e(baseShapes); !e.empty(); e.popFront()) {
         UnownedBaseShape *base = e.front().unbarrieredGet();
         if (base->fixupBaseShapeTableEntry()) {
-            StackBaseShape sbase(base);
             ReadBarriered<UnownedBaseShape *> b(base);
-            e.rekeyFront(&sbase, b);
+            e.rekeyFront(base, b);
         }
     }
 }
@@ -1410,13 +1466,35 @@ JSCompartment::sweepBaseShapeTable()
         if (IsBaseShapeAboutToBeFinalizedFromAnyThread(&base)) {
             e.removeFront();
         } else if (base != e.front().unbarrieredGet()) {
-            StackBaseShape sbase(base);
             ReadBarriered<UnownedBaseShape *> b(base);
-            e.rekeyFront(&sbase, b);
+            e.rekeyFront(base, b);
         }
     }
 }
 
+#ifdef JSGC_HASH_TABLE_CHECKS
+
+void
+JSCompartment::checkBaseShapeTableAfterMovingGC()
+{
+    if (!baseShapes.initialized())
+        return;
+
+    for (BaseShapeSet::Enum e(baseShapes); !e.empty(); e.popFront()) {
+        UnownedBaseShape *base = e.front().unbarrieredGet();
+        CheckGCThingAfterMovingGC(base);
+        if (base->getObjectParent())
+            CheckGCThingAfterMovingGC(base->getObjectParent());
+        if (base->getObjectMetadata())
+            CheckGCThingAfterMovingGC(base->getObjectMetadata());
+
+        BaseShapeSet::Ptr ptr = baseShapes.lookup(base);
+        MOZ_ASSERT(ptr.found() && &*ptr == &e.front());
+    }
+}
+
+#endif // JSGC_HASH_TABLE_CHECKS
+
 void
 BaseShape::finalize(FreeOp *fop)
 {

js/src/vm/Shape.h

@@ -582,8 +582,6 @@ BaseShape::baseUnowned()
 /* Entries for the per-compartment baseShapes set of unowned base shapes. */
 struct StackBaseShape : public DefaultHasher<ReadBarrieredUnownedBaseShape>
 {
-    typedef const StackBaseShape *Lookup;
-
     uint32_t flags;
     const Class *clasp;
     JSObject *parent;
@@ -602,8 +600,50 @@ struct StackBaseShape : public DefaultHasher<ReadBarrieredUnownedBaseShape>
                           JSObject *parent, JSObject *metadata, uint32_t objectFlags);
     explicit inline StackBaseShape(Shape *shape);
 
-    static inline HashNumber hash(const StackBaseShape *lookup);
+    struct Lookup
-    static inline bool match(UnownedBaseShape *key, const StackBaseShape *lookup);
+    {
+        uint32_t flags;
+        const Class *clasp;
+        JSObject *hashParent;
+        JSObject *matchParent;
+        JSObject *hashMetadata;
+        JSObject *matchMetadata;
+
+        MOZ_IMPLICIT Lookup(const StackBaseShape &base)
+          : flags(base.flags),
+            clasp(base.clasp),
+            hashParent(base.parent),
+            matchParent(base.parent),
+            hashMetadata(base.metadata),
+            matchMetadata(base.metadata)
+        {}
+
+        MOZ_IMPLICIT Lookup(UnownedBaseShape *base)
+          : flags(base->getObjectFlags()),
+            clasp(base->clasp()),
+            hashParent(base->getObjectParent()),
+            matchParent(base->getObjectParent()),
+            hashMetadata(base->getObjectMetadata()),
+            matchMetadata(base->getObjectMetadata())
+        {
+            MOZ_ASSERT(!base->isOwned());
+        }
+
+        // For use by generational GC post barriers.
+        Lookup(uint32_t flags, const Class *clasp,
+               JSObject *hashParent, JSObject *matchParent,
+               JSObject *hashMetadata, JSObject *matchMetadata)
+          : flags(flags),
+            clasp(clasp),
+            hashParent(hashParent),
+            matchParent(matchParent),
+            hashMetadata(hashMetadata),
+            matchMetadata(matchMetadata)
+        {}
+    };
+
+    static inline HashNumber hash(const Lookup& lookup);
+    static inline bool match(UnownedBaseShape *key, const Lookup& lookup);
 
     // For RootedGeneric<StackBaseShape*>
     void trace(JSTracer *trc);