Change js::Debug::objects to have referents as keys, rather than cross-compartment wrappers of referents.

This adds support for cross-compartment WeakMaps and changes js::Debug::objects to be one. It eliminates the vexing JSMSG_DEBUG_STREAMS_CROSSED error messsage.

The GC interaction between jsgc and jsdbg is a little more complex now; like the cross-compartment wrapper maps, Debug::objects must be marked (just once) during per-compartment GC. In other ways this is a simplification.
This commit is contained in:
Jason Orendorff 2011-06-20 18:26:05 -05:00
parent 7b711756ef
commit 4370701afc
7 changed files with 134 additions and 59 deletions

View File

@ -0,0 +1,15 @@
// Debug.Object referents can be transparent wrappers of objects in the debugger compartment.
var g = newGlobal('new-compartment');
g.f = function (a, b) { return a + "/" + b; };
var dbg = Debug(g);
var hits = 0;
dbg.hooks = {
debuggerHandler: function (frame) {
var f = frame.eval("f").return;
assertEq(f.call(null, "a", "b").return, "a/b");
hits++;
}
};
g.eval("debugger;");
assertEq(hits, 1);

View File

@ -0,0 +1,14 @@
// Debug.Objects keep their referents alive.
var g = newGlobal('new-compartment');
var dbg = Debug(g);
var arr = [];
dbg.hooks = {debuggerHandler: function (frame) { arr.push(frame.eval("[]").return); }};
g.eval("for (var i = 0; i < 10; i++) debugger;");
assertEq(arr.length, 10);
gc();
for (var i = 0; i < arr.length; i++)
assertEq(arr[i].class, "Array");

View File

@ -354,8 +354,7 @@ MSG_DEF(JSMSG_CCW_REQUIRED, 271, 1, JSEXN_TYPEERR, "{0}: argument must
MSG_DEF(JSMSG_DEBUG_BAD_RESUMPTION, 272, 0, JSEXN_TYPEERR, "debugger resumption value must be undefined, {throw: val}, {return: val}, or null")
MSG_DEF(JSMSG_ASSIGN_FUNCTION_OR_NULL, 273, 1, JSEXN_TYPEERR, "value assigned to {0} must be a function or null")
MSG_DEF(JSMSG_DEBUG_NOT_LIVE, 274, 1, JSEXN_ERR, "{0} is not live")
MSG_DEF(JSMSG_DEBUG_STREAMS_CROSSED, 275, 0, JSEXN_INTERNALERR, "the debuggee has a cross-compartment wrapper of an object in the debugger compartment; can't create a Debug.Object with that wrapper as its referent")
MSG_DEF(JSMSG_DEBUG_OBJECT_WRONG_OWNER, 276, 0, JSEXN_TYPEERR, "Debug.Object belongs to a different Debug")
MSG_DEF(JSMSG_DEBUG_OBJECT_PROTO, 277, 0, JSEXN_TYPEERR, "Debug.Object.prototype is not a valid Debug.Object")
MSG_DEF(JSMSG_DEBUG_LOOP, 278, 0, JSEXN_TYPEERR, "cannot debug an object in same compartment as debugger or a compartment that is already debugging the debugger")
MSG_DEF(JSMSG_DEBUG_NOT_IDLE, 279, 1, JSEXN_ERR, "cannot {0} debugging: a debuggee script is on the stack")
MSG_DEF(JSMSG_DEBUG_OBJECT_WRONG_OWNER, 275, 0, JSEXN_TYPEERR, "Debug.Object belongs to a different Debug")
MSG_DEF(JSMSG_DEBUG_OBJECT_PROTO, 276, 0, JSEXN_TYPEERR, "Debug.Object.prototype is not a valid Debug.Object")
MSG_DEF(JSMSG_DEBUG_LOOP, 277, 0, JSEXN_TYPEERR, "cannot debug an object in same compartment as debugger or a compartment that is already debugging the debugger")
MSG_DEF(JSMSG_DEBUG_NOT_IDLE, 278, 1, JSEXN_ERR, "cannot {0} debugging: a debuggee script is on the stack")

View File

@ -74,7 +74,6 @@ extern Class DebugObject_class;
enum {
JSSLOT_DEBUGOBJECT_OWNER,
JSSLOT_DEBUGOBJECT_CCW, // cross-compartment wrapper
JSSLOT_DEBUGOBJECT_COUNT
};
@ -252,42 +251,31 @@ Debug::wrapDebuggeeValue(JSContext *cx, Value *vp)
{
assertSameCompartment(cx, object);
// FIXME This is not quite what we want. Ideally we would get a transparent
// wrapper no matter what sort of object *vp is. Oh well!
if (!cx->compartment->wrap(cx, vp)) {
vp->setUndefined();
return false;
}
if (vp->isObject()) {
JSObject *ccwobj = &vp->toObject();
vp->setUndefined();
JSObject *obj = &vp->toObject();
// Debug.Object can't reflect objects from the current compartment.
// FIXME Ideally this shouldn't be possible. See FIXME comment above.
if (!ccwobj->isCrossCompartmentWrapper()) {
JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_DEBUG_STREAMS_CROSSED);
return false;
}
ObjectWeakMap::AddPtr p = objects.lookupForAdd(ccwobj);
ObjectWeakMap::AddPtr p = objects.lookupForAdd(obj);
if (p) {
vp->setObject(*p->value);
} else {
// Create a new Debug.Object for ccwobj.
// Create a new Debug.Object for obj.
JSObject *proto = &object->getReservedSlot(JSSLOT_DEBUG_OBJECT_PROTO).toObject();
JSObject *dobj = NewNonFunction<WithProto::Given>(cx, &DebugObject_class, proto, NULL);
if (!dobj || !dobj->ensureClassReservedSlots(cx))
return false;
dobj->setPrivate(obj);
dobj->setReservedSlot(JSSLOT_DEBUGOBJECT_OWNER, ObjectValue(*object));
dobj->setReservedSlot(JSSLOT_DEBUGOBJECT_CCW, ObjectValue(*ccwobj));
if (!objects.relookupOrAdd(p, ccwobj, dobj)) {
if (!objects.relookupOrAdd(p, obj, dobj)) {
js_ReportOutOfMemory(cx);
return false;
}
vp->setObject(*dobj);
}
} else if (!cx->compartment->wrap(cx, vp)) {
vp->setUndefined();
return false;
}
return true;
}
@ -312,7 +300,7 @@ Debug::unwrapDebuggeeValue(JSContext *cx, Value *vp)
return false;
}
*vp = dobj->getReservedSlot(JSSLOT_DEBUGOBJECT_CCW);
vp->setObject(*(JSObject *) dobj->getPrivate());
}
return true;
}
@ -531,6 +519,24 @@ Debug::dispatchHook(JSContext *cx, js::Value *vp, DebugObservesMethod observesEv
// === Debug JSObjects
void
Debug::markCrossCompartmentDebugObjectReferents(JSTracer *tracer)
{
JSCompartment *comp = tracer->context->runtime->gcCurrentCompartment;
// Mark all objects in comp that are referents of Debug.Objects in other
// compartments.
const GlobalObjectSet &debuggees = comp->getDebuggees();
for (GlobalObjectSet::Range r = debuggees.all(); !r.empty(); r.popFront()) {
const GlobalObject::DebugVector *debuggers = r.front()->getDebuggers();
for (Debug **p = debuggers->begin(); p != debuggers->end(); p++) {
Debug *dbg = *p;
if (dbg->object->compartment() != comp)
dbg->objects.markKeysInCompartment(tracer);
}
}
}
bool
Debug::mark(GCMarker *trc, JSCompartment *comp, JSGCInvocationKind gckind)
{
@ -782,14 +788,12 @@ Debug::unwrapDebuggeeArgument(JSContext *cx, Value *vp)
JSObject *obj = NonNullObject(cx, v);
if (obj) {
if (obj->clasp == &DebugObject_class) {
// Get the cross-compartment wrapper (which we unwrap in the next if-block).
if (!unwrapDebuggeeValue(cx, &v))
return NULL;
obj = &v.toObject();
}
if (obj->isCrossCompartmentWrapper()) {
obj = &obj->getProxyPrivate().toObject();
return &v.toObject();
}
if (obj->isCrossCompartmentWrapper())
return &obj->getProxyPrivate().toObject();
}
return obj;
}
@ -1709,10 +1713,24 @@ static JSFunctionSpec DebugFrame_methods[] = {
// === Debug.Object
static void
DebugObject_trace(JSTracer *trc, JSObject *obj)
{
if (JSObject *obj = (JSObject *) obj->getPrivate())
MarkObject(trc, *obj, "Debug.Object referent");
}
Class DebugObject_class = {
"Object", JSCLASS_HAS_PRIVATE | JSCLASS_HAS_RESERVED_SLOTS(JSSLOT_DEBUGOBJECT_COUNT),
PropertyStub, PropertyStub, PropertyStub, StrictPropertyStub,
EnumerateStub, ResolveStub, ConvertStub
EnumerateStub, ResolveStub, ConvertStub, NULL,
NULL, /* reserved0 */
NULL, /* checkAccess */
NULL, /* call */
NULL, /* construct */
NULL, /* xdrObject */
NULL, /* hasInstance */
DebugObject_trace
};
static JSObject *
@ -1731,8 +1749,8 @@ DebugObject_checkThis(JSContext *cx, Value *vp, const char *fnname)
// Forbid Debug.Object.prototype, which is of class DebugObject_class
// but isn't a real working Debug.Object. The prototype object is
// distinguished by having an 'undefined' referent.
if (thisobj->getReservedSlot(JSSLOT_DEBUGOBJECT_CCW).isUndefined()) {
// distinguished by having no referent.
if (!thisobj->getPrivate()) {
JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
"Debug.Object", fnname, "prototype object");
return NULL;
@ -1740,25 +1758,20 @@ DebugObject_checkThis(JSContext *cx, Value *vp, const char *fnname)
return thisobj;
}
#define THIS_DEBUGOBJECT_CCW(cx, vp, fnname, obj) \
JSObject *obj = DebugObject_checkThis(cx, vp, fnname); \
if (!obj) \
return false; \
obj = &obj->getReservedSlot(JSSLOT_DEBUGOBJECT_CCW).toObject(); \
JS_ASSERT(obj->isCrossCompartmentWrapper())
#define THIS_DEBUGOBJECT_REFERENT(cx, vp, fnname, obj) \
THIS_DEBUGOBJECT_CCW(cx, vp, fnname, obj); \
obj = JSWrapper::wrappedObject(obj)
#define THIS_DEBUGOBJECT_REFERENT(cx, vp, fnname, obj) \
JSObject *obj = DebugObject_checkThis(cx, vp, fnname); \
if (!obj) \
return false; \
obj = (JSObject *) obj->getPrivate(); \
JS_ASSERT(obj)
#define THIS_DEBUGOBJECT_OWNER_REFERENT(cx, vp, fnname, dbg, obj) \
JSObject *obj = DebugObject_checkThis(cx, vp, fnname); \
if (!obj) \
return false; \
Debug *dbg = Debug::fromChildJSObject(obj); \
obj = &obj->getReservedSlot(JSSLOT_DEBUGOBJECT_CCW).toObject(); \
JS_ASSERT(obj->isCrossCompartmentWrapper()); \
obj = JSWrapper::wrappedObject(obj)
obj = (JSObject *) obj->getPrivate(); \
JS_ASSERT(obj)
static JSBool
DebugObject_construct(JSContext *cx, uintN argc, Value *vp)

View File

@ -98,14 +98,8 @@ class Debug {
}
};
// The map from debuggee objects to their Debug.Object instances. However, to avoid
// holding cross-compartment references directly, the keys in this map are the
// referents' CCWs, not the referents themselves. Thus, to find the Debug.Object for a
// debuggee object, you must first find its CCW, and then look that up here.
//
// Using CCWs for keys when it's really their referents' liveness that determines the
// table entry's liveness is delicate; see comments on CCWReferentKeyMarkPolicy.
typedef WeakMap<JSObject *, JSObject *, DefaultHasher<JSObject *>, CCWReferentKeyMarkPolicy>
// The map from debuggee objects to their Debug.Object instances.
typedef WeakMap<JSObject *, JSObject *, DefaultHasher<JSObject *>, CrossCompartmentMarkPolicy>
ObjectWeakMap;
ObjectWeakMap objects;
@ -212,6 +206,7 @@ class Debug {
// objects that are definitely live but not yet marked, it marks them and
// returns true. If not, it returns false.
//
static void markCrossCompartmentDebugObjectReferents(JSTracer *tracer);
static bool mark(GCMarker *trc, JSCompartment *compartment, JSGCInvocationKind gckind);
static void sweepAll(JSContext *cx);
static void sweepCompartment(JSContext *cx, JSCompartment *compartment);

View File

@ -2270,6 +2270,7 @@ MarkAndSweep(JSContext *cx, JSCompartment *comp, JSGCInvocationKind gckind GCTIM
if (comp) {
for (JSCompartment **c = rt->compartments.begin(); c != rt->compartments.end(); ++c)
(*c)->markCrossCompartmentWrappers(&gcmarker);
Debug::markCrossCompartmentDebugObjectReferents(&gcmarker);
} else {
js_MarkScriptFilenames(rt);
}

View File

@ -82,7 +82,7 @@ namespace js {
//
// MarkPolicy(JSTracer *)
//
// and the following static member functions:
// and the following member functions:
//
// bool keyMarked(Key &k)
// bool valueMarked(Value &v)
@ -144,8 +144,8 @@ class WeakMapBase {
virtual void sweep(JSTracer *tracer) = 0;
private:
// Link in a list of all the WeakMaps we have marked in this garbage collection,
// headed by JSRuntime::gcWeakMapList.
// Link in a list of WeakMaps to mark iteratively and sweep in this garbage
// collection, headed by JSRuntime::gcWeakMapList.
WeakMapBase *next;
};
@ -199,6 +199,18 @@ class WeakMap : public HashMap<Key, Value, HashPolicy, RuntimeAllocPolicy>, publ
}
#endif
}
public:
// For use with cross-compartment WeakMaps.
void markKeysInCompartment(JSTracer *tracer) {
JSCompartment *comp = tracer->context->runtime->gcCurrentCompartment;
JS_ASSERT(comp);
MarkPolicy t(tracer);
for (Range r = Base::all(); !r.empty(); r.popFront()) {
if (!t.keyMarked(r.front().key))
t.markKey(r.front().key, "cross-compartment WeakMap key");
}
}
};
template <>
@ -237,6 +249,32 @@ class DefaultMarkPolicy<JSObject *, JSObject *> {
JSTracer *tracer;
};
// A MarkPolicy for WeakMaps whose Keys and values may be objects in arbitrary
// compartments within a runtime.
class CrossCompartmentMarkPolicy {
public:
CrossCompartmentMarkPolicy(JSTracer *t)
: tracer(t), comp(t->context->runtime->gcCurrentCompartment) {}
bool keyMarked(JSObject *k) { return isMarked(k); }
bool valueMarked(JSObject *v) { return isMarked(v); }
void markKey(JSObject *k, const char *description) {
js::gc::MarkObject(tracer, *k, description);
}
void markValue(JSObject *v, const char *description) {
js::gc::MarkObject(tracer, *v, description);
}
private:
// During per-compartment GC, if a key or value object is outside the gc
// compartment, consider it marked.
bool isMarked(JSObject *obj) {
return (comp && obj->compartment() != comp) || !IsAboutToBeFinalized(tracer->context, obj);
}
JSTracer *tracer;
JSCompartment *comp;
};
// The class of JavaScript WeakMap objects.
extern Class WeakMapClass;