Closed Bug 920571 Opened 11 years ago Closed 11 years ago

Very frequent crashes under SourceSurfaceCG::InitFromData

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla27

People

(Reporter: dzbarsky, Unassigned)

References

Details

(Keywords: regression)

Attachments

(1 file)

Don't read from the trailing stride
1.63 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review 
I started seeing this crash very frequently starting a few days ago.  This is a nightly build, but I can try to reproduce in a debug build if that will help.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000169200000
0x00007fff92c59ac1 in memmove$VARIANT$sse42 ()
(gdb) bt
#0  0x00007fff92c59ac1 in memmove$VARIANT$sse42 ()
#1  0x0000000102e0b6a6 in mozilla::gfx::SourceSurfaceCG::InitFromData ()
#2  0x0000000102e0c871 in mozilla::gfx::DrawTargetCG::CreateSourceSurfaceFromData ()
#3  0x0000000102a486a3 in gfxPlatform::GetSourceSurfaceForSurface ()
#4  0x0000000102a45f03 in gfxPattern::GetPattern ()
#5  0x0000000102a1ba8d in gfxContext::FillAzure ()
#6  0x0000000102a1b85b in gfxContext::Fill ()
#7  0x0000000102a24183 in gfxSurfaceDrawable::Draw ()
#8  0x0000000102a55466 in gfxUtils::DrawPixelSnapped ()
#9  0x0000000101667d9d in imgFrame::Draw ()
#10 0x000000010166115c in mozilla::image::RasterImage::DrawWithPreDownscaleIfNeeded ()
#11 0x000000010166133a in mozilla::image::RasterImage::Draw ()
#12 0x0000000101717a25 in DrawImageInternal ()
#13 0x0000000101717fdd in nsLayoutUtils::DrawBackgroundImage ()
#14 0x00000001016dbf38 in nsImageRenderer::DrawBackground ()
#15 0x00000001016d8428 in nsCSSRendering::PaintBackgroundWithSC ()
#16 0x00000001016d76c0 in nsCSSRendering::PaintBackground ()
#17 0x00000001016f2891 in nsDisplayBackgroundImage::Paint ()
#18 0x00000001016af6b8 in mozilla::FrameLayerBuilder::DrawThebesLayer ()
#19 0x0000000102a75d2d in mozilla::layers::ClientThebesLayer::PaintBuffer ()
#20 0x0000000102a75c43 in mozilla::layers::ClientThebesLayer::PaintThebes ()
#21 0x0000000102a75ec7 in mozilla::layers::ClientThebesLayer::RenderLayer ()
#22 0x0000000102a73cb8 in mozilla::layers::ClientContainerLayer::RenderLayer ()
#23 0x0000000102a73cb8 in mozilla::layers::ClientContainerLayer::RenderLayer ()
#24 0x0000000102a73cb8 in mozilla::layers::ClientContainerLayer::RenderLayer ()
#25 0x0000000102a74f5e in mozilla::layers::ClientLayerManager::EndTransactionInternal ()
#26 0x0000000102a74fea in mozilla::layers::ClientLayerManager::EndTransaction ()
#27 0x00000001016ef521 in nsDisplayList::PaintForFrame ()
#28 0x00000001016eeeec in nsDisplayList::PaintRoot ()
#29 0x000000010171340c in nsLayoutUtils::PaintFrame ()
#30 0x0000000101732060 in PresShell::Paint ()
Perhaps caused by bug 912944
Steps to reproduce?
Keywords: regression, regressionwindow-wanted
(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> Perhaps caused by bug 912944

I haven't actually landed that yet, so probably not.

Not sure what could cause this. Maybe the gfxImageSurface we get the data from has had its data freed out from under it, or is otherwise invalid in some way?
(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> Steps to reproduce?

I have a Wunderlist tab pinned as an app and Firefox crashes most of the time when I switch to it.
Here's a debug stack if it helps:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x000000016d500000
0x00007fff92c59ab0 in memmove$VARIANT$sse42 ()
(gdb) bt
#0  0x00007fff92c59ab0 in memmove$VARIANT$sse42 ()
#1  0x00000001050430a6 in mozilla::gfx::SourceSurfaceCG::InitFromData (this=0x1639de0e0, aData=0x16cc0057c "'G??.H??1K??2L??1K??2L??4N??/I??'A??+E??)C??'A??%???&@??)C??-G??0J??0J??+E??'A??'A??+E??.H??-G??*D??-G??,F??'C??)E??.L??*H??*H??4T??7U??1O??/M??5S??9W??7U??3Q??2P??;U??.H??)C??,F??-G??1K??0J??&@??7U??"..., aSize=@0x7fff5fbf90d8, aStride=8192, aFormat=mozilla::gfx::FORMAT_B8G8R8X8) at /Users/dzbarsky/mozilla/inbound/gfx/2d/SourceSurfaceCG.cpp:130
#2  0x0000000105044dd4 in mozilla::gfx::DrawTargetCG::CreateSourceSurfaceFromData (this=0x11cced680, aData=0x16cc0057c "'G??.H??1K??2L??1K??2L??4N??/I??'A??+E??)C??'A??%???&@??)C??-G??0J??0J??+E??'A??'A??+E??.H??-G??*D??-G??,F??'C??)E??.L??*H??*H??4T??7U??1O??/M??5S??9W??7U??3Q??2P??;U??.H??)C??,F??-G??1K??0J??&@??7U??"..., aSize=@0x7fff5fbf90d8, aStride=8192, aFormat=mozilla::gfx::FORMAT_B8G8R8X8) at /Users/dzbarsky/mozilla/inbound/gfx/2d/DrawTargetCG.cpp:205
#3  0x00000001047255d3 in gfxPlatform::GetSourceSurfaceForSurface (this=0x11becd1a0, aTarget=0x11cced680, aSurface=0x13f3ead40) at /Users/dzbarsky/mozilla/inbound/gfx/thebes/gfxPlatform.cpp:736
#4  0x0000000104720a35 in gfxPattern::GetPattern (this=0x11ee84660, aTarget=0x11cced680, aPatternTransform=0x0) at /Users/dzbarsky/mozilla/inbound/gfx/thebes/gfxPattern.cpp:188
#5  0x00000001046d7b46 in GeneralPattern::operator mozilla::gfx::Pattern& (this=0x7fff5fbf98b0) at /Users/dzbarsky/mozilla/inbound/gfx/thebes/gfxContext.cpp:48
#6  0x00000001046cf18a in gfxContext::FillAzure (this=0x11bc65ca0, aOpacity=1) at /Users/dzbarsky/mozilla/inbound/gfx/thebes/gfxContext.cpp:2110
#7  0x00000001046ceef8 in gfxContext::Fill (this=0x11bc65ca0) at /Users/dzbarsky/mozilla/inbound/gfx/thebes/gfxContext.cpp:340
#8  0x00000001046dce01 in gfxSurfaceDrawable::Draw (this=0x13f3eada0, aContext=0x11bc65ca0, aFillRect=@0x7fff5fbf9d40, aRepeat=false, aFilter=@0x7fff5fbf9bf8, aTransform=@0x7fff5fbf9b50) at /Users/dzbarsky/mozilla/inbound/gfx/thebes/gfxDrawable.cpp:133
#9  0x000000010473e335 in gfxUtils::DrawPixelSnapped (aContext=0x11bc65ca0, aDrawable=0x13f3eac80, aUserSpaceToImageSpace=@0x7fff5fbf9dc0, aSubimage=@0x7fff5fbf9d60, aSourceRect=@0x7fff5fbf9da0, aImageRect=@0x7fff5fbf9d80, aFill=@0x7fff5fbf9d40, aFormat=gfxImageFormatRGB24, aFilter=gfxPattern::FILTER_GOOD, aImageFlags=24) at /Users/dzbarsky/mozilla/inbound/gfx/thebes/gfxUtils.cpp:490
#10 0x0000000101c5b581 in imgFrame::Draw (this=0x13f4152a0, aContext=0x11bc65ca0, aFilter=gfxPattern::FILTER_GOOD, aUserSpaceToImageSpace=@0x7fff5fbf9fc0, aFill=@0x7fff5fbfa230, aPadding=@0x7fff5fbf9f10, aSubimage=@0x7fff5fbf9f70, aImageFlags=24) at /Users/dzbarsky/mozilla/inbound/image/src/imgFrame.cpp:487
#11 0x0000000101c44701 in mozilla::image::RasterImage::DrawWithPreDownscaleIfNeeded (this=0x1676d2480, aFrame=0x13f4152a0, aContext=0x11bc65ca0, aFilter=gfxPattern::FILTER_GOOD, aUserSpaceToImageSpace=@0x7fff5fbfa200, aFill=@0x7fff5fbfa230, aSubimage=@0x7fff5fbfa250, aFlags=24) at /Users/dzbarsky/mozilla/inbound/image/src/RasterImage.cpp:2570
#12 0x0000000101c44ad5 in mozilla::image::RasterImage::Draw (this=0x1676d2480, aContext=0x11bc65ca0, aFilter=gfxPattern::FILTER_GOOD, aUserSpaceToImageSpace=@0x7fff5fbfa200, aFill=@0x7fff5fbfa230, aSubimage=@0x7fff5fbfa250, aWhichFrame=1, aFlags=24) at /Users/dzbarsky/mozilla/inbound/image/src/RasterImage.cpp:2655
#13 0x0000000101dd0f97 in DrawImageInternal (aRenderingContext=0x153d99480, aImage=0x1676d2480, aGraphicsFilter=gfxPattern::FILTER_GOOD, aDest=@0x7fff5fbfa640, aFill=@0x7fff5fbfa650, aAnchor=@0x7fff5fbfa5d0, aDirty=@0x7fff5fbfa6e0, aImageSize=@0x7fff5fbfa408, aSVGContext=0x0, aImageFlags=24) at /Users/dzbarsky/mozilla/inbound/layout/base/nsLayoutUtils.cpp:4089
#14 0x0000000101dd1650 in nsLayoutUtils::DrawBackgroundImage (aRenderingContext=0x153d99480, aImage=0x1676d2480, aImageSize=@0x7fff5fbfa408, aGraphicsFilter=gfxPattern::FILTER_GOOD, aDest=@0x7fff5fbfa640, aFill=@0x7fff5fbfa650, aAnchor=@0x7fff5fbfa5d0, aDirty=@0x7fff5fbfa6e0, aImageFlags=16) at /Users/dzbarsky/mozilla/inbound/layout/base/nsLayoutUtils.cpp:4250
#15 0x0000000101d495f0 in nsImageRenderer::DrawBackground (this=0x7fff5fbfa5d8, aPresContext=0x13ef54800, aRenderingContext=@0x153d99480, aDest=@0x7fff5fbfa640, aFill=@0x7fff5fbfa650, aAnchor=@0x7fff5fbfa5d0, aDirty=@0x7fff5fbfa6e0) at /Users/dzbarsky/mozilla/inbound/layout/base/nsCSSRendering.cpp:4801
#16 0x0000000101d44bbb in nsCSSRendering::PaintBackgroundWithSC (aPresContext=0x13ef54800, aRenderingContext=@0x153d99480, aForFrame=0x11a473800, aDirtyRect=@0x1617c6d30, aBorderArea=@0x7fff5fbfa9f8, aBackgroundSC=0x117d264b0, aBorder=@0x16765a020, aFlags=4, aBGClipRect=0x0, aLayer=0) at /Users/dzbarsky/mozilla/inbound/layout/base/nsCSSRendering.cpp:2742
#17 0x0000000101d43dc1 in nsCSSRendering::PaintBackground (aPresContext=0x13ef54800, aRenderingContext=@0x153d99480, aForFrame=0x11a473800, aDirtyRect=@0x1617c6d30, aBorderArea=@0x7fff5fbfa9f8, aFlags=4, aBGClipRect=0x0, aLayer=0) at /Users/dzbarsky/mozilla/inbound/layout/base/nsCSSRendering.cpp:1563
#18 0x0000000101d790f5 in nsDisplayBackgroundImage::PaintInternal (this=0x1617c6d00, aBuilder=0x7fff5fbfbd40, aCtx=0x153d99480, aBounds=@0x1617c6d30, aClipRect=0x0) at /Users/dzbarsky/mozilla/inbound/layout/base/nsDisplayList.cpp:2190
#19 0x0000000101d78fe6 in nsDisplayBackgroundImage::Paint (this=0x1617c6d00, aBuilder=0x7fff5fbfbd40, aCtx=0x153d99480) at /Users/dzbarsky/mozilla/inbound/layout/base/nsDisplayList.cpp:2177
#20 0x0000000101ce5405 in mozilla::FrameLayerBuilder::DrawThebesLayer (aLayer=0x16420f400, aContext=0x11bc65ca0, aRegionToDraw=@0x7fff5fbfb010, aRegionToInvalidate=@0x7fff5fbfb098, aCallbackData=0x7fff5fbfbd40) at /Users/dzbarsky/mozilla/inbound/layout/base/FrameLayerBuilder.cpp:3302

I currently have this up in a debugger, is there anything else I can do to help?
I think this is a regression from bug 917703.

We create a gfxSubimageSurface which uses a sub-rect of the original image, which shares the underlying data (and stride), just with a different size and start position.

This stride isn't actually correct for the very last row of pixels, since the stride takes us to the start position of the next row (which doesn't exist) and may be past the end of the allocation. If this crosses a page boundary, then trying to read it will crash.
This seems like a pretty subtle bug, and easy mistake to make. It would be nice if we could find a way to prevent it somehow.
Attachment #810845 - Flags: review?(jmuizelaar)
Comment on attachment 810845 [details] [diff] [review]
Don't read from the trailing stride

Review of attachment 810845 [details] [diff] [review]:
-----------------------------------------------------------------

Can you fix this for the other backends too?
Attachment #810845 - Flags: review?(jmuizelaar) → review-
There is an assumption in code that the buffer for gfxImageSurface is at least stride*height (sometimes larger.)  This was one bad thing that happened when that is not the case, perhaps it isn't the only one?  Perhaps the gfxImageSurface could probably be made safer, and the code clearer, if we also kept track of the size of the buffer?
Comment on attachment 810845 [details] [diff] [review]
Don't read from the trailing stride

Review of attachment 810845 [details] [diff] [review]:
-----------------------------------------------------------------

Can you fix this for the other backends too?
Comment on attachment 810845 [details] [diff] [review]
Don't read from the trailing stride

This doesn't need to be an r-
Attachment #810845 - Flags: review- → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/f79b68cb9b1b

Looks like the other backends get this right, with the possible exception of D2D (since that's handled internally, the docs don't mention what the behaviour for the last line).
https://hg.mozilla.org/mozilla-central/rev/f79b68cb9b1b
https://hg.mozilla.org/mozilla-central/rev/ff6b715cefdc
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Bug 923172 is a dup of this bug, but has a lot more information.
Issue is resolved - clearing old keywords - qa-wanted clean-up
Keywords: regressionwindow-wanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: