436663 - [Mac] Crash [@ OTL::GCommon::GetLookups] with Arial and line of exactly 510 characters consisting of unique Hebrew words

Status: VERIFIED FIXED

(Core :: Graphics, defect, P1)

Description

[Uri Bernstein]

Attachment: testcase

The page in http://www.haaretz.co.il/hasite/spages/988750.html crashes Firefox:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.1a1pre) Gecko/2008053002 Minefield/3.1a1pre

This is with 10.5.2. I'll test this on Intel/10.5.3 later and report.

Crasher report ID: 18e158a3-2ef6-11dd-8f1f-001cc4e2bf68

After much minimizing efforts, it seems that the following conditions have to be met for the page to crash:
- Font should be Arial.
- There must be a line consisting of 510 characters exactly, which could be Hebrew letters, spaces, and punctuation.
- All words on the line should be unique, and nut not appear earlier on the page.
- Words that aren't unique, or that appeared earlier on the page, simply don't count towards the 510 character count (so you can add a word to the 510-chars line and still have it crash if you also add it somewhere before the line).

Comment 1

[Uri Bernstein]

Regression range is 2008-01-28 to 2008-01-29:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-01-28+03%3A00&maxdate=2008-01-29+05%3A00&cvsroot=%2Fcvsroot

Bug 410728 seems like a possible candidate for causing this.

Nominating to block 1.9.0.1 since this is a fairly recent crash regression encountered in real world.

Comment 2

[Uri Bernstein]

This doesn't crash on Intel/10.5.3. When I get back to my PPC machine, I'll try to upgrade it to 10.5.3 and see if that helps.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://crash-stats.mozilla.com/report/index/18e158a3-2ef6-11dd-8f1f-001cc4e2bf68

Looks like it crashes deep in ATS. Probably an ATS bug, especially given it works in 10.5.3 (and 10.4.11 for me); maybe it was a 10.5 bug fixed in 10.5.3?

Comment 4

[Uri Bernstein]

Still crashing on this with my PPC machine after upgrading to 10.5.3:

http://crash-stats.mozilla.com/report/index/2d12e178-3483-11dd-9ea0-001cc4e2bf68?p=1

Comment 5

[Uri Bernstein]

After removing a version of Arial I had in my account's Fonts directory, I'm now also crashing on the Intel/10.5.3 machine:

http://crash-stats.mozilla.com/report/index/ca6a678c-34a0-11dd-997e-001cc4e2bf68?p=1

I wasn't, however, able to get a crash on a 10.4.11 machine.

Comment 6

[Uri Bernstein]

... and apparently I'm not alone:

http://crash-stats.mozilla.com/report/list?range_unit=weeks&query_search=signature&query_type=contains&signature=ATS%400x79bc4&date=2008-06-07&query=ATS%400x79bc4&range_value=1

Comment 7

[John Daggett (:jtd)]

Confirmed crash on 10.5.3 Intel with RC2:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008053008 Firefox/3.0

Mac OS X 10.5.3 (9D34)

Comment 8

[Uri Bernstein]

Attachment: Code that crashes

This is a minimized C++/Carbon standalone program that produces the same crash.

As far as I can tell, this means that this is indeed an ATSUI bug (in 10.5). However, we might want to consider ways to work around it and avoid the crash.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks Uri!!! Can you report the bug to Apple?

We can probably work around it just by having GuessMaximumStringLength max out at 500 or so. Can you try that?

Comment 10

[John Daggett (:jtd)]

Attachment: code that crashes, lookup ATSUFontIDs by name

Yeah, this code is a huge help!!

ATSUFontIDs are not fixed, so I changed the code to lookup the fonts via the Postscript name.  I'll log a bug with Apple and ping the ATSUI engineer, I'm sure he loves hearing about these bugs. ;)

Comment 11

[John Daggett (:jtd)]

Attachment: initial patch, cap GuessMaximumStringLength at 500

needs more testing but the testcase no longer crashes with this

Comment 12

[John Daggett (:jtd)]

Logged as Apple bug 5996598.

Comment 13

[John Daggett (:jtd)]

Testcase stack crawl:

#0  0x93797bc4 in OTL::GCommon::GetLookups ()
#1  0x9374a2d2 in ProcessRunCommon ()
#2  0x93749d6f in ProcessGSUBRun ()
#3  0x9373ae3b in ApplyMorphForRun ()
#4  0x9374728f in ApplyMorph ()
#5  0x9373a20f in _eLLCLayoutText ()
#6  0x9373a0c3 in LLCLayoutText ()
#7  0x92e433e2 in ATSULayoutGlyphs ()
#8  0x92e432ce in TTextLineLayout::LayoutGlyphVector ()
#9  0x92e543de in TTextLineLayout::EnsureLayoutIsUpToDate ()
#10 0x92e5eecd in TTextLineLayout::GetGlyphBounds ()
#11 0x92e5edfe in ATSUGetGlyphBounds ()
#12 0x00001f37 in main (argc=1, argv=0xbffff7b8) at /Users/jd/Desktop/test/atsuistrcrash/main.cpp:79

Comment 14

[John Daggett (:jtd)]

Note from Apple ATSUI dev:

"I believe we already fixed this crasher in SnowLeopard.  If I can  
reproduce and show the fix on Leopard, I can probably get it into an  
SU.  Thanks."

SnowLeopard is 10.6, due out next year(?).

Comment 15

[Uri Bernstein]

John, are you going to ask for review on your patch? I'm crashing on this every once in a while, and it's very annoying.

Comment 16

[John Daggett (:jtd)]

Tested with latest 10.5 update (Mac OS X 10.5.5 (9F33)), not fixed.

Comment 17

[John Daggett (:jtd)]

Attachment: patch, v.0.2, cap string length only on 10.5

Comment 18

[John Daggett (:jtd)]

(In reply to comment #15)
> John, are you going to ask for review on your patch? I'm crashing on this every
> once in a while, and it's very annoying.

Once this has been reviewed and checked in on trunk, I'll work on getting this approved and back ported to 1.9.0.x.

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 339010 [details] [diff] [review]
patch, v.0.2, cap string length only on 10.5

Workaround looks fine to me.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 339010 [details] [diff] [review]
patch, v.0.2, cap string length only on 10.5

ugh

Comment 21

[John Daggett (:jtd)]

Checked in
505564b8749d
2008-09-26 16:51 +0900

Comment 22

[Marcia Knous [:marcia]]

verified fixed using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081013 Minefield/3.1b2pre. I verified using the testcase in Comment 0.

Comment 23

[Mike Connor [:mconnor]]

Comment on attachment 339010 [details] [diff] [review]
patch, v.0.2, cap string length only on 10.5

John, can you comment as to risk for branch and request approval for 1.9.0.5 if its still appropriate?

Comment 24

[John Daggett (:jtd)]

Comment on attachment 339010 [details] [diff] [review]
patch, v.0.2, cap string length only on 10.5

(In reply to comment #23)
> (From update of attachment 339010 [details] [diff] [review])
> John, can you comment as to risk for branch and request approval for 1.9.0.5 if
> its still appropriate?

This is a low-risk patch since we are just working around an underlying Apple bug.  I spoke with Uri about this at the summit, he said he experienced this relatively frequently, so I imagine Hebrew users are hitting this fairly often.

Comment 25

[Daniel Veditz [:dveditz]]

Comment on attachment 339010 [details] [diff] [review]
patch, v.0.2, cap string length only on 10.5

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 26

[John Daggett (:jtd)]

Checked in on cvs trunk.

Comment 28

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.5 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.5pre) Gecko/2008120104 GranParadiso/3.0.5pre. Verified the crash with a Firefox 3.0.4 instance.

Comment 29

[John Daggett (:jtd)]

Fixed in 10.6, SL seed 10A222.  Trying to see if the fix can be backported to 10.5.