532341 - debug everythingelse boxes are perma-orange with crash in mochitest-browser-chrome, after 12/1 tracemonkey merge

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

Starting with the first cycle to include today's tracemonkey merge (changeset e2860a4dcf0c), the windows & linux debug everythingelse boxes are perma-orange,  with a crash in mochitest-browser-chrome.

(Note that whatever's broken here would probably affect mac debug everythingelse, too -- we just don't have any mac debug boxes on the Firefox-Unittest page)

Windows logs:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259701177.1259710150.18460.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259703572.1259711629.3244.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259712959.1259722108.24838.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259718452.1259725212.26248.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259727553.1259734225.27047.gz

Linux logs:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259693904.1259701835.19878.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259699463.1259706295.6943.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259701152.1259711786.5277.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259709371.1259718766.20261.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259711367.1259721155.14226.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259715818.1259723869.11872.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259716936.1259724448.17969.gz

Comment 1

[Daniel Holbert [:dholbert]]

Looks like this might sporadically affect the optimized everythingelse column, too -- here's a recent sporadic crash in mochitest-browser-chrome from that column:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1259714134.1259719363.26866.gz
WINNT 5.2 mozilla-central test everythingelse on 2009/12/01 16:35:34

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: snippet of failure log

Attaching the relevant chunk from a tinderbox log.

Source:
http://tinderbox.mozilla.org/showlog.cgi?tree=Firefox-Unittest&errorparser=unittest&logfile=1259701177.1259710150.18460.gz&buildtime=1259701177&buildname=WINNT%205.2%20mozilla-central%20debug%20test%20everythingelse&fulltext=1
WINNT 5.2 mozilla-central debug test everythingelse on 2009/12/01 12:59:37
"s: moz2-win32-slave28"

Comment 3

[Arisu]

Attachment: Test case to reproduce crash in CheckStrictFormals()

I was testing the latest hourlies and several sites got my attention by crashing the browser.
Analyzing the minidumps revealed the location mozjs!CheckStrictFormals and I started to reduce
the javascript to a minimal test case for reproducing this crash. What I learned is, that:

Declaring any function with 5 arguments and 7 local variables in it while having
javascript.options.strict set to True will cause every build after changeset e2860a4dcf0c to crash
in /js/src/jsparse.cpp!CheckStrictFormals() because in line 1273 tc->decls.lookup(atom) returns
NULL and ALE_DEFN() defers it.

If you check the log you've linked to, you'll see that the last file it mentions before crashing is
/testing/mochitest/tests/SimpleTest/EventUtils.js, which contains the function synthesizeMouse()
starting at line 199. It has 5 arguments and 7 local variables.

My stack trace:
   mozjs!CheckStrictFormals                 [/js/src/jsparse.cpp:1273]
   mozjs!Statement                          [/js/src/jsparse.cpp:4574]
   mozjs!JSCompiler::compileScript          [/js/src/jsparse.cpp:910]
   mozjs!JS_EvaluateUCScriptForPrincipals   [/js/src/jsapi.cpp:5058]
     xul!nsJSContext::EvaluateString        [/dom/base/nsjsenvironment.cpp:1713]
     xul!nsScriptLoader::EvaluateScript     [/content/base/src/nsscriptloader.cpp:716]

matches the one in the failure log you've attached. I've checked jsparse.cpp in the TM tree to find
the changeset which introduced CheckStrictFormals and found Bug 514580 linked with it.
If you read comment 11 there, it seems that this problem is already known and being investigated.

Comment 4

[timeless]

arisu: great work. fwiw if you write bug 514580 comment 11 as such, it'll be properly linked :)

Comment 5

[Arisu]

(In reply to comment #4)
Thanks for your tip, that's useful to know!
And sorry for the screwed up wrapping, I forgot to set that to 80...

Comment 6

[Brendan Eich [:brendan]]

I hit this bug using tracemonkey builds and patched it -- the fix was in bug 532041. Looks like someone pushed it to m-c for me already.

/be

Comment 7

[Robert Sayre]

(In reply to comment #6)
> I hit this bug using tracemonkey builds and patched it -- the fix was in bug
> 532041. Looks like someone pushed it to m-c for me already.

Yeah, I did. Was just waiting for results to come back, since I didn't know whether it would fix the problem.

Jim, I'm going to assign this to you so the test case attached here can get added to the js suite.

Comment 8

[Brendan Eich [:brendan]]

Beltzner: none of the ES5 strict mode work is 1.9.2-bound so I don't think this bug needs to be blocking1.9.2+.

/be

Comment 9

[Jim Blandy :jimb]

Attachment: Test that duplicate argument checks use JSFunction::u.i.names correctly.

Comment 10

[Jim Blandy :jimb]

http://hg.mozilla.org/tracemonkey/rev/4360498880c0

Comment 11

[Bob Clary [:bc] (inactive)]

(In reply to comment #10)
> http://hg.mozilla.org/tracemonkey/rev/4360498880c0

misses call to reportCompare. Causes orange on jsreftests. Please check the unittest tinderbox or themasta pushlog to make sure the jsreftests aren't regressing.

Comment 12

[Jim Blandy :jimb]

(In reply to comment #11)
> misses call to reportCompare. Causes orange on jsreftests. Please check the
> unittest tinderbox or themasta pushlog to make sure the jsreftests aren't
> regressing.

Thanks for watching out.

http://hg.mozilla.org/tracemonkey/rev/860a58855453