Subject: Re: bug#46791: 27.1; crash at gtk_label_new()

From: YASUOKA Masahiko <[email-masked]>
Date: Sat, 27 Feb 2021 12:35:56 +0900 (JST)

On Fri, 26 Feb 2021 16:37:03 +0200
Eli Zaretskii <[email-masked]> wrote:
>> Date: Fri, 26 Feb 2021 16:32:06 +0900 (JST)
>> From: YASUOKA Masahiko <[email-masked]>
>> 
>> 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,