Closed Bug 483439 Opened 11 years ago Closed 11 years ago

Crash if text has both fill and stroke

Categories

(Core :: SVG, defect, critical)

x86
All
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: stebs, Assigned: longsonr)

References

()

Details

(4 keywords)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090314 Shiretoko/3.1b4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090314 Shiretoko/3.1b4pre

Site with SVG and SMIL Examples

Reproducible: Always

Steps to Reproduce:
Goto url above

Actual Results:  
Instant Crash on load


Crashes reproducably with Branch 20090313, 20090314 AND Trunk 20090313 (no others tested)
Fx 3.0.7 is ok

Also crashes with disabled JIT

Crash reports:
http://crash-stats.mozilla.com/report/index/637c3560-ad42-49b9-813d-7bed12090314
http://crash-stats.mozilla.com/report/index/c1ed8153-961d-479e-8a0d-a924a2090314
http://crash-stats.mozilla.com/report/index/1ecad3d9-be69-4447-9b5a-d6a1c2090314
Did you try in safe mode and with a new profile?
Component: General → SVG
Keywords: crash
Product: Firefox → Core
QA Contact: general → general
Whiteboard: DUPEME
Version: unspecified → Trunk
Yes, crashes with new Profile and in Safemode
This is also crashing on latest hourly - Vista HP SP2 RC
Since I'm using hourly's crash-reports won't help, will add later once I grab a nightly build.

Setting to 'New' / windows all

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090314 Minefield/3.2a1pre Firefox/3.0.4 ID:20090314101536
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Tested nightly trunks back to 1/1/9 and still getting crashes.

Most seem to be xul.dll crashes as reported.  

Lost track of builds tested, but one build produced this crash report:
http://crash-stats.mozilla.com/report/index/f2962d1b-4f82-4e62-8c70-9f0542090314?p=1
GetAdvanceForGlyphs	gfx/thebes/src/gfxFont.cpp:482
gfxTextRun::ComputeLigatureData	gfx/thebes/src/gfxFont.cpp:1471
xul.dll@0x3320e2	
CharacterIterator::NextChar	layout/svg/base/src/nsSVGGlyphFrame.cpp:1423
nsSVGGlyphFrame::AddBoundingBoxesToPath	layout/svg/base/src/nsSVGGlyphFrame.cpp:555

479             const gfxTextRun::DetailedGlyph *details = aTextRun->GetDetailedGlyphs(i);
482                 advance += details->mAdvance;

1403     const DetailedGlyph *GetDetailedGlyphs(PRUint32 aCharIndex) {
1404         return mDetailedGlyphs ? mDetailedGlyphs[aCharIndex].get() : nsnull;

Sure looks like it could be null...
Assignee: nobody → roc
Component: SVG → GFX: Thebes
QA Contact: general → thebes
Summary: Crash on load on Site with SVG examples [@ xul.dll@0x282899] → Crash on load on Site with SVG examples [@ GetAdvanceForGlyphs]
works:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081202 Minefield/3.2a1pre ID:20081202034510
http://hg.mozilla.org/mozilla-central/rev/03b31906e289
fails:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081203 Minefield/3.2a1pre ID:20081203041857
http://hg.mozilla.org/mozilla-central/rev/0d300ab7a8cf

=> range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=03b31906e289&tochange=0d300ab7a8cf
Keywords: regression
Depends on: 437704
Whiteboard: DUPEME
with all "stroke:" rules removed from testcase there's no crash
Assignee: roc → longsonr
Component: GFX: Thebes → SVG
QA Contact: thebes → general
Attached patch patchSplinter Review
Attachment #367432 - Flags: superreview?(roc)
Attachment #367432 - Flags: review?(roc)
Attachment #367432 - Flags: superreview?(roc)
Attachment #367432 - Flags: superreview+
Attachment #367432 - Flags: review?(roc)
Attachment #367432 - Flags: review+
checked in http://hg.mozilla.org/mozilla-central/rev/361f0a25faa9
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: blocking1.9.1?
Resolution: --- → FIXED
Flags: blocking1.9.1? → blocking1.9.1+
Whiteboard: [needs 191 landing]
Why does this change fix the crash?
AddCharactersToPath iterates through the characters and so does AddBoundingBoxesToPath. If fill and stroke are both set then AddBoundingBoxesToPath starts at the last character in the string + 1 and reads memory outside the bounds that it should.
Thanks. Good to have a summary. So essentially CharacterIterator should only be used once. Would it not be good to have a comment in the code warning about that? And perhaps CharacterIterator could store a pointer to the last good memory so it can make sure it doesn't read too far and assert if someone tries to make it do so?
checked in http://hg.mozilla.org/releases/mozilla-1.9.1/rev/70c45ea1857c
Keywords: fixed1.9.1
Whiteboard: [needs 191 landing]
(In reply to comment #14)
> Thanks. Good to have a summary. So essentially CharacterIterator should only be
> used once. Would it not be good to have a comment in the code warning about
> that? And perhaps CharacterIterator could store a pointer to the last good
> memory so it can make sure it doesn't read too far and assert if someone tries
> to make it do so?

Good ideas, although we don't necessarily need to land them on 1.9.1.

CharacterIterator can figure out where the last good memory is already, in fact it even has an assert but it's a line or so too late. Can you raise a follow up bug so we don't forget please?
For the trunk maybe CharacterIterator should just wrap back to the beginning or alternatively have some kind of reset member function.
Blocks: 484953
Sure thing. Filed bug 484953.
Summary: Crash on load on Site with SVG examples [@ GetAdvanceForGlyphs] → Crash text has fill and stroke
Blocks: 487408
Summary: Crash text has fill and stroke → Crash if text has both fill and stroke
Verified fixed on the 1.9.1 branch using  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090409 Shiretoko/3.5b4pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090409 Shiretoko/3.5b4pre.

Verified fixed on the trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090409 Minefield/3.6a1pre and  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090409 Minefield/3.6a1pre

No crash with test case on any of those builds.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 488755
testcase checked in with bug 484953
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.