Subject: bug#38966: closed (Re: bug#38966: 27.0.60; Assertion failure in set_cursor_from_row)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 06 Jan 2020 19:51:03 +0000

Your bug report

#38966: 27.0.60; Assertion failure in set_cursor_from_row

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
38966: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=38966
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
From: Eli Zaretskii <[email-masked]>
To: martin rudalics <[email-masked]>
Cc: [email-masked]
Date: Mon, 06 Jan 2020 21:50:51 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> Cc: [email-masked]
> From: martin rudalics <[email-masked]>
> Date: Mon, 6 Jan 2020 20:06:21 +0100
> 
>  > I don't expect to have a window that has no lines showing text.  I
>  > believe we don't allow creation/resizing of windows to such a small
>  > size?  If that's not guaranteed, I'm okay with adding an assertion
>  > somewhere, but that would be a separate problem: we never expected
>  > such a calamity even before tab-lines were added.
> 
> We did.

OK, let me rephrase: _I_ didn't, okay?  IOW, the display code doesn't,
and AFAIR never did.

(For some reason I seem to make people angry today, and you seem to be
one of them.  Apologies -- I don't know for what.)

> Try with emacs 26 loading my old test-popup-2.el (attached once
> more).  Type C-x 2 and make the lower window as low as you can.  Now
> hit F2.  The lower window shows no text.

Why is such a window useful?  I thought we had a minimum window height
beyond which we didn't allow to resize windows.  Isn't that true
anymore?

> I plan a slight modification that would allow the minibuffer window to
> shrink to zero lines whenever it's not used.  OTOH, zero size "normal"
> windows would allow to show the minibuffer window in a frame that can
> acommodate only one or two lines.

All I'm saying is that making the display code prepared to deal with
such windows might take more changes, and this would be a separate
issue, suitable for master, not for the release branch.  OK?

>  > If we don't call set_cursor_from_row, we will not have a cursor
>  > displayed where it should be, which is a display bug in and of itself.
> 
> Isn't that statement overly conservative?  Sooner or later, Emacs will
> at least have to allow a mode where the region is not destroyed when a
> window is scrolled, allowing the cursor to disappear from the visible
> part of its buffer.

That's not the same situation.  If point is not in the window, it's OK
not to display the cursor.  But I'm talking about a situation where
point _is_ in the window.

>  > Sorry, but that ship sailed a long time ago.  You will have to make do
>  > with the comments in dispextern.h, which document this weirdness.
> 
> Which comment?

This one:

  /* True means row is a mode or header/tab-line.  */
  bool_bf mode_line_p : 1;

> In either case, rest assured that ordinary people will
> have considerable troubles understanding code like the proposed
> 
> 	  if (row->tab_line_p)
> 	    ++row;
> 	  if (row->mode_line_p)
> 	    ++row;

I added comments explaining what's going on.

> Notwithstanding my concerns, the patch fixes the bug here.

Thanks, pushed to the emacs-27 branch, and closing the bug.

Subject: 27.0.60; Assertion failure in set_cursor_from_row
From: martin rudalics <[email-masked]>
To: Bug-Gnu-Emacs <[email-masked]>
Date: Mon, 6 Jan 2020 10:19:02 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

In GNU Emacs 27.0.60 (build 13, x86_64-w64-mingw32)
  of 2020-01-05 built on MACHNO
Repository revision: 7f01dfca5600fcd3e3548aee50734aab4b96b02f
Repository branch: emacs-27

With emacs -Q load the attached test-set-cursor-from-row.el and use
the mouse to drag the upper mode line up and down.  As soon as one of
the windows gets sufficiently small, I get an assertion failure like

Thread 1 hit Breakpoint 1, terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at ../../src/emacs.c:371
371	  signal (sig, SIG_DFL);
(gdb) bt
#0  terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at ../../src/emacs.c:371
#1  0x00000004002c13cd in die (msg=0x400974295 <get_next_element+3989> "!row->mode_line_p", file=0x400970642 <DEFAULT_REHASH_SIZE+4958> "../../src/xdisp.c", line=16249) at ../../src/alloc.c:7246
#2  0x000000040007ca89 in set_cursor_from_row (w=0x76c8500, row=0x76b0070, matrix=0x76cd580, delta=0, delta_bytes=0, dy=0, dvpos=0) at ../../src/xdisp.c:16249
#3  0x0000000400087177 in redisplay_window (window=XIL(0x76c8505), just_this_one_p=false) at ../../src/xdisp.c:18780
#4  0x000000040007c87c in redisplay_window_0 (window=XIL(0x76c8505)) at ../../src/xdisp.c:16190
#5  0x000000040031a182 in internal_condition_case_1 (bfun=0x40007c83d <redisplay_window_0>, arg=XIL(0x76c8505), handlers=XIL(0x6421ebb), hfun=0x40007c801 <redisplay_window_error>) at ../../src/eval.c:1379
#6  0x000000040007c7d3 in redisplay_windows (window=XIL(0x76c8505)) at ../../src/xdisp.c:16170
#7  0x000000040007c785 in redisplay_windows (window=XIL(0x76c82f5)) at ../../src/xdisp.c:16164
#8  0x000000040007b0ad in redisplay_internal () at ../../src/xdisp.c:15638
#9  0x00000004000789af in redisplay () at ../../src/xdisp.c:14865
#10 0x00000004001b6162 in read_char (commandflag=1, map=XIL(0x77dcfb3), prev_event=XIL(0), used_mouse_menu=0xbff25f, end_time=0x0) at ../../src/keyboard.c:2493
#11 0x00000004001c93dd in read_key_sequence (keybuf=0xbff490, prompt=XIL(0), dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true, prevent_redisplay=false) at ../../src/keyboard.c:9553
#12 0x00000004001b28fc in command_loop_1 () at ../../src/keyboard.c:1350
#13 0x000000040031a08d in internal_condition_case (bfun=0x4001b23c6 <command_loop_1>, handlers=XIL(0x90), hfun=0x4001b1807 <cmd_error>) at ../../src/eval.c:1355
#14 0x00000004001b1f3c in command_loop_2 (ignore=XIL(0)) at ../../src/keyboard.c:1091
#15 0x00000004003194e2 in internal_catch (tag=XIL(0xdfe0), func=0x4001b1f0a <command_loop_2>, arg=XIL(0)) at ../../src/eval.c:1116
#16 0x00000004001b1e92 in command_loop () at ../../src/keyboard.c:1070
#17 0x0000000000000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Lisp Backtrace:
"redisplay_internal (C function)" (0x0)

The immediate cause is that the *scratch* window has a header _and_ a
tab line.  Disabling either of these makes the failure disappear.

The culprit is IIUC this part of xdisp.c

       /* Finally, fall back on the first row of the window after the
	 header line (if any).  This is slightly better than not
	 displaying the cursor at all.  */
       if (!row)
	{
	  row = matrix->rows;
	  if (row->mode_line_p)
	    ++row;
	}
       set_cursor_from_row (w, row, matrix, 0, 0, 0, 0);

which is no more valid because the first row is now the tab line of
the *scratch* window.  In this case the line we get by ++row is the
header line triggering the subsequent assertion failure

   /* Don't even try doing anything if called for a mode-line or
      header-line row, since the rest of the code isn't prepared to
      deal with such calamities.  */
   eassert (!row->mode_line_p);
   if (row->mode_line_p)
     return false;

If my analysis is correct, we should probably skip two lines to move
to the first text line of the window.  But I would not exclude the
possibility (I have not tried to produce such behavior yet) that that
third line does not even exist (what would the "(if any)" in the
comment otherwise allude to?) or at least is the mode line of that
window and the assertion failure would trigger again.

So I think we should do two things there: (1) skip subsequent lines
with row->mode_line_p enabled and (2) not call set_cursor_from_row
when we have consumed all rows.

BTW: I would also recommend to rename mode_line_p to something like
say "no_text_line_p".  Presently, people like me not used to hack the
display code, might consider mode_line_p verbatim and not apply its
semantics to header or tab lines.

martin