Bug 16054 – [GTK] Crash when GlyphPage::fill is called with more than 256 bytes of data

• Component: WebKitGTK
• Status: RESOLVED FIXED

[doug turner]

when loading a cached version of of a webpage (sorry haven't reduced to a testcase), GtkLauncher crashes as follows:


#0  0xb7f90410 in ?? ()
#1  0xbfc921cc in ?? ()
#2  0x00000006 in ?? ()
#3  0x00000f00 in ?? ()
#4  0xb59ab811 in raise () from /lib/tls/i686/cmov/libc.so.6
#5  0xb59acfb9 in abort () from /lib/tls/i686/cmov/libc.so.6
#6  0xb59e0e0a in __fsetlocking () from /lib/tls/i686/cmov/libc.so.6
#7  0xb59e869f in mallopt () from /lib/tls/i686/cmov/libc.so.6
#8  0xb59e8742 in free () from /lib/tls/i686/cmov/libc.so.6
#9  0xb5b933b1 in operator delete () from /usr/lib/libstdc++.so.6
#10 0xb77455a5 in WTF::RefCounted<WebCore::GlyphPage>::deref (this=0x8651040)
    at ../../../JavaScriptCore/wtf/RefCounted.h:52
#11 0xb77456c8 in WTF::RefPtr<WebCore::GlyphPage>::operator= (this=0x83af3fc, optr=0x0)
    at ../../../JavaScriptCore/wtf/RefPtr.h:110
#12 0xb7744a39 in WebCore::GlyphPageTreeNode::initializePage (this=0x83af3f8, fontData=0x82783b0,
    pageNumber=2969) at ../../../WebCore/platform/GlyphPageTreeNode.cpp:150
#13 0xb7744ea9 in WebCore::GlyphPageTreeNode::getChild (this=0x837b520, fontData=0x82783b0,
    pageNumber=2969) at ../../../WebCore/platform/GlyphPageTreeNode.cpp:241
#14 0xb77464d1 in WebCore::GlyphPageTreeNode::getRootChild (fontData=0x82783b0, pageNumber=2969)
    at ../../../WebCore/platform/GlyphPageTreeNode.h:130
#15 0xb774b9ca in WebCore::Font::glyphDataForCharacter (this=0x845d038, c=760114, mirror=false)
    at ../../../WebCore/platform/Font.cpp:374
#16 0xb774ca0f in WebCore::WidthIterator::advance (this=0xbfc92b34, offset=5, glyphBuffer=0x0)
    at ../../../WebCore/platform/Font.cpp:158
#17 0xb774d125 in WebCore::Font::floatWidthForSimpleText (this=0x845d038, run=@0xbfc92bf0,
    style=@0xbfc92bdc, glyphBuffer=0x0) at ../../../WebCore/platform/Font.cpp:705
#18 0xb774d179 in WebCore::Font::floatWidth (this=0x845d038, run=@0xbfc92bf0, style=@0xbfc92bdc)
    at ../../../WebCore/platform/Font.cpp:698
#19 0xb774d1cf in WebCore::Font::width (this=0x845d038, run=@0xbfc92bf0, style=@0xbfc92bdc)
    at ../../../WebCore/platform/Font.cpp:518
#20 0xb770c574 in WebCore::RenderText::widthFromCache (this=0x845e9ac, f=@0x845d038, start=0, len=5,
    xPos=0) at ../../../WebCore/rendering/RenderText.cpp:415
#21 0xb770a486 in WebCore::RenderText::calcPrefWidths (this=0x845e9ac, leadWidth=0)
    at ../../../WebCore/rendering/RenderText.cpp:616
#22 0xb770a9bf in WebCore::RenderText::trimmedPrefWidths (this=0x845e9ac, leadWidth=0,
    beginMinW=@0xbfc92da8, beginWS=@0xbfc92da3, endMinW=@0xbfc92da4, endWS=@0xbfc92da2,
    hasBreakableChar=@0xbfc92daf, hasBreak=@0xbfc92dae, beginMaxW=@0xbfc92d9c, endMaxW=@0xbfc92d98,
    minW=@0xbfc92dbc, maxW=@0xbfc92db8, stripFrontSpaces=@0xbfc92ddb)
    at ../../../WebCore/rendering/RenderText.cpp:430
#23 0xb7678ac7 in WebCore::RenderBlock::calcInlinePrefWidths (this=0x845e604)
    at ../../../WebCore/rendering/RenderBlock.cpp:3763
#24 0xb7678f76 in WebCore::RenderBlock::calcPrefWidths (this=0x845e604)
    at ../../../WebCore/rendering/RenderBlock.cpp:3419
#25 0xb76f1bc5 in WebCore::RenderTableCell::calcPrefWidths (this=0x845e604)
    at ../../../WebCore/rendering/RenderTableCell.cpp:108
#26 0xb7654eb6 in WebCore::AutoTableLayout::recalcColumn (this=0x845c3e0, effCol=3)
    at ../../../WebCore/rendering/AutoTableLayout.cpp:84
#27 0xb7655682 in WebCore::AutoTableLayout::fullRecalc (this=0x845c3e0)
    at ../../../WebCore/rendering/AutoTableLayout.cpp:214
#28 0xb76569a4 in WebCore::AutoTableLayout::calcPrefWidths (this=0x845c3e0, minWidth=@0x845c354,
    maxWidth=@0x845c358) at ../../../WebCore/rendering/AutoTableLayout.cpp:252
#29 0xb76f4ca5 in WebCore::RenderTable::calcPrefWidths (this=0x845c314)
    at ../../../WebCore/rendering/RenderTable.cpp:535
#30 0xb768a523 in WebCore::RenderBox::minPrefWidth (this=0x845c314)
    at ../../../WebCore/rendering/RenderBox.cpp:183
#31 0xb7675b91 in WebCore::RenderBlock::calcBlockPrefWidths (this=0x845bc9c)
    at ../../../WebCore/rendering/RenderBlock.cpp:3888
#32 0xb7678f83 in WebCore::RenderBlock::calcPrefWidths (this=0x845bc9c)
    at ../../../WebCore/rendering/RenderBlock.cpp:3421
#33 0xb76f1bc5 in WebCore::RenderTableCell::calcPrefWidths (this=0x845bc9c)
    at ../../../WebCore/rendering/RenderTableCell.cpp:108
#34 0xb7654eb6 in WebCore::AutoTableLayout::recalcColumn (this=0x849de80, effCol=2)
    at ../../../WebCore/rendering/AutoTableLayout.cpp:84
#35 0xb7655682 in WebCore::AutoTableLayout::fullRecalc (this=0x849de80)
    at ../../../WebCore/rendering/AutoTableLayout.cpp:214
#36 0xb76569a4 in WebCore::AutoTableLayout::calcPrefWidths (this=0x849de80, minWidth=@0x849ddf4,
    maxWidth=@0x849ddf8) at ../../../WebCore/rendering/AutoTableLayout.cpp:252
#37 0xb76f4ca5 in WebCore::RenderTable::calcPrefWidths (this=0x849ddb4)
    at ../../../WebCore/rendering/RenderTable.cpp:535
#38 0xb768a523 in WebCore::RenderBox::minPrefWidth (this=0x849ddb4)
    at ../../../WebCore/rendering/RenderBox.cpp:183
#39 0xb76f6bb3 in WebCore::RenderTable::calcWidth (this=0x849ddb4)
    at ../../../WebCore/rendering/RenderTable.cpp:224
#40 0xb76f5911 in WebCore::RenderTable::layout (this=0x849ddb4)
    at ../../../WebCore/rendering/RenderTable.cpp:277
#41 0xb765ff49 in WebCore::RenderObject::layoutIfNeeded (this=0x849ddb4)
    at ../../../WebCore/rendering/RenderObject.h:484
#42 0xb7682c0e in WebCore::RenderBlock::layoutBlockChildren (this=0x8356f34, relayoutChildren=true,
    maxFloatBottom=@0xbfc93810) at ../../../WebCore/rendering/RenderBlock.cpp:1224
#43 0xb7683669 in WebCore::RenderBlock::layoutBlock (this=0x8356f34, relayoutChildren=true)
    at ../../../WebCore/rendering/RenderBlock.cpp:584
#44 0xb7672470 in WebCore::RenderBlock::layout (this=0x8356f34)
    at ../../../WebCore/rendering/RenderBlock.cpp:492
#45 0xb765ff49 in WebCore::RenderObject::layoutIfNeeded (this=0x8356f34)
    at ../../../WebCore/rendering/RenderObject.h:484
#46 0xb7682c0e in WebCore::RenderBlock::layoutBlockChildren (this=0x8247934, relayoutChildren=true,
    maxFloatBottom=@0xbfc93a70) at ../../../WebCore/rendering/RenderBlock.cpp:1224
#47 0xb7683669 in WebCore::RenderBlock::layoutBlock (this=0x8247934, relayoutChildren=true)
    at ../../../WebCore/rendering/RenderBlock.cpp:584
#48 0xb7672470 in WebCore::RenderBlock::layout (this=0x8247934)
    at ../../../WebCore/rendering/RenderBlock.cpp:492
#49 0xb765ff49 in WebCore::RenderObject::layoutIfNeeded (this=0x8247934)
    at ../../../WebCore/rendering/RenderObject.h:484
#50 0xb7682c0e in WebCore::RenderBlock::layoutBlockChildren (this=0x80b5dac, relayoutChildren=true,
    maxFloatBottom=@0xbfc93cd0) at ../../../WebCore/rendering/RenderBlock.cpp:1224
#51 0xb7683669 in WebCore::RenderBlock::layoutBlock (this=0x80b5dac, relayoutChildren=true)
    at ../../../WebCore/rendering/RenderBlock.cpp:584
#52 0xb7672470 in WebCore::RenderBlock::layout (this=0x80b5dac)
    at ../../../WebCore/rendering/RenderBlock.cpp:492
#53 0xb765ff49 in WebCore::RenderObject::layoutIfNeeded (this=0x80b5dac)
    at ../../../WebCore/rendering/RenderObject.h:484
#54 0xb7682c0e in WebCore::RenderBlock::layoutBlockChildren (this=0x80afa84, relayoutChildren=true,
    maxFloatBottom=@0xbfc93f30) at ../../../WebCore/rendering/RenderBlock.cpp:1224
#55 0xb7683669 in WebCore::RenderBlock::layoutBlock (this=0x80afa84, relayoutChildren=true)
    at ../../../WebCore/rendering/RenderBlock.cpp:584
#56 0xb7672470 in WebCore::RenderBlock::layout (this=0x80afa84)
    at ../../../WebCore/rendering/RenderBlock.cpp:492
#57 0xb771381f in WebCore::RenderView::layout (this=0x80afa84)
    at ../../../WebCore/rendering/RenderView.cpp:111
#58 0xb75fc2c0 in WebCore::FrameView::layout (this=0x80a6700, allowSubtree=true)
    at ../../../WebCore/page/FrameView.cpp:434
#59 0xb75fc7f1 in WebCore::FrameView::layoutTimerFired (this=0x80a6700)
    at ../../../WebCore/page/FrameView.cpp:684
#60 0xb75fda9f in WebCore::Timer<WebCore::FrameView>::fired (this=0x80a67d4)
    at ../../../WebCore/platform/Timer.h:98

In GlyphPage::fill() in WebCore/platform/gtk/GlyphPageTreeNodeGtk.cpp, we loop over the bufferLength and repeatedly call setGlyphDataForIndex with each glyph/font data.

The problem with this is that setGlyphDataForIndex defined in WebCore/platform/GlyphPageTreeNode.h basically pokes these values into a a fixed array of size 256.

[doug turner]

Created attachment 17405 [details]
not a real fix

I am not sure if this is the right approach.. in fact, i am pretty sure that it isn't.

The ASSERT should be checked in so that we do not hit this problem in the future.  As for the second part, it seams that the caller to fill() needs to be notified that only some of the data could be processed.

[Alp Toker]

*** Bug 14446 has been marked as a duplicate of this bug. ***

[Alp Toker]

(In reply to comment #1)
> Created an attachment (id=17405) [edit]
> not a real fix
> 
> I am not sure if this is the right approach.. in fact, i am pretty sure that it
> isn't.
> 

This is straight from the Windows port (WebCore/platform/win/GlyphPageTreeNodeWin.cpp):


bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const FontData* fontData)
{
    // The bufferLength will be greater than the glyph page size if the buffer has Unicode supplementary characters.
    // We won't support this for now.
    if (bufferLength > GlyphPage::size)
        return false;

    bool haveGlyphs = false;
    CGGlyph localGlyphBuffer[GlyphPage::size];
    wkGetGlyphs(fontData->platformData().cgFont(), buffer, localGlyphBuffer, bufferLength);
    for (unsigned i = 0; i < GlyphPage::size; i++) {
        Glyph glyph = localGlyphBuffer[i];
        if (!glyph)
            setGlyphDataForIndex(i, 0, 0);
        else {
            setGlyphDataForIndex(i, glyph, fontData);
            haveGlyphs = true;
        }
    }
    return haveGlyphs;
}

Looks like this strategy is just fine for now, and fixes a real crasher bug. Nice catch!

Looking at this line from Win:

    for (unsigned i = 0; i < GlyphPage::size; i++)

We do this instead:

    for (unsigned i = 0; i < bufferLength; i++)

Both yield the same result AFAICT. I wonder which is more correct.

[Alp Toker]

Comment on attachment 17405 [details]
not a real fix

This fix is good enough and fixes some major crashers. The Win port does the same thing.

The return needs to be moved before the face is locked, but otherwise looks good.

[Alp Toker]

Landed in r27914. If there are further issues we can open a new bug, or RFE for better Unicode handling and/or complex text support.

[David Kilzer (:ddkilzer)]

It's too bad there isn't a way to share the common code.