Subject: Re: bug#77233: 31.0.50; crash if message starts with a space and then without it

From: Gerd Möllmann <[email-masked]>
Date: Mon, 24 Mar 2025 21:12:19 +0100

Pip Cet <[email-masked]> writes:

> "Eli Zaretskii" <[email-masked]> writes:
>
>>> Date: Mon, 24 Mar 2025 14:14:11 +0000
>>> From: Pip Cet <[email-masked]>
>>> Cc: Daniel Clemente <[email-masked]>, [email-masked]
>>>
>>> "Eli Zaretskii" <[email-masked]> writes:
>>>
>>> >> #2  0x00005555556730a9 in cmcheckmagic (tty=0x555555a983a0) at cm.c:122
>>> >> 122        emacs_abort ();
>>> >> (gdb) list
>>> >> 117      if (frame_size_change_delayed (XFRAME (tty->top_frame)))
>>> >> 118        return;
>>> >> 119      if (curX (tty) == FrameCols (tty))
>>> >> 120        {
>>> >> 121          if (!MagicWrap (tty) || curY (tty) >= FrameRows (tty) - 1)
>>> >> 122        emacs_abort ();
>>> >> 123          if (tty->termscript)
>>> >> 124        putc ('\r', tty->termscript);
>>> >> 125          putc ('\r', tty->output);
>>> >> 126          if (tty->termscript)
>>> >> (gdb) p curY(tty)
>>> >> $1 = 60
>>> >> (gdb) p FrameRows(tty)-1
>>> >> $2 = 60
>>> >> (gdb) p FrameCols(tty)
>>> >> $3 = 100
>>> >> (gdb)
>>> >
>>> > What is the value of curX(tty) ?  And if (as I'd expect) it is 100,
>>> > then do you have any idea how come it became 100 for a 2-character
>>> > message?
>>>
>>> It is equal to FrameCols (tty), as established in line 119 (this
>>> requires a breakpoint in emacs_abort rather than terminate_due_to_signal).
>>>
>>> > Can anyone else reproduce this?
>>>
>>> I can, on GNU/Linux with no special options and in an xterm.
>>
>> And you can explain how come curX(tty) got such a large value?
>
> We tried to print a character at (cols-1,rows-1), which left the
> remembered cursor position at (cols,rows-1).  According to the comments
> in term.c, "some terminals" would have scrolled the entire terminal when
> we wrote that character, which is the situation detected by
> cmcheckmagic.
>
> The code Gerd pushed looks like this:
>
> static void
> tty_write_glyphs (struct frame *f, struct glyph *string, int len)
> {
>   struct tty_display_info *tty = FRAME_TTY (f);
>   /* Don't dare write in last column of bottom line, if Auto-Wrap,
>      since that would scroll the whole frame on some terminals.  */
>   if (AutoWrap (tty)
>       && curY (tty) + 1 == FRAME_TOTAL_LINES (f)
>       && curX (tty) + len == FRAME_COLS (f)
>       && curX (tty) < FRAME_COLS (f) - 1
>       && len > 0)
>     {
>       /* Write glyphs except the first. */
>       int old_x = curX (tty), old_y = curY (tty);
>       tty_write_glyphs_1 (f, string + 1, len - 1);
>
>       /* Insert the first glyph, shifting the rest right.  */
>       cmgoto (tty, old_y, old_x);
>       tty_insert_glyphs (f, string, 1);
>     }
>   else
>     tty_write_glyphs_1 (f, string, len);
> }
>
> in our case (len == 1), we end up in the "else" branch because curX ==
> cols - 1, attempting to print a single character in the bottom-right
> corner of the terminal.
>
> The "else" branch simply prints the character and leaves the cursor
> position "invalid".
>
> Note that xterm (the application) doesn't appear to scroll the terminal
> in this case, so it's a bit of a historical exercise to fix things on
> those terminals that would have, but IIUC, the right way is to retrieve
> the character at (cols-2,rows-1), goto (cols-2,rows-1), print the
> character that's supposed to end up at (cols-1,rows-1), goto
> (cols-2,rows-1) again, then print the retrieved character.
>
> This is complicated because we'd need to retrieve the character at
> (cols-2, rows-1) from the frame matrix if tty_write_glyphs is called
> with len == 1 and the cursor is at (cols-1,rows-1).
>
> Anyway, here's a patch that does this, but it seems too complicated to
> be worth it to me:
>
> diff --git a/src/term.c b/src/term.c
> index e15b7a0887e..ccee6c650e3 100644
> --- a/src/term.c
> +++ b/src/term.c
> @@ -971,16 +971,36 @@ tty_write_glyphs (struct frame *f, struct glyph *string, int len)
>    if (AutoWrap (tty)
>        && curY (tty) + 1 == FRAME_TOTAL_LINES (f)
>        && curX (tty) + len == FRAME_COLS (f)
> -      && curX (tty) < FRAME_COLS (f) - 1

Ah, I see. It got into the else branch for len = 1. I think I
misinterpreted what is happening. It didn't even try to insert. Sorry
for that.

>        && len > 0)
>      {
> -      /* Write glyphs except the first. */
> -      int old_x = curX (tty), old_y = curY (tty);
> -      tty_write_glyphs_1 (f, string + 1, len - 1);
> +      if (len > 1)
> +	{
> +	  /* Write glyphs except the first. */
> +	  int old_x = curX (tty), old_y = curY (tty);
> +	  tty_write_glyphs_1 (f, string + 1, len - 1);
>  
> -      /* Insert the first glyph, shifting the rest right.  */
> -      cmgoto (tty, old_y, old_x);
> -      tty_insert_glyphs (f, string, 1);
> +	  /* Insert the first glyph, shifting the rest right.  */
> +	  cmgoto (tty, old_y, old_x);
> +	  tty_insert_glyphs (f, string, 1);
> +	}
> +      else
> +	{
> +	  struct glyph glyphs[2];
> +	  glyphs[1] = string[0];
> +	  if (!f->current_matrix || f->current_matrix->matrix_h < curY (tty))
> +	    return;
> +	  struct glyph_row *row = &f->current_matrix->rows[curY (tty)];
> +	  int area = 0;
> +	  int x = curX (tty) - 1;
> +	  while (x > row->used[area])
> +	    {
> +	      x -= row->used[area];
> +	      if (area++ == LAST_AREA)
> +		return;
> +	    }
> +	  glyphs[0] = row->glyphs[area][x];
> +	  tty_write_glyphs (f, glyphs, 2);
> +	}

Something like that, yes.

>      }
>    else
>      tty_write_glyphs_1 (f, string, len);