Closed Bug 459968 Opened 12 years ago Closed 12 years ago
Text Run::Get Adjusted Spacing Array "Attempting to allocate excessively large array" with pre-line
Loading the testcase causes loads of badness: ###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file gfx/thebes/src/gfxSkipChars.cpp, line 92 ###!!! ASSERTION: Text run does not map enough text for our reflow: 'gfxSkipCharsIterator(iter).ConvertOriginalToSkipped(offset + length) <= mTextRun->GetLength()', file layout/generic/nsTextFrameThebes.cpp, line 5768 ###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file gfx/thebes/src/gfxSkipChars.cpp, line 92 ###!!! ASSERTION: Substring out of range: 'aStart + aMaxLength <= mCharacterCount', file gfx/thebes/src/gfxFont.cpp, line 1960 ###!!! ASSERTION: Substring out of range: 'aStart + aLength <= mCharacterCount', file gfx/thebes/src/gfxFont.cpp, line 1911 ###!!! ASSERTION: Bad offset looking for glyphrun: 'aOffset <= mCharacterCount', file gfx/thebes/src/gfxFont.cpp, line 2158 ###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 69 ###!!! ABORT: file nsTArray.cpp, line 70 (gdb) f 9 #9 0x018a8fea in gfxTextRun::GetAdjustedSpacingArray (this=0x1c9f0790, aStart=6, aEnd=4, aProvider=0xbfffa83c, aSpacingStart=6, aSpacingEnd=4, aSpacing=0xbfff91f4) at /Users/jruderman/central/gfx/thebes/src/gfxFont.cpp:1560 1560 if (!aSpacing->AppendElements(aEnd - aStart)) (gdb) p aEnd $1 = 4 (gdb) p aStart $2 = 6
"Attempting to allocate excessively large array" is only an abort in my local tree. Should it be an abort for all debug-build users? It seems to lead to buffer overflows and stack corruption more often than not.
This didn't crash for me using a trunk debug build so it may be fixed there. I'll attach the assertions I still see, though. Jesse, any idea on a severity rating? Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b1pre) Gecko/20081023100737 Minefield/3.1b1pre
(In reply to comment #2) > This didn't crash for me using a trunk debug build I only tested it on Linux though, and just realized this was filed as a Mac bug. Doh.
You're right, the only reason it "crashes" for me is that I have "Attempting to allocate excessively large array" set to abort. Without that, it doesn't crash. Nothing Mac-specific about that :)
The other assertions still scare me, though, because they sound buffer-overflowy .
roc agrees that it looks scary and volunteered to try to fix it.
Assignee: nobody → roc
This is all integer overflow stuff. This patch fixes some of those issues by clamping conversions from float to nscoord and by using NSCoordSaturatingAdd in a few places. With this patch, we assert like this: ###!!! ASSERTION: shouldn't have unconstrained widths anymore: 'psd->mRightEdge != NS_UNCONSTRAINEDSIZE', file /Users/roc/mozilla-trunk/layout/generic/nsLineLayout.cpp, line 2411 ###!!! ASSERTION: should no longer be using unconstrained widths: 'aWidth != NS_UNCONSTRAINEDSIZE', file /Users/roc/mozilla-trunk/layout/generic/nsLineLayout.cpp, line 179 because widths have been clamped to nscoord_MIN/nscoord_MAX. I'm not sure what to do about that. The assertions are innocuous, maybe this long after the reflow branch landing, we should take them out?
Whiteboard: [sg:critical?] → [sg:critical?][needs review]
Comment on attachment 346388 [details] [diff] [review] fix >- return NSToCoordRound(aValue.GetFloatValue() * >+ return NSToCoordRoundWithClamp(aValue.GetFloatValue() * > NS_ceil(aPresContext->AppUnitsPerDevPixel() * > zeroWidth)); Maybe fix up the indentation here if there's a nice way to do it without crossing the 80th column. r+sr=dbaron
Whiteboard: [sg:critical?][needs review] → [sg:critical?][needs landing]
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Initially landed on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/57e6a1e9041e http://hg.mozilla.org/mozilla-central/rev/026147c91538 and then backed out due to crashes caused by bug 430332: http://hg.mozilla.org/mozilla-central/rev/9eaf91dad68c and then relanded on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/54946665b7c0 and thus fixed on both mozilla-central and releases/mozilla-1.9.1.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?][needs landing] → [sg:critical?]
Target Milestone: --- → mozilla1.9.1b3
I tried to verify the fix on 1.9.1 but I'm also not able to reproduce the crash with an older build from November last year and the given testcase. Jesse, do I have to meet special conditions?
OS: Mac OS X → All
Hardware: x86 → All
Are you using a debug build? In comment 0, I incorrectly identified this bug as an abort.
Ok, the crash is fixed. At least on trunk. Have to build a Shiretoko debug build first. Jesse, I still can see the following assertion: ###!!! ASSERTION: bad width: 'Not Reached', file /data/mozilla/build/mozilla-central/mozilla/layout/generic/nsLineLayout.cpp, line 182 nsLineLayout::BeginLineReflow(int, int, int, int, int, int) (nsUnicharUtils.cpp:) nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, int*, LineReflowStatus*, int) (nsUnicharUtils.cpp:) nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, int*) (nsUnicharUtils.cpp:) nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) (nsUnicharUtils.cpp:) nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) (nsUnicharUtils.cpp:) There is no bug filed on that. I believe I should do that. It hasn't be security related, right?
Status: RESOLVED → VERIFIED
If you're not satisfied with bug 371561, bug 323381, or bug 387080, sure. If you can reduce the testcase significantly, you can file a new public bug report with the new testcase. If you can't reduce the testcase much, I guess the best thing to do is to file a new bug but refer to the testcase in this bug.
Oh, bug 323381 does it cover. Thanks for pointing it out. It's summary was a bit too short, so I wasn't able to find it.
Also no crash anymore with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090211 Shiretoko/3.1b3pre ID:20090211221038
Landed the crashtest: https://hg.mozilla.org/integration/mozilla-inbound/rev/0fbbab9a6a0e
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.