1391449 - Devtools encounter objects in invisibleToDebugger compartments too often

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Jim Blandy :jimb]

Ordinary JavaScript debugging should never encounter objects in invisibleToDebugger compartments, but it seems that it does.

In bug 1389787, Oriol observed that calling Debugger.Object.prototype.unwrap from ObjectActor.prototype.grip caused test failures because it throws an error. D.O.p.unwrap only throws an exception if the referent of the wrapper the D.O refers to is in an invisibleToDebugger compartment. Such objects should be very rare, and there's certainly no reason the ordinary devtools tests should encounter them. However, comments 14-16 in bug 1389787 suggest that they do.

We need to figure out where these objects are coming from, whether their reachability is normal and expected, and adapt the devtools accordingly.

Looking over the cases where invisibleToDebugger compartments are created, I suspect that these objects are from SimpleGlobalObject globals, used by WebIDL to parse JSON strings being converted to WebIDL dictionaries.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Sorry for the lag here.

So SimpleGlobalObject is used for three things:

1) Parsing JSON strings and convert to Web IDL dictionaries, as comment 0 says.  This should generally not lead to any objects escaping out of the Init() call unless the dictionary type involved contains "any" or "object" (or promises or typed arrays, in theory, but those can't result from parsing JSON).  And we don't implement the "init from JSON" operation on dictionaries that have "any" or "object" in them (see typeSafeToJSONify in Codegen.py), so there really should be no object escaping here.

2) Worker debugger globals.  I can't tell you what the escaping/nonescaping behavior here is, but I'm pretty sure those are invisible to debugger on purpose.

3) TelemetryTestFixture's mCleanGlobal.  This is test-only code, I assume.

The other sources of hidden-from-debugger compartments are:

* The singleton scopes set up by XPCJSRuntime::InitSingletonScopes.  I can totally believe people do stuff with those that might escape out into the world.  If we have an rr session with one of these unexpected hidden-from-debugger objects, we should be able to easily check whether its global is one of these three scopes.

* Sandboxes created with the "invisibleToDebugger" option.  All the uses seem to be in devtools code.

My money is on the singleton scopes and someone misusing them somehow.  I someone can give me sane steps to reproduce the failure locally, I'm happy to spend a bit of quality time in rr to answer the question for sure.

Comment 2

[Oriol Brufau [:Oriol]]

The failure in browser_console.js was due to a CPOW.

1. Enable e10s.
2. Load http://example.com
3. Open browser console
4. Run this code:
    Components.utils.import('resource://gre/modules/jsdebugger.jsm');
    addDebuggerToGlobal(this);
    var global = Cu.Sandbox(Cu.getObjectPrincipal({}));
    var dbg = new Debugger().addDebuggee(global);
    var obj = dbg.makeDebuggeeValue(gBrowser.selectedBrowser._contentWindow);
    obj.unwrap();

Attempting to unwrap throws:
    TypeError: object in compartment marked as invisible to Debugger

Comment 3

[Boris Zbarsky [:bzbarsky]]

Thanks.  With those steps, here's what I see at the point when we report the JSMSG_DEBUG_INVISIBLE_COMPARTMENT exception:

  (rr) p &unwrapped.ptr->global()
  $4 = (js::GlobalObject *) 0x7f0851791100
  (rr) p $4->getClass()->name
  $6 = 0x7f086a128166 "Sandbox"

That sandbox was created inside XPCJSRuntime::InitSingletonScopes, like so:

  rv = CreateSandboxObject(cx, &v, nsXPConnect::SystemPrincipal(), privilegedJunkScopeOptions);

so I'm winning my bet from comment 1 so far.  ;)

unwrapped.ptr in this case is a CPOW, of course.  As the comment at http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/js/ipc/WrapperOwner.cpp#1191 says:

        // All CPOWs live in the privileged junk scope.

OK, so maybe we should have a separate junk scope, one that's visible to the debugger, for CPOWs?

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

> OK, so maybe we should have a separate junk scope, one that's visible to the debugger, for CPOWs?

I guess so, although I don't foresee anything good happening if we do allow the debugger to peer through CPOWs. As long as the object is identified as a CPOW by the debugger, I'd rather keep it opaque. However, if this is breaking some important debugger invariant, then I guess we should fix it.

Comment 5

[Oriol Brufau [:Oriol]]

In bug 1394559 I'm considering all objects that can't be unwrapped by the debugger to be unsafe, because they usually throw when accessing properties. Since CPOWs shouldn't be accessed either, it's not a big problem if they can't be unwrapped. In fact it might be easier this way, less checks.

Comment 6

[Oriol Brufau [:Oriol]]

CPOWs aren't invisible to debugger since bug 1442737, so I guess this can be closed.
In fact this breaks some assumptions of the devtools, the CPOW is accessed, and this crashes the tab (bug 1544175).

Comment 7

[Jim Blandy :jimb]

Closing.