368461 - Crash [@ nsLineBox::DisableResizeReflowOptimization] with MathML and <td>

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Loading the testcase in a debug build makes Firefox crash [@ nsLineBox::DisableResizeReflowOptimization].

Debug-only?

Comment 1

[Bernd]

debug only crashes usually indicate access to memory  that is either 0xdeadbeef or 0xdddddddd

I don't see a crash with slightly dated (1 or 2 weeks) debug build

I see however a couple of asserts:

###!!! ASSERTION: shouldn't use unconstrained widths anymore: 'availableWidth !=
 NS_UNCONSTRAINEDSIZE', file d:/moz_src/mozilla/layout/generic/nsHTMLReflowState
.cpp, line 214

WARNING: invalid markup: file d:/moz_src/mozilla/layout/mathml/base/src/nsMathML
msubsupFrame.cpp, line 167
WARNING: GetBoundingMetrics failed: file d:/moz_src/mozilla/layout/mathml/base/s
rc/nsMathMLContainerFrame.cpp, line 105

Comment 2

[Jesse Ruderman]

I think this is a recent regression, since I've seen it several times in the last few days but didn't see it before.

Comment 3

[Mats Palmgren (inactive)]

nsMathMLContainerFrame::ReflowForeignChild() constructs the nsLineLayout
passing a null line list iterator (ok I think):
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/mathml/base/src/nsMathMLContainerFrame.cpp&rev=1.159&root=/cvsroot&mark=988-989#975
which means 'mLineBox' will not be assigned:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineLayout.cpp&rev=3.252&root=/cvsroot&mark=137-140#94
and its members contains random data.
When we reflow we use it here and crash:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineLayout.cpp&rev=3.252&root=/cvsroot&mark=790#762

Patch coming up...

Comment 4

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 5

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

Comment on attachment 253100 [details] [diff] [review]
Patch rev. 1

>+#ifdef NS_DEBUG
>+    nsLineList_iterator() { memset(&mCurrent, 0xdd, sizeof(mCurrent)); }
>+#else
>     // Auto generated default constructor OK.
>     // Auto generated copy-constructor OK.
>+#endif

r+sr=dbaron, except every time you did the above pattern, the #endif should be one line earlier, since the auto-generated copy constructor is still fine and still used.

Comment 6

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

Though, actually, 0xdd is usually used to mean deleted memory.  I'm not sure what a normal marker for uninitialized memory is, but maybe use 0xda if nobody comes up with something better?  timeless?

Comment 7

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

0xcd is apparently used by something (debug crt on windows?), so how about using that?

Comment 8

[Mats Palmgren (inactive)]

0xcd sounds good to me.
http://www.nobugs.org/developer/win32/debug_crt_heap.html
http://www.microsoft.com/msj/1198/c/c1198.aspx

Comment 9

[Jesse Ruderman]

Mats, can you check this in (with dbaron's changes)?

Comment 10

[Mats Palmgren (inactive)]

Moved the #endif and changed the value to 0xcd.

Checked in to trunk at 2007-02-08 05:53 PST

Comment 11

[Jesse Ruderman]

Crashtest checked in.