Closed Bug 618406 Opened 9 years ago Closed 9 years ago

crash [@ @0x0 | gfxFT2LockedFace::GetUVSGlyph ] from the checkin for bug 569770

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: cork, Assigned: karlt)

References

()

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(2 files)

Attached file Crash test
I'm getting a ton of crashes from the checkin for bug 569770:
bp-2c5545a5-b271-4329-b41c-782f92101210
bp-b3f5708e-a1d4-469e-bf6c-250a42101210
bp-4a21b3a2-cb00-49cf-8794-7e2cf2101210
bp-80ae45d6-85e5-479c-9022-105a02101210

The regression range points right at this bug:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9ab76fbcb6d7&tochange=11e328a49e0a

Its only Linux x86_64 systems as far as i can see.
Blocks: 569770
blocking2.0: --- → ?
Summary: Crash from the checkin for bug 569770 → crash [@ 0x0 | gfxFT2LockedFace::GetUVSGlyph] from the checkin for bug 569770
Summary: crash [@ 0x0 | gfxFT2LockedFace::GetUVSGlyph] from the checkin for bug 569770 → crash [@ @0x0 | gfxFT2LockedFace::GetUVSGlyph ] from the checkin for bug 569770
Keywords: crash
This is from brand new code, and is a regression since beta 7. It's quite crashy, the #2 topcrasher today on Linux, but that's still only a small number of crashes, 22.

This could still turn into a beta 8 blocker.
Assignee: nobody → karlt
blocking2.0: ? → beta9+
Keywords: regression, topcrash
Cork:

Did you submit all 22 of those crashes?
I have 27 as of before i started regression searching (stoped remembered to uncheck it by then).
Cork: what version of freetype is installed on your system? If it's earlier than 2.4.4, I think this may be a freetype bug that was recently fixed (http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=e891e4d6f130408d171724723673472a4e0359f9).
It's 2.4.2, the version used in ubuntu 10.10 and (for the momet at least) 11.04.
I'm having trouble understanding why that patch was necessary.

FT_Face_GetCharVariantIndex will only try to call vcmap->clazz->char_var_index
if find_variant_selector_charmap returns non-NULL ...

http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/base/ftobjs.c?id=6aee69096f7c04f3c16fa3d1a097180f9fb62154#n3285

... but find_variant_selector_charmap only returns non-NULL when
FT_Get_CMap_Format( charmap ) == 14.

http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/base/ftobjs.c?id=6aee69096f7c04f3c16fa3d1a097180f9fb62154#n1062
I don't reproduce with freetype 2.4.3, but I can reproduce with
http://security.ubuntu.com/ubuntu/pool/main/f/freetype/libfreetype6_2.4.2-2ubuntu0.1_amd64.deb
In FreeType versions 2.4.0 to 2.4.3, when FT_CONFIG_OPTION_OLD_INTERNALS is defined, find_variant_selector_charmap always returns the first cmap.
Fixed here:

http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=ac09390afcfaf2c63b75ffee5c0759e29359f9ac
I don't see any way to access FreeType's preloaded cmap data to use
gfxFontUtils::MapUVSToGlyphFormat14 without making a copy of the whole cmap
table (with all subtables).

FT_Library_Version is available to check for versions that may have the bug,
http://freetype.sourceforge.net/freetype2/docs/reference/ft2-version.html#FT_Library_Version

and for possibly buggy versions presence of the symbol FT_Alloc indicates
FT_CONFIG_OPTION_OLD_INTERNALS and so presence of the bug.
This is in our testsuite as layout/reftests/font-face/ivs-1.html but we should remove random-if(gtk2Widget).
gfxFontUtils has the necessary functions for copying the variant selector
subtable from the cmap table and for performing the lookup.
However, managing the lifetime of the subtable copy gets a bit fiddly,
particularly for gfxFT2Fonts consumers where the FontEntry does not expire.

Functional IVS on systems with broken IVS support doesn't seem an important
enough hoop to jump through given the addition code required, so I suggest a
miminal fix to simply avoid crashy versions of FT_Face_GetCharVariantIndex.
Attachment #497198 - Attachment is patch: true
Attachment #497198 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 497198 [details] [diff] [review]
avoid crashy versions of FT_Face_GetCharVariantIndex

Yes, I'd agree: it's not worth managing a whole separate code path to work around this by using the gfxFontUtils code - just detect the buggy FT version and don't support the feature. So this looks fine.
Attachment #497198 - Flags: review?(jfkthame) → review+
http://hg.mozilla.org/mozilla-central/rev/2b3626056e13
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
No longer blocks: 569770
blocking2.0: beta9+ → betaN+
Crash Signature: [@ @0x0 | gfxFT2LockedFace::GetUVSGlyph ]
You need to log in before you can comment on or make changes to this bug.