453397 - TM: fatal assert: script->main <= target && target < script->code + script->length, at jsopcode.cpp:5182

Status: VERIFIED FIXED

Description

[Boris Zbarsky [:bz] (still digging out from vacation mail)]

Created attachment 336606 [details]
Testcase

BUILD: tracemonkey branch revision f4909cae663d

STEPS TO REPRODUCE:  run "js -j test.js" on the attached JS file.

ACTUAL RESULTS: fatal assert with this stack:

JS_Assert
ReconstructPCStack
js_ReconstructStackDepth
js_ExecuteTree

Assertion failure: script->main <= target && target < script->code + script->length, at jsopcode.cpp:5182

I ran into this while playing with the testcase in bug 451605, so this might be related to that issue.  In any case, the behavior seems to definitely depend on having all the loops, and on having that one if statement, and on that if statement sometimes evaluating false and sometimes true.  So if I change the 2 in that test to a 5 or make realStep smaller, the bug goes away.  I've tried to reduce the number of iterations as much as I could.  numRows and numCols can be increased to 5 (from 4) if desired; in that case the 2 can be changed to a 1, and realStep to something like 0.8.  But with the loop sizes at 4, using 1 and 0.8 doesn't trigger the bug.

In any case, the testcase seems to be about as small as I can make it.

Comment 1

[Boris Zbarsky [:bz] (still digging out from vacation mail)]

Ok, somewhere between rev f4909cae663d and rev a2f22fccef27 I started hitting this bug every single time on the testcase for bug 451605....  I tried hg bisect to figure out when things broke, but so many of the intermediate changesets either don't build or crash in some totally different part of the code on the testcases that I didn't get far...

Comment 2

[Boris Zbarsky [:bz] (still digging out from vacation mail)]

So I do hit the asserts about setting a bogus thisp that I added before I hit this bug.  I just hit this bug before I hit the gc crash from the bogus thisp.

Comment 3

[David Anderson [:dvander]]

The control flow of this example is an outer, inner, and innermost nested tree.  The inner has a call depth of 1, the others have a call depth of 0.

What appears to be happening is that we're bailing out in the innermost tree as a BRANC_EXIT and not reconstructing the frames right because we don't process the exact guards that are chained together (i.e., we skip the inner nested guard with a calldepth of 1).

Andreas has a patch in bug 454402 to fix this -- and in fact it seems to help, we get further in executing the program, but regress again with the same error when trying to extend the inner tree.  Will report back when I find more.

Comment 4

[Andreas Gal :gal]

Created attachment 339542 [details] [diff] [review]
Pick the right nested guard to extend the tree from.

Comment 5

[Andreas Gal :gal]

Created attachment 339543 [details] [diff] [review]
Added a couple asserts and handle nesting depth 1 correctly.

Comment 6

[Andreas Gal :gal]

Created attachment 339545 [details] [diff] [review]
Move tree exit/call guard handling into the builtin.

Comment 7

[Andreas Gal :gal]

Latest patch speeds up SS. Total score is now 1299 on my machine (improved by 70ms or so).

Comment 8

[David Anderson [:dvander]]

Pushed fix as changeset http://hg.mozilla.org/tracemonkey/rev/f657d39d36e8

Comment 9

[Bob Clary [:bc:]]

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-453397.js,v  <--  regress-453397.js
initial revision: 1.1

http://hg.mozilla.org/mozilla-central/rev/aad88696e0f5

Comment 10

[Henrik Skupin (:whimboo)]

Please see bug 456810 which still crashes with the same stack on trunk.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f657d39d36e8

Comment 12

[Bob Clary [:bc:]]

v 1.9.1, 1.9.2