Subject: bug#7728: 24.0.50; GDB backtrace from abort

From: Eli Zaretskii <[email-masked]>
Date: Sun, 09 Jan 2011 23:18:14 +0200

Ping!

This is a bug that I think we should fix for Emacs 23.3.

> Date: Sat, 01 Jan 2011 20:02:17 +0200
> From: Eli Zaretskii <[email-masked]>
> Cc: [email-masked]
> 
> > From: Stefan Monnier <[email-masked]>
> > Cc: Drew Adams <[email-masked]>,  [email-masked]
> > Date: Sat, 25 Dec 2010 15:35:31 -0500
> > 
> > If selected_window is nil because of Fset_window_configuration, then it
> > is presumably nil for all the intervening and some of the subsequent
> > code, and I suspect fixing it earlier in the call chain will be
> > preferable: e.g., it doesn't make sense to "display_and_set_cursor" in
> > the "selected_window = nil" case.
> 
> After some looking around, I'm sorry to report that I don't see how to
> do that.  Here are the details:
> 
> We set selected_window to nil as a semi-kludgey way of preventing
> select-window from storing in the old selected window the value of
> point of the buffer that has been restored into that window.
> select-window has this code:
> 
>   sf = SELECTED_FRAME ();
>   if (XFRAME (WINDOW_FRAME (w)) != sf)
>     {
>       XFRAME (WINDOW_FRAME (w))->selected_window = window;
>       /* Use this rather than Fhandle_switch_frame
> 	 so that FRAME_FOCUS_FRAME is moved appropriately as we
> 	 move around in the state where a minibuffer in a separate
> 	 frame is active.  */
>       Fselect_frame (WINDOW_FRAME (w), norecord);
>       /* Fselect_frame called us back so we've done all the work already.  */
>       eassert (EQ (window, selected_window));
>       return window;
>     }
>   else
>     sf->selected_window = window;
> 
>   /* Store the current buffer's actual point into the
>      old selected window.  It belongs to that window,
>      and when the window is not selected, must be in the window.  */
>   if (!NILP (selected_window))
>     {
>       ow = XWINDOW (selected_window);
>       if (! NILP (ow->buffer))
> 	set_marker_both (ow->pointm, ow->buffer,
> 			 BUF_PT (XBUFFER (ow->buffer)),
> 			 BUF_PT_BYTE (XBUFFER (ow->buffer)));
>     }
> 
>   selected_window = window;
> 
> The last `if' clause is what we want to bypass, when we restore window
> configuration as the last part of save-window-excursion.  Note that
> select-window stores the right value into selected_window right after
> that, but the call to Fselect_frame that crashes happens before that,
> so selected_window is still nil.
> 
> Now, the sequence of calls leading to the crash inside Fselect_frame
> is as follows:
> 
>   Fselect_frame
>   -> do_switch_frame
>      -> Fredirect_frame_focus
>         -> w32_frame_rehighlight / XTframe_rehighlight
>            -> x_frame_rehighlight
>               -> frame_highlight
>                  -> x_update_cursor
>                     -> update_cursor_in_window_tree
>                        -> update_window_cursor
>                           -> display_and_set_cursor
>                              -> erase_phys_cursor
>                                 -> window_text_bottom_y
>                                    -> die
> 
> My conclusion after studying this is that everything that happens
> below Fselect_frame is reasonable: we switch to the frame and redraw
> the cursor in all of its windows.  In particular,
> update_cursor_in_window_tree simply walks the entire window tree of
> the newly selected frame.  I don't see how we can avoid any of this
> when selected_window is nil, because selected_window has nothing to do
> with the windows that are being processed.  None of these functions
> even references selected_window, which is TRT.  The first place that
> does reference selected_window is the CURRENT_MODE_LINE_HEIGHT macro
> used in window_text_bottom_y, and that leads to the abort.
> 
> So I see 2 ways to prevent this particular problem:
> 
>  1) Handle the case of selected_window == Qnil in
>     CURRENT_MODE_LINE_FACE_ID.
> 
>  2) Change the code of Fset_window_configuration and Fselect_window,
>     to have some other way of preventing the latter from storing point
>     in the old selected window, without setting selected_window to
>     nil.
> 
> Any other ideas are welcome.
>