Last Comment Bug 499862 - "ASSERTION: invalid array index" with overflow:scroll, float:left, text-transform, changing <style>
: "ASSERTION: invalid array index" with overflow:scroll, float:left, text-trans...
Status: RESOLVED FIXED
: [sg:critical?]
: assertion, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: Layout
: Trunk
: x86 Mac OS X
: P2 normal (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Mozilla Corporation)
: layout
:
:
: 306939
  Show dependency treegraph
 
Reported: 2009-06-22 18:58 PDT by Jesse Ruderman
Modified: 2010-07-15 13:55 PDT (History)
7 users (show)
dveditz: blocking1.9.0.19-
dveditz: wanted1.9.0.x+
roc: in‑testsuite+
See Also:
Crash Signature:
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  final+
  ---
  ---
  .2-fixed
  ---
  .9-fixed


Attachments
testcase (380 bytes, text/html)
2009-06-22 18:58 PDT, Jesse Ruderman
no flags Details
fix (1.06 KB, patch)
2010-02-23 23:25 PST, Robert O'Callahan (:roc) (Mozilla Corporation)
smontagu: review+
mbeltzner: approval1.9.2.2+
mbeltzner: approval1.9.1.9+
mbeltzner: approval1.9.0.19-
Details | Diff | Splinter Review
simpler testcase (150 bytes, text/html)
2010-02-23 23:36 PST, Robert O'Callahan (:roc) (Mozilla Corporation)
no flags Details

Summon comment box

Description Jesse Ruderman 2009-06-22 18:58:33 PDT
Created attachment 384546 [details]
testcase

###!!! ASSERTION: Should have been cleared: 'mBreakSinks.IsEmpty()', file layout/generic/nsTextFrameThebes.cpp, line 648

###!!! ASSERTION: Should have Reset() before destruction!: 'mCurrentWord.Length() == 0', file content/base/src/nsLineBreaker.cpp, line 51

###!!! ASSERTION: invalid array index: 'i < Length()', file nsTArray.h, line 317

###!!! ASSERTION: Hmm, something went wrong, aOffset should have been found: 'mGlyphRuns[start].mCharacterOffset <= aOffset', file gfx/thebes/src/gfxFont.cpp, line 2189

Security-sensitive because the "invalid array index" assertion is in an unchecked array access function.
Comment 1 Robert O'Callahan (:roc) (Mozilla Corporation) 2010-02-23 23:25:21 PST
Created attachment 428646 [details] [review]
fix

We need to flush out break sinks etc even if there are no mapped flows. In this testcase, the textrun finishes after the T (since it's in a float by itself) so we exit too early from FlushFrames. Flushing out the break sinks is needed so that we tell the text-transform textrun where the capitalized characters are.
Comment 2 Robert O'Callahan (:roc) (Mozilla Corporation) 2010-02-23 23:36:57 PST
Created attachment 428648 [details]
simpler testcase

This testcase is simpler but still fires the first assertion.
Comment 3 Robert O'Callahan (:roc) (Mozilla Corporation) 2010-03-05 02:22:08 PST
http://hg.mozilla.org/mozilla-central/rev/9c24556c14c3

I checked in my simple testcase, which doesn't lead directly to a security issue.
Comment 4 Robert O'Callahan (:roc) (Mozilla Corporation) 2010-03-05 02:23:31 PST
Comment on attachment 428646 [details] [review]
fix

Patch should apply to all 1.9.x branches.
Comment 5 Mike Beltzner [:beltzner] 2010-03-05 13:29:44 PST
Comment on attachment 428646 [details] [review]
fix

a=beltzner for all branches
Comment 7 Robert O'Callahan (:roc) (Mozilla Corporation) 2010-03-09 15:57:06 PST
Checked into 1.9.0.
Comment 8 Robert O'Callahan (:roc) (Mozilla Corporation) 2010-03-09 18:25:19 PST
I backed this out of 1.9.0 because it caused crashes there. Is it worth figuring those out, or should we just leave this unfixed on 1.9.0?
Comment 9 Mike Beltzner [:beltzner] 2010-03-10 12:58:59 PST
Comment on attachment 428646 [details] [review]
fix

I'm fine to leave these unfixed, yeah. It does mean we can't open up this bug for a while longer, though.
Comment 10 Al Billings [:abillings] 2010-03-22 14:06:52 PDT
Verified for 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100311 Shiretoko/3.5.9pre (.NET CLR 3.5.30729) using testcase.

Verified for 1.9.2 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3pre) Gecko/20100315 Namoroka/3.6.3pre (.NET CLR 3.5.30729). (This was built just after the release branch was cut but before any checkins were made so it is the same as 1.9.2.2.)

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