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

From: Gerd Möllmann <[email-masked]>
Date: Tue, 25 Mar 2025 05:48:23 +0100

Gerd Möllmann <[email-masked]> writes:

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

Or maybe simply use the glyph at string - 1 when cur_x == last column.
STRING is always part of a row, and we don't have rows that are
only partially filled, or are so narrow that string - 1 isn't valid.