Re: [PATCH] Call target_terminal_ours() before refreshing TUI's frame info

[Patrick Palka <patrick@parcs.ath.cx>]

On Tue, Jun 2, 2015 at 11:52 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> In some cases tui_show_frame_info() may get called while the inferior's
> terminal settings are still in effect.  But when we call this function
> we absolutely need to have our terminal settings in effect because the
> function is responsible for redrawing TUI's windows following a change
> in the selected frame or a change in the PC.  If our terminal settings
> are not in effect, the screen does not get redrawn properly, causing
> temporary display artifacts (which can be fixed via ^L).
>
> This scenario happens most prominently when stepping through a program
> in TUI while a watchpoint is in effect.
>
> Here is an example backtrace for when tui_show_frame_info() gets called
> while target_terminal_is_inferior() == 1:
>
>   #1  0x00000000004988ee in tui_selected_frame_level_changed_hook (level=0)
>   #2  0x0000000000617b99 in select_frame (fi=0x18c9820)
>   #3  0x0000000000617c3f in get_selected_frame (message=message@entry=0x0)
>   #4  0x00000000004ce534 in update_watchpoint (b=b@entry=0x2d9a760,
>       reparse=reparse@entry=0)
>   #5  0x00000000004d625e in insert_breakpoints ()
>   #6  0x0000000000531cfe in keep_going (ecs=ecs@entry=0x7ffea7884ac0)
>   #7  0x00000000005326d7 in process_event_stop_test (ecs=ecs@entry=0x7ffea7884ac0)
>   #8  0x000000000053596e in handle_inferior_event_1 (ecs=0x7ffea7884ac0)
>
> The fix is simple: call target_terminal_ours() before calling
> tui_show_frame_info() in TUI's frame-changed hook, making sure to
> restore the original terminal settings afterwards.
>
> gdb/ChangeLog:
>
>         * tui/tui-hooks.c (tui_selected_frame_level_changed_hook): Call
>         target_terminal_ours() before calling tui_show_frame_info(), and
>         restore the original terminal settings afterwards.
> ---
>  gdb/tui/tui-hooks.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
> index e53f526..3331d67 100644
> --- a/gdb/tui/tui-hooks.c
> +++ b/gdb/tui/tui-hooks.c
> @@ -127,11 +127,15 @@ tui_selected_frame_level_changed_hook (int level)
>  {
>    struct frame_info *fi;
>    CORE_ADDR pc;
> +  struct cleanup *old_chain;
>
>    /* Negative level means that the selected frame was cleared.  */
>    if (level < 0)
>      return;
>
> +  old_chain = make_cleanup_restore_target_terminal ();
> +  target_terminal_ours ();
> +
>    fi = get_selected_frame (NULL);
>    /* Ensure that symbols for this frame are read in.  Also, determine
>       the source language of this frame, and switch to it if
> @@ -160,6 +164,8 @@ tui_selected_frame_level_changed_hook (int level)
>        tui_check_data_values (fi);
>        tui_refreshing_registers = 0;
>      }
> +
> +  do_cleanups (old_chain);
>  }
>
>  /* Called from print_frame_info to list the line we stopped in.  */
> --
> 2.4.2.387.gf86f31a.dirty
>

Ping.  Could this patch make it into 7.10?  It is pretty harmless.