bug#37385: 27.0.50; Crash on multibyte assertion violation

bug#37385: 27.0.50; Crash on multibyte assertion violation

[Juri Linkov]

Testing the tabs branch helped to expose a bug in master:

In GNU Emacs 27.0.50 (build 6, x86_64-pc-linux-gnu)
 of 2019-09-11 built on localhost
Repository revision: 4d90fadf27ccbb98e0e174304cb4e3008bf364fc
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Linux Mint 19.1

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
#4  0x00005555556239db in display_menu_bar (w=0x5555565cae40) at xdisp.c:23533
#5  0x000055555560d61e in redisplay_window (window=XIL(0x5555565cae45), just_this_one_p=false) at xdisp.c:17821
#6  0x0000555555603b79 in redisplay_window_0 (window=XIL(0x5555565cae45)) at xdisp.c:15116
#7  0x0000555555921b23 in internal_condition_case_1 (bfun=0x555555603b37 <redisplay_window_0>, arg=XIL(0x5555565cae45), handlers=XIL(0x7fffe995aad3), hfun=0x555555603aff <redisplay_window_error>) at eval.c:1379
#8  0x0000555555603ad1 in redisplay_windows (window=XIL(0x5555565cae45)) at xdisp.c:15096
#9  0x00005555556024c2 in redisplay_internal () at xdisp.c:14579
#10 0x00005555555ffea3 in redisplay () at xdisp.c:13806
#11 0x00005555557b7948 in read_char (commandflag=1, map=XIL(0x555556a2aba3), prev_event=XIL(0), used_mouse_menu=0x7fffffffdd25, end_time=0x0) at keyboard.c:2472
#12 0x00005555557c930f in read_key_sequence (keybuf=0x7fffffffdf10, prompt=XIL(0), dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true, prevent_redisplay=false) at keyboard.c:9125
#13 0x00005555557b3d8b in command_loop_1 () at keyboard.c:1345
#14 0x0000555555921a48 in internal_condition_case (bfun=0x5555557b390d <command_loop_1>, handlers=XIL(0x90), hfun=0x5555557b2ed7 <cmd_error>) at eval.c:1355
#15 0x00005555557b34f4 in command_loop_2 (ignore=XIL(0)) at keyboard.c:1091
#16 0x0000555555920ea2 in internal_catch (tag=XIL(0xcdb0), func=0x5555557b34c7 <command_loop_2>, arg=XIL(0)) at eval.c:1116
#17 0x00005555557b3492 in command_loop () at keyboard.c:1070
#18 0x00005555557b29be in recursive_edit_1 () at keyboard.c:714
#19 0x00005555557b2bb6 in Frecursive_edit () at keyboard.c:786
#20 0x00005555557a8acc in main (argc=3, argv=0x7fffffffe368) at emacs.c:2086

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:

diff --git a/src/xdisp.c b/src/xdisp.c
index 94f969f37c..5730145268 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2984,7 +2984,9 @@ init_iterator (struct it *it, struct window *w,
   it->dp = window_display_table (w);
 
   /* Are multibyte characters enabled in current_buffer?  */
-  it->multibyte_p = !NILP (BVAR (current_buffer, enable_multibyte_characters));
+  it->multibyte_p = WINDOW_MENU_BAR_P (w)
+    || WINDOW_TOOL_BAR_P (w)
+    || !NILP (BVAR (current_buffer, enable_multibyte_characters));
 
   /* Get the position at which the redisplay_end_trigger hook should
      be run, if it is to be run at all.  */
@@ -6864,7 +6866,9 @@ reseat_1 (struct it *it, struct text_pos pos, bool set_stop_p)
   it->method = GET_FROM_BUFFER;
   it->object = it->w->contents;
   it->area = TEXT_AREA;
-  it->multibyte_p = !NILP (BVAR (current_buffer, enable_multibyte_characters));
+  it->multibyte_p = WINDOW_MENU_BAR_P (it->w)
+    || WINDOW_TOOL_BAR_P (it->w)
+    || !NILP (BVAR (current_buffer, enable_multibyte_characters));
   it->sp = 0;
   it->string_from_display_prop_p = false;
   it->string_from_prefix_prop_p = false;

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.  */

bug#37385: 27.0.50; Crash on multibyte assertion violation

[Juri Linkov]

>> 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.)

Thanks for the proper fix.  I tried and it works without problems.
Also I found third place (in addition to two places in your patch)
where STRING_MULTIBYTE could be used for reseat_to_string.  Is the
following change needed as well?

diff --git a/src/xdisp.c b/src/xdisp.c
index 9f999c7903..af6faf9d34 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13804,7 +13812,8 @@ tool_bar_height (struct frame *f, int *n_rows, bool pixelwise)
   temp_row->reversed_p = false;
   it.first_visible_x = 0;
   it.last_visible_x = WINDOW_PIXEL_WIDTH (w);
-  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));
   it.paragraph_embedding = L2R;
 
   while (!ITERATOR_AT_END_P (&it))

bug#37385: 27.0.50; Crash on multibyte assertion violation

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 37385@debbugs.gnu.org
> Date: Fri, 13 Sep 2019 00:30:11 +0300
> 
> Thanks for the proper fix.  I tried and it works without problems.
> Also I found third place (in addition to two places in your patch)
> where STRING_MULTIBYTE could be used for reseat_to_string.  Is the
> following change needed as well?

Yes, thanks.  I installed all of them, and I'm closing this bug
report.