Bug 939993 - Fix places where HashTable::AddPtr could be used with out-of-date hash value under GGC r=sfink

This commit is contained in:
Jon Coppeard 2013-11-25 11:26:10 +00:00
parent 5ef337c5a4
commit 29990f52e3
6 changed files with 117 additions and 44 deletions

66
js/src/jshashutil.h Normal file
View File

@ -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

View File

@ -14,6 +14,7 @@
#include "jsautooplen.h"
#include "jscntxt.h"
#include "jsgc.h"
#include "jshashutil.h"
#include "jsobj.h"
#include "jsprf.h"
#include "jsscript.h"
@ -2270,7 +2271,11 @@ struct types::ArrayTableKey : public DefaultHasher<types::ArrayTableKey>
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) {
@ -2295,11 +2300,8 @@ TypeCompartment::setTypeToHomogenousArray(ExclusiveContext *cx,
}
}
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 {
@ -2315,7 +2317,8 @@ TypeCompartment::setTypeToHomogenousArray(ExclusiveContext *cx,
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;
}
@ -3747,9 +3750,9 @@ ExclusiveContext::getNewType(const Class *clasp, TaggedProto proto_, JSFunction
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);
@ -3787,15 +3790,7 @@ ExclusiveContext::getNewType(const Class *clasp, TaggedProto proto_, JSFunction
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
@ -3862,7 +3857,7 @@ ExclusiveContext::getLazyType(const Class *clasp, TaggedProto proto)
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());
@ -3875,7 +3870,7 @@ ExclusiveContext::getLazyType(const Class *clasp, TaggedProto proto)
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;

View File

@ -8,6 +8,7 @@
#include "jscntxt.h"
#include "jscompartment.h"
#include "jshashutil.h"
#include "jsnum.h"
#include "jsobj.h"
#include "jswrapper.h"
@ -652,7 +653,7 @@ Debugger::wrapEnvironment(JSContext *cx, Handle<Env*> env, MutableHandleValue rv
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 {
@ -663,7 +664,7 @@ Debugger::wrapEnvironment(JSContext *cx, Handle<Env*> env, MutableHandleValue rv
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;
}
@ -687,7 +688,7 @@ Debugger::wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp)
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 {
@ -699,7 +700,8 @@ Debugger::wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp)
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;
}
@ -2757,14 +2759,13 @@ 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;
}
@ -3649,14 +3650,13 @@ 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;
}

View File

@ -75,6 +75,17 @@ class DebuggerWeakMap : private WeakMap<Key, Value, DefaultHasher<Key> >
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);

View File

@ -7,6 +7,7 @@
#include "jscntxt.h"
#include "jscompartment.h"
#include "jsfriendapi.h"
#include "jshashutil.h"
#include "jsobj.h"
#include "selfhosted.out.h"
@ -865,7 +866,7 @@ GetObjectAllocKindForClone(JSRuntime *rt, JSObject *obj)
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);
@ -904,9 +905,12 @@ CloneObject(JSContext *cx, HandleObject srcObj, CloneMemory &clonedObjects)
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;

View File

@ -14,6 +14,7 @@
#include "jsatom.h"
#include "jscntxt.h"
#include "jshashutil.h"
#include "jsobj.h"
#include "js/HashTable.h"
@ -1468,8 +1469,7 @@ BaseShape::getUnowned(ExclusiveContext *cx, const StackBaseShape &base)
if (!table.initialized() && !table.init())
return nullptr;
BaseShapeSet::AddPtr p = table.lookupForAdd(&base);
DependentAddPtr<BaseShapeSet> p(cx, table, &base);
if (p)
return *p;
@ -1483,7 +1483,7 @@ BaseShape::getUnowned(ExclusiveContext *cx, const StackBaseShape &base)
UnownedBaseShape *nbase = static_cast<UnownedBaseShape *>(nbase_);
if (!table.relookupOrAdd(p, &base, nbase))
if (!p.add(table, &base, nbase))
return nullptr;
return nbase;
@ -1595,9 +1595,8 @@ EmptyShape::getInitialShape(ExclusiveContext *cx, const Class *clasp, TaggedProt
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;
@ -1616,11 +1615,9 @@ EmptyShape::getInitialShape(ExclusiveContext *cx, const Class *clasp, TaggedProt
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;
}