Crash [@ ReparentFrame] with moz-column-count, floats and display: table-row

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
12 years ago
4 years ago

People

(Reporter: martijn.martijn, Assigned: roc)

Tracking

({crash, testcase})

Trunk
x86
All
crash, testcase
Points:
---
Bug Flags:
wanted1.8.1.x +
wanted1.8.0.x ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse null deref?] fixed by bug 359371 on trunk, but not branch?, crash signature)

Attachments

(4 attachments)

(Reporter)

Description

12 years ago
See upcoming testcase, which crashes Mozilla on load.
It also crashes 1.8 branch builds, so it doesn't seem to be a regression.

Talkback ID: TB22233772X
ReparentFrame   nsBlockFrame::HandleOverflowPlaceholdersOnPulledLine   nsBlockFrame::Reflow
(Reporter)

Comment 1

12 years ago
Created attachment 234437 [details]
testcase
(Reporter)

Comment 2

12 years ago
Created attachment 234438 [details]
original testcase

Updated

12 years ago
Assignee: nobody → mats.palmgren
OS: Windows XP → All

Comment 3

12 years ago
Created attachment 236235 [details]
trace + frame dump

Comment 4

12 years ago
Created attachment 236243 [details] [diff] [review]
Patch rev. 1

When the nsBlockReflowContext destroys float continuations due to the
reflow being COMPLETE:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockReflowContext.cpp&rev=1.143&root=/cvsroot&mark=725,735#703

in some cases [the placeholders for] these continuations does not live in
the frame tree (including the overflowOutOfFlowsProperty list) but in
nsBlockReflowState::mOverflowPlaceholders which will then contain stale
placeholders and the crash occurs if the rest of the reflow looks at this
data, e.g. nsBlockFrame::HandleOverflowPlaceholdersOnPulledLine.

There is this comment:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&root=/cvsroot&mark=5601-5604#5568
which I guess is correct in a way, provided that no-one is using that
part of the block reflow state after that point.
After reading through the code and trying various ways to fix this, I get
the impression that there are several ways this can happen though.

This patch fixes the crash for the attached testcases, but I'm unsure
if it's the correct fix. I think there is at least one path through
nsLineLayout::ReflowFrame where a similar crash might happen if we're
unlucky.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineLayout.cpp&rev=3.238&root=/cvsroot&mark=1155,1163#1126

Comment 5

12 years ago
I just asked Robert about a similiar thing in tables, isn't it wrong to mark a frame complete if nsBlockReflowState::mOverflowPlaceholders is not empty?
Blocks are more complicated, unfortunately. We mark the block complete when the regular in-flow content has all been reflowed. But there could still be float content that needs to be reflowed, and hence overflowing placeholders.
I think the basic idea of removing these destroyed floats' placeholders from blockReflowState's mOverflowPlaceholders, when a float completes and destroys its next-in-flows, is the right idea. I don't see how this patch does that though
Mats, are you still on this or do you want me to take it?

Comment 9

12 years ago
You're most welcome to :-)
Assignee: mats.palmgren → roc
FWIW, I don't see this crahs on the trunk (linux) any more, both testcases load w/o crashing here.
(Reporter)

Comment 11

12 years ago
In a 2006-12-07 build, it still crashes, not anymore in a 2006-12-08 build (it hangs there, in a 2006-12-30 build, it doesn't even hang anymore).
Marking worksforme?
Roc, you still have work to do here, even if this doesn't appear to crash any more?
No, I think I fixed this in bug 359371. Perhaps the reflow branch stopped the crash and bug 359371 stopped the hang.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
This still happens on branch (since there's no reflow branch). Is it worth fixing there?
Flags: blocking1.8.1.4?
Did you check? bug 359371 is fixed on branch.
Attachment 234437 [details] crashes Firefox 2.0.0.2 for me.
s/2.0.0.2/2.0.0.3
The branch crash appears to be a null dereference. Is there an exploitable flaw here?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:nse null deref?]
Whiteboard: [sg:nse null deref?] → [sg:nse null deref?] fixed by bug 359371 on trunk, but not branch?
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x?
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.4?
Flags: wanted1.8.1.x? → wanted1.8.1.x+
Crash Signature: [@ ReparentFrame]
Group: core-security
You need to log in before you can comment on or make changes to this bug.