865260 - crash in js::IsStandardClassResolved

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[Scoobidiver (away)]

It first showed up in 23.0a1/20130424 and is currently #1 top crasher in that build despite other recent top crashers. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=acf388eaf9e9&tochange=fef5f202b2dc
It's likely a regression from bug 860494.

Signature 	js::IsStandardClassResolved(JSObject*, js::Class*) More Reports Search
UUID	e7174af5-4b77-4b09-abaf-16dfb2130424
Date Processed	2013-04-24 14:25:56
Uptime	214
Last Crash	3.7 minutes before submission
Install Age	20.4 minutes since version was first installed.
Install Time	2013-04-24 14:05:27
Product	Firefox
Version	23.0a1
Build ID	20130424030917
Release Channel	nightly
OS	Mac OS X
OS Version	10.8.3 12D78
Build Architecture	amd64
Build Architecture Info	family 6 model 42 stepping 7
Crash Reason	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address	0x20
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x6741GL Layers? GL Context? GL Context+ GL Layers+ 
Processor Notes 	sp-processor01.phx1.mozilla.com_3159:2012; exploitability tool failed: 127
EMCheckCompatibility	True
Adapter Vendor ID	0x1002
Adapter Device ID	0x6741

Frame 	Module 	Signature 	Source
0 	XUL 	js::IsStandardClassResolved 	obj-firefox/x86_64/dist/include/js/Value.h:1040
1 	XUL 	JS_ResolveStandardClass 	js/src/jsapi.cpp:2043
2 	XUL 	nsCxPusher::Push 	content/base/src/nsContentUtils.cpp:3142
3 	XUL 	nsWindowSH::NewResolve 	dom/base/nsDOMClassInfo.cpp:4973
4 	XUL 	js::ObjectImpl::nativeLookup 	js/src/vm/ObjectImpl.cpp:302
5 	XUL 	int js::baseops::LookupProperty< 	js/src/jsobj.cpp:3507
6 	XUL 	xpc::XPCWrappedNativeXrayTraits::resolveOwnProperty 	js/xpconnect/wrappers/XrayWrapper.cpp:1033
7 	XUL 	xpc::XrayUtils::HasNativeProperty 	js/xpconnect/wrappers/XrayWrapper.cpp:1370
8 	XUL 	nsPlainTextSerializer::DoOpenContainer::bulletCharArray 	
9 	XUL 	xpc::AccessCheck::isCrossOriginAccessPermitted 	js/xpconnect/wrappers/AccessCheck.cpp:243
10 	XUL 	xpc::GetXrayType 	obj-firefox/x86_64/dist/include/mozilla/dom/BindingUtils.h:1653
11 	XUL 	xpc::FilteringWrapper<xpc::XrayWrapper<js::SecurityWrapper<js::CrossCompartmentW 	js/xpconnect/wrappers/AccessCheck.h:103
12 	XUL 	js::Proxy::get 	js/src/jsproxy.h:365
13 	XUL 	proxy_GetGeneric 	js/src/jsproxy.cpp:2813
14 	XUL 	js::GetPropertyOperation 	js/src/jsobjinlines.h:161
15 	XUL 	js::Interpret 	js/src/jsinterp.cpp:2246
16 	libsystem_c.dylib 	libsystem_c.dylib@0x2d1b3 	
17 	XUL 	_ZZL6EncodeP9JSContextN2JS6HandleIP14JSLinearStringEEPKtS7_NS1_13MutableHandleIN 	
18 	XUL 	JSObject::updateSlotsForSpan 	js/src/jsobj.cpp:2321
19 	XUL 	CMMFCertOrEncCertTemplate 	
20 	XUL 	_ZZL6EncodeP9JSContextN2JS6HandleIP14JSLinearStringEEPKtS7_NS1_13MutableHandleIN 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3AIsStandardClassResolved%28JSObject*%2C+js%3A%3AClass*%29
https://crash-stats.mozilla.com/report/list?signature=JS_ResolveStandardClass%28JSContext*%2C+JSObject*%2C+int%2C+int*%29

Comment 1

[Boris Zbarsky [:bzbarsky]]

So I have steps to reproduce: load http://www.jango.com/stations/263448187/tunein?gcid=1&l=0

In a debug build, I get:

Assertion failure: obj->isGlobal(), at ../../../mozilla/js/src/jsapi.cpp:2039
(gdb) frame 0
#0  JS_ResolveStandardClass (cx=0x127bc8e00, objArg=0x12c0805c0, id={asBits = 4523703488}, resolved=0x7fff5fbf34c4) at jsapi.cpp:2039
2039            JS_ASSERT(obj->isGlobal());

In this case "obj" is an Xray proxy.  The stack is:

#0  JS_ResolveStandardClass (cx=0x127bc8e00, objArg=0x12c0805c0, id={asBits = 4523703488}, resolved=0x7fff5fbf34c4) at jsapi.cpp:2039
#1  0x0000000103ede332 in nsWindowSH::NewResolve (this=0x110a8f460, wrapper=0x110a626d0, cx=0x10fc62900, obj_=0x12c0805c0, id_={asBits = 4523703488}, flags=0, objp=0x7fff5fbf36f0, _retval=0x7fff5fbf36ff) at nsDOMClassInfo.cpp:4973
#2  0x0000000103ee08ea in non-virtual thunk to nsWindowSH::NewResolve(nsIXPConnectWrappedNative*, JSContext*, JSObject*, jsid, unsigned int, JSObject**, bool*) () at nsDOMClassInfo.cpp:1188
#3  0x000000010472ea4e in xpc::XPCWrappedNativeXrayTraits::resolveOwnProperty (this=0x107b067d8, cx=0x10fc62900, jsWrapper=@0x107bd87d0, wrapper={<js::HandleBase<JSObject *>> = {<No data fields>}, ptr = 0x7fff5fbf3a78}, holder={<js::HandleBase<JSObject *>> = {<No data fields>}, ptr = 0x7fff5fbf3948}, id={<js::HandleBase<jsid>> = {<No data fields>}, ptr = 0x7fff5fbf3a98}, desc=0x7fff5fbf3908, flags=0) at XrayWrapper.cpp:1033
#4  0x000000010473466c in xpc::XrayUtils::HasNativeProperty (cx=0x10fc62900, wrapper={<js::HandleBase<JSObject *>> = {<No data fields>}, ptr = 0x7fff5fbf3a78}, id={<js::HandleBase<jsid>> = {<No data fields>}, ptr = 0x7fff5fbf3a98}, hasProp=0x7fff5fbf3a17) at XrayWrapper.cpp:1370

So pretty definitely fallout from bug 860494.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Why are we reaching JS_ResolveStandardClass?  It's inside a !ObjectIsNativeWrapper(cx, obj) block!

Looks like xpc::WrapperFactory::IsXrayWrapper(obj) is true, as expected.  So presumably xpc::AccessCheck::wrapperSubsumes(obj) is false?

Looks like this is an Xray with the two principals involved both being codebase principals for different jango pages which are same-origin, but one has document.domain set and the other does not.

Given that, ObjectIsNativeWrapper is patently the wrong check to be doing if we want to exclude Xrays here, no?

Comment 3

[Bobby Holley (:bholley)]

Right, so previously the Xray NewResolve hook would only be called for XOWs with the whitelisted subset of ids that made it through the outer FilteringWrapper. But now that we check IsNativeProperty, we can hit any of them in the XOW case. I'll audit the hook and whip up a patch.

Comment 4

[Bobby Holley (:bholley)]

Attachment: Use IsXrayWrapper rather than ObjectIsNativeWrapper in nsWindowSH. v1

There are some other uses of ObjectIsNativeWrapper in other scriptable helpers
that are tempting to remove as well, but it's probably just better to wait for
that stuff to just go away. Given that the issue we're running into here is
Window-specific, there's not a pressing need to fix the other stuff.

Comment 5

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=2b3c10b192fe

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 741479 [details] [diff] [review]
Use IsXrayWrapper rather than ObjectIsNativeWrapper in nsWindowSH. v1

Hmm.  So this used to only push if we were called on some other JSContext, but now we'll always push?  Is that going to be OK perf-wise?

r=me modulo that

Comment 7

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky (:bz) from comment #6)
> Comment on attachment 741479 [details] [diff] [review]
> Use IsXrayWrapper rather than ObjectIsNativeWrapper in nsWindowSH. v1
> 
> Hmm.  So this used to only push if we were called on some other JSContext,
> but now we'll always push?  Is that going to be OK perf-wise?

AutoPushJSContext (distinct from nsCxPusher, AutoJSContext, and SafeAutoJSContext) only pushes if cx doesn't match stack-top.

Comment 8

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7badc495dbba

Comment 9

[Boris Zbarsky [:bzbarsky]]

Right, but it still has to go get the stack and so forth...

I guess I'll just hope it's not a problem in practice and that we'll kill off this stack thing soon.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

In case it's useful, clicking this link leads me to a crash with this signature http://t.co/IJKFYJT4Zg (don't judge me)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7badc495dbba

Comment 13

[Scoobidiver (away)]

Now that the fix of bug 860494 was uplifted in 22.0a2/20130430, it's #4 top crasher in 22.0a2 and will be #1 in a few hours.

Comment 14

[Alex Keybl [:akeybl]]

Comment on attachment 741479 [details] [diff] [review]
Use IsXrayWrapper rather than ObjectIsNativeWrapper in nsWindowSH. v1

pre-emptively approving for Aurora

Comment 15

[Bobby Holley (:bholley)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e83d9239be4a

Comment 16

[Ioana (away)]

I can't reproduce the issue with the steps in comment 1 on Firefox builds with the bug, so I couldn't verify the fix manually either.

I did check the stats in Socorro for the three signatures though, and there have been no crashes on Firefox 23 and 24 in the last 4 weeks.