From patchwork Wed Aug 27 16:42:56 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 2555 Received: (qmail 4298 invoked by alias); 27 Aug 2014 16:43:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4267 invoked by uid 89); 27 Aug 2014 16:43:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-qa0-f41.google.com Received: from mail-qa0-f41.google.com (HELO mail-qa0-f41.google.com) (209.85.216.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 27 Aug 2014 16:43:01 +0000 Received: by mail-qa0-f41.google.com with SMTP id j7so483032qaq.28 for ; Wed, 27 Aug 2014 09:42:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type; bh=moErjGTJ0TwamcbuVVZ+anhuOy8exZU5hqOUYZ0iAfg=; b=QLcXzl73uPeq05RgPcE6k1UlMIHNYt1s/9tuTuKiVcJBuTTPfsJFxYInJndv2w7UZX ndhgX6eQ7FeDRHRL6WSGnvwBUGDm457aFYS7FZfT4Aq3i43m3oBW2/wFOIDY2DqoQCKn 1ibcah8enBkMLZ1/nDlhlmtgLLPnptxG79pc5RKqIR3R9VqjD3i8p8aeJI/91AQ2xe5K X8v4RK4Q51vXxNHzt0I6x5i2/r6X2xTu7WUVPojAnvaC3gzp/eGekBeSfm4aBXBDkFMh VehGvYm6d/LfYX6OGSQ/p3pzi7aAvN09MyWJhO6N0+PgRwCoZ6CNMieUVbCvmDlQaQxP kJ+Q== X-Gm-Message-State: ALoCoQnxD7fO9a2AJKW/twD+OwD+X1w10p79ZD4o36szrnqHrPPeKTWn33rj9g4+jKjW98ClUAFw X-Received: by 10.224.38.10 with SMTP id z10mr60364985qad.52.1409157778729; Wed, 27 Aug 2014 09:42:58 -0700 (PDT) Received: from [192.168.1.130] (ool-4353af5c.dyn.optonline.net. [67.83.175.92]) by mx.google.com with ESMTPSA id w6sm2464896qas.26.2014.08.27.09.42.57 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 27 Aug 2014 09:42:57 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Wed, 27 Aug 2014 12:42:56 -0400 (EDT) To: Sergio Durigan Junior cc: Patrick Palka , Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH] Fix terminal state corruption when starting a program from within TUI In-Reply-To: <878um951oc.fsf@redhat.com> Message-ID: References: <1408990671-6023-1-git-send-email-patrick@parcs.ath.cx> <53FDC401.30509@redhat.com> <878um951oc.fsf@redhat.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 X-IsSubscribed: yes 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 From d510c0efc4d2359e069d29d9754754564fbce47b Mon Sep 17 00:00:00 2001 From: Patrick Palka 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) (¤t_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 #include @@ -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