Closed Bug 374815 Opened 18 years ago Closed 15 years ago

SVG pattern crash [@ SurfacePatternDrawFunc]

Categories

(Core :: SVG, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jruderman, Unassigned)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(2 files, 1 obsolete file)

Attached image testcase (crashes Firefox when loaded) (deleted) —
This testcase causes Firefox to attempt to allocate a huge block of memory and crash. Exception: EXC_BAD_ACCESS (0x0001) Codes: KERN_PROTECTION_FAILURE (0x0002) at 0x000000c4 Thread 0 Crashed: 0 libthebes.dylib 0x07b421c7 SurfacePatternDrawFunc + 236 (cairo-nquartz-surface.c:524) 1 com.apple.CoreGraphics 0x903d974f CGContextDrawPattern + 212 2 com.apple.CoreGraphics 0x903d9498 pattern_replicate_pattern + 924 3 com.apple.CoreGraphics 0x90344088 pattern_tile_pattern + 7536 4 com.apple.CoreGraphics 0x90342310 CGPatternFilterDelegateTilePattern + 83 5 libRIP.A.dylib 0x9430afdc ripc_GetPattern + 1098 6 libRIP.A.dylib 0x94307928 ripc_GetColor + 199 7 libRIP.A.dylib 0x94306d03 ripc_Render + 207 8 libRIP.A.dylib 0x94311f54 ripc_DrawPath + 302 9 com.apple.CoreGraphics 0x90393c9b CGContextDrawPath + 169 10 com.apple.CoreGraphics 0x903a7bae CGContextFillPath + 25 11 libthebes.dylib 0x07b437d0 _cairo_nquartz_surface_fill + 263 (cairo-nquartz-surface.c:1241) 12 libthebes.dylib 0x07b36981 _cairo_surface_fill + 236 (cairo-surface.c:1411) 13 libthebes.dylib 0x07b2102a _cairo_gstate_fill + 186 (cairo-gstate.c:994) 14 libthebes.dylib 0x07b18217 _moz_cairo_fill_preserve + 40 (cairo.c:1947) 15 libthebes.dylib 0x07b06712 gfxContext::Fill() + 20 (gfxContext.cpp:135) 16 libgklayout.dylib 0x19b8cf2c nsSVGPathGeometryFrame::Render(nsSVGRenderState*) + 448 (nsSVGPathGeometryFrame.cpp:650) 17 libgklayout.dylib 0x19b8e401 nsSVGPathGeometryFrame::PaintSVG(nsSVGRenderState*, nsRect*) + 65 (nsSVGPathGeometryFrame.cpp:346) 18 libgklayout.dylib 0x19b9821f nsSVGUtils::PaintChildWithEffects(nsSVGRenderState*, nsRect*, nsIFrame*) + 737 (nsSVGUtils.cpp:956) 19 libgklayout.dylib 0x19b8b3dc nsSVGOuterSVGFrame::Paint(nsIRenderingContext&, nsRect const&, nsPoint) + 366 (nsSVGOuterSVGFrame.cpp:483) 20 libgklayout.dylib 0x19b8b4a0 nsDisplaySVG::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) + 74 (nsSVGOuterSVGFrame.cpp:377) 21 libgklayout.dylib 0x1966dddb nsDisplayList::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) const + 61 (nsDisplayList.cpp:298)
Not sure if it matters, but I was testing with window.innerWidth == 1237.
Doesn't crash on linux.
Can't reproduce on intel mac either: WFM on Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a6pre) Gecko/20070613 Minefield/3.0a6pre
Still crashes for me on Intel Mac (freshly clobbered debug build).
Thanks Jesse - I was using an optimized build. Rebuilt with --enable-debug and it crashes now on this test case. In fact I managed two different crashes, one like the original: 0 XUL 0x2164cc76 SurfacePatternDrawFunc + 185 (cairo-quartz-surface.c:487) 1 com.apple.CoreGraphics 0x903d76af CGContextDrawPattern + 212 and another in... 0 libthebes.dylib 0x0103223d _moz_cairo_surface_get_type + 6 1 libthebes.dylib 0x0100a072 gfxASurface::GetType() const + 20 I also found some ignored error statuses in the cairo-quartz code which may be the culprit. I'll look into this further.
Reproducing the bug in the builds where it wasn't crashing for me straight away was just a matter of resizing the browser to allocate even more memory. The cause was that failures in clone_similar are leaving the surface NULL; I thought the intent was to return a nil surface... which would also have been broken. So I've put in a check for both in this patch, which prevents the crash for me. I'll be putting a version of this into cairo trunk next week.
Attached patch check the status... doh (deleted) — Splinter Review
Scratch that, just checking the status of clone_similar fixes the problem.
Attachment #268481 - Attachment is obsolete: true
Attachment #268489 - Attachment is patch: true
Attachment #268489 - Attachment mime type: application/octet-stream → text/plain
I still see this crash with Firefox trunk. Brian, what's the status of the patch in comment 7? Has it been incorporated into Cairo?
(In reply to comment #8) > I still see this crash with Firefox trunk. > > Brian, what's the status of the patch in comment 7? Has it been incorporated > into Cairo? Oops. No, sorry Jesse, I hadn't. Just pushed it with a bunch of other small mac fixes: http://gitweb.freedesktop.org/?p=cairo;a=commit;h=6fec51990e90901ebafbb872a9e618cb70d17911 The patch isn't the one above, some refactoring went on since, but it contains the fix. Carl spoke recently of an imminent release of cairo, so it should be possible to sync up again soon.
This should be fixed when bug 404092 lands, if someone can add the dependency.
Depends on: 404092
This isn't actually any better with an updated cairo; we seem to allocate a huge chunk of memory, but I haven't tracked down what/where yet.
bug 385228 also has cairo allocating a huge piece of memory for a surface only on macs. Related?
The bug's morphed a bit, the huge malloc is now here: #0 0x959a59f1 in malloc_error_break () #1 0x959a09df in szone_error () #2 0x958c8ff6 in allocate_pages () #3 0x958c97e2 in large_and_huge_malloc () #4 0x958c7f15 in malloc_zone_calloc () #5 0x909e29b6 in ripl_Create () #6 0x909ecb72 in ripc_GetColor () #7 0x909cd832 in ripc_Render () #8 0x909d68a0 in ripc_DrawPath () #9 0x928c0687 in CGContextDrawPath () #10 0x928c05d4 in CGContextFillPath () #11 0x11e38dce in _cairo_quartz_surface_fill (abstract_surface=0x1f587bd0, op=CAIRO_OPERATOR_OVER, source=0xbfffcaac, path=0x83437c, fill_rule=CAIRO_FILL_RULE_WINDING, tolerance=0.10000000000000001, antialias=CAIRO_ANTIALIAS_DEFAULT) at /Users/brianewins/projects/ffox/mozilla/gfx/cairo/cairo/src/cairo-quartz-surface.c:1273 #12 0x11e31878 in _cairo_surface_fill (surface=0x1f5c21d0, op=CAIRO_OPERATOR_OVER, source=0xbfffcbdc, path=0x83437c, fill_rule=CAIRO_FILL_RULE_WINDING, tolerance=0.10000000000000001, antialias=CAIRO_ANTIALIAS_DEFAULT) at /Users/brianewins/projects/ffox/mozilla/gfx/cairo/cairo/src/cairo-surface.c:1578 #13 0x11e1ecf7 in _cairo_gstate_fill (gstate=0x1f5c21d0, path=0x83437c) at /Users/brianewins/projects/ffox/mozilla/gfx/cairo/cairo/src/cairo-gstate.c:1073 #14 0x11e167b5 in _moz_cairo_fill_preserve (cr=0x2) at /Users/brianewins/projects/ffox/mozilla/gfx/cairo/cairo/src/cairo.c:2106 #15 0x11df968c in gfxContext::Fill (this=0x0) at /Users/brianewins/projects/ffox/mozilla/gfx/thebes/src/gfxContext.cpp:136 ...etc So there's a problem with a huge fill. If this is related to bug 385228 its not that surprising. Also shortly afterwards there's a second failed allocation: Sat Dec 1 00:00:27 Macintosh.local firefox-bin[53124] <Error>: CGBitmapContextCreateImage: failed to allocate 516320000 bytes. Sat Dec 1 00:00:28 Macintosh.local firefox-bin[53124] <Error>: CGImageCreate: invalid image provider: NULL. The second message of this pair is easily fixed, we're not checking the result of CGDataProviderCreateWithData. The first is in part because we're using the full image_rect dimensions _cairo_quartz_surface_release_dest_image instead of just the interest_rect, which is something I was looking at anyway.
I debugged this further back, the problem looks to be in the svg code. The first huge numbers seen by cairo, before any of the massive allocs, are here (see args in #1, #2): #0 _cairo_quartz_surface_create_internal (cgContext=0x18481600, content=CAIRO_CONTENT_COLOR_ALPHA, width=48320, height=3500) at /Users/brianewins/projects/ffox/mozilla/gfx/cairo/cairo/src/cairo-quartz-surface.c:1741 #1 0x1176dfb4 in _moz_cairo_quartz_surface_create (format=CAIRO_FORMAT_ARGB32, width=48320, height=3500) at /Users/brianewins/projects/ffox/mozilla/gfx/cairo/cairo/src/cairo-quartz-surface.c:1909 #2 0x11735b50 in gfxQuartzSurface::gfxQuartzSurface (this=0x18483ad0, size=@0xbfffc188, format=ImageFormatARGB32) at /Users/brianewins/projects/ffox/mozilla/gfx/thebes/src/gfxQuartzSurface.cpp:49 #3 0x11735bb5 in gfxQuartzSurface::gfxQuartzSurface (this=0x18483ad0, size=@0xbfffc188, format=ImageFormatARGB32) at /Users/brianewins/projects/ffox/mozilla/gfx/thebes/src/gfxQuartzSurface.cpp:56 #4 0x1173651b in gfxPlatformMac::CreateOffscreenSurface (this=0x13e03e10, size=@0xbfffc248, imageFormat=gfxASurface::ImageFormatARGB32) at /Users/brianewins/projects/ffox/mozilla/gfx/thebes/src/gfxPlatformMac.cpp:69 #5 0x12d8cc84 in nsSVGPatternFrame::PaintPattern (this=0xce7bd8, surface=0xbfffc384, patternMatrix=0xbfffc320, aSource=0xce7cd8, aGraphicOpacity=1) at /Users/brianewins/projects/ffox/mozilla/layout/svg/base/src/nsSVGPatternFrame.cpp:320 ^^ the huge extents are calculated in #5, here: 286 float patternWidth, patternHeight; 287 bbox->GetWidth(&patternWidth); 288 bbox->GetHeight(&patternHeight); this is the first time I see the large extents appearing. Dumping some variables: (gdb) p *(nsSVGRect *)callerBBox.mRawPtr ... mX = 60, mY = 40, mWidth = 604, mHeight = 50 (gdb) p *(nsSVGMatrix *)callerCTM.mRawPtr ... mA = 1, mB = 0, mC = 0, mD = 1, mE = 0, mF = 0 (gdb) p *(nsSVGRect *)bbox.mRawPtr ... mX = 0, mY = 0, mWidth = 48320, mHeight = 3500 ^ this is 80x604, and 70*50. I don't know why they're not both 80 (say) but the fact that these are a multiple of the callerBBox sizes looks suspiciously like a units problem? Its needs an svg person to look at this though. You can trap this bug by setting a conditional breakpoint at _cairo_quartz_surface_create_internal, break if width > 2000 or so and you should see the stack above. Hope this helps someone!
The huge numbers should be clamped by the nsSVGUtils::ConvertToSurfaceSize call to more sensible values before being passed to cairo.
Now I get an OOM complaint from malloc, but no Gecko assertions or crashes.
Flags: wanted1.9.2+
Now I don't even get an OOM complaint. WFM! (but in-testsuite- because it's still slow)
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite-
Resolution: --- → WORKSFORME
Crash Signature: [@ SurfacePatternDrawFunc]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: