388602 - ###!!! ASSERTION: invalid array index: 'i < Length()' when loading some webpages

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Frank Wein [:mcsmurf]]

I found several websites where I get
###!!! ASSERTION: invalid array index: 'i < Length()', file d:\moz\mozilla\obj-s
uite-debug\dist\include\xpcom\nsTArray.h, line 318
when loading such a website. Sites I found are for example http://lxr.mozilla.org/seamonkey/search?string=nslinebreaker or http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey. I also got this assertion while trying to enter a bug report in this form here, so there are probably many ways to trigger it. The stack trace is always like that:
0:000> kp
ChildEBP RetAddr  
WARNING: Stack unwind information not available. Following frames may be wrong.
00129244 002f6a3d ntdll!DbgBreakPoint
*** WARNING: Unable to verify checksum for D:\moz\mozilla\obj-suite-debug\dist\bin\components\gklayout.dll
0012965c 02414d71 xpcom_core!NS_DebugBreak_P(unsigned int aSeverity = 1, char * aStr = 0x02b7c330 "invalid array index", char * aExpr = 0x02b7c344 "i < Length()", char * aFile = 0x02b7c354 "d:\moz\mozilla\obj-suite-debug\dist\include\xpcom\nsTArray.h", int aLine = 318)+0x27d [d:\moz\mozilla\xpcom\base\nsdebugimpl.cpp @ 350]
0012967c 02413d33 gklayout!nsTArray<unsigned char>::ElementAt(unsigned int i = 0)+0x31 [d:\moz\mozilla\obj-suite-debug\dist\include\xpcom\nstarray.h @ 318]
0012968c 025d8ee7 gklayout!nsTArray<unsigned char>::operator[](unsigned int i = 0)+0x13 [d:\moz\mozilla\obj-suite-debug\dist\include\xpcom\nstarray.h @ 352]
0012a6b0 024088d2 gklayout!nsLineBreaker::AppendText(class nsIAtom * aLangGroup = 0x033202e8, unsigned char * aText = 0x05c70f60 "mZdiff:+224 (+229/-5)???", unsigned int aLength = 0x15, unsigned int aFlags = 3, class nsILineBreakSink * aSink = 0x00000000)+0x317 [d:\moz\mozilla\content\base\src\nslinebreaker.cpp @ 277]
0012a728 02408012 gklayout!BuildTextRunsScanner::SetupBreakSinksForTextRun(class gfxTextRun * aTextRun = 0x05c70e60, int aIsExistingTextRun = 0, int aSuppressSink = 1)+0x242 [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 1870]
0012ba7c 02406b7d gklayout!BuildTextRunsScanner::BuildTextRunForFrames(void * aTextBuffer = 0x0012bab1)+0xcf2 [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 1773]
0012caa8 024071ce gklayout!BuildTextRunsScanner::FlushFrames(int aFlushLineBreaks = 1)+0x12d [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 1248]
0012cad8 02407240 gklayout!BuildTextRunsScanner::ScanFrame(class nsIFrame * aFrame = 0x05c1ca1c)+0x25e [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 1390]
0012cb08 024094ca gklayout!BuildTextRunsScanner::ScanFrame(class nsIFrame * aFrame = 0x05cdc360)+0x2d0 [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 1402]
0012cebc 02408d13 gklayout!BuildTextRuns(class nsIRenderingContext * aRC = 0x05cc0818, class nsTextFrame * aForFrame = 0x05c1c9dc, class nsIFrame * aLineContainer = 0x05c1c548, class nsLineList_iterator * aForFrameLine = 0x00000000)+0x54a [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 1199]
0012cf2c 02410eda gklayout!nsTextFrame::EnsureTextRun(class nsIRenderingContext * aRC = 0x05cc0818, class nsIFrame * aLineContainer = 0x00000000, class nsLineList_iterator * aLine = 0x00000000, unsigned int * aFlowEndInTextRun = 0x0012d008)+0x83 [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 1953]
0012d030 02411464 gklayout!nsTextFrame::AddInlineMinWidthForFlow(class nsIRenderingContext * aRenderingContext = 0x05cc0818, struct nsIFrame::InlineMinWidthData * aData = 0x0012d144)+0x2a [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 4926]
0012d04c 024212f2 gklayout!nsTextFrame::AddInlineMinWidth(class nsIRenderingContext * aRenderingContext = 0x05cc0818, struct nsIFrame::InlineMinWidthData * aData = 0x0012d144)+0x54 [d:\moz\mozilla\layout\generic\nstextframethebes.cpp @ 5010]
0012d088 024279f9 gklayout!nsContainerFrame::DoInlineIntrinsicWidth(class nsIRenderingContext * aRenderingContext = 0x05cc0818, struct nsIFrame::InlineIntrinsicWidthData * aData = 0x0012d144, nsLayoutUtils::IntrinsicWidthType aType = MIN_WIDTH (0))+0x142 [d:\moz\mozilla\layout\generic\nscontainerframe.cpp @ 606]
0012d0a0 024212f2 gklayout!nsInlineFrame::AddInlineMinWidth(class nsIRenderingContext * aRenderingContext = 0x05cc0818, struct nsIFrame::InlineMinWidthData * aData = 0x0012d144)+0x19 [d:\moz\mozilla\layout\generic\nsinlineframe.cpp @ 228]
0012d0dc 024279f9 gklayout!nsContainerFrame::DoInlineIntrinsicWidth(class nsIRenderingContext * aRenderingContext = 0x05cc0818, struct nsIFrame::InlineIntrinsicWidthData * aData = 0x0012d144, nsLayoutUtils::IntrinsicWidthType aType = MIN_WIDTH (0))+0x142 [d:\moz\mozilla\layout\generic\nscontainerframe.cpp @ 606]
0012d0f4 0242a883 gklayout!nsInlineFrame::AddInlineMinWidth(class nsIRenderingContext * aRenderingContext = 0x05cc0818, struct nsIFrame::InlineMinWidthData * aData = 0x0012d144)+0x19 [d:\moz\mozilla\layout\generic\nsinlineframe.cpp @ 228]
0012d174 023d0645 gklayout!nsBlockFrame::GetMinWidth(class nsIRenderingContext * aRenderingContext = 0x05cc0818)+0x273 [d:\moz\mozilla\layout\generic\nsblockframe.cpp @ 675]
0012d20c 02527832 gklayout!nsLayoutUtils::IntrinsicForContainer(class nsIRenderingContext * aRenderingContext = 0x05cc0818, class nsIFrame * aFrame = 0x05c1c548, nsLayoutUtils::IntrinsicWidthType aType = MIN_WIDTH (0))+0x145 [d:\moz\mozilla\layout\base\nslayoututils.cpp @ 1404]

Possibly this is a regression, but I do not build debug builds that often, so I cannot tell.

Comment 1

[Frank Wein [:mcsmurf]]

Masayuki: You added the code in nsLineBreaker.cpp (which leads to the assert) in Bug 255990. Can you take a look at this?

Comment 2

[Frank Wein [:mcsmurf]]

> 0012a6b0 024088d2 gklayout!nsLineBreaker::AppendText(class nsIAtom * aLangGroup
= 0x033202e8, unsigned char * aText = 0x05c70f60 "mZdiff:+224 (+229/-5)???",
unsigned int aLength = 0x15, unsigned int aFlags = 3, class nsILineBreakSink *
aSink = 0x00000000)+0x317 [d:\moz\mozilla\content\base\src\nslinebreaker.cpp @
277]

BTW: For the tinderbox page I get two asserts, once with line 273 in stack and once with line 277 as above.

Comment 3

[Eli Friedman]

I originally filed this as security-sensitive, because it's an array out-of-bounds access; there isn't anything especially interesting in the other bug, though...

Comment 4

[Scott MacGregor]

This assertion just started showing up in mail recently too. Especially in mail compose where it fires on almost every character I type. Bug 255990 looks like it could be a likely cause.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'll check this ASAP, sorry.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

fix.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why don't you just add "&& aSink" to "if (aFlags & BREAK_ALLOW_INSIDE)"?

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #7)
> Why don't you just add "&& aSink" to "if (aFlags & BREAK_ALLOW_INSIDE)"?

Roc, your code (for PRUnichar*) is not so. What means that aSink is null?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

When aSink is null, the break data is not needed; we're only calling the linebreaker to give it text it might need for context. Therefore the break array is not needed.

In the PRUnichar code, I always allocate the array even when it's not needed. That should probably be changed to match the PRUint8 code.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv2.0

Thanks. Let's take this patch.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+    if (aSink) {
+      breakState[offset] = mAfterSpace && !isSpace &&
+        (aFlags & (offset == 0 ? BREAK_ALLOW_INITIAL : BREAK_ALLOW_INSIDE));
+      mAfterSpace = isSpace;
+    }

This is not right. mAfterSpace must be set even if aSink is null. Otherwise later breaks will change depending on whether we provided a sink.

+        if (aSink && aFlags & BREAK_ALLOW_INSIDE) {

I prefer parens around the & expression.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv2.1

er, sorry for the mistake.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in.