bug#73022: 31.0.50; Crash in build_frame_matrix_from_leaf_window after C-x 2 and reducing terminal size

[Daniel Clemente <n142857@gmail.com>]

> Please try the attached diff (from my heavily edited copy of master, if
> it doesn't apply cleanly, complain immediately rather than messing up
> your Emacs).  It should fix two silly bugs in window.el that make a
> frame's safe minimum size much too large and includes the fix I proposed
> earlier here.

It applies well; thanks.
By the way, (window-min-size) still answers 4 (with and without your patch).

Emacs feels a bit "stronger" in the C-x 2 scenario, meaning that it
survives shrinking+enlarging the terminal, but it still crashes
sometimes.
But it also crashes in a new situation: just by shrinking the
normal/initial/unsplit frame, i.e. without needing to do C-x 2. It
didn't crash before, so the patch doesn't make the whole Emacs more
stable yet.


Without doing C-x 2:

#4  0x00005555556c3016 in emacs_abort () at sysdep.c:2391
#5  0x000055555566d36f in cmcheckmagic (tty=0x55555608fa80) at cm.c:122
#6  0x0000555555671b1b in tty_write_glyphs (f=0x555556012ff0,
string=0x7ffff0f4ec10, len=80) at term.c:831
#7  0x000055555567c11d in write_glyphs (f=0x555556012ff0,
string=0x7ffff0f4dd10, len=80) at terminal.c:164
#8  0x0000555555591b60 in update_frame_line (f=0x555556012ff0, vpos=3,
updating_menu_p=false) at dispnew.c:5326

126          if (tty->termscript)
(gdb) p curY (tty)
$1 = 3
(gdb) p FrameRows (tty) - 1
$2 = 3
(gdb)


This one still happens (this is after C-x 2):
#5  0x000055555558b697 in build_frame_matrix_from_leaf_window
(frame_matrix=0x555556050420,
    w=0x555556013210) at dispnew.c:2647

>
> And it moves the assignment of FrameRows to handle_window_change_signal
> in dispnew.c.  Doing it in adjust_frame_size was silly (as Gerd Möllmann
> noticed earlier).  FrameRows should be the height of the tty which can
> be smaller than the height of our frame.  Emacs is supposed to store it
> but not to modify it according to our capabilities.
>

I don't have the expert opinion of how it should be. But I agree that
a source of the problem is that the tty can be smaller than the frame.
Frame and terminal may have different sizes and this creates
inconsistencies.

For instance, Eli recently added this code (dispnew.c):

  /* This should never happen, but evidently sometimes does if one
     resizes the frame quickly enough.  Prevent aborts in cmcheckmagic.  */
  if (vpos >= FRAME_TOTAL_LINES (f))
    return;

But this is checking the *frame*.  Later, the assertion in
cmcheckmagic will be made about the *terminal*.
I think the frame never gets smaller than 2 lines (there's code in
handle_window_change_signal to prevent it), but the tty can. So if
we're in a 1-line terminal, and updating the 2nd line, the above code
sees nothing wrong (the frame still has 2 lines) but cmcheckmagick
won't like it (the terminal doesn't have a 2nd line).
(Usual disclaimer: I don't know very well how this works. I'm often
brainstorming).

> And it moves the assignment of FrameRows to handle_window_change_signal
> in dispnew.c.

In practice, that code (in the new place you put it) is still inside an if: the
  if (width > 5 && height > 2)
in handle_window_change_signal. This means that in e.g. a 1-line
terminal, you won't be setting FrameRows to the actual number of lines
of terminal (1), as you wanted. It will still be the old value, e.g.
2, and it may make cmcheckmagic crash by trying to do things in line 2
(which exists in the frame, which has 2 lines) but not in the terminal
(which doesn't have a 2nd line).


One option seems to have FrameRows always match the amount of terminal
lines. Can we remove the "if (… height >2)" limitation, and allow
1-line, 2-line frames etc.? (To really match the terminal size). Other
changes may be needed in other places, to avoid working with very
small frames..

Otherwise, if we let FrameRows be larger than the amount of terminal
lines, another option could be:
At the beginning of tty_write_glyphs, check whether we've been asked
to write glyphs in a line which is higher than the amount of lines in
the terminal. If so, return without writing anything. (Because the
line would be invisible).
Or maybe in some of the callers of tty_write_glyphs. Don't call
tty_write_glyphs to write glyphs in a line which is not visible (it
exists in the frame, but doesn't exist in the terminal).


By the way, I saw that the cmcheckmagic code checks curY (tty) >=
FrameRows (tty) - 1, note the -1, whereas the dispnew.c code I quote
above doesn't use the -1. I hope this is ok. As mentioned they're
checking different things so it may be ok, though I don't understand
why the -1.