606665 - crash [@ js::Compiler::defineGlobals(JSContext*, js::GlobalScope&, JSScript*) ][@ js::Compiler::defineGlobals ][@ js::Bindings::lastShape()]

Status: RESOLVED DUPLICATE of bug 660638

(Core :: JavaScript Engine, defect)

Description

[Scoobidiver (away)]

It is a residual crash signature that exists in trunk builds for at least one month.
It is #201 top crasher in 4.0b8pre for the last week.

Signature	js::Compiler::defineGlobals(JSContext*, js::GlobalScope&, JSScript*)
UUID	d60fbdb0-dd65-416f-89bb-a31f82101022
Time 	2010-10-22 21:43:05.550123
Uptime	122
Last Crash	85230 seconds (23.7 hours) before submission
Install Age	6076 seconds (1.7 hours) since version was first installed.
Product	Firefox
Version	4.0b8pre
Build ID	20101022043027
Branch	2.0
OS	Windows NT
OS Version	6.1.7600
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 13
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x4
App Notes 	AdapterVendorID: 10de, AdapterDeviceID: 01d3

Frame 	Module 	Signature [Expand] 	Source
0 	mozjs.dll 	js::Compiler::defineGlobals 	js/src/jsparse.cpp:1079
1 	mozjs.dll 	js::Compiler::compileScript 	js/src/jsparse.cpp:995
2 	mozjs.dll 	JS_EvaluateUCScriptForPrincipals 	js/src/jsapi.cpp:4876
3 	mozjs.dll 	JS_EvaluateUCScriptForPrincipalsVersion 	js/src/jsapi.cpp:4857
4 	xul.dll 	nsJSContext::EvaluateString 	dom/base/nsJSEnvironment.cpp:1724

More reports at:
http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=js%3A%3ACompiler%3A%3AdefineGlobals%28JSContext*%2C%20js%3A%3AGlobalScope%26%2C%20JSScript*%29

Comment 1

[Bob Clary [:bc] (inactive)]

1. http://www.takdin.co.il/search/index.aspx?formName=psika#1
2. crash bp-6eb1264d-5061-494c-bae2-ce1072110210

0 	XUL 	js::Compiler::defineGlobals 	js/src/jsparse.cpp:1216
1 	XUL 	js::Compiler::compileScript 	js/src/jsparse.cpp:1138
2 	XUL 	EvaluateUCScriptForPrincipalsCommon 	js/src/jsapi.cpp:4928
3 	XUL 	JS_EvaluateUCScriptForPrincipalsVersion 	js/src/jsapi.cpp:4950
4 	XUL 	nsJSContext::EvaluateString 	dom/base/nsJSEnvironment.cpp:1554
5 	XUL 	nsScriptLoader::EvaluateScript 	content/base/src/nsScriptLoader.cpp:906

crashes opt/debug mac/linux at least.

Comment 2

[Bob Clary [:bc] (inactive)]

Attachment: testcase

test with array constructor and 65535 arguments. Add one more and it throws constructor has too many arguments.

Comment 3

[Marcia Knous [:marcia]]

Adding the other sig so it gets picked up in crash stats.

Comment 4

[carmi.grushko]

Problem seems to manifest itself on the testcase when the number of arguments is greater than 65529. Problematic line seems to be js/src/jsscript.cpp:1434  -

if (script->nfixed + cg->maxStackDepth >= JS_BIT(16)) {
        ReportCompileErrorNumber(....);
        goto bad;
    }

maxStackDepth seems to be (6+#arguments), and so even if the number of arguments is smaller than 64k, SpiderMonkey crashes.

By the way, does anybody know why the stack depth is limited to 64k? performance issues? also, what should be the solution? breaking-up the parsing tree ?

Comment 5

[Bob Clary [:bc] (inactive)]

I have only seen this on http://www.takdin.co.il/ in automation but it practically excludes Firefox from the site. If it is a simple fix, can we think about fixing this soon?

Comment 6

[Sheila Mooney]

We minused this because it's really not a candidate for this group to track. It's really up to the JS team to look at this and prioritize it and submit for approval when a patch is ready. I added dmandelin to the bug.

Comment 7

[Bob Clary [:bc] (inactive)]

nightlies have begun to show a slightly different signature for this crash: 

js::Bindings::lastShape()

js::Compiler::compileScript(JSContext*, JSObject*, js::StackFrame*, JSPrincipals*, unsigned int, unsigned short const*, unsigned int, char const*, unsigned int, JSVersion, JSString*, unsigned int)

EvaluateUCScriptForPrincipalsCommon(JSContext*, JSObject*, JSPrincipals*, unsigned short const*, unsigned int, char const*, unsigned int, jsval_layout*, JSVersion)

JS_EvaluateUCScriptForPrincipalsVersion nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*)

Comment 8

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

Attachment: patch

This fixes the crash. NewScriptFromCG was returning NULL after reporting a script too big error. The problem is that compileScript didn't correctly check the return value for NULL (which is sort of weird--I hope I'm not missing something).

This could also have been causing us to crash on OOM.

Comment 9

[Paul Biggar]

Bill's patch looks like what I pushed yesterday for bug 660670. Dup?

Comment 10

[Paul Biggar]

(In reply to comment #9)
> Bill's patch looks like what I pushed yesterday for bug 660670. Dup?

Um, bug 660638.