Bug 939993 - Fix places where HashTable::AddPtr could be used with out-of-date hash value under GGC r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 25 Nov 2013 11:26:10 +0000
changeset 157837 e7e1771dd294ad7f6f6c1b2bd2150f77e8962185
parent 157836 2402eb83959796196c9ace50febc53e60d9a4da0
child 157838 9710fcac8fc0e551309f790197763a5a12018f58
push id25726
push usercbook@mozilla.com
push dateThu, 28 Nov 2013 10:47:25 +0000
treeherdermozilla-central@cdca43b7657d [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 - Fix places where HashTable::AddPtr could be used with out-of-date hash value under GGC r=sfink
js/src/jshashutil.h
js/src/jsinfer.cpp
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
js/src/vm/SelfHosting.cpp
js/src/vm/Shape.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jshashutil.h
@@ -0,0 +1,66 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=8 sts=4 et sw=4 tw=99:
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef jshashutil_h
+#define jshashutil_h
+
+#include "jscntxt.h"
+
+namespace js {
+
+/*
+ * Used to add entries to a js::HashMap or HashSet where the key depends on a GC
+ * thing that may be moved by generational collection between the call to
+ * lookupForAdd() and relookupOrAdd().
+ */
+template <class T>
+struct DependentAddPtr
+{
+    typedef typename T::AddPtr AddPtr;
+    typedef typename T::Entry Entry;
+
+    template <class Lookup>
+    DependentAddPtr(const ExclusiveContext *cx, const T &table, const Lookup &lookup)
+      : addPtr(table.lookupForAdd(lookup))
+#ifdef JSGC_GENERATIONAL
+      , cx(cx)
+      , originalGcNumber(cx->zone()->gcNumber())
+#endif
+        {}
+
+    template <class KeyInput, class ValueInput>
+    bool add(T &table, const KeyInput &key, const ValueInput &value) {
+#ifdef JSGC_GENERATIONAL
+        bool gcHappened = originalGcNumber != cx->zone()->gcNumber();
+        if (gcHappened)
+            return table.putNew(key, value);
+#endif
+        return table.relookupOrAdd(addPtr, key, value);
+    }
+
+    typedef void (DependentAddPtr::* ConvertibleToBool)();
+    void nonNull() {}
+
+    bool found() const                 { return addPtr.found(); }
+    operator ConvertibleToBool() const { return found() ? &DependentAddPtr::nonNull : 0; }
+    const Entry &operator*() const     { return *addPtr; }
+    const Entry *operator->() const    { return &*addPtr; }
+
+  private:
+    AddPtr addPtr ;
+#ifdef JSGC_GENERATIONAL
+    const ExclusiveContext *cx;
+    const uint64_t originalGcNumber;
+#endif
+
+    DependentAddPtr() MOZ_DELETE;
+    DependentAddPtr(const DependentAddPtr&) MOZ_DELETE;
+    DependentAddPtr& operator=(const DependentAddPtr&) MOZ_DELETE;
+};
+
+} // namespace js
+
+#endif
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -9,16 +9,17 @@
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/PodOperations.h"
 
 #include "jsapi.h"
 #include "jsautooplen.h"
 #include "jscntxt.h"
 #include "jsgc.h"
+#include "jshashutil.h"
 #include "jsobj.h"
 #include "jsprf.h"
 #include "jsscript.h"
 #include "jsstr.h"
 #include "jsworkers.h"
 #include "prmjtime.h"
 
 #include "gc/Marking.h"
@@ -2265,17 +2266,21 @@ GetValueTypeForTable(const Value &v)
 }
 
 struct types::ArrayTableKey : public DefaultHasher<types::ArrayTableKey>
 {
     Type type;
     JSObject *proto;
 
     ArrayTableKey()
-        : type(Type::UndefinedType()), proto(nullptr)
+      : type(Type::UndefinedType()), proto(nullptr)
+    {}
+
+    ArrayTableKey(Type type, JSObject *proto)
+      : type(type), proto(proto)
     {}
 
     static inline uint32_t hash(const ArrayTableKey &v) {
         return (uint32_t) (v.type.raw() ^ ((uint32_t)(size_t)v.proto >> 2));
     }
 
     static inline bool match(const ArrayTableKey &v1, const ArrayTableKey &v2) {
         return v1.type == v2.type && v1.proto == v2.proto;
@@ -2290,37 +2295,35 @@ TypeCompartment::setTypeToHomogenousArra
         arrayTypeTable = cx->new_<ArrayTypeTable>();
         if (!arrayTypeTable || !arrayTypeTable->init()) {
             arrayTypeTable = nullptr;
             cx->compartment()->types.setPendingNukeTypes(cx);
             return;
         }
     }
 
-    ArrayTableKey key;
-    key.type = elementType;
-    key.proto = obj->getProto();
-    ArrayTypeTable::AddPtr p = arrayTypeTable->lookupForAdd(key);
-
+    ArrayTableKey key(elementType, obj->getProto());
+    DependentAddPtr<ArrayTypeTable> p(cx, *arrayTypeTable, key);
     if (p) {
         obj->setType(p->value);
     } else {
         /* Make a new type to use for future arrays with the same elements. */
         RootedObject objProto(cx, obj->getProto());
         TypeObject *objType = newTypeObject(cx, &ArrayObject::class_, objProto);
         if (!objType) {
             cx->compartment()->types.setPendingNukeTypes(cx);
             return;
         }
         obj->setType(objType);
 
         if (!objType->unknownProperties())
             objType->addPropertyType(cx, JSID_VOID, elementType);
 
-        if (!arrayTypeTable->relookupOrAdd(p, key, objType)) {
+        key.proto = objProto;
+        if (!p.add(*arrayTypeTable, key, objType)) {
             cx->compartment()->types.setPendingNukeTypes(cx);
             return;
         }
     }
 }
 
 void
 TypeCompartment::fixArrayType(ExclusiveContext *cx, JSObject *obj)
@@ -3742,19 +3745,19 @@ ExclusiveContext::getNewType(const Class
     JS_ASSERT_IF(fun_, proto_.isObject());
     JS_ASSERT_IF(proto_.isObject(), isInsideCurrentCompartment(proto_.toObject()));
 
     TypeObjectSet &newTypeObjects = compartment_->newTypeObjects;
 
     if (!newTypeObjects.initialized() && !newTypeObjects.init())
         return nullptr;
 
-    TypeObjectSet::AddPtr p = newTypeObjects.lookupForAdd(TypeObjectSet::Lookup(clasp, proto_));
+    DependentAddPtr<TypeObjectSet> p(this, newTypeObjects,
+                                     TypeObjectSet::Lookup(clasp, proto_));
     SkipRoot skipHash(this, &p); /* Prevent the hash from being poisoned. */
-    uint64_t originalGcNumber = zone()->gcNumber();
     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
@@ -3782,25 +3785,17 @@ ExclusiveContext::getNewType(const Class
         proto.isObject()
         ? 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.
-     */
-    bool gcHappened = zone()->gcNumber() != originalGcNumber;
-    bool added =
-        gcHappened ? newTypeObjects.putNew(TypeObjectSet::Lookup(clasp, proto), type.get())
-                   : newTypeObjects.relookupOrAdd(p, TypeObjectSet::Lookup(clasp, proto), type.get());
-    if (!added)
+    if (!p.add(newTypeObjects, TypeObjectSet::Lookup(clasp, proto), type.get()))
         return nullptr;
 
 #ifdef JSGC_GENERATIONAL
     if (proto.isObject() && hasNursery() && nursery().isInside(proto.toObject())) {
         asJSContext()->runtime()->gcStoreBuffer.putGeneric(
             NewTypeObjectsSetRef(&newTypeObjects, type.get(), proto.toObject()));
     }
 #endif
@@ -3857,30 +3852,30 @@ ExclusiveContext::getLazyType(const Clas
 
     AutoEnterAnalysis enter(this);
 
     TypeObjectSet &table = compartment()->lazyTypeObjects;
 
     if (!table.initialized() && !table.init())
         return nullptr;
 
-    TypeObjectSet::AddPtr p = table.lookupForAdd(TypeObjectSet::Lookup(clasp, proto));
+    DependentAddPtr<TypeObjectSet> p(this, table, TypeObjectSet::Lookup(clasp, proto));
     if (p) {
         TypeObject *type = *p;
         JS_ASSERT(type->lazy());
 
         return type;
     }
 
     Rooted<TaggedProto> protoRoot(this, proto);
     TypeObject *type = compartment()->types.newTypeObject(this, clasp, protoRoot, false);
     if (!type)
         return nullptr;
 
-    if (!table.relookupOrAdd(p, TypeObjectSet::Lookup(clasp, protoRoot), type))
+    if (!p.add(table, TypeObjectSet::Lookup(clasp, protoRoot), type))
         return nullptr;
 
     type->singleton = (JSObject *) TypeObject::LAZY_SINGLETON;
 
     return type;
 }
 
 /////////////////////////////////////////////////////////////////////
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -3,16 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "vm/Debugger-inl.h"
 
 #include "jscntxt.h"
 #include "jscompartment.h"
+#include "jshashutil.h"
 #include "jsnum.h"
 #include "jsobj.h"
 #include "jswrapper.h"
 
 #include "frontend/BytecodeCompiler.h"
 #include "gc/Marking.h"
 #include "jit/BaselineJIT.h"
 #include "js/Vector.h"
@@ -647,28 +648,28 @@ Debugger::wrapEnvironment(JSContext *cx,
 
     /*
      * DebuggerEnv should only wrap a debug scope chain obtained (transitively)
      * from GetDebugScopeFor(Frame|Function).
      */
     JS_ASSERT(!env->is<ScopeObject>());
 
     JSObject *envobj;
-    ObjectWeakMap::AddPtr p = environments.lookupForAdd(env);
+    DependentAddPtr<ObjectWeakMap> p(cx, environments, env);
     if (p) {
         envobj = p->value;
     } else {
         /* Create a new Debugger.Environment for env. */
         JSObject *proto = &object->getReservedSlot(JSSLOT_DEBUG_ENV_PROTO).toObject();
         envobj = NewObjectWithGivenProto(cx, &DebuggerEnv_class, proto, nullptr, TenuredObject);
         if (!envobj)
             return false;
         envobj->setPrivateGCThing(env);
         envobj->setReservedSlot(JSSLOT_DEBUGENV_OWNER, ObjectValue(*object));
-        if (!environments.relookupOrAdd(p, env, envobj)) {
+        if (!p.add(environments, env, envobj)) {
             js_ReportOutOfMemory(cx);
             return false;
         }
 
         CrossCompartmentKey key(CrossCompartmentKey::DebuggerEnvironment, object, env);
         if (!object->compartment()->putWrapper(key, ObjectValue(*envobj))) {
             environments.remove(env);
             js_ReportOutOfMemory(cx);
@@ -682,29 +683,30 @@ Debugger::wrapEnvironment(JSContext *cx,
 bool
 Debugger::wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp)
 {
     assertSameCompartment(cx, object.get());
 
     if (vp.isObject()) {
         RootedObject obj(cx, &vp.toObject());
 
-        ObjectWeakMap::AddPtr p = objects.lookupForAdd(obj);
+        DependentAddPtr<ObjectWeakMap> p(cx, objects, obj);
         if (p) {
             vp.setObject(*p->value);
         } else {
             /* Create a new Debugger.Object for obj. */
             JSObject *proto = &object->getReservedSlot(JSSLOT_DEBUG_OBJECT_PROTO).toObject();
             JSObject *dobj =
                 NewObjectWithGivenProto(cx, &DebuggerObject_class, proto, nullptr, TenuredObject);
             if (!dobj)
                 return false;
             dobj->setPrivateGCThing(obj);
             dobj->setReservedSlot(JSSLOT_DEBUGOBJECT_OWNER, ObjectValue(*object));
-            if (!objects.relookupOrAdd(p, obj, dobj)) {
+
+            if (!p.add(objects, obj, dobj)) {
                 js_ReportOutOfMemory(cx);
                 return false;
             }
 
             if (obj->compartment() != object->compartment()) {
                 CrossCompartmentKey key(CrossCompartmentKey::DebuggerObject, object, obj);
                 if (!object->compartment()->putWrapper(key, ObjectValue(*dobj))) {
                     objects.remove(obj);
@@ -2752,24 +2754,23 @@ Debugger::newDebuggerScript(JSContext *c
     return scriptobj;
 }
 
 JSObject *
 Debugger::wrapScript(JSContext *cx, HandleScript script)
 {
     assertSameCompartment(cx, object.get());
     JS_ASSERT(cx->compartment() != script->compartment());
-    ScriptWeakMap::AddPtr p = scripts.lookupForAdd(script);
+    DependentAddPtr<ScriptWeakMap> p(cx, scripts, script);
     if (!p) {
         JSObject *scriptobj = newDebuggerScript(cx, script);
         if (!scriptobj)
             return nullptr;
 
-        /* The allocation may have caused a GC, which can remove table entries. */
-        if (!scripts.relookupOrAdd(p, script, scriptobj)) {
+        if (!p.add(scripts, script, scriptobj)) {
             js_ReportOutOfMemory(cx);
             return nullptr;
         }
 
         CrossCompartmentKey key(CrossCompartmentKey::DebuggerScript, object, script);
         if (!object->compartment()->putWrapper(key, ObjectValue(*scriptobj))) {
             scripts.remove(script);
             js_ReportOutOfMemory(cx);
@@ -3644,24 +3645,23 @@ Debugger::newDebuggerSource(JSContext *c
     return sourceobj;
 }
 
 JSObject *
 Debugger::wrapSource(JSContext *cx, HandleScriptSource source)
 {
     assertSameCompartment(cx, object.get());
     JS_ASSERT(cx->compartment() != source->compartment());
-    SourceWeakMap::AddPtr p = sources.lookupForAdd(source);
+    DependentAddPtr<SourceWeakMap> p(cx, sources, source);
     if (!p) {
         JSObject *sourceobj = newDebuggerSource(cx, source);
         if (!sourceobj)
             return nullptr;
 
-        /* The allocation may have caused a GC, which can remove table entries. */
-        if (!sources.relookupOrAdd(p, source, sourceobj)) {
+        if (!p.add(sources, source, sourceobj)) {
             js_ReportOutOfMemory(cx);
             return nullptr;
         }
 
         CrossCompartmentKey key(CrossCompartmentKey::DebuggerSource, object, source);
         if (!object->compartment()->putWrapper(key, ObjectValue(*sourceobj))) {
             sources.remove(source);
             js_ReportOutOfMemory(cx);
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -71,16 +71,27 @@ class DebuggerWeakMap : private WeakMap<
     using Base::all;
     using Base::trace;
 
     bool init(uint32_t len = 16) {
         return Base::init(len) && zoneCounts.init();
     }
 
     template<typename KeyInput, typename ValueInput>
+    bool putNew(const KeyInput &k, const ValueInput &v) {
+        JS_ASSERT(v->compartment() == Base::compartment);
+        if (!incZoneCount(k->zone()))
+            return false;
+        bool ok = Base::putNew(k, v);
+        if (!ok)
+            decZoneCount(k->zone());
+        return ok;
+    }
+
+    template<typename KeyInput, typename ValueInput>
     bool relookupOrAdd(AddPtr &p, const KeyInput &k, const ValueInput &v) {
         JS_ASSERT(v->compartment() == Base::compartment);
         JS_ASSERT(!p.found());
         if (!incZoneCount(k->zone()))
             return false;
         bool ok = Base::relookupOrAdd(p, k, v);
         if (!ok)
             decZoneCount(k->zone());
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2,16 +2,17 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jscntxt.h"
 #include "jscompartment.h"
 #include "jsfriendapi.h"
+#include "jshashutil.h"
 #include "jsobj.h"
 #include "selfhosted.out.h"
 
 #include "builtin/Intl.h"
 #include "builtin/ParallelArray.h"
 #include "builtin/TypedObject.h"
 #include "gc/Marking.h"
 #include "vm/ForkJoin.h"
@@ -860,17 +861,17 @@ GetObjectAllocKindForClone(JSRuntime *rt
     if (CanBeFinalizedInBackground(kind, obj->getClass()))
         kind = GetBackgroundAllocKind(kind);
     return kind;
 }
 
 static JSObject *
 CloneObject(JSContext *cx, HandleObject srcObj, CloneMemory &clonedObjects)
 {
-    CloneMemory::AddPtr p = clonedObjects.lookupForAdd(srcObj.get());
+    DependentAddPtr<CloneMemory> p(cx, clonedObjects, srcObj.get());
     if (p)
         return p->value;
     RootedObject clone(cx);
     if (srcObj->is<JSFunction>()) {
         if (srcObj->as<JSFunction>().isWrappable()) {
             clone = srcObj;
             if (!cx->compartment()->wrap(cx, &clone))
                 return nullptr;
@@ -899,19 +900,22 @@ CloneObject(JSContext *cx, HandleObject 
     } else if (srcObj->is<ArrayObject>()) {
         clone = NewDenseEmptyArray(cx, nullptr, TenuredObject);
     } else {
         JS_ASSERT(srcObj->isNative());
         clone = NewObjectWithGivenProto(cx, srcObj->getClass(), nullptr, cx->global(),
                                         GetObjectAllocKindForClone(cx->runtime(), srcObj),
                                         SingletonObject);
     }
-    if (!clone || !clonedObjects.relookupOrAdd(p, srcObj.get(), clone.get()) ||
-        !CloneProperties(cx, srcObj, clone, clonedObjects))
-    {
+    if (!clone)
+        return nullptr;
+    if (!p.add(clonedObjects, srcObj, clone))
+        return nullptr;
+    if (!CloneProperties(cx, srcObj, clone, clonedObjects)) {
+        clonedObjects.remove(srcObj);
         return nullptr;
     }
     return clone;
 }
 
 static bool
 CloneValue(JSContext *cx, MutableHandleValue vp, CloneMemory &clonedObjects)
 {
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -9,16 +9,17 @@
 #include "vm/Shape-inl.h"
 
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MathAlgorithms.h"
 #include "mozilla/PodOperations.h"
 
 #include "jsatom.h"
 #include "jscntxt.h"
+#include "jshashutil.h"
 #include "jsobj.h"
 
 #include "js/HashTable.h"
 
 #include "jscntxtinlines.h"
 #include "jsobjinlines.h"
 
 #include "vm/ObjectImpl-inl.h"
@@ -1463,32 +1464,31 @@ 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;
 
-    BaseShapeSet::AddPtr p = table.lookupForAdd(&base);
-
+    DependentAddPtr<BaseShapeSet> p(cx, table, &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 (!p.add(table, &base, nbase))
         return nullptr;
 
     return nbase;
 }
 
 /* static */ UnownedBaseShape *
 BaseShape::lookupUnowned(ThreadSafeContext *cx, const StackBaseShape &base)
 {
@@ -1590,19 +1590,18 @@ 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;
-    InitialShapeSet::AddPtr p =
-        table.lookupForAdd(Lookup(clasp, proto, parent, metadata, nfixed, objectFlags));
-
+    DependentAddPtr<InitialShapeSet>
+        p(cx, table, 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);
     RootedObject parentRoot(cx, parent);
     RootedObject metadataRoot(cx, metadata);
 
@@ -1611,21 +1610,19 @@ 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)))
-    {
+    Lookup lookup(clasp, protoRoot, parentRoot, metadataRoot, nfixed, objectFlags);
+    if (!p.add(table, lookup, InitialShapeEntry(shape, protoRoot)))
         return nullptr;
-    }
 
     return shape;
 }
 
 /* static */ Shape *
 EmptyShape::getInitialShape(ExclusiveContext *cx, const Class *clasp, TaggedProto proto,
                             JSObject *parent, JSObject *metadata,
                             AllocKind kind, uint32_t objectFlags)