Last Comment Bug 457375 - "ASSERTION: comparing iterators over different lists" with -moz-column, null character, height
: "ASSERTION: comparing iterators over different lists" with -moz-column, null ...
Status: VERIFIED FIXED
: [sg:critical?]
: assertion, crash, hang, testcase, verified1.9.0.4
Product: Core
Classification: Components
Component: Layout
: Trunk
: All All
: -- critical (vote)
: mozilla1.9.1b2
Assigned To: Mats Palmgren [:mats]
: layout
:
:
: 306939 377438
  Show dependency treegraph
 
Reported: 2008-09-26 20:55 PDT by Jesse Ruderman
Modified: 2008-11-29 19:14 PST (History)
9 users (show)
jruderman: in‑testsuite+
See Also:
Crash Signature:


Attachments
testcase (hangs/crashes Firefox when loaded) (216 bytes, text/html)
2008-09-26 20:55 PDT, Jesse Ruderman
no flags Details
Trace + frame dump (17.20 KB, text/html)
2008-10-06 20:48 PDT, Mats Palmgren [:mats]
no flags Details
Patch rev. 1 (711 bytes, patch)
2008-10-06 20:53 PDT, Mats Palmgren [:mats]
roc: review+
roc: superreview+
dveditz: approval1.9.0.4+
Details | Diff | Splinter Review
crashtest.diff (819 bytes, patch)
2008-10-11 17:22 PDT, Mats Palmgren [:mats]
no flags Details | Diff | Splinter Review

Summon comment box

Description Jesse Ruderman 2008-09-26 20:55:38 PDT
Created attachment 340690 [details]
testcase (hangs/crashes Firefox when loaded)

Loading the testcase in a trunk debug build triggers:

###!!! ASSERTION: comparing iterators over different lists: 'mListLink == aOther.mListLink', file /Users/jruderman/central/layout/base/../generic/nsLineBox.h, line 690

###!!! ABORT: running past end: 'mCurrent != mListLink', file /Users/jruderman/central/layout/base/../generic/nsLineBox.h, line 611

The abort usually indicates heap corruption.  This testcase makes nightlies hang rather than crash, but I'm filing as security-sensitive to be on the safe side.

Gary Kwong did the hard part of finding a reproducible testcase triggering the bug.  I just did the easy part of reducing it ;)
Comment 1 Mats Palmgren [:mats] 2008-10-06 20:48:57 PDT
Created attachment 342018 [details]
Trace + frame dump
Comment 2 Mats Palmgren [:mats] 2008-10-06 20:53:06 PDT
Created attachment 342020 [details] [review]
Patch rev. 1

When switching from the overflow lines to normal lines we must reset
'mInOverflowLines' or we'll compare 'mLine' to wrong list on the
next call.  See the printf's at the top and frame dump in the previous
attachment for details.
Comment 3 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-10-07 18:16:35 PDT
Comment on attachment 342020 [details] [review]
Patch rev. 1

r+sr if you move it into "if (currentlyInOverflowLines) {"
Comment 4 Mats Palmgren [:mats] 2008-10-07 18:26:10 PDT
That's not necessary for correctness though, do you think it's faster?
If so, why?
Comment 5 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-10-07 18:30:27 PDT
It's cleaner there. If we're currentlyInOverflowLines, then we toggle mInOverflowLines to null; otherwise the other branch of the 'if' toggles it to something non-null.
Comment 6 Mats Palmgren [:mats] 2008-10-07 18:42:36 PDT
Ok, I'll fix that.
Comment 7 Mats Palmgren [:mats] 2008-10-11 17:22:29 PDT
Created attachment 342746 [details] [review]
crashtest.diff
Comment 8 Mats Palmgren [:mats] 2008-10-11 18:16:57 PDT
http://hg.mozilla.org/mozilla-central/rev/f6ed4aa2071c

Holding the crashtest until 1.9.0.x is released with the fix.

-> FIXED
Comment 9 Daniel Holbert [:dholbert] 2008-10-13 13:13:36 PDT
(In reply to comment #8)
> http://hg.mozilla.org/mozilla-central/rev/f6ed4aa2071c

FWIW, the URL for the fixed (landed) version of the patch, addressing the change suggested in comment 3 & comment 5, is:
http://hg.mozilla.org/mozilla-central/raw-diff/f6ed4aa2071c/layout/generic/nsBlockFrame.cpp
Comment 10 Daniel Veditz 2008-10-17 10:33:53 PDT
Comment on attachment 342020 [details] [review]
Patch rev. 1

Approved for 1.9.0.4, a=dveditz for release-drivers
Comment 11 Mats Palmgren [:mats] 2008-10-18 08:44:47 PDT
Landed on CVS trunk for 1.9.0.4:
mozilla/layout/generic/nsBlockFrame.cpp 	3.958
Comment 12 Al Billings [:abillings] 2008-10-21 15:40:01 PDT
Verified for 1.9.0.4 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102104 GranParadiso/3.0.4pre.
Comment 13 Al Billings [:abillings] 2008-10-21 15:40:40 PDT
Verified for trunk with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081020 Minefield/3.1b2pre.

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