Closed Bug 964512 Opened 10 years ago Closed 9 years ago

crash in RenderItemWithFallback(tag_STRING_ANALYSIS*, int, int, _ABC*)

Categories

(Core :: Graphics: Text, defect)

x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox40 --- fixed
firefox41 --- verified

People

(Reporter: kbrosnan, Assigned: jfkthame)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-0f94f1b4-7375-4b93-b21c-a6a3c2140127.
=============================================================

Mostly older Intel Graphics cards, though some ATI/AMD. Spoke with a user that had a GMA950 that crashed on specific sites. Stack shows that fonts may be involved?

0 	usp10.dll 	RenderItemWithFallback(tag_STRING_ANALYSIS *,int,int,_ABC *) 	
1 	usp10.dll 	RenderItem(tag_STRING_ANALYSIS *,int,int,_ABC *) 	
2 	usp10.dll 	ScriptStringAnalyzeGlyphs(tag_STRING_ANALYSIS *) 	
3 	usp10.dll 	ScriptStringAnalyse 	
4 	lpk.dll 	LpkStringAnalyse 	
5 	lpk.dll 	GetWindowLongW 	
6 	gdi32.dll 	NtGdiGetTextExtentExW 	
7 	gdi32.dll 	GetTextExtentPoint32W 	
8 	xul.dll 	gfxGDIFont::Initialize() 	gfx/thebes/gfxGDIFont.cpp
9 	xul.dll 	gfxGDIFont::ShapeText(gfxContext *,wchar_t const *,unsigned int,unsigned int,int,gfxShapedText *,bool) 	gfx/thebes/gfxGDIFont.cpp
10 	xul.dll 	gfxFont::ShapeFragmentWithoutWordCache<wchar_t>(gfxContext *,wchar_t const *,unsigned int,unsigned int,int,gfxTextRun *) 	gfx/thebes/gfxFont.cpp
11 	xul.dll 	gfxFont::ShapeTextWithoutWordCache<wchar_t>(gfxContext *,wchar_t const *,unsigned int,unsigned int,int,gfxTextRun *) 	gfx/thebes/gfxFont.cpp
12 	xul.dll 	gfxFont::SplitAndInitTextRun<wchar_t>(gfxContext *,gfxTextRun *,wchar_t const *,unsigned int,unsigned int,int) 	gfx/thebes/gfxFont.cpp
13 		@0x45004d 	
14 	mozglue.dll 	je_calloc 	memory/mozjemalloc/jemalloc.c
15 	mozglue.dll 	je_realloc 	memory/mozjemalloc/jemalloc.c
16 	xul.dll 	nsTArray_base<nsTArrayInfallibleAllocator,nsTArray_CopyElements<nsCOMPtr<nsIDirectoryServiceProvider> > >::EnsureCapacity(unsigned int,unsigned int) 	xpcom/glue/nsTArray-inl.h
17 	xul.dll 	nsTArray_Impl<gfxFont *,nsTArrayInfallibleAllocator>::AppendElements<gfxFont *>(gfxFont * const *,unsigned int) 	obj-firefox/dist/include/nsTArray.h
18 	xul.dll 	gfxFontEntry::TryGetSVGData(gfxFont *) 	gfx/thebes/gfxFont.cpp
19 	xul.dll 	CreateObserversForAnimatedGlyphs 	layout/generic/nsTextFrame.cpp
20 	xul.dll 	BuildTextRunsScanner::FlushLineBreaks(gfxTextRun *) 	layout/generic/nsTextFrame.cpp
21 	xul.dll 	BuildTextRunsScanner::FlushFrames(bool,bool) 	layout/generic/nsTextFrame.cpp
22 	xul.dll 	BuildTextRuns 	layout/generic/nsTextFrame.cpp
This is a crash in Uniscribe. We need a testcase to try and reproduce the problem and either work around it or get it fixed by Microsoft.  The Uniscribe version (usp10.dll, 1.626.7601.18009) is relatively recent so I'm guessing this is a new bug introduced by a recent MS update.
Keywords: testcase-wanted
Partial listing of URLs. Twitter dominates as does PDF.js. Taking a quick look Twitter might default to Arial on Windows. 

* https://twitter.com/
* about:blank
* https://www.facebook.com/
* http://mail.yahoo.com/
* https://login.yahoo.com/config/login_verify2?
* http://www.ktkol.om.pttk.pl/materialy/PTKol_Budowa_i_mechanika_roweru_gorskiego_20130415.pdf
* http://englishforeveryone.org/PDFs/Dialogue%20Practice.pdf
* http://www.karel.com.tr/sites/default/files/belge/doc/MSserisi_santra_kullanim_kilavuzu.pdf
* http://www.immt.pwr.wroc.pl/~lewandowski/zadania/kol_1-me.pdf
* http://www.netsdaily.com/
* http://docs.whirlpool.eu/_doc/501930700384GR.pdf
* http://www.karel.com.tr/sites/default/files/belge/doc/ms48-brosur-online.pdf
* http://volker704.tumblr.com/
* http://skarga.edu.pl/lic/phocadownload/tw_katy.pdf
* http://masralarabia.com/
* http://www.phie.pl/pdf/phe-2011/phe-2011-3-428.pdf
* http://www.ktkol.om.pttk.pl/materialy/OM_PTTK_-_Egzamin_PTKol_-_21.05.2009.pdf
* http://jamesclear.com/body-language-how-to-be-confident
* about:privatebrowsing
* http://adrotator.se/?redirect&placement=243677&cookie=1&random=0.4256955640096286&domain=adrotator.se
* http://css-tricks.com/
* http://extabit.com/file/27avt82lt5fwr/Peter.Pan.2003.720P.BRRIP.XVID.AC3-MAJESTiC.part1.rar
* http://laban.vn/?utm_source=laban&u=ced4fdb6cb9d47156ffec3f9fcbed8269d21&utm_campain=201401
* http://www.thaqafnafsak.com/2013/03/steps-for-internet-security.html
* http://www.mmo.org.tr/resimler/dosya_ekler/5ac487be5f08888_ek.pdf
* http://www.thomas.ru/sites/default/files/pdf/gaw_vestfalia_xt.pdf
* http://www.metrolyrics.com/if-im-not-in-love-with-you-lyrics-kathy-troccoli.html
* http://mhealth.cu.edu.eg/
* http://www.people.hbs.edu/acuddy/in%20press,%20carney,%20cuddy,%20&%20yap,%20psych%20science.pdf
* http://www.karel.com.tr/bilgi/pbx-terimler-sozlugu-nedir-acilimi-nedir-ne-demek
* http://www.youtube.com/
* http://omigaplus.inspsearch.com/search/web?fcoid=417&q=%D8%A7%D8%AC%D9%85%D9%84+%D8%B5%D9%88%D8%AA+%D9%82%D8%B1%D8%A7%D9%86
* https://king.com/?type=partner&subtype1=royalgames&subtype2=candycrush_gamepage#!play/candycrush
* http://www.employeebenefits.co.uk/Journals/2013/01/17/a/e/v/Perks-Map---International-Reward---2013.pdf
* http://vube.com/AngelikaVee/pz94ltMGjS/L/vote?t=s
* http://www.agiathimia.com/Old%20Papers/agiathimia56.pdf
* http://www.ktkol.om.pttk.pl/materialy/OM_PTTK_-_Egzamin_PTKol_-_26.05.2011.pdf
* http://www.klemens.sav.sk/fiusav/doc/filozofia/2009/5/505-511.pdf
* http://sonndos.tumblr.com/page/3
* http://ems6.net/r/?F=rusdy23tq5bm67sc8cgj9uyp9y4m9kpgwldf5dkcvr7cwpa9bwvds6q-4748922
* https://twitter.com/followers
* https://twitter.com/MorgensternT/status/424602842390081537/photo/1
* https://twitter.com/?lang=hu&logged_out=1#
* http://www.dsplog.com/2008/02/03/understanding-an-ofdm-transmission/
* http://www.phie.pl/pdf/phe-2006/phe-2006-3-166.pdf
* http://mail.centrum.sk/download.php?msg_id=00000000b933000877c00239b179&idx=3&filename=ebill_7400455474.pdf&r=90.40055770384393
* http://photobucket.com/images/site%20models?page=1
* https://login.live.com/login.srf?wa=wsignin1.0&rpsnv=12&ct=1389430027&rver=6.1.6206.0&wp=MBI&wreply=http:%2F%2Fmail.live.com%2Fdefault.aspx&lc=1033&id=64855&mkt=en-us&cbcxt=mai&snsc=1
I have this bug.
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150513174244

Steps to reproduce:

1. Go to twitter.com
2. Try to log in
3. Firefox crashes!



Actual results:

Firefox crashes every time I try to log in to twitter. Using Safe Mode or creating a new profile doesn't help.
Chrome doesn't have this issue.


Expected results:

Twitter succesfully loads.
The crash is occurring from within gfxGDIFont::Initialize, at the point where we try to get the width of the font's "0" glyph. I'm guessing perhaps there's a font present that doesn't support the ASCII digits, and the low-level code called by GetTextExtentPoint32W doesn't handle the missing-character situation safely.

As a possible fix, I suggest we should verify that the character exists (using GetGlyphIndicesW) before trying to ask for the metrics.
sashafan2, if you could test the build from http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-466c358b5049/try-win32/ and confirm whether this avoids the crash, it would be really helpful. Thanks!
Flags: needinfo?(sashafan2)
John, wdyt? Whether or not we can verify that this fixes the specific crash reported here, it seems like a good precaution and should have no adverse impact.
Attachment #8612808 - Flags: review?(jdaggett)
(In reply to Jonathan Kew (:jfkthame) from comment #8)
> sashafan2, if you could test the build from
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-
> 466c358b5049/try-win32/ and confirm whether this avoids the crash, it would
> be really helpful. Thanks!

Jonathan,
OK! I'll test it.
Flags: needinfo?(sashafan2)
Assignee: nobody → jfkthame
Attachment #8612808 - Flags: review?(jdaggett) → review+
Marking 'leave-open' for now, as the patch just pushed is a speculative fix: let's see what effect it has on crash-stats before deciding if we can resolve this bug.

Currently, https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=RenderItemWithFallback%28tag_STRING_ANALYSIS*%2C+int%2C+int%2C+_ABC*%29 shows approximately 500 crashes in the last 28 days.

If the patch does help, we should see an effect on the graph at https://crash-stats.mozilla.com/home/products/Firefox/versions/41.0a1#duration=14:date_range_type=build within a few days after it has merged to trunk.
Keywords: leave-open
I don't see any reports of this signature on crash-stats for Firefox 42, or for FF 41 builds since this patch landed... I think we can regard it as fixed.

This landed for 41, so it's currently on aurora as well as m-c; given that it appears to fix the crash, and given how safe the patch looks, I think we should consider uplifting it to Firefox 40 as well.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 8612808 [details] [diff] [review]
Check for existence of character before trying to get its metrics in gfxGDIFont::Initialize

Approval Request Comment
[Feature/regressing bug #]: long-standing crasher, perhaps worse since recent Windows updates (per comment 1)?
[User impact if declined]: crashes, often at startup, for some Windows users -- dependent on whether graphics acceleration is enabled, and depending on installed fonts
[Describe test coverage new/current, TreeHerder]: existing WinXP and non-accelerated Win7/8 tests all exercise this code heavily
[Risks and why]: very low risk - just adds checks to avoid calling a Windows API with no documented error-handling for characters that are not supported by the font
[String/UUID change made/needed]: none
Attachment #8612808 - Flags: approval-mozilla-beta?
Comment on attachment 8612808 [details] [diff] [review]
Check for existence of character before trying to get its metrics in gfxGDIFont::Initialize

If it is fixing a bug without side effect, we want it in beta.
Attachment #8612808 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Target Milestone: --- → mozilla41
Restrict Comments: true
You need to log in before you can comment on or make changes to this bug.