Fix PR20494 - User input stops being echoed in CLI

This patch fixes a problem that problem triggers if you start an
inferior, e.g., with the "start" command, in a UI created with the
new-ui command, and then run a foreground execution command in the
main UI.  Once the program stops for the latter command, typing in the
main UI no longer echoes back to the user.

The problem revolves around this:

- gdb_has_a_terminal computes its result lazily, on first call.

  that is what saves gdb's initial main UI terminal state (the UI
  associated with stdin):

          our_terminal_info.ttystate = serial_get_tty_state (stdin_serial);

  This is the state that target_terminal_ours() restores.

- In this scenario, the gdb_has_a_terminal function happens to be
  first ever called from within the target_terminal_init call in
  startup_inferior:

      (top-gdb) bt
      #0  gdb_has_a_terminal () at src/gdb/inflow.c:157
      #1  0x000000000079db22 in child_terminal_init_with_pgrp () at src/gdb/inflow.c:217
       [...]
      #4  0x000000000065bacb in target_terminal_init () at src/gdb/target.c:456
      #5  0x00000000004676d2 in startup_inferior () at src/gdb/fork-child.c:531
       [...]
      #7  0x000000000046b168 in linux_nat_create_inferior () at src/gdb/linux-nat.c:1112
       [...]
      #9  0x00000000005f20c9 in start_command (args=0x0, from_tty=1) at src/gdb/infcmd.c:657

If the command to start the inferior is issued on the main UI, then
readline will have deprepped the terminal when we reach the above, and
the problem doesn't appear.

If however the command is issued on a non-main UI, then when we reach
that gdb_has_a_terminal call, the main UI's terminal state is still
set to whatever readline has sets it to in rl_prep_terminal, which
happens to have echo disabled.  Later, when the following synchronous
execution command finishes, we'll call target_terminal_ours to restore
gdb's the main UI's terminal settings, and that restores the terminal
state with echo disabled...

Conceptually, the fix is to move the gdb_has_a_terminal call earlier,
to someplace during GDB initialization, before readline/ncurses have
had a chance to change terminal settings.  Turns out that
"set_initial_gdb_ttystate" is exactly such a place.

I say conceptually, because the fix actually inlines the
gdb_has_a_terminal part that saves the terminal state in
set_initial_gdb_ttystate and then simplifies gdb_has_a_terminal, since
there's no point in making gdb_has_a_terminal do lazy computation.

gdb/ChangeLog:
2016-08-23  Pedro Alves  <palves@redhat.com>

	PR gdb/20494
	* inflow.c (our_terminal_info, initial_gdb_ttystate): Update
	comments.
	(enum gdb_has_a_terminal_flag_enum, gdb_has_a_terminal_flag):
	Delete.
	(set_initial_gdb_ttystate): Record our_terminal_info here too,
	instead of ...
	(gdb_has_a_terminal): ... here.  Reimplement in terms of
	initial_gdb_ttystate.  Make static.
	* terminal.h (gdb_has_a_terminal): Delete declaration.
	(set_initial_gdb_ttystate): Add comment.
	* top.c (show_interactive_mode): Use input_interactive_p instead
	of gdb_has_a_terminal.

gdb/testsuite/ChangeLog:
2016-08-23  Pedro Alves  <palves@redhat.com>

	PR gdb/20494
	* gdb.base/new-ui-echo.c: New file.
	* gdb.base/new-ui-echo.exp: New file.
This commit is contained in:
Pedro Alves 2016-08-23 16:03:28 +01:00
parent 38334d6de4
commit d9de1fe3d5
7 changed files with 206 additions and 47 deletions

View file

@ -1,3 +1,19 @@
2016-08-23 Pedro Alves <palves@redhat.com>
PR gdb/20494
* inflow.c (our_terminal_info, initial_gdb_ttystate): Update
comments.
(enum gdb_has_a_terminal_flag_enum, gdb_has_a_terminal_flag):
Delete.
(set_initial_gdb_ttystate): Record our_terminal_info here too,
instead of ...
(gdb_has_a_terminal): ... here. Reimplement in terms of
initial_gdb_ttystate. Make static.
* terminal.h (gdb_has_a_terminal): Delete declaration.
(set_initial_gdb_ttystate): Add comment.
* top.c (show_interactive_mode): Use input_interactive_p instead
of gdb_has_a_terminal.
2016-08-22 Pedro Alves <palves@redhat.com>
PR gdb/20505

View file

@ -74,13 +74,17 @@ struct terminal_info
};
/* Our own tty state, which we restore every time we need to deal with
the terminal. This is only set once, when GDB first starts. The
settings of flags which readline saves and restores and
the terminal. This is set once, when GDB first starts, and then
whenever we enter/leave TUI mode (gdb_save_tty_state). The
settings of flags which readline saves and restores are
unimportant. */
static struct terminal_info our_terminal_info;
/* Snapshot of our own tty state taken during initialization of GDB.
This is used as the initial tty state given to each new inferior. */
/* Snapshot of the initial tty state taken during initialization of
GDB, before readline/ncurses have had a chance to change it. This
is used as the initial tty state given to each new spawned
inferior. Unlike our_terminal_info, this is only ever set
once. */
static serial_ttystate initial_gdb_ttystate;
static struct terminal_info *get_inflow_inferior_data (struct inferior *);
@ -136,61 +140,37 @@ gdb_getpgrp (void)
}
#endif
enum gdb_has_a_terminal_flag_enum
{
yes, no, have_not_checked
}
gdb_has_a_terminal_flag = have_not_checked;
/* Set the initial tty state that is to be inherited by new inferiors. */
/* See terminal.h. */
void
set_initial_gdb_ttystate (void)
{
/* Note we can't do any of this in _initialize_inflow because at
that point stdin_serial has not been created yet. */
initial_gdb_ttystate = serial_get_tty_state (stdin_serial);
}
/* Does GDB have a terminal (on stdin)? */
int
gdb_has_a_terminal (void)
{
switch (gdb_has_a_terminal_flag)
if (initial_gdb_ttystate != NULL)
{
case yes:
return 1;
case no:
return 0;
case have_not_checked:
/* Get all the current tty settings (including whether we have a
tty at all!). Can't do this in _initialize_inflow because
serial_fdopen() won't work until the serial_ops_list is
initialized. */
our_terminal_info.ttystate
= serial_copy_tty_state (stdin_serial, initial_gdb_ttystate);
#ifdef F_GETFL
our_terminal_info.tflags = fcntl (0, F_GETFL, 0);
#endif
gdb_has_a_terminal_flag = no;
if (stdin_serial != NULL)
{
our_terminal_info.ttystate = serial_get_tty_state (stdin_serial);
if (our_terminal_info.ttystate != NULL)
{
gdb_has_a_terminal_flag = yes;
#ifdef PROCESS_GROUP_TYPE
our_terminal_info.process_group = gdb_getpgrp ();
our_terminal_info.process_group = gdb_getpgrp ();
#endif
}
}
return gdb_has_a_terminal_flag == yes;
default:
/* "Can't happen". */
return 0;
}
}
/* Does GDB have a terminal (on stdin)? */
static int
gdb_has_a_terminal (void)
{
return initial_gdb_ttystate != NULL;
}
/* Macro for printing errors from ioctl operations */
#define OOPSY(what) \

View file

@ -99,10 +99,10 @@ extern int gdb_setpgid (void);
/* Set up a serial structure describing standard input. In inflow.c. */
extern void initialize_stdin_serial (void);
extern int gdb_has_a_terminal (void);
extern void gdb_save_tty_state (void);
/* Take a snapshot of our initial tty state before readline/ncurses
have had a chance to alter it. */
extern void set_initial_gdb_ttystate (void);
/* Set the process group of the caller to its own pid, or do nothing

View file

@ -1,3 +1,9 @@
2016-08-23 Pedro Alves <palves@redhat.com>
PR gdb/20494
* gdb.base/new-ui-echo.c: New file.
* gdb.base/new-ui-echo.exp: New file.
2016-08-23 Yao Qi <yao.qi@linaro.org>
* gdb.server/connect-stopped-target.exp (do_test): Pass "" to

View file

@ -0,0 +1,30 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2016 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
volatile int global = 0;
int
main (void)
{
global = 1; /* set break main console here */
global = 1;
global = 1; /* set break extra console here */
global = 1;
return 0;
}

View file

@ -0,0 +1,127 @@
# Copyright 2016 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Regression test for PR 20494 (User input stops being echoed in CLI).
# Before that bug was fixed, starting an inferior in a non-main UI
# would result in GDB saving readline's prepped terminal state as
# gdb's "own" terminal state (i.e., target_terminal_ours state),
# resulting in subsequent synchronous execution commands in the main
# UI disabling input echo.
standard_testfile
set compile_options "debug"
if {[build_executable $testfile.exp $testfile ${srcfile} ${compile_options}] == -1} {
untested "failed to compile $testfile"
return -1
}
# Start gdb and create an extra console UI. Start the inferior in the
# DRIVER console (either "main" or "extra"), and then enter a
# synchronous execution command in the extra console. Before PR 20494
# was fixed, if DRIVER was a secondary UI, GDB would lose input echo
# on the main UI after the synchronous execution command. We test
# with both main and extra UIs as driver consoles for completeness.
proc echo_test {driver} {
global srcfile testfile
global gdb_prompt
global gdb_spawn_id
global gdb_main_spawn_id extra_spawn_id
global decimal
clean_restart $testfile
# Save the main UI's spawn ID.
set gdb_main_spawn_id $gdb_spawn_id
# Create the new PTY for the secondary console UI.
spawn -pty
set extra_spawn_id $spawn_id
set extra_tty_name $spawn_out(slave,name)
gdb_test_multiple "new-ui console $extra_tty_name" "new-ui" {
-re "New UI allocated\r\n$gdb_prompt $" {
}
}
with_spawn_id $extra_spawn_id {
set test "initial prompt on extra console"
gdb_test_multiple "" $test {
-re "$gdb_prompt $" {
pass $test
}
}
}
set main_console [list $gdb_main_spawn_id "main console"]
set extra_console [list $extra_spawn_id "extra console"]
if {$driver == "main"} {
set con1 $main_console
set con2 $extra_console
} else {
set con1 $extra_console
set con2 $main_console
}
set con1_spawn_id [lindex $con1 0]
set con2_spawn_id [lindex $con2 0]
set con1_name [lindex $con1 1]
set con2_name [lindex $con2 1]
set bp_lineno [gdb_get_line_number "set break $con1_name here"]
with_spawn_id $con1_spawn_id {
gdb_test "break $srcfile:$bp_lineno" \
"Breakpoint $decimal .*$srcfile, line $bp_lineno\\." \
"set breakpoint using $con1_name"
gdb_run_cmd
gdb_test "" "set break $con1_name here .*" "run to breakpoint on $con1_name"
}
with_spawn_id $con2_spawn_id {
set test "breakpoint hit reported on $con2_name too"
gdb_test_multiple "" $test {
-re "Breakpoint $decimal, .* set break $con1_name here " {
pass $test
}
}
gdb_test "next" "global = 1;" "next on $con2_name"
}
# Ensure echo remains enabled in both consoles.
with_spawn_id $con1_spawn_id {
gdb_test "print 1" "^print 1\r\n\\\$1 = 1" "print on $con1_name echoes"
}
with_spawn_id $con2_spawn_id {
gdb_test "print 2" "^print 2\r\n\\\$2 = 2" "print on $con2_name echoes"
}
}
# The test driver.
proc test_driver {} {
with_test_prefix "extra console as driver" {
echo_test "extra"
}
with_test_prefix "main console as driver" {
echo_test "main"
}
}
test_driver

View file

@ -1719,7 +1719,7 @@ show_interactive_mode (struct ui_file *file, int from_tty,
if (interactive_mode == AUTO_BOOLEAN_AUTO)
fprintf_filtered (file, "Debugger's interactive mode "
"is %s (currently %s).\n",
value, gdb_has_a_terminal () ? "on" : "off");
value, input_interactive_p (current_ui) ? "on" : "off");
else
fprintf_filtered (file, "Debugger's interactive mode is %s.\n", value);
}