From patchwork Sat Nov 22 20:39:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 3853 Received: (qmail 25413 invoked by alias); 22 Nov 2014 20:39:20 -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 25403 invoked by uid 89); 22 Nov 2014 20:39:19 -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; Sat, 22 Nov 2014 20:39:18 +0000 Received: by mail-qa0-f41.google.com with SMTP id f12so4938869qad.0 for ; Sat, 22 Nov 2014 12:39:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=LvRfJJ0mMy+3qyRTNQcZ71M7qIXoHYMLoGvQrICWR5c=; b=dnAs1Aa5Pas6n5oggMQt7LhARFHzY+7wLMDA486THiXIOsgElgZV3v3HJ2rYWRxxUK s6j1qE2j9freLOAp3lWw1kLqnb76q2/7xUMo4rhfSpF8PrFU+9TFzkuLUTZmiNW95+Pk tHQe3NHmZ6j0FBUMiVlWBnzMZ3cEadHbyfQYndPveUV2+TwlpB7eGBAKno38oXx8fLd/ nbn1w7LJr0y+Xr8xJKardXsiufLuAjQ9KegFcRtShUkBnk+wsViKl+UGbpBXVRbz/pUN xhetzLDx4SezYpeg/ox9NZS8fecQhlP96OlHBD3hf8RctUvIzlyxTIo9uHnSWLw9s4vH dkow== X-Gm-Message-State: ALoCoQlatP0cAGyA1TKanb4MBFZ4XZOmbGCUjtbK2d6QQX7GLXqH5CYPucnqfmKbi25DN3Zk9Df/ X-Received: by 10.140.22.201 with SMTP id 67mr16870770qgn.16.1416688755066; Sat, 22 Nov 2014 12:39:15 -0800 (PST) Received: from localhost.localdomain (ool-4353af5c.dyn.optonline.net. [67.83.175.92]) by mx.google.com with ESMTPSA id u95sm7907093qgu.47.2014.11.22.12.39.13 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 22 Nov 2014 12:39:14 -0800 (PST) From: Patrick Palka To: gdb-patches@sourceware.org Cc: Patrick Palka Subject: [PATCH] [RFC] Don't propagate our current terminal state to the inferior Date: Sat, 22 Nov 2014 15:39:08 -0500 Message-Id: <1416688748-20448-1-git-send-email-patrick@parcs.ath.cx> X-IsSubscribed: yes Currently when we start an inferior we have the inferior inherit our terminal state. Under TUI, our terminal is highly modified by ncurses and readline. So when starting an inferior under TUI, the inferior will have a highly modified terminal state which will interfere with standard I/O. For example, $ gdb gdb (gdb) break main (gdb) run (gdb) print puts ("a\nb") a b $1 = 4 (gdb) [enter TUI mode] (gdb) run (gdb) [exit TUI mode] (gdb) print puts ("a\nb") a b $2 = 4 (gdb) print puts ("a\r\nb\r") a b $3 = 6 As you can see, when we start the inferior under the regular interface, puts() prints the text properly. But when we start the inferior under TUI, puts() does not print the text properly. This is because when we start the inferior under TUI it inherits our current terminal state which has been modified by ncurses to, among other things, require an explicit \r\n to print a new line. As a result the inferior performs standard I/O in an unexpected way. Because of this discrepancy, it doesn't seem like a good idea to have the inferior inherit our _current_ terminal state for it may have been modified by readline and/or ncurses. Instead, we should have the inferior inherit a pristine snapshot of our terminal state taken before readline or ncurses have had a chance to alter it. This enables the inferior to run in a more accurate way, more closely mimicking its behavior had the program run standalone. And it fixes the above mentioned issue. I wonder, does this change make sense? What do others think? Tested on x86_64-unknown-linux-gnu. * terminal.h (set_initial_inferior_ttystate): Declare. * inflow.c (initial_inferior_ttystate): New static variable. (set_initial_inferior_ttystate): New setter. (child_terminal_init_with_pgrp): Copy initial_inferior_ttystate instead of our current terminal state. * top.c (gdb_init): Call set_initial_inferior_ttystate. --- gdb/inflow.c | 13 ++++++++++++- gdb/terminal.h | 2 ++ gdb/top.c | 4 ++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/gdb/inflow.c b/gdb/inflow.c index 8902174..7b432ad 100644 --- a/gdb/inflow.c +++ b/gdb/inflow.c @@ -79,6 +79,10 @@ struct terminal_info unimportant. */ static struct terminal_info our_terminal_info; +/* The initial tty state given to each new inferior. It is a snapshot of our + own tty state taken during initialization of GDB. */ +static serial_ttystate initial_inferior_ttystate; + static struct terminal_info *get_inflow_inferior_data (struct inferior *); #ifdef PROCESS_GROUP_TYPE @@ -156,6 +160,13 @@ show_interactive_mode (struct ui_file *file, int from_tty, fprintf_filtered (file, "Debugger's interactive mode is %s.\n", value); } +/* Set the initial tty state that is to be inherited by new inferiors. */ +void +set_initial_inferior_ttystate (void) +{ + initial_inferior_ttystate = serial_get_tty_state (stdin_serial); +} + /* Does GDB have a terminal (on stdin)? */ int gdb_has_a_terminal (void) @@ -227,7 +238,7 @@ child_terminal_init_with_pgrp (int pgrp) { xfree (tinfo->ttystate); tinfo->ttystate = serial_copy_tty_state (stdin_serial, - our_terminal_info.ttystate); + initial_inferior_ttystate); /* Make sure that next time we call terminal_inferior (which will be before the program runs, as it needs to be), we install the new diff --git a/gdb/terminal.h b/gdb/terminal.h index 433aa7d..186bce2 100644 --- a/gdb/terminal.h +++ b/gdb/terminal.h @@ -103,6 +103,8 @@ extern int gdb_has_a_terminal (void); extern void gdb_save_tty_state (void); +extern void set_initial_inferior_ttystate (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/top.c b/gdb/top.c index 83d858a..c4b5c2c 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1886,6 +1886,10 @@ gdb_init (char *argv0) initialize_stdin_serial (); + /* Take a snapshot of our tty state before readline/ncurses have had a chance + to alter it. */ + set_initial_inferior_ttystate (); + async_init_signals (); /* We need a default language for parsing expressions, so simple