Bug 782467 - Remove sharpObjectMap and simplify obj_toSource; r=Waldo r=njn

The sharpObjectMap is only needed to check for recursion now. This means we can
share the necessary cycle detector code with busyArrays and remove the last
chunk of sharp related code.

--HG--
rename : content/html/content/test/test_bug786564.html => content/html/content/test/test_bug742549.html
extra : rebase_source : 0bcfacf3d5c4f7fbfa1774263456b9c2f7288d01
This commit is contained in:
Terrence Cole 2012-08-17 16:48:02 -07:00
parent 8575595649
commit 6c40544761
8 changed files with 113 additions and 434 deletions

View File

@ -236,6 +236,7 @@ typedef MutableHandle<Value> MutableHandleValue;
* rooted.
*/
typedef JSObject * RawObject;
typedef JSString * RawString;
/*
* By default, pointers should use the inheritance hierarchy to find their

View File

@ -1364,39 +1364,6 @@ JSObject::makeDenseArraySlow(JSContext *cx, HandleObject obj)
}
#if JS_HAS_TOSOURCE
class ArraySharpDetector
{
JSContext *cx;
bool success;
bool alreadySeen;
bool sharp;
public:
ArraySharpDetector(JSContext *cx)
: cx(cx),
success(false),
alreadySeen(false),
sharp(false)
{}
bool init(HandleObject obj) {
success = js_EnterSharpObject(cx, obj, NULL, &alreadySeen, &sharp);
if (!success)
return false;
return true;
}
bool initiallySharp() const {
JS_ASSERT_IF(sharp, alreadySeen);
return sharp;
}
~ArraySharpDetector() {
if (success && !sharp)
js_LeaveSharpObject(cx, NULL);
}
};
JS_ALWAYS_INLINE bool
IsArray(const Value &v)
{
@ -1411,13 +1378,13 @@ array_toSource_impl(JSContext *cx, CallArgs args)
Rooted<JSObject*> obj(cx, &args.thisv().toObject());
RootedValue elt(cx);
ArraySharpDetector detector(cx);
if (!detector.init(obj))
AutoCycleDetector detector(cx, obj);
if (!detector.init())
return false;
StringBuffer sb(cx);
if (detector.initiallySharp()) {
if (detector.foundCycle()) {
if (!sb.append("[]"))
return false;
goto make_string;
@ -1481,52 +1448,6 @@ array_toSource(JSContext *cx, unsigned argc, Value *vp)
}
#endif
class AutoArrayCycleDetector
{
JSContext *cx;
JSObject *obj;
uint32_t genBefore;
BusyArraysSet::AddPtr hashPointer;
bool cycle;
JS_DECL_USE_GUARD_OBJECT_NOTIFIER
public:
AutoArrayCycleDetector(JSContext *cx, JSObject *obj JS_GUARD_OBJECT_NOTIFIER_PARAM)
: cx(cx),
obj(obj),
cycle(true)
{
JS_GUARD_OBJECT_NOTIFIER_INIT;
}
bool init()
{
BusyArraysSet &set = cx->busyArrays;
hashPointer = set.lookupForAdd(obj);
if (!hashPointer) {
if (!set.add(hashPointer, obj))
return false;
cycle = false;
genBefore = set.generation();
}
return true;
}
~AutoArrayCycleDetector()
{
if (!cycle) {
if (genBefore == cx->busyArrays.generation())
cx->busyArrays.remove(hashPointer);
else
cx->busyArrays.remove(obj);
}
}
bool foundCycle() { return cycle; }
protected:
};
static bool
array_join_sub(JSContext *cx, CallArgs &args, bool locale)
{
@ -1539,7 +1460,7 @@ array_join_sub(JSContext *cx, CallArgs &args, bool locale)
if (!obj)
return false;
AutoArrayCycleDetector detector(cx, obj);
AutoCycleDetector detector(cx, obj);
if (!detector.init())
return false;
@ -1553,7 +1474,6 @@ array_join_sub(JSContext *cx, CallArgs &args, bool locale)
if (!GetLengthProperty(cx, obj, &length))
return false;
// Steps 4 and 5
RootedString sepstr(cx, NULL);
if (!locale && args.hasDefined(0)) {

View File

@ -63,6 +63,41 @@
using namespace js;
using namespace js::gc;
bool
js::AutoCycleDetector::init()
{
ObjectSet &set = cx->cycleDetectorSet;
hashsetAddPointer = set.lookupForAdd(obj);
if (!hashsetAddPointer) {
if (!set.add(hashsetAddPointer, obj))
return false;
cyclic = false;
hashsetGenerationAtInit = set.generation();
}
return true;
}
js::AutoCycleDetector::~AutoCycleDetector()
{
if (!cyclic) {
if (hashsetGenerationAtInit == cx->cycleDetectorSet.generation())
cx->cycleDetectorSet.remove(hashsetAddPointer);
else
cx->cycleDetectorSet.remove(obj);
}
}
void
js::TraceCycleDetectionSet(JSTracer *trc, js::ObjectSet &set)
{
for (js::ObjectSet::Enum e(set); !e.empty(); e.popFront()) {
JSObject *prior = e.front();
MarkObjectRoot(trc, const_cast<JSObject **>(&e.front()), "cycle detector table entry");
if (prior != e.front())
e.rekeyFront(e.front());
}
}
struct CallbackData
{
CallbackData(JSMallocSizeOfFun f) : mallocSizeOf(f), n(0) {}
@ -296,7 +331,7 @@ js::NewContext(JSRuntime *rt, size_t stackChunkSize)
JS_ASSERT(cx->findVersion() == JSVERSION_DEFAULT);
if (!cx->busyArrays.init()) {
if (!cx->cycleDetectorSet.init()) {
Foreground::delete_(cx);
return NULL;
}
@ -1069,7 +1104,7 @@ JSContext::JSContext(JSRuntime *rt)
defaultCompartmentObject_(NULL),
stack(thisDuringConstruction()),
parseMapPool_(NULL),
sharpObjectMap(thisDuringConstruction()),
cycleDetectorSet(thisDuringConstruction()),
argumentFormatMap(NULL),
lastMessage(NULL),
errorReporter(NULL),
@ -1376,7 +1411,7 @@ JSContext::sizeOfIncludingThis(JSMallocSizeOfFun mallocSizeOf) const
* ones have been found by DMD to be worth measuring. More stuff may be
* added later.
*/
return mallocSizeOf(this) + busyArrays.sizeOfExcludingThis(mallocSizeOf);
return mallocSizeOf(this) + cycleDetectorSet.sizeOfExcludingThis(mallocSizeOf);
}
void
@ -1390,8 +1425,7 @@ JSContext::mark(JSTracer *trc)
if (isExceptionPending())
MarkValueRoot(trc, &exception, "exception");
if (sharpObjectMap.depth > 0)
js_TraceSharpMap(trc, &sharpObjectMap);
TraceCycleDetectionSet(trc, cycleDetectorSet);
MarkValueRoot(trc, &iterValue, "iterValue");
}

View File

@ -43,27 +43,39 @@ JS_BEGIN_EXTERN_C
struct DtoaState;
JS_END_EXTERN_C
struct JSSharpInfo {
bool hasGen;
bool isSharp;
JSSharpInfo() : hasGen(false), isSharp(false) {}
};
typedef js::HashMap<JSObject *, JSSharpInfo> JSSharpTable;
struct JSSharpObjectMap {
unsigned depth;
uint32_t sharpgen;
JSSharpTable table;
JSSharpObjectMap(JSContext *cx) : depth(0), sharpgen(0), table(js::TempAllocPolicy(cx)) {
table.init();
}
};
namespace js {
typedef HashSet<JSObject *> ObjectSet;
/* Detects cycles when traversing an object graph. */
class AutoCycleDetector
{
JSContext *cx;
JSObject *obj;
bool cyclic;
uint32_t hashsetGenerationAtInit;
ObjectSet::AddPtr hashsetAddPointer;
JS_DECL_USE_GUARD_OBJECT_NOTIFIER
public:
AutoCycleDetector(JSContext *cx, JSObject *obj
JS_GUARD_OBJECT_NOTIFIER_PARAM)
: cx(cx), obj(obj), cyclic(true)
{
JS_GUARD_OBJECT_NOTIFIER_INIT;
}
~AutoCycleDetector();
bool init();
bool foundCycle() { return cyclic; }
};
/* Updates references in the cycle detection set if the GC moves them. */
extern void
TraceCycleDetectionSet(JSTracer *trc, ObjectSet &set);
namespace mjit {
class JaegerRuntime;
}
@ -1100,10 +1112,6 @@ VersionIsKnown(JSVersion version)
return VersionNumber(version) != JSVERSION_UNKNOWN;
}
typedef HashSet<JSObject *,
DefaultHasher<JSObject *>,
SystemAllocPolicy> BusyArraysSet;
inline void
FreeOp::free_(void* p) {
if (shouldFreeLater()) {
@ -1227,8 +1235,7 @@ struct JSContext : js::ContextFriendFields
public:
/* State for object and array toSource conversion. */
JSSharpObjectMap sharpObjectMap;
js::BusyArraysSet busyArrays;
js::ObjectSet cycleDetectorSet;
/* Argument formatter support for JS_{Convert,Push}Arguments{,VA}. */
JSArgumentFormatMap *argumentFormatMap;

View File

@ -102,266 +102,24 @@ JS_ObjectToOuterObject(JSContext *cx, JSObject *obj_)
return GetOuterObject(cx, obj);
}
static bool
MarkSharpObjects(JSContext *cx, HandleObject obj, JSIdArray **idap, JSSharpInfo *value)
{
JS_CHECK_RECURSION(cx, return false);
JSIdArray *ida;
JSSharpObjectMap *map = &cx->sharpObjectMap;
JS_ASSERT(map->depth >= 1);
JSSharpInfo sharpid;
JSSharpTable::Ptr p = map->table.lookup(obj);
if (!p) {
if (!map->table.put(obj.get(), sharpid))
return false;
ida = JS_Enumerate(cx, obj);
if (!ida)
return false;
bool ok = true;
RootedId id(cx);
for (int i = 0, length = ida->length; i < length; i++) {
id = ida->vector[i];
RootedObject obj2(cx);
RootedShape prop(cx);
ok = JSObject::lookupGeneric(cx, obj, id, &obj2, &prop);
if (!ok)
break;
if (!prop)
continue;
bool hasGetter, hasSetter;
RootedValue value(cx), setter(cx);
if (obj2->isNative()) {
Shape *shape = (Shape *) prop;
hasGetter = shape->hasGetterValue();
hasSetter = shape->hasSetterValue();
if (hasGetter)
value = shape->getterValue();
if (hasSetter)
setter = shape->setterValue();
} else {
hasGetter = hasSetter = false;
}
if (hasSetter) {
/* Mark the getter, then set val to setter. */
if (hasGetter && value.isObject()) {
Rooted<JSObject*> vobj(cx, &value.toObject());
ok = MarkSharpObjects(cx, vobj, NULL, NULL);
if (!ok)
break;
}
value = setter;
} else if (!hasGetter) {
ok = JSObject::getGeneric(cx, obj, obj, id, &value);
if (!ok)
break;
}
if (value.isObject()) {
Rooted<JSObject*> vobj(cx, &value.toObject());
if (!MarkSharpObjects(cx, vobj, NULL, NULL)) {
ok = false;
break;
}
}
}
if (!ok || !idap)
JS_DestroyIdArray(cx, ida);
if (!ok)
return false;
} else {
if (!p->value.hasGen && !p->value.isSharp) {
p->value.hasGen = true;
}
sharpid = p->value;
ida = NULL;
}
if (idap)
*idap = ida;
if (value)
*value = sharpid;
return true;
}
bool
js_EnterSharpObject(JSContext *cx, HandleObject obj, JSIdArray **idap, bool *alreadySeen, bool *isSharp)
{
if (!JS_CHECK_OPERATION_LIMIT(cx))
return false;
*alreadySeen = false;
JSSharpObjectMap *map = &cx->sharpObjectMap;
JS_ASSERT_IF(map->depth == 0, map->table.count() == 0);
JS_ASSERT_IF(map->table.count() == 0, map->depth == 0);
JSSharpTable::Ptr p;
JSSharpInfo sharpid;
JSIdArray *ida = NULL;
/* From this point the control must flow either through out: or bad:. */
if (map->depth == 0) {
JS_KEEP_ATOMS(cx->runtime);
/*
* Although MarkSharpObjects tries to avoid invoking getters,
* it ends up doing so anyway under some circumstances; for
* example, if a wrapped object has getters, the wrapper will
* prevent MarkSharpObjects from recognizing them as such.
* This could lead to js_LeaveSharpObject being called while
* MarkSharpObjects is still working.
*
* Increment map->depth while we call MarkSharpObjects, to
* ensure that such a call doesn't free the hash table we're
* still using.
*/
map->depth = 1;
bool success = MarkSharpObjects(cx, obj, &ida, &sharpid);
JS_ASSERT(map->depth == 1);
map->depth = 0;
if (!success)
goto bad;
JS_ASSERT(!sharpid.isSharp);
if (!idap) {
JS_DestroyIdArray(cx, ida);
ida = NULL;
}
} else {
/*
* It's possible that the value of a property has changed from the
* first time the object's properties are traversed (when the property
* ids are entered into the hash table) to the second (when they are
* converted to strings), i.e., the JSObject::getProperty() call is not
* idempotent.
*/
p = map->table.lookup(obj);
if (!p) {
if (!map->table.put(obj.get(), sharpid))
goto bad;
goto out;
}
sharpid = p->value;
}
if (sharpid.isSharp || sharpid.hasGen)
*alreadySeen = true;
out:
if (!sharpid.isSharp) {
if (idap && !ida) {
ida = JS_Enumerate(cx, obj);
if (!ida)
goto bad;
}
map->depth++;
}
if (idap)
*idap = ida;
*isSharp = sharpid.isSharp;
return true;
bad:
/* Clean up the sharpObjectMap table on outermost error. */
if (map->depth == 0) {
JS_UNKEEP_ATOMS(cx->runtime);
map->sharpgen = 0;
map->table.clear();
}
return false;
}
void
js_LeaveSharpObject(JSContext *cx, JSIdArray **idap)
{
JSSharpObjectMap *map = &cx->sharpObjectMap;
JS_ASSERT(map->depth > 0);
if (--map->depth == 0) {
JS_UNKEEP_ATOMS(cx->runtime);
map->sharpgen = 0;
map->table.clear();
}
if (idap) {
if (JSIdArray *ida = *idap) {
JS_DestroyIdArray(cx, ida);
*idap = NULL;
}
}
}
void
js_TraceSharpMap(JSTracer *trc, JSSharpObjectMap *map)
{
JS_ASSERT(map->depth > 0);
/*
* During recursive calls to MarkSharpObjects a non-native object or
* object with a custom getProperty method can potentially return an
* unrooted value or even cut from the object graph an argument of one of
* MarkSharpObjects recursive invocations. So we must protect map->table
* entries against GC.
*
* We can not simply use JSTempValueRooter to mark the obj argument of
* MarkSharpObjects during recursion as we have to protect *all* entries
* in JSSharpObjectMap including those that contains otherwise unreachable
* objects just allocated through custom getProperty. Otherwise newer
* allocations can re-use the address of an object stored in the hashtable
* confusing js_EnterSharpObject. So to address the problem we simply
* mark all objects from map->table.
*
* An alternative "proper" solution is to use JSTempValueRooter in
* MarkSharpObjects with code to remove during finalization entries
* with otherwise unreachable objects. But this is way too complex
* to justify spending efforts.
*/
for (JSSharpTable::Range r = map->table.all(); !r.empty(); r.popFront()) {
JSObject *tmp = r.front().key;
MarkObjectRoot(trc, &tmp, "sharp table entry");
JS_ASSERT(tmp == r.front().key);
}
}
#if JS_HAS_TOSOURCE
static JSBool
obj_toSource(JSContext *cx, unsigned argc, Value *vp)
{
CallArgs args = CallArgsFromVp(argc, vp);
bool comma = false;
const jschar *vchars;
size_t vlength;
Value *val;
JSString *gsop[2];
SkipRoot skipGsop(cx, &gsop, 2);
JS_CHECK_RECURSION(cx, return JS_FALSE);
Value localroot[4];
PodArrayZero(localroot);
AutoArrayRooter tvr(cx, ArrayLength(localroot), localroot);
/* If outermost, we need parentheses to be an expression, not a block. */
bool outermost = (cx->sharpObjectMap.depth == 0);
RootedObject obj(cx, ToObject(cx, args.thisv()));
if (!obj)
return false;
JSIdArray *ida;
bool alreadySeen = false;
bool isSharp = false;
if (!js_EnterSharpObject(cx, obj, &ida, &alreadySeen, &isSharp))
return false;
/* If outermost, we need parentheses to be an expression, not a block. */
bool outermost = (cx->cycleDetectorSet.count() == 0);
if (!ida) {
/*
* We've already seen obj, so don't serialize it again (particularly as
* we might recur in the process): just serialize an empty object.
*/
JS_ASSERT(alreadySeen);
AutoCycleDetector detector(cx, obj);
if (!detector.init())
return false;
if (detector.foundCycle()) {
JSString *str = js_NewStringCopyZ(cx, "{}");
if (!str)
return false;
@ -369,62 +127,36 @@ obj_toSource(JSContext *cx, unsigned argc, Value *vp)
return true;
}
JS_ASSERT(!isSharp);
if (alreadySeen) {
JSSharpTable::Ptr p = cx->sharpObjectMap.table.lookup(obj);
JS_ASSERT(p);
JS_ASSERT(!p->value.isSharp);
p->value.isSharp = true;
}
/* Automatically call js_LeaveSharpObject when we leave this frame. */
class AutoLeaveSharpObject {
JSContext *cx;
JSIdArray *ida;
public:
AutoLeaveSharpObject(JSContext *cx, JSIdArray *ida) : cx(cx), ida(ida) {}
~AutoLeaveSharpObject() {
js_LeaveSharpObject(cx, &ida);
}
} autoLeaveSharpObject(cx, ida);
StringBuffer buf(cx);
if (outermost && !buf.append('('))
return false;
if (!buf.append('{'))
return false;
/*
* We have four local roots for cooked and raw value GC safety. Hoist the
* "localroot + 2" out of the loop using the val local, which refers to
* the raw (unconverted, "uncooked") values.
*/
val = localroot + 2;
Value val[2];
PodArrayZero(val);
AutoArrayRooter tvr2(cx, ArrayLength(val), val);
RootedId id(cx);
for (int i = 0; i < ida->length; i++) {
/* Get strings for id and value and GC-root them via vp. */
id = ida->vector[i];
Rooted<JSLinearString*> idstr(cx);
JSString *gsop[2];
SkipRoot skipGsop(cx, &gsop, 2);
AutoIdVector idv(cx);
if (!GetPropertyNames(cx, obj, JSITER_OWNONLY, &idv))
return false;
bool comma = false;
for (size_t i = 0; i < idv.length(); ++i) {
RootedId id(cx, idv[i]);
RootedObject obj2(cx);
RootedShape prop(cx);
if (!JSObject::lookupGeneric(cx, obj, id, &obj2, &prop))
return false;
/*
* Convert id to a value and then to a string. Decide early whether we
* prefer get/set or old getter/setter syntax.
*/
JSString *s = ToString(cx, IdToValue(id));
if (!s || !(idstr = s->ensureLinear(cx)))
RootedShape shape(cx);
if (!JSObject::lookupGeneric(cx, obj, id, &obj2, &shape))
return false;
/* Decide early whether we prefer get/set or old getter/setter syntax. */
int valcnt = 0;
if (prop) {
if (shape) {
bool doGet = true;
if (obj2->isNative()) {
Shape *shape = (Shape *) prop;
unsigned attrs = shape->attributes();
if (attrs & JSPROP_GETTER) {
doGet = false;
@ -448,13 +180,22 @@ obj_toSource(JSContext *cx, unsigned argc, Value *vp)
}
}
/* Convert id to a linear string. */
RawString s = ToString(cx, IdToValue(id));
if (!s)
return false;
Rooted<JSLinearString*> idstr(cx, s->ensureLinear(cx));
if (!idstr)
return false;
/*
* If id is a string that's not an identifier, or if it's a negative
* integer, then it must be quoted.
*/
if (JSID_IS_ATOM(id)
? !IsIdentifier(idstr)
: (!JSID_IS_INT(id) || JSID_TO_INT(id) < 0)) {
: (!JSID_IS_INT(id) || JSID_TO_INT(id) < 0))
{
s = js_QuoteString(cx, idstr, jschar('\''));
if (!s || !(idstr = s->ensureLinear(cx)))
return false;
@ -469,14 +210,13 @@ obj_toSource(JSContext *cx, unsigned argc, Value *vp)
continue;
/* Convert val[j] to its canonical source form. */
JSString *valstr = js_ValueToSource(cx, val[j]);
RootedString valstr(cx, js_ValueToSource(cx, val[j]));
if (!valstr)
return false;
localroot[j].setString(valstr); /* local root */
vchars = valstr->getChars(cx);
const jschar *vchars = valstr->getChars(cx);
if (!vchars)
return false;
vlength = valstr->length();
size_t vlength = valstr->length();
/*
* Remove '(function ' from the beginning of valstr and ')' from the
@ -536,7 +276,7 @@ obj_toSource(JSContext *cx, unsigned argc, Value *vp)
if (outermost && !buf.append(')'))
return false;
JSString *str = buf.finishString();
RawString str = buf.finishString();
if (!str)
return false;
args.rval().setString(str);

View File

@ -1080,20 +1080,6 @@ class ValueArray {
ValueArray(js::Value *v, size_t c) : array(v), length(c) {}
};
/* For manipulating JSContext::sharpObjectMap. */
extern bool
js_EnterSharpObject(JSContext *cx, js::HandleObject obj, JSIdArray **idap, bool *alreadySeen, bool *isSharp);
extern void
js_LeaveSharpObject(JSContext *cx, JSIdArray **idap);
/*
* Mark objects stored in map if GC happens between js_EnterSharpObject
* and js_LeaveSharpObject. GC calls this when map->depth > 0.
*/
extern void
js_TraceSharpMap(JSTracer *trc, JSSharpObjectMap *map);
extern JSBool
js_HasOwnPropertyHelper(JSContext *cx, js::LookupGenericOp lookup, js::HandleObject obj,
js::HandleId id, js::MutableHandleValue rval);

View File

@ -51,7 +51,6 @@ typedef struct JSArgumentFormatMap JSArgumentFormatMap;
typedef struct JSGCThing JSGCThing;
typedef struct JSGenerator JSGenerator;
typedef struct JSNativeEnumerator JSNativeEnumerator;
typedef struct JSSharpObjectMap JSSharpObjectMap;
typedef struct JSTryNote JSTryNote;
/* Friend "Advanced API" typedefs. */

View File

@ -23,15 +23,7 @@ function test()
q = [];
q.__defineGetter__("0", q.toString);
q[2] = q;
try
{
q.toSource();
throw new Error("didn't throw");
}
catch (e)
{
assertEq(e instanceof InternalError, true, "bad error: " + e);
}
assertEq(q.toSource(), "[\"\", , []]", "wrong string");
reportCompare(expect, actual, summary);