401734 - Crash [@ nsBidiPresUtils::InitLogicalArray] [@ nsLayoutUtils::GetFloatFromPlaceholder] with float, columns and bidi characters

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P3)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build after 100ms.

This seems to have regressed between 2007-07-12 and 2007-07-13:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-07-12+04&maxdate=2007-07-13+09&cvsroot=%2Fcvsroot
Not sure what made it start crashing.

http://crash-stats.mozilla.com/report/index/a9dab351-870d-11dc-88d1-001a4bd43ed6
0  	nsBidiPresUtils::InitLogicalArray(nsIFrame*)  	 mozilla/layout/base/nsBidiPresUtils.cpp:497
1 	nsBidiPresUtils::Resolve(nsBlockFrame*, nsIFrame*, int) 	mozilla/layout/base/nsBidiPresUtils.cpp:286
2 	nsBlockFrame::ResolveBidi() 	mozilla/layout/generic/nsBlockFrame.cpp:6739
3 	nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsBlockFrame.cpp:916
4 	nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) 	mozilla/layout/generic/nsContainerFrame.cpp:722
5 	nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, int, nsCollapsingMargin*) 	mozilla/layout/generic/nsColumnSetFrame.cpp:520
6 	nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsColumnSetFrame.cpp:749
7 	nsBlockReflowContext::ReflowBlock(nsRect const&, int, nsCollapsingMargin&, int, int, nsMargin&, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) 	mozilla/layout/generic/nsBlockReflowContext.cpp:339
8 	nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:2973
9 	nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:2216
10 	nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) 	mozilla/layout/generic/nsBlockFrame.cpp:1854
etc..

Comment 1

[Uri Bernstein]

nsPlaceholderFrame::GetRealFrameForPlaceholder() is returning NULL for a frame that actually is a placeholder frame. I think this should never happen. If I'm right, the problem isn't in bidi code.

Comment 2

[Boris Zbarsky [:bzbarsky]]

It can happen only if the frame tree gets broken somehow.  So yes, this is probably not a bidi problem per se.

Comment 3

[Simon Montagu :smontagu]

It would be easy enough to wallpaper over this, but the real issue is that nsPlaceholderFrame::GetRealFrameForPlaceholder at http://mxr.mozilla.org/seamonkey/source/layout/base/nsBidiPresUtils.cpp#496 asserts 
Null out-of-flow for placeholder?: 'outOfFlow', file nsPlaceholderFrame.h, line 175
and returns null

This looks like it might be similar to bug 386266, but I'm not cc-ed on that.

Comment 4

[Boris Zbarsky [:bzbarsky]]

You are now.

Comment 5

[Simon Montagu :smontagu]

Out of the checkins in the regression range, bug 315920 might be related.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Seems somewhat doubtful, since none of the code involved in that bug should be triggered by this testcase...  Then again, nothing else jumps out at me except bug 255990, which might have changed the wrapping...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Frame tree when we start nsCSSFrameConstructor::ContentRemoved

The overflow placeholder being a direct child of the second column of the <div> instead of being wrapped in a continuation for the <span> (akin to the way fantasai implemented phantom continuations for pagination) seems a little weird to me.

Comment 8

[Boris Zbarsky [:bzbarsky]]

And in fact, when we finish ContentRemoved all the span stuff is gone except for that lone overflow placeholder.

Which make sense: normally removing the in-flows is the job of the ancestor of the frame being removed, so if the inner span got removed the outer span should have removed the overflow placeholders, but with the outer span being removed the assumption is that removing _its_ next-in-flows will remove the in-flows of kids.  Except that this in-flow of a kid is not the child of an in-flow of the outer span.

Comment 9

[Simon Montagu :smontagu]

Attachment: Testcase without RTL characters

This certainly isn't a bidi bug: the same testcase without RTL characters triggers the same assertion and crashes in nsIFrame::GetStyleDisplay (sometimes it only crashes on reload)

Comment 10

[Simon Montagu :smontagu]

Top of the stack when attachment 286789 [details] crashes:
gklayout.dll!nsIFrame::GetStyleDisplay()  Line 95 + 0xa bytes	C++
gklayout.dll!nsLayoutUtils::GetFloatFromPlaceholder(nsIFrame * aFrame=0x05483fcc)  Line 208 + 0x8 bytes	C++
gklayout.dll!nsBlockFrame::CollectFloats(nsIFrame * aFrame=0x05483fcc, nsFrameList & aList={...}, nsIFrame * * aTail=0x0012cb74, int aFromOverflow=0x00000000, int aCollectSiblings=0x00000001)  Line 6575 + 0x9 bytes	C++
gklayout.dll!nsBlockFrame::ReparentFloats(nsIFrame * aFirstFrame=0x05483fcc, nsBlockFrame * aOldParent=0x0548402c, int aFromOverflow=0x00000000, int aReparentSiblings=0x00000001)  Line 1679	C++
gklayout.dll!nsBlockFrame::PullFrameFrom(nsBlockReflowState & aState={...}, nsLineBox * aLine=0x0549148c, nsBlockFrame * aFromContainer=0x0548402c, int aFromOverflowLine=0x00000000, nsLineList_iterator aFromLine={...}, nsIFrame * & aFrameResult=0x00000000)  Line 2444	C++
gklayout.dll!nsBlockFrame::PullFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, nsIFrame * & aFrameResult=0x00000000)  Line 2317 + 0x35 bytes	C++
gklayout.dll!nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012d050, LineReflowStatus * aLineReflowStatus=0x0012cdcc, int aAllowPullUp=0x00000001)  Line 3421 + 0x18 bytes	C++
gklayout.dll!nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012d050)  Line 3237 + 0x2a bytes	C++
gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012d050)  Line 2269 + 0x1b bytes	C++
gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1854 + 0x1b bytes	C++
gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x03511b40, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0x00000000)  Line 940 + 0xf bytes	C++
gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x05490d64, nsPresContext * aPresContext=0x03511b40, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0x00000000, int aY=0x00000000, unsigned int aFlags=0x00000000, unsigned int & aStatus=0x00000000, nsOverflowContinuationTracker * aTracker=0x00000000)  Line 722 + 0x21 bytes	C++
gklayout.dll!nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0x00000000, const nsColumnSetFrame::ReflowConfig & aConfig={...}, int aUnboundedLastColumn=0x00000000, nsCollapsingMargin * aBottomMarginCarriedOut=0x0012d844)  Line 522	C++
gklayout.dll!nsColumnSetFrame::Reflow(nsPresContext * aPresContext=0x03511b40, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0x00000000)  Line 749 + 0x20 bytes	C++
gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=0x00000000, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0x00000000, int aIsAdjacentWithTop=0x00000001, nsMargin & aComputedOffsets={...}, nsLineBox * aLine=0x05491540, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0x00000000, nsBlockReflowState & aState={...})  Line 339 + 0x2c bytes	C++
gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012df20)  Line 2973 + 0x4f bytes	C++
gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012df20)  Line 2216 + 0x1b bytes	C++
gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1854 + 0x1b bytes	C++
gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x03511b40, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0x00000000)  Line 940 + 0xf bytes	C++
gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=0x00000001, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0x00000000, int aIsAdjacentWithTop=0x00000001, nsMargin & aComputedOffsets={...}, nsLineBox * aLine=0x05490cb0, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0x00000000, nsBlockReflowState & aState={...})  Line 339 + 0x2c bytes	C++
gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012ea94)  Line 2973 + 0x4f bytes	C++
gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012ea94)  Line 2216 + 0x1b bytes	C++
gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1854 + 0x1b bytes	C++
gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x03511b40, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0x00000000)  Line 940 + 0xf bytes	C++

Comment 11

[Martijn Wargers (dead)]

If someone wants to have a "nsLayoutUtils::GetFloatFromPlaceholder(nsIFrame*)" crash testcase, I think I would be able to provide one:
http://crash-stats.mozilla.com/report/index/cfe3ef87-87b9-11dc-b9bd-001a4bd43ef6
I guess that's basically the same crash.

Comment 12

[Martijn Wargers (dead)]

Sorry, never mind, didn't see the testcase that Simon attached.

Comment 13

[Mats Palmgren (inactive)]

Should be fixed by bug 404721.  I've included the testcases on this bug
in the "mochitest" patch on that bug.

Comment 14

[Martijn Wargers (dead)]

Attachment: testcase3

Comment 15

[Martijn Wargers (dead)]

Attachment: testcase4

Comment 16

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

All testcases here, except testcase3, WFM on 10.5.4 Mac, Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080818150509 Minefield/3.1a2pre


Testcase3 crashes at another location:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000040
Crashed Thread:  0

Thread 0 Crashed:
0   libgklayout.dylib             	0x0a9da024 nsLineList::begin() + 20 (nsLineBox.h:1141)
1   libgklayout.dylib             	0x0a9da0e6 nsBlockFrame::begin_lines() + 20 (nsBlockFrame.h:151)
2   libgklayout.dylib             	0x0aa0ff50 nsBlockInFlowLineIterator::Prev() + 58 (nsBlockFrame.cpp:5194)
3   libgklayout.dylib             	0x0aab958d BuildTextRuns(gfxContext*, nsTextFrame*, nsIFrame*, nsLineList_iterator const*) + 1039 (nsTextFrameThebes.cpp:976)
4   libgklayout.dylib             	0x0aab9a4b nsTextFrame::EnsureTextRun(gfxContext*, nsIFrame*, nsLineList_iterator const*, unsigned int*) + 289 (nsTextFrameThebes.cpp:1873)
5   libgklayout.dylib             	0x0aabc4e4 nsTextFrame::AddInlineMinWidthForFlow(nsIRenderingContext*, nsIFrame::InlineMinWidthData*) + 90 (nsTextFrameThebes.cpp:5260)
6   libgklayout.dylib             	0x0aabc989 nsTextFrame::AddInlineMinWidth(nsIRenderingContext*, nsIFrame::InlineMinWidthData*) + 65 (nsTextFrameThebes.cpp:5364)
7   libgklayout.dylib             	0x0aa28656 nsContainerFrame::DoInlineIntrinsicWidth(nsIRenderingContext*, nsIFrame::InlineIntrinsicWidthData*, nsLayoutUtils::IntrinsicWidthType) + 416 (nsContainerFrame.cpp:693)
8   libgklayout.dylib             	0x0aa2bbdb nsFirstLetterFrame::AddInlineMinWidth(nsIRenderingContext*, nsIFrame::InlineMinWidthData*) + 39 (nsFirstLetterFrame.cpp:150)

Comment 17

[Martijn Wargers (dead)]

Comment on attachment 316799 [details]
testcase3

I've file the testcase3 crashing issue now as bug 451532.

Comment 18

[Martijn Wargers (dead)]

Comment on attachment 327669 [details]
testcase4

Testcase4 is also crashing for me in current trunk build, it has the same stack trace as testcase3.

Comment 19

[Mats Palmgren (inactive)]

crashtests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8887533419b3

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8887533419b3