186287 - Crash entering international smoketest top site (Flash corrupts stack)

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P1)

Description

[Tracy Walker [:tracy]]

seen on Mac OSX commercial build 2002-12-20-03-trunk

joins.com strikes again.

-go to the test URL
crash every time.

I disabled java and still crashed.  Unfortunately crash log isn't working
correctly on refresh machine. :-/  Can someone please help out there. This crash
is easily reproducable. Thank you.

Comment 1

[Simon Montagu :smontagu]

Mac-specific so reassigning to nhotta.

Comment 2

[nhottanscp]

It also crash with yesterday's build 2002-12-19-08-trunk.
I don't get crash report.

Comment 3

[Yuying Long]

No crash with me on 12-20-08 trunk build on:
Mac 10.2.2, 10.1.5 and OS 9.1-JA.

Comment 4

[Tracy Walker [:tracy]]

Attachment: crash log

got crash log working and I'm consistantly getting this crash.

Note:  tested back to Wednesdays smoketest build 2002-12-18-03-trunk; it
crashed once, but I couldn't reproduce on reinstall.

Comment 5

[nhottanscp]

The crash is in layout. Simon, could you work with layout group and find the
right owner?
I am building the trunk now.

Comment 6

[nhottanscp]

list of check ins for a week

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Flayout&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=week&mindate=&maxdate=&cvsroot=%2Fcvsroot

cc to jkeiser%netscape.com, dbaron%fas.harvard.edu who have checked in actively
last couple of days.

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Did this start between Wednesday and Thursday or between Thursday and Friday?

The stack looks a little bogus, since I don't see how
nsTableRowGroupFrame::Reflow calls nsSmallVoidArray::ElementAt.

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Oh, I guess comment 2 suggests it was between Wednesday and Thursday.

Are you sure it's platform-specific rather than being dependent on font size,
window size, or the like?

Comment 9

[Simon Montagu :smontagu]

Attachment: Stack from the URL

This stack isn't associated with the crash, but it may provide info which will
help focussing the investigation.

Comment 10

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 109840 [details]
crash log 

>PPC Thread State:\
>  srr0: 0x00288540 srr1: 0x0000f030                vrsave: 0x00000000\
...

It would be useful to find out from someone with a Mac debug build why it's
crashing (whether it's a null or garbage pointer, and which pointer).  It might
also be possible to learn this from disassembly and registers -- I don't
understand why talkback data always comes with the registers but not the
disassembly, since it's pretty hard to learn anything from the registers
without the disassembly...

Comment 11

[nhottanscp]

My debug build does not crash. I am doing opt build now.

Comment 12

[Simon Montagu :smontagu]

-> sfraser for now

Comment 13

[Simon Fraser [no longer active]]

Do MachO builds show the crash?

Comment 14

[kinmoz]

I'm not sure if this is related or not, but when I load this page on Win32, I
hit lots of assertions of the  form:

###!!! ASSERTION: we shouldn't have empty continuations anymore: '!emptyContinua
tion', file y:/mozilla/layout/html/base/src/nsLineLayout.cpp, line 2148
Break: at file y:/mozilla/layout/html/base/src/nsLineLayout.cpp, line 2148

Comment 15

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I doubt it's related.  I added that assertion because I didn't notice any pages
where it fired, and I wanted to know if I could remove a bunch of code.  It
seems I can't, and I'll remove the assertion sometime soon when the tree is open...

Comment 16

[nhottanscp]

I can reproduce the crash using my local opt build.

If I revert nsLineLayout.cpp rev=3.150 to rev=3.149 then it does not crash.

I tested three times and it always crashes with rev=3.150 but no crash with
rev=3.149.

Comment 17

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I could understand that fixing the assertion, but what does it have to do with
the crash?

Comment 18

[Simon Fraser [no longer active]]

I can't repro this crash in the 2002-12-20-03-trunk or 2002-12-20-08-trunk
builds (at the end of a DSL line).

Comment 19

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

smontagu pointed out that this could in some way be related to bug 149336.  What
version of flash do people who are seeing the crash have installed?

Comment 20

[Simon Fraser [no longer active]]

I don't crash with version 6.0r67 of the Flash plugin, but I do with 6.0r29 (the
default for OS X installs). So this could be related to bug 149336.

Comment 21

[nhottanscp]

I used nsLineLayout.cpp rev=3.150 and changed ns4xPluginInstance.cpp to
enable the ifdef code for my Mac build and the crash is gone.

Comment 22

[nhottanscp]

The version of Flash on my machine is 6.0r29.

Comment 23

[Simon Fraser [no longer active]]

Flash 6.0r47 does not crash on this site.

Comment 24

[Simon Fraser [no longer active]]

peterl: I need help here. We need to enable this code for Mac OS, to disable
linveconnect for Flash 6.0r29:

http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/ns4xPluginInstance.cpp#769

But I can't find a way to sniff the plugin version in this code. I need to get
at the plugin description and check for a string ending in "r29", or get the
file spec and open it, looking for a 'vers' resource. Any ideas?

Comment 25

[Simon Fraser [no longer active]]

removing smoketest keyword, since upgrading the plugin fixes this.

Comment 26

[Peter Lubczynski]

Attachment: patch v.1

The swliveconect attribute is no longer needed by Flash because they use
XPConnect. Here's a patch that enables the hack for bug 149336 for all
platforms.

Comment 27

[serge (gone)]

Comment on attachment 110177 [details] [diff] [review]
patch v.1

AFAIK we do not have this problem on win32, so why exec this extra code for
XP_WIN?

Comment 28

[Simon Montagu :smontagu]

This is the second time that this problem has come up for a specific platform
and kept the tree closed for most of a working day. If we keep the fix
platform-specific, even multi-platform-specific, it will very likely happen
again some time on the remaining platforms and waste another day.

Comment 29

[Simon Fraser [no longer active]]

Comment on attachment 110177 [details] [diff] [review]
patch v.1

r/sr on the patch, but please change the comment to state that flash plugins
don't use the 'swliveconnect' param, otherwise it looks like you're breaking
stuff. Is there a cleaner way to remove this param? Also, is there ever a
chance that some other plugin will implement "application/x-shockwave-flash"
and want to use this param?

Comment 30

[Simon Fraser [no longer active]]

Giving to peterl to check in the patch, after considering my concerns.

Comment 31

[Olivier Cahagne]

what's the status on this patch ? I'm asking as bug is a blocker.

Comment 32

[Kevin McCluskey (gone)]

nsbeta1+

Comment 33

[Kevin McCluskey (gone)]

Comment on attachment 110177 [details] [diff] [review]
patch v.1

r=[e-mail]

Risk is low: We already execute this code on XP_UNIX and the change is for a
specific mimetype and attribute which is not used by flash anymore.

Comment 34

[Peter Lubczynski]

Here's the comment I put the source:

Some older versions of Flash have a bug in them
that causes the stack to become currupt if we
pass swliveconect=1 in the NPP_NewProc arrays.
See bug 149336 (UNIX), bug 186287 (Mac)

The code below disables the attribute unless
the environment variable:
MOZILLA_PLUGIN_DISABLE_FLASH_SWLIVECONNECT_HACK
is set.

It is okay to disable this attribute because
back in 4.x, scripting required liveconnect to
start Java which was slow. Scripting no longer
requires starting Java and is quick plus controled
from the browser, so Flash now ignores this attribute.

This code can not be put at the time of creating
the array because we may need to examine the
stream header to determine we want Flash.

...to add, IIRC, currently Flash has something like 97% distribution so it's
highly unlikely another plugin is using this mimetype and would need this
attribute for anything in Gecko and if they did, there is an override.

Patch in trunk, marking FIXED.

Comment 35

[Tracy Walker [:tracy]]

verified with recent builds