Closed Bug 534082 Opened 15 years ago Closed 15 years ago

Crash at Wikipedia with -moz-column and list-item [@ nsLineBox::MarkDirty()] [@ nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int)]

Categories

(Core :: Layout, defect, P1)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.3a1
Tracking Flags:
Tracking Status
blocking2.0 --- alpha1+
status1.9.2 --- .2-fixed
status1.9.1 --- .8-fixed

People

(Reporter: bc, Assigned: dbaron)

References

(
URL
)

Details

(4 keywords, Whiteboard: [sg:critical?])

Crash Data

Attachments

(3 files, 2 obsolete files)

reduced testcase
203 bytes, text/html
Details
patch
3.51 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
the part of the patch that's relevant to 1.9.2 and 1.9.1
1.09 KB, patch
roc
: superreview+
dveditz
: approval1.9.2.2+
dveditz
: approval1.9.1.8+
dveditz
: approval1.9.0.18+
Details | Diff | Splinter Review 
1. http://en.wikipedia.org/wiki/Special:Search?search=house
2. crash 1.9.3 but not 1.9.2

+		this	0xdddddddd {mFirstChild=??? mBounds={...} mAscent=??? ...}	nsLineBox * const


>	gklayout.dll!nsLineBox::MarkDirty()  Line 252 + 0x3 bytes	C++
 	gklayout.dll!nsBlockFrame::DoRemoveFrame(nsIFrame * aDeletedFrame=0x00000000, unsigned int aFlags=0)  Line 5406	C++
 	gklayout.dll!RemoveBlockChild(nsIFrame * aFrame=0x0915d560, int aRemoveOnlyFluidContinuations=1)  Line 5183	C++
 	gklayout.dll!nsBlockFrame::DoRemoveFrame(nsIFrame * aDeletedFrame=0x0915d560, unsigned int aFlags=0)  Line 5414 + 0x17 bytes	C++
 	gklayout.dll!nsBlockFrame::DeleteNextInFlowChild(nsPresContext * aPresContext=0x05bf97a0, nsIFrame * aNextInFlow=0x0915d468, int aDeletingEmptyFrames=1)  Line 5532	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=0, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=1, nsLineBox * aLine=0x0953a7d0, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0, nsBlockReflowState & aState={...})  Line 359	C++
 	gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012a0cc)  Line 3115 + 0x45 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012a0cc)  Line 2384 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1885 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=3)  Line 993 + 0xf bytes	C++
 	gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x0915d400, nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=9242, int aY=0, unsigned int aFlags=0, unsigned int & aStatus=3, nsOverflowContinuationTracker * aTracker=0x00000000)  Line 774 + 0x21 bytes	C++
 	gklayout.dll!nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=3, const nsColumnSetFrame::ReflowConfig & aConfig={...}, int aUnboundedLastColumn=0, nsCollapsingMargin * aBottomMarginCarriedOut=0x0012a794, nsColumnSetFrame::ColumnBalanceData & aColData={...})  Line 675	C++
 	gklayout.dll!nsColumnSetFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=3)  Line 1035 + 0x22 bytes	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=0, nsLineBox * aLine=0x08bcd480, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=3, nsBlockReflowState & aState={...})  Line 310 + 0x2c bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012af14)  Line 3115 + 0x45 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012af14)  Line 2384 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1885 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 993 + 0xf bytes	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=0, nsLineBox * aLine=0x089eba88, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0, nsBlockReflowState & aState={...})  Line 310 + 0x2c bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012ba2c)  Line 3115 + 0x45 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012ba2c)  Line 2384 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1885 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 993 + 0xf bytes	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=1, nsLineBox * aLine=0x01bb03d0, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0, nsBlockReflowState & aState={...})  Line 310 + 0x2c bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012c544)  Line 3115 + 0x45 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012c544)  Line 2384 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1885 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 993 + 0xf bytes	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=1, nsLineBox * aLine=0x00000000, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0, nsBlockReflowState & aState={...})  Line 310 + 0x2c bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowFloat(nsBlockReflowState & aState={...}, const nsRect & aFloatAvailableSpace={...}, nsIFrame * aFloat=0x01bafa20, nsMargin & aFloatMargin={...}, unsigned int & aReflowStatus=0)  Line 5657 + 0x35 bytes	C++
 	gklayout.dll!nsBlockReflowState::FlowAndPlaceFloat(nsIFrame * aFloat=0x01bafa20, unsigned int & aReflowStatus=0, int aForceFit=1)  Line 768	C++
 	gklayout.dll!nsBlockReflowState::AddFloat(nsLineLayout * aLineLayout=0x0012d09c, nsIFrame * aFloat=0x01bafa20, int aAvailableWidth=58428, unsigned int & aReflowStatus=0)  Line 580 + 0x17 bytes	C++
 	gklayout.dll!nsLineLayout::AddFloat(nsIFrame * aFloat=0x01bafa20, int aAvailableWidth=58428, unsigned int & aReflowStatus=0)  Line 218	C++
 	gklayout.dll!nsLineLayout::ReflowFrame(nsIFrame * aFrame=0x01bafa88, unsigned int & aReflowStatus=0, nsHTMLReflowMetrics * aMetrics=0x00000000, int & aPushedFrame=0)  Line 892 + 0x1d bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, nsIFrame * aFrame=0x01bafa88, LineReflowStatus * aLineReflowStatus=0x0012d024)  Line 3745 + 0x16 bytes	C++
 	gklayout.dll!nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, nsFlowAreaRect & aFloatAvailableSpace={...}, int & aAvailableSpaceHeight=0, nsFloatManager::SavedState * aFloatStateBeforeLine=0x0012d090, int * aKeepReflowGoing=0x0012d4a4, LineReflowStatus * aLineReflowStatus=0x0012d158, int aAllowPullUp=1)  Line 3539 + 0x23 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012d4a4)  Line 3396 + 0x39 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012d4a4)  Line 2439 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1885 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 993 + 0xf bytes	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=0, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=1, nsLineBox * aLine=0x01bb0420, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0, nsBlockReflowState & aState={...})  Line 310 + 0x2c bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012dfbc)  Line 3115 + 0x45 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012dfbc)  Line 2384 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1885 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 993 + 0xf bytes	C++
 	gklayout.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=0, int aIsAdjacentWithTop=1, nsLineBox * aLine=0x01bb0448, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0, nsBlockReflowState & aState={...})  Line 310 + 0x2c bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012ead4)  Line 3115 + 0x45 bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012ead4)  Line 2384 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1885 + 0x1b bytes	C++
 	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 993 + 0xf bytes	C++
 	gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x01b9d750, nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0, int aY=0, unsigned int aFlags=0, unsigned int & aStatus=0, nsOverflowContinuationTracker * aTracker=0x00000000)  Line 774 + 0x21 bytes	C++
 	gklayout.dll!nsCanvasFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 554	C++
 	gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x01ba9d00, nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0, int aY=0, unsigned int aFlags=3, unsigned int & aStatus=0, nsOverflowContinuationTracker * aTracker=0x00000000)  Line 774 + 0x21 bytes	C++
 	gklayout.dll!nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState * aState=0x0012f450, int aAssumeHScroll=0, int aAssumeVScroll=1, nsHTMLReflowMetrics * aMetrics=0x0012f3a4, int aFirstPass=1)  Line 545 + 0x30 bytes	C++
 	gklayout.dll!nsHTMLScrollFrame::ReflowContents(ScrollReflowState * aState=0x0012f450, const nsHTMLReflowMetrics & aDesiredSize={...})  Line 639 + 0x35 bytes	C++
 	gklayout.dll!nsHTMLScrollFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 840 + 0x13 bytes	C++
 	gklayout.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x01ba9e78, nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0, int aY=0, unsigned int aFlags=0, unsigned int & aStatus=0, nsOverflowContinuationTracker * aTracker=0x00000000)  Line 774 + 0x21 bytes	C++
 	gklayout.dll!ViewportFrame::Reflow(nsPresContext * aPresContext=0x05bf97a0, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 285 + 0x2d bytes	C++
 	gklayout.dll!PresShell::DoReflow(nsIFrame * target=0x01ba9b00, int aInterruptible=1)  Line 7373	C++
 	gklayout.dll!PresShell::ProcessReflowCommands(int aInterruptible=1)  Line 7493 + 0x10 bytes	C++
 	gklayout.dll!PresShell::FlushPendingNotifications(mozFlushType aType=Flush_InterruptibleLayout)  Line 4930 + 0x12 bytes	C++
 	gklayout.dll!PresShell::ReflowEvent::Run()  Line 7180	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012fa18)  Line 527 + 0x19 bytes	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00d924b0, int mayWait=1)  Line 250 + 0x16 bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 170 + 0xc bytes	C++
 	toolkitcomps.dll!nsAppStartup::Run()  Line 182 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=4, char * * argv=0x00d3c800, const nsXREAppData * aAppData=0x00d3cf90)  Line 3499 + 0x25 bytes	C++
 	firefox.exe!NS_internal_main(int argc=4, char * * argv=0x00d3c800)  Line 158 + 0x12 bytes	C++
 	firefox.exe!wmain(int argc=4, wchar_t * * argv=0x00d389d8)  Line 120 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 594 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 414	C
 	kernel32.dll!7c817077() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
 	mozjs.dll!nanojit::Assembler::resourceConsistencyCheck()  Line 256 + 0x37 bytes	C++
 	uconv.dll!nsShiftJISToUnicodeConstructor(nsISupports * aOuter=0x00720070, const nsID & aIID={...}, void * * aResult=0x00730065)  Line 436 + 0x8a bytes	C++
 	xpcom_core.dll!nsSupportsCharImpl::AddRef()  Line 449 + 0xa3 bytes	C++
Can we capture the raw page as a testcase in case it changes? This looks like a windows debug crash, does it crash in opt? Is the OS "All" accurate?

If this doesn't crash 1.9.2 when did it regress? That might point us right at the problem. Easier to find if it crashes in opt nightly builds, of course, so we need that answer first.
blocking2.0: --- → ?
status1.9.2: --- → unaffected
Keywords: regression, regressionwindow-wanted
Whiteboard: [sg:critical?]
the stack was windows debug. reproduced in mac as well so All is correct. I don't crash mac trunk opt though. If no one beats me to it, I'll take a look when I get home.
indeed this seems a debug crash - also no crash in windows opt build.
Works for me on a mozilla-central build (an no valgrind warnings either, with DEBUG_TRACEMALLOC_PRESARENA).
This involves -moz-column (notice nsColumnSetFrame on the stack and column-lists on the page).  So the crash probably depends on the width of the browser window and the fonts involved.

bc, I think your best bet is to add code to the testcase that loops through a range of body widths (or window sizes).  Then you can reduce with Lithium.  Then, change the testcase to use only monospace fonts, ch horizontal units, and em vertical units. Finally, figure out which body width triggers the bug and remove the looping.

Or we could fix bug 508473, which would make it possible for me to stop ignoring nsColumnSetFrame crashes I encounter while fuzzing.  It might fix this crash too.
Summary: Crash [@ nsLineBox::MarkDirty()] → Crash [@ nsLineBox::MarkDirty()] involving -moz-column on Wikipedia
reproduced crash on load with mac trunk debug build from today and a window width of 600px

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0xc0000023
0x034b0f2f in nsLineBox::MarkDirty (this=0xc0000003) at nsLineBox.h:252
252	    mFlags.mDirty = 1;
(gdb) bt
#0  0x034b0f2f in nsLineBox::MarkDirty (this=0xc0000003) at nsLineBox.h:252
#1  0x034be41e in nsBlockFrame::DoRemoveFrame (this=0x20c5e098, aDeletedFrame=0x0, aFlags=0) at /work/mozilla/builds/1.9.3/mozilla/layout/generic/nsBlockFrame.cpp:5405
#2  0x034be5f2 in RemoveBlockChild (aFrame=0x20c5dd58, aRemoveOnlyFluidContinuations=1) at /work/mozilla/builds/1.9.3/mozilla/layout/generic/nsBlockFrame.cpp:5182
#3  0x034be47d in nsBlockFrame::DoRemoveFrame (this=0x20c5d330, aDeletedFrame=0x20c5dd58, aFlags=0) at /work/mozilla/builds/1.9.3/mozilla/layout/generic/nsBlockFrame.cpp:5414
#4  0x034be54f in nsBlockFrame::DeleteNextInFlowChild (this=0x20c5d330, aPresContext=0x6d56600, aNextInFlow=0x20c62358, aDeletingEmptyFrames=1) at /work/mozilla/builds/1.9.3/mozilla/layout/generic/nsBlockFrame.cpp:5530
#5  0x034cce04 in nsBlockReflowContext::ReflowBlock (this=0xbfff6f00, aSpace=@0xbfff6fb4, aApplyTopMargin=0, aPrevMargin=@0xbfff7728, aClearance=0, aIsAdjacentWithTop=1, aLine=0x20b16ba0, aFrameRS=@0xbfff6db0, aFrameReflowStatus=@0xbfff6fa4, aState=@0xbfff76b8) at /work/mozilla/builds/1.9.3/mozilla/layout/generic/nsBlockReflowContext.cpp:354

...

I'll point lithium at it unless Jesse gets a testcase first!
Attached file testcase (obsolete) — 

    
  
Keywords: testcase-wanted → testcase

    
    

    
  

      
      
      
      

        Attached file
          
        testcase (obsolete)
        — 
      

    


  
Now independent of window size and ready for automatic reduction.

        Attachment #417667 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached file
          
        reduced testcase
        — 
      

    


  

        Attachment #417739 -
        Attachment is obsolete: true

    
  
Summary: Crash [@ nsLineBox::MarkDirty()] involving -moz-column on Wikipedia → Crash [@ nsLineBox::MarkDirty()] at Wikipedia with -moz-column and list-item

    
    

    
  
In an opt build, this crash shows up as nsBlockFrame::DoRemoveFrame, which I guess means nsLineBox::MarkDirty got inlined.
Summary: Crash [@ nsLineBox::MarkDirty()] at Wikipedia with -moz-column and list-item → Crash at Wikipedia with -moz-column and list-item [@ nsLineBox::MarkDirty()] [@ nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int)]

    
    

    
  
None of the testcases crash for me with opt builds on Windows or Linux.

    
    

    
  
Crashes reliably for me on Mac :/

    
    

    
  
I hit this semi-reliably on a real wikipedia page:  when I load http://en.wikipedia.org/wiki/Suffolk_County,_Massachusetts#History and then right-click on both the "Revere" and then "Winthrop" links in the second paragraph of that section to open them in new tabs.

I got useful valgrind output for those steps, and then realized it's the same problem as this bug.  The valgrind output makes the problem relatively straightforward (same for my wikipedia case and attachment 417793 [details]):

==30699== Invalid read of size 8
==30699==    at 0x1B5B9803: nsLineList_iterator::operator++() (nsLineBox.h:593)
==30699==    by 0x1B5E5AFE: nsLineList_iterator::next() (nsLineBox.h:664)
==30699==    by 0x1B5F1137: nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int) (nsBlockFrame.cpp:5476)
==30699==    by 0x1B5F0C25: nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int) (nsBlockFrame.cpp:5486)
==30699==    by 0x1B5FE0CC: nsBlockReflowContext::ReflowBlock(nsRect const&, int, nsCollapsingMargin&, int, int, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) (nsBlockReflowContext.cpp:354)

==30699==  Address 0x21c0d700 is 0 bytes inside a block of size 16 free'd
==30699==    at 0x4C24A7A: operator delete(void*) (vg_replace_malloc.c:346)
==30699==    by 0x1B5F1092: nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int) (nsBlockFrame.cpp:5419)
==30699==    by 0x1B5F0C25: nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int) (nsBlockFrame.cpp:5486)
==30699==    by 0x1B5FE0CC: nsBlockReflowContext::ReflowBlock(nsRect const&, int, nsCollapsingMargin&, int, int, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) (nsBlockReflowContext.cpp:354)

In particular, this is a regression from http://hg.mozilla.org/mozilla-central/rev/21d443acefc5 .

What's happening in this case is that this call to |delete lineList|:
http://hg.mozilla.org/mozilla-central/file/aabd98c04dc9/layout/generic/nsBlockFrame.cpp#l5350
invalidates the iterators |line| and |line_end| which are then used here:
http://hg.mozilla.org/mozilla-central/file/aabd98c04dc9/layout/generic/nsBlockFrame.cpp#l5407

(I haven't thought about how we might fix this.  At least not yet.)
blocking2.0: ? → alpha1

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  
This fixes the crash that I see on the testcase and the valgrind warning on the testcase.

The changes that fix the testcase are the ones in DoRemoveFrame; all the rest is from code audit of the other sites changed by the patch that caused this.

Given how broken StealFrame is, I'm surprised it's used anywhere.  Do we actually need that code?
Assignee: nobody → dbaron
Status: NEW → ASSIGNED

        Attachment #419128 -
        Flags: review?(tnikkel)

    
    

    
  
Above patch passed try server, modulo a Mac crash on mochitest-plain that I've never seen before but looks related to DOM workers:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1261672344.1261679984.6328.gz

    
    

    
  
We use StealFrame when handling overflow containers.

    
    

    
  
Comment on attachment 419128 [details] [diff] [review]
patch

Thanks for cleaning up my mess.

        Attachment #419128 -
        Flags: review?(tnikkel) → review+
Flags: in-testsuite?
Keywords: regressionwindow-wanted

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/131e6354c8ad

Might want to get the StealFrame changes on 1.9.1 at some point (and, I suppose, 1.9.2), but I want to let this bake a bit first.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Priority: -- → P1
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1

    
    

    
  


  
This diff is against 1.9.2, but it looks like it should apply cleanly to 1.9.1 as well.

        Attachment #419179 -
        Flags: superreview?(roc) → superreview+

        Attachment #419179 -
        Flags: approval1.9.2.1?

        Attachment #419179 -
        Flags: approval1.9.1.8?

        Attachment #419179 -
        Flags: approval1.9.0.18?

          status1.9.1:
          --- → wanted
Flags: wanted1.9.0.x+

        Attachment #419179 -
        Flags: approval1.9.1.8?

        Attachment #419179 -
        Flags: approval1.9.1.8+

        Attachment #419179 -
        Flags: approval1.9.0.18?

        Attachment #419179 -
        Flags: approval1.9.0.18+

    
    

    
  
Comment on attachment 419179 [details] [diff] [review]
the part of the patch that's relevant to 1.9.2 and 1.9.1

Approved for 1.9.1.8 and 1.9.0.18, a=dveditz for release-drivers
Whiteboard: [sg:critical?] → [sg:critical?][needs 1.9.1/1.9.0 landing]

    
    

    
  
Checking in nsBlockFrame.cpp;
/cvsroot/mozilla/layout/generic/nsBlockFrame.cpp,v  <--  nsBlockFrame.cpp
new revision: 3.968; previous revision: 3.967
done
Keywords: fixed1.9.0.18

    
    

    
  
Has anyone actually seen this crash on 1.9.1? I've tried it in a debug 1.9.1.6pre build that I have and a 1.9.1.8pre build that I made today and the reduced testcase doesn't crash.

    
    

    
  
No.  Only a part of the patch (which patched a bunch of similar problems found by code audit) was relevant to 1.9.1 and 1.9.0, and it wasn't the part that fixed the original crash.

          status1.9.2:
          unaffected → wanted
Whiteboard: [sg:critical?][needs 1.9.1/1.9.0 landing] → [sg:critical?]

    
    

    
  
Comment on attachment 419179 [details] [diff] [review]
the part of the patch that's relevant to 1.9.2 and 1.9.1

Approved for 1.9.2.2, a=dveditz for release-drivers

        Attachment #419179 -
        Flags: approval1.9.2.2? → approval1.9.2.2+
Group: core-security

    
    

    
  
Verifying FIXED, tested with latest nightly:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a2pre) Gecko/20100223 Minefield/3.7a2pre (.NET CLR 3.5.30729)
Status: RESOLVED → VERIFIED

    
    

    
  
(In reply to comment #25)
> No.  Only a part of the patch (which patched a bunch of similar problems found
> by code audit) was relevant to 1.9.1 and 1.9.0, and it wasn't the part that
> fixed the original crash.

What about for 1.9.2?

    
    

    
  
1.9.0 and 1.9.1 and 1.9.2

    
    

    
  
in-testsuite+ http://hg.mozilla.org/mozilla-central/rev/6d21a2262e80
Flags: in-testsuite? → in-testsuite+

    
    

    
  
Nothing to verify here for 1.9.0, 1.9.1, or 1.9.2 since it doesn't crash on the branches.
Crash Signature: [@ nsLineBox::MarkDirty()]
[@ nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int)]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: