bug#46791: 27.1; crash at gtk_label_new()

bug#46791: 27.1; crash at gtk_label_new()

[YASUOKA Masahiko]

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.

A backtrace by gdb

(gdb) bt
#0  _rthread_tls_destructors (thread=0xadfdf3e3ad0) at /usr/src/lib/libc/thread/rthread_tls.c:180
#1  0x00000adfdef1396e in handle_fatal_signal (sig=Variable "sig" is not available.
) at sysdep.c:1793
#2  0x00000adfdef139f2 in deliver_thread_signal (sig=Variable "sig" is not available.
) at sysdep.c:1767
#3  0x00000adfdef127f9 in deliver_fatal_thread_signal (sig=Variable "sig" is not available.
) at sysdep.c:1805
#4  0x00000adfdef13a3a in handle_sigsegv (sig=11, siginfo=0xadfdf3e3c30, arg=Variable "arg" is not available.
) at sysdep.c:1890
#5  <signal handler called>
#6  0x00000ae226ab9961 in gtk_label_new () from /usr/local/lib/libgtk-3.so.2201.0
#7  0x00000adfdeedd087 in update_frame_tool_bar (f=Variable "f" is not available.
) at gtkutil.c:4712
#8  0x00000adfdee444fe in redisplay_window (window=0xae275466c35, just_this_one_p=false) at xdisp.c:14152
#9  0x00000adfdee3ef94 in redisplay_window_0 (window=Variable "window" is not available.
) at xdisp.c:16314
#10 0x00000adfdef86b1f in internal_condition_case_1 (bfun=Variable "bfun" is not available.
) at eval.c:1380
#11 0x00000adfdee3e55d in redisplay_windows (window=0xae275466c35) at xdisp.c:16294
#12 0x00000adfdee1219a in redisplay_internal () at xdisp.c:15762
#13 0x00000adfdeef8d70 in read_char (commandflag=1, map=0xae24f0ae3c3, prev_event=0x0, used_mouse_menu=0x7f7ffffda2f7, end_time=0x0) at keyboard.c:2493
#14 0x00000adfdeef67ea in read_key_sequence (keybuf=Variable "keybuf" is not available.
) at keyboard.c:9553
#15 0x00000adfdeef51c0 in command_loop_1 () at keyboard.c:1350
#16 0x00000adfdef86a76 in internal_condition_case (bfun=Variable "bfun" is not available.
) at eval.c:1356
#17 0x00000adfdef06450 in command_loop_2 (ignore=Variable "ignore" is not available.
) at keyboard.c:1091
#18 0x00000adfdef86347 in internal_catch (tag=Variable "tag" is not available.
) at eval.c:1117
#19 0x00000adfdeef405a in command_loop () at keyboard.c:1070
#20 0x00000adfdeef3f21 in recursive_edit_1 () at keyboard.c:714
#21 0x00000adfdeef424a in Frecursive_edit () at keyboard.c:786
#22 0x00000adfdeef2e78 in main (argc=Cannot access memory at address 0x0
) at emacs.c:2062
(gdb) 


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:

Index: src/gtkutil.c
--- src/gtkutil.c.orig
+++ src/gtkutil.c
@@ -5019,11 +5019,7 @@ 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))
-	: "";
+      const char *label;
 
       ti = gtk_toolbar_get_nth_item (GTK_TOOLBAR (wtoolbar), j);
 
@@ -5133,6 +5129,11 @@ update_frame_tool_bar (struct frame *f)
               continue;
             }
         }
+
+      label = (EQ (style, Qimage) || (vert_only && horiz)) ? NULL
+	: STRINGP (PROP (TOOL_BAR_ITEM_LABEL))
+	? SSDATA (PROP (TOOL_BAR_ITEM_LABEL))
+	: "";
 
       /* If there is an existing widget, check if it's stale; if so,
 	 remove it and make a new tool item from scratch.  */


The crash doesn't happen after the diff is applied.


In GNU Emacs 27.1 (build 1, x86_64-unknown-openbsd, GTK+ Version 3.24.23)
 of 2021-02-24 built on yasuoka-ob1.tokyo.iiji.jp
Repository revision: f7d512d526f0b515194e5ef243120e30547ae1c7
Repository branch: work
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: OpenBSD yasuoka-ob1.tokyo.iiji.jp 6.9 GENERIC.MP#215 amd64

Recent messages:
For information about GNU Emacs and the GNU system, type <f1> C-a.
Quit [2 times]
Setting up Mew world...
Updating status...done
Setting up Mew world...done
Scanning +inbox...done
Making completion list... [2 times]

Configured using:
 'configure --build=amd64-unknown-openbsd --without-sound
 --with-x-toolkit=gtk3 --prefix=/usr/local --sysconfdir=/etc
 --mandir=/usr/local/man --infodir=/usr/local/info
 --localstatedir=/var --disable-silent-rules --disable-gtk-doc
 'CFLAGS=-O2 -pipe -g' CPPFLAGS=-I/usr/local/include
 'LDFLAGS=-L/usr/local/lib -g''

Configured features:
XPM JPEG TIFF GIF PNG RSVG DBUS GSETTINGS GLIB NOTIFY KQUEUE GNUTLS
LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 XDBE XIM MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LC_CTYPE: ja_JP.UTF-8
  value of $LANG: ja_JP.UTF-8
  value of $XMODIFIERS: 
  locale-coding-system: utf-8-unix

Major mode: Summary

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow vc-git diff-mode easy-mmode emacsbug message rmc puny dired
dired-loaddefs format-spec rfc822 mml mml-sec password-cache epa
derived epg epg-config gnus-util rmail rmail-loaddefs
text-property-search time-date subr-x seq byte-opt gv bytecomp
byte-compile cconv mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils pp mew-varsx mew-unix mew-auth
mew-config mew-imap2 mew-imap mew-nntp2 mew-nntp mew-pop mew-smtp
mew-ssl mew-ssh mew-net mew-highlight mew-sort mew-fib mew-ext
mew-refile mew-demo mew-attach mew-draft mew-message mew-thread
mew-virtual mew-summary4 mew-summary3 mew-summary2 mew-summary
mew-search mew-pick mew-passwd mew-scan mew-syntax mew-bq mew-smime
mew-pgp mew-header mew-exec mew-mark mew-mime mew-edit mew-decode
mew-encode mew-cache mew-minibuf mew-complete mew-addrbook mew-local
mew-vars3 mew-vars2 mew-vars mew-env mew-lang-jp mew-mule3 mew-mule
mew-gemacs easymenu mew-key mew-func mew-blvs mew-const mew edmacro
kmacro cl-loaddefs cl-lib japan-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese
eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese composite charscript charprop case-table
epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads dbusbind kqueue lcms2
dynamic-setting system-font-setting font-render-setting move-toolbar
gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 101329 6741)
 (symbols 48 12110 3)
 (strings 32 36425 1488)
 (string-bytes 1 1044411)
 (vectors 16 18772)
 (vector-slots 8 476303 14814)
 (floats 8 49 42)
 (intervals 56 602 0)
 (buffers 1000 13))

bug#46791: 27.1; crash at gtk_label_new()

[Eli Zaretskii]

> Date: Fri, 26 Feb 2021 16:32:06 +0900 (JST)
> From: YASUOKA Masahiko <yasuoka@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);
         }
 

bug#46791: 27.1; crash at gtk_label_new()

[YASUOKA Masahiko]

On Fri, 26 Feb 2021 16:37:03 +0200
Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 26 Feb 2021 16:32:06 +0900 (JST)
>> From: YASUOKA Masahiko <yasuoka@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.

Yes.  I tested your patch, it seems to fix the problem.

Thanks,

bug#46791: 27.1; crash at gtk_label_new()

[Eli Zaretskii]

> Date: Sat, 27 Feb 2021 12:35:56 +0900 (JST)
> Cc: 46791@debbugs.gnu.org
> From: YASUOKA Masahiko <yasuoka@yasuoka.net>
> 
> > 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.
> 
> Yes.  I tested your patch, it seems to fix the problem.

Thanks, I installed the change on the emacs-27 branch, for the
upcoming Emacs 27.2, and I'm therefore marking this bug done.