From 13099086fa8371c93cc0f4d16c278ee75baae1e3 Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Mon, 25 Nov 2013 15:04:35 +0100 Subject: [PATCH] Backed out changeset 042ab55b8476 (bug 939993) for Spidermonkey rootanalysis orange on a CLOSED TREE --- js/public/HashTable.h | 1 - js/src/jshashutil.h | 66 --------------------------------------- js/src/jsinfer.cpp | 35 ++++++++++++--------- js/src/vm/Debugger.cpp | 20 ++++++------ js/src/vm/Debugger.h | 11 ------- js/src/vm/SelfHosting.cpp | 12 +++---- js/src/vm/Shape.cpp | 17 +++++----- 7 files changed, 44 insertions(+), 118 deletions(-) delete mode 100644 js/src/jshashutil.h diff --git a/js/public/HashTable.h b/js/public/HashTable.h index 2855f78b185f..03265f5d1b88 100644 --- a/js/public/HashTable.h +++ b/js/public/HashTable.h @@ -1511,7 +1511,6 @@ class HashTable : private AllocPolicy 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)); diff --git a/js/src/jshashutil.h b/js/src/jshashutil.h deleted file mode 100644 index 6192061b6480..000000000000 --- a/js/src/jshashutil.h +++ /dev/null @@ -1,66 +0,0 @@ -/* -*- 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 -struct DependentAddPtr -{ - typedef typename T::AddPtr AddPtr; - typedef typename T::Entry Entry; - - template - 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 - 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 diff --git a/js/src/jsinfer.cpp b/js/src/jsinfer.cpp index d1e4e222e73f..821aae1c9484 100644 --- a/js/src/jsinfer.cpp +++ b/js/src/jsinfer.cpp @@ -14,7 +14,6 @@ #include "jsautooplen.h" #include "jscntxt.h" #include "jsgc.h" -#include "jshashutil.h" #include "jsobj.h" #include "jsprf.h" #include "jsscript.h" @@ -2271,11 +2270,7 @@ struct types::ArrayTableKey : public DefaultHasher JSObject *proto; ArrayTableKey() - : type(Type::UndefinedType()), proto(nullptr) - {} - - ArrayTableKey(Type type, JSObject *proto) - : type(type), proto(proto) + : type(Type::UndefinedType()), proto(nullptr) {} static inline uint32_t hash(const ArrayTableKey &v) { @@ -2300,8 +2295,11 @@ TypeCompartment::setTypeToHomogenousArray(ExclusiveContext *cx, } } - ArrayTableKey key(elementType, obj->getProto()); - DependentAddPtr p(cx, *arrayTypeTable, key); + ArrayTableKey key; + key.type = elementType; + key.proto = obj->getProto(); + ArrayTypeTable::AddPtr p = arrayTypeTable->lookupForAdd(key); + if (p) { obj->setType(p->value); } else { @@ -2317,8 +2315,7 @@ TypeCompartment::setTypeToHomogenousArray(ExclusiveContext *cx, if (!objType->unknownProperties()) objType->addPropertyType(cx, JSID_VOID, elementType); - key.proto = objProto; - if (!p.add(*arrayTypeTable, key, objType)) { + if (!arrayTypeTable->relookupOrAdd(p, key, objType)) { cx->compartment()->types.setPendingNukeTypes(cx); return; } @@ -3750,9 +3747,9 @@ ExclusiveContext::getNewType(const Class *clasp, TaggedProto proto_, JSFunction if (!newTypeObjects.initialized() && !newTypeObjects.init()) return nullptr; - DependentAddPtr p(this, newTypeObjects, - TypeObjectSet::Lookup(clasp, proto_)); + TypeObjectSet::AddPtr p = newTypeObjects.lookupForAdd(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); @@ -3790,7 +3787,15 @@ ExclusiveContext::getNewType(const Class *clasp, TaggedProto proto_, JSFunction if (!type) return nullptr; - if (!p.add(newTypeObjects, TypeObjectSet::Lookup(clasp, proto), type.get())) + /* + * 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) return nullptr; #ifdef JSGC_GENERATIONAL @@ -3857,7 +3862,7 @@ ExclusiveContext::getLazyType(const Class *clasp, TaggedProto proto) if (!table.initialized() && !table.init()) return nullptr; - DependentAddPtr p(this, table, TypeObjectSet::Lookup(clasp, proto)); + TypeObjectSet::AddPtr p = table.lookupForAdd(TypeObjectSet::Lookup(clasp, proto)); if (p) { TypeObject *type = *p; JS_ASSERT(type->lazy()); @@ -3870,7 +3875,7 @@ ExclusiveContext::getLazyType(const Class *clasp, TaggedProto proto) if (!type) return nullptr; - if (!p.add(table, TypeObjectSet::Lookup(clasp, protoRoot), type)) + if (!table.relookupOrAdd(p, TypeObjectSet::Lookup(clasp, protoRoot), type)) return nullptr; type->singleton = (JSObject *) TypeObject::LAZY_SINGLETON; diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index ded4f2b2753c..e319a43419ca 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -8,7 +8,6 @@ #include "jscntxt.h" #include "jscompartment.h" -#include "jshashutil.h" #include "jsnum.h" #include "jsobj.h" #include "jswrapper.h" @@ -653,7 +652,7 @@ Debugger::wrapEnvironment(JSContext *cx, Handle env, MutableHandleValue rv JS_ASSERT(!env->is()); JSObject *envobj; - DependentAddPtr p(cx, environments, env); + ObjectWeakMap::AddPtr p = environments.lookupForAdd(env); if (p) { envobj = p->value; } else { @@ -664,7 +663,7 @@ Debugger::wrapEnvironment(JSContext *cx, Handle env, MutableHandleValue rv return false; envobj->setPrivateGCThing(env); envobj->setReservedSlot(JSSLOT_DEBUGENV_OWNER, ObjectValue(*object)); - if (!p.add(environments, env, envobj)) { + if (!environments.relookupOrAdd(p, env, envobj)) { js_ReportOutOfMemory(cx); return false; } @@ -688,7 +687,7 @@ Debugger::wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp) if (vp.isObject()) { RootedObject obj(cx, &vp.toObject()); - DependentAddPtr p(cx, objects, obj); + ObjectWeakMap::AddPtr p = objects.lookupForAdd(obj); if (p) { vp.setObject(*p->value); } else { @@ -700,8 +699,7 @@ Debugger::wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp) return false; dobj->setPrivateGCThing(obj); dobj->setReservedSlot(JSSLOT_DEBUGOBJECT_OWNER, ObjectValue(*object)); - - if (!p.add(objects, obj, dobj)) { + if (!objects.relookupOrAdd(p, obj, dobj)) { js_ReportOutOfMemory(cx); return false; } @@ -2759,13 +2757,14 @@ Debugger::wrapScript(JSContext *cx, HandleScript script) { assertSameCompartment(cx, object.get()); JS_ASSERT(cx->compartment() != script->compartment()); - DependentAddPtr p(cx, scripts, script); + ScriptWeakMap::AddPtr p = scripts.lookupForAdd(script); if (!p) { JSObject *scriptobj = newDebuggerScript(cx, script); if (!scriptobj) return nullptr; - if (!p.add(scripts, script, scriptobj)) { + /* The allocation may have caused a GC, which can remove table entries. */ + if (!scripts.relookupOrAdd(p, script, scriptobj)) { js_ReportOutOfMemory(cx); return nullptr; } @@ -3650,13 +3649,14 @@ Debugger::wrapSource(JSContext *cx, HandleScriptSource source) { assertSameCompartment(cx, object.get()); JS_ASSERT(cx->compartment() != source->compartment()); - DependentAddPtr p(cx, sources, source); + SourceWeakMap::AddPtr p = sources.lookupForAdd(source); if (!p) { JSObject *sourceobj = newDebuggerSource(cx, source); if (!sourceobj) return nullptr; - if (!p.add(sources, source, sourceobj)) { + /* The allocation may have caused a GC, which can remove table entries. */ + if (!sources.relookupOrAdd(p, source, sourceobj)) { js_ReportOutOfMemory(cx); return nullptr; } diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index 38996b80b05a..336d4100706c 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -75,17 +75,6 @@ class DebuggerWeakMap : private WeakMap > return Base::init(len) && zoneCounts.init(); } - template - 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 bool relookupOrAdd(AddPtr &p, const KeyInput &k, const ValueInput &v) { JS_ASSERT(v->compartment() == Base::compartment); diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp index c47050d7954b..48a2f44546e9 100644 --- a/js/src/vm/SelfHosting.cpp +++ b/js/src/vm/SelfHosting.cpp @@ -7,7 +7,6 @@ #include "jscntxt.h" #include "jscompartment.h" #include "jsfriendapi.h" -#include "jshashutil.h" #include "jsobj.h" #include "selfhosted.out.h" @@ -866,7 +865,7 @@ GetObjectAllocKindForClone(JSRuntime *rt, JSObject *obj) static JSObject * CloneObject(JSContext *cx, HandleObject srcObj, CloneMemory &clonedObjects) { - DependentAddPtr p(cx, clonedObjects, srcObj.get()); + CloneMemory::AddPtr p = clonedObjects.lookupForAdd(srcObj.get()); if (p) return p->value; RootedObject clone(cx); @@ -905,12 +904,9 @@ CloneObject(JSContext *cx, HandleObject srcObj, CloneMemory &clonedObjects) GetObjectAllocKindForClone(cx->runtime(), srcObj), SingletonObject); } - if (!clone) - return nullptr; - if (!p.add(clonedObjects, srcObj, clone)) - return nullptr; - if (!CloneProperties(cx, srcObj, clone, clonedObjects)) { - clonedObjects.remove(srcObj); + if (!clone || !clonedObjects.relookupOrAdd(p, srcObj.get(), clone.get()) || + !CloneProperties(cx, srcObj, clone, clonedObjects)) + { return nullptr; } return clone; diff --git a/js/src/vm/Shape.cpp b/js/src/vm/Shape.cpp index ca0c98853a09..a86e483727de 100644 --- a/js/src/vm/Shape.cpp +++ b/js/src/vm/Shape.cpp @@ -14,7 +14,6 @@ #include "jsatom.h" #include "jscntxt.h" -#include "jshashutil.h" #include "jsobj.h" #include "js/HashTable.h" @@ -1469,7 +1468,8 @@ BaseShape::getUnowned(ExclusiveContext *cx, const StackBaseShape &base) if (!table.initialized() && !table.init()) return nullptr; - DependentAddPtr p(cx, table, &base); + BaseShapeSet::AddPtr p = table.lookupForAdd(&base); + if (p) return *p; @@ -1483,7 +1483,7 @@ BaseShape::getUnowned(ExclusiveContext *cx, const StackBaseShape &base) UnownedBaseShape *nbase = static_cast(nbase_); - if (!p.add(table, &base, nbase)) + if (!table.relookupOrAdd(p, &base, nbase)) return nullptr; return nbase; @@ -1595,8 +1595,9 @@ EmptyShape::getInitialShape(ExclusiveContext *cx, const Class *clasp, TaggedProt return nullptr; typedef InitialShapeEntry::Lookup Lookup; - DependentAddPtr - p(cx, table, Lookup(clasp, proto, parent, metadata, nfixed, objectFlags)); + InitialShapeSet::AddPtr p = + table.lookupForAdd(Lookup(clasp, proto, parent, metadata, nfixed, objectFlags)); + if (p) return p->shape; @@ -1615,9 +1616,11 @@ EmptyShape::getInitialShape(ExclusiveContext *cx, const Class *clasp, TaggedProt return nullptr; new (shape) EmptyShape(nbase, nfixed); - Lookup lookup(clasp, protoRoot, parentRoot, metadataRoot, nfixed, objectFlags); - if (!p.add(table, lookup, InitialShapeEntry(shape, protoRoot))) + if (!table.relookupOrAdd(p, Lookup(clasp, protoRoot, parentRoot, metadataRoot, nfixed, objectFlags), + InitialShapeEntry(shape, protoRoot))) + { return nullptr; + } return shape; }