489836 - TM: chiptune causes "Assertion failure: !ti->typeMap.matches(ti_other->typeMap)"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Jesse Ruderman]

1. Load http://www.chiptune.com/
2. Focus the address bar
3. Press enter (reloading the page)

Result:

Assertion failure: !ti->typeMap.matches(ti_other->typeMap), at /Users/jruderman/tracemonkey/js/src/jstracer.cpp:3562

Comment 1

[Andreas Gal :gal]

This is a bad bug. Can you capture the page? Its important that we don't lose the test case.

I don't think we have to hold the beta for this. This should not have catastrophic consequences in OPT builds (maybe a slowdown, but no crash or correctness errors).

Comment 2

[Jesse Ruderman]

Attachment: captured as zip

"Not your momma's AJAX" indeed.  Had to hack its XHR function and convince it not to run in "debug" mode despite being loaded from a URL it didn't expect.

Note sure this is complete.  Will try to reduce with Lithium.

Comment 3

[Jesse Ruderman]

Attachment: partially reduced (one file, 200 lines)

Comment 4

[Jesse Ruderman]

Attachment: reduced testcase (shell, 10 lines)

Comment 5

[Brendan Eich [:brendan]]

Jesse, you are a rock star! ;-)

/be

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect (now working in Linux) shows this is probably related to bug 481793 :

The first bad revision is:
changeset:   25778:ec90dd58f1da
user:        Andreas Gal
date:        Fri Mar 06 17:25:04 2009 -0800
summary:     Better coordination of nested tree recording (481793, r=dmandelin).

Comment 7

[Andreas Gal :gal]

I think #6 is a red herring. It makes us succeed compiling the case earlier. With some modifications the bug would very likely exist earlier too.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

The bug is here:

checking nested types 0x30dc70: callee0=O/F this0=O/O argv0=I/I argv1=I/I vars0=I/I global0=B/B global1=I/D  (demotes 1)
checking nested types 0x30ded0: callee0=O/F this0=O/O argv0=I/I argv1=I/D vars0=I/I global0=B/B global1=I/I  (demotes 1)

Trace #1 expects argv1=I and global1=D.
Trace #2 expects argv1=D and global1=I.

A new trace is recorded with argv1=I and global1=I. argv1 is undemotable, so it tries to record argv1=D, global1=I, which already exists. The problem is that findNestedCompatiblePeer isn't realizing that global1=D somewhere else. Or the problem is that global1 was never marked as undemotable in the first place.

I won't be able to attempt a patch until Wednesday in case someone else wants to try.

Comment 9

[Andreas Gal :gal]

Yeah, I think the problem is that we start re-recording due to an argc mismatch (bug found by graydon, we can't call an inner tree if argc mismatches), but we already have a tree wih a different argc. The assert is agnostic to that. I will try to provide this with some additional debug output.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

argc isn't changing, it's purely types I think

Comment 11

[Andreas Gal :gal]

This is potentially a dup of 2 pending bugs. cc'ing dmandelin.

Comment 12

[Andreas Gal :gal]

Actually this doesn't crash for me any more with tip. Might need to bisect.

Comment 13

[Andreas Gal :gal]

dmandelin ran into a similar one. We think we know whats going on. I am stealing this. Please steal back if you made progress yourself.

Comment 14

[Andreas Gal :gal]

Attachment: patch

Comment 15

[Andreas Gal :gal]

Comment on attachment 376522 [details] [diff] [review]
patch

No longer try to call the tree with the fewest demotes. The better fix for that is to avoid the floating point math in the outer tree, which the new oracle demote patch does.

Comment 17

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 376522 [details] [diff] [review]
patch

Nice cleanup.

Comment 18

[Robert Sayre]

http://hg.mozilla.org/tracemonkey/rev/df0f692ad0c9

Comment 19

[David Mandelin [:dmandelin]]

Comment on attachment 376522 [details] [diff] [review]
patch

Yes, I didn't expect to see this change simplify the code, so that's great.

Comment 20

[Jim Blandy :jimb]

It seems that our correctness depends on the oracle never giving false negatives.  That is, if the JIT marks a given slot as undemotable, then if the oracle doesn't always call that slot undemotable, then we assert.  (Naturally, false positives are benign.)

So the oracle isn't just an optimization: it must give certain answers in some circumstances.

Comment 21

[Robert Sayre]

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

Comment 22

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6798c81129bf

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with testcase in comment 0 with the following debug builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090522 Minefield/3.6a1pre ID:20090522133810

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre)
Gecko/20090522 Shiretoko/3.5pre ID:20090522153422

Comment 24

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

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