265857 - Crash with malformed html [@ nsContainerFrame::PositionChildViews] [@ nsBlockFrame::ReflowFloat]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert Strong (they/them - no direct email)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041023 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041023 Firefox/1.0

New profile with no extensions and disabled java to simplify testing. Generated
random html and opened it with mozilla. Ten minutes later the browser crashed.
Verified with the following UA's and sent talkback for each. Testcase to follow.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041023 Firefox/1.0
TB1491736K

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041023
TB1491698Q

Reproducible: Always
Steps to Reproduce:
1. Disable java
2. View attached testcase
3.

Actual Results:  
Crash

Expected Results:  
No crash

Comment 1

[Robert Strong (they/them - no direct email)]

Attachment: Testcase

Comment 2

[Robert Strong (they/them - no direct email)]

adding keywords crash and testcase

Comment 3

[Robert Strong (they/them - no direct email)]

When opening the testcase directly from bugzilla I didn't experience the crash.
I saved it locally and viewed it and then I was able to reproduce the crash. I
expect your mileage may vary.

Comment 4

[Hermann Schwab]

testcase is a 64 KB sized file containing random junk, can you reduce it?

Link to the two talkback reports and others with same signature:

34 reports with stack signature nsBlockFrame::ReflowFloat 
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsBlockFrame%3A%3AReflowFloat&vendor=All&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

Comment 5

[Mats Palmgren (inactive)]

Attachment: Assertions before the crash

Comment 6

[Mats Palmgren (inactive)]

Attachment: Crash stack

Comment 7

[Robert Strong (they/them - no direct email)]

(In reply to comment #4)
> testcase is a 64 KB sized file containing random junk, can you reduce it?
Is it necessary to reduce it further since there is a trace attached to the bug?
If so, I can try to reduce it further... it was over 150 KB before I trimmed it
down and the returns were diminishing.

Comment 8

[Mats Palmgren (inactive)]

The testcase is not random junk - it's UTF-16 encoded.
I used validator.w3.org to show the source (at the end):
http://validator.w3.org/check?uri=https%3A%2F%2Fbugzilla.mozilla.org%2Fattachment.cgi%3Fid%3D163207%26action%3Dview&charset=%28detect+automatically%29&ss=1

Or you can use "View Source" in an older Mozilla that does not crash -
Mozilla 1.0.2 for example.

A non-UTF-16 testcase that triggers the crash would be appreciated.

Comment 9

[Robert Strong (they/them - no direct email)]

(In reply to comment #8)
> A non-UTF-16 testcase that triggers the crash would be appreciated.
I have been unable to save the testcase as UTF-8 and reproduce the crash.

One thing I have noticed is that the latest Firefox Aviary tends to crash more
often with UTF-16 files than the latest Mozilla. In most cases it occurs when
navigating away from the page after it was loaded. The attached testcase was the
only example I have been able to produce that also crashes the latest Mozilla.

Comment 10

[OstGote!]

Both crashes from comment 0 have as stack signature nsBlockFrame::ReflowFloat,
so I'm a little bit confused about the summary.

Comment 11

[Mats Palmgren (inactive)]

The crash is because we use a deleted frame (mContent = 0xdddddddd), my
crash data is from a Linux debug build, the talkbacks are from a Windows Opt
build so they can differ when the problem first surface.
In debug builds we overwrite deleted frames with the pattern above to make
memory problems easier to spot. (See FrameArena::FreeFrame() in nsPresShell.cpp).

Comment 12

[Boris Zbarsky [:bzbarsky]]

Yes, a minimal testcase would help a lot here.... it's very difficult to debug
crashes of this sort on huge pages.

Comment 13

[Bill Mason]

I was going to try to reduce the testcase, but I can't make the existing
testcase crash in either FF 1.0RC1 or a 20041027 trunk build, PC/WinXP.

Comment 14

[Robert Strong (they/them - no direct email)]

It seems that as soon as the file is smaller than 64 KB it stops crashing for
me. I have had a little luck with replacing sections of garbage with exact
copies of repeating garbage towards the start of the file but it still stops
crashing as soon as it gets below 64 KB. I'll continue to try but I don't
believe I will be able to reduce it in size though I believe it may be possible
to make at least some of the garbage repeating.

Comment 15

[Boris Zbarsky [:bzbarsky]]

So does using 64000 letter 'a's crash?  What about 64000 spaces?  That is, are
the exact bytes used relevant?  Or just the number of bytes?

Comment 16

[Robert Strong (they/them - no direct email)]

(In reply to comment #15)
> So does using 64000 letter 'a's crash?  What about 64000 spaces?  That is, are
> the exact bytes used relevant?  Or just the number of bytes?
Using 64000 a's or spaces does not crash. Some of the bytes appear to be
relevant though not all... I am able to replace large sections with repeating
a's or spaces and still cause a crash as long as the file size is above 64 KB. I
will continue replacing sections with a's in an attempt to simplify the testcase.

Comment 17

[Robert Strong (they/them - no direct email)]

Attachment: Testcase

Better testcase using a's in a div to fill in the file so a crash will occur
with the following at the end. Note: I noticed that a crash will not occur if
there is one less a or 8 more a's and Java must be disabled for this to crash
as with bug 265986.
<APPLET VSPACE="124602258205"></APPLET>
<DIV>1</DIV>
<LI STYLE="FLOAT:LEFT;"></LI>

Comment 18

[Robert Strong (they/them - no direct email)]

Attachment: Assertion before crash

This is when using the new testcase

Comment 19

[timeless]

bug 265986 also mentions java disabled

Comment 20

[Mats Palmgren (inactive)]

Regression window: 2004102205 -- 2004102305, after backing out bug 265027 it
does not crash (I removed "aState.mBottomEdge != NS_UNCONSTRAINEDSIZE" but
kept "!aUpdateMaximumWidth"). The problem is that there is a continuation
created for BODY and for some reason the placeholder(LI) remains in the
first block whereas the float is on the second (next-in-flow), then since
the next-in-flow is empty (no lines) it gets wiped. At least that's what I
think happens... See the attachment that follows.

Comment 21

[Mats Palmgren (inactive)]

Attachment: Frame dump

This is during reflow just before the crash.

Comment 22

[Asa Dotzler [:asa]]

Since this is a regression, we should try to get it fixed for 1.8 but not going
to block the alpha on this. 

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix?

This patch is untested (I'm still building). The basic idea is sound --- we
should never make continuations when the height is unconstrained, even if
integer overflows make it look like we've exceeded the available height. We may
just continue on to crash some other way, though. The only really good general
solution  that I can think of is to complete the switch to floats in layout.
Comments appreciated...

Comment 24

[Robert Strong (they/them - no direct email)]

The fix that caused this regression was from one of the Zalewski's generated
html crashes and this testcase was discovered / created in a similar manner.
Unless there appears to be other crashes occurring due to reasons other than
generated html that fixing this will fix I would hope no one would spend any
time on fixing this bug especially with the proper solution being the switch to
using floats. I expect there would also be a chance of this fix causing another
regression as bug 265027 caused this one though I may very well be mistaken in this.

Comment 25

[Hermann Schwab]

wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050116

tested both testcases, Reload and Shift-Reload, multiple times.
looking at the second testcase, may be related to the working of bug 255120

Comment 26

[Robert Strong (they/them - no direct email)]

wfm as well with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b)
Gecko/20050117 Firefox/1.0+

Checked both testcases with java turned off per the steps to reproduce as well
as with java turned on.

Comment 27

[Mats Palmgren (inactive)]

This crash is easy to reproduce with both attached testcases using a
debug build.  After applying the attached patch I could not make it crash.

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 170771 [details] [diff] [review]
fix?

r+sr=bzbarsky

Comment 29

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in