I think this testcase causes crashes on older versions of Windows (XP? 2003?).

http://hg.mozilla.org/mozilla-central/annotate/4a3866321a14/gfx/cairo/cairo/src/cairo-gstate.c#l1848 _cairo_scaled_font_glyph_path on line 1883 fails so 'clip' is never assigned and contains random stack data when '_cairo_clip_fini' (_cairo_clip_reset) is executed on line 1901.

There's also an unrelated bug in '_cairo_win32_print_gdi_error' which fails to report the gdi error. It's the 'GetGlyphOutlineW' that fails on line 1712 and '_cairo_win32_print_gdi_error' is called on line 1717 but the error message never appears for me (on XP). http://hg.mozilla.org/mozilla-central/annotate/4a3866321a14/gfx/cairo/cairo/src/cairo-win32-font.c#l1677 This patch makes _cairo_win32_print_gdi_error work for me. The error message is "_cairo_win32_scaled_font_glyph_path: Cannot complete this function."

Jesse on IRC: "is it likely that lots of cairo bugs are like that?" I don't read cairo code that much so I don't want to speculate on that. Maybe we could override some of those Windows functions and inject errors...

For this testcase though, I expect that valgrind would have caught it.

Comment on attachment 502390 [details] [diff] [review] crash fix Make sure to coordinate with jeff on getting this upstream, and when you check in do the additional patched-cairo dance.

Comment on attachment 502391 [details] [diff] [review] print_gdi_error fix Same thing with this patch

Would we ever write back to 'clip'? So far this looks like a bogus read, and as long as it's only reading we might be OK. Unless of course we use 'clip' to define the memory extent of something else we're writing to.

Looks like it tries to do some fairly complex cleanup and dest of memory structures per https://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-surface.c#570 It might result in memory corruption similar to a double-free scenario.

The crash fix here should also fix the crash in bug 595423 (which is sg:critical).

I haven't filed bugs on https://bugs.freedesktop.org/ yet because I can't find a way to report a security sensitive bug there...

Given that this is sg:critical and we have patches, I'm going to block 2.0 on this. Feel free to land the patches ASAP!

Filed bugs upstream: https://bugs.freedesktop.org/show_bug.cgi?id=33318 https://bugs.freedesktop.org/show_bug.cgi?id=33319

http://hg.mozilla.org/mozilla-central/rev/6db090a3aaa0 http://hg.mozilla.org/mozilla-central/rev/ba49fff91d82 Crash test intentionally not landed yet.

This seemed to cause a: Regression :( Trace Malloc Leaks increase 14.5% on MacOSX 10.5.2 Firefox ------------------------------------------------------------------------ Previous: avg 1815027.333 stddev 138039.291 of 30 runs up to revision 8d9465a318f5 New : avg 2078330.000 stddev 3397.793 of 5 runs since revision ba49fff91d82 Perhaps it's worth backing out to see what if it gets better.

For the record, the reported leak is an expected regression from bug 627498. Talos automatic leak detection blamed the wrong changeset, bug 627860 filed. See discussion in mozilla.dev.tree-management for details. Nothing to worry about here.

I'd like to unhide this bug so I can land the crash test. It's been fixed since Firefox 4 so I think that's safe to do now, but this was a bug in upstream Cairo so I thought I'd ask. Chris Wilson said 2012-02-09 on https://bugs.freedesktop.org/show_bug.cgi?id=33318 "That code is now obsolete."

I'm opening this up after talking to Dveditz.

Landed the crash test: https://hg.mozilla.org/integration/mozilla-inbound/rev/848697281a2f