Bug 796073: Identify debuggees only by Debugger.Object instances and CCWs that can be unwrapped securely. r=luke

This patch affects behavior that a lot of debugger tests rely on; I've
tried to update the tests without losing coverage of behavior that is still
supported:

- A prior patch in this series removes uses of addDebuggee to generate
  Debugger.Object instances referring to random objects, using
  makeDebuggeeValue instead.

- The test debug/Debugger-debuggees-07.js is deleted, because it's testing
  for the very behavior we're removing. Other tests are trimmed to remove
  usage that is no longer supported.

- The new test debug/Debugger-debuggees-17.js checks that we reject objects
  that don't designate debuggees. The existing test
  Debugger-debuggees-06.js checks that non-object values are properly
  rejected.

- The new test debug/Debugger-debuggees-18.js checks that globals are
  correctly identified regardless of how we designate them.

js/src/jit-test/tests/debug/Debugger-debuggees-07.js

@@ -1,24 +0,0 @@
-// Handle proto-less objects passed to addDebuggee.
-
-var g = newGlobal('new-compartment');
-var obj = g.eval("Object.create(null)");
-var dbg = new Debugger;
-
-// hasDebuggee and removeDebuggee must tolerate proto-less objects.
-assertEq(dbg.hasDebuggee(obj), false);
-
-// addDebuggee may either succeed or throw a TypeError. Don't care.
-var added;
-try {
-    dbg.addDebuggee(obj);
-    added = true;
-} catch (exc) {
-    if (!(exc instanceof TypeError))
-        throw exc;
-    added = false;
-}
-
-assertEq(dbg.hasDebuggee(obj), added);
-assertEq(dbg.getDebuggees().length, added ? 1 : 0);
-assertEq(dbg.removeDebuggee(obj), undefined);
-assertEq(dbg.hasDebuggee(obj), false);

js/src/jit-test/tests/debug/Debugger-debuggees-08.js

@@ -1,4 +1,3 @@
-// addDebuggee(obj), where obj is not global, adds obj's global.
 // Adding a debuggee more than once is redundant.
 
 var dbg = new Debugger;
@@ -19,18 +18,6 @@ assertEq(dbg.addDebuggee(g), w);
 usual();
 assertEq(dbg.addDebuggee(w), w);
 usual();
-dbg.addDebuggee(g.Math);
-usual();
-dbg.addDebuggee(g.eval("(function () {})"));
-usual();
-
-// w2 is a Debugger.Object in g. Treat it like any other object in g; don't auto-unwrap it.
-g.g2 = newGlobal('new-compartment');
-g.eval("var w2 = new Debugger().addDebuggee(g2)");
-dbg.addDebuggee(g.w2);
-usual();
-assertEq(!dbg.hasDebuggee(g.g2), true);
-assertEq(dbg.hasDebuggee(g.w2), true);
 
 // Removing the debuggee once is enough.
 assertEq(dbg.removeDebuggee(g), undefined);

js/src/jit-test/tests/debug/Debugger-debuggees-09.js

@@ -1,5 +1,4 @@
 // |jit-test| debug
-// Random objects can be the argument to hasDebuggee and removeDebuggee.
 // If hasDebuggee(x) is false, removeDebuggee(x) does nothing.
 
 var dbg = new Debugger;
@@ -12,16 +11,12 @@ function check(obj) {
     assertEq(dbg.removeDebuggee(obj), undefined);
 }
 
-// objects in this compartment, which we can't debug
-check(this);
-check({});
+// global objects which happen not to be debuggees at the moment
 var g1 = newGlobal('same-compartment');
 check(g1);
-check(g1.eval("({})"));
 
 // objects in a compartment that is already debugging us
 var g2 = newGlobal('new-compartment');
 g2.parent = this;
 g2.eval("var dbg = new Debugger(parent);");
 check(g2);
-check(g2.dbg);

js/src/jit-test/tests/debug/Debugger-debuggees-17.js

@@ -0,0 +1,26 @@
+// addDebuggee, hasDebuggee, and removeDebuggee all throw if presented with
+// objects that are not valid global object designators.
+
+load(libdir + 'asserts.js');
+
+var dbg = new Debugger;
+
+function check(bad) {
+  print("check(" + uneval(bad) + ")");
+  assertThrowsInstanceOf(function () { dbg.addDebuggee(bad); }, TypeError);
+  assertEq(dbg.getDebuggees().length, 0);
+  assertThrowsInstanceOf(function () { dbg.hasDebuggee(bad); }, TypeError);
+  assertThrowsInstanceOf(function () { dbg.removeDebuggee(bad); }, TypeError);
+}
+
+var g = newGlobal();
+check(g.Object());
+check(g.Object);
+check(g.Function(""));
+
+// A Debugger.Object belonging to a different Debugger is not a valid way
+// to designate a global, even if its referent is a global.
+var g2 = newGlobal();
+var dbg2 = new Debugger;
+var d2g2 = dbg2.addDebuggee(g2);
+check(d2g2);

js/src/jit-test/tests/debug/Debugger-debuggees-18.js

@@ -0,0 +1,117 @@
+// Debugger.prototype.{addDebuggee,hasDebuggee,removeDebuggee} recognize globals
+// regardless of how they are specified.
+
+var dbg = new Debugger;
+
+// Assert that dbg's debuggees are exactly the set passed as arguments.
+// The arguments are assumed to be Debugger.Object instances referring to
+// globals without wrappers --- which is the sort returned by addDebuggee.
+function assertDebuggees() {
+  print("assertDebuggees([" + [g.toSource() for each (g in arguments)] + "])");
+  var debuggees = dbg.getDebuggees();
+  assertEq(arguments.length, debuggees.length);
+  for each (g in arguments)
+    assertEq(debuggees.indexOf(g) != -1, true);
+}
+
+var g1 = newGlobal(); g1.toSource = function () { return "[global g1]"; };
+var g2 = newGlobal(); g2.toSource = function () { return "[global g2]"; };
+
+assertDebuggees();
+
+// Produce every possible way to designate g1, for us to play with.
+// Globals can be designated by any of the following:
+//
+// - "CCW": a Cross-Compartment Wrapper (CCW) of a global object
+// - "D.O": a Debugger.Object whose referent is a global object
+// - "D.O of CCW": a Debugger.Object whose referent is a CCW of a
+//   global object, where the CCW can be securely unwrapped
+//
+// There's no direct "G", since globals are always in their own
+// compartments, never the debugger's; if we ever viewed them directly,
+// that would be a compartment violation.
+
+// "dg1" means "Debugger.Object referring (directly) to g1".
+var dg1 = dbg.addDebuggee(g1);
+dg1.toSource = function() { return "[Debugger.Object for global g1]"; };
+assertEq(dg1.global, dg1);
+assertEq(dg1.unwrap(), dg1);
+assertDebuggees(dg1);
+
+// We need to add g2 as a debuggee; that's the only way to get a D.O referring
+// to it without a wrapper.
+var dg2 = dbg.addDebuggee(g2);
+dg2.toSource = function() { return "[Debugger.Object for global g2]"; };
+assertEq(dg2.global, dg2);
+assertEq(dg2.unwrap(), dg2);
+assertDebuggees(dg1, dg2);
+
+// "dwg1" means "Debugger.Object referring to CCW of g1".
+var dwg1 = dg2.makeDebuggeeValue(g1);
+assertEq(dwg1.global, dg2);
+assertEq(dwg1.unwrap(), dg1);
+dwg1.toSource = function() { return "[Debugger.Object for CCW of global g1]"; };
+
+assertDebuggees(dg1, dg2);
+assertEq(dbg.removeDebuggee(g1), undefined);
+assertEq(dbg.removeDebuggee(g2), undefined);
+assertDebuggees();
+
+// Systematically cover all the single-global possibilities:
+//
+//  | added as    | designated as | addDebuggee | hasDebuggee | removeDebuggee |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | (not added) | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | CCW         | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | D.O         | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | D.O of CCW  | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+
+// Cover the "(not added)" section of the table, other than "addDebuggee":
+assertEq(dbg.hasDebuggee(g1), false);
+assertEq(dbg.hasDebuggee(dg1), false);
+assertEq(dbg.hasDebuggee(dwg1), false);
+
+assertEq(dbg.removeDebuggee(g1), undefined); assertDebuggees();
+assertEq(dbg.removeDebuggee(dg1), undefined); assertDebuggees();
+assertEq(dbg.removeDebuggee(dwg1), undefined); assertDebuggees();
+
+// Try all operations adding the debuggee using |addAs|, and operating on it
+// using |designateAs|, thereby covering one row of the table (outside the '(not
+// added)' section), and one case in the '(not added)', 'designated as' section.
+//
+// |Direct| should be the Debugger.Object referring directly to the debuggee
+// global, for checking the results from addDebuggee and getDebuggees.
+function combo(addAs, designateAs, direct) {
+  print("combo(" + uneval(addAs) + ", " + uneval(designateAs) + ")");
+  assertDebuggees();
+  assertEq(dbg.addDebuggee(addAs), direct);
+  assertDebuggees(direct);
+  assertEq(dbg.addDebuggee(designateAs), direct);
+  assertDebuggees(direct);
+  assertEq(dbg.hasDebuggee(designateAs), true);
+  assertEq(dbg.removeDebuggee(designateAs), undefined);
+  assertDebuggees();
+}
+
+combo(g1, g1, dg1);
+combo(dg1, g1, dg1);
+combo(dwg1, g1, dg1);
+
+combo(g1, dg1, dg1);
+combo(dg1, dg1, dg1);
+combo(dwg1, dg1, dg1);
+
+combo(g1, dwg1, dg1);
+combo(dg1, dwg1, dg1);
+combo(dwg1, dwg1, dg1);

js/src/jit-test/tests/debug/Debugger-findScripts-09.js

@@ -2,10 +2,11 @@
 load(libdir + 'asserts.js');
 
 var dbg = new Debugger();
+var g = newGlobal();
 assertEq(dbg.findScripts().length, 0);
 assertEq(dbg.findScripts({}).length, 0);
 
-assertEq(dbg.findScripts({global:{}}).length, 0);
+assertEq(dbg.findScripts({global:g}).length, 0);
 assertThrowsInstanceOf(function () { dbg.findScripts({global:null}); }, TypeError);
 assertThrowsInstanceOf(function () { dbg.findScripts({global:true}); }, TypeError);
 assertThrowsInstanceOf(function () { dbg.findScripts({global:4}); }, TypeError);

js/src/vm/Debugger.cpp

@@ -1662,29 +1662,43 @@ Debugger::setUncaughtExceptionHook(JSContext *cx, unsigned argc, Value *vp)
     return true;
 }
 
-JSObject *
+GlobalObject *
 Debugger::unwrapDebuggeeArgument(JSContext *cx, const Value &v)
 {
-    /*
-     * The argument to {add,remove,has}Debuggee may be
-     *   - a Debugger.Object belonging to this Debugger: return its referent
-     *   - a cross-compartment wrapper: return the wrapped object
-     *   - any other non-Debugger.Object object: return it
-     * If it is a primitive, or a Debugger.Object that belongs to some other
-     * Debugger, throw a TypeError.
-     */
-    JSObject *obj = NonNullObject(cx, v);
-    if (obj) {
-        if (obj->getClass() == &DebuggerObject_class) {
-            Value rv = v;
-            if (!unwrapDebuggeeValue(cx, &rv))
-                return NULL;
-            return &rv.toObject();
-        }
-        if (IsCrossCompartmentWrapper(obj))
-            return &GetProxyPrivate(obj).toObject();
+    if (!v.isObject()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_UNEXPECTED_TYPE,
+                             "argument", "not a global object");
+        return NULL;
     }
-    return obj;
+
+    RootedObject obj(cx, &v.toObject());
+
+    /* If it's a Debugger.Object belonging to this debugger, dereference that. */
+    if (obj->getClass() == &DebuggerObject_class) {
+        Value rv = v;
+        if (!unwrapDebuggeeValue(cx, &rv))
+            return NULL;
+        obj = &rv.toObject();
+    }
+
+    /* If we have a cross-compartment wrapper, dereference as far as is secure. */
+    obj = UnwrapObjectChecked(cx, obj);
+    if (!obj)
+        return NULL;
+
+    /* If that produced an outer window, innerize it. */
+    obj = GetInnerObject(cx, obj);
+    if (!obj)
+        return NULL;
+
+    /* If that didn't produce a global object, it's an error. */
+    if (!obj->isGlobal()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_UNEXPECTED_TYPE,
+                             "argument", "not a global object");
+        return NULL;
+    }
+
+    return &obj->asGlobal();
 }
 
 JSBool
@@ -1692,14 +1706,14 @@ Debugger::addDebuggee(JSContext *cx, unsigned argc, Value *vp)
 {
     REQUIRE_ARGC("Debugger.addDebuggee", 1);
     THIS_DEBUGGER(cx, argc, vp, "addDebuggee", args, dbg);
-    RootedObject referent(cx, dbg->unwrapDebuggeeArgument(cx, args[0]));
-    if (!referent)
+    Rooted<GlobalObject*> global(cx, dbg->unwrapDebuggeeArgument(cx, args[0]));
+    if (!global)
         return false;
-    Rooted<GlobalObject*> global(cx, &referent->global());
+
     if (!dbg->addDebuggeeGlobal(cx, global))
         return false;
 
-    Value v = ObjectValue(*referent);
+    Value v = ObjectValue(*global);
     if (!dbg->wrapDebuggeeValue(cx, &v))
         return false;
     args.rval().set(v);
@@ -1711,10 +1725,9 @@ Debugger::removeDebuggee(JSContext *cx, unsigned argc, Value *vp)
 {
     REQUIRE_ARGC("Debugger.removeDebuggee", 1);
     THIS_DEBUGGER(cx, argc, vp, "removeDebuggee", args, dbg);
-    JSObject *referent = dbg->unwrapDebuggeeArgument(cx, args[0]);
-    if (!referent)
+    GlobalObject *global = dbg->unwrapDebuggeeArgument(cx, args[0]);
+    if (!global)
         return false;
-    GlobalObject *global = &referent->global();
     if (dbg->debuggees.has(global))
         dbg->removeDebuggeeGlobal(cx->runtime->defaultFreeOp(), global, NULL, NULL);
     args.rval().setUndefined();
@@ -1726,10 +1739,10 @@ Debugger::hasDebuggee(JSContext *cx, unsigned argc, Value *vp)
 {
     REQUIRE_ARGC("Debugger.hasDebuggee", 1);
     THIS_DEBUGGER(cx, argc, vp, "hasDebuggee", args, dbg);
-    JSObject *referent = dbg->unwrapDebuggeeArgument(cx, args[0]);
-    if (!referent)
+    GlobalObject *global = dbg->unwrapDebuggeeArgument(cx, args[0]);
+    if (!global)
         return false;
-    args.rval().setBoolean(!!dbg->debuggees.lookup(&referent->global()));
+    args.rval().setBoolean(!!dbg->debuggees.lookup(global));
     return true;
 }
 
@@ -2011,10 +2024,9 @@ class Debugger::ScriptQuery {
         if (global.isUndefined()) {
             matchAllDebuggeeGlobals();
         } else {
-            JSObject *referent = debugger->unwrapDebuggeeArgument(cx, global);
-            if (!referent)
+            GlobalObject *globalObject = debugger->unwrapDebuggeeArgument(cx, global);
+            if (!globalObject)
                 return false;
-            GlobalObject *globalObject = &referent->global();
 
             /*
              * If the given global isn't a debuggee, just leave the set of

js/src/vm/Debugger.h

@@ -139,7 +139,7 @@ class Debugger {
     JSTrapStatus parseResumptionValue(Maybe<AutoCompartment> &ac, bool ok, const Value &rv,
                                       Value *vp, bool callHook = true);
 
-    JSObject *unwrapDebuggeeArgument(JSContext *cx, const Value &v);
+    GlobalObject *unwrapDebuggeeArgument(JSContext *cx, const Value &v);
 
     static void traceObject(JSTracer *trc, RawObject obj);
     void trace(JSTracer *trc);