Bug 939993 - Check that AddPtrs are used only with matching Lookup values r=sfink
☠☠ backed out by 556a2db58cad ☠ ☠
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 19 Nov 2013 22:53:32 +0000
changeset 156696 1469f9e856c0c4cb80ff5cce118f2c5ed756dbac
parent 156695 5c370cd2f75acbef7cc740d4a582ad86840215ec
child 156697 35c62ee3a3f8e380d6032679005e7e77499d1a0d
push id36528
push userjcoppeard@mozilla.com
push dateThu, 21 Nov 2013 14:00:50 +0000
treeherdermozilla-inbound@1469f9e856c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs939993
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 939993 - Check that AddPtrs are used only with matching Lookup values r=sfink
js/public/HashTable.h
js/src/jscntxt.h
js/src/jsinfer.cpp
js/src/vm/Shape.cpp
--- a/js/public/HashTable.h
+++ b/js/public/HashTable.h
@@ -1506,16 +1506,17 @@ class HashTable : private AllocPolicy
     // Note: |l| may be a reference to a piece of |u|, so this function
     // must take care not to use |l| after moving |u|.
     template <class U>
     bool relookupOrAdd(AddPtr& p, const Lookup &l, U &&u)
     {
         p.mutationCount = mutationCount;
         {
             mozilla::ReentrancyGuard g(*this);
+            JS_ASSERT(prepareHash(l) == p.keyHash); // l has not been destroyed
             p.entry_ = &lookup(l, p.keyHash, sCollisionBit);
         }
         return p.found() || add(p, mozilla::Forward<U>(u));
     }
 
     void remove(Ptr p)
     {
         JS_ASSERT(table);
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -222,16 +222,28 @@ struct ThreadSafeContext : ContextFriend
     }
 
     inline js::Nursery &nursery() {
         JS_ASSERT(hasNursery());
         return runtime_->gcNursery;
     }
 #endif
 
+    uint64_t generationalGcNumber() {
+#ifdef JSGC_GENERATIONAL
+        return hasNursery() ? runtime_->gcNumber : 0;
+#else
+        return 0;
+#endif
+    }
+
+    uint64_t hasGenerationalGcHappened(uint64_t originalGcNumber) {
+        return generationalGcNumber() != originalGcNumber;
+    }
+
     /*
      * Allocator used when allocating GCThings on this context. If we are a
      * JSContext, this is the Zone allocator of the JSContext's zone.
      * Otherwise, this is a per-thread allocator.
      *
      * This does not live in PerThreadData because the notion of an allocator
      * is only per-thread when off the main thread. The runtime (and the main
      * thread) can have more than one zone, each with its own allocator, and
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -3745,17 +3745,17 @@ ExclusiveContext::getNewType(const Class
 
     TypeObjectSet &newTypeObjects = compartment_->newTypeObjects;
 
     if (!newTypeObjects.initialized() && !newTypeObjects.init())
         return nullptr;
 
     TypeObjectSet::AddPtr p = newTypeObjects.lookupForAdd(TypeObjectSet::Lookup(clasp, proto_));
     SkipRoot skipHash(this, &p); /* Prevent the hash from being poisoned. */
-    uint64_t originalGcNumber = zone()->gcNumber();
+    uint64_t originalGcNumber = generationalGcNumber();
     if (p) {
         TypeObject *type = *p;
         JS_ASSERT(type->clasp == clasp);
         JS_ASSERT(type->proto.get() == proto_.raw());
 
         /*
          * If set, the type's newScript indicates the script used to create
          * all objects in existence which have this type. If there are objects
@@ -3784,23 +3784,24 @@ ExclusiveContext::getNewType(const Class
         ? proto.toObject()->lastProperty()->hasObjectFlag(BaseShape::NEW_TYPE_UNKNOWN)
         : true;
 
     RootedTypeObject type(this, compartment_->types.newTypeObject(this, clasp, proto, markUnknown));
     if (!type)
         return nullptr;
 
     /*
-     * If a GC has occured, then the hash we calculated may be invalid, as it
-     * is based on proto, which may have been moved.
+     * If a generational collection has occurred, then the hash we calculated may
+     * be invalid, as it is based on proto, which may have been moved.
      */
-    bool gcHappened = zone()->gcNumber() != originalGcNumber;
+    TypeObjectSet::Lookup lookup(clasp, proto);
+    bool gcHappened = hasGenerationalGcHappened(originalGcNumber);
     bool added =
-        gcHappened ? newTypeObjects.putNew(TypeObjectSet::Lookup(clasp, proto), type.get())
-                   : newTypeObjects.relookupOrAdd(p, TypeObjectSet::Lookup(clasp, proto), type.get());
+        gcHappened ? newTypeObjects.putNew(lookup, type.get())
+                   : newTypeObjects.relookupOrAdd(p, lookup, type.get());
     if (!added)
         return nullptr;
 
 #ifdef JSGC_GENERATIONAL
     if (proto.isObject() && hasNursery() && nursery().isInside(proto.toObject())) {
         asJSContext()->runtime()->gcStoreBuffer.putGeneric(
             NewTypeObjectsSetRef(&newTypeObjects, type.get(), proto.toObject()));
     }
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -1463,32 +1463,41 @@ StackBaseShape::AutoRooter::trace(JSTrac
 /* static */ UnownedBaseShape*
 BaseShape::getUnowned(ExclusiveContext *cx, const StackBaseShape &base)
 {
     BaseShapeSet &table = cx->compartment()->baseShapes;
 
     if (!table.initialized() && !table.init())
         return nullptr;
 
+    uint64_t originalGcNumber = cx->generationalGcNumber();
     BaseShapeSet::AddPtr p = table.lookupForAdd(&base);
 
     if (p)
         return *p;
 
     StackBaseShape::AutoRooter root(cx, &base);
 
     BaseShape *nbase_ = js_NewGCBaseShape<CanGC>(cx);
     if (!nbase_)
         return nullptr;
 
     new (nbase_) BaseShape(base);
 
     UnownedBaseShape *nbase = static_cast<UnownedBaseShape *>(nbase_);
 
-    if (!table.relookupOrAdd(p, &base, nbase))
+    /*
+     * If a generational collection has occurred then the hash we calculated may
+     * be invalid, as it is based on the objects inside StackBaseShape, which
+     * may have been moved.
+     */
+    bool gcHappened = cx->hasGenerationalGcHappened(originalGcNumber);
+    bool added = gcHappened ? table.putNew(&base, nbase)
+                            : table.relookupOrAdd(p, &base, nbase);
+    if (!added)
         return nullptr;
 
     return nbase;
 }
 
 /* static */ UnownedBaseShape *
 BaseShape::lookupUnowned(ThreadSafeContext *cx, const StackBaseShape &base)
 {
@@ -1590,16 +1599,17 @@ EmptyShape::getInitialShape(ExclusiveCon
     JS_ASSERT_IF(parent, cx->isInsideCurrentCompartment(parent));
 
     InitialShapeSet &table = cx->compartment()->initialShapes;
 
     if (!table.initialized() && !table.init())
         return nullptr;
 
     typedef InitialShapeEntry::Lookup Lookup;
+    uint64_t originalGcNumber = cx->generationalGcNumber();
     InitialShapeSet::AddPtr p =
         table.lookupForAdd(Lookup(clasp, proto, parent, metadata, nfixed, objectFlags));
 
     if (p)
         return p->shape;
 
     SkipRoot skip(cx, &p); /* The hash may look like a GC pointer and get poisoned. */
     Rooted<TaggedProto> protoRoot(cx, proto);
@@ -1611,21 +1621,27 @@ EmptyShape::getInitialShape(ExclusiveCon
     if (!nbase)
         return nullptr;
 
     Shape *shape = cx->compartment()->propertyTree.newShape(cx);
     if (!shape)
         return nullptr;
     new (shape) EmptyShape(nbase, nfixed);
 
-    if (!table.relookupOrAdd(p, Lookup(clasp, protoRoot, parentRoot, metadataRoot, nfixed, objectFlags),
-                             InitialShapeEntry(shape, protoRoot)))
-    {
+    /*
+     * If a generational collection has occurred, then the hash we calculated
+     * may be invalid, as it is based on objects which may have been moved.
+     */
+    Lookup lookup(clasp, protoRoot, parentRoot, metadataRoot, nfixed, objectFlags);
+    InitialShapeEntry entry(shape, protoRoot);
+    bool gcHappened = cx->hasGenerationalGcHappened(originalGcNumber);
+    bool added = gcHappened ? table.putNew(lookup, entry)
+                            : table.relookupOrAdd(p, lookup, entry);
+    if (!added)
         return nullptr;
-    }
 
     return shape;
 }
 
 /* static */ Shape *
 EmptyShape::getInitialShape(ExclusiveContext *cx, const Class *clasp, TaggedProto proto,
                             JSObject *parent, JSObject *metadata,
                             AllocKind kind, uint32_t objectFlags)