Subject: bug#18997: 24.4; IDLWAVE: Emacs crashes when trying to change the window while in the IDL shell

From: Jonathan Woithe <[email-masked]>
Date: Tue, 22 Sep 2015 16:18:02 +0930

I have a user who is encountering the same problem.  The system is
Slackware64 14.1 which ships with emacs 24.3.
   
To reproduce the bug one does not need IDL on the system: just use "bash" as
a standin.  Create a symlink called /bin/idl pointing to /bin/bash.  Create 
a skeleton IDL program called test.pro (the content is irrelevant).  Then:

  * emacs test.pro

  * C-c,C-s

    This produces a large number of GdkPixbuf messages on the console as the
    IDL window is opened (and in this test case, bash is started).  There
    are three messages:
      (emacs:2384): GdkPixbuf-CRITICAL **: gdk_pixbuf_new_from_data: assertion
        `bits_per_sample == 8' failed
      (emacs:2384): GdkPixbuf-CRITICAL **: gdk_pixbuf_add_alpha: assertion
        `GDK_IS_PIXBUF (pixbuf)' failed
      (emacs:2384): GLib-GObject-CRITICAL **: g_object_unref: assertion
        `G_IS_OBJECT (object)' failed
    These are repeated in no apparent order.

  * Click in the test.pro window.  Emacs segfaults.

Emacs versions 23.4 and earlier all seem to work just fine.  Version 24.1
and later crash out (some as above, some as soon as C-c,C-s is used).  For
24.1 and later it doesn't matter whether gtk2 or gtk3 is selected via
"configure".

A git bisect indicates that the fundamental problem was introduced in commit
0afb4571a7b54dc7693e605f7ec8a0a3a9251b4d, which added the initial gtk3
support (between 23.4 and 24.1).  Examining the core dump from this commit,
the crash happens at src/gtkutil.c:3743, in xg_get_tool_bar_widgets() called
from xg_tool_item_stale_p().  clist is NULL, so its dereference on line
3743 to find c1 triggers the segfault.

The bug is associated with the new implementation of
xg_get_pixbuf_from_pix_and_mask() in src/gtkutil.c which calls the new   
function xg_get_pixbuf_from_pixmap().  XGetImage() is returning an XImage
with bitmap_unit set to 32.  This is passed to gdk_pixbuf_new_from_data() as
the bits_per_sample parameter.  The gdk_pixbuf_new_from_data() documentation
states:

  Currently only RGB images with 8 bits per sample are supported.
Since emacs is not in general passing 8 as the bits_per_sample parameter the
assertion in gdk_pixbuf_new_from_data() fails and NULL is returned.  This   
restriction on 8 bits per sample within gdk_pixbuf_new_from_data applies to 
both gtk2 and gtk3.

As an aside I'm not sure that XImage::bitmap_unit is the correct field to
use for gdk_pixbuf_new_from_data()'s bits_per_sample parameter.  There's not
much documentation around for the XImage structure.  What I can find
describes selected fields as this:

  bitmap_unit    = quant. of scanline 8, 16, 32
  bitmap_pad     = 8, 16, 32 either XY or ZFormat
  depth          = depth of image
  bits_per_pixel = bits per pixel (ZFormat)

Emacs uses the XYPixmap format so bits_per_pixel is irrelevant, and
"bitmap_pad" sounds more like a padding than anything else.  That leaves
either "bitmap_unit" or "depth" as apparently feasible options.  For what
it's worth, when emcas calls xg_get_pixbuf_from_pixmap() on my 24-bit
Truecolour display, bitmap_unit and bitmap_pad are always 32, depth is
either 24 or 1 and bits_per_pixel is always 1.

The XImage returned by XGetImage() will almost certainly have more than 8
bits per pixel these days and I therefore cannot see a way to interface it
with gdk_pixbuf_new_from_data() (which only accepts 8 bits per pixel).    
Consequently I think the only way to address the bug is to partially roll 
back the changes to xg_get_pixbuf_from_pix_and_mask() made in commit
0afb4571a7b54dc7693e605f7ec8a0a3a9251b4d.  A patch to do this (against
today's git master, (57d48c364cf24c9f99f3d03eb74d9d2a81f4ae31) is at the end
of this email.  With this patch applied, git master no longer crashes for me
when gtk2 is in use.  The problem is that GdkDrawable, GdkPixmap and their  
associated methods are not present in gtk3 which means compiling this patch 
for gtk3 will fail.

Unfortunately I am not familiar enough with the emacs code and
the way it manages these tool bar button icons to come up with something
which will work under gtk3.  As a result, the patch below is a starting 
point rather than a complete fix.

Regards
  jonathan

This patch prevents emacs segfaulting under Linux when running idlwave (and
possibly other things) when compiled for gtk2.  Attempting to compile for
gtk3 will fail since a gtk3 solution is not yet known (the gtk2 approach
uses features which have been removed in gtk3).

Signed-off-by: Jonathan Woithe <[email-masked]>

--- a/src/gtkutil.c     2015-09-22 14:59:25.911372841 +0930
+++ b/src/gtkutil.c     2015-09-22 15:05:21.349350817 +0930
@@ -254,55 +254,33 @@
   return gdk_cursor_new_for_display (gdpy, GDK_LEFT_PTR);
 }
 
-static GdkPixbuf *
-xg_get_pixbuf_from_pixmap (struct frame *f, Pixmap pix)
-{
-  int iunused;
-  GdkPixbuf *tmp_buf;
-  Window wunused;
-  unsigned int width, height, uunused;
-  XImage *xim;
-
-  XGetGeometry (FRAME_X_DISPLAY (f), pix, &wunused, &iunused, &iunused,
-                &width, &height, &uunused, &uunused);
-
-  xim = XGetImage (FRAME_X_DISPLAY (f), pix, 0, 0, width, height,
-                   ~0, XYPixmap);
-  if (!xim) return 0;
-
-  tmp_buf = gdk_pixbuf_new_from_data ((guchar *) xim->data,
-                                      GDK_COLORSPACE_RGB,
-                                      FALSE,
-                                      xim->bitmap_unit,
-                                      width,
-                                      height,
-                                      xim->bytes_per_line,
-                                      NULL,
-                                      NULL);
-  XDestroyImage (xim);
-  return tmp_buf;
-}
-
 /* Apply GMASK to GPIX and return a GdkPixbuf with an alpha channel.  */
 
 static GdkPixbuf *
 xg_get_pixbuf_from_pix_and_mask (struct frame *f,
                                  Pixmap pix,
-                                 Pixmap mask)
+                                 Pixmap mask,
+                                 GdkColormap *cmap)
 {
   int width, height;
   GdkPixbuf *icon_buf, *tmp_buf;
-
-  tmp_buf = xg_get_pixbuf_from_pixmap (f, pix);
+  GdkDisplay *gdpy = gdk_x11_lookup_xdisplay (FRAME_X_DISPLAY (f));
+  GdkPixmap *gpix = gdk_pixmap_foreign_new_for_display (gdpy, pix);
+  GdkPixmap *gmask = mask ? gdk_pixmap_foreign_new_for_display (gdpy, mask) : 0;
+
+  gdk_drawable_get_size (gpix, &width, &height);
+  tmp_buf = gdk_pixbuf_get_from_drawable (NULL, gpix, cmap,
+                                          0, 0, 0, 0, width, height);
   icon_buf = gdk_pixbuf_add_alpha (tmp_buf, FALSE, 0, 0, 0);
   g_object_unref (G_OBJECT (tmp_buf));
 
-  width = gdk_pixbuf_get_width (icon_buf);
-  height = gdk_pixbuf_get_height (icon_buf);
-
-  if (mask)
+  if (gmask)
     {
-      GdkPixbuf *mask_buf = xg_get_pixbuf_from_pixmap (f, mask);
+      GdkPixbuf *mask_buf = gdk_pixbuf_get_from_drawable (NULL,
+                                                          gmask,
+                                                          NULL,
+                                                          0, 0, 0, 0,
+                                                          width, height);
       guchar *pixels = gdk_pixbuf_get_pixels (icon_buf);
       guchar *mask_pixels = gdk_pixbuf_get_pixels (mask_buf);
       int rowstride = gdk_pixbuf_get_rowstride (icon_buf);
@@ -368,6 +346,7 @@
                          GtkImage *old_widget)
 {
   GdkPixbuf *icon_buf;
+  GdkColormap *cmap;
 
   /* If we have a file, let GTK do all the image handling.
      This seems to be the only way to make insensitive and activated icons
@@ -404,7 +383,8 @@
      not associated with the img->pixmap.  The img->pixmap may be removed
      by clearing the image cache and then the tool bar redraw fails, since
      Gtk+ assumes the pixmap is always there.  */
-  icon_buf = xg_get_pixbuf_from_pix_and_mask (f, img->pixmap, img->mask);
+  cmap = gtk_widget_get_colormap (widget);
+  icon_buf = xg_get_pixbuf_from_pix_and_mask (f, img->pixmap, img->mask, cmap);
 
   if (icon_buf)
     {
@@ -1490,7 +1470,8 @@
 {
   GdkPixbuf *gp = xg_get_pixbuf_from_pix_and_mask (f,
                                                    icon_pixmap,
-                                                   icon_mask);
+                                                   icon_mask,
+                                                   NULL);
   if (gp)
     gtk_window_set_icon (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), gp);
 }