regression? crash [@ gfxWindowsFontGroup::WhichFontSupportsChar(nsTArray<nsRefPtr<FontEntry> > const&, unsigned int)] appears as top of stack for both FF and TB. No FF crashes prior to 3.1 aka 3.5, so perhaps a regression? For thunderbird, about 1/3-1/2 look to be crashes during startup. 99% windows, but here are two Mac crashes: TB bp-324d1f10-ed11-4e33-9830-94dc92090717 SM bp-c17887a3-fb93-44fc-bff3-81adf2091002 bp-02466c92-dc16-42a0-aa23-46e352091025 0 thunderbird.exe gfxWindowsFontGroup::WhichFontSupportsChar gfx/thebes/src/gfxWindowsFonts.cpp:2383 1 thunderbird.exe gfxWindowsFontGroup::WhichPrefFontSupportsChar gfx/thebes/src/gfxWindowsFonts.cpp:2513 2 thunderbird.exe gfxFontGroup::FindFontForChar gfx/thebes/src/gfxFont.cpp:1129 3 thunderbird.exe gfxFontGroup::ComputeRanges gfx/thebes/src/gfxFont.cpp:1182 4 thunderbird.exe gfxWindowsFontGroup::InitTextRunUniscribe gfx/thebes/src/gfxWindowsFonts.cpp:2607 5 thunderbird.exe gfxWindowsFontGroup::MakeTextRun gfx/thebes/src/gfxWindowsFonts.cpp:1444 6 thunderbird.exe TextRunWordCache::MakeTextRun gfx/thebes/src/gfxTextRunWordCache.cpp:683 7 thunderbird.exe gfxTextRunWordCache::MakeTextRun gfx/thebes/src/gfxTextRunWordCache.cpp:992 8 thunderbird.exe gfxTextRunCache::MakeTextRun gfx/thebes/src/gfxTextRunCache.cpp:89 9 thunderbird.exe nsThebesFontMetrics::AutoTextRun::AutoTextRun gfx/src/thebes/nsThebesFontMetrics.h:171 10 thunderbird.exe nsThebesFontMetrics::GetWidth gfx/src/thebes/nsThebesFontMetrics.cpp:340 11 thunderbird.exe nsThebesRenderingContext::GetWidthInternal gfx/src/thebes/nsThebesRenderingContext.cpp:869 12 thunderbird.exe nsRenderingContextImpl::GetWidth gfx/src/shared/nsRenderingContextImpl.cpp:170 13 thunderbird.exe nsLayoutUtils::GetStringWidth layout/base/nsLayoutUtils.cpp:2487 14 thunderbird.exe nsTreeBodyFrame::AdjustForCellText layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:1521 15 thunderbird.exe nsTreeBodyFrame::PaintText layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:3618 16 thunderbird.exe nsTreeBodyFrame::PaintCell layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:3306 17 thunderbird.exe nsTreeBodyFrame::PaintRow layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:3097 18 thunderbird.exe nsTreeBodyFrame::PaintTreeBody layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:2900 19 thunderbird.exe PaintTreeBody layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:2828 20 thunderbird.exe nsDisplayGeneric::Paint layout/base/nsDisplayList.h:875 21 thunderbird.exe nsDisplayList::Paint layout/base/nsDisplayList.cpp:313 22 thunderbird.exe nsDisplayClip::Paint layout/base/nsDisplayList.cpp:978 23 thunderbird.exe nsDisplayList::Paint layout/base/nsDisplayList.cpp:313 24 thunderbird.exe nsLayoutUtils::PaintFrame layout/base/nsLayoutUtils.cpp:1114 25 thunderbird.exe PresShell::Paint layout/base/nsPresShell.cpp:5775 26 thunderbird.exe nsViewManager::RenderViews view/src/nsViewManager.cpp:648 FF crashes with comments: bp-14bd7ab9-18db-41c3-89e7-db8202091017 bp-4b194b29-ceb9-4324-8afe-dfed62090904 Also appears in SM 2.0 http://crash-stats.mozilla.com/report/index/88e0b210-3e72-453b-b107-c5a592091012

From a quick look at this bug I'd argue the problem could be gfxWindowsFontGroup::FamilyListToArrayList, gfxWindowsFonts.cpp:1325, doesn't actually check if it actually finds the FontEntry, it just adds it to the array. That probably needs an if (fe.get()) around it. I'm not sure if this works but QA could try to put fonts in the pref font list which they know don't exist, and see if it chokes on that. This function would get called if you enter a character which doesn't exist in the normal font, and it starts looking through the pref and system fonts to see which character does support it. A good symbol for this case is the unicode 'Comet' http://www.fileformat.info/info/unicode/char/2604/index.htm, which only exists in a few fonts.

This is topcrash #3 in 3.5.4.

I bet Joe wants to own this.

I so don't! But I know someone who might.

If URLs would be helpful here, we can dig them out. Please let us know.

Sam, URL's would definitely help to try and reproduce this somehow.

(In reply to comment #1) > From a quick look at this bug I'd argue the problem could be > gfxWindowsFontGroup::FamilyListToArrayList, gfxWindowsFonts.cpp:1325, doesn't > actually check if it actually finds the FontEntry, it just adds it to the > array. That probably needs an if (fe.get()) around it. I'm not sure if this > works but QA could try to put fonts in the pref font list which they know don't > exist, and see if it chokes on that. This function would get called if you > enter a character which doesn't exist in the normal font, and it starts looking > through the pref and system fonts to see which character does support it. A > good symbol for this case is the unicode 'Comet' > http://www.fileformat.info/info/unicode/char/2604/index.htm, which only exists > in a few fonts. Hrm, played around with this a bit. In theory this should never happen since gfxWindowsPlatform::ResolveFontName should prevent any non existant font from ever beind added to the string list in FamilyListToArray. This the only way a null pointer could ever get into the list is if gfxFontFamily::FindFontForStyle returns nsnull.

Since it hasn't been mentioned in this bug yet... We didn't take many changes to this area of code in 1.9.1.4. The only two I can think of are bug 516709 and bug 496573.

(In reply to comment #9) > Since it hasn't been mentioned in this bug yet... > We didn't take many changes to this area of code in 1.9.1.4. The only two I can > think of are bug 516709 and bug 496573. So, bug 516709 is very interesting here. And I have a theory on what could happen. Although you'd need to introduce some font with a corrupted CMAP table somewhere to reproduce this, and I haven't quite figured out where. If some font family would be corrupted, CreateFontEntry will now return nsnull. Where it never used to. The FontFamily::FamilyAddStylesProc could then in theory add no font styles to the FontFamily (see gfxWindowsFonts.cpp:283). When this happens a call to FindFontForStyle could hit the assertion conditions at gfxFont.cpp:147 and then at gfxFont.cpp:298 or gfxFont.cpp:212, which obviously wouldn't go off, and it would return nsnull. This is behavior which is not allowed. Will propegate and cause gfxWindowsPlatform::FindFontEntry to return nsnull, and cause FamilyListToArray list to add a null font to the font entry list. Which would later cause this crash as it tries to traverse that list.

Since every other call to FindFontEntry null-checks the return value. It's probably a good idea to do that at this location too. Added a proposed patch for this.

As Bas notes, this looks like a regression from 516709. Crap, crap, crap. Probably needs to block 3.6b1.

I agree that we should get it fixed on mozilla-1.9.2 right-quick, but also think we can go to beta without it. Our current plan is to update the beta as early as next week, actually, so the fix should roll to users quickly.

I've been able to reproduce the issue using the following (quite farfetched) steps. Ofcourse this is unlikely to be what is happening in the wild. 1. Install the Swiss Neue font with corrupt CMAP table. 2. Add the following lines to your prefs.js: user_pref("font.default.x-western", "swiss-neue"); user_pref("font.name.swiss-neue.x-western", "Swiss Neue"); 3. Load the attached html which uses a unicode character.

So I think the patch here is correct, it will fix the problem. Running through the crash URLs, there are a large number of Chinese pages and many other non-English pages. Of the fonts listed as default pref fonts in all.js, only a subset of these are available with an English version of Windows. Specifically, I'd really like to confirm that the Chinese font "MS Song" has a valid cmap.

Improved patch that uses proper whitespacing. The old one was using Cairo's modeline and had tabs in it.

Comment on attachment 409278 [details] [diff] [review] Proposed patch for preventing null items from entering list This will fix the crashes, we should land this ASAP. We should still track down which pref fonts are causing the problem and confirm that the cmap checking code isn't rejecting fonts unnecessarily but that can happen after this patch has landed.

Comment on attachment 409398 [details] [diff] [review] Improved bugfix patch Not sure what's changed but looks good.

Restoring flags... again.

Landed on trunk: http://hg.mozilla.org/mozilla-central/rev/0eb54d1ad4e0

Cheng took a call from a user reporting this problem. He sent us his fonts and I went through and looked at them. There's one font that had a cmap structure problem, a duplicate cmap range. For some reason, the user had set this as his default font: user_pref("font.name.serif.x-western", "Arial MT Black"); user_pref("font.size.variable.x-western", 18); The font says it's an Arial face but it looks like a hacked up version of the real version. The copyright string in the name table had: 呹灥晡捥₩⁔桥⁍潮潴祰攠䍯牰潲慴楯渠灬挮⁄慴愠ꤠ周攠䵯湯瑹灥⁃潲灯牡瑩潮⁰汣⽔祰攠卯汵瑩潮猠䥮挮‱㤹〭ㄹ㤲⸠䅬氠剩杨瑳⁒敳敲癥搀 It would be interesting to know where this font came from. There were other funky fonts on the user's system but this was the only one with a cmap problem.

John: Were you able to reproduce the crash with that font? Any chance we can get this on 1.9.2 and 1.9.1 today? Is the patch ready (or is it the same one)? I'll be around to approve it.

Comment on attachment 409398 [details] [diff] [review] Improved bugfix patch Approved for 1.9.1.5. a=ss (This doesn't need 1.9.2 approval since it blocks.)

Pushed to 1.9.1 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/00e5f186048d Yes, with the funky font from the user set as the default x-western sans-serif font, the browser crashes almost immediately. With the fix it does not. The patch is simply enough that it applies to any branch without change.

Patches already landed on trunk and 1.9.1. Marking as fixed.

This needs to land on GECKO1914_20091006_RELBRANCH for 1.9.1.5.

Landed on 1.9.1 RELBRANCH http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c6d27d9d1e62

I can has test plz?

(In reply to comment #30) The actual fix here is in pref font handling code. We can change the pref font in mochitest but that pref font (the bad cmap font) has to be installed on the system for the test to really be valid.

(In reply to comment #15) > Created an attachment (id=409341) [details] > Font with malformed CMAP > > I've been able to reproduce the issue using the following (quite farfetched) > steps. Ofcourse this is unlikely to be what is happening in the wild. > > 1. Install the Swiss Neue font with corrupt CMAP table. > 2. Add the following lines to your prefs.js: > user_pref("font.default.x-western", "swiss-neue"); > user_pref("font.name.swiss-neue.x-western", "Swiss Neue"); > 3. Load the attached html which uses a unicode character. Verified fixed in the 1.9.1 branch with last night's nightly build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5pre) Gecko/20091102 Shiretoko/3.5.5pre (.NET CLR 3.5.30729)) and the testcase above. I will mark it as verified for .5 when we get release branch builds.

Does this bug still need to be hidden at this point?

Landed on 1.9.2 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/51741697c83a

Verified with the official 1.9.1.5 build 1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)). This is fixed.

Verified fixed on trunk and 1.9.2 with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091105 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20091105045542 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b2pre) Gecko/20091105 Namoroka/3.6b2pre (.NET CLR 3.5.30729) ID:20091105045233