Bug 21953 – Small Caps font crashes webkit

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Jon@Chromium]

This bug originated in the Chromium bug tracker see http://code.google.com/p/chromium/issues/detail?id=1491

The bug does not impact IE7 or FF3.  It crashes Safari 3.1 for Windows and Chromium.  The stack trace attached.

Stack trace :
(142c.10e4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=e8a0e17f ebx=00cbce2c ecx=00cbce2c edx=00000003 esi=00000000 edi=00000000
eip=0131eed2 esp=00b7f2e8 ebp=00b7f370 iopl=0         nv up ei pl zr na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010246
chrome_1000000!WebCore::SimpleFontData::smallCapsFontData+0x17:
0131eed2 39b754040000    cmp     dword ptr [edi+454h],esi ds:0023:00000454=????????
2:027> g
(142c.10e4): Access violation - code c0000005 (!!! second chance !!!)
eax=e8a0e17f ebx=00cbce2c ecx=00cbce2c edx=00000003 esi=00000000 edi=00000000
eip=0131eed2 esp=00b7f2e8 ebp=00b7f370 iopl=0         nv up ei pl zr na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000246
chrome_1000000!WebCore::SimpleFontData::smallCapsFontData+0x17:
0131eed2 39b754040000    cmp     dword ptr [edi+454h],esi ds:0023:00000454=????????
2:027> k
ChildEBP RetAddr  
00b7f370 010a970b chrome_1000000!WebCore::SimpleFontData::smallCapsFontData+0x17
[c:\b\slave\chrome-official-2\build\src\webkit\port\platform\graphics\simplefontdatawin.cpp
@ 135]
00b7f3a0 010a8f3f chrome_1000000!WebCore::Font::glyphDataForCharacter+0x24b
[c:\b\slave\chrome-official-2\build\src\webkit\pending\font.cpp @ 460]
00b7f3f0 010a9c88 chrome_1000000!WebCore::WidthIterator::advance+0xf2
[c:\b\slave\chrome-official-2\build\src\webkit\pending\font.cpp @ 162]
00b7f428 010a9c58 chrome_1000000!WebCore::Font::floatWidthForSimpleText+0x1e
[c:\b\slave\chrome-official-2\build\src\webkit\pending\font.cpp @ 718]
00b7f438 010a980b chrome_1000000!WebCore::Font::floatWidth+0x41
[c:\b\slave\chrome-official-2\build\src\webkit\pending\font.cpp @ 710]
00b7f440 010ffbf2 chrome_1000000!WebCore::Font::width+0x9
[c:\b\slave\chrome-official-2\build\src\webkit\pending\font.cpp @ 515]
00b7f4dc 010ff393 chrome_1000000!WebCore::RenderText::calcPrefWidths+0x4e9
[c:\b\slave\chrome-official-2\build\src\webkit\pending\rendertext.cpp @ 649]
00b7f520 010d4a5f chrome_1000000!WebCore::RenderText::trimmedPrefWidths+0x38
[c:\b\slave\chrome-official-2\build\src\webkit\pending\rendertext.cpp @ 463]
00b7f5ac 010d438a chrome_1000000!WebCore::RenderBlock::calcInlinePrefWidths+0x300
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 3754]
00b7f5cc 010e34e6 chrome_1000000!WebCore::RenderBlock::calcPrefWidths+0x92
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 3432]
00b7f5d4 010e580b chrome_1000000!WebCore::RenderBox::minPrefWidth+0x11
[c:\b\slave\chrome-official-2\build\src\third_party\webkit\webcore\rendering\renderbox.cpp
@ 179]
00b7f5f4 010e5679 chrome_1000000!WebCore::RenderBox::calcWidthUsing+0x86
[c:\b\slave\chrome-official-2\build\src\third_party\webkit\webcore\rendering\renderbox.cpp
@ 1177]
00b7f628 010ce457 chrome_1000000!WebCore::RenderBox::calcWidth+0x25e
[c:\b\slave\chrome-official-2\build\src\third_party\webkit\webcore\rendering\renderbox.cpp
@ 1116]
00b7f6a4 010ce31f chrome_1000000!WebCore::RenderBlock::layoutBlock+0x108
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 532]
00b7f6b0 011350f9 chrome_1000000!WebCore::RenderBlock::layout+0x17
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 495]
00b7f79c 010ce5ae chrome_1000000!WebCore::RenderBlock::layoutInlineChildren+0x220
[c:\b\slave\chrome-official-2\build\src\webkit\pending\bidi.cpp @ 884]
00b7f824 010ce31f chrome_1000000!WebCore::RenderBlock::layoutBlock+0x25f
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 583]
00b7f830 010cf872 chrome_1000000!WebCore::RenderBlock::layout+0x17
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 495]
00b7f89c 010ce5be chrome_1000000!WebCore::RenderBlock::layoutBlockChildren+0x32a
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 1233]
00b7f924 010ce31f chrome_1000000!WebCore::RenderBlock::layoutBlock+0x26f
[c:\b\slave\chrome-official-2\build\src\webkit\pending\renderblock.cpp @ 587]

[Mihnea Ovidenie]

Hello,

I was able to reproduce the problem only on WindowsXP, never on Mac Leopard.

Here is a trace for the problem using the latest WebKit debug build on WinXP:

>	WebKit.dll!WebCore::SimpleFontData::smallCapsFontData(const WebCore::FontDescription & fontDescription={...})  Line 101 + 0x3 bytes	C++
 	WebKit.dll!WebCore::Font::glyphDataForCharacter(int c=581, bool mirror=false, bool forceSmallCaps=false)  Line 243 + 0xc bytes	C++
 	WebKit.dll!WebCore::WidthIterator::advance(int offset=12, WebCore::GlyphBuffer * glyphBuffer=0x00000000)  Line 112 + 0x14 bytes	C++
 	WebKit.dll!WebCore::Font::floatWidthForSimpleText(const WebCore::TextRun & run={...}, WebCore::GlyphBuffer * glyphBuffer=0x00000000)  Line 525	C++
 	WebKit.dll!WebCore::Font::floatWidth(const WebCore::TextRun & run={...})  Line 503 + 0xe bytes	C++
 	WebKit.dll!WebCore::Font::width(const WebCore::TextRun & run={...})  Line 298 + 0xc bytes	C++
 	WebKit.dll!WebCore::RenderText::widthFromCache(const WebCore::Font & f={...}, int start=2, int len=12, int xPos=17)  Line 393	C++
 	WebKit.dll!WebCore::RenderText::calcPrefWidths(int leadWidth=0)  Line 600 + 0x1b bytes	C++
 	WebKit.dll!WebCore::RenderText::trimmedPrefWidths(int leadWidth=0, int & beginMinW=-858993460, bool & beginWS=true, int & endMinW=-858993460, bool & endWS=true, bool & hasBreakableChar=true, bool & hasBreak=true, int & beginMaxW=-858993460, int & endMaxW=-858993460, int & minW=0, int & maxW=0, bool & stripFrontSpaces=true)  Line 407 + 0x16 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::calcInlinePrefWidths()  Line 4017	C++
 	WebKit.dll!WebCore::RenderBlock::calcPrefWidths()  Line 3698	C++
 	WebKit.dll!WebCore::RenderBox::minPrefWidth()  Line 215 + 0x12 bytes	C++
 	WebKit.dll!WebCore::RenderBox::calcWidthUsing(WebCore::WidthType widthType=Width, int cw=950)  Line 1368 + 0x12 bytes	C++
 	WebKit.dll!WebCore::RenderBox::calcWidth()  Line 1307 + 0xe bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlock(bool relayoutChildren=false)  Line 604 + 0x12 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layout()  Line 565 + 0x14 bytes	C++
 	WebKit.dll!WebCore::RenderObject::layoutIfNeeded()  Line 511 + 0x30 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutInlineChildren(bool relayoutChildren=true, int & repaintTop=0, int & repaintBottom=0)  Line 848	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlock(bool relayoutChildren=true)  Line 657	C++
 	WebKit.dll!WebCore::RenderBlock::layout()  Line 565 + 0x14 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlockChildren(bool relayoutChildren=true, int & maxFloatBottom=142)  Line 1334 + 0x18 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlock(bool relayoutChildren=true)  Line 661	C++
 	WebKit.dll!WebCore::RenderBlock::layout()  Line 565 + 0x14 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlockChildren(bool relayoutChildren=true, int & maxFloatBottom=0)  Line 1334 + 0x18 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlock(bool relayoutChildren=true)  Line 661	C++
 	WebKit.dll!WebCore::RenderBlock::layout()  Line 565 + 0x14 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlockChildren(bool relayoutChildren=true, int & maxFloatBottom=0)  Line 1334 + 0x18 bytes	C++
 	WebKit.dll!WebCore::RenderBlock::layoutBlock(bool relayoutChildren=true)  Line 661	C++
 	WebKit.dll!WebCore::RenderBlock::layout()  Line 565 + 0x14 bytes	C++
 	WebKit.dll!WebCore::RenderView::layout()  Line 121	C++
 	WebKit.dll!WebCore::FrameView::layout(bool allowSubtree=true)  Line 528 + 0x12 bytes	C++
 	WebKit.dll!WebCore::FrameView::layoutTimerFired(WebCore::Timer<WebCore::FrameView> * __formal=0x01912c70)  Line 736	C++
 	WebKit.dll!WebCore::Timer<WebCore::FrameView>::fired()  Line 99 + 0x29 bytes	C++
 	WebKit.dll!WebCore::TimerBase::fireTimers(double fireTime=1225386376.5731516, const WTF::Vector<WebCore::TimerBase *,0> & firingTimers={...})  Line 347 + 0xf bytes	C++
 	WebKit.dll!WebCore::TimerBase::sharedTimerFired()  Line 368 + 0x12 bytes	C++
 	WebKit.dll!WebCore::TimerWindowWndProc(HWND__ * hWnd=0x00130da0, unsigned int message=49965, unsigned int wParam=0, long lParam=0)  Line 102 + 0x8 bytes	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x28 bytes	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
 	user32.dll!_DispatchMessageWorker@8()  + 0xdc bytes	
 	user32.dll!_DispatchMessageW@4()  + 0xf bytes	
 	Safari.exe!RSSPrefsDlg::`vector deleting destructor'()  + 0x20f bytes	
 	Safari.exe!run()  + 0x9d bytes	
 	Safari.exe!_wWinMain@16()  + 0x34b bytes	
 	Safari.exe!_free()  + 0x1a1 bytes	
 	kernel32.dll!_BaseProcessStart@4()  + 0x23 bytes	

In function const GlyphData& Font::glyphDataForCharacter(UChar32 c, bool mirror, bool forceSmallCaps) const, the following code snippet shows the access violation:

const SimpleFontData* characterFontData = FontCache::getFontDataForCharacters(*this, codeUnits, codeUnitsLength);
if (useSmallCapsFont)
    characterFontData = characterFontData->smallCapsFontData(m_fontDescription);

useSmallCapsFont is true while loading the above URL but characterFontData is 0 after calling getFontDataForCharacters, therefore the access violation occurs.

Hope that helps,
Mihnea

[Glenn Wilson]

Created attachment 24977 [details]
Test Reduction for bug 21953

Reduction attached.

Something with setting the charset to UTF-8 and the 'Ê' and 'Œ' characters together as small-caps.

Investigating further.

[Glenn Wilson]

Created attachment 25023 [details]
Possible patch to issue 21953

Here is a possible fix for this issue.

So, here's what I think is happening to cause a crash:

1.  Font calls FontCache:getFontCacheForCharacters, which gives SimpleFontData for a given set of characters. 
2.  FontCache::getFontCacheForCharacters eventually gets the SimpleFontData that works for the first character 'Ê', but does not have a character mapped for the next character 'Œ'.
3.  getFontCacheForCharacters ends up returning null, because the SimpleFontData it was trying to return doesn't contain all the characters.
4.  Font, seeing that it has the smallCaps flag, tries to call a method on the SimpleFontData that was returned null, and it crashes.

So, this crash was caused by any html that is trying to use the small-caps variant on a font that only contains the first character in a set of text, but not a subsequent character.

This change merely modifies Font to not call the small caps method if getFontCacheForCharacters returns null.  It was already doing a similar check immediately afterwards, so this seems like the right way to guard against this possibility.

[Glenn Wilson]

It is probably also worth noting that the reason why this happens in Windows only is that the default, "fallback" font on Windows is the "System" font, which does not contain a character for 'Œ' (0x00CE).

On OSX, the default font is "Geneva", I believe.  This font *does* have a character for 0x00CE.

[Eric Seidel (no email)]

Comment on attachment 25023 [details]
Possible patch to issue 21953

Hyatt or mitz are the font experts.  I expect you should be able to convince one of them to review this tiny patch for you.

Comment on attachment 25023 [details]
Possible patch to issue 21953

The code change is correct but the analysis in the bug and in the change log is wrong. The reduction contains only a single code point, U+028C (UTF-8 encoded as CA 8C). There is a font containing a glyph for this character, so there is no problem. However, its uppercase counterpart is U+0245, for which there happens to be no font containing a glyph, leading to characterFontData being 0. This is handled correctly in the non-small-caps case, but not in the small-caps case. Please revise the change log.

[Glenn Wilson]

Created attachment 25528 [details]
Improved possible patch for issue 21953

Here's a revised version with the correct descriptions in the Changelogs.

Thanks for the feedback!

Comment on attachment 25528 [details]
Improved possible patch for issue 21953

r=me
Thanks for fixing this crash!

[Darin Fisher (:fishd, Google)]

http://trac.webkit.org/changeset/38806