1648821 - MOZ_SKIA_DISABLE_ASSERTS=1 does not suppress skia assertions on fuzzing opt builds

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

I think this should have been suppressed via MOZ_SKIA_DISABLE_ASSERTS=1 but it was not. We are seeing many instances of similar issues that also seem to be failed suppression. These are getting in the way of fuzzing and limiting the effectiveness of the fuzzers. Please prioritize this issue accordingly[1].

[1] https://firefox-source-docs.mozilla.org/tools/fuzzing/index.html#fuzz-blockers

#0 0x55c2e516cf59 in mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:33:3
#1 0x7fdadccd16f4 in sk_abort_no_print() gfx/skia/skia/src/ports/SkMemory_mozalloc.cpp:24:5
#2 0x7fdad4aeba24 in operator() gfx/skia/skia/include/private/SkTDArray.h:361:9
#3 0x7fdad4aeba24 in SkTDArray<short>::resizeStorageToAtLeast(int) gfx/skia/skia/include/private/SkTDArray.h:361:9
#4 0x7fdad4aea785 in mozilla::gfx::ConvolutionFilter::ComputeResizeFilter(mozilla::gfx::ConvolutionFilter::ResizeMethod, int, int) gfx/2d/ConvolutionFilter.cpp:105:12
#5 0x7fdad5a9ea08 in nsresult mozilla::image::DownscalingFilter<mozilla::image::ColorManagementFilter<mozilla::image::SurfaceSink> >::Configure<mozilla::image::ColorManagementConfig, mozilla::image::SurfaceConfig>(mozilla::image::DownscalingConfig const&, mozilla::image::ColorManagementConfig const&, mozilla::image::SurfaceConfig const&) image/DownscalingFilter.h:146:19
#6 0x7fdad5a8ef66 in mozilla::Maybe<mozilla::image::SurfacePipe> mozilla::image::SurfacePipeFactory::MakePipe<mozilla::image::DownscalingConfig, mozilla::image::ColorManagementConfig, mozilla::image::SurfaceConfig>(mozilla::image::DownscalingConfig const&, mozilla::image::ColorManagementConfig const&, mozilla::image::SurfaceConfig const&) image/SurfacePipeFactory.h:587:25
#7 0x7fdad5a2ba12 in mozilla::image::SurfacePipeFactory::CreateSurfacePipe(mozilla::image::Decoder*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, mozilla::gfx::SurfaceFormat, mozilla::Maybe<mozilla::image::AnimationParams> const&, _qcms_transform*, mozilla::image::SurfacePipeFlags) image/SurfacePipeFactory.h:482:22
#8 0x7fdad5a3a864 in mozilla::image::nsBMPDecoder::AllocateSurface() image/decoders/nsBMPDecoder.cpp:891:29
#9 0x7fdad5b07dd5 in mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_0::operator()(mozilla::image::nsBMPDecoder::State, char const*, unsigned long) const image/decoders/nsBMPDecoder.cpp:450:20
#10 0x7fdad5a3359a in BufferedRead<(lambda at image/decoders/nsBMPDecoder.cpp:432:7)> image/StreamingLexer.h:605:11
#11 0x7fdad5a3359a in Lex<(lambda at image/decoders/nsBMPDecoder.cpp:432:7)> image/StreamingLexer.h:470:26
#12 0x7fdad5a3359a in mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*) image/decoders/nsBMPDecoder.cpp:430:17
#13 0x7fdad58c02c7 in mozilla::image::Decoder::Decode(mozilla::image::IResumable*) image/Decoder.cpp:172:19
#14 0x7fdad58d3580 in mozilla::image::DecodedSurfaceProvider::Run() image/DecodedSurfaceProvider.cpp:122:34
#15 0x7fdad5900398 in mozilla::image::DecodePoolWorker::Run() image/DecodePool.cpp:276:23
#16 0x7fdad1eb79e5 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1234:14
#17 0x7fdad1ec28dc in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:504:10
#18 0x7fdad3255942 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:302:20
#19 0x7fdad312f607 in RunInternal ipc/chromium/src/base/message_loop.cc:316:10
#20 0x7fdad312f607 in RunHandler ipc/chromium/src/base/message_loop.cc:309:3
#21 0x7fdad312f607 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:291:3
#22 0x7fdad1eb02a7 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:447:10
#23 0x7fdaf7928d3e in _pt_root nsprpub/pr/src/pthreads/ptthread.c:201:5
#24 0x7fdaf756f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
#25 0x7fdaf659541c in clone /build/glibc-LK5gWL/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Comment 1

[Tyson Smith [:tsmith]]

Attachment: testdata.bmp

Comment 2

[Tyson Smith [:tsmith]]

Maybe this is legit? I can open a seprate issue for that if needed. On a debug build with MOZ_SKIA_DISABLE_ASSERTS=1 set I see:

rax = 0x00007ffed9a0e937   rdx = 0x000055d0e12d42e0
rcx = 0x0000000000000003   rbx = 0x0000000000000000
rsi = 0x0000000000000000   rdi = 0x000055d0e12d42e0
rbp = 0x00007f9a8484bdf0   rsp = 0x00007f9a8484bde8
r8 = 0x0000000000000000    r9 = 0x0000000000000004
r10 = 0xfffffffffffff944   r11 = 0x0000000000000000
r12 = 0x0000000000000000   r13 = 0x00007f9a50005560
r14 = 0x000000006c000000   r15 = 0x0000000000000001
rip = 0x00007f9a50005ae0
OS|Linux|0.0.0 Linux 4.15.0-106-generic #107~16.04.1-Ubuntu SMP Thu Jun 4 15:40:05 UTC 2020 x86_64
CPU|amd64|family 6 model 158 stepping 9|4
GPU|||
Crash|SIGSEGV|0x7f9a50005ae0|15
15|0|||||0x7f9a50005ae0
15|1|libxul.so|mozilla::gfx::ConvolutionFilter::ComputeResizeFilter(mozilla::gfx::ConvolutionFilter::ResizeMethod, int, int)|hg:hg.mozilla.org/mozilla-central:gfx/2d/ConvolutionFilter.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|105|0x1d
15|2|libxul.so|nsresult mozilla::image::DownscalingFilter<mozilla::image::ColorManagementFilter<mozilla::image::SurfaceSink> >::Configure<mozilla::image::ColorManagementConfig, mozilla::image::SurfaceConfig>(mozilla::image::DownscalingConfig const&, mozilla::image::ColorManagementConfig const&, mozilla::image::SurfaceConfig const&)|hg:hg.mozilla.org/mozilla-central:image/DownscalingFilter.h:324d5257f6f7cd287565dd822b13d3b5acc0ef14|146|0x1d
15|3|libxul.so|mozilla::Maybe<mozilla::image::SurfacePipe> mozilla::image::SurfacePipeFactory::MakePipe<mozilla::image::DownscalingConfig, mozilla::image::ColorManagementConfig, mozilla::image::SurfaceConfig>(mozilla::image::DownscalingConfig const&, mozilla::image::ColorManagementConfig const&, mozilla::image::SurfaceConfig const&)|hg:hg.mozilla.org/mozilla-central:image/SurfacePipeFactory.h:324d5257f6f7cd287565dd822b13d3b5acc0ef14|587|0x16
15|4|libxul.so|mozilla::image::SurfacePipeFactory::CreateSurfacePipe(mozilla::image::Decoder*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, mozilla::gfx::SurfaceFormat, mozilla::Maybe<mozilla::image::AnimationParams> const&, _qcms_transform*, mozilla::image::SurfacePipeFlags)|hg:hg.mozilla.org/mozilla-central:image/SurfacePipeFactory.h:324d5257f6f7cd287565dd822b13d3b5acc0ef14|482|0x8
15|5|libxul.so|mozilla::image::nsBMPDecoder::AllocateSurface()|hg:hg.mozilla.org/mozilla-central:image/decoders/nsBMPDecoder.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|891|0x21
15|6|libxul.so|mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_0::operator()(mozilla::image::nsBMPDecoder::State, char const*, unsigned long) const|hg:hg.mozilla.org/mozilla-central:image/decoders/nsBMPDecoder.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|450|0x11
15|7|libxul.so|mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)|hg:hg.mozilla.org/mozilla-central:image/decoders/nsBMPDecoder.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|430|0xad0
15|8|libxul.so|mozilla::image::Decoder::Decode(mozilla::image::IResumable*)|hg:hg.mozilla.org/mozilla-central:image/Decoder.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|172|0x2a
15|9|libxul.so|mozilla::image::DecodedSurfaceProvider::Run()|hg:hg.mozilla.org/mozilla-central:image/DecodedSurfaceProvider.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|122|0x25
15|10|libxul.so|mozilla::image::DecodePoolWorker::Run()|hg:hg.mozilla.org/mozilla-central:image/DecodePool.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|276|0x1f
15|11|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|1234|0xe
15|12|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|504|0xc
15|13|libxul.so|mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|332|0x13
15|14|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:324d5257f6f7cd287565dd822b13d3b5acc0ef14|316|0x17
15|15|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:324d5257f6f7cd287565dd822b13d3b5acc0ef14|291|0x8
15|16|libxul.so|nsThread::ThreadFunc(void*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:324d5257f6f7cd287565dd822b13d3b5acc0ef14|447|0x8
15|17|libnspr4.so|_pt_root|hg:hg.mozilla.org/mozilla-central:nsprpub/pr/src/pthreads/ptthread.c:324d5257f6f7cd287565dd822b13d3b5acc0ef14|201|0x7
15|18|libpthread.so.0|start_thread|/build/glibc-LK5gWL/glibc-2.23/nptl/pthread_create.c|333|0x11
15|19|libc.so.6|__clone|||0x6d

Comment 3

[Tyson Smith [:tsmith]]

Would it be reasonable to add || FUZZING to this[1] ifdef? To help avoid hitting false positives on ASan (opt) fuzzing builds?

void sk_abort_no_print() {
#ifdef SK_DEBUG
    const char* env = PR_GetEnv("MOZ_SKIA_DISABLE_ASSERTS");

https://searchfox.org/mozilla-central/source/gfx/skia/skia/src/ports/SkMemory_mozalloc.cpp#18

Comment 4

[Tyson Smith [:tsmith]]

A Pernosco session from a fuzzing opt build is available here: https://pernos.co/debug/UMdjMK4bxl2XDnEg0omMRg/index.html

Comment 5

[Timothy Nikkel (:tnikkel)]

This skia code just seems broken

https://searchfox.org/mozilla-central/rev/21f2b48e01f2e14a94e8d39a665b56fcc08ecbdb/gfx/skia/skia/include/private/SkTDArray.h#354

It computes a value that the code comment says can be larger than 2^31 (correct) and then asserts that it fits in a signed int. This code is the same on (what I think is) the main skia repo.

I'm thinking about just making sure the values that we pass into skia via the image downscaling filter don't trigger this and then see if the fuzzers can hit this problem via a normal skia drawing path.

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1648821. Limit the values we pass to SkConvolutionFilter1D::reserveAdditional so that SkTDArray::resizeStorageToAtLeast works correctly. r?lsalzman

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dcef40aa0be0
Limit the values we pass to SkConvolutionFilter1D::reserveAdditional so that SkTDArray::resizeStorageToAtLeast works correctly. r=lsalzman

Comment 8

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/dcef40aa0be0