707613 - Crash in jsdScript::GetParameterNames at js::AutoCompartment::enter or JS_FunctionHasLocalNames or js::Bindings::getLocalNameArray

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Scoobidiver (away)]

It's a new crash signature that first appeared in 11.0a1/20110412.
It's currently #1 top crasher in this build.
The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9740118b9dcc&tochange=c2102c45c8da

There are two kinds of stack trace depending on the CPU:
* amd64:
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	js::AutoCompartment::enter 	js/src/jswrapper.cpp:453
1 	xul.dll 	DEBUG_CheckWrapperThreadSafety 	js/xpconnect/src/XPCWrappedNative.cpp:3588
2 	xul.dll 	JSAutoEnterCompartment::enter 	js/src/jsapi.cpp:1361
3 	xul.dll 	js::EmptyShape::getInitialShape 	js/src/jsscope.cpp:1384
4 	xul.dll 	jsdScript::GetParameterNames 	js/jsd/jsd_xpc.cpp:1292
5 	xul.dll 	xul.dll@0x3ebcaf 	
6 	xul.dll 	JSObject::getChildProperty 	js/src/jsscope.cpp:424
7 	xul.dll 	XPTC__InvokebyIndex 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke_asm_x86_64.asm:129
8 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1554

* x86 + Mac OS X:
Frame 	Module 	Signature [Expand] 	Source
0 	mozjs.dll 	js::AutoCompartment::enter 	js/src/jswrapper.cpp:453
1 	mozjs.dll 	JSAutoEnterCompartment::enter 	js/src/jsapi.cpp:1361
2 	xul.dll 	jsdScript::GetParameterNames 	js/jsd/jsd_xpc.cpp:1292
3 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
4 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1554
5 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:626
6 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:3461
7 	mozjs.dll 	js::RunScript 	js/src/jsinterp.cpp:581
8 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:644
9 	mozjs.dll 	js::Invoke 	js/src/jsinterp.h:148
...

More crash reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3AAutoCompartment%3A%3Aenter%28%29
https://crash-stats.mozilla.com/report/list?signature=js%3A%3AAutoCompartment%3A%3Aenter

Comment 1

[Scoobidiver (away)]

I add a related crash signature:
Frame 	Module 	Signature [Expand] 	Source
0 	mozjs.dll 	JS_FunctionHasLocalNames 	js/src/jsdbgapi.cpp:419
1 	xul.dll 	jsdScript::GetParameterNames 	js/jsd/jsd_xpc.cpp:1297
2 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
3 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1554
4 	mozjs.dll 	js::mjit::stubs::UncachedCallHelper 	js/src/methodjit/InvokeHelpers.cpp:479
5 	mozjs.dll 	js::mjit::stubs::UncachedCall 	js/src/methodjit/InvokeHelpers.cpp:430
...

https://crash-stats.mozilla.com/report/list?signature=JS_FunctionHasLocalNames

Comment 2

[Scoobidiver (away)]

There's another crash signature:
Frame 	Module 	Signature [Expand] 	Source
0 	mozjs.dll 	js::Bindings::getLocalNameArray 	js/src/jsscript.cpp:204
1 	mozjs.dll 	JS_GetFunctionLocalNameArray 	js/src/jsdbgapi.cpp:426
2 	xul.dll 	jsdScript::GetParameterNames 	js/jsd/jsd_xpc.cpp:1309
3 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
4 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1554
5 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:626
6 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:3461
7 	mozjs.dll 	js::RunScript 	js/src/jsinterp.cpp:581
...

https://crash-stats.mozilla.com/report/list?version=Firefox:11.0a1&signature=js%3A%3ABindings%3A%3AgetLocalNameArray%28JSContext*%2C%20js%3A%3AVector%3CJSAtom*%2C%20int%2C%20js%3A%3ATempAllocPolicy%3E*%29

Comment 3

[Robert Kaiser]

js::AutoCompartment::enter() has done a jump for 0 over a week to 41 crashes on yesterday's 11.0a1 trunk data, mostly with the 2011-11-04 Nightly build.

Comment 4

[Brian Hackett [Laid off!]]

Attachment: suspected fix

All these crashes are under jsdScript methods, with what looks like a corrupt JSFunction.  JSD is storing a script/function pair in a hashtable, with entries that get cleared when the script is destroyed.  Entries in the table can get filled in from stack frames lazily, using the frame's script and its JS_GetFrameFunction.  With the objshrink changes, JS_GetFrameFunction can now return a non-canonical (cloned) function for the script, which may be collected before the script itself is.  I think this is happening and causing these crashes and also bug 707614.

Comment 5

[Brian Hackett [Laid off!]]

Oh, and the fix restores the earlier behavior of JS_GetFrameFunction, returning the script's canonical function.

Comment 6

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bfb517a374fc

Comment 7

[Brian Hackett [Laid off!]]

r=luke via IRC

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/bfb517a374fc

Comment 9

[Scoobidiver (away)]

(In reply to Ed Morley [:edmorley] from comment #8)
> https://hg.mozilla.org/mozilla-central/rev/bfb517a374fc
I don't see it in m-c:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=1+week+ago&enddate=now

Comment 10

[Brian Hackett [Laid off!]]

(In reply to Scoobidiver from comment #9)
> (In reply to Ed Morley [:edmorley] from comment #8)
> > https://hg.mozilla.org/mozilla-central/rev/bfb517a374fc
> I don't see it in m-c:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?startdate=1+week+ago&enddate=now

It is under bf8259fcab61, one of the merges with hidden changesets.  The first nightly this went into was todays.  There are still crashes with the same signatures, but the volume seems to have gone down a lot.  Is there any way that builds against older revisions could show up in the statistics for today's nightly?  I've been through all the JSD code manipulating script functions, and with this patch things look correct now.

Comment 11

[Scoobidiver (away)]

(In reply to Brian Hackett (:bhackett) from comment #10)
> There are still crashes with the same signatures, but the volume seems to have
> gone down a lot.
I don't think so as you must wait the next build to have final stats for the current build.

> Is there any way that builds against older revisions could show up in the 
> statistics for today's nightly?
No. 11.0a1/20111207031110 build change set is http://hg.mozilla.org/mozilla-central/rev/489f2d51b011 and contains https://hg.mozilla.org/mozilla-central/rev/bfb517a374fc.

Comment 12

[Brian Hackett [Laid off!]]

Attachment: suspected fix (round 2)

Rip out all JSD code which tries to correlate scripts with functions, and just fetch the script's function from the API as required and avoid any possibility of an incorrect function being used.

Comment 13

[Robert Kaiser]

(In reply to Brian Hackett (:bhackett) from comment #10)
> Is there any
> way that builds against older revisions could show up in the statistics for
> today's nightly? 

If you look at the "Table" tab and still see this happening for a build ID after you checked in, then the crash is still happening in builds from that day, which is what I'm seeing for the js::AutoCompartment::enter ones at least.

Comment 14

[Brian Hackett [Laid off!]]

(In reply to Brian Hackett (:bhackett) from comment #12)
> Created attachment 579740 [details] [diff] [review] [diff] [details] [review]
> suspected fix (round 2)
> 
> Rip out all JSD code which tries to correlate scripts with functions, and
> just fetch the script's function from the API as required and avoid any
> possibility of an incorrect function being used.

OK, I think the remaining crashes are due to SetMethodFunction in the emitter.  This changes the function associated with a compiled script, but at this point the new script hook has already been called and the debugger will continue to use the old function, which has correct information but is not rooted by the script, and vulnerable to being collected.

This is still the right fix to the JSD glue code.

Comment 15

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/331e25310bf7

Comment 16

[Scoobidiver (away)]

(In reply to Brian Hackett (:bhackett) from comment #15)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/331e25310bf7
I see it in m-c: http://hg.mozilla.org/mozilla-central/rev/331e25310bf7
In addition, there have been no crashes since 11.1a1/20111209.