bug#47207: 28.0.50; decode_next_window_args crash

[Alan Mackenzie <acm@muc.de>]

Hello, Martin.

On Thu, Apr 15, 2021 at 16:45:20 +0200, martin rudalics wrote:
>  > That's quite an involved thread.  I've read through it, but can't
>  > remember seeing a firm decision on how to fix the problem, or even
>  > if it was fixed then.

> It wasn't fixed then.

Ah.

>  > The current problem in minibuf.c seems to be that we assume
>  > f->minibuffer_window to be non-nil in several (~6) places, rather
>  > than checking it.

> We assume that it is a live window.

Indeed, yes.

>  > In most of these places, if we detect NILP (f->m_w), we can simply skip
>  > the action we were intending to take.  This is certainly the case in the
>  > bit that gave you the trouble in read_minibuf_unwind, where we're
>  > stepping through all frames looking for the expired minibuffer.

> The most secure way is to skip the action whenever f->minibuffer_window
> fails the WINDOW_LIVE_P check.

OK, DONE.

>  > It might well be that we never call do_switch_frame to a tool-tip frame,
>  > for example.  Do you know if this is the case?  This could save some
>  > checking code.

> I wrote some code that avoids that a tooltip window ever gets selected
> and do_switch_frame returns silently when asked to switch to a tooltip
> frame.  But I wouldn't rely on that - the display engine might still try
> to select a tooltip window for some reason.

OK.

>  > So, where do we go from here?  I'm quite willing to make these changes
>  > to minibuf.c.  Would that be OK with everybody else?

> It would be OK with me (and should fix Bug#47774 too).

Could you please look at my patch (below).  It seems to fix the bug you
gave a recipe for a couple of posts back.  I'm hoping it can be
committed as it is.

> And please look into Bug#47781 next.

OK.



diff --git a/src/minibuf.c b/src/minibuf.c
index c9831fd50f..a3c1b99bf3 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -112,13 +112,15 @@ choose_minibuf_frame (void)
 {
   if (FRAMEP (selected_frame)
       && FRAME_LIVE_P (XFRAME (selected_frame))
+      && WINDOW_LIVE_P (XFRAME (selected_frame)->minibuffer_window)
       && !EQ (minibuf_window, XFRAME (selected_frame)->minibuffer_window))
     {
       struct frame *sf = XFRAME (selected_frame);
-      /* I don't think that any frames may validly have a null minibuffer
-	 window anymore.  */
-      if (NILP (sf->minibuffer_window))
-	emacs_abort ();
+      /* I don't think that any frames may validly have a null
+	 minibuffer window anymore.  (2021-04-15): Tooltip frames have
+	 a null MB.  Comment out the following.  */
+      /* if (NILP (sf->minibuffer_window)) */
+      /* 	emacs_abort (); */
 
       minibuf_window = sf->minibuffer_window;
     }
@@ -195,7 +197,9 @@ move_minibuffers_onto_frame (struct frame *of, bool for_deletion)
 	&& (for_deletion || minibuf_follows_frame () || FRAME_INITIAL_P (of))))
     return;
   if (FRAME_LIVE_P (f)
-      && !EQ (f->minibuffer_window, of->minibuffer_window))
+      && !EQ (f->minibuffer_window, of->minibuffer_window)
+      && WINDOW_LIVE_P (f->minibuffer_window) /* F not a tootip frame */
+      && WINDOW_LIVE_P (of->minibuffer_window))
     {
       zip_minibuffer_stacks (f->minibuffer_window, of->minibuffer_window);
       if (for_deletion && XFRAME (MB_frame) != of)
@@ -636,6 +640,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   mini_frame = WINDOW_FRAME (XWINDOW (minibuf_window));
 
   if (minibuf_level > 1
+      && WINDOW_LIVE_P (XFRAME (MB_frame)->minibuffer_window)
       && !EQ (XWINDOW (XFRAME (selected_frame)->minibuffer_window)->frame,
 	      MB_frame)
       && minibuf_moves_frame_when_opened ()
@@ -908,11 +913,13 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   unbind_to (count, Qnil);
 
   /* Switch the frame back to the calling frame.  */
-  if ((!EQ (selected_frame, calling_frame)
-       || !EQ (XWINDOW (XFRAME (calling_frame)->minibuffer_window)->frame,
-	       calling_frame))
-      && FRAMEP (calling_frame)
-      && FRAME_LIVE_P (XFRAME (calling_frame)))
+  if (FRAMEP (calling_frame)
+      && FRAME_LIVE_P (XFRAME (calling_frame))
+      && (!EQ (selected_frame, calling_frame)
+	  || (WINDOW_LIVE_P (XFRAME (calling_frame)->minibuffer_window)
+	      && !EQ (XWINDOW (XFRAME (calling_frame)->minibuffer_window)
+		      ->frame,
+		      calling_frame))))
     call2 (intern ("select-frame-set-input-focus"), calling_frame, Qnil);
 
   /* Add the value to the appropriate history list, if any.  This is
@@ -1056,10 +1063,13 @@ read_minibuf_unwind (void)
     {
       f = XFRAME (exp_MB_frame);
       window = f->minibuffer_window;
-      w = XWINDOW (window);
-      if (EQ (w->frame, exp_MB_frame)
-	  && EQ (w->contents, nth_minibuffer (minibuf_level)))
-	goto found;
+      if (WINDOW_LIVE_P (window))
+	{
+	  w = XWINDOW (window);
+	  if (EQ (w->frame, exp_MB_frame)
+	      && EQ (w->contents, nth_minibuffer (minibuf_level)))
+	    goto found;
+	}
     }
   return; /* expired minibuffer not found.  Maybe we should output an
 	     error, here. */


> martin

-- 
Alan Mackenzie (Nuremberg, Germany).