Closed Bug 344557 Opened 14 years ago Closed 14 years ago

[columns] Crash [@ nsLineBox::DeleteLineList] with moz-column-count and generated content

Categories

(Core :: Layout, defect, critical)

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: martijn.martijn, Assigned: mats)

References

Details

(5 keywords, Whiteboard: [sg:critical?] uses freed memory)

Crash Data

Attachments

(6 files, 1 obsolete file)

See upcoming testcase, which crashes current trunk build and Firefox1.5.05.
Talkback ID: TB20910283Z
0x0000012c
nsLineBox::DeleteLineList   nsBlockFrame::DoRemoveFrame   nsBlockFrame::RemoveFrame   nsFrameManager::RemoveFrame   nsCachedStyleData::Destroy  

I'm afraid it window size dependant, so the testcase contains a window.resizeTo(1440, 600); function, your browser should allow the resize.
Attached file testcase
Attached file Original testcase
Reproducable on Linux, trunk and 1.8 branch debug builds.
Looks like a bad overflow line list in the crashes I have.

(gdb) bt
#0  0xdddddddd in ?? ()
#1  0x410b2708 in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&) (aPresContext=0x8925070, aLines=@0x8783680) at nsLineBox.cpp:369
#2  0x41060265 in nsBlockFrame::Destroy() (this=0x89dfec0) at nsBlockFrame.cpp:305
#3  0x4106b97a in nsBlockFrame::DoRemoveFrame(nsIFrame*, int, int) (this=0x8809ec8, aDeletedFrame=0x89dfec0, aDestroyFrames=1,
    aRemoveOnlyFluidContinuations=0) at nsBlockFrame.cpp:5889
#4  0x4106b1ef in nsBlockFrame::RemoveFrame(nsIAtom*, nsIFrame*) (this=0x8809ec8, aListName=0x0, aOldFrame=0x89dfec0) at nsBlockFrame.cpp:5671
#5  0x41030e33 in nsFrameManager::RemoveFrame(nsIFrame*, nsIAtom*, nsIFrame*) (this=0x88dfecc, aParentFrame=0x89a17f8, aListName=0x0, aOldFrame=0x89dfec0)
    at nsFrameManager.cpp:720
#6  0x40ff66a3 in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, int, int) (this=0x88e0118, aContainer=0x88ed920, aChild=0x8937008,
    aIndexInContainer=142646984, aInReinsertContent=1) at nsCSSFrameConstructor.cpp:10081
#7  0x40ff5c6b in nsCSSFrameConstructor::ReinsertContent(nsIContent*, nsIContent*) (this=0x88e0118, aContainer=0x88ed920, aChild=0x8937008)
    at nsCSSFrameConstructor.cpp:9694
#8  0x40ffb620 in nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState&, nsIFrame*, nsIFrame*, nsIFrame*) (this=0x88e0118, aState=@0xbfffe2a0,

(gdb) fr 2
#2  0x41451932 in nsBlockFrame::Destroy(nsPresContext*) (this=0x89b2298, aPresContext=0x8760770) at nsBlockFrame.cpp:308
308         nsLineBox::DeleteLineList(aPresContext, *overflowLines);
(gdb) p overflowLines
$1 = (nsLineList *) 0x89b300c
(gdb) p *overflowLines
$2 = {mLink = {_mNext = 0x89ab47c, _mPrev = 0xdddddddd}}
(gdb)
OS: Windows XP → All
I have a fix, I think...
Assignee: nobody → mats.palmgren
We crash with the same stack also on 1.8.0 branch.
Flags: blocking1.9a1?
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
Attached file Frame dump and stack
We have a frame in |mLines| with a sibling in the overflow lines
and when we try to destroy the overflow lines we will access the
already destroyed sibling frame:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&rev=3.783&root=/cvsroot&mark=301,306#286

which was destroyed when traversing the sibling chain here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineBox.cpp&rev=1.109&root=/cvsroot&mark=367-371#360
Attached patch Patch rev. 1 (obsolete) — Splinter Review
Attachment #229428 - Flags: superreview?(roc)
Attachment #229428 - Flags: review?(roc)
Flags: blocking1.8.0.5? → blocking1.8.0.6?
(In reply to comment #6)
> which was destroyed when traversing the sibling chain here:
> http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineBox.cpp&rev=1.109&root=/cvsroot&mark=367-371#360

...which is the wrong way to delete the frames in a line; you need to check the child count to do that.  What is DeleteLineList supposed to be doing?
Flags: blocking1.8.1? → blocking1.8.1+
Whiteboard: [sg:critical?] uses freed memory
It's deleting all the frames and lines in the line list. The frame loop is OK because we're doing all lines, not just one line.
> We have a frame in |mLines| with a sibling in the overflow lines

This should not be happening. This is the underlying bug.
Attachment #229428 - Attachment is obsolete: true
Attachment #229428 - Flags: superreview?(roc)
Attachment #229428 - Flags: review?(roc)
The regression window is: 2006-01-20-16  --  2006-01-21-05
More specifically, it's the the nsBlockFrame patch from bug 310638,
I manually reverted that change and it fixed the testcase in this bug,
I'm pretty sure it brings back the old crash though...
Blocks: 310638
Keywords: regression
Attachment #230339 - Attachment description: Testcase #3 (nsBolckFrame crash that bug 310638 fixed) → Testcase #3 (nsBlockFrame crash that bug 310638 fixed)
Attached file Frame dump #2
Ok, I've found the real bug. Here's a frame tree to illustrate.
In the first round we're removing the blue frame, which have a
next-in-flow in the overflow line list (this is the case that
bug 310638 fixed). In this particular case though, we have a
next sibling as well (red). This is what trips us later because
we will pick up that sibling and set it as next sibling on our
prev sibling, on line 5863:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&rev=3.784&root=/cvsroot&mark=5863,5959-5969#5859
which happens to be on the main line list, which will crash
when this frame tree is destroyed later.

When we switch from the main line list to the overflow line list
(on line 5969) we need to null out |prevSibling|...
Attached patch Patch rev. 2Splinter Review
This patch fixes the crash. "Original testcase" still crash but that
turned out to be bug 334602. After applying Boris' patch for that the
testcase does not crash. There are still a few nasty assertions triggered
by it though but I think we can handle them in separate bugs:

###!!! ASSERTION: overflow list w/o frames: 'mFrames.NotEmpty()',
      file nsInlineFrame.cpp, line 362
###!!! ASSERTION: reflow dirty lines failed: 'NS_SUCCEEDED(rv)',
      file nsBlockFrame.cpp, line 916
###!!! ASSERTION: next in flow should have been deleted: '!kidNextInFlow',
      file nsColumnSetFrame.cpp, line 503
WARNING: Content has no document.: 
      file nsTextFrame.cpp, line 5948
WARNING: Reflow of frame failed in nsLineLayout:
      file nsLineLayout.cpp, line 997
Attachment #230404 - Flags: superreview?(roc)
Attachment #230404 - Flags: review?(roc)
Flags: blocking1.8.0.6? → blocking1.8.0.6+
Whiteboard: [sg:critical?] uses freed memory → [sg:critical?] [needs review roc] uses freed memory
Attachment #230404 - Flags: superreview?(roc)
Attachment #230404 - Flags: superreview+
Attachment #230404 - Flags: review?(roc)
Attachment #230404 - Flags: review+
I filed bug 346405 for the remaining assertions/crash on "Original testcase".

Checked in to trunk at 2006-07-29 02:38 PDT.
(I will ask for 1.8 and 1.8.0 branch approval in a couple of days...)

-> FIXED
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED
Whiteboard: [sg:critical?] [needs review roc] uses freed memory → [sg:critical?] uses freed memory
Are you going to go for 1.8.1 approval here?
Attachment #230404 - Flags: approval1.8.1?
Attachment #230404 - Flags: approval1.8.0.7?
Comment on attachment 230404 [details] [diff] [review]
Patch rev. 2

a=schrep for drivers.  Thanks guys - Land away!
Attachment #230404 - Flags: approval1.8.1? → approval1.8.1+
Checked in to MOZILLA_1_8_BRANCH at 2006-08-05 21:18 PDT.
Keywords: fixed1.8.1
Comment on attachment 230404 [details] [diff] [review]
Patch rev. 2

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #230404 - Flags: approval1.8.0.7? → approval1.8.0.7+
Checked in to MOZILLA_1_8_0_BRANCH at 2006-08-10 02:57 PDT.
Keywords: fixed1.8.0.7
https://bugzilla.mozilla.org/attachment.cgi?id=229114
ff2b2 debug/nightly windows/linux no crash
verified fixed 1.8
v.fixed on both branches with 8/24 nightly builds, no crash with either testcase:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7pre) Gecko/20060824 Firefox/1.5.0.7pre
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b2) Gecko/20060824 BonEcho/2.0b2
Group: security
Flags: in-testsuite?
Crash Signature: [@ nsLineBox::DeleteLineList]
Added crashtests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/308b0c3494ef
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.