bug#37385: 27.0.50; Crash on multibyte assertion violation

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Wed, 11 Sep 2019 23:24:03 +0300
> 
> Configured using:
>  'configure --with-x-toolkit=no --enable-checking=yes,glyphs
>  --enable-check-lisp-object-type 'CFLAGS=-O0 -g3''
> 
> These steps reproduce the crash in master:
> 
> 0. emacs -Q
> 1. Eval: (define-key global-map [menu-bar test] '("Test ⮿" keymap))
> 2. Visit an image file, e.g. etc/images/attach.pbm
> 
> #0  0x00005555557a61b8 in terminate_due_to_signal (sig=6, 
> backtrace_limit=2147483647) at emacs.c:374
> #1  0x00005555558c7555 in die (msg=0x555555ae73ee "SINGLE_BYTE_CHAR_P (c)", 
> file=0x555555ae4790 "xdisp.c", line=7250) at alloc.c:7256
> #2  0x00005555555e9e56 in get_next_display_element (it=0x7fffffff89b0) at 
> xdisp.c:7250
> #3  0x000055555562938c in display_string (string=0x0, 
> lisp_string=XIL(0x5555567a0204), face_string=XIL(0), face_string_pos=0, 
> start=0, it=0x7fffffff89b0, field_width=7, precision=0, max_x=674, 
> multibyte=-1) at xdisp.c:25489
> [...]
> An assertion violation is in get_next_display_element:
> 
>         if (! it->multibyte_p && ! ASCII_CHAR_P (c))
>           {
>             eassert (SINGLE_BYTE_CHAR_P (c));
> 
> The menu item uses a multibyte char:
> 
>   c = 11199 (#o25677, #x2bbf, ?⮿)
> 
> But init_iterator sets multibyte_p in the menu-bar window to the
> value of enable-multibyte-characters in the current buffer where
> enable-multibyte-characters is nil when an image file is visited in
> image-mode:
> 
>   /* Are multibyte characters enabled in current_buffer?  */
>   it->multibyte_p = !NILP (BVAR (current_buffer, 
> enable_multibyte_characters));
> 
> I tried the following fix and it prevents the crash:

Thanks, but this is a backward-incompatible change on too low a level.
It is a long-standing "convention" in Emacs that Lisp strings rendered
as part of, or in relation to, unibyte buffers are assumed unibyte by
default, and I don't want to change that -- who knows how many places
in the code rely on this implicit assumption?

Also, the change in reseat_1 looks unnecessary, as I'd be surprised if
that function was called in your use case (reseat_1 is used only when
displaying buffers, not strings).

Please try an alternative patch below.  (I don't have access to an X
build without a toolkit, so I cannot test this myself.)

Stepping a notch back, I cannot say I like this "non-ASCII art"
implementation for tabs.  It has two annoying problems:

  . it looks unprofessional on GUI frames
  . it requires you to determine whether the frame/font used for the
    menu can display this character, which is not easy

Why not use an image of a plus sign on GUI frames, and a simple ASCII
"+" on TTY frames and frames that have no image support?  I think the
result will be much better.

Thanks.

diff --git a/src/xdisp.c b/src/xdisp.c
index 94f969f..d342da5 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -12994,7 +12994,8 @@ redisplay_tool_bar (struct frame *f)
 
   /* Build a string that represents the contents of the tool-bar.  */
   build_desired_tool_bar_string (f);
-  reseat_to_string (&it, NULL, f->desired_tool_bar_string, 0, 0, 0, -1);
+  reseat_to_string (&it, NULL, f->desired_tool_bar_string,
+                   0, 0, 0, STRING_MULTIBYTE (f->desired_tool_bar_string));
   /* FIXME: This should be controlled by a user option.  But it
      doesn't make sense to have an R2L tool bar if the menu bar cannot
      be drawn also R2L, and making the menu bar R2L is tricky due
@@ -23531,7 +23532,7 @@ display_menu_bar (struct window *w)
       /* Display the item, pad with one space.  */
       if (it.current_x < it.last_visible_x)
        display_string (NULL, string, Qnil, 0, 0, &it,
-                       SCHARS (string) + 1, 0, 0, -1);
+                       SCHARS (string) + 1, 0, 0, STRING_MULTIBYTE (string));
     }
 
   /* Fill out the line with spaces.  */