GNU bug report logs - #46791
27.1; crash at gtk_label_new()

Previous Next

Full log

Message #10 received at 46791 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: YASUOKA Masahiko <yasuoka <at> yasuoka.net>
Cc: 46791 <at> debbugs.gnu.org
Subject: Re: bug#46791: 27.1; crash at gtk_label_new()
Date: Fri, 26 Feb 2021 16:37:03 +0200

> Date: Fri, 26 Feb 2021 16:32:06 +0900 (JST)
> From: YASUOKA Masahiko <yasuoka <at> yasuoka.net>
> 
> When I'm using Mew(https://mew.org/) on emacs 27.1, emacs crashes
> frequently.  It happens when I am composing a mail message in "draft
> mode" of Mew.
> [...]
> In src/gtkutil.c, update_frame_tool_bar():
> 
>     5197           ti = xg_make_tool_item (f, w, &wbutton, label, i, horiz, text_image);
> 
> this "label" is invalid when the crash happens.  This "label" 
> 
>     5006   for (i = j = 0; i < f->n_tool_bar_items; ++i)
>     5007     {
>     5008       bool enabled_p = !NILP (PROP (TOOL_BAR_ITEM_ENABLED_P));
>     5009       bool selected_p = !NILP (PROP (TOOL_BAR_ITEM_SELECTED_P));
> 
>     5022       const char *label
>     5023         = (EQ (style, Qimage) || (vert_only && horiz)) ? NULL
>     5024         : STRINGP (PROP (TOOL_BAR_ITEM_LABEL))
>     5025         ? SSDATA (PROP (TOOL_BAR_ITEM_LABEL))
>     5026         : "";
> 
> is set at the begining of the loop(#5006),
> 
>     5065       specified_file = file_for_image (image);
>     5066       if (!NILP (specified_file) && !NILP (Ffboundp (Qx_gtk_map_stock)))
>     5067         stock = call1 (Qx_gtk_map_stock, specified_file);
>     5068
> 
> it sometimes become invalid just after #5067.  Then it is passed to
> gtk_label_new() through xg_make_tool_item(), the crash will happen.
> 
> Since we can get a valid "label" pointer again by setting it in the
> same way of the beginning of the loop, we can fix the bug by moving
> the initialization of "label" to a place just before it is used.  The
> following diff does this:

Thanks.  Could you please try the slightly different patch below?  It
is IMO safer, since it doesn't depend on a 'char *' pointer into a
Lisp string's data to remain valid after some point in the code.

diff --git a/src/gtkutil.c b/src/gtkutil.c
index d824601..825fbe1 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -5019,11 +5019,10 @@ update_frame_tool_bar (struct frame *f)
       GtkWidget *wbutton = NULL;
       Lisp_Object specified_file;
       bool vert_only = ! NILP (PROP (TOOL_BAR_ITEM_VERT_ONLY));
-      const char *label
-	= (EQ (style, Qimage) || (vert_only && horiz)) ? NULL
-	: STRINGP (PROP (TOOL_BAR_ITEM_LABEL))
-	? SSDATA (PROP (TOOL_BAR_ITEM_LABEL))
-	: "";
+      Lisp_Object label
+	= (EQ (style, Qimage) || (vert_only && horiz))
+	? Qnil
+	: PROP (TOOL_BAR_ITEM_LABEL);
 
       ti = gtk_toolbar_get_nth_item (GTK_TOOLBAR (wtoolbar), j);
 
@@ -5136,8 +5135,11 @@ update_frame_tool_bar (struct frame *f)
 
       /* If there is an existing widget, check if it's stale; if so,
 	 remove it and make a new tool item from scratch.  */
-      if (ti && xg_tool_item_stale_p (wbutton, stock_name, icon_name,
-				      img, label, horiz))
+      if (ti && xg_tool_item_stale_p (wbutton, stock_name, icon_name, img,
+				      NILP (label)
+				      ? NULL
+				      : STRINGP (label) ? SSDATA (label) : "",
+				      horiz))
 	{
 	  gtk_container_remove (GTK_CONTAINER (wtoolbar),
 				GTK_WIDGET (ti));
@@ -5194,7 +5196,11 @@ update_frame_tool_bar (struct frame *f)
 #else
 	  if (w) gtk_misc_set_padding (GTK_MISC (w), hmargin, vmargin);
 #endif
-          ti = xg_make_tool_item (f, w, &wbutton, label, i, horiz, text_image);
+          ti = xg_make_tool_item (f, w, &wbutton,
+				  NILP (label)
+				  ? NULL
+				  : STRINGP (label) ? SSDATA (label) : "",
+				  i, horiz, text_image);
           gtk_toolbar_insert (GTK_TOOLBAR (wtoolbar), ti, j);
         }
 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.