Bug 283351
| Summary: | [Skia] Ensure correct SkPixmap::[writable_]addr*() methods are used for image data access | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Adrian Perez <aperez> |
| Component: | Layout and Rendering | Assignee: | Adrian Perez <aperez> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | bfulgham, bugs-noreply, simon.fraser, webkit-bug-importer, zalan, zimmermann |
| Priority: | P2 | Keywords: | InRadar |
| Version: | Other | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=311095 | ||
Adrian Perez
The SkPixmap::[writable_]addr{8,16,32,64,F16}() functions include assertions
(which may be enabled via the SKIA_DEBUG=ON CMake option) that check whether
the pixel format bit width matches the intended usage of the method. For
example ::addr32() works with ARGB8888/BGRA8888/etc. but will crash on the
assertion with an RGB565 image.
We should audit the call sites for these functions and make sure the correct
ones are used. When handling can be generic regardless of the pixel format
bit widthl, usage must be replaced with the unsuffixed ::[writable_]addr()
functions instead.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/140575154>
Adrian Perez
Heh, I am now hitting an assertion related to this, after 310215@main (see bug #311095):
Thread 1 "TestWebKitFavic" received signal SIGILL, Illegal instruction.
sk_abort_no_print () at /sdk/webkit/Source/ThirdParty/skia/src/ports/SkMemory_malloc.cpp:60
60 __builtin_trap();
(gdb) up
#1 0x00007fffedc53f72 in SkPixmap::addr8() const::{lambda()#1}::operator()() const (this=0x7fffffffbfc7) at Skia/Headers/top/skia/core/SkPixmap.h:327
327 SkASSERT(1 == fInfo.bytesPerPixel());
(gdb) up
#2 0x00007fffedc53f0a in SkPixmap::addr8 (this=0x7fffffffc0a8) at Skia/Headers/top/skia/core/SkPixmap.h:327
327 SkASSERT(1 == fInfo.bytesPerPixel());
(gdb) up
#3 0x00007fffedc53dc5 in SkPixmap::addr8 (this=0x7fffffffc0a8, x=0, y=0) at Skia/Headers/top/skia/core/SkPixmap.h:401
401 return (const uint8_t*)((const char*)this->addr8() + (size_t)y * fRowBytes + (x << 0));
(gdb) up
#4 0x00007fffedc533d1 in SkPixmap::writable_addr8 (this=0x7fffffffc0a8, x=0, y=0) at Skia/Headers/top/skia/core/SkPixmap.h:509
509 return const_cast<uint8_t*>(this->addr8(x, y));
(gdb) up
#5 0x00007fffedc5328e in WebKit::skiaImageToCairoSurface (image=...) at /sdk/webkit/Source/WebKit/UIProcess/gtk/GtkUtilities.cpp:178
178 RefPtr<cairo_surface_t> surface = adoptRef(cairo_image_surface_create_for_data(pixmap.writable_addr8(0, 0), CAIRO_FORMAT_ARGB32, pixmap.width(), pixmap.height(), pixmap.rowBytes()));
Preparing a patch now...
Adrian Perez
Pull request: https://github.com/WebKit/WebKit/pull/61713
Adrian Perez
*** Bug 282380 has been marked as a duplicate of this bug. ***
EWS
Committed 310261@main (a6bf9f89d0b3): <https://commits.webkit.org/310261@main>
Reviewed commits have been landed. Closing PR #61713 and removing active labels.