Last Comment Bug 707613 - Crash in jsdScript::GetParameterNames at js::AutoCompartment::enter or JS_FunctionHasLocalNames or js::Bindings::getLocalNameArray
: Crash in jsdScript::GetParameterNames at js::AutoCompartment::enter or JS_Fun...
Status: RESOLVED FIXED
: crash, regression, topcrash
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- critical with 1 vote (vote)
: mozilla11
Assigned To: Brian Hackett (:bhackett)
:
Mentors:
: 707614 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-05 00:27 PST by Scoobidiver (away)
Modified: 2012-02-01 13:57 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
suspected fix (2.17 KB, patch)
2011-12-05 08:30 PST, Brian Hackett (:bhackett)
bhackett1024: review+
Details | Diff | Review
suspected fix (round 2) (5.62 KB, patch)
2011-12-07 10:32 PST, Brian Hackett (:bhackett)
luke: review+
Details | Diff | Review

Description Scoobidiver (away) 2011-12-05 00:27:21 PST
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) 2011-12-05 00:37:41 PST
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) 2011-12-05 00:47:03 PST
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 (not working on stability any more) 2011-12-05 08:20:34 PST
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 (:bhackett) 2011-12-05 08:30:13 PST
Created attachment 579074 [details] [diff] [review]
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 (:bhackett) 2011-12-05 08:34:09 PST
Oh, and the fix restores the earlier behavior of JS_GetFrameFunction, returning the script's canonical function.
Comment 6 Brian Hackett (:bhackett) 2011-12-05 17:49:09 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/bfb517a374fc
Comment 7 Brian Hackett (:bhackett) 2011-12-05 17:49:40 PST
r=luke via IRC
Comment 8 Ed Morley [:emorley] 2011-12-06 10:06:03 PST
https://hg.mozilla.org/mozilla-central/rev/bfb517a374fc
Comment 9 Scoobidiver (away) 2011-12-07 07:22:40 PST
(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 (:bhackett) 2011-12-07 08:58:32 PST
(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) 2011-12-07 09:12:42 PST
(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 (:bhackett) 2011-12-07 10:32:54 PST
Created attachment 579740 [details] [diff] [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.
Comment 13 Robert Kaiser (not working on stability any more) 2011-12-07 11:18:59 PST
(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 (:bhackett) 2011-12-07 12:54:30 PST
(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 (:bhackett) 2011-12-07 13:42:01 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/331e25310bf7
Comment 16 Scoobidiver (away) 2011-12-12 03:32:51 PST
(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.
Comment 17 Scoobidiver (away) 2011-12-12 03:34:30 PST
*** Bug 707614 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.