1343511 - ppc64 - Firefox 52 crash at mozilla::gfx::CopyRect

Status: NEW

(Core :: Graphics, defect, P3)

Description

[Martin Stránský [:stransky] (ni? me)]

Crash at mozilla::gfx::CopyRect(), Big endian, in:

MOZ_RELEASE_ASSERT(aSrc->GetFormat() == aDest->GetFormat(),...)

FilterNodeSoftware tries to render to two different surfaces.
aSrc->GetFormat() = mozilla::gfx::B8G8R8A8
aDest->GetFormat() = mozilla::gfx::A8R8G8B8

Backtrace is at:
https://bugzilla.redhat.com/attachment.cgi?id=1256457

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: fix SourceSurfaceCairo formats on big-endian

Does this patch help, Martin?

I am guessing that the source surface was a SourceSurfaceCairo, in which case this function was clearly not right and losing the endianness of the format. The ones in HelpersCairo.h do it correctly, though.

Comment 2

[Martin Stránský [:stransky] (ni? me)]

Thanks, I tested that but it does not help, the crash seems to me somewhere else. The ppc64 build produces lots of error messages like:

GraphicsCriticalError: |[0][GFX1]: Unknown image format 0|
GraphicsCriticalError: |[0][GFX1]: Unknown image format 1|

which means mozilla::gfx::B8G8R8A8 and mozilla::gfx::B8G8R8X8 does not match expected  mozilla::gfx::A8R8G8B8_UINT32 one and here this patch helps.
I fixed lots of the places where error is reported but FilterNodeSoftware module seems to be specific and require exact "mozilla::gfx::B8G8R8A8" format. I still investigate that.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

The problem is actually here: 
https://dxr.mozilla.org/mozilla-central/source/gfx/2d/HelpersCairo.h?q=CairoFormatToGfxFormat&redirect_type=direct#255

CairoFormatToGfxFormat() translates CAIRO_FORMAT_ARGB32 to BE/LE variants SurfaceFormat::A8R8G8B8_UINT32 while nsFilter* modules has hardcoded SurfaceFormat::B8R8G8A8 everywhere which does not match on BE.

backtrace where the incompatible SurfaceFormat::A8R8G8B8_UINT32 surface is created:

#0  0x00003fff78044250 in mozilla::gfx::CairoFormatToGfxFormat(cairo_format_t) (format=CAIRO_FORMAT_ARGB32)
    at /root/rpmbuild/BUILD/firefox-52-debug/gfx/2d/HelpersCairo.h:242
#1  0x00003fff7804473c in mozilla::gfx::GfxFormatForCairoSurface(_cairo_surface*) (surface=0x3fff3fc12130)
    at /root/rpmbuild/BUILD/firefox-52-debug/gfx/2d/DrawTargetCairo.cpp:690
#2  0x00003fff78044e5c in mozilla::gfx::DrawTargetCairo::Snapshot() (this=0x3fff3f7a9700)
    at /root/rpmbuild/BUILD/firefox-52-debug/gfx/2d/DrawTargetCairo.cpp:718
#3  0x00003fff79d7b118 in nsFilterInstance::BuildSourceImage(mozilla::gfx::DrawTarget*) (this=this@entry=0x3ffffa749ee0, aTargetDT=aTargetDT@entry=0x3fff3f7a1be0) at /root/rpmbuild/BUILD/firefox-52-debug/layout/svg/nsFilterInstance.cpp:475
#4  0x00003fff79d7b470 in nsFilterInstance::Render(mozilla::gfx::DrawTarget*) (this=this@entry=0x3ffffa749ee0, aDrawTarget=aDrawTarget@entry=0x3fff3f7a1be0) at /root/rpmbuild/BUILD/firefox-52-debug/layout/svg/nsFilterInstance.cpp:501
#5  0x00003fff79d89ea8 in nsFilterInstance::PaintFilteredFrame(nsIFrame*, mozilla::gfx::DrawTarget*, gfxMatrix const&, nsSVGFilterPaintCallback*, nsRegion const*) (aFilteredFrame=<optimized out>, aDrawTarget=0x3fff3f7a1be0, aTransform=..., aPaintCallback=0x3ffffa74a1e8, aDirtyArea=0x3ffffa74a208) at /root/rpmbuild/BUILD/firefox-52-debug/layout/svg/nsFilterInstance.cpp:77
#6  0x00003fff79da4f00 in nsSVGIntegrationUtils::PaintFilter(nsSVGIntegrationUtils::PaintFramesParams const&) (aParams=...)
    at /root/rpmbuild/BUILD/firefox-52-debug/layout/svg/nsSVGIntegrationUtils.cpp:957
#7  0x00003fff79b8e378 in nsDisplayFilter::PaintAsLayer(nsDisplayListBuilder*, nsRenderingContext*, mozilla::layers::LayerManager*) (this=0x3fff4f5bd1f8, aBuilder=<optimized out>, aCtx=<optimized out>, aManager=<optimized out>)
    at /root/rpmbuild/BUILD/firefox-52-debug/layout/base/nsDisplayList.cpp:7411
#8  0x00003fff79b1aec8 in mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::FrameLayerBuilder::ClippedDisplayItem>&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsRenderingContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const&, float, float, int) (aCtx=0x3ffffa74a720, aContext=0x3fff3fca0550, aItem=0x3fff4f5bd1f8, aManager=0x3fff3faaf500, aBuilder=0x3ffffa74b360)
    at /root/rpmbuild/BUILD/firefox-52-debug/layout/base/FrameLayerBuilder.cpp:3749
#9  0x00003fff79b1aec8 in mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::FrameLayerBuilder::ClippedDisplayItem>&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsRenderingContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const&, float, float, int) (this=this@entry=0x3fff3fc63340, aItems=..., aRect=..., aContext=aContext@entry=0x3fff3fca0550, aRC=aRC@entry=0x3ffffa74a720, aBuilder=aBuilder@entry=0x3ffffa74b360, aPresContext=aPresContext@entry=0x3fff512ba800, aOffset=..., aXScale=1, aYScale=<optimized out>, aCommonClipCount=0)
    at /root/rpmbuild/BUILD/firefox-52-debug/layout/base/FrameLayerBuilder.cpp:5914
#10 0x00003fff79b1b8f4 in mozilla::FrameLayerBuilder::DrawPaintedLayer(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*) (aLayer=0x3fff3fc37c00, aContext=0x3fff3fca0550, aRegionToDraw=..., aDirtyRegion=..., aClip=<optimized out>, aRegionToInvalidate=..., aCallbackData=0x3ffffa74b360) at /root/rpmbuild/BUILD/firefox-52-debug/layout/base/FrameLayerBuilder.cpp:6106
#11 0x00003fff78146eec in mozilla::layers::ClientPaintedLayer::PaintThebes() (this=this@entry=0x3fff3fc37c00)
    at /root/rpmbuild/BUILD/firefox-52-debug/gfx/layers/client/ClientPaintedLayer.cpp:89
#12 0x00003fff7814bf8c in mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) (this=0x3fff3fc37c00, aReadback=0x3ffffa74aaf0)
    at /root/rpmbuild/BUILD/firefox-52-debug/gfx/layers/client/ClientPaintedLayer.cpp:137
#13 0x00003fff7814dcec in mozilla::layers::ClientContainerLayer::RenderLayer() (this=0x3fff3fc37400)
    at /root/rpmbuild/BUILD/firefox-52-debug/gfx/layers/client/ClientContainerLayer.h:62
#14 0x00003fff78145ee0 in mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) (this=this@entry=0x3fff7e1a22f0, aCallback=aCallback@entry=
    @0x3fff7b99d770: 0x3fff79b1b1b8 <mozilla::FrameLayerBuilder::DrawPaintedLayer(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> con

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Attachment: patch

This patch fixes the crash although I'm not sure it's 100% correct.

Comment 5

[Lee Salzman [:lsalzman]]

(In reply to Martin Stránský from comment #4)
> Created attachment 8842861 [details] [diff] [review]
> patch
> 
> This patch fixes the crash although I'm not sure it's 100% correct.

That patch is dangerous because it doesn't entirely fix the issue of how DrawTargetCairo disregards the Moz2D convention and uses the _UINT32 version.
So you need to make sure ALL Cairo rendering functionality works when converted to B8G8R8A8 if you were to start doing that.

Comment 6

[Lee Salzman [:lsalzman]]

(In reply to Lee Salzman [:lsalzman] from comment #5)
> (In reply to Martin Stránský from comment #4)
> > Created attachment 8842861 [details] [diff] [review]
> > patch
> > 
> > This patch fixes the crash although I'm not sure it's 100% correct.
> 
> That patch is dangerous because it doesn't entirely fix the issue of how
> DrawTargetCairo disregards the Moz2D convention and uses the _UINT32 version.
> So you need to make sure ALL Cairo rendering functionality works when
> converted to B8G8R8A8 if you were to start doing that.

The other thing that may be worth investigating is to just change Moz2d in its entirety to use the _UINT32 enums where it means them.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

(In reply to Lee Salzman [:lsalzman] from comment #6)
> (In reply to Lee Salzman [:lsalzman] from comment #5)
> > (In reply to Martin Stránský from comment #4)
> > > Created attachment 8842861 [details] [diff] [review]
> > > patch
> > > 
> > > This patch fixes the crash although I'm not sure it's 100% correct.
> > 
> > That patch is dangerous because it doesn't entirely fix the issue of how
> > DrawTargetCairo disregards the Moz2D convention and uses the _UINT32 version.
> > So you need to make sure ALL Cairo rendering functionality works when
> > converted to B8G8R8A8 if you were to start doing that.
> 
> The other thing that may be worth investigating is to just change Moz2d in
> its entirety to use the _UINT32 enums where it means them.

I tried to do so but that didn't work as nsFilter* modules has hardcoded SurfaceFormat::B8R8G8A8 and fails with the LE/BE pixel format variants. I guess it needs to be solved in nsFilter* first.