"ASSERTION: Incorrect aState.mPrevChild before inserting line at end"

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
Layout
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: tnikkel)

Tracking

(Blocks: 2 bugs, {assertion, testcase, verified1.9.2})

Trunk
mozilla1.9.3a1
x86
Mac OS X
assertion, testcase, verified1.9.2
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.0.x -
in-testsuite +

Firefox Tracking Flags

(blocking1.9.2 needed, status1.9.2 .2-fixed, blocking1.9.1 needed, status1.9.1 .9-fixed)

Details

(Whiteboard: [sg:critical?])

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
Created attachment 423454 [details]
testcase

###!!! ASSERTION: Incorrect aState.mPrevChild before inserting line at end: 'aState.mPrevChild == mFrames.LastChild()', file /Users/jruderman/mozilla-central/layout/generic/nsBlockFrame.cpp, line 2162

bz marked a previous bug with this assertion as security-sensitive (bug 522170), so I'm filing this one as security-sensitive.
(Reporter)

Updated

8 years ago
Group: core-security
(Assignee)

Comment 1

8 years ago
On trunk this assertion shouldn't itself lead to a security problem. On 3.6 and earlier it is likely to cause a messed up frame tree, so a security problem is likely just around the corner.
(Assignee)

Comment 2

8 years ago
So I think the problem lies with this bit of code in nsBlockFrame::ReflowInlineFrame http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp#3838

    // If we just ended a first-letter frame or reflowed a placeholder then 
    // don't split the line and don't stop the line reflow...
    if (!(frameReflowStatus & NS_INLINE_BREAK_FIRST_LETTER_COMPLETE) && 
        nsGkAtoms::placeholderFrame != frameType) {
      // Split line after the current frame
      *aLineReflowStatus = LINE_REFLOW_STOP;
      rv = SplitLine(aState, aLineLayout, aLine, aFrame->GetNextSibling(), aLineReflowStatus);
      NS_ENSURE_SUCCESS(rv, rv);
    }

At this point *aLineReflowStatus is already LINE_REFLOW_STOP because the reflow status was break after for the frame we just reflowed, but we don't split the line because of the first letter complete check. And so we stop reflow after 1 of 2 frames on the line and when we get back to nsBlockFrame::ReflowDirtyLines mPrevChild is not the last frame on the line. Adding "|| *aLineReflowStatus == LINE_REFLOW_STOP" to the if condition fixes the asserts for me.
Sounds right.
(Assignee)

Comment 4

8 years ago
Created attachment 423615 [details] [diff] [review]
patch
Assignee: nobody → tnikkel
Attachment #423615 - Flags: review?(roc)
Attachment #423615 - Flags: review?(roc) → review+
(Assignee)

Comment 5

8 years ago
http://hg.mozilla.org/mozilla-central/rev/537f60b8e8df
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
(Assignee)

Comment 6

8 years ago
After some bake time on trunk this is something that would probably be good to have on the branches.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Whiteboard: [sg:critical?]
(Assignee)

Updated

8 years ago
Attachment #423615 - Flags: approval1.9.2.2?
Attachment #423615 - Flags: approval1.9.1.9?
Comment on attachment 423615 [details] [diff] [review]
patch

a=beltzner for 1.9.2.2 and 1.9.1.9
Attachment #423615 - Flags: approval1.9.2.2?
Attachment #423615 - Flags: approval1.9.2.2+
Attachment #423615 - Flags: approval1.9.1.9?
Attachment #423615 - Flags: approval1.9.1.9+
(Assignee)

Comment 8

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/dda32ceb3b5b
status1.9.1: wanted → .9-fixed
(Assignee)

Comment 9

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/9185a0d68573
status1.9.2: wanted → .2-fixed
I notice with the testcase post-fix that I'm not seeing the assert above anymore on OS X but I do get:

###!!! ASSERTION: overflow list w/o frames: 'mFrames.NotEmpty()', file /Users/albill/mozilla-192/layout/generic/nsInlineFrame.cpp, line 374


Is this a non-issue?
(Assignee)

Comment 11

8 years ago
I don't get that assertion on trunk, I would guess because of bug 533379 which is fixed on trunk but not 1.9.2.
Well, that isn't *this* assertion so I will call this verified1.9.2.
Keywords: verified1.9.2
verified 1.9.2-2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.2) Gecko/20100321 Firefox/3.6.2 ID:20100321170417 (debug build) - the testcase didn't caused the assertion from comment #0 - for comment 10 i have made a comment in bug 533379 - maybe that bug need to land on 1.9.2 too
Flags: wanted1.9.0.x-
Group: core-security
(Assignee)

Comment 14

7 years ago
Added crashtest
http://hg.mozilla.org/mozilla-central/rev/f8c56496ae7d
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.