465580 - TM: Crash when selecting "options" in new Yahoo Mail

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Faxmaster]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081118 Minefield/3.1b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081118 Minefield/3.1b2pre

Firefox crashes immediately when selecting "options" from within the new Yahoo Mail interface.

Reproducible: Always

Steps to Reproduce:
1. Log into Yahoo Mail.
2. Select "options."
Actual Results:  
Firefox crashes immediately.

Expected Results:  
Firefox should load the "options" sub-menu without crashing.

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

Reported in the daily thread on the Mozillzine forums: 

http://forums.mozillazine.org/viewtopic.php?p=5019385#p5019385 and following: 

User set JIT.content to false and now Yahoo is reported to be working per the poster in the forums.

Comment 2

[Ria Klaassen (not reading all bugmail)]

Not sure if its the same crash but Firefox crashes like hell when I type "crash" in the Keywords field.. :)

Comment 3

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #2)
> Not sure if its the same crash but Firefox crashes like hell when I type
> "crash" in the Keywords field.. :)

Forgot to say: with Adblock Plus enabled.

Comment 4

[Jim Jeffery not reading bug-mail 1/2/11]

Ria & Faxmaster, are you getting crash-reporter up ?  post links to the crashes if so.

Comment 5

[Carsten Book [:Tomcat]]

i build currently a new debug build to see whats crashing here.

Comment 6

[Faxmaster]

Jim: Firefox says it's unable to create a crash report.

Comment 7

[Faxmaster]

Got one: http://crash-stats.mozilla.com/report/index/ccc99cec-6de8-4dca-9ac6-51a920081118?p=1

Comment 8

[Faxmaster]

Regression between 20081114 and 20081115 builds.  

Additional crash report using 20081115: http://crash-stats.mozilla.com/report/index/347ddb93-f202-499b-aa14-8db120081118?p=1

Comment 9

[Carsten Book [:Tomcat]]

(In reply to comment #5)
> i build currently a new debug build to see whats crashing here.

This is works for me with a latest Debug Build - BuildID=20081118223418
Any chance this is fixed in a latest hourly after the tracemonkey merge ?

Comment 10

[Brendan Eich [:brendan]]

This could be a recently fixed bug dup. In case anyone gets this in a debugger: go up to js_MonitorLoopEdge and look at:

cx->fp->script->filename
cx->fp->script->lineno
js_FramePCToLineNumber(cx, cx->fp)

Try to wget that filename and extract the function whose body starts on that lineno. Thanks,

/be

Comment 11

[Faxmaster]

Using latest hourly (build 20081118224742) still getting crashes.  Two more reports:
http://crash-stats.mozilla.com/report/index/7f1fcbb8-1f5c-434b-b472-252320081119?p=1
http://crash-stats.mozilla.com/report/index/c3919245-947e-420b-88bf-a69920081119

Comment 12

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #4)
> Ria & Faxmaster, are you getting crash-reporter up ?  post links to the crashes
> if so.

The problem is still reproducible with yesterday's build but not with the latest hourly, so the Adblock Plus crash was possibly not related to this bug and fixed by a recent checkin :).

Comment 13

[Andreas Gal :gal]

Faxmaster seems to disagree so lets leave this open for a bit.

Comment 14

[Faxmaster]

Upon further testing, I've found the following:
1. If Adblock is enabled, the crash occurs.
2. If Adblock is enabled and javascript.options.jit.content is set to false, there is no crash.
3. If Adblock is disabled and javascript.options.jit.content is set to true, there is no crash.

This behavior has been tested on today's nightly (20081119).

Comment 15

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

Reproduced, debugging.

Comment 16

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

Attachment: test case

We're not guarding somewhere, simple test case to reproduce this crash attached.  callee for toString is NULL the second time around and that's not guarded.

Comment 17

[Tony Chung [:tchung]]

STR crashes on mac nightly as well.   Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081119 Minefield/3.1b2pre

My Stack: http://crash-stats.mozilla.com/report/index/36519057-6902-4542-877c-ed7020081119

Comment 18

[Brendan Eich [:brendan]]

(In reply to comment #16)
> Created an attachment (id=349062) [details]
> test case
> 
> We're not guarding somewhere, simple test case to reproduce this crash
> attached.  callee for toString is NULL the second time around and that's not
> guarded.

As noted in another bug (can't find the ref atm), trace-type-classifying null as object means we have to add extra not-null guards. Bleah. Investigate giving null its own typecode (5 since 3 is taken by JSVAL_BOXED).

/be

Comment 19

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

Attachment: proposed fix + test case

Guard that callee != NULL.

Comment 20

[Andreas Gal :gal]

Comment on attachment 349066 [details] [diff] [review]
proposed fix + test case

I suggest aborting trace if this == null during recording right before the if you inserted. Otherwise r=me.

Comment 21

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

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

Comment 22

[Tony Chung [:tchung]]

can we try and get this landed for beta 2 on m-c?

Comment 23

[Brendan Eich [:brendan]]

David, Andreas: is there a bug on file about typing null and object differently?

/be

Comment 24

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #22)
> can we try and get this landed for beta 2 on m-c?

No. Please don't do this. Adding beta 2 blockers will delay builds, which delays your QA start, which delays ship. If you think something should be a beta 2 blocker, ask a driver. I'll approve this for b2 if there are cycles in the tree, but I don't think this blocks.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 349066 [details] [diff] [review]
proposed fix + test case

a191b2=beltzner, if you want this to make b2, please hurry to find a landing spot.

Comment 26

[Jeff Walden [:Waldo]]

Patch landed in m-c.

Comment 27

[Bob Clary [:bc] (inactive)]

testNullCallee already in js1_8_1/trace/trace-test.js

Comment 28

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2