[PATCH] Fix terminal state corruption when starting a program from within TUI

[PATCH] Fix terminal state corruption when starting a program from within TUI

[Patrick Palka]

The TUI terminal state becomes corrupted (e.g. key sequences such as
Alt_F and Alt_B no longer work) when one attaches to an inferior process
(via "run" or "attach") from within TUI.  This terminal corruption
remains until you switch out of TUI mode.

This happens because the terminal state is not properly saved when
switching to and out from TUI mode.  Although the functions tui_enable()
and tui_disable() both call the function target_terminal_save_ours() to
save the terminal state, this function is a no-op unless GDB has already
attached to an inferior process.  This is because only the "native"
target has a useful implementation of target_terminal_save_ours()
(namely child_terminal_save_ours()) and we only have the "native" target
in our target vector if GDB has already attached to an inferior process.

So without an inferior process, switching to and from TUI mode does not
actually save the terminal state.  Therefore when you attach to an
inferior process from within TUI mode, the proper terminal state is not
restored (after swapping from the inferior's terminal back to the GDB
terminal).

To fix this we just have to ensure that the terminal state is always
being properly saved when switching from and to TUI mode.  To achieve
this, this patch removes the polymorphic function
target_terminal_save_ours() and replaces it with a regular function
gdb_save_tty_state() that always saves the terminal state.

Tested on x86_64-unknown-linux-gnu by running "make check", no new
regressions.

gdb/ChangeLog:
	* target.h (struct target_ops::to_terminal_save_ours): Remove
	declaration.
	(target_terminal_save_ours): Remove macro.
	* target-delegates.c: Regenerate.
	* inf-child.c (inf_child_target): Don't set the nonexistent
	field to_terminal_save_ours.
	* inferior.h (child_terminal_save_ours): Rename to ...
	(gdb_save_tty_state): ... this.
	* inflow.c (child_terminal_save_ours): Rename to ...
	(gdb_save_tty_state): ... this.
---
 gdb/inf-child.c        |  1 -
 gdb/inferior.h         |  2 +-
 gdb/inflow.c           |  2 +-
 gdb/target-delegates.c | 26 --------------------------
 gdb/target.h           | 10 ----------
 gdb/tui/tui.c          |  4 ++--
 6 files changed, 4 insertions(+), 41 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 6d95e5e..9867ee6 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -481,7 +481,6 @@ inf_child_target (void)
   t->to_terminal_init = child_terminal_init;
   t->to_terminal_inferior = child_terminal_inferior;
   t->to_terminal_ours_for_output = child_terminal_ours_for_output;
-  t->to_terminal_save_ours = child_terminal_save_ours;
   t->to_terminal_ours = child_terminal_ours;
   t->to_terminal_info = child_terminal_info;
   t->to_post_startup_inferior = inf_child_post_startup_inferior;
diff --git a/gdb/inferior.h b/gdb/inferior.h
index afc29e2..269b6af 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -110,7 +110,7 @@ extern void child_terminal_info (struct target_ops *self, const char *, int);
 
 extern void term_info (char *, int);
 
-extern void child_terminal_save_ours (struct target_ops *self);
+extern void gdb_save_tty_state (void);
 
 extern void child_terminal_ours (struct target_ops *self);
 
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 4b105d1..8902174 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -241,7 +241,7 @@ child_terminal_init_with_pgrp (int pgrp)
    and gdb must be able to restore it correctly.  */
 
 void
-child_terminal_save_ours (struct target_ops *self)
+gdb_save_tty_state (void)
 {
   if (gdb_has_a_terminal ())
     {
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 843a954..fe989ff 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -813,28 +813,6 @@ debug_terminal_ours (struct target_ops *self)
 }
 
 static void
-delegate_terminal_save_ours (struct target_ops *self)
-{
-  self = self->beneath;
-  self->to_terminal_save_ours (self);
-}
-
-static void
-tdefault_terminal_save_ours (struct target_ops *self)
-{
-}
-
-static void
-debug_terminal_save_ours (struct target_ops *self)
-{
-  fprintf_unfiltered (gdb_stdlog, "-> %s->to_terminal_save_ours (...)\n", debug_target.to_shortname);
-  debug_target.to_terminal_save_ours (&debug_target);
-  fprintf_unfiltered (gdb_stdlog, "<- %s->to_terminal_save_ours (", debug_target.to_shortname);
-  target_debug_print_struct_target_ops_p (&debug_target);
-  fputs_unfiltered (")\n", gdb_stdlog);
-}
-
-static void
 delegate_terminal_info (struct target_ops *self, const char *arg1, int arg2)
 {
   self = self->beneath;
@@ -3814,8 +3792,6 @@ install_delegators (struct target_ops *ops)
     ops->to_terminal_ours_for_output = delegate_terminal_ours_for_output;
   if (ops->to_terminal_ours == NULL)
     ops->to_terminal_ours = delegate_terminal_ours;
-  if (ops->to_terminal_save_ours == NULL)
-    ops->to_terminal_save_ours = delegate_terminal_save_ours;
   if (ops->to_terminal_info == NULL)
     ops->to_terminal_info = delegate_terminal_info;
   if (ops->to_kill == NULL)
@@ -4068,7 +4044,6 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_terminal_inferior = tdefault_terminal_inferior;
   ops->to_terminal_ours_for_output = tdefault_terminal_ours_for_output;
   ops->to_terminal_ours = tdefault_terminal_ours;
-  ops->to_terminal_save_ours = tdefault_terminal_save_ours;
   ops->to_terminal_info = default_terminal_info;
   ops->to_kill = tdefault_kill;
   ops->to_load = tdefault_load;
@@ -4212,7 +4187,6 @@ init_debug_target (struct target_ops *ops)
   ops->to_terminal_inferior = debug_terminal_inferior;
   ops->to_terminal_ours_for_output = debug_terminal_ours_for_output;
   ops->to_terminal_ours = debug_terminal_ours;
-  ops->to_terminal_save_ours = debug_terminal_save_ours;
   ops->to_terminal_info = debug_terminal_info;
   ops->to_kill = debug_kill;
   ops->to_load = debug_load;
diff --git a/gdb/target.h b/gdb/target.h
index 4d91b6b..85763ba 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -506,8 +506,6 @@ struct target_ops
       TARGET_DEFAULT_IGNORE ();
     void (*to_terminal_ours) (struct target_ops *)
       TARGET_DEFAULT_IGNORE ();
-    void (*to_terminal_save_ours) (struct target_ops *)
-      TARGET_DEFAULT_IGNORE ();
     void (*to_terminal_info) (struct target_ops *, const char *, int)
       TARGET_DEFAULT_FUNC (default_terminal_info);
     void (*to_kill) (struct target_ops *)
@@ -1417,14 +1415,6 @@ extern void target_terminal_inferior (void);
 
 extern int target_supports_terminal_ours (void);
 
-/* Save our terminal settings.
-   This is called from TUI after entering or leaving the curses
-   mode.  Since curses modifies our terminal this call is here
-   to take this change into account.  */
-
-#define target_terminal_save_ours() \
-     (*current_target.to_terminal_save_ours) (&current_target)
-
 /* Print useful information about our terminal status, if such a thing
    exists.  */
 
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 7add8ba..f16c9be 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -425,7 +425,7 @@ tui_enable (void)
   tui_refresh_all_win ();
 
   /* Update gdb's knowledge of its terminal.  */
-  target_terminal_save_ours ();
+  gdb_save_tty_state ();
   tui_update_gdb_sizes ();
 }
 
@@ -455,7 +455,7 @@ tui_disable (void)
   tui_setup_io (0);
 
   /* Update gdb's knowledge of its terminal.  */
-  target_terminal_save_ours ();
+  gdb_save_tty_state ();
 
   tui_active = 0;
   tui_update_gdb_sizes ();
-- 
2.1.0

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Pedro Alves]

On 08/25/2014 07:17 PM, Patrick Palka wrote:
> The TUI terminal state becomes corrupted (e.g. key sequences such as
> Alt_F and Alt_B no longer work) when one attaches to an inferior process
> (via "run" or "attach") from within TUI.  This terminal corruption
> remains until you switch out of TUI mode.

This has bugged me for a long while, and I simply got used to
working around it.  Thanks a lot for tracking it down!

This is OK.

> gdb/ChangeLog:

> 	* inferior.h (child_terminal_save_ours): Rename to ...
> 	(gdb_save_tty_state): ... this.

Could you move this to terminal.h, right next to gdb_has_a_terminal
please ?

> 	* inflow.c (child_terminal_save_ours): Rename to ...
> 	(gdb_save_tty_state): ... this.

Thanks,
Pedro Alves

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Patrick Palka]

On Wed, 27 Aug 2014, Pedro Alves wrote:

> On 08/25/2014 07:17 PM, Patrick Palka wrote:
>> The TUI terminal state becomes corrupted (e.g. key sequences such as
>> Alt_F and Alt_B no longer work) when one attaches to an inferior process
>> (via "run" or "attach") from within TUI.  This terminal corruption
>> remains until you switch out of TUI mode.
>
> This has bugged me for a long while, and I simply got used to
> working around it.  Thanks a lot for tracking it down!
>
> This is OK.
>
>> gdb/ChangeLog:
>
>> 	* inferior.h (child_terminal_save_ours): Rename to ...
>> 	(gdb_save_tty_state): ... this.
>
> Could you move this to terminal.h, right next to gdb_has_a_terminal
> please ?
>
>> 	* inflow.c (child_terminal_save_ours): Rename to ...
>> 	(gdb_save_tty_state): ... this.
>
> Thanks,
> Pedro Alves
>
>

Done, thanks for reviewing. I moved the declaration to terminal.h and 
updated the ChangeLog accordingly.  Meanwhile I noticed I omitted 
ChangeLog entries for the changes in tui/tui.c, which I now added.  Note 
that I don't have commit access so I would appreciate if someone would
commit this on my behalf.

Patrick

("git am --scissors" should be able to apply this patch email properly. I 
hope.)

-- >8 --

Subject: [PATCH] Fix terminal state corruption when starting a program from within TUI

The TUI terminal state becomes corrupted (e.g. key sequences such as
Alt_F and Alt_B no longer work) when one attaches to an inferior process
(via "run" or "attach") from within TUI.  This terminal corruption
remains until you switch out of TUI mode.

This happens because the terminal state is not properly saved when
switching to and out from TUI mode.  Although the functions tui_enable()
and tui_disable() both call the function target_terminal_save_ours() to
save the terminal state, this function is a no-op unless GDB has already
attached to an inferior process.  This is because only the "native"
target has a useful implementation of target_terminal_save_ours()
(namely child_terminal_save_ours()) and we only have the "native" target
in our target vector if GDB has already attached to an inferior process.

So without an inferior process, switching to and from TUI mode does not
actually save the terminal state.  Therefore when you attach to an
inferior process from within TUI mode, the proper terminal state is not
restored (after swapping from the inferior's terminal back to the GDB
terminal).

To fix this we just have to ensure that the terminal state is always
being properly saved when switching from and to TUI mode.  To achieve
this, this patch removes the polymorphic function
target_terminal_save_ours() and replaces it with a regular function
gdb_save_tty_state() that always saves the terminal state.

Tested on x86_64-unknown-linux-gnu by running "make check", no new
regressions.

gdb/ChangeLog:
 	* target.h (struct target_ops::to_terminal_save_ours): Remove
 	declaration.
 	(target_terminal_save_ours): Remove macro.
 	* target-delegates.c: Regenerate.
 	* inf-child.c (inf_child_target): Don't set the nonexistent
 	field to_terminal_save_ours.
 	* inferior.h (child_terminal_save_ours): Remove declaration.
 	* terminal.h (gdb_save_tty_state): New declaration.
 	* inflow.c (child_terminal_save_ours): Rename to ...
 	(gdb_save_tty_state): ... this.
 	* tui/tui.c: Include terminal.h.
 	(tui_enable): Use gdb_save_tty_state instead of
 	target_terminal_save_ours.
 	(tui_disable): Likewise.
---
  gdb/inf-child.c        |  1 -
  gdb/inferior.h         |  2 --
  gdb/inflow.c           |  2 +-
  gdb/target-delegates.c | 26 --------------------------
  gdb/target.h           | 10 ----------
  gdb/terminal.h         |  2 ++
  gdb/tui/tui.c          |  5 +++--
  7 files changed, 6 insertions(+), 42 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 6d95e5e..9867ee6 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -481,7 +481,6 @@ inf_child_target (void)
    t->to_terminal_init = child_terminal_init;
    t->to_terminal_inferior = child_terminal_inferior;
    t->to_terminal_ours_for_output = child_terminal_ours_for_output;
-  t->to_terminal_save_ours = child_terminal_save_ours;
    t->to_terminal_ours = child_terminal_ours;
    t->to_terminal_info = child_terminal_info;
    t->to_post_startup_inferior = inf_child_post_startup_inferior;
diff --git a/gdb/inferior.h b/gdb/inferior.h
index afc29e2..58557a4 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -110,8 +110,6 @@ extern void child_terminal_info (struct target_ops *self, const char *, int);

  extern void term_info (char *, int);

-extern void child_terminal_save_ours (struct target_ops *self);
-
  extern void child_terminal_ours (struct target_ops *self);

  extern void child_terminal_ours_for_output (struct target_ops *self);
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 4b105d1..8902174 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -241,7 +241,7 @@ child_terminal_init_with_pgrp (int pgrp)
     and gdb must be able to restore it correctly.  */

  void
-child_terminal_save_ours (struct target_ops *self)
+gdb_save_tty_state (void)
  {
    if (gdb_has_a_terminal ())
      {
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 843a954..fe989ff 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -813,28 +813,6 @@ debug_terminal_ours (struct target_ops *self)
  }

  static void
-delegate_terminal_save_ours (struct target_ops *self)
-{
-  self = self->beneath;
-  self->to_terminal_save_ours (self);
-}
-
-static void
-tdefault_terminal_save_ours (struct target_ops *self)
-{
-}
-
-static void
-debug_terminal_save_ours (struct target_ops *self)
-{
-  fprintf_unfiltered (gdb_stdlog, "-> %s->to_terminal_save_ours (...)\n", debug_target.to_shortname);
-  debug_target.to_terminal_save_ours (&debug_target);
-  fprintf_unfiltered (gdb_stdlog, "<- %s->to_terminal_save_ours (", debug_target.to_shortname);
-  target_debug_print_struct_target_ops_p (&debug_target);
-  fputs_unfiltered (")\n", gdb_stdlog);
-}
-
-static void
  delegate_terminal_info (struct target_ops *self, const char *arg1, int arg2)
  {
    self = self->beneath;
@@ -3814,8 +3792,6 @@ install_delegators (struct target_ops *ops)
      ops->to_terminal_ours_for_output = delegate_terminal_ours_for_output;
    if (ops->to_terminal_ours == NULL)
      ops->to_terminal_ours = delegate_terminal_ours;
-  if (ops->to_terminal_save_ours == NULL)
-    ops->to_terminal_save_ours = delegate_terminal_save_ours;
    if (ops->to_terminal_info == NULL)
      ops->to_terminal_info = delegate_terminal_info;
    if (ops->to_kill == NULL)
@@ -4068,7 +4044,6 @@ install_dummy_methods (struct target_ops *ops)
    ops->to_terminal_inferior = tdefault_terminal_inferior;
    ops->to_terminal_ours_for_output = tdefault_terminal_ours_for_output;
    ops->to_terminal_ours = tdefault_terminal_ours;
-  ops->to_terminal_save_ours = tdefault_terminal_save_ours;
    ops->to_terminal_info = default_terminal_info;
    ops->to_kill = tdefault_kill;
    ops->to_load = tdefault_load;
@@ -4212,7 +4187,6 @@ init_debug_target (struct target_ops *ops)
    ops->to_terminal_inferior = debug_terminal_inferior;
    ops->to_terminal_ours_for_output = debug_terminal_ours_for_output;
    ops->to_terminal_ours = debug_terminal_ours;
-  ops->to_terminal_save_ours = debug_terminal_save_ours;
    ops->to_terminal_info = debug_terminal_info;
    ops->to_kill = debug_kill;
    ops->to_load = debug_load;
diff --git a/gdb/target.h b/gdb/target.h
index 4d91b6b..85763ba 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -506,8 +506,6 @@ struct target_ops
        TARGET_DEFAULT_IGNORE ();
      void (*to_terminal_ours) (struct target_ops *)
        TARGET_DEFAULT_IGNORE ();
-    void (*to_terminal_save_ours) (struct target_ops *)
-      TARGET_DEFAULT_IGNORE ();
      void (*to_terminal_info) (struct target_ops *, const char *, int)
        TARGET_DEFAULT_FUNC (default_terminal_info);
      void (*to_kill) (struct target_ops *)
@@ -1417,14 +1415,6 @@ extern void target_terminal_inferior (void);

  extern int target_supports_terminal_ours (void);

-/* Save our terminal settings.
-   This is called from TUI after entering or leaving the curses
-   mode.  Since curses modifies our terminal this call is here
-   to take this change into account.  */
-
-#define target_terminal_save_ours() \
-     (*current_target.to_terminal_save_ours) (&current_target)
-
  /* Print useful information about our terminal status, if such a thing
     exists.  */

diff --git a/gdb/terminal.h b/gdb/terminal.h
index cc10242..433aa7d 100644
--- a/gdb/terminal.h
+++ b/gdb/terminal.h
@@ -101,6 +101,8 @@ extern void initialize_stdin_serial (void);

  extern int gdb_has_a_terminal (void);

+extern void gdb_save_tty_state (void);
+
  /* Set the process group of the caller to its own pid, or do nothing
     if we lack job control.  */
  extern int gdb_setpgid (void);
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 7add8ba..a02c855 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -37,6 +37,7 @@
  #include "inferior.h"
  #include "symtab.h"
  #include "source.h"
+#include "terminal.h"

  #include <ctype.h>
  #include <signal.h>
@@ -425,7 +426,7 @@ tui_enable (void)
    tui_refresh_all_win ();

    /* Update gdb's knowledge of its terminal.  */
-  target_terminal_save_ours ();
+  gdb_save_tty_state ();
    tui_update_gdb_sizes ();
  }

@@ -455,7 +456,7 @@ tui_disable (void)
    tui_setup_io (0);

    /* Update gdb's knowledge of its terminal.  */
-  target_terminal_save_ours ();
+  gdb_save_tty_state ();

    tui_active = 0;
    tui_update_gdb_sizes ();
-- 
2.1.0

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Sergio Durigan Junior]

On Wednesday, August 27 2014, Patrick Palka wrote:

> Note that I don't have commit access so I would appreciate if someone
> would
> commit this on my behalf.

Thanks a lot for the patch :-).  TUI needs some love, and it is always
nice to see patches for it!

I just noticed you don't have copyright assignment on file, right?
Since your patch is not trivial, I believe you will have to sign the
copyright assignment to FSF before we can push the patch for you.  I am
sending you the form off-list; it is a simple procedure, but it can take
1 or 2 months depending on your location.

If you do have copyright assignment on file, please let me know and I
can push the patch right away.

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Patrick Palka]

On Wed, Aug 27, 2014 at 12:15 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Wednesday, August 27 2014, Patrick Palka wrote:
>
>> Note that I don't have commit access so I would appreciate if someone
>> would
>> commit this on my behalf.
>
> Thanks a lot for the patch :-).  TUI needs some love, and it is always
> nice to see patches for it!
>
> I just noticed you don't have copyright assignment on file, right?
> Since your patch is not trivial, I believe you will have to sign the
> copyright assignment to FSF before we can push the patch for you.  I am
> sending you the form off-list; it is a simple procedure, but it can take
> 1 or 2 months depending on your location.
>
> If you do have copyright assignment on file, please let me know and I
> can push the patch right away.

Thanks.  I should have a copyright assignment on file for binutils and
for GDB.  I completed this process back in January.

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Sergio Durigan Junior]

On Wednesday, August 27 2014, Patrick Palka wrote:

> Thanks.  I should have a copyright assignment on file for binutils and
> for GDB.  I completed this process back in January.

Oh, nice, I see your name in the git log now.  Sorry about that, I was
looking at the gdb/MAINTAINERS file.

I will push this patch for you, but you can also request an account at
<https://sourceware.org/cgi-bin/pdw/ps_form.cgi> in order to gain push
access to the repository.  If you choose to do that, you can also update
the gdb/MAINTAINERS file with your info.

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Sergio Durigan Junior]

On Wednesday, August 27 2014, Patrick Palka wrote:

> Done, thanks for reviewing. I moved the declaration to terminal.h and
> updated the ChangeLog accordingly.  Meanwhile I noticed I omitted
> ChangeLog entries for the changes in tui/tui.c, which I now added.
> Note that I don't have commit access so I would appreciate if someone
> would
> commit this on my behalf.

It seems your MUA has messed with your patch, and it doesn't apply
cleanly for me.  Could you try sending it attached?

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Patrick Palka]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 763 bytes --]

On Wed, 27 Aug 2014, Sergio Durigan Junior wrote:

> On Wednesday, August 27 2014, Patrick Palka wrote:
>
>> Done, thanks for reviewing. I moved the declaration to terminal.h and
>> updated the ChangeLog accordingly.  Meanwhile I noticed I omitted
>> ChangeLog entries for the changes in tui/tui.c, which I now added.
>> Note that I don't have commit access so I would appreciate if someone
>> would
>> commit this on my behalf.
>
> It seems your MUA has messed with your patch, and it doesn't apply
> cleanly for me.  Could you try sending it attached?
>
> Thanks,
>
> -- 
> Sergio
> GPG key ID: 0x65FC5E36
> Please send encrypted e-mail if possible
> http://sergiodj.net/
>

I was afraid that that might happen :/ Sorry about that.  Patch is 
attached.

Patrick

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=0001-Fix-terminal-state-corruption-when-starting-a-progra.patch, Size: 8671 bytes --]

From d510c0efc4d2359e069d29d9754754564fbce47b Mon Sep 17 00:00:00 2001
From: Patrick Palka <patrick@parcs.ath.cx>
Date: Mon, 25 Aug 2014 10:40:32 -0400
Subject: [PATCH] Fix terminal state corruption when starting a program from
 within TUI

The TUI terminal state becomes corrupted (e.g. key sequences such as
Alt_F and Alt_B no longer work) when one attaches to an inferior process
(via "run" or "attach") from within TUI.  This terminal corruption
remains until you switch out of TUI mode.

This happens because the terminal state is not properly saved when
switching to and out from TUI mode.  Although the functions tui_enable()
and tui_disable() both call the function target_terminal_save_ours() to
save the terminal state, this function is a no-op unless GDB has already
attached to an inferior process.  This is because only the "native"
target has a useful implementation of target_terminal_save_ours()
(namely child_terminal_save_ours()) and we only have the "native" target
in our target vector if GDB has already attached to an inferior process.

So without an inferior process, switching to and from TUI mode does not
actually save the terminal state.  Therefore when you attach to an
inferior process from within TUI mode, the proper terminal state is not
restored (after swapping from the inferior's terminal back to the GDB
terminal).

To fix this we just have to ensure that the terminal state is always
being properly saved when switching from and to TUI mode.  To achieve
this, this patch removes the polymorphic function
target_terminal_save_ours() and replaces it with a regular function
gdb_save_tty_state() that always saves the terminal state.

Tested on x86_64-unknown-linux-gnu by running "make check", no new
regressions.

gdb/ChangeLog:
	* target.h (struct target_ops::to_terminal_save_ours): Remove
	declaration.
	(target_terminal_save_ours): Remove macro.
	* target-delegates.c: Regenerate.
	* inf-child.c (inf_child_target): Don't set the nonexistent
	field to_terminal_save_ours.
	* inferior.h (child_terminal_save_ours): Remove declaration.
	* terminal.h (gdb_save_tty_state): New declaration.
	* inflow.c (child_terminal_save_ours): Rename to ...
	(gdb_save_tty_state): ... this.
	* tui/tui.c: Include terminal.h.
	(tui_enable): Use gdb_save_tty_state instead of
	target_terminal_save_ours.
	(tui_disable): Likewise.
---
 gdb/inf-child.c        |  1 -
 gdb/inferior.h         |  2 --
 gdb/inflow.c           |  2 +-
 gdb/target-delegates.c | 26 --------------------------
 gdb/target.h           | 10 ----------
 gdb/terminal.h         |  2 ++
 gdb/tui/tui.c          |  5 +++--
 7 files changed, 6 insertions(+), 42 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 6d95e5e..9867ee6 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -481,7 +481,6 @@ inf_child_target (void)
   t->to_terminal_init = child_terminal_init;
   t->to_terminal_inferior = child_terminal_inferior;
   t->to_terminal_ours_for_output = child_terminal_ours_for_output;
-  t->to_terminal_save_ours = child_terminal_save_ours;
   t->to_terminal_ours = child_terminal_ours;
   t->to_terminal_info = child_terminal_info;
   t->to_post_startup_inferior = inf_child_post_startup_inferior;
diff --git a/gdb/inferior.h b/gdb/inferior.h
index afc29e2..58557a4 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -110,8 +110,6 @@ extern void child_terminal_info (struct target_ops *self, const char *, int);
 
 extern void term_info (char *, int);
 
-extern void child_terminal_save_ours (struct target_ops *self);
-
 extern void child_terminal_ours (struct target_ops *self);
 
 extern void child_terminal_ours_for_output (struct target_ops *self);
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 4b105d1..8902174 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -241,7 +241,7 @@ child_terminal_init_with_pgrp (int pgrp)
    and gdb must be able to restore it correctly.  */
 
 void
-child_terminal_save_ours (struct target_ops *self)
+gdb_save_tty_state (void)
 {
   if (gdb_has_a_terminal ())
     {
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 843a954..fe989ff 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -813,28 +813,6 @@ debug_terminal_ours (struct target_ops *self)
 }
 
 static void
-delegate_terminal_save_ours (struct target_ops *self)
-{
-  self = self->beneath;
-  self->to_terminal_save_ours (self);
-}
-
-static void
-tdefault_terminal_save_ours (struct target_ops *self)
-{
-}
-
-static void
-debug_terminal_save_ours (struct target_ops *self)
-{
-  fprintf_unfiltered (gdb_stdlog, "-> %s->to_terminal_save_ours (...)\n", debug_target.to_shortname);
-  debug_target.to_terminal_save_ours (&debug_target);
-  fprintf_unfiltered (gdb_stdlog, "<- %s->to_terminal_save_ours (", debug_target.to_shortname);
-  target_debug_print_struct_target_ops_p (&debug_target);
-  fputs_unfiltered (")\n", gdb_stdlog);
-}
-
-static void
 delegate_terminal_info (struct target_ops *self, const char *arg1, int arg2)
 {
   self = self->beneath;
@@ -3814,8 +3792,6 @@ install_delegators (struct target_ops *ops)
     ops->to_terminal_ours_for_output = delegate_terminal_ours_for_output;
   if (ops->to_terminal_ours == NULL)
     ops->to_terminal_ours = delegate_terminal_ours;
-  if (ops->to_terminal_save_ours == NULL)
-    ops->to_terminal_save_ours = delegate_terminal_save_ours;
   if (ops->to_terminal_info == NULL)
     ops->to_terminal_info = delegate_terminal_info;
   if (ops->to_kill == NULL)
@@ -4068,7 +4044,6 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_terminal_inferior = tdefault_terminal_inferior;
   ops->to_terminal_ours_for_output = tdefault_terminal_ours_for_output;
   ops->to_terminal_ours = tdefault_terminal_ours;
-  ops->to_terminal_save_ours = tdefault_terminal_save_ours;
   ops->to_terminal_info = default_terminal_info;
   ops->to_kill = tdefault_kill;
   ops->to_load = tdefault_load;
@@ -4212,7 +4187,6 @@ init_debug_target (struct target_ops *ops)
   ops->to_terminal_inferior = debug_terminal_inferior;
   ops->to_terminal_ours_for_output = debug_terminal_ours_for_output;
   ops->to_terminal_ours = debug_terminal_ours;
-  ops->to_terminal_save_ours = debug_terminal_save_ours;
   ops->to_terminal_info = debug_terminal_info;
   ops->to_kill = debug_kill;
   ops->to_load = debug_load;
diff --git a/gdb/target.h b/gdb/target.h
index 4d91b6b..85763ba 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -506,8 +506,6 @@ struct target_ops
       TARGET_DEFAULT_IGNORE ();
     void (*to_terminal_ours) (struct target_ops *)
       TARGET_DEFAULT_IGNORE ();
-    void (*to_terminal_save_ours) (struct target_ops *)
-      TARGET_DEFAULT_IGNORE ();
     void (*to_terminal_info) (struct target_ops *, const char *, int)
       TARGET_DEFAULT_FUNC (default_terminal_info);
     void (*to_kill) (struct target_ops *)
@@ -1417,14 +1415,6 @@ extern void target_terminal_inferior (void);
 
 extern int target_supports_terminal_ours (void);
 
-/* Save our terminal settings.
-   This is called from TUI after entering or leaving the curses
-   mode.  Since curses modifies our terminal this call is here
-   to take this change into account.  */
-
-#define target_terminal_save_ours() \
-     (*current_target.to_terminal_save_ours) (&current_target)
-
 /* Print useful information about our terminal status, if such a thing
    exists.  */
 
diff --git a/gdb/terminal.h b/gdb/terminal.h
index cc10242..433aa7d 100644
--- a/gdb/terminal.h
+++ b/gdb/terminal.h
@@ -101,6 +101,8 @@ extern void initialize_stdin_serial (void);
 
 extern int gdb_has_a_terminal (void);
 
+extern void gdb_save_tty_state (void);
+
 /* Set the process group of the caller to its own pid, or do nothing
    if we lack job control.  */
 extern int gdb_setpgid (void);
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 7add8ba..a02c855 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -37,6 +37,7 @@
 #include "inferior.h"
 #include "symtab.h"
 #include "source.h"
+#include "terminal.h"
 
 #include <ctype.h>
 #include <signal.h>
@@ -425,7 +426,7 @@ tui_enable (void)
   tui_refresh_all_win ();
 
   /* Update gdb's knowledge of its terminal.  */
-  target_terminal_save_ours ();
+  gdb_save_tty_state ();
   tui_update_gdb_sizes ();
 }
 
@@ -455,7 +456,7 @@ tui_disable (void)
   tui_setup_io (0);
 
   /* Update gdb's knowledge of its terminal.  */
-  target_terminal_save_ours ();
+  gdb_save_tty_state ();
 
   tui_active = 0;
   tui_update_gdb_sizes ();
-- 
2.1.0

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Sergio Durigan Junior]

On Wednesday, August 27 2014, Patrick Palka wrote:

> I was afraid that that might happen :/ Sorry about that.  Patch is
> attached.

Thanks, pushed:

  <https://sourceware.org/ml/gdb-cvs/2014-08/msg00078.html>

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Patrick Palka]

On Wed, 27 Aug 2014, Sergio Durigan Junior wrote:

> On Wednesday, August 27 2014, Patrick Palka wrote:
>
>> I was afraid that that might happen :/ Sorry about that.  Patch is
>> attached.
>
> Thanks, pushed:
>
>  <https://sourceware.org/ml/gdb-cvs/2014-08/msg00078.html>

Thank you!  Though it seems that the ChangeLog entry is missing.

>
> -- 
> Sergio
> GPG key ID: 0x65FC5E36
> Please send encrypted e-mail if possible
> http://sergiodj.net/
>

Re: [PATCH] Fix terminal state corruption when starting a program from within TUI

[Sergio Durigan Junior]

On Wednesday, August 27 2014, Patrick Palka wrote:

> Thank you!  Though it seems that the ChangeLog entry is missing.

You're right, my bad.  Checked-in:

  <https://sourceware.org/ml/gdb-cvs/2014-08/msg00079.html>

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/