Closed Bug 857238 Opened 7 years ago Closed 5 years ago
compartment mismatch with location in ns
Window SH::New Resolve with Xrays
I saw a couple of these on Aurora DefinePropertyById LocationSetter<nsIDOMWindow> JSCompartment::wrap nsWindowSH::NewResolve xpc::XPCWrappedNativeXrayTraits::resolveOwnProperty https://crash-stats.mozilla.com/report/index/d726d7fe-ae3f-45a2-abf5-0d37d2130326 https://crash-stats.mozilla.com/report/index/b37cf30c-57ae-41ff-a1aa-f721a2130326
Hm, so I think part of that stack is garbage, but my guess is that the call to JS_DefinePropertyById in the (sLocation_id == id) branch of nsWindowSH::NewResolve is busted in the Xray case: http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMClassInfo.cpp#5305 What happens in this function _looks_ highly suspicious when accessed over Xrays, but I can't actually find anything wrong with it. |cx| and |obj| should be same-compartment, |id| should be an interned atom, and we appear to be wrapping |v| into the cx compartment right before the DefineProperty call. bz, see anything I don't in that code block?
The obvious thing I think I see is that if WrapNative depends on cx and scope being same-compartment then there's no reason they would be in this case, right? But apart from that, this code looks sane.... I wish the same-compartment assert were broken up into several separate asserts for the obj, value, id, etc... then we'd know which one is failing. :(
(In reply to Boris Zbarsky (:bz) from comment #2) > The obvious thing I think I see is that if WrapNative depends on cx and > scope being same-compartment then there's no reason they would be in this > case, right? Yeah, my thoughts exactly. It looks like the WrapNative immediately invokes nsXPConnect::WrapNativeToJSVal, which invokes the static NativeInterface2JSObject doppelganger in nsXPConnect.cpp, which, as its first action, enters the compartment of aScope on the cx: http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/nsXPConnect.cpp#1144 > I wish the same-compartment assert were broken up into several separate > asserts for the obj, value, id, etc... then we'd know which one is failing. > :( Me too!
Well, we can change that and see what crash-stats has to say for itself... Or I guess we could try to reproduce, right?
who can own this between you three? :)
The obvious zapping location with Xrays didn't cause any badness, so this is probably just some weird addon. Given that none of the three of us can identify any problem here by inspection, I'm going to downgrade this to sec-other. If it shows up some more, I'll consider landing some diagnostics, like maybe just splitting up the same compartment assertion.
This has changed enough in the last few years that I think it isn't worth keeping this open.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → INCOMPLETE
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.