Friday, May 01, 2020

Fix GIMP crash with x11/gtk+2 diff [Was: Re: GIMP open file crash in gimp-2.10.18p1 still exists]

Stuart Henderson writes:

> On 2020/04/29 15:13, Jacqueline Jolicoeur wrote:
>> Hi,
>>
>> I have noticed gimp-2.10.18p1 crashes everytime I try to open a file. Removing all my GIMP cache and config files has not resolved it.
>>
>> I am aware this was posted earlier. I wanted to report that this bug still exists as of today with the recent snapshots.
>
> We don't have any ideas yet. I haven't been able to reproduce it recently,
> the crash is in gtk+2 in possibly something theme-related so maybe try a different
> theme or try a different window manager or desktop environment and see if that
> makes it go away?
>
> The upstream bug is currently at https://gitlab.gnome.org/GNOME/gdk-pixbuf/-/issues/148

Here is a diff for x11/gtk+2 to fix GIMP crashing. Feedback and tests
are welcome.

"Image Data" and "Example 1. put_pixel() example" were useful.

https://developer.gimp.org/api/2.0/gdk-pixbuf/gdk-pixbuf-gdk-pixbuf.html

Example 1 was very similar to the code in pixbuf-render.c's compute_hint
function.

https://marc.info/?l=openbsd-ports&m=157538140605480&w=2

I got the same stacktrace as brynet@ posted here. (x0=23, x1=24, y0=31,
y1=32.) Some stuff was optimized out but I got it to print by going back
to earlier stack frames or by slightly modifying the code with a fix
that didn't work (that's how I got num_channels = 3).

Given these values of x0, x1, y0, y1 and num_channels it seems like it
goes beyond the end of the pixbuf buffer. It was going out of bounds on
(n_channels != 4 && a != *(p++)) according to gdb. If anything,
num_channels being 3 with my change prevents it from advancing the p
pointer too far.

RGBA each has a byte for red, green, blue and alpha. In this case,
num_channels = 3 so there is no alpha channel.

Each row looks like this with possible padding at the end:
RGBARGBA...
In this case there is no alpha either so it could look like:
RGBRGB...

rowstride = 72 in this case so there are 72 bytes in each row

if (r != *(p++) ||
g != *(p++) ||
b != *(p++) ||
(n_channels != 4 && a != *(p++)))

p is positioned to where it wants to compute hints like
THEME_CONSTANT_ROWS. I infer that THEME_CONSTANT_ROWS means r = g = b
like the grey theme that crashes. The first 2 lines should be sufficient
to check r = g = b, so I think that even the last two lines with b and
n_channels can be removed. Although, I don't know the exact meaning of
THEME_CONSTANT_ROWS and how alpha affects it so this could be something
to ask upstream.

The last line currently reads: if there is no alpha channel and alpha is
not equal to X (where X is RGBAX) then the theme has a false bit for
THEME_CONSTANT_ROWS.

I think this is incorrect and should say if there _is_ an alpha channel.

Testing
=======
As others have reported, this was hard to reproduce.

To test keep spamming this loop until it crashes. It crashes randomly
but within the first ~10 tries on the current version of gtk+2.

Edit > Preferences > Themes > Grey
Ctrl-O for open file prompt
Escape to close
Edit > Preferences > Themes > Dark
Ctrl-O for open file prompt
Escape to close

With this patch it no longer crashes in my testing.

Index: Makefile
===================================================================
RCS file: /cvs/ports/x11/gtk+2/Makefile,v
retrieving revision 1.233
diff -u -p -u -p -r1.233 Makefile
--- Makefile 10 Nov 2019 21:44:07 -0000 1.233
+++ Makefile 1 May 2020 10:30:50 -0000
@@ -9,7 +9,7 @@ GNOME_PROJECT= gtk+
PKGNAME-main= gtk+2-${GNOME_VERSION}
PKGNAME-cups= gtk+2-cups-${GNOME_VERSION}

-REVISION-main= 8
+REVISION-main= 9
REVISION-cups= 4

CATEGORIES= x11 devel
Index: patches/patch-modules_engines_pixbuf_pixbuf-render_c
===================================================================
RCS file: patches/patch-modules_engines_pixbuf_pixbuf-render_c
diff -N patches/patch-modules_engines_pixbuf_pixbuf-render_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-modules_engines_pixbuf_pixbuf-render_c 1 May 2020 10:30:50 -0000
@@ -0,0 +1,18 @@
+$OpenBSD$
+
+fixes https://gitlab.gnome.org/GNOME/gdk-pixbuf/-/issues/148
+
+only check for alpha if there is an alpha channel
+
+Index: modules/engines/pixbuf/pixbuf-render.c
+--- modules/engines/pixbuf/pixbuf-render.c.orig
++++ modules/engines/pixbuf/pixbuf-render.c
+@@ -603,7 +603,7 @@ compute_hint (GdkPixbuf *pixbuf,
+ if (r != *(p++) ||
+ g != *(p++) ||
+ b != *(p++) ||
+- (n_channels != 4 && a != *(p++)))
++ (n_channels == 4 && a != *(p++)))
+ {
+ hints &= ~THEME_CONSTANT_ROWS;
+ if (!(hints & THEME_MISSING))

No comments:

Post a Comment