Bug 178852

Summary: [GTK][Stable] Crash on WebCore::SharedBuffer::data() on 2.18.1
Product: WebKit Reporter: Miguel Gomez <magomez>
Component: WebKitGTKAssignee: Miguel Gomez <magomez>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, Hironori.Fujii, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=1505755
https://bugzilla.redhat.com/show_bug.cgi?id=1508044
Attachments:
Description Flags
Patch cgarcia: review+

Description Miguel Gomez 2017-10-26 00:48:58 PDT
We have a crash when decoding a GIF image. This is the backtrace:

Thread no. 1 (9 frames)
 #0 WebCore::SharedBuffer::data() const at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/SharedBuffer.cpp:100
 #1 GIFImageReader::data(unsigned long) const at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h:298
 #2 GIFImageReader::parse(unsigned long, unsigned long, bool) at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:412
 #3 GIFImageReader::decode(WebCore::GIFImageDecoder::GIFQuery, unsigned int) at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:360
 #4 WebCore::GIFImageDecoder::decode(unsigned int, WebCore::GIFImageDecoder::GIFQuery, bool) at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:349
 #5 WebCore::GIFImageDecoder::frameCount() const at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:67
 #6 WebCore::GIFImageDecoder::frameBufferAtIndex(unsigned long) at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:144
 #7 WebCore::ImageDecoder::createFrameImageAtIndex(unsigned long, WebCore::SubsamplingLevel, WebCore::DecodingOptions const&) at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/image-decoders/ImageDecoder.cpp:218
 #8 WebCore::ImageFrameCache::<lambda()>::operator() at /usr/src/debug/webkitgtk4-2.18.1-1.fc27.x86_64/Source/WebCore/platform/graphics/ImageFrameCache.cpp:294
Comment 1 Miguel Gomez 2017-10-26 02:11:03 PDT
The data is null when trying to decode the
Comment 2 Fujii Hironori 2017-10-26 02:55:08 PDT
Bug 178510?
Comment 3 Carlos Garcia Campos 2017-10-26 03:04:29 PDT
(In reply to Fujii Hironori from comment #2)
> Bug 178510?

The bt is the same but ScalableImageDecoder doesn't exist in 2.18, I guess we need a similar fix for 2.18 or bring back the locks we had in the GIF decoder.
Comment 4 Miguel Gomez 2017-10-26 03:30:50 PDT
(In reply to Carlos Garcia Campos from comment #3)
> (In reply to Fujii Hironori from comment #2)
> > Bug 178510?
> 
> The bt is the same but ScalableImageDecoder doesn't exist in 2.18, I guess
> we need a similar fix for 2.18 or bring back the locks we had in the GIF
> decoder.

ImageDecoder was renamed to ScalableImageDecoder because a multiplatform ImageDecoder class added, and ScalableImageDecoder inherits from it.

But I think you're right and this is the fix for the problem. I hadn't realized at all that those locks were added, and I was wondering why I couldn't reproduce the problem with current ToT. And I guess this is the reason.

I'll backport the locks patch and check whether they fix the crash.

Thanks for your help Fujii!!
Comment 5 Miguel Gomez 2017-10-26 05:25:38 PDT
Created attachment 324993 [details]
Patch
Comment 6 Carlos Garcia Campos 2017-10-26 06:13:15 PDT
Comment on attachment 324993 [details]
Patch

Thanks
Comment 7 Carlos Garcia Campos 2017-10-26 06:14:16 PDT
Committed r224017