97444 - crasher on javascript menu [@JS_GetPrivate]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[jeremy.m]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.3+)
Gecko/20010827
BuildID:    2001082703

javascript menu crashes mozilla

Reproducible: Always
Steps to Reproduce:
go to www.alamy.com, mouse over an image, roll the mouse over the javascript
menu that appears.
witness mozilla crash & burn.

Actual Results:  crash

Comment 1

[Emmanuel Bourg]

Crashed For Me with Gecko/2001080110 on NT4

TB34671248H

Comment 2

[Chase Tingley]

Confirmed on a linux cvs build from 20010827.

I see this on console just before receiving SIGABRT:

Assertion failure: OBJ_GET_CLASS(cx, obj)->flags & JSCLASS_HAS_PRIVATE, at
jsapi.c:1885

Comment 3

[Chase Tingley]

Attachment: stack trace

Comment 4

[Phil Schwartau]

Attachment: Reduced HTML testcase

Comment 5

[Phil Schwartau]

Steps to reproduce: 

1. Load the reduced testcase in Mozilla (won't work in NN4.7, IE4.7)
2. Mouseover the Mozilla image : a menu appears below it
3. Mouseover the menu
4. CRASH!

Comment 6

[Phil Schwartau]

Attachment: WinNT stack trace

Comment 7

[Phil Schwartau]

The site uses a Macromedia "Fireworks" JS menu template for rollovers.
I will attach the JS file below. Note it contains two copyright warnings,
one for Macromedia and also one for Netscape.

For further info on Macromedia Fireworks, see:

http://www.macromedia.com/support/fireworks/ts/documents/preloads_explained.htm
and                    /support/dreamweaver/ts/documents/fireworks_graphics.htm

Comment 8

[Phil Schwartau]

Attachment: fw_menu.js

Comment 9

[Phil Schwartau]

Note: my WinNT stack trace is from a debug WinNT build 2001-08-24.
However, looks exactly the same as the trace tingley got -

cc'ing Brendan and jband: does this look like JS Engine?

Comment 10

[Brendan Eich [:brendan]]

This could be JS engine, or maybe XPConnect.  We need to get this in a debugger
and I'll poke around.  Phil, can you set it up and mail me, and I'll stop by
tomorrow?  Thanks.

/be

Comment 11

[John Bandhauer]

Brendan: I just looked at this bit in the debugger. The problem is that the code  
down in js_Call in preparation for the call to js_ReportIsNotFunction swaps in a 
different fp->fun when fp->fun was previously null. But fp->argv[-2] is not a 
function object. So the code in JS_GetFrameFunctionObject is returning a plain 
JSObject which the caller (in this case GetFramePrincipal) reasonably assumes is 
a (possibly cloned) function object.

I'm thinking that maybe that code should be swapping the whole frame rather than 
just fp->fun. I wrote a patch that makes it not crash. I'll attach it for 
comment. If this *is* the right fix then we ought to determine if there are 
other similar cases to be fixed too.

Comment 12

[John Bandhauer]

Attachment: potential fix

Comment 13

[Brendan Eich [:brendan]]

I like it, r/sr=[e-mail].  I don't know of other cases than the one
my XXXbe comment cited, which you fixed.  The two other js_ReportIsNotFunction
calls come from places that haven't pushed a doomed frame.

I think we should try to get this fixed in 0.9.4.

/be

Comment 14

[Chase Tingley]

bug 98207 is another crash at JS_GetPrivate -- might be interesting to see if
the patch fixes that one too.

Comment 15

[John Bandhauer]

I'll take this bug.

Comment 16

[Brendan Eich [:brendan]]

tingley: bug 98207 is more likely a dup of bug 97293.

jband, you need r=, eh?  How about it, cc: list?  I'll send mail.  I'd like to
get your fix in 0.9.4.

/be

Comment 17

[Brendan Eich [:brendan]]

Comment on attachment 48094 [details] [diff] [review]
potential fix

sr=[e-mail], for sure.

/be

Comment 18

[John Bandhauer]

Setting target milestone for 0.9.4.

Comment 19

[rogerl (gone)]

I know this tune. r=rogerl

Comment 20

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 48094 [details] [diff] [review]
potential fix

r=shaver

Comment 21

[John Bandhauer]

Fix checked into trunk. I emailed drivers for branch checkin approval. Thanks.

Comment 22

[Asa Dotzler [:asa]]

Comment on attachment 48094 [details] [diff] [review]
potential fix

a=asa for checkin to the 0.9.4 branch.

Comment 23

[John Bandhauer]

checked in to branch too. Thanks.

Comment 24

[Phil Schwartau]

VERIFIED FIXED on trunk and 0.9.4 branch using binaries dated 20010909xx,
200100910xx on WinNT, Linux, and Mac. Tried both the given URL and the 
reduced HTML testcase. I did not crash on any mouseovers, no matter how
many times I moused over the drop-down menu -

Comment 25

[Bob Clary [:bc] (inactive)]

too old.