16054 2007-11-19 12:35:34 -0800 [GTK] Crash when GlyphPage::fill is called with more than 256 bytes of data 2007-11-19 21:46:39 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Mac OS X 10.4 RESOLVED FIXED Gtk P2 Normal --- 1 dougt webkit-unassigned alp oldest_to_newest 61707 0 dougt 2007-11-19 12:35:34 -0800 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. 61708 1 17405 dougt 2007-11-19 12:39:50 -0800 Created attachment 17405 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. 61726 2 alp 2007-11-19 17:57:05 -0800 *** Bug 14446 has been marked as a duplicate of this bug. *** 61727 3 alp 2007-11-19 17:57:47 -0800 (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. 61728 4 17405 alp 2007-11-19 18:38:35 -0800 Comment on attachment 17405 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. 61729 5 alp 2007-11-19 18:41:19 -0800 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. 61741 6 ddkilzer 2007-11-19 21:46:39 -0800 It's too bad there isn't a way to share the common code. 17405 2007-11-19 12:39:50 -0800 2007-11-19 18:38:35 -0800 not a real fix crashfix.txt text/plain 978 dougt SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vR2x5cGhQYWdlVHJlZU5vZGUuaAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBXZWJDb3JlL3BsYXRmb3JtL0dseXBoUGFnZVRyZWVOb2RlLmggICAgICAgIChyZXZpc2lvbiAy NzkxMCkKKysrIFdlYkNvcmUvcGxhdGZvcm0vR2x5cGhQYWdlVHJlZU5vZGUuaCAgICAgICAgKHdv cmtpbmcgY29weSkKQEAgLTc4LDYgKzc4LDcgQEAKICAgICB9CiAgICAgdm9pZCBzZXRHbHlwaERh dGFGb3JJbmRleCh1bnNpZ25lZCBpbmRleCwgR2x5cGggZywgY29uc3QgRm9udERhdGEqIGYpCiAg ICAgeworICAgICAgICBBU1NFUlQoaW5kZXggPCBzaXplKTsKICAgICAgICAgbV9nbHlwaHNbaW5k ZXhdLmdseXBoID0gZzsKICAgICAgICAgbV9nbHlwaHNbaW5kZXhdLmZvbnREYXRhID0gZjsKICAg ICB9CkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL2d0ay9HbHlwaFBhZ2VUcmVlTm9kZUd0ay5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9ndGsvR2x5cGhQYWdlVHJlZU5vZGVHdGsu Y3BwICAgICAgIChyZXZpc2lvbiAyNzkxMCkKKysrIFdlYkNvcmUvcGxhdGZvcm0vZ3RrL0dseXBo UGFnZVRyZWVOb2RlR3RrLmNwcCAgICAgICAod29ya2luZyBjb3B5KQpAQCAtNDEsNiArNDEsOSBA QAogICAgIGlmICghZmFjZSkKICAgICAgICAgcmV0dXJuIGZhbHNlOwoKKyAgICBpZiAoYnVmZmVy TGVuZ3RoID4gMjU2KQorICAgICAgICByZXR1cm4gZmFsc2U7CisKICAgICBib29sIGhhdmVHbHlw aHMgPSBmYWxzZTsKICAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgYnVmZmVyTGVuZ3RoOyBp KyspIHsKICAgICAgICAgR2x5cGggZ2x5cGggPSBGY0ZyZWVUeXBlQ2hhckluZGV4KGZhY2UsIGJ1 ZmZlcltpXSk7