From patchwork Tue Jul 24 02:16:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 28580 Received: (qmail 116472 invoked by alias); 24 Jul 2018 02:16:57 -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 116269 invoked by uid 89); 24 Jul 2018 02:16:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=Compare, Continue X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 24 Jul 2018 02:16:53 +0000 Received: from smtp.ebox.ca (smtp.electronicbox.net [96.127.255.82]) by barracuda.ebox.ca with ESMTP id T9lzpQWrG4FqO21u (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 23 Jul 2018 22:16:45 -0400 (EDT) Received: from simark.lan (unknown [192.222.164.54]) by smtp.ebox.ca (Postfix) with ESMTP id B3DC4441B21; Mon, 23 Jul 2018 22:16:44 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH] Fix restoring of inferior terminal settings Date: Mon, 23 Jul 2018 22:16:43 -0400 Message-Id: <20180724021643.16041-1-simon.marchi@polymtl.ca> X-IsSubscribed: yes I noticed that the child_terminal_save_inferior function was not used since the commit f6ac5f3d63e0 ("Convert struct target_ops to C++"). I was able to make a little test program to illustrate the problem (see test case). I think we're just missing the override of the terminal_save_inferior method in inf_child_target (along with the other terminal-related methods). Instead of creating a new test, I thought that gdb.base/term.exp was a good candidate for testing that gdb restores properly the inferior's terminal settings. gdb/ChangeLog: * inf-child.h (inf_child_target) : New. * inf-child.c (inf_child_target::terminal_save_inferior): New. gdb/testsuite/ChangeLog: * gdb.base/term.exp: Compare terminal settings with values from the inferior. * gdb.base/term.c: Get and set terminal settings. --- gdb/inf-child.c | 6 +++ gdb/inf-child.h | 1 + gdb/testsuite/gdb.base/term.c | 22 +++++++++ gdb/testsuite/gdb.base/term.exp | 81 ++++++++++++++++++++++++++++----- 4 files changed, 99 insertions(+), 11 deletions(-) diff --git a/gdb/inf-child.c b/gdb/inf-child.c index 2f5babebce7f..44aa2f66fbfe 100644 --- a/gdb/inf-child.c +++ b/gdb/inf-child.c @@ -113,6 +113,12 @@ inf_child_target::terminal_inferior () child_terminal_inferior (this); } +void +inf_child_target::terminal_save_inferior () +{ + child_terminal_save_inferior (this); +} + void inf_child_target::terminal_ours_for_output () { diff --git a/gdb/inf-child.h b/gdb/inf-child.h index 6316f3062da1..98969bc5fa31 100644 --- a/gdb/inf-child.h +++ b/gdb/inf-child.h @@ -46,6 +46,7 @@ public: bool supports_terminal_ours () override; void terminal_init () override; void terminal_inferior () override; + void terminal_save_inferior () override; void terminal_ours_for_output () override; void terminal_ours () override; void terminal_info (const char *, int) override; diff --git a/gdb/testsuite/gdb.base/term.c b/gdb/testsuite/gdb.base/term.c index 26d2c006ee4d..40ca62bb98cb 100644 --- a/gdb/testsuite/gdb.base/term.c +++ b/gdb/testsuite/gdb.base/term.c @@ -15,7 +15,29 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +#include +#include + +static struct termios t; + +static void +break_here () +{ +} + int main () { + tcgetattr (0, &t); + break_here (); + + /* Disable ECHO. */ + t.c_lflag &= ~ECHO; + tcsetattr (0, TCSANOW, &t); + tcgetattr (0, &t); + break_here (); + + tcgetattr (0, &t); + break_here (); + return 0; } diff --git a/gdb/testsuite/gdb.base/term.exp b/gdb/testsuite/gdb.base/term.exp index 031a50b703b5..77307894e040 100644 --- a/gdb/testsuite/gdb.base/term.exp +++ b/gdb/testsuite/gdb.base/term.exp @@ -13,6 +13,9 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +# Test that GDB saves and restores terminal settings correctly. Also check +# the output of the "info terminal" command. + if { [prepare_for_testing "failed to prepare" term term.c] } { return -1 } @@ -20,28 +23,84 @@ if { [prepare_for_testing "failed to prepare" term term.c] } { # Once before running the program. gdb_test "info terminal" \ "No saved terminal information.*" \ - "test info terminal" + "test info terminal pre-execution" -if ![runto_main] then { - fail "can't run to main" +if ![runto break_here] then { + fail "can't run to break_here" return 0 } -# Once while the program is running and stopped. +# Read the inferior's terminal settings, saved in the T global variable. + +proc read_term_settings_from_inferior {} { + set termios(c_iflag) [get_hexadecimal_valueof "t.c_iflag" oops] + set termios(c_oflag) [get_hexadecimal_valueof "t.c_oflag" oops] + set termios(c_cflag) [get_hexadecimal_valueof "t.c_cflag" oops] + set termios(c_lflag) [get_hexadecimal_valueof "t.c_lflag" oops] + + return [array get termios] +} + +# Validate that gdb's notion of the inferior's terminal settings are consistent +# with the values read from the inferior. + +proc compare_gdb_and_inferior_settings { t } { + global decimal + array set termios $t + + gdb_test "info terminal" \ + [multi_line "Inferior's terminal status .currently saved by GDB.:" \ + "File descriptor flags = .*" \ + "Process group = $decimal" \ + "c_iflag = ${termios(c_iflag)}, c_oflag = ${termios(c_oflag)}," \ + "c_cflag = ${termios(c_cflag)}, c_lflag = ${termios(c_lflag)}.*" ] +} -# While only native targets save terminal status, we still test -# everywhere to make sure that the command doesn't misbehave. if {[target_info gdb_protocol] == ""} { - set term_re "Inferior's terminal status .currently saved by GDB.:.*" + # Record the initial terminal settings. Verify that GDB's version of the + # inferior's terminal settings is right. + with_test_prefix "initial" { + array set termios1 [read_term_settings_from_inferior] + compare_gdb_and_inferior_settings [array get termios1] + } + + # Continue until after the inferior removes ECHO from its terminal settings. + gdb_continue_to_breakpoint "continue until after tcsetattr" + + # After the inferior has changed its terminal settings, check that GDB's + # saved version reflects the new settings correctly. + with_test_prefix "post tcsetattr" { + array set termios2 [read_term_settings_from_inferior] + compare_gdb_and_inferior_settings [array get termios2] + + # Make sure that the current settings are different than the initial + # settings... otherwise this test is meaningless. + gdb_assert {${termios1(c_lflag)} != ${termios2(c_lflag)}} + } + + # Continue again... + gdb_continue_to_breakpoint "continue again" + + # ... and verify again, to validate that when resuming, GDB restored the + # inferior's terminal settings correctly. + with_test_prefix "after last resume" { + array set termios3 [read_term_settings_from_inferior] + compare_gdb_and_inferior_settings [array get termios3] + gdb_assert {${termios2(c_iflag)} == ${termios3(c_iflag)}} + gdb_assert {${termios2(c_oflag)} == ${termios3(c_oflag)}} + gdb_assert {${termios2(c_cflag)} == ${termios3(c_cflag)}} + gdb_assert {${termios2(c_lflag)} == ${termios3(c_lflag)}} + } } else { - set term_re "No saved terminal information\\." + # While only native targets save terminal status, we still test + # that the command doesn't misbehave. + gdb_test "info terminal" "No saved terminal information\\." "info terminal at breakpoint" } -gdb_test "info terminal" $term_re "info terminal at breakpoint" - +delete_breakpoints gdb_continue_to_end # One last time after the program having exited. gdb_test "info terminal" \ "No saved terminal information.*" \ - "test info terminal #2" + "test info terminal post-execution"