Bug 201755 – [Cairo] Image::drawTiled → Cairo::drawSurface → cairo_paint_with_alpha → segfault happens in pixman

• Component: Platform
• Status: RESOLVED FIXED

[Fujii Hironori]

[Cairo] Image::drawTiled → Cairo::drawSurface → cairo_paint_with_alpha → segfault happens in pixman

Today, I saw WinCairo crash in <https://news.yahoo.co.jp>.

Callstack:

> cairo.dll!_pixman_implementation_create_general()	C
> cairo.dll!_pixman_gradient_walker_write_wide()	C
> cairo.dll!pixman_image_composite32()	C
> cairo.dll!composite_boxes(void * _dst, _cairo_operator op, _cairo_surface * abstract_src, _cairo_surface * abstract_mask, int src_x, int src_y, int mask_x, int mask_y, int dst_x, int dst_y, _cairo_boxes_t * boxes, const _cairo_rectangle_int * extents) Line 538	C
> cairo.dll!composite_aligned_boxes(const cairo_spans_compositor * compositor, const _cairo_composite_rectangles * extents, _cairo_boxes_t * boxes) Line 688	C
> cairo.dll!clip_and_composite_boxes(const cairo_spans_compositor * compositor, _cairo_composite_rectangles * extents, _cairo_boxes_t * boxes) Line 883	C
> cairo.dll!_cairo_spans_compositor_mask(const cairo_compositor * _compositor, _cairo_composite_rectangles * extents) Line 1000	C
> cairo.dll!_cairo_compositor_paint(const cairo_compositor * compositor, _cairo_surface * surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 67	C
> cairo.dll!_cairo_image_surface_paint(void * abstract_surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 931	C
> cairo.dll!_cairo_surface_paint(_cairo_surface * surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 2199	C
> cairo.dll!_cairo_gstate_paint(_cairo_gstate * gstate) Line 1061	C
> cairo.dll!_cairo_default_context_paint_with_alpha(void * abstract_cr, double alpha) Line 971	C
> cairo.dll!cairo_paint_with_alpha(_cairo * cr, double alpha) Line 2248	C
> WebKit2.dll!WebCore::Cairo::drawPatternToCairoContext(_cairo * cr, _cairo_pattern * pattern, const WebCore::FloatRect & destRect, float alpha) Line 156	C++
> WebKit2.dll!WebCore::Cairo::drawSurface(WebCore::PlatformContextCairo & platformContext, _cairo_surface * surface, const WebCore::FloatRect & destRect, const WebCore::FloatRect & originalSrcRect, WebCore::InterpolationQuality imageInterpolationQuality, float globalAlpha, const WebCore::Cairo::ShadowState & shadowState) Line 988	C++
> WebKit2.dll!WebCore::Cairo::drawNativeImage(WebCore::PlatformContextCairo & platformContext, _cairo_surface * surface, const WebCore::FloatRect & destRect, const WebCore::FloatRect & srcRect, const WebCore::ImagePaintingOptions & options, float globalAlpha, const WebCore::Cairo::ShadowState & shadowState) Line 903	C++
> WebKit2.dll!WebCore::GraphicsContextImplCairo::drawNativeImage(const WTF::RefPtr<_cairo_surface,WTF::DumbPtrTraits<_cairo_surface> > & image, const WebCore::FloatSize & imageSize, const WebCore::FloatRect & destRect, const WebCore::FloatRect & srcRect, const WebCore::ImagePaintingOptions & options) Line 283	C++
> WebKit2.dll!WebCore::GraphicsContext::drawNativeImage(const WTF::RefPtr<_cairo_surface,WTF::DumbPtrTraits<_cairo_surface> > & image, const WebCore::FloatSize & imageSize, const WebCore::FloatRect & destRect, const WebCore::FloatRect & srcRect, const WebCore::ImagePaintingOptions & options) Line 130	C++
> WebKit2.dll!WebCore::drawNativeImage(const WTF::RefPtr<_cairo_surface,WTF::DumbPtrTraits<_cairo_surface> > & image, WebCore::GraphicsContext & context, const WebCore::FloatRect & destRect, const WebCore::FloatRect & srcRect, const WebCore::IntSize & imageSize, const WebCore::ImagePaintingOptions & options) Line 62	C++
> WebKit2.dll!WebCore::BitmapImage::draw(WebCore::GraphicsContext & context, const WebCore::FloatRect & destRect, const WebCore::FloatRect & srcRect, const WebCore::ImagePaintingOptions & options) Line 272	C++
> WebKit2.dll!WebCore::Image::drawTiled(WebCore::GraphicsContext & ctxt, const WebCore::FloatRect & destRect, const WebCore::FloatPoint & srcPoint, const WebCore::FloatSize & scaledTileSize, const WebCore::FloatSize & spacing, const WebCore::ImagePaintingOptions & options) Line 182	C++
> WebKit2.dll!WebCore::GraphicsContextImpl::drawTiledImageImpl(WebCore::GraphicsContext & context, WebCore::Image & image, const WebCore::FloatRect & destination, const WebCore::FloatPoint & source, const WebCore::FloatSize & tileSize, const WebCore::FloatSize & spacing, const WebCore::ImagePaintingOptions & options) Line 50	C++
> WebKit2.dll!WebCore::GraphicsContextImplCairo::drawTiledImage(WebCore::Image & image, const WebCore::FloatRect & destination, const WebCore::FloatPoint & source, const WebCore::FloatSize & tileSize, const WebCore::FloatSize & spacing, const WebCore::ImagePaintingOptions & imagePaintingOptions) Line 271	C++
> WebKit2.dll!WebCore::GraphicsContext::drawTiledImage(WebCore::Image & image, const WebCore::FloatRect & destination, const WebCore::FloatPoint & source, const WebCore::FloatSize & tileSize, const WebCore::FloatSize & spacing, const WebCore::ImagePaintingOptions & options) Line 740	C++
> WebKit2.dll!WebCore::RenderBoxModelObject::paintFillLayerExtended(const WebCore::PaintInfo & paintInfo, const WebCore::Color & color, const WebCore::FillLayer & bgLayer, const WebCore::LayoutRect & rect, WebCore::BackgroundBleedAvoidance bleedAvoidance, WebCore::InlineFlowBox * box, const WebCore::LayoutSize & boxSize, WebCore::CompositeOperator op, WebCore::RenderElement * backgroundObject, WebCore::BaseBackgroundColorUsage baseBgColorUsage) Line 963	C++
> WebKit2.dll!WebCore::RenderBox::paintFillLayer(const WebCore::PaintInfo & paintInfo, const WebCore::Color & c, const WebCore::FillLayer & fillLayer, const WebCore::LayoutRect & rect, WebCore::BackgroundBleedAvoidance bleedAvoidance, WebCore::CompositeOperator op, WebCore::RenderElement * backgroundObject, WebCore::BaseBackgroundColorUsage baseBgColorUsage) Line 1680	C++
> WebKit2.dll!WebCore::RenderBox::paintFillLayers(const WebCore::PaintInfo & paintInfo, const WebCore::Color & color, const WebCore::FillLayer & fillLayer, const WebCore::LayoutRect & rect, WebCore::BackgroundBleedAvoidance bleedAvoidance, WebCore::CompositeOperator op, WebCore::RenderElement * backgroundObject) Line 1670	C++
> WebKit2.dll!WebCore::RenderBox::paintBackground(const WebCore::PaintInfo & paintInfo, const WebCore::LayoutRect & paintRect, WebCore::BackgroundBleedAvoidance bleedAvoidance) Line 1395	C++
> WebKit2.dll!WebCore::RenderBox::paintBoxDecorations(WebCore::PaintInfo & paintInfo, const WebCore::LayoutPoint & paintOffset) Line 1350	C++
> WebKit2.dll!WebCore::RenderBlock::paintObject(WebCore::PaintInfo & paintInfo, const WebCore::LayoutPoint & paintOffset) Line 1224	C++
> WebKit2.dll!WebCore::RenderBlock::paint(WebCore::PaintInfo & paintInfo, const WebCore::LayoutPoint & paintOffset) Line 1104	C++
> WebKit2.dll!WebCore::RenderLayer::paintBackgroundForFragments(const WTF::Vector<WebCore::LayerFragment,1,WTF::CrashOnOverflow,16> & layerFragments, WebCore::GraphicsContext & context, WebCore::GraphicsContext & contextForTransparencyLayer, const WebCore::LayoutRect & transparencyPaintDirtyRect, bool haveTransparency, const WebCore::RenderLayer::LayerPaintingInfo & localPaintingInfo, WTF::OptionSet<WebCore::PaintBehavior> paintBehavior, WebCore::RenderObject * subtreePaintRootForRenderer) Line 4913	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4554	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4250	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerWithEffects(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4233	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4172	C++
> WebKit2.dll!WebCore::RenderLayer::paintList(WebCore::RenderLayer::LayerList layerIterator, WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4689	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4581	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4250	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerWithEffects(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4233	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4172	C++
> WebKit2.dll!WebCore::RenderLayer::paintList(WebCore::RenderLayer::LayerList layerIterator, WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4689	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4581	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4250	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayerWithEffects(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4233	C++
> WebKit2.dll!WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext & context, const WebCore::RenderLayer::LayerPaintingInfo & paintingInfo, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags) Line 4172	C++
> WebKit2.dll!WebCore::RenderLayer::paint(WebCore::GraphicsContext & context, const WebCore::LayoutRect & damageRect, const WebCore::LayoutSize & subpixelOffset, WTF::OptionSet<WebCore::PaintBehavior> paintBehavior, WebCore::RenderObject * subtreePaintRoot, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag> paintFlags, WebCore::RenderLayer::SecurityOriginPaintPolicy paintPolicy) Line 4034	C++
> WebKit2.dll!WebCore::FrameView::paintContents(WebCore::GraphicsContext & context, const WebCore::IntRect & dirtyRect, WebCore::Widget::SecurityOriginPaintPolicy securityOriginPaintPolicy) Line 4156	C++
> WebKit2.dll!WebCore::ScrollView::paint(WebCore::GraphicsContext & context, const WebCore::IntRect & rect, WebCore::Widget::SecurityOriginPaintPolicy securityOriginPaintPolicy) Line 1217	C++
> WebKit2.dll!WebKit::WebPage::drawRect(WebCore::GraphicsContext & graphicsContext, const WebCore::IntRect & rect) Line 1792	C++
> WebKit2.dll!WebKit::DrawingAreaCoordinatedGraphics::display(WebKit::UpdateInfo & updateInfo) Line 760	C++
> WebKit2.dll!WebKit::DrawingAreaCoordinatedGraphics::display() Line 673	C++
> WebKit2.dll!WebKit::DrawingAreaCoordinatedGraphics::displayTimerFired() Line 653	C++
> WebKit2.dll!WTF::RunLoop::Timer<WebKit::DrawingAreaCoordinatedGraphics>::fired() Line 152	C++
> WTF.dll!WTF::RunLoop::TimerBase::timerFired() Line 141	C++
> WTF.dll!WTF::RunLoop::wndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 60	C++
> WTF.dll!WTF::RunLoop::RunLoopWndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 39	C++
> [External Code]	
> WTF.dll!WTF::RunLoop::run() Line 73	C++
> WebKit2.dll!WebKit::AuxiliaryProcessMain<WebKit::WebProcess,WebKit::WebProcessMain>(int argc, char * * argv) Line 67	C++
> WebKit2.dll!WebKit::WebProcessMainWin(int argc, char * * argv) Line 49	C++
> WebKitWebProcess.exe!main(int argc, char * * argv) Line 33	C++
> [External Code]	

This callstack is not reliable for pixman function names because the following issue isn't fixed yet.

pixman doesn't have the debug information · Issue #26 · WebKitForWindows/WinCairoRequirements
https://github.com/WebKitForWindows/WinCairoRequirements/issues/26

[Fujii Hironori]

In Image::drawTiled, this Image size was 640x435.
But, visibleSrcRect was 640.0x435.00031.

In Cairo::drawSurface, it create a subsurface by taking enclosingIntRect of srcRect.
https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp?rev=249217#L935

> IntRect expandedSrcRect(enclosingIntRect(srcRect));

I guess expandedSrcRect would be 640x436.

[Fujii Hironori]

Cairo's bug ticket:

Crash in fast_composite_scaled_bilinear_sse2_8888_8888_cover_SRC (#85) · Issues · cairo / cairo · GitLab
https://gitlab.freedesktop.org/cairo/cairo/issues/85

[Fujii Hironori]

I think there are two problems.

1.
In Image::drawTiled, visibleSrcRect can be larger than the image size because it is calculated by using division twice, as
 FloatSize scale(scaledTileSize / intrinsicTileSize);
 visibleSrcRect.setWidth(destRect.width() / scale.width());
This can be converted into one division and one multiplication:
 FloatSize scale(intrinsicTileSize / scaledTileSize);
 visibleSrcRect.setWidth(destRect.width() * scale.width());

2.
Cairo::drawSurface is creating a larger subsurface than the original surface.

[Fujii Hironori]

Created attachment 378720 [details]
Patch

[Fujii Hironori]

It's easy to check srcRect is slightly larger than the surface size by adding the following ASSERT.

diff --git a/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp b/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp
index a895dadd042..12a691b3d28 100644
--- a/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp
+++ b/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp
@@ -933,6 +933,7 @@ void drawSurface(PlatformContextCairo& platformContext, cairo_surface_t* surface
     if (srcRect.x() || srcRect.y() || srcRect.size() != cairoSurfaceSize(surface)) {
         // Cairo subsurfaces don't support floating point boundaries well, so we expand the rectangle.
         IntRect expandedSrcRect(enclosingIntRect(srcRect));
+        ASSERT(IntRect({ }, cairoSurfaceSize(surface)).contains(expandedSrcRect));
 
         // We use a subsurface here so that we don't end up sampling outside the originalSrcRect rectangle.
         // See https://bugs.webkit.org/show_bug.cgi?id=58309


It's easy to reproduce similar crashes by expanding srcRect 1px taller.

diff --git a/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp b/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp
index a895dadd042..db63260c36b 100644
--- a/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp
+++ b/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp
@@ -916,6 +916,7 @@ void drawSurface(PlatformContextCairo& platformContext, cairo_surface_t* surface
         return;
 
     FloatRect srcRect = originalSrcRect;
+    srcRect.expand({0, 1});
 
     // We need to account for negative source dimensions by flipping the rectangle.
     if (originalSrcRect.width() < 0) {


But, it's difficult to create a test case reproducing the crash.

[Fujii Hironori]

Comment on attachment 378720 [details]
Patch

Clearing flags on attachment: 378720

Committed r249937: <https://trac.webkit.org/changeset/249937>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.