Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
@ 2021-06-14 21:23 Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 01/16] Test interrupting programs that block SIGINT [gdb/9425, gdb/14559] Pedro Alves
                   ` (18 more replies)
  0 siblings, 19 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:23 UTC (permalink / raw)
  To: gdb-patches

New in v2:
  - run + detach no longer results in the detached process dying with
    SIGHUP.  GDB no longer queries about it on detach.
  - the testsuite gets fewer "tty /dev/tty" as a result.
  - documentation updated accordingly.
  - documentation now explicitly mentions that you can use "signal
    SIGINT" after Ctrl-C.
  - Added missing "@noindent" per Eli's review.
  - the gdb.base/annota1.exp patch has been meanwhile merged

For review convenience, I've put this in the users/palves/ctrl-c
branch.

Currently, on GNU/Linux, it is not possible to interrupt with Ctrl-C
programs that block or ignore SIGINT, with e.g., sigprocmask or
signal(SIGINT, SIG_IGN).  You type Ctrl-C, but nothing happens.

Similarly, if a program uses sigwait to wait for SIGINT, and the
program receives a SIGINT, the SIGINT is _not_ intercepted by ptrace,
it goes straight to the inferior.  These problems have been known for
years, and recorded in gdb/9425, gdb/14559.

This is a consequence of how GDB implements interrupting programs with
Ctrl-C -- when GDB spawns a new process, it makes the process use the
same terminal as GDB, and then makes the process's process group be
the foreground process group in GDB's terminal.  This means that when
the process is running in the foreground, after e.g. "continue", when
the user types Ctrl-C, the kernel sends a SIGINT to the foreground
process group, which is the inferior.  GDB then intercepts the SIGINT
via ptrace, the same way it intercepts any other signal, stops all the
other threads of the inferior if in all-stop, and presents the
"Program received SIGINT" stop to the user.

This series address the problem by turning Ctrl-C handling around such
that the SIGINT always reaches GDB first, not the inferior.  That is
done by making GDB put inferiors in their own terminal/session created
by GDB.  I.e., GDB creates a pseudo-terminal master/slave pair, makes
the inferior run with the slave as its terminal, and pumps
output/input on the master end.  Because the inferior is run with its
own session/terminal, GDB is free to remain as the foreground process
in its own terminal, which means that the Ctrl-C SIGINT always reaches
GDB first, instead of reaching the inferior first, and then GDB
reacting to the ptrace-intercepted SIGINT.  Because GDB gets the
SIGINT first, GDB is then free to handle it by interrupting the
program any way it sees fit.

The series will then make GDB interrupt the program with SIGSTOP
instead of SIGINT, which always works even if the inferior
blocks/ignores SIGINT -- SIGSTOP can't be ignored.  (In the future GDB
may even switch to PTRACE_INTERRUPT, though that's a project of its
own.)

Having the inferior in its own terminal also means that GDB is in
control of when inferior output is flushed to the screen.  When
debugging with the CLI, this means that inferior output is now never
interpersed with GDB's output in an unreadable fashion.  This will
also allow fixing the problem of inferior output really messing up the
screen in the TUI, forcing users to Ctrl-L to refresh the screen.
This series does not address the TUI part, but it shouldn't be too
hard -- I wrote a quick&dirty prototype patch doing that a few years
back, so I know it works.

Tested on GNU/Linux native, gdbserver and gdbserver + "maint target
set-non-stop on".  Also build-tested on mingw32-w64, Solaris 11, and
OpenBSD.

More details in each of the patches in the series.

The first patch adds tests that fail currently, but will pass at the
end of the series.

The main GDB changes are in patches #12 and #15.

Patch #1-#5, #9-#11, #13-#14 could go in without the main changes.

All documentation (manual, NEWS) is in the last patch -- patch #16.

Pedro Alves (16):
  Test interrupting programs that block SIGINT [gdb/9425, gdb/14559]
  prefork_hook: Remove 'args' parameter
  Make gdb.base/long-inferior-output.exp fail fast
  Fix gdb.multi/multi-term-settings.exp race
  Don't check parent pid in
    gdb.threads/{ia64-sigill,siginfo-threads,watchthreads-reorder}.c
  Special-case "set inferior-tty /dev/tty"
  Make inferior/GDB share terminal in tests expecting output after
    detach
  Make inferior/GDB share terminal in tests that exercise GDB/inferior
    reading same input
  gdb.mi/mi-logging.exp, don't send input to GDB while the inferior is
    running
  target_terminal::ours_for_output before printing signal received
  Move scoped_ignore_sigttou to gdbsupport/
  Always put inferiors in their own terminal/session [gdb/9425,
    gdb/14559]
  exists_non_stop_target: Avoid flushing frames
  convert previous_inferior_ptid to strong reference to thread_info
  GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR
    gdb/9425, PR gdb/14559]
  Document pseudo-terminal and interrupting changes

 gdb/doc/gdb.texinfo                           | 117 ++-
 gdb/NEWS                                      |  23 +
 gdb/Makefile.in                               |   1 -
 gdb/event-top.c                               |  10 +-
 gdb/fork-child.c                              |  10 +-
 gdb/inf-ptrace.c                              |  24 +-
 gdb/inf-ptrace.h                              |   8 +
 gdb/infcmd.c                                  |  39 +-
 gdb/inferior.h                                |   4 +
 gdb/inflow.c                                  | 719 +++++++++++++++---
 gdb/infrun.c                                  | 179 ++++-
 gdb/infrun.h                                  |  16 +
 gdb/linux-nat.c                               | 115 ++-
 gdb/linux-nat.h                               |   2 +
 gdb/nat/fork-inferior.c                       | 120 ++-
 gdb/nat/fork-inferior.h                       |  30 +-
 gdb/procfs.c                                  |   1 -
 gdb/ser-unix.c                                |   2 +-
 gdb/target.c                                  |  40 +-
 gdb/terminal.h                                |   3 +
 .../gdb.base/annota-input-while-running.exp   |   4 +
 gdb/testsuite/gdb.base/annota1.exp            |   3 +-
 .../gdb.base/bp-cmds-continue-ctrl-c.exp      |   3 +
 .../gdb.base/continue-all-already-running.exp |   3 +
 gdb/testsuite/gdb.base/infcall-input.exp      |  12 +-
 .../gdb.base/interrupt-daemon-attach.exp      |   2 +-
 gdb/testsuite/gdb.base/interrupt-daemon.exp   |   4 +-
 gdb/testsuite/gdb.base/interrupt-noterm.exp   |   2 +-
 gdb/testsuite/gdb.base/interrupt.exp          |   4 +-
 .../gdb.base/long-inferior-output.exp         |  31 +-
 gdb/testsuite/gdb.base/random-signal.exp      |   3 +-
 gdb/testsuite/gdb.base/range-stepping.exp     |   2 +-
 gdb/testsuite/gdb.base/sigint-masked-out.c    |  43 ++
 gdb/testsuite/gdb.base/sigint-masked-out.exp  | 109 +++
 gdb/testsuite/gdb.base/sigint-sigwait.c       |  80 ++
 gdb/testsuite/gdb.base/sigint-sigwait.exp     | 113 +++
 gdb/testsuite/gdb.gdb/selftest.exp            |   4 +-
 gdb/testsuite/gdb.mi/mi-logging.exp           |  94 ++-
 gdb/testsuite/gdb.mi/new-ui-mi-sync.exp       |   2 +-
 .../gdb.multi/multi-target-interrupt.exp      |   2 +-
 .../gdb.multi/multi-target-no-resumed.exp     |   2 +-
 .../gdb.multi/multi-term-settings.exp         |  92 ++-
 gdb/testsuite/gdb.server/reconnect-ctrl-c.exp |   4 +-
 gdb/testsuite/gdb.threads/async.exp           |   2 +-
 .../gdb.threads/continue-pending-status.exp   |   2 +-
 gdb/testsuite/gdb.threads/ia64-sigill.c       |   5 -
 gdb/testsuite/gdb.threads/leader-exit.exp     |   2 +-
 gdb/testsuite/gdb.threads/manythreads.exp     |   4 +-
 .../process-dies-while-detaching.exp          |   3 +
 gdb/testsuite/gdb.threads/pthreads.exp        |   2 +-
 gdb/testsuite/gdb.threads/schedlock.exp       |  11 +-
 gdb/testsuite/gdb.threads/siginfo-threads.c   |   5 -
 gdb/testsuite/gdb.threads/sigthread.exp       |   2 +-
 .../gdb.threads/watchthreads-reorder.c        |   5 -
 gdb/testsuite/lib/gdb.exp                     |  19 +
 gdb/tui/tui-win.c                             |   3 +
 gdbserver/fork-child.cc                       |  15 +-
 gdbsupport/Makefile.am                        |   1 +
 gdbsupport/Makefile.in                        |  15 +-
 gdbsupport/managed-tty.cc                     |  22 +
 gdbsupport/managed-tty.h                      |  42 +
 .../scoped_ignore_sigttou.h                   |   8 +-
 62 files changed, 1994 insertions(+), 255 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/sigint-masked-out.c
 create mode 100644 gdb/testsuite/gdb.base/sigint-masked-out.exp
 create mode 100644 gdb/testsuite/gdb.base/sigint-sigwait.c
 create mode 100644 gdb/testsuite/gdb.base/sigint-sigwait.exp
 create mode 100644 gdbsupport/managed-tty.cc
 create mode 100644 gdbsupport/managed-tty.h
 rename gdb/inflow.h => gdbsupport/scoped_ignore_sigttou.h (90%)


base-commit: c9923e71ff57ce6e824833560aae59057c6f5783
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 01/16] Test interrupting programs that block SIGINT [gdb/9425, gdb/14559]
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
@ 2021-06-14 21:23 ` Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 02/16] prefork_hook: Remove 'args' parameter Pedro Alves
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:23 UTC (permalink / raw)
  To: gdb-patches

This adds a couple testcases that exercise interrupting programs that
block SIGINT.  One tests a program that uses sigprocmask to mask out
SIGINT, and the other tests a program that waits for SIGINT with
sigwait.  On GNU/Linux, it is currently impossible to interrupt such a
program with Ctrl-C, and you can't interrupt it with the "interrupt"
command in all-stop mode either.

These currently fail, and are suitably kfailed.  The rest of the
series will fix the fails.

Tested on GNU/Linux native, gdbserver, and gdbserver + "maint set
target-non-stop on".

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/9425
	PR gdb/14559
	* gdb.base/sigint-masked-out.c: New file.
	* gdb.base/sigint-masked-out.exp: New file.
	* gdb.base/sigint-sigwait.c: New file.
	* gdb.base/sigint-sigwait.exp: New file.

Change-Id: Iddb70c0a2c92550027fccb6f51ecba1292d233c8
---
 gdb/testsuite/gdb.base/sigint-masked-out.c   |  43 ++++++++
 gdb/testsuite/gdb.base/sigint-masked-out.exp | 101 ++++++++++++++++++
 gdb/testsuite/gdb.base/sigint-sigwait.c      |  80 ++++++++++++++
 gdb/testsuite/gdb.base/sigint-sigwait.exp    | 105 +++++++++++++++++++
 4 files changed, 329 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/sigint-masked-out.c
 create mode 100644 gdb/testsuite/gdb.base/sigint-masked-out.exp
 create mode 100644 gdb/testsuite/gdb.base/sigint-sigwait.c
 create mode 100644 gdb/testsuite/gdb.base/sigint-sigwait.exp

diff --git a/gdb/testsuite/gdb.base/sigint-masked-out.c b/gdb/testsuite/gdb.base/sigint-masked-out.c
new file mode 100644
index 00000000000..dce28182add
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sigint-masked-out.c
@@ -0,0 +1,43 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+#include <unistd.h>
+#include <signal.h>
+
+static void
+done ()
+{
+}
+
+int
+main (int argc, char *argv[])
+{
+  int i;
+  sigset_t sigs;
+
+  alarm (30);
+
+  sigfillset (&sigs);
+  sigprocmask (SIG_SETMASK, &sigs, NULL);
+
+  done ();
+
+  while (1)
+    sleep (1);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/sigint-masked-out.exp b/gdb/testsuite/gdb.base/sigint-masked-out.exp
new file mode 100644
index 00000000000..0391152e93a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sigint-masked-out.exp
@@ -0,0 +1,101 @@
+# Copyright 2021 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/>.
+
+# Make sure that we can interrupt an inferior that has all signals
+# masked out, including SIGINT, with both Ctrl-C and the "interrupt"
+# command.
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile {debug}]} {
+    return -1
+}
+
+# Test interrupting with Ctrl-C.
+
+proc_with_prefix test_ctrl_c {} {
+    global binfile
+    global gdb_prompt
+
+    clean_restart $binfile
+
+    if ![runto "done"] {
+	fail "can't run to done function"
+	return
+    }
+
+    gdb_test_multiple "continue" "" {
+	-re "Continuing" {
+	    pass $gdb_test_name
+	}
+    }
+
+    after 200
+
+    send_gdb "\003"
+
+    gdb_test_multiple "" "ctrl-c stops process" {
+	-timeout 5
+	-re -wrap "(received signal SIGINT|stopped).*" {
+	    pass $gdb_test_name
+	}
+	timeout {
+	    setup_kfail "gdb/9425" *-*-*
+	    fail "$gdb_test_name (timeout)"
+	}
+    }
+}
+
+# Test interrupting with the "interrupt" command.
+
+proc_with_prefix test_interrupt_cmd {} {
+    global binfile
+    global gdb_prompt
+
+    clean_restart $binfile
+
+    if ![runto "done"] {
+	fail "can't run to done function"
+	return
+    }
+
+    gdb_test_multiple "continue&" "" {
+	-re "Continuing\\.\r\n$gdb_prompt " {
+	    pass $gdb_test_name
+	}
+    }
+
+    after 200
+
+    gdb_test_multiple "interrupt" "" {
+	-re "$gdb_prompt " {
+	    pass $gdb_test_name
+	}
+    }
+
+    gdb_test_multiple "" "interrupt cmd stops process" {
+	-timeout 5
+	-re "(received signal SIGINT|stopped)" {
+	    pass $gdb_test_name
+	}
+	timeout {
+	    setup_kfail "gdb/14559" *-*-*
+	    fail "$gdb_test_name (timeout)"
+	}
+    }
+}
+
+test_ctrl_c
+test_interrupt_cmd
diff --git a/gdb/testsuite/gdb.base/sigint-sigwait.c b/gdb/testsuite/gdb.base/sigint-sigwait.c
new file mode 100644
index 00000000000..8d1408c5955
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sigint-sigwait.c
@@ -0,0 +1,80 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <signal.h>
+#include <string.h>
+#include <unistd.h>
+
+static void
+handle_sigint (int signo)
+{
+}
+
+static void
+done ()
+{
+}
+
+int
+main (int argc, char **argv)
+{
+  sigset_t signal_set;
+  struct sigaction s;
+
+  alarm (30);
+
+  s.sa_flags = SA_RESTART;
+  s.sa_handler = handle_sigint;
+
+  if (sigaction (SIGINT, &s, NULL) < 0)
+    {
+      perror ("<%s> Error sigaction: ");
+      exit (2);
+    }
+
+  if (sigfillset (&signal_set) < 0)
+    {
+      perror ("<main> Error sigfillset(): ");
+      exit (2);
+    }
+
+  done ();
+
+  while (1)
+    {
+      int sig;
+
+      /* Wait for queued signals.  .*/
+      if (sigwait (&signal_set, &sig) != 0)
+	{
+	  perror ("<main> Error sigwait(): ");
+	  exit (1);
+	}
+
+      /* Process signal.  */
+      if (sig == SIGINT)
+	{
+	  printf ("Terminating.\n");
+	  exit (0);
+	}
+
+      printf ("Unhandled signal [%s]\n", strsignal (sig));
+    }
+}
diff --git a/gdb/testsuite/gdb.base/sigint-sigwait.exp b/gdb/testsuite/gdb.base/sigint-sigwait.exp
new file mode 100644
index 00000000000..1dd706fc1a4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sigint-sigwait.exp
@@ -0,0 +1,105 @@
+# Copyright 2021 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/>.
+
+# Make sure that we can interrupt an inferior that has all signals
+# masked out, including SIGINT, and then waits for signals with
+# sigwait.  Test interrupting with both Ctrl-C and the "interrupt"
+# command.
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile {debug}]} {
+    return -1
+}
+
+# Test interrupting with Ctrl-C.
+
+proc_with_prefix test_ctrl_c {} {
+    global binfile
+    global gdb_prompt
+
+    clean_restart $binfile
+
+    if ![runto "done"] {
+	fail "can't run to done function"
+	return
+    }
+
+    gdb_test_multiple "continue" "" {
+	-re "Continuing" {
+	    pass $gdb_test_name
+	}
+    }
+
+    after 200
+
+    send_gdb "\003"
+
+    global exited_normally_re
+
+    gdb_test_multiple "" "ctrl-c stops process" {
+	-re -wrap "(received signal SIGINT|stopped).*" {
+	    pass $gdb_test_name
+	}
+	-re -wrap "Inferior.*exited normally.*" {
+	    setup_kfail "gdb/9425" *-*-*
+	    fail "$gdb_test_name (the program exited)"
+	}
+    }
+}
+
+# Test interrupting with the "interrupt" command.
+
+proc_with_prefix test_interrupt_cmd {} {
+    global binfile
+    global gdb_prompt
+
+    clean_restart $binfile
+
+    if ![runto "done"] {
+	fail "can't run to done function"
+	return
+    }
+
+    gdb_test_multiple "continue&" "" {
+	-re "Continuing\\.\r\n$gdb_prompt " {
+	    pass $gdb_test_name
+	}
+    }
+
+    after 200
+
+    gdb_test_multiple "interrupt" "" {
+	-re "$gdb_prompt " {
+	    pass $gdb_test_name
+	}
+    }
+
+    global exited_normally_re
+
+    gdb_test_multiple "" "interrupt cmd stops process" {
+	-timeout 5
+	-re "(received signal SIGINT|stopped)" {
+	    pass $gdb_test_name
+	}
+	-re "Inferior.*exited normally" {
+	    setup_kfail "gdb/14559" *-*-*
+	    fail "$gdb_test_name (the program exited)"
+	}
+    }
+}
+
+test_ctrl_c
+test_interrupt_cmd
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 02/16] prefork_hook: Remove 'args' parameter
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 01/16] Test interrupting programs that block SIGINT [gdb/9425, gdb/14559] Pedro Alves
@ 2021-06-14 21:23 ` Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 03/16] Make gdb.base/long-inferior-output.exp fail fast Pedro Alves
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:23 UTC (permalink / raw)
  To: gdb-patches

prefork_hook's 'args' parameter is only used in debug output in
gdbserver.  Remove it.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* fork-child.c (prefork_hook): Remove 'args' parameter.  All
	callers adjusted.
	* nat/fork-inferior.h (prefork_hook): Remove 'args' parameter.
	All callers adjusted.
	* nat/fork-inferior.c (fork_inferior): Adjust.

gdbserver/fork-child.cc
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* fork-child.cc (prefork_hook): Remove 'args' parameter, and
	references.

Change-Id: Iaf8977af7dd6915c123b0d50ded93395bdafd920
---
 gdb/fork-child.c        | 2 +-
 gdb/nat/fork-inferior.c | 2 +-
 gdb/nat/fork-inferior.h | 7 +++----
 gdbserver/fork-child.cc | 7 +------
 4 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index 3ce7d64b855..41135b53f9b 100644
--- a/gdb/fork-child.c
+++ b/gdb/fork-child.c
@@ -59,7 +59,7 @@ static struct ui *saved_ui = NULL;
 /* See nat/fork-inferior.h.  */
 
 void
-prefork_hook (const char *args)
+prefork_hook ()
 {
   gdb_assert (saved_ui == NULL);
   /* Retain a copy of our UI, since the child will replace this value
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index d280e1120cc..7d56250c979 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -317,7 +317,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
 
   /* Perform any necessary actions regarding to TTY before the
      fork/vfork call.  */
-  prefork_hook (allargs.c_str ());
+  prefork_hook ();
 
   /* It is generally good practice to flush any possible pending stdio
      output prior to doing a fork, to avoid the possibility of both
diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h
index aae2aa6a854..f45ec9047c8 100644
--- a/gdb/nat/fork-inferior.h
+++ b/gdb/nat/fork-inferior.h
@@ -58,10 +58,9 @@ extern ptid_t startup_inferior (process_stratum_target *proc_target,
 				struct target_waitstatus *mystatus,
 				ptid_t *myptid);
 
-/* Perform any necessary tasks before a fork/vfork takes place.  ARGS
-   is a string containing all the arguments received by the inferior.
-   This function is mainly used by fork_inferior.  */
-extern void prefork_hook (const char *args);
+/* Perform any necessary tasks before a fork/vfork takes place.  This
+   function is mainly used by fork_inferior.  */
+extern void prefork_hook ();
 
 /* Perform any necessary tasks after a fork/vfork takes place.  This
    function is mainly used by fork_inferior.  */
diff --git a/gdbserver/fork-child.cc b/gdbserver/fork-child.cc
index a431e7ef889..9678133243d 100644
--- a/gdbserver/fork-child.cc
+++ b/gdbserver/fork-child.cc
@@ -42,14 +42,9 @@ restore_old_foreground_pgrp (void)
 /* See nat/fork-inferior.h.  */
 
 void
-prefork_hook (const char *args)
+prefork_hook ()
 {
   client_state &cs = get_client_state ();
-  if (debug_threads)
-    {
-      debug_printf ("args: %s\n", args);
-      debug_flush ();
-    }
 
 #ifdef SIGTTOU
   signal (SIGTTOU, SIG_DFL);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 03/16] Make gdb.base/long-inferior-output.exp fail fast
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 01/16] Test interrupting programs that block SIGINT [gdb/9425, gdb/14559] Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 02/16] prefork_hook: Remove 'args' parameter Pedro Alves
@ 2021-06-14 21:23 ` Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 04/16] Fix gdb.multi/multi-term-settings.exp race Pedro Alves
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:23 UTC (permalink / raw)
  To: gdb-patches

A local change I was working on had a bug that made GDB lose inferior
output, which was caught by the gdb.base/long-inferior-output.exp
testcase.  While debugging the problem, I found it useful to have the
testcase fail quickly when it noticed output was missing.

Also, tighten the regexes to make sure that we don't get
spurious/repeated/bogus output between each "this is line number ..."
line.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.base/long-inferior-output.exp: Don't set "more" when we see
	an unexpected "this is line number" number.

Change-Id: I53e8499bd8fdbf961431a7f2a94d263da6a9f573
---
 .../gdb.base/long-inferior-output.exp         | 31 +++++++++++++++++--
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/long-inferior-output.exp b/gdb/testsuite/gdb.base/long-inferior-output.exp
index 294786ffff2..c0e18029600 100644
--- a/gdb/testsuite/gdb.base/long-inferior-output.exp
+++ b/gdb/testsuite/gdb.base/long-inferior-output.exp
@@ -47,21 +47,46 @@ if { ![runto_main] } then {
 set printing_done_line [gdb_get_line_number "printing done"]
 gdb_test "break $printing_done_line" ".*" "set breakpoint after printing"
 
-send_gdb "continue\n"
+gdb_test_multiple "continue" "" {
+    -re "Continuing\.\r\n" {
+	pass $gdb_test_name
+    }
+}
 
 set expected_lines 3000
 set more 1
 set i 0
 while {$more} {
     set more 0
+
+    # Make sure that we don't get spurious/repeated/bogus output
+    # between each "this is line number ..."  line, with an anchor.
+    # But consume any output that precedes the first line, because
+    # when testing against GDBserver, we'll get:
+    #
+    #   continue
+    #   Continuing.
+    #   PASS: gdb.base/long-inferior-output.exp: continue
+    #   Remote debugging from host ::1, port 40044
+    #   this is line number 0
+    #   ...
+    if {$i != 0} {
+	set anchor "^"
+    } else {
+	set anchor ""
+    }
+
     gdb_expect {
 	-i $inferior_spawn_id
-	-ex "this is line number $i" {
+	-re "${anchor}this is line number $i\r\n" {
 	    incr i
-           if {$i != $expected_lines} {
+	    if {$i != $expected_lines} {
 		set more 1
 	    }
 	}
+	-re "this is line number \[^\r\n\]*\r\n" {
+	    # If we see this, we've lost output.
+	}
     }
 }
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 04/16] Fix gdb.multi/multi-term-settings.exp race
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (2 preceding siblings ...)
  2021-06-14 21:23 ` [PATCH v2 03/16] Make gdb.base/long-inferior-output.exp fail fast Pedro Alves
@ 2021-06-14 21:23 ` Pedro Alves
  2021-06-14 21:23 ` [PATCH v2 05/16] Don't check parent pid in gdb.threads/{ia64-sigill, siginfo-threads, watchthreads-reorder}.c Pedro Alves
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:23 UTC (permalink / raw)
  To: gdb-patches

The gdb.multi/multi-term-settings.exp testcase sometimes fails like so:

 Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.multi/multi-term-settings.exp ...
 FAIL: gdb.multi/multi-term-settings.exp: inf1_how=attach: inf2_how=attach: stop with control-c (SIGINT)

It's easier to reproduce if you stress the machine at the same time, like e.g.:

  $ stress -c 24

Looking at gdb.log, we see:

 (gdb) attach 60422
 Attaching to program: build/gdb/testsuite/outputs/gdb.multi/multi-term-settings/multi-term-settings, process 60422
 [New Thread 60422.60422]
 Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...
 Reading symbols from /usr/lib/debug//lib/x86_64-linux-gnu/libc-2.31.so...
 Reading symbols from /lib64/ld-linux-x86-64.so.2...
 (No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
 0x00007f2fc2485334 in __GI___clock_nanosleep (clock_id=<optimized out>, clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7ffe23126940, rem=rem@entry=0x0) at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
 78	../sysdeps/unix/sysv/linux/clock_nanosleep.c: No such file or directory.
 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=attach: inf2_how=attach: inf2: attach
 set schedule-multiple on
 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=attach: inf2_how=attach: set schedule-multiple on
 info inferiors
   Num  Description       Connection                         Executable
   1    process 60404     1 (extended-remote localhost:2349) build/gdb/testsuite/outputs/gdb.multi/multi-term-settings/multi-term-settings
 * 2    process 60422     1 (extended-remote localhost:2349) build/gdb/testsuite/outputs/gdb.multi/multi-term-settings/multi-term-settings
 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=attach: inf2_how=attach: info inferiors
 pid=60422, count=46
 pid=60422, count=47
 pid=60422, count=48
 pid=60422, count=49
 pid=60422, count=50
 pid=60422, count=51
 pid=60422, count=52
 pid=60422, count=53
 pid=60422, count=54
 pid=60422, count=55
 pid=60422, count=56
 pid=60422, count=57
 pid=60422, count=58
 pid=60422, count=59
 pid=60422, count=60
 pid=60422, count=61
 pid=60422, count=62
 pid=60422, count=63
 pid=60422, count=64
 pid=60422, count=65
 pid=60422, count=66
 pid=60422, count=67
 pid=60422, count=68
 pid=60422, count=69
 pid=60404, count=54
 pid=60404, count=55
 pid=60404, count=56
 pid=60404, count=57
 pid=60404, count=58
 PASS: gdb.multi/multi-term-settings.exp: inf1_how=attach: inf2_how=attach: continue
 Quit
 (gdb) FAIL: gdb.multi/multi-term-settings.exp: inf1_how=attach: inf2_how=attach: stop with control-c (SIGINT)

If you look at the testcase's sources, you'll see that the intention
is to resumes the program with "continue", wait to see a few of those
"pid=..., count=..." lines, and then interrupt the program with
Ctrl-C.  But somehow, that resulted in GDB printing "Quit", instead of
the Ctrl-C stopping the program with SIGINT.

Here's what is happening:

 #1 - those "pid=..., count=..." lines we see above weren't actually
      output by the inferior after it has been continued (see #1).
      Note that "inf1_how" and "inf2_how" are "attach".  What happened
      is that those "pid=..., count=..." lines were output by the
      inferiors _before_ they were attached to.  We see them at that
      point instead of earlier, because that's where the testcase
      reads from the inferiors' spawn_ids.

 #2 - The testcase mistakenly thinks those "pid=..., count=..." lines
      happened after the continue was processed by GDB, meaning it has
      waited enough, and so sends the Ctrl-C.  GDB hasn't yet passed
      the terminal to the inferior, so the Ctrl-C results in that
      Quit.

The fix here is twofold:

 #1 - flush inferior output right after attaching

 #2 - consume the "Continuing" printed by "continue", indicating the
      inferior has the terminal.  This is the same as done throughout
      the testsuite to handle this exact problem of sending Ctrl-C too
      soon.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.multi/multi-term-settings.exp (create_inferior): Flush
	inferior output.
	(coretest): Use $gdb_test_name.  After issuing "continue", wait
	for "Continuing".

Change-Id: Iba7671dfe1eee6b98d29cfdb05a1b9aa2f9defb9
---
 .../gdb.multi/multi-term-settings.exp         | 40 ++++++++++++++++---
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.multi/multi-term-settings.exp b/gdb/testsuite/gdb.multi/multi-term-settings.exp
index 20ec03d94b3..dcc6f2ece0f 100644
--- a/gdb/testsuite/gdb.multi/multi-term-settings.exp
+++ b/gdb/testsuite/gdb.multi/multi-term-settings.exp
@@ -95,6 +95,22 @@ proc create_inferior {which_inf inf_how} {
 	if {[gdb_test "attach $testpid" \
 		 "Attaching to program: .*, process $testpid.*(in|at).*" \
 		 "attach"] == 0} {
+
+	    # The program is now stopped, but if testing against
+	    # gdbserver, then the inferior's output emmitted before it
+	    # stopped isn't flushed unless we explicitly do so,
+	    # because it is on a different spawn_id.  Do it now, to
+	    # avoid confusing tests further below.
+	    gdb_test_multiple "" "flush inferior output" {
+		-timeout 1
+		-i $test_spawn_id -re "pid=" {
+		    exp_continue
+		}
+		timeout {
+		    pass $gdb_test_name
+		}
+	    }
+
 	    return $test_spawn_id
 	}
     } else {
@@ -179,9 +195,9 @@ proc coretest {inf1_how inf2_how} {
 	uplevel 1 {
 	    if {$count1 >= 3 && $count2 >= 3} {
 		if $expect_ttou {
-		    fail "$test (expected SIGTTOU)"
+		    fail "$gdb_test_name (expected SIGTTOU)"
 		} else {
-		    pass $test
+		    pass $gdb_test_name
 		}
 	    } else {
 		exp_continue
@@ -195,8 +211,20 @@ proc coretest {inf1_how inf2_how} {
     set count1 0
     set count2 0
 
-    set test "continue"
-    gdb_test_multiple $test $test {
+    # We're going to interrupt with Ctrl-C.  For this to work we must
+    # be sure to consume the "Continuing." message first, or GDB may
+    # still own the terminal.  Also, note that in the attach case, we
+    # flushed inferior output right after attaching, so that we're
+    # sure that the "pid=" lines we see are emitted by the inferior
+    # after it is continued, instead of having been emitted before it
+    # was attached to.
+    gdb_test_multiple "continue" "continue, hand over terminal" {
+	-re "Continuing" {
+	    pass $gdb_test_name
+	}
+    }
+
+    gdb_test_multiple "" "continue" {
 	-i $infs_spawn_ids -re "pid=$pid1, count=" {
 	    incr count1
 	    pass_or_exp_continue
@@ -207,9 +235,9 @@ proc coretest {inf1_how inf2_how} {
 	}
 	-i $gdb_spawn_id -re "received signal SIGTTOU.*$gdb_prompt " {
 	    if $expect_ttou {
-		pass "$test (expected SIGTTOU)"
+		pass "$gdb_test_name (expected SIGTTOU)"
 	    } else {
-		fail "$test (SIGTTOU)"
+		fail "$gdb_test_name (SIGTTOU)"
 	    }
 	}
     }
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 05/16] Don't check parent pid in gdb.threads/{ia64-sigill, siginfo-threads, watchthreads-reorder}.c
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (3 preceding siblings ...)
  2021-06-14 21:23 ` [PATCH v2 04/16] Fix gdb.multi/multi-term-settings.exp race Pedro Alves
@ 2021-06-14 21:23 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 06/16] Special-case "set inferior-tty /dev/tty" Pedro Alves
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:23 UTC (permalink / raw)
  To: gdb-patches

A following patch will make GDB always put spawned inferiors in their
own terminal session.  To do that, GDB forks twice, creating an extra
"session leader" process between gdb and the inferior process.

gdb.threads/{ia64-sigill,siginfo-threads,watchthreads-reorder}.c all
check whether the parent process is GDB.  If it isn't, they just bail,
and the testcase fails.  After the changes mentioned above, this
parent check will fail if GDB is putting inferiors in their own
terminal session, because in that case, the test's parent is the extra
"session leader" process between gdb and the test process.  The tracer
will be the test process's grandparent, not the direct parent.

Since the test programs already check whether there's a ptracer
attached, there's no real need for this parent pid check.  Just remove
it.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.threads/ia64-sigill.c (main): Don't check whether the parent
	pid is the tracer.
	* gdb.threads/siginfo-threads.c (main): Don't check whether the
	parent pid is the tracer.
	* gdb.threads/watchthreads-reorder.c (main): Don't check whether
	the parent pid is the tracer.

Change-Id: Iea0b06fb93c31bde1a0993c52b3fe8a5f408aec7
---
 gdb/testsuite/gdb.threads/ia64-sigill.c          | 5 -----
 gdb/testsuite/gdb.threads/siginfo-threads.c      | 5 -----
 gdb/testsuite/gdb.threads/watchthreads-reorder.c | 5 -----
 3 files changed, 15 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/ia64-sigill.c b/gdb/testsuite/gdb.threads/ia64-sigill.c
index ef224f49f08..f3d7014dbbf 100644
--- a/gdb/testsuite/gdb.threads/ia64-sigill.c
+++ b/gdb/testsuite/gdb.threads/ia64-sigill.c
@@ -292,11 +292,6 @@ main (int argc, char **argv)
 	  fprintf (stderr, "The testcase must be run by GDB!\n");
 	  exit (EXIT_FAILURE);
 	}
-      if (tracer != getppid ())
-	{
-	  fprintf (stderr, "The testcase parent must be our GDB tracer!\n");
-	  exit (EXIT_FAILURE);
-	}
     }
 
   /* SIGCONT our debugger in the case of our crash as we would deadlock
diff --git a/gdb/testsuite/gdb.threads/siginfo-threads.c b/gdb/testsuite/gdb.threads/siginfo-threads.c
index adaa7583e78..762f53ad684 100644
--- a/gdb/testsuite/gdb.threads/siginfo-threads.c
+++ b/gdb/testsuite/gdb.threads/siginfo-threads.c
@@ -376,11 +376,6 @@ main (int argc, char **argv)
 	  fprintf (stderr, "The testcase must be run by GDB!\n");
 	  exit (EXIT_FAILURE);
 	}
-      if (tracer != getppid ())
-	{
-	  fprintf (stderr, "The testcase parent must be our GDB tracer!\n");
-	  exit (EXIT_FAILURE);
-	}
     }
 
   /* SIGCONT our debugger in the case of our crash as we would deadlock
diff --git a/gdb/testsuite/gdb.threads/watchthreads-reorder.c b/gdb/testsuite/gdb.threads/watchthreads-reorder.c
index ff8ca9afe7c..789dc7ac741 100644
--- a/gdb/testsuite/gdb.threads/watchthreads-reorder.c
+++ b/gdb/testsuite/gdb.threads/watchthreads-reorder.c
@@ -301,11 +301,6 @@ main (int argc, char **argv)
 	  fprintf (stderr, "The testcase must be run by GDB!\n");
 	  exit (EXIT_FAILURE);
 	}
-      if (tracer != getppid ())
-	{
-	  fprintf (stderr, "The testcase parent must be our GDB tracer!\n");
-	  exit (EXIT_FAILURE);
-	}
     }
 
   /* SIGCONT our debugger in the case of our crash as we would deadlock
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 06/16] Special-case "set inferior-tty /dev/tty"
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (4 preceding siblings ...)
  2021-06-14 21:23 ` [PATCH v2 05/16] Don't check parent pid in gdb.threads/{ia64-sigill, siginfo-threads, watchthreads-reorder}.c Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 07/16] Make inferior/GDB share terminal in tests expecting output after detach Pedro Alves
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

The following patches will make GDB spawn inferiors in their own
session and tty by default.  Meaning, GDB will create and manage a pty
for the inferior instead of the inferior and GDB sharing the same
terminal and GDB having to juggle terminal settings depending on
whether the inferior running or gdb showing the prompt.

For some use cases however, it will still be useful to be able to tell
GDB to spawn the inferior in the same terminal & session as GDB, like
today.

Setting the inferior tty to "/dev/tty" seems like an obvious way to
get that, as /dev/tty is a special file that represents the terminal
for the current process.  This leaves "tty" with no arguments free for
a different behavior.

This patch hardcodes "/dev/tty" in is_gdb_terminal for that reason.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* inflow.c (is_gdb_terminal): Hardcode "/dev/tty".
	(new_tty): Don't create a tty if is_gdb_terminal is true.
	(new_tty_postfork): Always allocate a run_terminal.
	(create_tty_session): Don't create a session if sharing the
	terminal with GDB.

Change-Id: I4dc7958f823fa4e30c21a2c3fe4d8434a5d5ed40
---
 gdb/inflow.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/gdb/inflow.c b/gdb/inflow.c
index d241540c4cd..15f29dcd08e 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -222,6 +222,11 @@ is_gdb_terminal (const char *tty)
   struct stat other_tty;
   int res;
 
+  /* Users can explicitly set the inferior tty to "/dev/tty" to mean
+     "the GDB terminal".  */
+  if (strcmp (tty, "/dev/tty") == 0)
+    return TRIBOOL_TRUE;
+
   res = stat (tty, &other_tty);
   if (res == -1)
     return TRIBOOL_UNKNOWN;
@@ -796,9 +801,10 @@ check_syscall (const char *msg, int result)
 #endif
 
 void
-new_tty (void)
+new_tty ()
 {
-  if (inferior_thisrun_terminal == 0)
+  if (inferior_thisrun_terminal == nullptr
+      || is_gdb_terminal (inferior_thisrun_terminal))
     return;
 #if !defined(__GO32__) && !defined(_WIN32)
   int tty;
@@ -862,13 +868,13 @@ new_tty_postfork (void)
   /* Save the name for later, for determining whether we and the child
      are sharing a tty.  */
 
-  if (inferior_thisrun_terminal)
-    {
-      struct inferior *inf = current_inferior ();
-      struct terminal_info *tinfo = get_inflow_inferior_data (inf);
+  struct inferior *inf = current_inferior ();
+  struct terminal_info *tinfo = get_inflow_inferior_data (inf);
 
-      tinfo->run_terminal = xstrdup (inferior_thisrun_terminal);
-    }
+  if (inferior_thisrun_terminal != nullptr)
+    tinfo->run_terminal = xstrdup (inferior_thisrun_terminal);
+  else
+    tinfo->run_terminal = xstrdup ("/dev/tty");
 
   inferior_thisrun_terminal = NULL;
 }
@@ -927,7 +933,9 @@ create_tty_session (void)
 #ifdef HAVE_SETSID
   pid_t ret;
 
-  if (!job_control || inferior_thisrun_terminal == 0)
+  if (!job_control
+      || inferior_thisrun_terminal == nullptr
+      || is_gdb_terminal (inferior_thisrun_terminal))
     return 0;
 
   ret = setsid ();
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 07/16] Make inferior/GDB share terminal in tests expecting output after detach
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (5 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 06/16] Special-case "set inferior-tty /dev/tty" Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 08/16] Make inferior/GDB share terminal in tests that exercise GDB/inferior reading same input Pedro Alves
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

A following patch will make GDB put spawned inferiors in their own
terminal/session.  A consequence of that is that if you do "run", and
then "detach", GDB closes the terminal, so expecting inferior output
after detach no longer works, the inferior is alive, but its output
goes nowhere.

There's only one testcase that expects output out of an inferior after
detach.  Tweak it to output to GDB's terminal instead.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.threads/process-dies-while-detaching.exp: Issue "tty
	/dev/tty" before starting program.

Change-Id: Ic62bca178295763fb9c47657ee459fe715f7865e
---
 gdb/testsuite/gdb.threads/process-dies-while-detaching.exp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
index ac1aad26ec5..f01d4cf0666 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
@@ -215,6 +215,7 @@ proc test_detach {multi_process cmd} {
 	global binfile
 
 	clean_restart ${binfile}
+	gdb_test_no_output "tty /dev/tty"
 
 	if ![runto_main] {
 	    fail "can't run to main"
@@ -240,6 +241,7 @@ proc test_detach_watch {multi_process cmd} {
 	global binfile decimal
 
 	clean_restart ${binfile}
+	gdb_test_no_output "tty /dev/tty"
 
 	if ![runto_main] {
 	    fail "can't run to main"
@@ -276,6 +278,7 @@ proc test_detach_killed_outside {multi_process cmd} {
 	global binfile
 
 	clean_restart ${binfile}
+	gdb_test_no_output "tty /dev/tty"
 
 	if ![runto_main] {
 	    fail "can't run to main"
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 08/16] Make inferior/GDB share terminal in tests that exercise GDB/inferior reading same input
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (6 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 07/16] Make inferior/GDB share terminal in tests expecting output after detach Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 09/16] gdb.mi/mi-logging.exp, don't send input to GDB while the inferior is running Pedro Alves
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

Some testcases exercise some aspect that only makes sense when GDB and
the inferior are sharing the same terminal, meaning GDB and the
inferior are reading from the same input file.

This commit makes sure that continues to be tested even after GDB
changed to put inferiors in their own session/terminal by default, by
issuing "tty /dev/tty".  The tests would fail otherwise.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.base/annota-input-while-running.exp: Issue "tty /dev/tty"
	before starting program.
	* gdb.base/continue-all-already-running.exp: Likewise.
	* gdb.base/infcall-input.exp: Likewise.

Change-Id: Ia5f9061bf28a5e780194aa75b37b6058de0614ee
---
 .../gdb.base/annota-input-while-running.exp          |  4 ++++
 .../gdb.base/continue-all-already-running.exp        |  3 +++
 gdb/testsuite/gdb.base/infcall-input.exp             | 12 +++++++++++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/annota-input-while-running.exp b/gdb/testsuite/gdb.base/annota-input-while-running.exp
index 389cb644517..912565806e8 100644
--- a/gdb/testsuite/gdb.base/annota-input-while-running.exp
+++ b/gdb/testsuite/gdb.base/annota-input-while-running.exp
@@ -22,6 +22,10 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug] == -1} {
     return -1
 }
 
+# This testcase only makes sense when GDB and the inferior are reading
+# from the same input file (aka sharing the terminal's input buffer).
+gdb_test_no_output "tty /dev/tty"
+
 # Because runto_main doesn't know how to handle the prompt with annotations,
 # run to main before we set the annotation level.
 if ![runto_main] then {
diff --git a/gdb/testsuite/gdb.base/continue-all-already-running.exp b/gdb/testsuite/gdb.base/continue-all-already-running.exp
index de84897c766..caff93ab425 100644
--- a/gdb/testsuite/gdb.base/continue-all-already-running.exp
+++ b/gdb/testsuite/gdb.base/continue-all-already-running.exp
@@ -20,6 +20,9 @@ standard_testfile
 
 save_vars { GDBFLAGS } {
     set GDBFLAGS "$GDBFLAGS -ex \"set non-stop on\""
+    # This test only makes sense when GDB and the inferior are reading
+    # from the same input file / sharing the terminal.
+    append GDBFLAGS " -ex \"tty /dev/tty\""
     if { [prepare_for_testing "failed to prepare" ${testfile} $srcfile] } {
 	return -1
     }
diff --git a/gdb/testsuite/gdb.base/infcall-input.exp b/gdb/testsuite/gdb.base/infcall-input.exp
index fec0eb2a4d8..87bd049e139 100644
--- a/gdb/testsuite/gdb.base/infcall-input.exp
+++ b/gdb/testsuite/gdb.base/infcall-input.exp
@@ -23,10 +23,20 @@ if [target_info exists gdb,cannot_call_functions] {
     continue
 }
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+if {[build_executable "failed to compile" $testfile $srcfile debug]} {
     return -1
 }
 
+save_vars { GDBFLAGS } {
+    # This test only makes sense when GDB and the inferior are reading
+    # from the same input file / sharing the terminal.  If we instead
+    # let GDB put the inferior in its own session, then while the
+    # inferior is running in the foreground, input would be redirected
+    # to the inferior, and GDB would never see that input.
+    append GDBFLAGS " -ex \"tty /dev/tty\""
+    clean_restart $binfile
+}
+
 if ![runto_main] then {
     fail "couldn't run to main"
     return -1
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 09/16] gdb.mi/mi-logging.exp, don't send input to GDB while the inferior is running
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (7 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 08/16] Make inferior/GDB share terminal in tests that exercise GDB/inferior reading same input Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 10/16] target_terminal::ours_for_output before printing signal received Pedro Alves
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

The gdb.mi/mi-logging.exp testcase sends a sequence of execution
commands to the GDB terminal while the inferior is running, like this:

 send_gdb "1002-exec-step\n"
 send_gdb "1003-exec-next\n"

expecting that GDB will consume the "1003-exec-next" intput after the
inferior stops for the 1002-exec-step.

That's a flawed assumption in general, because the inferior itself
could consume the "1003-exec-next" input while it is stepping.

When GDB puts the inferior in its own terminal, while the inferior is
running, GDB marshals input from its terminal to the inferior's
terminal.  The result is that input typed while the inferior is
running never goes to GDB, and so the test fails.

The previous patch addressed issues like this by making the inferior
and GDB share the same terminal for tests that really wanted to test
some aspect of a shared terminal.  For gdb.mi/mi-logging.exp though,
there's really no reason to send input while the program is running,
so the fix here is to stop it from doing so.

While debugging the testcase, I ran into the fact that it reuses the
same log file for more than one [open] sequence, overwriting previous
runs.  The testcase also deletes the log files at the very end, which
makes it impossible to inspect the logs after a failure run.  The
patch addresses those issues as well.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.mi/mi-logging.exp: Do not reuse log files for different
	runs.  Delete logs at the start of the testcase, not at the
	end.
	(wait_open, gdb_test_file): New procedures.  Use them.

Change-Id: Ife215a82391a020041fd05478bd8dbee6e04d607
---
 gdb/testsuite/gdb.mi/mi-logging.exp | 94 +++++++++++++++++++++++------
 1 file changed, 77 insertions(+), 17 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
index ff47b0849c0..d50eaee22c8 100644
--- a/gdb/testsuite/gdb.mi/mi-logging.exp
+++ b/gdb/testsuite/gdb.mi/mi-logging.exp
@@ -30,9 +30,13 @@ if {[mi_runto_main] < 0} {
     return -1
 }
 
-set milogfile [standard_output_file "milog.txt"]
+set milogfile1 [standard_output_file "milog1.txt"]
+set milogfile2 [standard_output_file "milog2.txt"]
 
-mi_gdb_test "-gdb-set logging file $milogfile" ".*"
+remote_file host delete $milogfile1
+remote_file host delete $milogfile2
+
+mi_gdb_test "-gdb-set logging file $milogfile1" ".*"
 
 mi_gdb_test "-gdb-set logging overwrite on" ".*"
 
@@ -44,7 +48,7 @@ mi_next "logged next"
 
 mi_gdb_test "-gdb-set logging off" ".*" "logging off"
 
-set chan [open $milogfile]
+set chan [open $milogfile1]
 set logcontent [read $chan]
 close $chan
 
@@ -58,26 +62,84 @@ if [regexp "\\^done\[\r\n\]+$mi_log_prompt\\^running\[\r\n\]+\\*running,thread-i
 
 # Now try the redirect, which writes into the file only.
 
-mi_gdb_test "-gdb-set logging redirect on" ".*" "redirect logging on"
+mi_gdb_test "-gdb-set logging file $milogfile2" ".*"
 
-# Since all output will be going into the file, just keep sending commands
-# and don't expect anything to appear until logging is turned off.
+mi_gdb_test "999-gdb-set logging redirect on" ".*" "redirect logging on"
 
 send_gdb "1001-gdb-set logging on\n"
+
+# We can't open the file right away, because we're not waiting for
+# GDB's "^done" -- we could end up opening the file before GDB creates
+# it.  So spin waiting until the file exists.
+
+proc wait_open {filename} {
+    set ticks [expr 10 * $::timeout]
+    set t 0
+
+    while {![file exists $filename]} {
+	after 100
+
+	incr t
+	if {$t == $ticks} {
+	    return ""
+	}
+    }
+    return [open $filename]
+}
+
+set chan [wait_open $milogfile2]
+
+# Read GDB output in channel/file CHAN, expect the result.  PATTERN is
+# the pattern to match for a PASS.
+
+proc gdb_test_file {chan pattern test} {
+    global timeout
+
+    verbose -log "gdb_test_file: begin"
+
+    set ticks [expr 10 * $timeout]
+    set t 0
+    set logcontent ""
+
+    while {1} {
+	set r [read $chan]
+	append logcontent $r
+
+	send_log -- "$r"
+
+	if [regexp "$pattern" $logcontent] {
+	    pass "$test"
+	    break
+	} else {
+	    incr t
+	    if {$t == $ticks} {
+		fail "$test (timeout)"
+		break
+	    }
+	    after 100
+	}
+    }
+
+    verbose -log "gdb_test_file: end"
+}
+
+gdb_test_file $chan \
+    "1001\\^done\[\r\n\]+$mi_log_prompt" \
+    "redirect log file contents, set logging on"
+
 send_gdb "1002-exec-step\n"
-send_gdb "1003-exec-next\n"
+gdb_test_file $chan \
+    "1002\\^running\[\r\n\]+\\*running,thread-id=\"all\"\[\r\n\]+$mi_log_prompt\\*stopped,reason=\"end-stepping-range\",.*\[\r\n\]+$mi_log_prompt" \
+    "redirect log file contents, exec-step"
 
-mi_gdb_test "1004-gdb-set logging off" ".*" "redirect logging off"
+send_gdb "1003-exec-next\n"
+gdb_test_file $chan \
+    "1003\\^running\[\r\n\]+\\*running,thread-id=\"all\"\[\r\n\]+$mi_log_prompt\\*stopped,reason=\"end-stepping-range\",.*\[\r\n\]+$mi_log_prompt" \
+    "redirect log file contents, exec-next"
 
-set chan [open $milogfile]
-set logcontent [read $chan]
 close $chan
 
-if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*running,thread-id=\"all\"\[\r\n\]+$mi_log_prompt\\*stopped,reason=\"end-stepping-range\",.*\[\r\n\]+$mi_log_prompt.*1003\\^running\[\r\n\]+\\*running,thread-id=\"all\"\[\r\n\]+$mi_log_prompt\\*stopped,reason=\"end-stepping-range\",.*\[\r\n\]+$mi_log_prompt" $logcontent] {
-    pass "redirect log file contents"
-} else {
-    fail "redirect log file contents"
-}
+mi_gdb_test "1004-gdb-set logging off" ".*" "redirect logging off"
 
 # Now try enabling a redirect while GDB is already logging.  This used
 # to crash GDB.
@@ -94,5 +156,3 @@ with_test_prefix "redirect while already logging" {
 }
 
 mi_gdb_exit
-
-remote_file host delete $milogfile
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 10/16] target_terminal::ours_for_output before printing signal received
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (8 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 09/16] gdb.mi/mi-logging.exp, don't send input to GDB while the inferior is running Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 11/16] Move scoped_ignore_sigttou to gdbsupport/ Pedro Alves
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

A following patch will make GDB put spawned inferiors in their own
terminal/session (on GNU/Linux).  In that case, GDB is in control of
when is the inferior's output flushed to the screen.  A sync point is
when target_terminal state goes from inferior -> ours/ours_for_output.

The gdb.multi/multi-term-settings.exp testcase exposed a bug where an
inferior output flush is missing, resulting in this regression:

Good:

 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=run-share: inf2_how=run-session: info inferiors
 continue
 Continuing.
 pid=1275538, count=0
 pid=1276069, count=0

 Thread 1.1 "multi-term-sett" received signal SIGTTOU, Stopped (tty output).
 [Switching to process 1275538]
 0x00007ffff7ecda14 in __tcsetattr (fd=0, optional_actions=0, termios_p=0x7fffffffd450) at ../sysdeps/unix/sysv/linux/tcsetattr.c:83
 83	../sysdeps/unix/sysv/linux/tcsetattr.c: No such file or directory.
 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=run-share: inf2_how=run-session: continue (expected SIGTTOU)
 Quit
 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=run-share: inf2_how=run-session: stop with control-c (Quit)

Bad:

 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=run-share: inf2_how=run-session: info inferiors
 continue
 Continuing.
 pid=1287638, count=0

 Thread 1.1 "multi-term-sett" received signal SIGTTOU, Stopped (tty output).
 pid=1287663, count=0                      <<<<<< HERE
 [Switching to process 1287638]
 0x00007ffff7ecda14 in __tcsetattr (fd=0, optional_actions=0, termios_p=0x7fffffffd450) at ../sysdeps/unix/sysv/linux/tcsetattr.c:83
 83	../sysdeps/unix/sysv/linux/tcsetattr.c: No such file or directory.
 (gdb) FAIL: gdb.multi/multi-term-settings.exp: inf1_how=run-share: inf2_how=run-session: continue
 Quit
 (gdb) PASS: gdb.multi/multi-term-settings.exp: inf1_how=run-share: inf2_how=run-session: stop with control-c (Quit)


Notice the "<<<<<< HERE" line in the "Bad" output above -- that is
inferior output being printed on the screen _after_ GDB says the
thread stopped...  That's obviously bogus, the output was printed by
the inferior before it was stopped.

The fix is to claim back the terminal for output before printing the
"signal received SIGTTOU" message.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* infrun.c (normal_stop): Call target_terminal::ours_for_output
	before calling signal_received observers.

Change-Id: Iea106c33b4c585562fc3579ca85d43481fa214f0
---
 gdb/infrun.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4bd21fde590..1eb7526d246 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8537,7 +8537,10 @@ normal_stop (void)
   update_thread_list ();
 
   if (last.kind == TARGET_WAITKIND_STOPPED && stopped_by_random_signal)
-    gdb::observers::signal_received.notify (inferior_thread ()->suspend.stop_signal);
+    {
+      target_terminal::ours_for_output ();
+      gdb::observers::signal_received.notify (inferior_thread ()->suspend.stop_signal);
+    }
 
   /* As with the notification of thread events, we want to delay
      notifying the user that we've switched thread context until
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 11/16] Move scoped_ignore_sigttou to gdbsupport/
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (9 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 10/16] target_terminal::ours_for_output before printing signal received Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-17 21:49   ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 12/16] Always put inferiors in their own terminal/session [gdb/9425, gdb/14559] Pedro Alves
                   ` (7 subsequent siblings)
  18 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

A following patch will want to use scoped_ignore_sigttou in code
shared between GDB and GDBserver.  Move it under gdbsupport/.

Note that despite what inflow.h/inflow.c's first line says, inflow.c
is no longer about ptrace, it is about terminal management.  Some
other files were unnecessarily including inflow.h, I guess a leftover
from the days when inflow.c really was about ptrace.  Those inclusions
are simply dropped.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* Makefile.in (HFILES_NO_SRCDIR): Remove inflow.h.
	* inf-ptrace.c, inflow.c, procfs.c: Don't include "inflow.h".
	* inflow.h: Delete, moved to gdbsupport/ under a different name.
	* ser-unix.c: Don't include "inflow.h".  Include
	"gdbsupport/scoped_ignore_sigttou.h".

gdbsupport/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* scoped_ignore_sigttou.h: New file, moved from gdb/ and renamed.

Change-Id: Ie390abf42c3a78bec6d282ad2a63edd3e623559a
---
 gdb/Makefile.in                                    | 1 -
 gdb/inf-ptrace.c                                   | 1 -
 gdb/inflow.c                                       | 2 +-
 gdb/procfs.c                                       | 1 -
 gdb/ser-unix.c                                     | 2 +-
 gdb/inflow.h => gdbsupport/scoped_ignore_sigttou.h | 8 ++++----
 6 files changed, 6 insertions(+), 9 deletions(-)
 rename gdb/inflow.h => gdbsupport/scoped_ignore_sigttou.h (90%)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index f664d964536..000d5f950c7 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1336,7 +1336,6 @@ HFILES_NO_SRCDIR = \
 	inf-ptrace.h \
 	infcall.h \
 	inferior.h \
-	inflow.h \
 	inline-frame.h \
 	interps.h \
 	jit.h \
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index b6fa71fd2c0..afa38de6ef7 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -20,7 +20,6 @@
 #include "defs.h"
 #include "command.h"
 #include "inferior.h"
-#include "inflow.h"
 #include "terminal.h"
 #include "gdbcore.h"
 #include "regcache.h"
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 15f29dcd08e..0a9df6054b0 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -29,12 +29,12 @@
 #include <fcntl.h>
 #include "gdbsupport/gdb_select.h"
 
-#include "inflow.h"
 #include "gdbcmd.h"
 #ifdef HAVE_TERMIOS_H
 #include <termios.h>
 #endif
 #include "gdbsupport/job-control.h"
+#include "gdbsupport/scoped_ignore_sigttou.h"
 
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 23c0aa22a7a..529ee33df90 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -40,7 +40,6 @@
 #include <signal.h>
 #include <ctype.h>
 #include "gdb_bfd.h"
-#include "inflow.h"
 #include "auxv.h"
 #include "procfs.h"
 #include "observable.h"
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index e97dc2f925d..96d024eea3d 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -32,7 +32,7 @@
 #include "gdbcmd.h"
 #include "gdbsupport/filestuff.h"
 #include <termios.h>
-#include "inflow.h"
+#include "gdbsupport/scoped_ignore_sigttou.h"
 
 struct hardwire_ttystate
   {
diff --git a/gdb/inflow.h b/gdbsupport/scoped_ignore_sigttou.h
similarity index 90%
rename from gdb/inflow.h
rename to gdbsupport/scoped_ignore_sigttou.h
index 8a671c7c4c7..a31316460b4 100644
--- a/gdb/inflow.h
+++ b/gdbsupport/scoped_ignore_sigttou.h
@@ -1,4 +1,4 @@
-/* Low level interface to ptrace, for GDB when running under Unix.
+/* Support for signoring SIGTTOU.
 
    Copyright (C) 2003-2021 Free Software Foundation, Inc.
 
@@ -17,8 +17,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifndef INFLOW_H
-#define INFLOW_H
+#ifndef SCOPED_IGNORE_SIGTTOU_H
+#define SCOPED_IGNORE_SIGTTOU_H
 
 #include <unistd.h>
 #include <signal.h>
@@ -53,4 +53,4 @@ class scoped_ignore_sigttou
 #endif
 };
 
-#endif /* inflow.h */
+#endif /* SCOPED_IGNORE_SIGTTOU_H */
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 12/16] Always put inferiors in their own terminal/session [gdb/9425, gdb/14559]
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (10 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 11/16] Move scoped_ignore_sigttou to gdbsupport/ Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 13/16] exists_non_stop_target: Avoid flushing frames Pedro Alves
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

Currently, on GNU/Linux, it is not possible to interrupt with Ctrl-C
programs that block or ignore SIGINT, with e.g., sigprocmask or
signal(SIGINT, SIG_IGN).  You type Ctrl-C, but nothing happens.
Similarly, if a program uses sigwait to wait for SIGINT, and the
program receives a SIGINT, the SIGINT is _not_ intercepted by ptrace,
it goes straight to the inferior.  These problems have been known for
years, and recorded in gdb/9425, gdb/14559.

This is a consequence of how GDB implements interrupting programs with
Ctrl-C -- when GDB spawns a new process, it makes the process use the
same terminal as GDB, and then makes the process's process group be
the foreground process group in GDB's terminal.  This means that when
the process is running in the foreground, after e.g. "continue", when
the user types Ctrl-C, the kernel sends a SIGINT to the foreground
process group, which is the inferior.  GDB then intercepts the SIGINT
via ptrace, the same way it intercepts any other signal, stops all the
other threads of the inferior if in all-stop, and presents the
"Program received SIGINT" stop to the user.

This patch paves the way to address gdb/9425, gdb/14559, by turning
Ctrl-C handling around such that the SIGINT always reaches GDB first,
not the inferior.  That is done by making GDB put inferiors in their
own terminal/session created by GDB.  I.e., GDB creates a
pseudo-terminal master/slave pair, makes the inferior run with the
slave as its terminal, and pumps output/input on the master end.
Because the inferior is run with its own session/terminal, GDB is free
to remain as the foreground process in its own terminal, which means
that the Ctrl-C SIGINT always reaches GDB first, instead of reaching
the inferior first, and then GDB reacting to the ptrace-intercepted
SIGINT.  Because GDB gets the SIGINT first, GDB is then free to
handle it by interrupting the program any way it sees fit.  A
following patch will then make GDB interrupt the program with SIGSTOP
instead of SIGINT, which always works even if the inferior
blocks/ignores SIGINT -- SIGSTOP can't be ignored.  (In the future GDB
may even switch to PTRACE_INTERRUPT, though that's a project of its
own.)

Having the inferior in its own terminal also means that GDB is in
control of when inferior output is flushed to the screen.  When
debugging with the CLI, this means that inferior output is now never
interpersed with GDB's output in an unreadable fashion.  This will
also allow fixing the problem of inferior output really messing up the
screen in the TUI, forcing users to Ctrl-L to refresh the screen.
This patch does not address the TUI part, but it shouldn't be too hard
-- I wrote a quick&dirty prototype patch doing that a few years back,
so I know it works.

Implementation wise, here's what is happening:

 - when GDB spawns an inferior, unless the user asked otherwise with
   "tty /dev/tty", GDB creates a pty pair, and makes the slave end the
   inferior's terminal.  Note that starting an inferior on a given
   terminal already exists, given the "tty" command.  GDB records the
   master and slave ends of the pty.

 - GDB registers that new terminal's master end on the event loop.
   When the master is written to, it means the inferior has written
   some output on its terminal.  The event loop wakes up and GDB
   flushes the inferior output to its own terminal / to the screen.

 - When target_terminal state is switched to "inferior", with
   target_tarminal::inferiors(), GDB registers the stdin file
   descriptor on the event loop with a callback that forwards input
   typed on GDB's terminal to the inferior's tty.

 - Similarly, when GDB receives a SIGWINCH signal, meaning GDB's
   terminal was resized, GDB resizes the inferior's terminal too.

 - GDB puts the inferior in its own session, and there's a "session
   leader" process between GDB and the inferior.  The latter is
   because session leaders have special properties, one of which is,
   if they exit, all progresses in the foreground process group in the
   session get a SIGHUP.  If the spawned inferior was the session
   leader itself, if you were debugging an inferior that forks and
   follow to the child, if the parent (the session leader) exits, then
   the child would get a SIGHUP.  Forking twice when launching an
   inferior, and making the first child be the session leader, and the
   second child the inferior avoids that problem.

 - When the inferior exits or is killed, GDB sends a SIGHUP to the
   session leader, waits for the leader to exit and then destroys the
   terminal.  The session leader's SIGHUP handler makes the session
   leader pgrp be the foreground process group and then exits.  This
   sequence is important comparing to just closing the terminal and
   letting the session leader terminate due to the SIGHUP the kernel
   sends, because when the session leader exits, all processes in the
   foreground process group get a SIGHUP, meaning that if the detached
   process was still in the foreground, it would get a SIGHUP, and
   likely die.

 - The gdb.multi/multi-term-settings.exp was adjusted to test for
   shared and not-shared terminal/session.  Without the change, we get
   failures:

    FAIL: gdb.multi/multi-term-settings.exp: inf1_how=run: inf2_how=run: continue (expected SIGTTOU)
    FAIL: gdb.multi/multi-term-settings.exp: inf1_how=run: inf2_how=run: stop with control-c (Quit)

Tested on GNU/Linux native, gdbserver and gdbserver + "maint target
set-non-stop on".  Also build-tested tested on mingw32-w64, Solaris
11, and OpenBSD.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/9425
	PR gdb/14559
	* fork-child.c (child_has_managed_tty_hook): New.
	* inf-ptrace.c (inf_ptrace_me): If we created a managed tty, raise
	SIGSTOP.
	(inf_ptrace_handle_session_leader_fork): New.
	(inf_ptrace_target::create_inferior): Pass it down as
	handle_session_leader_fork callback.
	* inf-ptrace.h (inf_ptrace_target) <handle_session_leader_fork>:
	New virtual method.
	* inferior.h (child_terminal_on_sigwinch): Declare.
	* inflow.c: Include "gdbsupport/event-loop.h",
	"gdbsupport/refcounted-object.h", "gdbsupport/gdb_wait.h",
	"gdbsupport/managed-tty.h".
	(USES_FORK_CHILD): Define, and wrap fork-child.c-related code with
	it.
	(struct run_terminal_info): New.
	(struct terminal_info) <run_terminal>: Now a run_terminal_info.
	<process_group>: Default to -1.
	<save_from_tty>: New method.
	(sigint_ours): Update comments.
	(inferior_thisrun_terminal_pty_fd): New.
	(input_fd_redirected): New.
	(sharing_input_terminal): Adjust.
	(gdb_tcgetattr, gdb_tcsetattr, make_raw, class scoped_raw_termios)
	(child_terminal_flush_from_to, child_terminal_flush_stdout)
	(inferior_stdout_event_handler, inferior_stdin_event_handler): New.
	(child_terminal_inferior): Handle inferiors with gdb-managed ttys.
	(child_terminal_save_inferior): Handle inferiors with gdb-managed
	ttys.  Use save_from_tty.
	(child_terminal_ours_1): Handle inferiors with gdb-managed ttys.
	(terminal_info::~terminal_info): Use delete instead of xfree.
	(child_terminal_on_sigwinc): New.
	(inflow_inferior_exit): Release terminal created by GDB.
	(copy_terminal_info): Assert there's no run_terminal yet in TO
	yet.  Incref run_terminal after copying.
	(child_terminal_info): Handle inferiors with gdb-managed ttys.
	(new_tty_prefork): Allocate pseudo-terminal.
	(created_managed_tty): New.
	(new_tty): Remove __GO32__ and _WIN32 #ifdefs, not needed given
	USES_FORK_CHILD.
	(new_tty_postfork): Handle inferiors with gdb-managed ttys.
	(show_debug_managed_tty): New.
	(_initialize_inflow): Register "set/show debug managed-tty".
	* linux-nat.c (waitpid_sigstop, waitpid_fork)
	(linux_nat_target::handle_session_leader_fork): New.
	* linux-nat.h (linux_nat_target) <handle_session_leader_fork>:
	Declare override.
	* nat/fork-inferior.c: Include
	"gdbsupport/scoped_ignore_sigttou.h", "gdbsupport/managed-tty.h",
	<sys/types.h> and <sys/wait.h>.
	(session_leader_hup): New.
	(fork_inferior): Add handle_session_leader_fork parameter.  If the
	inferior has a gdb-managed tty, don't use vfork, and fork twice,
	with the first fork becoming the session leader.  Call
	handle_session_leader_fork.
	* nat/fork-inferior.h (fork_inferior): Add
	handle_session_leader_fork parameter and update comment.
	(child_has_managed_tty_hook): Declare.
	* terminal.h (created_managed_tty, child_gdb_owns_session):
	Declare.
	* tui/tui-win.c: Include "inferior.h".
	(tui_async_resize_screen): Call child_terminal_on_sigwinch.

gdbsupport/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/9425
	PR gdb/14559
	* Makefile.am (libgdbsupport_a_SOURCES): Add managed-tty.cc.
	* Makefile.in: Regenerate.
	* managed-tty.cc: New.
	* managed-tty.h: New.

gdbserver/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/9425
	PR gdb/14559
	* fork-child.cc (child_has_managed_tty_hook): New.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/9425
	PR gdb/14559
	* gdb.multi/multi-term-settings.exp (create_inferior): Document
	"run-session", "run-share" and "run-tty" instead of "run" and
	"tty".  Adjust to handle "run-session" vs "run-share".
	(coretest): Adjust to handle "run-session" vs "run-share".
	(how_modes): Use "run-session", "run-share" and "run-tty" instead
	of "run" and "tty".

Change-Id: I2569e189294044891e68a66401b381e4b999b19c
---
 gdb/fork-child.c                              |   8 +
 gdb/inf-ptrace.c                              |  23 +-
 gdb/inf-ptrace.h                              |   8 +
 gdb/inferior.h                                |   4 +
 gdb/inflow.c                                  | 699 +++++++++++++++---
 gdb/linux-nat.c                               |  76 ++
 gdb/linux-nat.h                               |   2 +
 gdb/nat/fork-inferior.c                       | 118 ++-
 gdb/nat/fork-inferior.h                       |  23 +-
 gdb/terminal.h                                |   3 +
 .../gdb.multi/multi-term-settings.exp         |  50 +-
 gdb/tui/tui-win.c                             |   3 +
 gdbserver/fork-child.cc                       |   8 +
 gdbsupport/Makefile.am                        |   1 +
 gdbsupport/Makefile.in                        |  15 +-
 gdbsupport/managed-tty.cc                     |  22 +
 gdbsupport/managed-tty.h                      |  42 ++
 17 files changed, 987 insertions(+), 118 deletions(-)
 create mode 100644 gdbsupport/managed-tty.cc
 create mode 100644 gdbsupport/managed-tty.h

diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index 41135b53f9b..10395718ed9 100644
--- a/gdb/fork-child.c
+++ b/gdb/fork-child.c
@@ -73,6 +73,14 @@ prefork_hook ()
 
 /* See nat/fork-inferior.h.  */
 
+bool
+child_has_managed_tty_hook ()
+{
+  return created_managed_tty ();
+}
+
+/* See nat/fork-inferior.h.  */
+
 void
 postfork_hook (pid_t pid)
 {
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index afa38de6ef7..5064cb39e65 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -61,6 +61,26 @@ inf_ptrace_me (void)
   /* "Trace me, Dr. Memory!"  */
   if (ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3) 0, 0) < 0)
     trace_start_error_with_name ("ptrace");
+  if (created_managed_tty ())
+    {
+      /* We're about to fork again, so that this child remains as
+	 session leader, and the grandchild becomes the real inferior.
+	 Let GDB grab control of this child, and enable tracing the
+	 grandchild fork.  */
+      raise (SIGSTOP);
+    }
+}
+
+/* fork_inferior handle_session_leader_fork hook.  Dispatches to
+   inf_ptrace_target.  */
+
+static pid_t
+inf_ptrace_handle_session_leader_fork (pid_t sl_pid)
+{
+  auto *proc_target = current_inferior ()->process_target ();
+  auto *ptrace_targ = static_cast<inf_ptrace_target *> (proc_target);
+
+  return ptrace_targ->handle_session_leader_fork (sl_pid);
 }
 
 /* Start a new inferior Unix child process.  EXEC_FILE is the file to
@@ -88,7 +108,8 @@ inf_ptrace_target::create_inferior (const char *exec_file,
     }
 
   pid_t pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL,
-			     NULL, NULL, NULL);
+			     NULL, NULL, NULL,
+			     inf_ptrace_handle_session_leader_fork);
 
   ptid_t ptid (pid);
   /* We have something that executes now.  We'll be running through
diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h
index 8aded9b60db..f5291ea0504 100644
--- a/gdb/inf-ptrace.h
+++ b/gdb/inf-ptrace.h
@@ -44,6 +44,14 @@ struct inf_ptrace_target : public inf_child_target
   void create_inferior (const char *, const std::string &,
 			char **, int) override;
 
+  /* Targets that support putting the inferior in its own gdb-managed
+     terminal must override this method.  */
+  virtual pid_t handle_session_leader_fork (pid_t sl_pid)
+  {
+    gdb_assert_not_reached ("handle_session_leader_fork called");
+    return -1;
+  }
+
   void mourn_inferior () override;
 
   bool thread_alive (ptid_t ptid) override;
diff --git a/gdb/inferior.h b/gdb/inferior.h
index f61b5889e85..b75c587e397 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -182,6 +182,10 @@ extern void child_pass_ctrlc (struct target_ops *self);
 
 extern void child_interrupt (struct target_ops *self);
 
+/* Called when we get a SIGWINCH, to manage the sizes of inferior
+   terminals created by GDB.  */
+extern void child_terminal_on_sigwinch ();
+
 /* From fork-child.c */
 
 /* Helper function to call STARTUP_INFERIOR with PID and NUM_TRAPS.
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 0a9df6054b0..35a51b26fb5 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -35,6 +35,10 @@
 #endif
 #include "gdbsupport/job-control.h"
 #include "gdbsupport/scoped_ignore_sigttou.h"
+#include "gdbsupport/event-loop.h"
+#include "gdbsupport/refcounted-object.h"
+#include "gdbsupport/gdb_wait.h"
+#include "gdbsupport/managed-tty.h"
 
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
@@ -44,6 +48,14 @@
 #define O_NOCTTY 0
 #endif
 
+/* Defined as 1 if the native target uses fork-child.c to spawn
+   processes.  */
+#if !defined(__GO32__) && !defined(_WIN32)
+# define USES_FORK_CHILD 1
+#else
+# define USES_FORK_CHILD 0
+#endif
+
 static void pass_signal (int);
 
 static void child_terminal_ours_1 (target_terminal_state);
@@ -52,6 +64,25 @@ static void child_terminal_ours_1 (target_terminal_state);
 
 static struct serial *stdin_serial;
 
+/* "run terminal" terminal info.  This is info about the terminal we
+   give to the inferior when it is started.  This is refcounted
+   because it is potentially shared between multiple inferiors -- a
+   fork child is associated with the same terminal as its parent.  */
+
+struct run_terminal_info : public refcounted_object
+{
+  /* The name of the tty (from the `tty' command) that we gave to the
+     inferior when it was started.  */
+  gdb::unique_xmalloc_ptr<char> ttyname;
+
+  /* The file descriptor of the master end of the pty created for the
+     inferior.  -1 if no terminal was created by GDB.  */
+  int pty_fd = -1;
+
+  /* The PID of the terminal's session leader.  */
+  pid_t session_leader = -1;
+};
+
 /* Terminal related info we need to keep track of.  Each inferior
    holds an instance of this structure --- we save it whenever the
    corresponding inferior stops, and restore it to the terminal when
@@ -63,9 +94,10 @@ struct terminal_info
 
   terminal_info &operator= (const terminal_info &) = default;
 
-  /* The name of the tty (from the `tty' command) that we gave to the
-     inferior when it was started.  */
-  char *run_terminal = nullptr;
+  /* Info about the tty that we gave to the inferior when it was
+     started.  This is potentially shared between multiple
+     inferiors.  */
+  run_terminal_info *run_terminal = nullptr;
 
   /* TTY state.  We save it whenever the inferior stops, and restore
      it when it resumes in the foreground.  */
@@ -85,11 +117,26 @@ struct terminal_info
      inf2's pgrp in the foreground instead of inf1's (which would be
      problematic since it would be left stopped: Ctrl-C wouldn't work,
      for example).  */
-  pid_t process_group = 0;
+  pid_t process_group = -1;
 #endif
 
   /* fcntl flags.  Saved and restored just like ttystate.  */
   int tflags = 0;
+
+  /* Save terminal settings from TTY_SERIAL.  */
+  void save_from_tty (serial *tty_serial)
+  {
+    xfree (this->ttystate);
+    this->ttystate = serial_get_tty_state (tty_serial);
+
+#ifdef HAVE_TERMIOS_H
+    this->process_group = tcgetpgrp (tty_serial->fd);
+#endif
+
+#ifdef F_GETFL
+    this->tflags = fcntl (tty_serial->fd, F_GETFL, 0);
+#endif
+  }
 };
 
 /* Our own tty state, which we restore every time we need to deal with
@@ -108,9 +155,10 @@ static serial_ttystate initial_gdb_ttystate;
 
 static struct terminal_info *get_inflow_inferior_data (struct inferior *);
 
-/* While the inferior is running, we want SIGINT and SIGQUIT to go to the
-   inferior only.  If we have job control, that takes care of it.  If not,
-   we save our handlers in these two variables and set SIGINT and SIGQUIT
+/* While the inferior is running, and the inferior is sharing the same
+   terminal as GDB, we want SIGINT and SIGQUIT to go to the inferior
+   only.  If we have job control, that takes care of it.  If not, we
+   save our handlers in these two variables and set SIGINT and SIGQUIT
    to SIG_IGN.  */
 
 static sighandler_t sigint_ours;
@@ -118,6 +166,8 @@ static sighandler_t sigint_ours;
 static sighandler_t sigquit_ours;
 #endif
 
+#if USES_FORK_CHILD
+
 /* The name of the tty (from the `tty' command) that we're giving to
    the inferior when starting it up.  This is only (and should only
    be) used as a transient global by new_tty_prefork,
@@ -125,6 +175,18 @@ static sighandler_t sigquit_ours;
    fork_inferior, while forking a new child.  */
 static const char *inferior_thisrun_terminal;
 
+#if GDB_MANAGED_TERMINALS
+
+/* The file descriptor of the master end of the pty that we're giving
+   to the inferior when starting it up, iff we created the terminal
+   ourselves.  This is set by new_tty_prefork, and like
+   INFERIOR_THISRUN_TERMINAL, is transient.  */
+static int inferior_thisrun_terminal_pty_fd = -1;
+
+#endif /* GDB_MANAGED_TERMINALS */
+
+#endif /* USES_FORK_CHILD */
+
 /* Track who owns GDB's terminal (is it GDB or some inferior?).  While
    target_terminal::is_ours() etc. tracks the core's intention and is
    independent of the target backend, this tracks the actual state of
@@ -137,6 +199,11 @@ static const char *inferior_thisrun_terminal;
    different terminal).  */
 static target_terminal_state gdb_tty_state = target_terminal_state::is_ours;
 
+/* True if stdin is redirected.  As long as this is true, any input
+   typed in GDB's terminal is forwarded to the foreground inferior's
+   gdb-managed terminal.  See inferior_stdin_event_handler.  */
+static bool input_fd_redirected = false;
+
 /* See terminal.h.  */
 
 void
@@ -309,7 +376,7 @@ sharing_input_terminal (inferior *inf)
 	 positive we just end up trying to save/restore terminal
 	 settings when we didn't need to or we actually can't.  */
       if (tinfo->run_terminal != NULL)
-	res = is_gdb_terminal (tinfo->run_terminal);
+	res = is_gdb_terminal (tinfo->run_terminal->ttyname.get ());
 
       /* If we still can't determine, assume yes.  */
       if (res == TRIBOOL_UNKNOWN)
@@ -319,6 +386,199 @@ sharing_input_terminal (inferior *inf)
   return res == TRIBOOL_TRUE;
 }
 
+#if GDB_MANAGED_TERMINALS
+
+/* Wrappers around tcgetattr/tcsetattr to log errors.  We don't throw
+   on error instead because an error here is most likely caused by
+   stdin having been closed (e.g., GDB lost its terminal), and we may
+   be called while handling/printing exceptions.  E.g., from
+   target_target::ours_for_output() before printing an exception
+   message.  */
+
+static int
+gdb_tcgetattr (int fd, struct termios *termios)
+{
+  if (tcgetattr (fd, termios) != 0)
+    {
+      managed_tty_debug_printf (_("tcgetattr(fd=%d) failed: %d (%s)\n"),
+				fd, errno, safe_strerror (errno));
+      return -1;
+    }
+
+  return 0;
+}
+
+/* See gdb_tcgetattr.  */
+
+static int
+gdb_tcsetattr (int fd, int optional_actions, struct termios *termios)
+{
+  if (tcsetattr (fd, optional_actions, termios) != 0)
+    {
+      managed_tty_debug_printf (_("tcsetattr(fd=%d) failed: %d (%s)\n"),
+				fd, errno, safe_strerror (errno));
+      return -1;
+    }
+
+  return 0;
+}
+
+/* Disable echo, canonical mode, and \r\n -> \n translation.  Leave
+   ISIG, since we want to grab Ctrl-C before the inferior sees it.  If
+   CLEAR_OFLAG is true, also clear the output modes, otherwise, leave
+   them unmodified.  */
+
+static void
+make_raw (struct termios *termios, bool clear_oflag)
+{
+  termios->c_iflag &= ~(INLCR | IGNCR | ICRNL);
+  if (clear_oflag)
+    termios->c_oflag = 0;
+  termios->c_lflag &= ~(ECHO | ICANON);
+  termios->c_cflag &= ~CSIZE;
+  termios->c_cflag |= CLOCAL | CS8;
+  termios->c_cc[VMIN] = 0;
+  termios->c_cc[VTIME] = 0;
+}
+
+/* RAII class to temporarily set the terminal to raw mode, with
+   `oflag` cleared.  See make_raw.  */
+
+class scoped_raw_termios
+{
+public:
+  scoped_raw_termios ()
+  {
+    if (gdb_tcgetattr (STDIN_FILENO, &m_saved_termios) == 0)
+      {
+	m_saved_termios_p = true;
+
+	struct termios raw_termios = m_saved_termios;
+	make_raw (&raw_termios, true);
+	gdb_tcsetattr (STDIN_FILENO, TCSADRAIN, &raw_termios);
+      }
+  }
+
+  ~scoped_raw_termios ()
+  {
+    if (m_saved_termios_p)
+      gdb_tcsetattr (STDIN_FILENO, TCSADRAIN, &m_saved_termios);
+  }
+
+private:
+  /* The saved termios data.  */
+  struct termios m_saved_termios;
+
+  /* True iff M_SAVED_TERMIOS is valid.  */
+  bool m_saved_termios_p;
+};
+
+/* Flush input/output from READ_FD to WRITE_FD.  WHAT is used for
+   logging purposes.  */
+
+static void
+child_terminal_flush_from_to (int read_fd, int write_fd, const char *what)
+{
+  char buf[1024];
+
+  gdb::optional<scoped_raw_termios> save_termios;
+  save_termios.emplace ();
+
+  while (1)
+    {
+      int r = read (read_fd, buf, sizeof (buf));
+      if (r <= 0)
+	{
+	  /* Restore terminal state before printing debug output or
+	     warnings.  */
+	  save_termios.reset ();
+
+	  if (r == 0)
+	    ;
+	  else if (r == -1 && errno == EAGAIN)
+	    ;
+	  else if (r == -1 && errno == EIO)
+	    {
+	      managed_tty_debug_printf (_("%s: bad read: closed?\n"),
+					what);
+	    }
+	  else
+	    {
+	      /* Unexpected.  */
+	      warning (_("%s: bad read: %d: (%d) %s"), what, r,
+		       errno, safe_strerror (errno));
+	    }
+	  return;
+	}
+
+      const char *p = buf;
+
+      while (r > 0)
+	{
+	  int w = write (write_fd, p, r);
+	  if (w == -1 && errno == EAGAIN)
+	    continue;
+	  else if (w <= 0)
+	    {
+	      int err = errno;
+
+	      /* Restore terminal state before printing the
+		 warning.  */
+	      save_termios.reset ();
+
+	      warning (_("%s: bad write: %d: (%d) %s"), what, r,
+		       err, safe_strerror (err));
+	      return;
+	    }
+
+	  r -= w;
+	  p += w;
+	}
+    }
+}
+
+/* Flush inferior terminal output to GDB's stdout.  Used when the
+   inferior is associated with a terminal created and managed by
+   GDB.  */
+
+static void
+child_terminal_flush_stdout (run_terminal_info *run_terminal)
+{
+  gdb_assert (run_terminal->pty_fd != -1);
+  child_terminal_flush_from_to (run_terminal->pty_fd, STDOUT_FILENO,
+				"stdout");
+}
+
+/* Event handler associated with the inferior's terminal pty.  Used
+   when the inferior is associated with a terminal created and managed
+   by GDB.  Whenever the inferior writes to its terminal, the event
+   loop calls this handler, which then flushes inferior terminal
+   output to GDB's stdout.  */
+
+static void
+inferior_stdout_event_handler (int error, gdb_client_data client_data)
+{
+  run_terminal_info *run_terminal = (run_terminal_info *) client_data;
+  child_terminal_flush_stdout (run_terminal);
+}
+
+/* Event handler associated with stdin.  Used when the inferior is
+   associated with a terminal created and managed by GDB.  Whenever
+   the user types on GDB's terminal, the event loop calls this
+   handler, which then flushes user input to the inferior's terminal
+   input.  */
+
+static void
+inferior_stdin_event_handler (int error, gdb_client_data client_data)
+{
+  run_terminal_info *run_terminal = (run_terminal_info *) client_data;
+  gdb_assert (run_terminal->pty_fd != -1);
+  child_terminal_flush_from_to (STDIN_FILENO, run_terminal->pty_fd,
+				"stdin");
+}
+
+#endif /* GDB_MANAGED_TERMINALS */
+
 /* Put the inferior's terminal settings into effect.  This is
    preparation for starting or resuming the inferior.  */
 
@@ -338,63 +598,98 @@ child_terminal_inferior (struct target_ops *self)
   terminal_info *tinfo = get_inflow_inferior_data (inf);
 
   if (gdb_has_a_terminal ()
-      && tinfo->ttystate != NULL
-      && sharing_input_terminal (inf))
+      && tinfo->ttystate != nullptr
+      && ((tinfo->run_terminal != nullptr
+	   && tinfo->run_terminal->pty_fd != -1)
+	  || sharing_input_terminal (inf)))
     {
-      int result;
+      if (!job_control)
+	{
+	  sigint_ours = signal (SIGINT, SIG_IGN);
+#ifdef SIGQUIT
+	  sigquit_ours = signal (SIGQUIT, SIG_IGN);
+#endif
+	}
 
       /* Ignore SIGTTOU since it will happen when we try to set the
 	 terminal's state (if gdb_tty_state is currently
 	 ours_for_output).  */
       scoped_ignore_sigttou ignore_sigttou;
 
-#ifdef F_GETFL
-      result = fcntl (0, F_SETFL, tinfo->tflags);
-      OOPSY ("fcntl F_SETFL");
-#endif
+#if GDB_MANAGED_TERMINALS
+      if (tinfo->run_terminal != nullptr
+	  && tinfo->run_terminal->pty_fd != -1)
+	{
+	  /* Set stdin to raw (see make_raw) so we can later marshal
+	     unadulterated input to the inferior's terminal, but leave
+	     the output flags intact.  Importantly, we don't want to
+	     disable \n -> \r\n translation on output, mainly to avoid
+	     the staircase effect in debug logging all over the code
+	     base while terminal_inferior is in effect.  */
+	  struct termios termios;
+
+	  if (gdb_tcgetattr (STDIN_FILENO, &termios) == 0)
+	    {
+	      make_raw (&termios, false);
+	      gdb_tcsetattr (STDIN_FILENO, TCSADRAIN, &termios);
+	    }
 
-      result = serial_set_tty_state (stdin_serial, tinfo->ttystate);
-      OOPSY ("setting tty state");
+	  /* Register our stdin-forwarder handler in the event
+	     loop.  */
+	  add_file_handler (0, inferior_stdin_event_handler,
+			    tinfo->run_terminal,
+			    string_printf ("stdin-forward-%d", inf->num),
+			    true);
 
-      if (!job_control)
+	  input_fd_redirected = true;
+	}
+      else
+#endif /* GDB_MANAGED_TERMINALS */
 	{
-	  sigint_ours = signal (SIGINT, SIG_IGN);
-#ifdef SIGQUIT
-	  sigquit_ours = signal (SIGQUIT, SIG_IGN);
+	  int result;
+
+#ifdef F_GETFL
+	  result = fcntl (0, F_SETFL, tinfo->tflags);
+	  OOPSY ("fcntl F_SETFL");
 #endif
-	}
 
-      if (job_control)
-	{
+	  result = serial_set_tty_state (stdin_serial, tinfo->ttystate);
+	  OOPSY ("setting tty state");
+
+	  if (job_control)
+	    {
 #ifdef HAVE_TERMIOS_H
-	  /* If we can't tell the inferior's actual process group,
-	     then restore whatever was the foreground pgrp the last
-	     time the inferior was running.  See also comments
-	     describing terminal_state::process_group.  */
+	      /* If we can't tell the inferior's actual process group,
+		 then restore whatever was the foreground pgrp the
+		 last time the inferior was running.  See also
+		 comments describing
+		 terminal_state::process_group.  */
 #ifdef HAVE_GETPGID
-	  result = tcsetpgrp (0, getpgid (inf->pid));
+	      result = tcsetpgrp (0, getpgid (inf->pid));
 #else
-	  result = tcsetpgrp (0, tinfo->process_group);
+	      result = tcsetpgrp (0, tinfo->process_group);
 #endif
-	  if (result == -1)
-	    {
+	      if (result == -1)
+		{
 #if 0
-	      /* This fails if either GDB has no controlling terminal,
-		 e.g., running under 'setsid(1)', or if the inferior
-		 is not attached to GDB's controlling terminal.  E.g.,
-		 if it called setsid to create a new session or used
-		 the TIOCNOTTY ioctl, or simply if we've attached to a
-		 process running on another terminal and we couldn't
-		 tell whether it was sharing GDB's terminal (and so
-		 assumed yes).  */
-	      fprintf_unfiltered
-		(gdb_stderr,
-		 "[tcsetpgrp failed in child_terminal_inferior: %s]\n",
-		 safe_strerror (errno));
+		  /* This fails if either GDB has no controlling
+		     terminal, e.g., running under 'setsid(1)', or if
+		     the inferior is not attached to GDB's controlling
+		     terminal.  E.g., if it called setsid to create a
+		     new session or used the TIOCNOTTY ioctl, or
+		     simply if we've attached to a process running on
+		     another terminal and we couldn't tell whether it
+		     was sharing GDB's terminal (and so assumed
+		     yes).  */
+		  fprintf_unfiltered
+		    (gdb_stderr,
+		     "[tcsetpgrp failed in child_terminal_inferior: %s]\n",
+		     safe_strerror (errno));
 #endif
-	    }
+		}
 #endif
-	}
+	    }
+	  }
 
       gdb_tty_state = target_terminal_state::is_inferior;
     }
@@ -444,21 +739,27 @@ child_terminal_save_inferior (struct target_ops *self)
   inferior *inf = current_inferior ();
   terminal_info *tinfo = get_inflow_inferior_data (inf);
 
+#if GDB_MANAGED_TERMINALS
+  run_terminal_info *run_terminal = tinfo->run_terminal;
+  if (run_terminal != nullptr && run_terminal->pty_fd != -1)
+    {
+      /* The inferior has its own terminal, so there are no settings
+	 to save.  However, do flush inferior output -- usually we'll
+	 be grabbing the terminal in reaction to an inferior stop, and
+	 it's only logical to print inferior output before we announce
+	 the stop, since the inferior printed it before it
+	 stopped.  */
+      child_terminal_flush_stdout (run_terminal);
+      return;
+    }
+#endif /* GDB_MANAGED_TERMINALS */
+
   /* No need to save/restore if the inferior is not sharing GDB's
      tty.  */
   if (!sharing_input_terminal (inf))
     return;
 
-  xfree (tinfo->ttystate);
-  tinfo->ttystate = serial_get_tty_state (stdin_serial);
-
-#ifdef HAVE_TERMIOS_H
-  tinfo->process_group = tcgetpgrp (0);
-#endif
-
-#ifdef F_GETFL
-  tinfo->tflags = fcntl (0, F_GETFL, 0);
-#endif
+  tinfo->save_from_tty (stdin_serial);
 }
 
 /* Switch terminal state to DESIRED_STATE, either is_ours, or
@@ -481,27 +782,37 @@ child_terminal_ours_1 (target_terminal_state desired_state)
 	 terminal's pgrp.  */
       scoped_ignore_sigttou ignore_sigttou;
 
-      /* Set tty state to our_ttystate.  */
       serial_set_tty_state (stdin_serial, our_terminal_info.ttystate);
 
-      /* If we only want output, then leave the inferior's pgrp in the
-	 foreground, so that Ctrl-C/Ctrl-Z reach the inferior
-	 directly.  */
+      /* If we only want output, then:
+	  - if the inferior is sharing GDB's session, leave the
+	    inferior's pgrp in the foreground, so that Ctrl-C/Ctrl-Z
+	    reach the inferior directly.
+	  - if the inferior has its own session, leave stdin
+            forwarding to the inferior.  */
       if (job_control && desired_state == target_terminal_state::is_ours)
 	{
+	  if (input_fd_redirected)
+	    {
+	      delete_file_handler (0);
+	      input_fd_redirected = false;
+	    }
+	  else
+	    {
 #ifdef HAVE_TERMIOS_H
-	  result = tcsetpgrp (0, our_terminal_info.process_group);
+	      result = tcsetpgrp (0, our_terminal_info.process_group);
 #if 0
-	  /* This fails on Ultrix with EINVAL if you run the testsuite
-	     in the background with nohup, and then log out.  GDB never
-	     used to check for an error here, so perhaps there are other
-	     such situations as well.  */
-	  if (result == -1)
-	    fprintf_unfiltered (gdb_stderr,
-				"[tcsetpgrp failed in child_terminal_ours: %s]\n",
-				safe_strerror (errno));
+	      /* This fails on Ultrix with EINVAL if you run the
+		 testsuite in the background with nohup, and then log
+		 out.  GDB never used to check for an error here, so
+		 perhaps there are other such situations as well.  */
+	      if (result == -1)
+		fprintf_unfiltered (gdb_stderr,
+				    "[tcsetpgrp failed in child_terminal_ours: %s]\n",
+				    safe_strerror (errno));
 #endif
 #endif /* termios */
+	    }
 	}
 
       if (!job_control && desired_state == target_terminal_state::is_ours)
@@ -608,7 +919,7 @@ static const struct inferior_key<terminal_info> inflow_inferior_data;
 
 terminal_info::~terminal_info ()
 {
-  xfree (run_terminal);
+  delete run_terminal;
   xfree (ttystate);
 }
 
@@ -627,17 +938,108 @@ get_inflow_inferior_data (struct inferior *inf)
   return info;
 }
 
-/* This is a "inferior_exit" observer.  Releases the TERMINAL_INFO member
-   of the inferior structure.  This field is private to inflow.c, and
-   its type is opaque to the rest of GDB.  PID is the target pid of
-   the inferior that is about to be removed from the inferior
-   list.  */
+#ifdef TIOCGWINSZ
+
+/* See inferior.h.  */
+
+void
+child_terminal_on_sigwinch ()
+{
+  struct winsize size;
+
+  if (ioctl (0, TIOCGWINSZ, &size) == -1)
+    return;
+
+  /* For each inferior that is connected to a terminal that we
+     created, resize the inferior's terminal to match GDB's.  */
+  for (inferior *inf : all_inferiors ())
+    {
+      terminal_info *info = inflow_inferior_data.get (inf);
+      if (info != nullptr
+	  && info->run_terminal != nullptr
+	  && info->run_terminal->pty_fd != -1)
+	ioctl (info->run_terminal->pty_fd, TIOCSWINSZ, &size);
+    }
+}
+
+#endif
+
+/* This is an "inferior_exit" observer.  Releases the TERMINAL_INFO
+   member of the inferior structure.  This field is private to
+   inflow.c, and its type is opaque to the rest of GDB.  If INF's
+   terminal is a GDB-managed terminal, then destroy it.  */
 
 static void
 inflow_inferior_exit (struct inferior *inf)
 {
   inf->terminal_state = target_terminal_state::is_ours;
-  inflow_inferior_data.clear (inf);
+
+  terminal_info *info = inflow_inferior_data.get (inf);
+  if (info != nullptr)
+    {
+      /* Release the terminal created by GDB, if there's one.  This
+	 closes the session leader process.  */
+      if (info->run_terminal != nullptr)
+	{
+	  run_terminal_info *run_terminal = info->run_terminal;
+
+	  /* The terminal may be used by other processes (e.g., if the
+	     inferior forked).  Only actually destroy the terminal if
+	     the refcount reaches 0.  */
+	  run_terminal->decref ();
+	  if (run_terminal->refcount () == 0)
+	    {
+#if GDB_MANAGED_TERMINALS
+	      if (run_terminal->pty_fd != -1)
+		{
+		  /* Flush any pending output and close the pty.  */
+		  delete_file_handler (run_terminal->pty_fd);
+		  child_terminal_flush_stdout (run_terminal);
+
+		  /* Explicitly send a SIGHUP instead of just closing
+		     the terminal and letting the kernel send it,
+		     because we want the session leader to have a
+		     chance to put itself in the foreground, so that
+		     its children, if any (e.g., we're detaching),
+		     don't get a SIGHUP too.  */
+		  kill (run_terminal->session_leader, SIGHUP);
+
+		  /* The session leader should exit in reaction to
+		     SIGHUP.  */
+		  managed_tty_debug_printf (_("reaping session leader "
+					      "for inf %d (sid=%d)\n"),
+					    inf->num,
+					    (int) run_terminal->session_leader);
+
+		  int status;
+		  int res = waitpid (run_terminal->session_leader,
+				     &status, 0);
+		  if (res == -1)
+		    warning (_("unexpected waitstatus "
+			       "reaping session leader for inf %d (sid=%d): "
+			       "res=-1, errno=%d (%s)"),
+			     inf->num, (int) run_terminal->session_leader,
+			     errno, safe_strerror (errno));
+		  else if (res != run_terminal->session_leader
+			   || !WIFEXITED (status)
+			   || WEXITSTATUS (status) != 0)
+		    warning (_("unexpected waitstatus "
+			       "reaping session leader for inf %d (sid=%d): "
+			       "res=%d, status=0x%x"),
+			     inf->num, (int) run_terminal->session_leader,
+			     res, status);
+
+		  /* We can now close the terminal.  */
+		  close (run_terminal->pty_fd);
+		}
+#endif /* GDB_MANAGED_TERMINALS */
+	      delete run_terminal;
+	    }
+	  info->run_terminal = nullptr;
+	}
+
+      inflow_inferior_data.clear (inf);
+    }
 }
 
 void
@@ -648,14 +1050,12 @@ copy_terminal_info (struct inferior *to, struct inferior *from)
   tinfo_to = get_inflow_inferior_data (to);
   tinfo_from = get_inflow_inferior_data (from);
 
-  xfree (tinfo_to->run_terminal);
+  gdb_assert (tinfo_to->run_terminal == nullptr);
   xfree (tinfo_to->ttystate);
 
   *tinfo_to = *tinfo_from;
-
-  if (tinfo_from->run_terminal)
-    tinfo_to->run_terminal
-      = xstrdup (tinfo_from->run_terminal);
+  if (tinfo_from->run_terminal != nullptr)
+    tinfo_from->run_terminal->incref ();
 
   if (tinfo_from->ttystate)
     tinfo_to->ttystate
@@ -702,6 +1102,25 @@ child_terminal_info (struct target_ops *self, const char *args, int from_tty)
   inf = current_inferior ();
   tinfo = get_inflow_inferior_data (inf);
 
+  /* child_terminal_save_inferior doesn't bother with saving terminal
+     settings if the inferior isn't sharing the terminal with GDB, so
+     refresh them now.  Note that if the inferior _is_ sharing a
+     terminal with GDB, then we must not refresh settings now, as that
+     would be reading GDB's terminal settings, not the inferiors.  */
+
+  serial *term_serial
+    = (tinfo->run_terminal->pty_fd != -1
+       ? serial_fdopen (tinfo->run_terminal->pty_fd)
+       : stdin_serial);
+  SCOPE_EXIT
+    {
+      if (term_serial != stdin_serial)
+	serial_un_fdopen (term_serial);
+    };
+
+  if (!sharing_input_terminal (inf))
+    tinfo->save_from_tty (term_serial);
+
   printf_filtered (_("Inferior's terminal status "
 		     "(currently saved by GDB):\n"));
 
@@ -765,9 +1184,12 @@ child_terminal_info (struct target_ops *self, const char *args, int from_tty)
   printf_filtered ("Process group = %d\n", (int) tinfo->process_group);
 #endif
 
-  serial_print_tty_state (stdin_serial, tinfo->ttystate, gdb_stdout);
+  serial_print_tty_state (term_serial, tinfo->ttystate, gdb_stdout);
 }
 \f
+
+#if USES_FORK_CHILD
+
 /* NEW_TTY_PREFORK is called before forking a new child process,
    so we can record the state of ttys in the child to be formed.
    TTYNAME is null if we are to share the terminal with gdb;
@@ -780,12 +1202,56 @@ child_terminal_info (struct target_ops *self, const char *args, int from_tty)
 void
 new_tty_prefork (const char *ttyname)
 {
-  /* Save the name for later, for determining whether we and the child
-     are sharing a tty.  */
-  inferior_thisrun_terminal = ttyname;
+  /* Save the name and fd for later, for determining whether we and
+     the child are sharing a tty.  */
+
+  if (ttyname != nullptr)
+    {
+      inferior_thisrun_terminal = ttyname;
+      inferior_thisrun_terminal_pty_fd = -1;
+    }
+#if GDB_MANAGED_TERMINALS
+  else
+    {
+      /* Open an unused pty master device.  */
+      int pty_fd = posix_openpt (O_RDWR | O_NONBLOCK | O_CLOEXEC | O_NOCTTY);
+      if (pty_fd == -1)
+	perror_with_name ("posix_openpt");
+
+      /* Grant access to the slave tty.  */
+      if (grantpt (pty_fd) == -1)
+	{
+	  int err = errno;
+	  close (pty_fd);
+	  errno = err;
+	  perror_with_name ("grantpt");
+	}
+
+      /* Unlock the pty master/slave pair.  */
+      if (unlockpt (pty_fd) == -1)
+	{
+	  close (pty_fd);
+	  perror_with_name ("unlockpt");
+	}
+
+      inferior_thisrun_terminal = ptsname (pty_fd);
+      inferior_thisrun_terminal_pty_fd = pty_fd;
+
+      if (initial_gdb_ttystate != nullptr)
+	{
+	  serial *pty_fd_serial = serial_fdopen (pty_fd);
+
+	  int result = serial_set_tty_state (pty_fd_serial,
+					     initial_gdb_ttystate);
+	  gdb_assert (result != -1);
+	  OOPSY ("setting tty state");
+
+	  serial_un_fdopen (pty_fd_serial);
+	}
+    }
+#endif
 }
 
-#if !defined(__GO32__) && !defined(_WIN32)
 /* If RESULT, assumed to be the return value from a system call, is
    negative, print the error message indicated by errno and exit.
    MSG should identify the operation that failed.  */
@@ -798,7 +1264,14 @@ check_syscall (const char *msg, int result)
       _exit (1);
     }
 }
-#endif
+
+/* See terminal.h.  */
+
+bool
+created_managed_tty ()
+{
+  return inferior_thisrun_terminal_pty_fd != -1;
+}
 
 void
 new_tty ()
@@ -806,7 +1279,6 @@ new_tty ()
   if (inferior_thisrun_terminal == nullptr
       || is_gdb_terminal (inferior_thisrun_terminal))
     return;
-#if !defined(__GO32__) && !defined(_WIN32)
   int tty;
 
 #ifdef TIOCNOTTY
@@ -855,12 +1327,13 @@ new_tty ()
 
   if (tty > 2)
     close (tty);
-#endif /* !go32 && !win32 */
 }
 
 /* NEW_TTY_POSTFORK is called after forking a new child process, and
    adding it to the inferior table, to store the TTYNAME being used by
-   the child, or null if it sharing the terminal with gdb.  */
+   the child, or null if it sharing the terminal with gdb.  If the
+   child is using a terminal created by GDB, the corresponding pty
+   master fd is stored.  */
 
 void
 new_tty_postfork (void)
@@ -870,15 +1343,38 @@ new_tty_postfork (void)
 
   struct inferior *inf = current_inferior ();
   struct terminal_info *tinfo = get_inflow_inferior_data (inf);
+  auto *run_terminal = new run_terminal_info ();
+  tinfo->run_terminal = run_terminal;
+  run_terminal->incref ();
 
   if (inferior_thisrun_terminal != nullptr)
-    tinfo->run_terminal = xstrdup (inferior_thisrun_terminal);
+    {
+      run_terminal->ttyname = make_unique_xstrdup (inferior_thisrun_terminal);
+      run_terminal->pty_fd = inferior_thisrun_terminal_pty_fd;
+      if (run_terminal->pty_fd != -1)
+	{
+	  run_terminal->session_leader = getsid (inf->pid);
+	  gdb_assert (run_terminal->session_leader != -1);
+	}
+
+      if (run_terminal->pty_fd != -1)
+	{
+	  add_file_handler (run_terminal->pty_fd,
+			    inferior_stdout_event_handler, run_terminal,
+			    string_printf ("pty_fd-%s",
+					   run_terminal->ttyname.get ()),
+			    true);
+	}
+    }
   else
-    tinfo->run_terminal = xstrdup ("/dev/tty");
+    run_terminal->ttyname = make_unique_xstrdup ("/dev/tty");
 
-  inferior_thisrun_terminal = NULL;
+  inferior_thisrun_terminal = nullptr;
+  inferior_thisrun_terminal_pty_fd = -1;
 }
 
+#endif /* USES_FORK_CHILD */
+
 \f
 /* Call set_sigint_trap when you need to pass a signal on to an attached
    process when handling SIGINT.  */
@@ -961,6 +1457,16 @@ initialize_stdin_serial (void)
   stdin_serial = serial_fdopen (0);
 }
 
+/* "show" callback for "set debug managed-tty".  */
+
+static void
+show_debug_managed_tty (ui_file *file, int from_tty,
+			cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Debugging of GDB-managed terminals is %s.\n"),
+		    value);
+}
+
 void _initialize_inflow ();
 void
 _initialize_inflow ()
@@ -968,6 +1474,13 @@ _initialize_inflow ()
   add_info ("terminal", info_terminal_command,
 	    _("Print inferior's saved terminal status."));
 
+  add_setshow_boolean_cmd
+    ("managed-tty", class_maintenance, &debug_managed_tty,
+     _("Set debugging of GDB-managed terminals."),
+     _("Show debugging of GDB-managed terminals."),
+     _("When non-zero, GDB-managed terminals specific debugging is enabled."),
+     nullptr, show_debug_managed_tty, &setdebuglist, &showdebuglist);
+
   /* OK, figure out whether we have job control.  */
   have_job_control ();
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 34a2aee41d7..d31bcd98e84 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1075,6 +1075,82 @@ linux_nat_post_attach_wait (ptid_t ptid, int *signalled)
   return status;
 }
 
+/* Wait for a SIGSTOP out of PID.  */
+
+static void
+waitpid_sigstop (pid_t pid)
+{
+  int status;
+  int res = waitpid (pid, &status, 0);
+  if (res == -1)
+    perror_with_name (_("waiting for child"));
+  else if (res != pid)
+    error (_("wait returned unexpected PID %d"), res);
+  else if (!WIFSTOPPED (status) || WSTOPSIG (status) != SIGSTOP)
+    error (_("wait returned unexpected status 0x%x"), status);
+}
+
+/* Wait for a fork event out of PID.  */
+
+static void
+waitpid_fork (pid_t pid)
+{
+  int status;
+  int res = waitpid (pid, &status, 0);
+  if (res == -1)
+    perror_with_name (_("waiting for child"));
+  else if (res != pid)
+    error (_("wait returned unexpected PID %d"), res);
+  else if (!WIFSTOPPED (status))
+    error (_("wait returned unexpected status 0x%x"), status);
+  else
+    {
+      int event = linux_ptrace_get_extended_event (status);
+      if (event != PTRACE_EVENT_FORK)
+	error (_("wait returned unexpected status 0x%x"), status);
+    }
+}
+
+pid_t
+linux_nat_target::handle_session_leader_fork (pid_t sl_pid)
+{
+  /* The first fork child is the session leader.  In turn its fork
+     child (i.e., GDB's granchild) is the inferior we want to debug.
+     Enable tracefork in order to trace the grandchild, and get its
+     pid.  */
+
+  waitpid_sigstop (sl_pid);
+
+  linux_enable_event_reporting (sl_pid, PTRACE_O_TRACEFORK);
+  ptrace (PTRACE_CONT, sl_pid, (PTRACE_TYPE_ARG3) 1, 0);
+
+  /* We should see a fork event now, for the second fork.  */
+  waitpid_fork (sl_pid);
+
+  /* Extract the grandchild's pid.  This is the final inferior
+     process.  */
+  unsigned long inf_pid;
+
+  if (ptrace (PTRACE_GETEVENTMSG, sl_pid, 0, &inf_pid) == -1)
+    perror_with_name (_("getting event message"));
+
+  /* The new child has a pending SIGSTOP.  We can't affect it until it
+     hits the SIGSTOP, but we're already attached.  */
+  waitpid_sigstop (inf_pid);
+
+  /* We don't need to continue debugging the session leader.  It's
+     simpler to just detach from it.  */
+  ptrace (PTRACE_DETACH, sl_pid, (PTRACE_TYPE_ARG3) 1, 0);
+
+  /* Resume the grandchild / inferior.  Disable event reporting to
+     avoid confusing startup_inferior with extra events as the
+     inferior goes through the shell.  */
+  linux_disable_event_reporting (inf_pid);
+  ptrace (PTRACE_CONT, inf_pid, (PTRACE_TYPE_ARG3) 1, 0);
+
+  return inf_pid;
+}
+
 void
 linux_nat_target::create_inferior (const char *exec_file,
 				   const std::string &allargs,
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 5426a5c6900..1f050165adb 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -40,6 +40,8 @@ class linux_nat_target : public inf_ptrace_target
   void create_inferior (const char *, const std::string &,
 			char **, int) override;
 
+  pid_t handle_session_leader_fork (pid_t sl_pid) override;
+
   void attach (const char *, int) override;
 
   void detach (inferior *, int) override;
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 7d56250c979..f140242ace5 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -27,7 +27,11 @@
 #include "gdbsupport/pathstuff.h"
 #include "gdbsupport/signals-state-save-restore.h"
 #include "gdbsupport/gdb_tilde_expand.h"
+#include "gdbsupport/scoped_ignore_sigttou.h"
+#include "gdbsupport/managed-tty.h"
 #include <vector>
+#include <sys/types.h>
+#include <sys/wait.h>
 
 extern char **environ;
 
@@ -81,6 +85,33 @@ class execv_argv
   std::string m_storage;
 };
 
+#if GDB_MANAGED_TERMINALS
+
+/* SIGHUP handler for the session leader processes.  GDB sends this
+   explicitly, though it'll also be called if GDB crashes and the
+   terminal is abruptly closed.  */
+
+static void
+session_leader_hup (int sig)
+{
+  scoped_ignore_sigttou ignore_sigttou;
+
+  /* We put the inferior (a child of the session leader) in the
+     foreground, so by default, on detach, if we did nothing else, the
+     inferior would get a SIGHUP when the terminal is closed by GDB.
+     That SIGHUP would likely kill the inferior.  To avoid it, we put
+     the session leader in the foreground before the terminal is
+     closed.  Only processes in the foreground process group get the
+     automatic SIGHUP, so the detached process doesn't get it.  */
+  int res = tcsetpgrp (0, getpid ());
+  if (res == -1)
+    trace_start_error (_("tcsetpgrp failed in session leader\n"));
+
+  _exit (0);
+}
+
+#endif
+
 /* Create argument vector for straight call to execvp.  Breaks up
    ALLARGS into an argument vector suitable for passing to execvp and
    stores it in M_ARGV.  E.g., on "run a b c d" this routine would get
@@ -271,7 +302,8 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
 	       void (*pre_trace_fun) (),
 	       const char *shell_file_arg,
 	       void (*exec_fun)(const char *file, char * const *argv,
-				char * const *env))
+				char * const *env),
+	       pid_t (*handle_session_leader_fork) (pid_t sl_pid))
 {
   pid_t pid;
   /* Set debug_fork then attach to the child while it sleeps, to debug.  */
@@ -354,7 +386,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
      actually be a call to fork(2) due to the fact that autoconf will
      ``#define vfork fork'' on certain platforms.  */
 #if !(defined(__UCLIBC__) && defined(HAS_NOMMU))
-  if (pre_trace_fun || debug_fork)
+  if (pre_trace_fun || debug_fork || child_has_managed_tty_hook ())
     pid = fork ();
   else
 #endif
@@ -405,6 +437,85 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
 
       restore_original_signals_state ();
 
+      /* Fork again so that the resulting inferior process is not the
+	 session leader.  This makes it possible for the inferior to
+	 exit without killing its own children.  If we instead let the
+	 inferior process be the session leader, when it exits, it'd
+	 cause a SIGHUP to be sent to all processes in its session
+	 (i.e., it's children).  The code is disabled on no-MMU
+	 machines because those can't do fork, only vfork.  In theory
+	 we could make this work with vfork by making the session
+	 leader process exec a helper process, probably gdb itself in
+	 a special mode (e.g., something like
+	   exec /proc/self/exe --session-leader-for PID
+      */
+#if GDB_MANAGED_TERMINALS
+      if (child_has_managed_tty_hook ())
+	{
+	  /* Fork again, to make sure the inferior is not the new
+	     session's leader.  */
+	  pid_t child2 = fork ();
+	  if (child2 != 0)
+	    {
+	      /* This is the parent / session leader process.  It just
+		 stays around until GDB closes the terminal.  */
+
+	      /* Gracefully handle SIGHUP.  */
+	      signal (SIGHUP, session_leader_hup);
+
+	      managed_tty_debug_printf
+		(_("session-leader (sid=%d): waiting for child pid=%d exit\n"),
+		 (int) getpid (), (int) child2);
+
+	      /* Reap the child/inferior exit status.  */
+	      int status;
+	      int res = waitpid (child2, &status, 0);
+
+	      managed_tty_debug_printf (_("session-leader (sid=%d): "
+					  "wait for child pid=%d returned: "
+					  "res=%d, waitstatus=0x%x\n"),
+					(int) getpid (), child2, res, status);
+
+	      if (res == -1)
+		warning (_("session-leader (sid=%d): unexpected waitstatus "
+			   "reaping child pid=%d: "
+			   "res=-1, errno=%d (%s)"),
+			 (int) getpid (), child2, errno, safe_strerror (errno));
+	      else if (res != child2)
+		warning (_("session-leader (sid=%d): unexpected waitstatus "
+			   "reaping child pid=%d: "
+			   "res=%d, status=0x%x"),
+			 (int) getpid (), child2, res, status);
+
+	      /* Don't exit yet.  While our direct child is gone,
+		 there may still be grandchildren attached to our
+		 session.  We'll exit when our parent (GDB) closes the
+		 pty, killing us with SIGHUP.  */
+	      while (1)
+		pause ();
+	    }
+	  else
+	    {
+	      /* This is the child / final inferior process.  */
+
+	      int res;
+
+	      /* Run the inferior in its own process group, and make
+		 it the session's foreground pgrp.  */
+
+	      res = gdb_setpgid ();
+	      if (res == -1)
+		trace_start_error (_("setpgid failed in grandchild"));
+
+	      scoped_ignore_sigttou ignore_sigttou;
+
+	      res = tcsetpgrp (0, getpid ());
+	      if (res == -1)
+		trace_start_error (_("tcsetpgrp failed in grandchild\n"));
+	    }
+	}
+#endif /* GDB_MANAGED_TERMINALS */
+
       /* There is no execlpe call, so we have to set the environment
 	 for our child in the global variable.  If we've vforked, this
 	 clobbers the parent, but environ is restored a few lines down
@@ -434,6 +545,9 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
   /* Restore our environment in case a vforked child clob'd it.  */
   environ = save_our_env;
 
+  if (child_has_managed_tty_hook () && handle_session_leader_fork != nullptr)
+    pid = handle_session_leader_fork (pid);
+
   postfork_hook (pid);
 
   /* Now that we have a child process, make it our target, and
diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h
index f45ec9047c8..c0fb2beb5f3 100644
--- a/gdb/nat/fork-inferior.h
+++ b/gdb/nat/fork-inferior.h
@@ -31,12 +31,15 @@ struct process_stratum_target;
    implementations.  */
 #define START_INFERIOR_TRAPS_EXPECTED 1
 
-/* Start an inferior Unix child process and sets inferior_ptid to its
-   pid.  EXEC_FILE is the file to run.  ALLARGS is a string containing
-   the arguments to the program.  ENV is the environment vector to
-   pass.  SHELL_FILE is the shell file, or NULL if we should pick
-   one.  EXEC_FUN is the exec(2) function to use, or NULL for the default
-   one.  */
+/* Start an inferior Unix child process and return its pid.  EXEC_FILE_ARG
+   is the file to run.  ALLARGS is a string containing the arguments
+   to the program.  ENV is the environment vector to pass.
+   SHELL_FILE_ARG is the shell file, or NULL if we should pick one.
+   EXEC_FUN is the exec(2) function to use, or NULL for the default
+   one.  HANDLE_SESSION_LEADER_FORK is the function to use to debug
+   the session leader process across the double-fork and extract the final
+   inferior PID if the inferior is using a gdb-managed terminal, or
+   NULL otherwise.  */
 
 /* This function is NOT reentrant.  Some of the variables have been
    made static to ensure that they survive the vfork call.  */
@@ -48,7 +51,9 @@ extern pid_t fork_inferior (const char *exec_file_arg,
 			    const char *shell_file_arg,
 			    void (*exec_fun) (const char *file,
 					      char * const *argv,
-					      char * const *env));
+					      char * const *env),
+			    pid_t (*handle_session_leader_fork) (pid_t sl_pid)
+			      = nullptr);
 
 /* Accept NTRAPS traps from the inferior.
 
@@ -70,6 +75,10 @@ extern void postfork_hook (pid_t pid);
    place.  This function is mainly used by fork_inferior.  */
 extern void postfork_child_hook ();
 
+/* True if the inferior child has a terminal created and managed by
+   GDB or GDBserver.  */
+extern bool child_has_managed_tty_hook ();
+
 /* Flush both stdout and stderr.  This function needs to be
    implemented differently on GDB and GDBserver.  */
 extern void gdb_flush_out_err ();
diff --git a/gdb/terminal.h b/gdb/terminal.h
index e1f642b8466..f8ef29bc2a9 100644
--- a/gdb/terminal.h
+++ b/gdb/terminal.h
@@ -27,6 +27,9 @@ extern void new_tty (void);
 
 extern void new_tty_postfork (void);
 
+/* Returns true if new_tty_prefork created a GDB-managed terminal.  */
+extern bool created_managed_tty ();
+
 extern void copy_terminal_info (struct inferior *to, struct inferior *from);
 
 /* Exchange the terminal info and state between inferiors A and B.  */
diff --git a/gdb/testsuite/gdb.multi/multi-term-settings.exp b/gdb/testsuite/gdb.multi/multi-term-settings.exp
index dcc6f2ece0f..5275391a73c 100644
--- a/gdb/testsuite/gdb.multi/multi-term-settings.exp
+++ b/gdb/testsuite/gdb.multi/multi-term-settings.exp
@@ -48,9 +48,18 @@ set attach_pid2 [spawn_id_get_pid $attach_spawn_id2]
 # Create inferior WHICH_INF.  INF_HOW is how to create it.  This can
 # be one of:
 #
-#  - "run" - for "run" command.
-#  - "tty" - for "run" + "tty TTY".
-#  - "attach" - for attaching to an existing process.
+#  - "run-session" - for "run" command, GDB puts inferior in its own
+#                    session and gives its own controlling terminal,
+#                    on systems that support it (e.g., GNU/Linux) (the
+#                    default).
+#
+#  - "run-share"   - for "run" command, GDB and inferior share terminal
+#                    ("set inferior-tty /dev/tty").
+#
+#  - "run-tty"     - for "run" + "tty TTY".
+#
+#  - "attach"      - for attaching to an existing process.
+
 proc create_inferior {which_inf inf_how} {
     global binfile
 
@@ -69,12 +78,22 @@ proc create_inferior {which_inf inf_how} {
 	}
     }
 
-    if {$inf_how == "run"} {
+    if {$inf_how == "run-session"} {
+	# Clear inferior-tty back to the default, in case we're
+	# running the testsuite with "set inferior-tty /dev/tty"
+	# forced.
+	gdb_test_no_output "set inferior-tty"
+	if [my_runto_main] {
+	    global inferior_spawn_id
+	    return $inferior_spawn_id
+	}
+    } elseif {$inf_how == "run-share"} {
+	gdb_test_no_output "set inferior-tty /dev/tty"
 	if [my_runto_main] {
 	    global inferior_spawn_id
 	    return $inferior_spawn_id
 	}
-    } elseif {$inf_how == "tty"} {
+    } elseif {$inf_how == "run-tty"} {
 	# Create the new PTY for the inferior.
 	spawn -pty
 	set inf_spawn_id $spawn_id
@@ -161,8 +180,21 @@ proc coretest {inf1_how inf2_how} {
     # "run" makes each inferior be a process group leader.  When we
     # run both inferiors in GDB's terminal/session, only one can end
     # up as the terminal's foreground process group, so it's expected
-    # that the other receives a SIGTTOU.
-    set expect_ttou [expr {$inf1_how == "run" && $inf2_how == "run"}]
+    # that the other receives a SIGTTOU.  Similarly, if we run one
+    # inferior in GDB's terminal, and another in its own GDB-created
+    # terminal/session, then if we resume the latter first, GDB won't
+    # put the former in the foreground either (since GDB must stay in
+    # the foreground to forward input and catch Ctrl-C).
+
+    # Only the native target knows how to put the inferior in its own
+    # terminal/session.
+    set can_per_inf_session [gdb_is_target_native]
+
+    set expect_ttou [expr {(($inf1_how == "run-share"
+			     || (!$can_per_inf_session
+				 && $inf1_how == "run-session"))
+			    && ($inf2_how == "run-share"
+				|| $inf2_how == "run-session"))}]
 
     global gdb_prompt
 
@@ -256,11 +288,11 @@ proc coretest {inf1_how inf2_how} {
     }
 }
 
-set how_modes {"run" "attach" "tty"}
+set how_modes {"run-session" "run-share" "run-tty" "attach"}
 
 foreach_with_prefix inf1_how $how_modes {
     foreach_with_prefix inf2_how $how_modes {
-	if {($inf1_how == "tty" || $inf2_how == "tty")
+	if {($inf1_how == "run-tty" || $inf2_how == "run-tty")
 	    && [target_info gdb_protocol] == "extended-remote"} {
 	    # No way to specify the inferior's tty in the remote
 	    # protocol.
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 4e75a66a00e..88a5f1c1d05 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -36,6 +36,7 @@
 #include "gdbsupport/event-loop.h"
 #include "gdbcmd.h"
 #include "async-event.h"
+#include "inferior.h"
 
 #include "tui/tui.h"
 #include "tui/tui-io.h"
@@ -565,6 +566,8 @@ tui_async_resize_screen (gdb_client_data arg)
 	}
       tui_redisplay_readline ();
     }
+
+  child_terminal_on_sigwinch ();
 }
 #endif
 
diff --git a/gdbserver/fork-child.cc b/gdbserver/fork-child.cc
index 9678133243d..23eff9fbf45 100644
--- a/gdbserver/fork-child.cc
+++ b/gdbserver/fork-child.cc
@@ -58,6 +58,14 @@ prefork_hook ()
 
 /* See nat/fork-inferior.h.  */
 
+bool
+child_has_managed_tty_hook ()
+{
+  return false;
+}
+
+/* See nat/fork-inferior.h.  */
+
 void
 postfork_hook (pid_t pid)
 {
diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am
index 6d4678c8c9b..ccc7d1e5c51 100644
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -56,6 +56,7 @@ libgdbsupport_a_SOURCES = \
     gdb_wait.cc \
     gdb_vecs.cc \
     job-control.cc \
+    managed-tty.cc \
     netstuff.cc \
     new-op.cc \
     pathstuff.cc \
diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in
index d7f2d4914b0..ed9385381c5 100644
--- a/gdbsupport/Makefile.in
+++ b/gdbsupport/Makefile.in
@@ -153,12 +153,13 @@ am_libgdbsupport_a_OBJECTS = agent.$(OBJEXT) btrace-common.$(OBJEXT) \
 	fileio.$(OBJEXT) filestuff.$(OBJEXT) format.$(OBJEXT) \
 	gdb-dlfcn.$(OBJEXT) gdb_tilde_expand.$(OBJEXT) \
 	gdb_wait.$(OBJEXT) gdb_vecs.$(OBJEXT) job-control.$(OBJEXT) \
-	netstuff.$(OBJEXT) new-op.$(OBJEXT) pathstuff.$(OBJEXT) \
-	print-utils.$(OBJEXT) ptid.$(OBJEXT) rsp-low.$(OBJEXT) \
-	run-time-clock.$(OBJEXT) safe-strerror.$(OBJEXT) \
-	scoped_mmap.$(OBJEXT) search.$(OBJEXT) signals.$(OBJEXT) \
-	signals-state-save-restore.$(OBJEXT) tdesc.$(OBJEXT) \
-	thread-pool.$(OBJEXT) xml-utils.$(OBJEXT) $(am__objects_1)
+	managed-tty.$(OBJEXT) netstuff.$(OBJEXT) new-op.$(OBJEXT) \
+	pathstuff.$(OBJEXT) print-utils.$(OBJEXT) ptid.$(OBJEXT) \
+	rsp-low.$(OBJEXT) run-time-clock.$(OBJEXT) \
+	safe-strerror.$(OBJEXT) scoped_mmap.$(OBJEXT) search.$(OBJEXT) \
+	signals.$(OBJEXT) signals-state-save-restore.$(OBJEXT) \
+	tdesc.$(OBJEXT) thread-pool.$(OBJEXT) xml-utils.$(OBJEXT) \
+	$(am__objects_1)
 libgdbsupport_a_OBJECTS = $(am_libgdbsupport_a_OBJECTS)
 AM_V_P = $(am__v_P_@AM_V@)
 am__v_P_ = $(am__v_P_@AM_DEFAULT_V@)
@@ -379,6 +380,7 @@ libgdbsupport_a_SOURCES = \
     gdb_wait.cc \
     gdb_vecs.cc \
     job-control.cc \
+    managed-tty.cc \
     netstuff.cc \
     new-op.cc \
     pathstuff.cc \
@@ -484,6 +486,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gdb_vecs.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gdb_wait.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/job-control.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/managed-tty.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/netstuff.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/new-op.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/pathstuff.Po@am__quote@
diff --git a/gdbsupport/managed-tty.cc b/gdbsupport/managed-tty.cc
new file mode 100644
index 00000000000..824cf0985c2
--- /dev/null
+++ b/gdbsupport/managed-tty.cc
@@ -0,0 +1,22 @@
+/* Support for GDB-managed terminals.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#include "gdbsupport/common-defs.h"
+#include "gdbsupport/managed-tty.h"
+
+bool debug_managed_tty = false;
diff --git a/gdbsupport/managed-tty.h b/gdbsupport/managed-tty.h
new file mode 100644
index 00000000000..d02d6c95659
--- /dev/null
+++ b/gdbsupport/managed-tty.h
@@ -0,0 +1,42 @@
+/* Support for GDB-managed terminals.
+
+   Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#ifndef MANAGED_TTY_H
+#define MANAGED_TTY_H
+
+/* Using host-dependent code here is fine, because the support for
+   creating terminals for inferiors is meant to be used with
+   child/native targets.  Note we disable the feature on no-MMU
+   architectures for now because those can't use fork, see
+   fork-inferior.c.  */
+#if (defined (__linux__)				\
+     && !(defined (__UCLIBC__) && defined (HAS_NOMMU)))
+# define GDB_MANAGED_TERMINALS 1
+#else
+# define GDB_MANAGED_TERMINALS 0
+#endif
+
+extern bool debug_managed_tty;
+
+/* Print a "managed-tty" debug statement.  */
+
+#define managed_tty_debug_printf(fmt, ...) \
+  debug_prefixed_printf_cond (debug_managed_tty, "managed-tty",fmt, ##__VA_ARGS__)
+
+#endif /* MANAGED_TTY_H */
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 13/16] exists_non_stop_target: Avoid flushing frames
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (11 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 12/16] Always put inferiors in their own terminal/session [gdb/9425, gdb/14559] Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 14/16] convert previous_inferior_ptid to strong reference to thread_info Pedro Alves
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

A following patch adds an exists_non_stop_target call in the
target_terminal routines, and that surprisingly caused a weird
regression / GDB crash:

 $ make check RUNTESTFLAGS="--target_board=native-extended-gdbserver" TESTS="gdb.base/signest.exp"
 ...
 configuring for gdbserver local testing (extended-remote)
 Using src/gdb/testsuite/config/extended-gdbserver.exp as tool-and-target-specific interface file.
 Running src/gdb/testsuite/gdb.base/signest.exp ...
 ERROR: GDB process no longer exists

Debugging the core, we see infinite recursion:

 (top-gdb) bt 20
 #0  0x0000561d6a1bfeff in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #1  0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #2  0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #3  0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #4  0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #5  0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #6  0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #7  0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #8  0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #9  0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #10 0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #11 0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #12 0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #13 0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #14 0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #15 0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #16 0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #17 0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #18 0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #19 0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 (More stack frames follow...)

 (top-gdb) bt -30
 #157054 0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #157055 0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #157056 0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #157057 0x0000561d6a1bfeb8 in get_frame_arch (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:2939
 #157058 0x0000561d6a1b989f in frame_unwind_find_by_frame (this_frame=0x561d6b19f9c0, this_cache=0x561d6b19f9d8) at src/gdb/frame-unwind.c:174
 #157059 0x0000561d6a1bff04 in frame_unwind_arch (next_frame=0x561d6b19f9c0) at src/gdb/frame.c:2950
 #157060 0x0000561d6a1bbc65 in frame_unwind_pc (this_frame=0x561d6b19f9c0) at src/gdb/frame.c:970
 #157061 0x0000561d6a1bf54c in get_frame_pc (frame=0x561d6b19fa90) at src/gdb/frame.c:2625
 #157062 0x0000561d6a1bf63e in get_frame_address_in_block (this_frame=0x561d6b19fa90) at src/gdb/frame.c:2655
 #157063 0x0000561d6a0cae7f in dwarf2_frame_cache (this_frame=0x561d6b19fa90, this_cache=0x561d6b19faa8) at src/gdb/dwarf2/frame.c:1010
 #157064 0x0000561d6a0cb928 in dwarf2_frame_this_id (this_frame=0x561d6b19fa90, this_cache=0x561d6b19faa8, this_id=0x561d6b19faf0) at src/gdb/dwarf2/frame.c:1227
 #157065 0x0000561d6a1baf72 in compute_frame_id (fi=0x561d6b19fa90) at src/gdb/frame.c:588
 #157066 0x0000561d6a1bb16e in get_frame_id (fi=0x561d6b19fa90) at src/gdb/frame.c:636
 #157067 0x0000561d6a1bb224 in get_stack_frame_id (next_frame=0x561d6b19fa90) at src/gdb/frame.c:650
 #157068 0x0000561d6a26ecd0 in insert_hp_step_resume_breakpoint_at_frame (return_frame=0x561d6b19fa90) at src/gdb/infrun.c:7809
 #157069 0x0000561d6a26b88a in handle_signal_stop (ecs=0x7ffc67022830) at src/gdb/infrun.c:6428
 #157070 0x0000561d6a269d81 in handle_inferior_event (ecs=0x7ffc67022830) at src/gdb/infrun.c:5741
 #157071 0x0000561d6a265bd0 in fetch_inferior_event () at src/gdb/infrun.c:4120
 #157072 0x0000561d6a244c24 in inferior_event_handler (event_type=INF_REG_EVENT) at src/gdb/inf-loop.c:41
 #157073 0x0000561d6a435cc4 in remote_async_serial_handler (scb=0x561d6b4a8990, context=0x561d6b4a4c48) at src/gdb/remote.c:14403
 #157074 0x0000561d6a460bc5 in run_async_handler_and_reschedule (scb=0x561d6b4a8990) at src/gdb/ser-base.c:138
 #157075 0x0000561d6a460cae in fd_event (error=0, context=0x561d6b4a8990) at src/gdb/ser-base.c:189
 #157076 0x0000561d6a76a191 in handle_file_event (file_ptr=0x561d6b233ae0, ready_mask=1) at src/gdbsupport/event-loop.cc:575
 #157077 0x0000561d6a76a743 in gdb_wait_for_event (block=1) at src/gdbsupport/event-loop.cc:701
 #157078 0x0000561d6a7694ee in gdb_do_one_event () at src/gdbsupport/event-loop.cc:237
 #157079 0x0000561d6a2df16b in start_event_loop () at src/gdb/main.c:421
 #157080 0x0000561d6a2df2b6 in captured_command_loop () at src/gdb/main.c:481
 #157081 0x0000561d6a2e0d16 in captured_main (data=0x7ffc67022bd0) at src/gdb/main.c:1353
 #157082 0x0000561d6a2e0da8 in gdb_main (args=0x7ffc67022bd0) at src/gdb/main.c:1370
 #157083 0x0000561d69eb3d82 in main (argc=13, argv=0x7ffc67022cf8, envp=0x7ffc67022d68) at src/gdb/gdb.c:33

This was caused by exists_non_stop_target flushing the frame cache via
scoped_restore_current_thread/switch_to_thread, while we're in the
middle of unwinding.

Fix this by making exists_non_stop_target only switch the inferior,
like done in target_pass_ctrlc.

The annota1.exp change is necessary because we'd get a regression
otherwise:

  @@ -238,8 +238,6 @@ Continuing.

   \032\032breakpoints-invalid

  -\032\032frames-invalid
  -
   \032\032breakpoint 3

   Breakpoint 3,
  @@ -276,7 +274,7 @@ printf.c
   \032\032pre-prompt
   (gdb)
   \032\032prompt
  -PASS: gdb.base/annota1.exp: continue to printf
  +FAIL: gdb.base/annota1.exp: continue to printf

... because this patch avoids flushing the frame cache that lead to
that missing frames-invalid.  We still need to match frames-invalid
because against gdbserver + "maint set target non-stop on", some other
code path flushes the frame cache resulting in the annotation being
emitted anyway.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* target.c (exists_non_stop_target): Use
	scoped_restore_current_inferior and set_current_inferior instead
	of scoped_restore_current_thread / switch_to_inferior_no_thread.

Change-Id: I8402483ee755e64e54d8b7c4a67c177557f569bd
---
 gdb/target.c                       | 11 +++++++++--
 gdb/testsuite/gdb.base/annota1.exp |  3 ++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index 6babfc56256..63aa3e95218 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4390,11 +4390,18 @@ exists_non_stop_target ()
   if (target_is_non_stop_p ())
     return true;
 
-  scoped_restore_current_thread restore_thread;
+  /* We can get here quite deep in core code, e.g., from
+     target_terminal::inferior().  Avoid switching thread context or
+     anything that would communicate with the target (e.g., to fetch
+     registers), or flushing e.g., the frame cache, as we may end up
+     called from within the frame building code.  We just switch
+     inferior in order to be able to call through the
+     target_stack.  */
+  scoped_restore_current_inferior restore_inferior;
 
   for (inferior *inf : all_inferiors ())
     {
-      switch_to_inferior_no_thread (inf);
+      set_current_inferior (inf);
       if (target_is_non_stop_p ())
 	return true;
     }
diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp
index 3689f49fa1b..0e28dc8556f 100644
--- a/gdb/testsuite/gdb.base/annota1.exp
+++ b/gdb/testsuite/gdb.base/annota1.exp
@@ -24,6 +24,7 @@ if ![target_can_use_run_cmd] {
 }
 
 set breakpoints_invalid "\r\n\032\032breakpoints-invalid\r\n"
+set frames_invalid "\r\n\032\032frames-invalid\r\n"
 
 #
 # test running programs
@@ -226,7 +227,7 @@ gdb_test_multiple "break printf" "break printf" {
 #
 # get to printf
 #
-set pat_begin "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\n${breakpoints_invalid}\r\n\032\032frames-invalid\r\n"
+set pat_begin "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\r\n${frames_invalid}${breakpoints_invalid}(${frames_invalid})?"
 set pat_adjust "warning: Breakpoint 3 address previously adjusted from $hex to $hex.\r\n"
 set pat_end "\r\n\032\032breakpoint 3\r\n\r\nBreakpoint 3, \r\n\032\032frame-begin 0 $hex\r\n\r\n(\032\032frame-address\r\n$hex\r\n\032\032frame-address-end\r\n in \r\n)*.*\032\032frame-function-name\r\n.*printf(@.*)?\r\n\032\032frame-args\r\n.*\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$"
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 14/16] convert previous_inferior_ptid to strong reference to thread_info
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (12 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 13/16] exists_non_stop_target: Avoid flushing frames Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-14 21:24 ` [PATCH v2 15/16] GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR gdb/9425, PR gdb/14559] Pedro Alves
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

While working on a patch, I spotted a regression in the
gdb.multi/multi-target-no-resumed.exp.exp testcase.  Debugging the
issue, I realized that the problem was related to how I was using
previous_inferior_ptid to look up the thread the user had last
selected.  The problem is that previous_inferior_ptid alone doesn't
tell you which target that ptid is from, and I was just always using
the current target, which was incorrect.  Two different targets may
have threads with the same ptid.

I decided to fix this by replacing previous_inferior_ptid with a
strong reference to the thread, called previous_thread.

A new update_previous_thread function is added can be used to updated
previous_thread from inferior_ptid.

This must be called in several places that really want to get rid of
previous_thread thread, and reset the thread id counter, otherwise we
get regressions like these:

  (gdb) info threads -gid
    Id   GId  Target Id                               Frame
 - * 1    1    Thread 2974541.2974541 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21
 - (gdb) PASS: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid
 + * 1    2    Thread 2958361.2958361 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21
 + (gdb) FAIL: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid

and:

  Core was generated by `build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/si'.
  Program terminated with signal SIGTRAP, Trace/breakpoint trap.
  #0  gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398
  398      kill (getpid (), SIGABRT);
 +[Current thread is 1 (LWP 2662066)]
  Restored records from core file build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/sigall.precsave.
  #0  gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398
  398      kill (getpid (), SIGABRT);

  continue
  Continuing.

 -Program received signal SIGABRT, Aborted.
 +Thread 1 received signal SIGABRT, Aborted.
  0x00007ffff7dfd55b in kill () at ../sysdeps/unix/syscall-template.S:78
  78     ../sysdeps/unix/syscall-template.S: No such file or directory.
 -(gdb) PASS: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT
 +(gdb) FAIL: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT

I.e., GDB was failing to restart the thread counter back to 1, because
the previous_thread thread was being help due to the strong reference.

Tested on GNU/Linux native, gdbserver and gdbserver + "maint set
target-non-stop on".

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* infcmd.c (kill_command, detach_command, disconnect_command):
	Call update_previous_thread.
	* infrun.c (previous_inferior_ptid): Delete.
	(previous_thread): New.
	(update_previous_thread): New.
	(proceed, init_wait_for_inferior): Call update_previous_thread.
	(normal_stop): Adjust to compare previous_thread and
	inferior_thread.  Call update_previous_thread.
	* infrun.h (update_previous_thread): Declare.
	* target.c (target_pre_inferior, target_preopen): Call
	update_previous_thread.

Change-Id: I4f06dd81f00848bb7d6d26a94ff091e2a4e646d9
---
 gdb/infcmd.c |  5 +++++
 gdb/infrun.c | 25 ++++++++++++++++++-------
 gdb/infrun.h |  4 ++++
 gdb/target.c |  5 +++++
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 8190ba36565..99ce0ec78fa 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2401,6 +2401,8 @@ kill_command (const char *arg, int from_tty)
 
   target_kill ();
 
+  update_previous_thread ();
+
   if (print_inferior_events)
     printf_unfiltered (_("[Inferior %d (%s) killed]\n"),
 		       infnum, pid_str.c_str ());
@@ -2756,6 +2758,8 @@ detach_command (const char *args, int from_tty)
 
   target_detach (current_inferior (), from_tty);
 
+  update_previous_thread ();
+
   /* The current inferior process was just detached successfully.  Get
      rid of breakpoints that no longer make sense.  Note we don't do
      this within target_detach because that is also used when
@@ -2794,6 +2798,7 @@ disconnect_command (const char *args, int from_tty)
   target_disconnect (args, from_tty);
   no_shared_libraries (NULL, from_tty);
   init_thread_list ();
+  update_previous_thread ();
   if (deprecated_detach_hook)
     deprecated_detach_hook ();
 }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1eb7526d246..3f40fa39b5a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -143,8 +143,18 @@ show_step_stop_if_no_debug (struct ui_file *file, int from_tty,
 /* proceed and normal_stop use this to notify the user when the
    inferior stopped in a different thread than it had been running
    in.  */
+static thread_info_ref previous_thread;
 
-static ptid_t previous_inferior_ptid;
+/* See infrun.h.  */
+
+void
+update_previous_thread ()
+{
+  if (inferior_ptid == null_ptid)
+    previous_thread = nullptr;
+  else
+    previous_thread = thread_info_ref::new_reference (inferior_thread ());
+}
 
 /* If set (default for legacy reasons), when following a fork, GDB
    will detach from one of the fork branches, child or parent.
@@ -3072,7 +3082,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
     }
 
   /* We'll update this if & when we switch to a new thread.  */
-  previous_inferior_ptid = inferior_ptid;
+  update_previous_thread ();
 
   regcache = get_current_regcache ();
   gdbarch = regcache->arch ();
@@ -3336,7 +3346,7 @@ init_wait_for_inferior (void)
 
   nullify_last_target_wait_ptid ();
 
-  previous_inferior_ptid = inferior_ptid;
+  update_previous_thread ();
 }
 
 \f
@@ -8559,11 +8569,11 @@ normal_stop (void)
      after this event is handled, so we're not really switching, only
      informing of a stop.  */
   if (!non_stop
-      && previous_inferior_ptid != inferior_ptid
-      && target_has_execution ()
       && last.kind != TARGET_WAITKIND_SIGNALLED
       && last.kind != TARGET_WAITKIND_EXITED
-      && last.kind != TARGET_WAITKIND_NO_RESUMED)
+      && last.kind != TARGET_WAITKIND_NO_RESUMED
+      && target_has_execution ()
+      && previous_thread != inferior_thread ())
     {
       SWITCH_THRU_ALL_UIS ()
 	{
@@ -8572,7 +8582,8 @@ normal_stop (void)
 			   target_pid_to_str (inferior_ptid).c_str ());
 	  annotate_thread_changed ();
 	}
-      previous_inferior_ptid = inferior_ptid;
+
+      update_previous_thread ();
     }
 
   if (last.kind == TARGET_WAITKIND_NO_RESUMED)
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 7ebb9fc9f4e..5d791bdc5b4 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -87,6 +87,10 @@ enum exec_direction_kind
 /* The current execution direction.  */
 extern enum exec_direction_kind execution_direction;
 
+/* Call this to point 'previous_thread' at the thread returned by
+   inferior_thread, or at nullptr, if there's no selected thread.  */
+extern void update_previous_thread ();
+
 extern void start_remote (int from_tty);
 
 /* Clear out all variables saying what to do when inferior is
diff --git a/gdb/target.c b/gdb/target.c
index 63aa3e95218..68d7e7c12d7 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2505,6 +2505,8 @@ target_pre_inferior (int from_tty)
 
   current_inferior ()->highest_thread_num = 0;
 
+  update_previous_thread ();
+
   agent_capability_invalidate ();
 }
 
@@ -2533,6 +2535,9 @@ target_preopen (int from_tty)
 	error (_("Program not killed."));
     }
 
+  /* Release reference to old previous thread.  */
+  update_previous_thread ();
+
   /* Calling target_kill may remove the target from the stack.  But if
      it doesn't (which seems like a win for UDI), remove it now.  */
   /* Leave the exec target, though.  The user may be switching from a
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 15/16] GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR gdb/9425, PR gdb/14559]
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (13 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 14/16] convert previous_inferior_ptid to strong reference to thread_info Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-07-08 23:05   ` Maciej W. Rozycki
  2021-06-14 21:24 ` [PATCH v2 16/16] Document pseudo-terminal and interrupting changes Pedro Alves
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

After the "Always put inferiors in their own terminal/session
[gdb/9425, gdb/14559]" change, when a user types "Ctrl-C" while the
inferior is running, GDB is the one who gets the SIGINT, not the
inferior process.  GDB then forwards the SIGINT to the inferior with
target_pass_ctrlc.

That was necessary but not not sufficient to fix PRs gdb/9425,
gdb/14559, because if a program blocks SIGINT with e.g. sigprocmask,
then if GDB sends it a SIGINT, the signal isn't ever delivered to the
process, so ptrace does not intercept it.  You type Ctrl-C, but
nothing happens.  Similarly, if a program uses sigwait to wait for
SIGINT, and the program receives a SIGINT, the SIGINT is _not_
intercepted by ptrace, it goes straight to the inferior.

Now that the Ctrl-C results in a SIGINT sent to GDB instead of the
inferior, we can make GDB interrupt the program any other way we like.
This patch makes non-stop-capable ports interrupt the program with
stop_all_threads / target_stop (i.e., SIGSTOP) instead of
target_pass_ctrlc (i.e., SIGINT), which always works -- SIGSTOP can't
be blocked/ignored.  (In the future GDB may even switch to
PTRACE_INTERRUPT on Linux, though that's a project of its own.)

Another advantage here is with multi-target -- currently, because GDB
relies on Ctrl-C stopping one thread, and then stopping all other
threads in reaction to that stop, target_pass_ctrlc tries to find one
inferior with a thread that is running, in any target.  If the
selected target for some reason fails to process the Ctrl-C request,
then the Ctrl-C ends up lost.  The mechanism implemented in this patch
is different -- we never have to pick a thread, inferior or target --
we're going to stop everything, so we end up in stop_all_threads.

For non-stop, the patch preserves the current behavior of only
stopping one thread in reaction to Ctrl-C, so it can still happen that
the thread that GDB selects to stop disappears and the Ctrl-C ends up
being lost.  However, because now GDB always sees the SIGINT first, we
can change how Ctrl-C behaves there too.  We could even make it
configurable -- for example, it could be reasonable that Ctrl-C simply
drops the CLI back to the prompt, without stopping anything at all.
That might be interesting for "set observer-mode on", at least.

This commit has a user-visible behavior change in all-stop mode --
when you interrupt the program with Ctrl-C, instead of:

  Thread 1 "threads" received signal SIGINT, Interrupt.

You'll get:

  Thread 1 "threads" stopped.

Which is what you already got with the "interrupt" command in non-stop
mode.

If you really want to pass a SIGINT to the program, you can then issue:

  (gdb) signal SIGINT

This commit also adjusts the testsuite to cope with that output
alternative.

With this change, the gdb.base/sigint-sigwait.exp and
gdb.base/sigint-masked-out.exp testcases now pass cleanly on
GNU/Linux, on both native debugging and gdbserver + "maint set
target-non-stop on", so the kfails are adjusted accordingly.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/9425
	PR gdb/14559
	* event-top.c (default_quit_handler): Mark infrun event-loop
	handler with mark_infrun_async_event_handler_ctrl_c instead of
	passing the Ctrl-C to the target directly with target_pass_ctrlc.
	* infcmd.c (interrupt_target_1): On non-stop targets, Mark infrun
	event-loop handler with
	mark_infrun_async_event_handler_interrupt_all instead of using
	target_interrupt.
	* infrun.c (interrupt_all_requested, switch_to_stop_thread)
	(sync_interrupt_all)
	(mark_infrun_async_event_handler_interrupt_all)
	(mark_infrun_async_event_handler_ctrl_c): New.
	(infrun_async_inferior_event_handler): Handle Ctrl-C/interrupt
	requests.
	* infrun.h (mark_infrun_async_event_handler_interrupt_all)
	(mark_infrun_async_event_handler_ctrl_c): Declare.
	* linux-nat.c (wait_for_signal): Don't handle Ctrl-C here.
	(linux_nat_wait_1): Handle it here, by marking the infrun event
	handler, and returning TARGET_WAITKIND_IGNORE with the quit flag
	still set.
	* target.c (maybe_pass_ctrlc): New.
	(target_terminal::inferior, target_terminal::restore_inferior):
	Use it.
	(target_pass_ctrlc): Ass there's no non-stop target pushed.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/9425
	PR gdb/14559
	* gdb.base/bp-cmds-continue-ctrl-c.exp: Expect "stopped" in
	alternative to "signal SIGINT".
	* gdb.base/interrupt-daemon-attach.exp: Likewise.
	* gdb.base/interrupt-daemon.exp: Likewise.
	* gdb.base/interrupt-noterm.exp: Likewise.
	* gdb.base/interrupt.exp: Likewise.
	* gdb.base/random-signal.exp: Likewise.
	* gdb.base/range-stepping.exp: Likewise.
	* gdb.gdb/selftest.exp: Likewise.
	* gdb.mi/new-ui-mi-sync.exp: Likewise.
	* gdb.multi/multi-target-interrupt.exp: Likewise.
	* gdb.multi/multi-target-no-resumed.exp: Likewise.
	* gdb.multi/multi-term-settings.exp: Likewise.
	* gdb.server/reconnect-ctrl-c.exp: Likewise.
	* gdb.threads/async.exp: Likewise.
	* gdb.threads/continue-pending-status.exp: Likewise.
	* gdb.threads/leader-exit.exp: Likewise.
	* gdb.threads/manythreads.exp: Likewise.
	* gdb.threads/pthreads.exp: Likewise.
	* gdb.threads/schedlock.exp: Likewise.
	* gdb.threads/sigthread.exp: Likewise.

	* lib/gdb.exp (can_interrupt_blocked_sigint): New.
	* gdb.base/sigint-masked-out.exp (test_ctrl_c)
	(test_interrupt_cmd): Use can_interrupt_blocked_sigint, and don't
	kfail if true.
	* gdb.base/sigint-sigwait.exp (test_ctrl_c, test_interrupt_cmd):
	Likewise.

Change-Id: I83c1b6a20deea1f1909156adde1d60b8f6f2629b
---
 gdb/event-top.c                               |  10 +-
 gdb/infcmd.c                                  |  34 +++-
 gdb/infrun.c                                  | 149 +++++++++++++++++-
 gdb/infrun.h                                  |  12 ++
 gdb/linux-nat.c                               |  39 +++--
 gdb/target.c                                  |  24 ++-
 .../gdb.base/bp-cmds-continue-ctrl-c.exp      |   3 +
 .../gdb.base/interrupt-daemon-attach.exp      |   2 +-
 gdb/testsuite/gdb.base/interrupt-daemon.exp   |   4 +-
 gdb/testsuite/gdb.base/interrupt-noterm.exp   |   2 +-
 gdb/testsuite/gdb.base/interrupt.exp          |   4 +-
 gdb/testsuite/gdb.base/random-signal.exp      |   3 +-
 gdb/testsuite/gdb.base/range-stepping.exp     |   2 +-
 gdb/testsuite/gdb.base/sigint-masked-out.exp  |  12 +-
 gdb/testsuite/gdb.base/sigint-sigwait.exp     |  12 +-
 gdb/testsuite/gdb.gdb/selftest.exp            |   4 +-
 gdb/testsuite/gdb.mi/new-ui-mi-sync.exp       |   2 +-
 .../gdb.multi/multi-target-interrupt.exp      |   2 +-
 .../gdb.multi/multi-target-no-resumed.exp     |   2 +-
 .../gdb.multi/multi-term-settings.exp         |   2 +-
 gdb/testsuite/gdb.server/reconnect-ctrl-c.exp |   4 +-
 gdb/testsuite/gdb.threads/async.exp           |   2 +-
 .../gdb.threads/continue-pending-status.exp   |   2 +-
 gdb/testsuite/gdb.threads/leader-exit.exp     |   2 +-
 gdb/testsuite/gdb.threads/manythreads.exp     |   4 +-
 gdb/testsuite/gdb.threads/pthreads.exp        |   2 +-
 gdb/testsuite/gdb.threads/schedlock.exp       |  11 +-
 gdb/testsuite/gdb.threads/sigthread.exp       |   2 +-
 gdb/testsuite/lib/gdb.exp                     |  19 +++
 29 files changed, 315 insertions(+), 57 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 002a7dc95e0..9741b23576a 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -1002,7 +1002,15 @@ default_quit_handler (void)
       if (target_terminal::is_ours ())
 	quit ();
       else
-	target_pass_ctrlc ();
+	{
+	  /* Let the even loop handle the quit/interrupt.  In some
+	     modes (e.g., "set non-stop off" + "maint set
+	     target-non-stop on"), it's not safe to request an
+	     interrupt right now, as we may be in the middle of
+	     handling some other event, and target_stop changes infrun
+	     state.  */
+	  mark_infrun_async_event_handler_ctrl_c ();
+	}
     }
 }
 
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 99ce0ec78fa..72051bdc095 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2846,7 +2846,39 @@ interrupt_target_1 (bool all_threads)
 	stop_current_target_threads_ns (inferior_ptid);
     }
   else
-    target_interrupt ();
+    {
+      if (exists_non_stop_target ())
+	{
+	  /* Ignore the interrupt request if everything is already
+	     stopped.  */
+	  auto any_resumed = [] ()
+	    {
+	      for (thread_info *thr : all_non_exited_threads ())
+		{
+		  if (thr->executing)
+		    return true;
+		  if (thr->suspend.waitstatus_pending_p)
+		    return true;
+		}
+	      return false;
+	    };
+
+	  if (any_resumed ())
+	    {
+	      /* Stop all threads, and report one single stop for all
+		 threads.  Since the "interrupt" command works
+		 asynchronously on all other modes (non-stop or true
+		 all-stop + stopping with SIGINT), i.e., the command
+		 finishes and GDB prints the prompt before the target
+		 actually stops, make this mode work the same, by
+		 deferring the actual synchronous stopping work to the
+		 event loop.  */
+	      mark_infrun_async_event_handler_interrupt_all ();
+	    }
+	}
+      else
+	target_interrupt ();
+    }
 
   disable_commit_resumed.reset_and_commit ();
 }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3f40fa39b5a..f3f24839f32 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -9440,13 +9440,158 @@ static const struct internalvar_funcs siginfo_funcs =
   NULL
 };
 
-/* Callback for infrun's target events source.  This is marked when a
-   thread has a pending status to process.  */
+/* True if the user used the "interrupt" command and we want to handle
+   the interruption via the event loop instead of immediately.  This
+   is so that "interrupt" always stops the program asynchronously in
+   all the different execution modes.  In particular, in "set non-stop
+   off" + "maint set target-non-stop on" mode, we want to
+   synchronously stop all threads with stop_all_threads, so we delay
+   doing that to the event loop, so that "interrupt" presents a prompt
+   immediately, and then presents the stop afterwards, just like what
+   happens in non-stop mode, or if the target is in true all-stop mode
+   and the interrupting is done by sending a SIGINT to the inferior
+   process.  */
+static bool interrupt_all_requested = false;
+
+/* Pick the thread to report the stop on and to switch to it.  */
+
+static void
+switch_to_stop_thread ()
+{
+  thread_info *stop_thr = nullptr;
+
+  if (previous_thread != nullptr && previous_thread->state == THREAD_RUNNING)
+    stop_thr = previous_thread.get ();
+  else
+    {
+      for (thread_info *thr : all_non_exited_threads ())
+	if (thr->state == THREAD_RUNNING)
+	  {
+	    stop_thr = thr;
+	    break;
+	  }
+    }
+
+  gdb_assert (stop_thr != nullptr);
+
+  switch_to_thread (stop_thr);
+}
+
+/* Synchronously stop all threads, saving interesting events as
+   pending events, and present a normal stop on one of the threads.
+   Preference is given to the "previous thread", which was the thread
+   that the user last resumed.  This is used in "set non-stop off" +
+   "maint set target-non-stop on" mode to stop the target in response
+   to Ctrl-C or the "interrupt" command.  */
+
+static void
+sync_interrupt_all ()
+{
+  /* Events are always processed with the main UI as current UI.  This
+     way, warnings, debug output, etc. are always consistently sent to
+     the main console.  */
+  scoped_restore save_ui = make_scoped_restore (&current_ui, main_ui);
+
+  /* Exposed by gdb.base/paginate-after-ctrl-c-running.exp.  */
+
+  /* Temporarily disable pagination.  Otherwise, the user would be
+     given an option to press 'q' to quit, which would cause an early
+     exit and could leave GDB in a half-baked state.  */
+  scoped_restore save_pagination
+    = make_scoped_restore (&pagination_enabled, false);
+
+  scoped_disable_commit_resumed disable_commit_resumed ("stopping for ctrl-c");
+
+  gdb_assert (!non_stop);
+
+  /* Stop all threads before picking which one to present the stop on
+     -- this is safer than the other way around because otherwise the
+     thread we pick could exit just while we try to stop it.  */
+  stop_all_threads ();
+
+  switch_to_stop_thread ();
+
+  target_waitstatus ws;
+  ws.kind = TARGET_WAITKIND_STOPPED;
+  ws.value.sig = GDB_SIGNAL_0;
+  set_last_target_status (current_inferior ()->process_target (),
+			  inferior_ptid, ws);
+  stopped_by_random_signal = true;
+  stop_print_frame = true;
+  normal_stop ();
+
+  inferior_event_handler (INF_EXEC_COMPLETE);
+
+  /* If a UI was in sync execution mode, and now isn't, restore its
+     prompt (a synchronous execution command has finished, and we're
+     ready for input).  */
+  all_uis_check_sync_execution_done ();
+}
+
+/* See infrun.h.  */
+
+void
+mark_infrun_async_event_handler_interrupt_all ()
+{
+  mark_infrun_async_event_handler ();
+  interrupt_all_requested = true;
+}
+
+/* See infrun.h.  */
+
+void
+mark_infrun_async_event_handler_ctrl_c ()
+{
+  mark_infrun_async_event_handler ();
+  set_quit_flag ();
+}
+
+/* Callback for infrun's target events source.  This is marked either
+   when a thread has a pending status to process, or a target
+   interrupt was requested, either with Ctrl-C or the "interrupt"
+   command and target is in non-stop mode.  */
 
 static void
 infrun_async_inferior_event_handler (gdb_client_data data)
 {
+  /* Handle a Ctrl-C while the inferior has the terminal, or an
+     "interrupt" cmd request.  */
+  if ((!target_terminal::is_ours () && check_quit_flag ())
+      || interrupt_all_requested)
+    {
+      interrupt_all_requested = false;
+
+      if (exists_non_stop_target ())
+	{
+	  if (non_stop)
+	    {
+	      /* Stop one thread, like it would happen if we were
+		 stopping with SIGINT sent to the foreground
+		 process.  */
+	      switch_to_stop_thread ();
+	      interrupt_target_1 (false);
+	    }
+	  else
+	    {
+	      /* Stop all threads, and report one single stop for all
+		 threads.  */
+	      sync_interrupt_all ();
+	    }
+	}
+      else
+	{
+	  /* Pass a Ctrl-C request to the target.  Usually this means
+	     sending a SIGINT to the inferior process.  */
+	  target_pass_ctrlc ();
+	}
+
+      /* Don't clear the event handler yet -- there may be pending
+	 events to process.  */
+      return;
+    }
+
   clear_async_event_handler (infrun_async_inferior_event_token);
+
   inferior_event_handler (INF_REG_EVENT);
 }
 
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 5d791bdc5b4..64fdfc7ab60 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -255,6 +255,18 @@ extern void infrun_async (int enable);
    loop.  */
 extern void mark_infrun_async_event_handler (void);
 
+/* Like mark_infrun_async_event_handler, and ask the event loop to
+   stop all threads, in response to an "interrupt" command.  */
+extern void mark_infrun_async_event_handler_interrupt_all ();
+
+/* Like mark_infrun_async_event_handler, and ask the event loop to
+   handle a "Ctrl-C" interruption request.  In some modes (e.g., "set
+   non-stop off" + "maint set target-non-stop on"), we interrupt the
+   target with target_stop, and it's not safe to use that right away,
+   as we may be in the middle of handling some other event, and
+   target_stop changes infrun state.  */
+extern void mark_infrun_async_event_handler_ctrl_c ();
+
 /* The global chain of threads that need to do a step-over operation
    to get past e.g., a breakpoint.  */
 extern struct thread_info *global_thread_step_over_chain_head;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d31bcd98e84..a40b3c30012 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2142,19 +2142,6 @@ wait_for_signal ()
 {
   linux_nat_debug_printf ("about to sigsuspend");
   sigsuspend (&suspend_mask);
-
-  /* If the quit flag is set, it means that the user pressed Ctrl-C
-     and we're debugging a process that is running on a separate
-     terminal, so we must forward the Ctrl-C to the inferior.  (If the
-     inferior is sharing GDB's terminal, then the Ctrl-C reaches the
-     inferior directly.)  We must do this here because functions that
-     need to block waiting for a signal loop forever until there's an
-     event to report before returning back to the event loop.  */
-  if (!target_terminal::is_ours ())
-    {
-      if (check_quit_flag ())
-	target_pass_ctrlc ();
-    }
 }
 
 /* Wait for LP to stop.  Returns the wait status, or 0 if the LWP has
@@ -3317,8 +3304,32 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
       /* We shouldn't end up here unless we want to try again.  */
       gdb_assert (lp == NULL);
 
-      /* Block until we get an event reported with SIGCHLD.  */
+      /* Block until we get an event reported with SIGCHLD or a SIGINT
+	 interrupt.  */
       wait_for_signal ();
+
+      /* If the quit flag is set, it means that the user pressed
+	 Ctrl-C and we're debugging a process that is running on a
+	 separate terminal, so we must forward the Ctrl-C to the
+	 inferior.  (If the inferior is sharing GDB's terminal, then
+	 the Ctrl-C reaches the inferior directly.)  If we were
+	 interrupted by Ctrl-C, return back to the event loop and let
+	 it handle interrupting the target (or targets).  */
+
+      if (!target_terminal::is_ours () && check_quit_flag ())
+	{
+	  mark_infrun_async_event_handler_ctrl_c ();
+
+	  linux_nat_debug_printf ("exit (quit flag)");
+
+	  /* If we got a SIGCHLD, need to end up here again. */
+	  async_file_mark ();
+
+	  ourstatus->kind = TARGET_WAITKIND_IGNORE;
+
+	  restore_child_signals_mask (&prev_mask);
+	  return minus_one_ptid;
+	}
     }
 
   gdb_assert (lp);
diff --git a/gdb/target.c b/gdb/target.c
index 68d7e7c12d7..ab4dc7bd5dd 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -927,6 +927,21 @@ target_terminal::init (void)
   m_terminal_state = target_terminal_state::is_ours;
 }
 
+/* Called after switching the terminal to the inferior.  If the user
+   hit C-c before, pretend that it was hit right here.  Non-stop
+   targets however are more complicated though, as interruption with
+   "set non-stop off" + "maint set target-non-stop on" wants to stop
+   all threads individually with target_stop (and synchronously wait
+   for the stops), so we let the event loop handle it, when it's
+   recursion-safe to do so.  */
+
+static void
+maybe_pass_ctrlc ()
+{
+  if (!exists_non_stop_target () && check_quit_flag ())
+    target_pass_ctrlc ();
+}
+
 /* See target/target.h.  */
 
 void
@@ -961,8 +976,7 @@ target_terminal::inferior (void)
 
   /* If the user hit C-c before, pretend that it was hit right
      here.  */
-  if (check_quit_flag ())
-    target_pass_ctrlc ();
+  maybe_pass_ctrlc ();
 }
 
 /* See target/target.h.  */
@@ -998,8 +1012,7 @@ target_terminal::restore_inferior (void)
 
   /* If the user hit C-c before, pretend that it was hit right
      here.  */
-  if (check_quit_flag ())
-    target_pass_ctrlc ();
+  maybe_pass_ctrlc ();
 }
 
 /* Switch terminal state to DESIRED_STATE, either is_ours, or
@@ -3797,6 +3810,9 @@ target_interrupt ()
 void
 target_pass_ctrlc (void)
 {
+  /* Non-stop targets interrupt programs with target_stop instead.  */
+  gdb_assert (!exists_non_stop_target ());
+
   /* Pass the Ctrl-C to the first target that has a thread
      running.  */
   for (inferior *inf : all_inferiors ())
diff --git a/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp b/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp
index d21f580af87..e7e7a4c43a6 100644
--- a/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp
+++ b/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp
@@ -87,6 +87,9 @@ proc do_test {} {
 	    -re "Program received signal SIGINT.*\r\n$gdb_prompt $" {
 		send_log "$internal_pass (SIGINT)\n"
 	    }
+	    -re "Program stopped.*\r\n$gdb_prompt $" {
+		send_log "$internal_pass (stopped)\n"
+	    }
 	    -re "Quit\r\n$gdb_prompt $" {
 		send_log "$internal_pass (Quit)\n"
 
diff --git a/gdb/testsuite/gdb.base/interrupt-daemon-attach.exp b/gdb/testsuite/gdb.base/interrupt-daemon-attach.exp
index e43bfe4f4ad..840bbec2dc1 100644
--- a/gdb/testsuite/gdb.base/interrupt-daemon-attach.exp
+++ b/gdb/testsuite/gdb.base/interrupt-daemon-attach.exp
@@ -81,7 +81,7 @@ proc do_test {} {
 	}
 
 	after 500 {send_gdb "\003"}
-	gdb_test "" "(Program|Thread .*) received signal SIGINT.*" \
+	gdb_test "" "(Program|Thread .*) (received signal SIGINT|stopped).*" \
 	    "stop with control-c"
 
 	remote_exec host "kill -9 $child_pid"
diff --git a/gdb/testsuite/gdb.base/interrupt-daemon.exp b/gdb/testsuite/gdb.base/interrupt-daemon.exp
index 2270ab0cbc2..0b0b6dc3e80 100644
--- a/gdb/testsuite/gdb.base/interrupt-daemon.exp
+++ b/gdb/testsuite/gdb.base/interrupt-daemon.exp
@@ -53,7 +53,7 @@ proc do_test {} {
 
 	set test "ctrl-c stops process"
 	gdb_test_multiple "" $test {
-	    -re "received signal SIGINT.*\r\n$gdb_prompt $" {
+	    -re "(received signal SIGINT|stopped).*\r\n$gdb_prompt $" {
 		pass $test
 	    }
 	}
@@ -79,7 +79,7 @@ proc do_test {} {
 
 	set test "interrupt cmd stops process"
 	gdb_test_multiple "" $test {
-	    -re "received signal SIGINT" {
+	    -re "(received signal SIGINT|stopped)" {
 		pass $test
 	    }
 	}
diff --git a/gdb/testsuite/gdb.base/interrupt-noterm.exp b/gdb/testsuite/gdb.base/interrupt-noterm.exp
index 5cfc6171dd4..089b43305fe 100644
--- a/gdb/testsuite/gdb.base/interrupt-noterm.exp
+++ b/gdb/testsuite/gdb.base/interrupt-noterm.exp
@@ -67,7 +67,7 @@ gdb_test_multiple $test $test {
 
 set test "inferior received SIGINT"
 gdb_test_multiple "" $test {
-    -re "\r\nProgram received signal SIGINT.*" {
+    -re "\r\nProgram (received signal SIGINT|stopped).*" {
 	# This appears after the prompt, which was already consumed
 	# above.
 	pass $test
diff --git a/gdb/testsuite/gdb.base/interrupt.exp b/gdb/testsuite/gdb.base/interrupt.exp
index 41788ccade2..30ca6d44285 100644
--- a/gdb/testsuite/gdb.base/interrupt.exp
+++ b/gdb/testsuite/gdb.base/interrupt.exp
@@ -82,7 +82,7 @@ if ![file exists $binfile] then {
 	send_gdb "\003"
 	set msg "send_gdb control C"
 	gdb_test_multiple "" $msg {
-	    -re "Program received signal SIGINT.*$gdb_prompt $" {
+	    -re "Program (received signal SIGINT|stopped).*$gdb_prompt $" {
 		pass $msg
 	    }
 	}
@@ -166,7 +166,7 @@ if ![file exists $binfile] then {
 	    set msg "Send Control-C, second time"
 	    send_gdb "\003"
 	    gdb_test_multiple "" "$msg" {
-		-re "Program received signal SIGINT.*$gdb_prompt $" {
+		-re "Program (received signal SIGINT|stopped).*$gdb_prompt $" {
 		    pass "$msg"
 		}
 	    }
diff --git a/gdb/testsuite/gdb.base/random-signal.exp b/gdb/testsuite/gdb.base/random-signal.exp
index 8511e83fce5..4c8806ce367 100644
--- a/gdb/testsuite/gdb.base/random-signal.exp
+++ b/gdb/testsuite/gdb.base/random-signal.exp
@@ -47,7 +47,8 @@ proc do_test {} {
     # For this to work we must be sure to consume the "Continuing."
     # message first, or GDB's signal handler may not be in place.
     after 500 {send_gdb "\003"}
-    gdb_test "" "Program received signal SIGINT.*" "stop with control-c"
+    gdb_test "" "Program (received signal SIGINT|stopped).*" \
+	"stop with control-c"
 }
 
 # With native debugging and "run" (with job control), the ctrl-c
diff --git a/gdb/testsuite/gdb.base/range-stepping.exp b/gdb/testsuite/gdb.base/range-stepping.exp
index c1ba8ef71cb..21bd7326370 100644
--- a/gdb/testsuite/gdb.base/range-stepping.exp
+++ b/gdb/testsuite/gdb.base/range-stepping.exp
@@ -193,7 +193,7 @@ if ![target_info exists gdb,nointerrupts] {
 		incr vcont_r_counter
 		exp_continue
 	    }
-	    -re "Program received signal SIGINT.*$gdb_prompt $" {
+	    -re "Program (received signal SIGINT|stopped).*$gdb_prompt $" {
 		pass $test
 	    }
 	}
diff --git a/gdb/testsuite/gdb.base/sigint-masked-out.exp b/gdb/testsuite/gdb.base/sigint-masked-out.exp
index 0391152e93a..77a599d12ef 100644
--- a/gdb/testsuite/gdb.base/sigint-masked-out.exp
+++ b/gdb/testsuite/gdb.base/sigint-masked-out.exp
@@ -36,6 +36,8 @@ proc_with_prefix test_ctrl_c {} {
 	return
     }
 
+    set can_interrupt [can_interrupt_blocked_sigint]
+
     gdb_test_multiple "continue" "" {
 	-re "Continuing" {
 	    pass $gdb_test_name
@@ -52,7 +54,9 @@ proc_with_prefix test_ctrl_c {} {
 	    pass $gdb_test_name
 	}
 	timeout {
-	    setup_kfail "gdb/9425" *-*-*
+	    if {!$can_interrupt} {
+		setup_kfail "gdb/9425" *-*-*
+	    }
 	    fail "$gdb_test_name (timeout)"
 	}
     }
@@ -71,6 +75,8 @@ proc_with_prefix test_interrupt_cmd {} {
 	return
     }
 
+    set can_interrupt [can_interrupt_blocked_sigint]
+
     gdb_test_multiple "continue&" "" {
 	-re "Continuing\\.\r\n$gdb_prompt " {
 	    pass $gdb_test_name
@@ -91,7 +97,9 @@ proc_with_prefix test_interrupt_cmd {} {
 	    pass $gdb_test_name
 	}
 	timeout {
-	    setup_kfail "gdb/14559" *-*-*
+	    if {!$can_interrupt} {
+		setup_kfail "gdb/14559" *-*-*
+	    }
 	    fail "$gdb_test_name (timeout)"
 	}
     }
diff --git a/gdb/testsuite/gdb.base/sigint-sigwait.exp b/gdb/testsuite/gdb.base/sigint-sigwait.exp
index 1dd706fc1a4..e37a2d2cbef 100644
--- a/gdb/testsuite/gdb.base/sigint-sigwait.exp
+++ b/gdb/testsuite/gdb.base/sigint-sigwait.exp
@@ -37,6 +37,8 @@ proc_with_prefix test_ctrl_c {} {
 	return
     }
 
+    set can_interrupt [can_interrupt_blocked_sigint]
+
     gdb_test_multiple "continue" "" {
 	-re "Continuing" {
 	    pass $gdb_test_name
@@ -54,7 +56,9 @@ proc_with_prefix test_ctrl_c {} {
 	    pass $gdb_test_name
 	}
 	-re -wrap "Inferior.*exited normally.*" {
-	    setup_kfail "gdb/9425" *-*-*
+	    if {!$can_interrupt} {
+		setup_kfail "gdb/9425" *-*-*
+	    }
 	    fail "$gdb_test_name (the program exited)"
 	}
     }
@@ -73,6 +77,8 @@ proc_with_prefix test_interrupt_cmd {} {
 	return
     }
 
+    set can_interrupt [can_interrupt_blocked_sigint]
+
     gdb_test_multiple "continue&" "" {
 	-re "Continuing\\.\r\n$gdb_prompt " {
 	    pass $gdb_test_name
@@ -95,7 +101,9 @@ proc_with_prefix test_interrupt_cmd {} {
 	    pass $gdb_test_name
 	}
 	-re "Inferior.*exited normally" {
-	    setup_kfail "gdb/14559" *-*-*
+	    if {!$can_interrupt} {
+		setup_kfail "gdb/14559" *-*-*
+	    }
 	    fail "$gdb_test_name (the program exited)"
 	}
     }
diff --git a/gdb/testsuite/gdb.gdb/selftest.exp b/gdb/testsuite/gdb.gdb/selftest.exp
index bee3010bca1..7679131b8e7 100644
--- a/gdb/testsuite/gdb.gdb/selftest.exp
+++ b/gdb/testsuite/gdb.gdb/selftest.exp
@@ -97,7 +97,7 @@ proc test_with_self { } {
 	send_gdb "\003"
 	# "Thread 1" is displayed iff Guile support is linked in.
 	gdb_expect {
-	    -re "(Thread .*|Program) received signal SIGINT.*$gdb_prompt $" {
+	    -re "(Thread .*|Program) (received signal SIGINT|stopped).*$gdb_prompt $" {
 		pass "$description"
 	    }
 	    -re ".*$gdb_prompt $" {
@@ -119,7 +119,7 @@ proc test_with_self { } {
     set description "send ^C to child process again"
     send_gdb "\003"
     gdb_expect {
-	-re "(Thread .*|Program) received signal SIGINT.*$gdb_prompt $" {
+	-re "(Thread .*|Program) (received signal SIGINT|stopped).*$gdb_prompt $" {
 	    pass "$description"
 	}
 	-re ".*$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp b/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
index 887bd60abcd..8eadc2f6f22 100644
--- a/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
+++ b/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
@@ -92,7 +92,7 @@ proc do_test {sync_command} {
 	gdb_test_multiple "interrupt" "$message" {
 	    -re "$gdb_prompt " {
 		gdb_test_multiple "" "$message" {
-		    -re "received signal SIGINT" {
+		    -re "(received signal SIGINT|stopped)" {
 			pass $message
 		    }
 		}
diff --git a/gdb/testsuite/gdb.multi/multi-target-interrupt.exp b/gdb/testsuite/gdb.multi/multi-target-interrupt.exp
index cafd60d31b1..36b47a40c1a 100644
--- a/gdb/testsuite/gdb.multi/multi-target-interrupt.exp
+++ b/gdb/testsuite/gdb.multi/multi-target-interrupt.exp
@@ -46,7 +46,7 @@ proc test_ctrlc {} {
 
 	set msg "send_gdb control C"
 	gdb_test_multiple "" $msg {
-	    -re "received signal SIGINT.*$gdb_prompt $" {
+	    -re "(received signal SIGINT|stopped).*$gdb_prompt $" {
 		pass $msg
 	    }
 	}
diff --git a/gdb/testsuite/gdb.multi/multi-target-no-resumed.exp b/gdb/testsuite/gdb.multi/multi-target-no-resumed.exp
index 6e84d9a6949..0ce26f28199 100644
--- a/gdb/testsuite/gdb.multi/multi-target-no-resumed.exp
+++ b/gdb/testsuite/gdb.multi/multi-target-no-resumed.exp
@@ -59,7 +59,7 @@ proc test_no_resumed_infs {inf_A inf_B} {
     # Now stop the program (all targets).
     send_gdb "\003"
     gdb_test_multiple "" "send_gdb control C" {
-	-re "received signal SIGINT.*$gdb_prompt $" {
+	-re "(received signal SIGINT|stopped).*$gdb_prompt $" {
 	    pass $gdb_test_name
 	}
     }
diff --git a/gdb/testsuite/gdb.multi/multi-term-settings.exp b/gdb/testsuite/gdb.multi/multi-term-settings.exp
index 5275391a73c..7796d508083 100644
--- a/gdb/testsuite/gdb.multi/multi-term-settings.exp
+++ b/gdb/testsuite/gdb.multi/multi-term-settings.exp
@@ -278,7 +278,7 @@ proc coretest {inf1_how inf2_how} {
     if {$expect_ttou} {
 	gdb_test "" "Quit" "stop with control-c (Quit)"
     } else {
-	gdb_test "" "received signal SIGINT.*" "stop with control-c (SIGINT)"
+	gdb_test "" "(received signal SIGINT|stopped).*" "stop with control-c (SIGINT)"
     }
 
     # Useful for debugging in case the Ctrl-C above fails.
diff --git a/gdb/testsuite/gdb.server/reconnect-ctrl-c.exp b/gdb/testsuite/gdb.server/reconnect-ctrl-c.exp
index 11aa5147c78..2121c47e873 100644
--- a/gdb/testsuite/gdb.server/reconnect-ctrl-c.exp
+++ b/gdb/testsuite/gdb.server/reconnect-ctrl-c.exp
@@ -49,7 +49,7 @@ with_test_prefix "preparation" {
   gdb_test "disconnect" ".*"
 }
 
-# Connect, continue, send Ctrl-C and expect a SIGINT stop.
+# Connect, continue, send Ctrl-C and expect a stop.
 
 proc connect_continue_ctrl_c {} {
     global gdbserver_protocol gdbserver_gdbport
@@ -67,7 +67,7 @@ proc connect_continue_ctrl_c {} {
     }
 
     after 1000 {send_gdb "\003"}
-    gdb_test "" "Program received signal SIGINT.*" "stop with control-c"
+    gdb_test "" "Program (received signal SIGINT|stopped).*" "stop with control-c"
 }
 
 with_test_prefix "first" {
diff --git a/gdb/testsuite/gdb.threads/async.exp b/gdb/testsuite/gdb.threads/async.exp
index c49deb1f8b5..f7be1271b7b 100644
--- a/gdb/testsuite/gdb.threads/async.exp
+++ b/gdb/testsuite/gdb.threads/async.exp
@@ -80,7 +80,7 @@ proc test_current_thread {expected_thr} {
     gdb_test_multiple $test $test {
 	-re "^interrupt\r\n$gdb_prompt " {
 	    gdb_test_multiple "" $test {
-		-re "Thread .* received signal SIGINT, Interrupt\\." {
+		-re "Thread .* (received signal SIGINT, Interrupt|stopped)\\." {
 		    pass $test
 		}
 	    }
diff --git a/gdb/testsuite/gdb.threads/continue-pending-status.exp b/gdb/testsuite/gdb.threads/continue-pending-status.exp
index c0321c39d11..ca82290e260 100644
--- a/gdb/testsuite/gdb.threads/continue-pending-status.exp
+++ b/gdb/testsuite/gdb.threads/continue-pending-status.exp
@@ -116,7 +116,7 @@ for {set i 0} {$i < $attempts} {incr i} {
 
 	set msg "caught interrupt"
 	gdb_test_multiple "" $msg {
-	    -re "Thread .* received signal SIGINT.*$gdb_prompt $" {
+	    -re "Thread .* (received signal SIGINT|stopped).*$gdb_prompt $" {
 		pass $msg
 	    }
 	}
diff --git a/gdb/testsuite/gdb.threads/leader-exit.exp b/gdb/testsuite/gdb.threads/leader-exit.exp
index fe06e6d4f29..fb76bba3e3b 100644
--- a/gdb/testsuite/gdb.threads/leader-exit.exp
+++ b/gdb/testsuite/gdb.threads/leader-exit.exp
@@ -55,7 +55,7 @@ send_gdb "\003"
 
 set test "caught interrupt"
 gdb_test_multiple "" $test {
-    -re "Thread .* received signal SIGINT.*$gdb_prompt $" {
+    -re "Thread .* (received signal SIGINT|stopped).*$gdb_prompt $" {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/gdb.threads/manythreads.exp b/gdb/testsuite/gdb.threads/manythreads.exp
index b0004b912b7..992c46a3874 100644
--- a/gdb/testsuite/gdb.threads/manythreads.exp
+++ b/gdb/testsuite/gdb.threads/manythreads.exp
@@ -56,7 +56,7 @@ gdb_test_multiple "continue" "first continue" {
 # we don't lose GDB's output while we do it.
 remote_expect host 1 { timeout { } }
 
-# Send a Ctrl-C and wait for the SIGINT.
+# Send a Ctrl-C and wait for the stop.
 
 proc interrupt_and_wait { message } {
     global gdb_prompt
@@ -70,7 +70,7 @@ proc interrupt_and_wait { message } {
 	-re "\\\[\[^\]\]* exited\\\]\r\n" {
 	    exp_continue
 	}
-	-re " received signal SIGINT.*$gdb_prompt $" {
+	-re " (received signal SIGINT|stopped).*$gdb_prompt $" {
 	    pass "$message"
 	}
 	-re "$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.threads/pthreads.exp b/gdb/testsuite/gdb.threads/pthreads.exp
index 86258b1d874..4bf8544db52 100644
--- a/gdb/testsuite/gdb.threads/pthreads.exp
+++ b/gdb/testsuite/gdb.threads/pthreads.exp
@@ -204,7 +204,7 @@ proc check_control_c {} {
     send_gdb "\003"
     set description "Stopped with a ^C"
     gdb_expect {
-	-re "Thread .* received signal SIGINT.*$gdb_prompt $" {
+	-re "Thread .* (received signal SIGINT|stopped).*$gdb_prompt $" {
 	    pass $description
 	}
 	-re "Quit.*$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.threads/schedlock.exp b/gdb/testsuite/gdb.threads/schedlock.exp
index 90c70587220..ab25c3d0388 100644
--- a/gdb/testsuite/gdb.threads/schedlock.exp
+++ b/gdb/testsuite/gdb.threads/schedlock.exp
@@ -68,14 +68,9 @@ proc stop_process { description } {
   # For this to work we must be sure to consume the "Continuing."
   # message first, or GDB's signal handler may not be in place.
   after 1000 {send_gdb "\003"}
-  gdb_expect {
-    -re "Thread .* received signal SIGINT.*$gdb_prompt $"
-      {
-	pass $description
-      }
-    timeout
-      {
-	fail "$description (timeout)"
+  gdb_test_multiple "" $description {
+      -re "Thread .* (received signal SIGINT|stopped).*$gdb_prompt $" {
+	  pass $description
       }
   }
 }
diff --git a/gdb/testsuite/gdb.threads/sigthread.exp b/gdb/testsuite/gdb.threads/sigthread.exp
index 867aef9a710..43f39406380 100644
--- a/gdb/testsuite/gdb.threads/sigthread.exp
+++ b/gdb/testsuite/gdb.threads/sigthread.exp
@@ -49,4 +49,4 @@ after 500 {send_gdb "\003"}
 
 # Make sure we do not get an internal error from hitting Control-C
 # while many signals are flying back and forth.
-gdb_test "" "Thread .* received signal SIGINT.*" "stop with control-c"
+gdb_test "" "Thread .* (received signal SIGINT|stopped).*" "stop with control-c"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index d8c684c7238..5aeebb3704b 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2870,6 +2870,25 @@ gdb_caching_proc supports_memtag {
     return 0
 }
 
+# Return true if the target is able to interrupt a program that blocks
+# SIGINT.
+proc can_interrupt_blocked_sigint {} {
+    if {[istarget *-*-linux*]} {
+	gdb_test_multiple "maint show target-non-stop" "" {
+	    -re "(is|currently) on.*$::gdb_prompt $" {
+	    }
+	    -re "(is|currently) off.*$::gdb_prompt $" {
+		# GDBserver still tries to interrupt programs with
+		# SIGINT.
+		return 0
+	    }
+	}
+    }
+
+    # Assume possible.
+    return 1
+}
+
 # Return 1 if the target supports hardware single stepping.
 
 proc can_hardware_single_step {} {
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (14 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 15/16] GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR gdb/9425, PR gdb/14559] Pedro Alves
@ 2021-06-14 21:24 ` Pedro Alves
  2021-06-15 12:56   ` Eli Zaretskii via Gdb-patches
  2021-06-15 12:34 ` [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Eli Zaretskii via Gdb-patches
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-14 21:24 UTC (permalink / raw)
  To: gdb-patches

This documents changes done in previous patches:

 - the fact that on GNU/Linux, GDB creates a pseudo-terminal for the
   inferior instead of juggling terminal settings.

 - That when the inferior and GDB share the terminal, you can't
   interrupt some programs with Ctrl-C.

 - That on GNU/Linux, you may get "Program stopped." instead of
   "Program received SIGINT" in response to Ctrl-C.

 - That run+detach may result in the program dying with SIGHUP.

I was surprised that we do not currently have a node/section
specifically to talk about interrupting programs.  Thus I've added a
new "Interrupting" section under the "Stopping and Continuing"
chapter, with some xrefs to other sections.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* NEWS: Document pseudo-terminal, "tty /dev/tty" and Ctrl-C/SIGINT
	changes.  Document "set/show debug managed-tty".

gdb/doc/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.texinfo (Input/Output): Document that GDB may start a
	program associated with a pseudo-terminal.  Document "tty
	/dev/tty".  Document "set/show debug managed-tty".
	(Attach): Document what happens on run+detach on systems where GDB
	creates a pseudo-terminal for the inferior.
	(Stopping and Continuing): Add new Interrupting node.
	(Background Execution): Add anchor.
	(Features for Debugging MS Windows PE Executables): Add anchor.

Change-Id: I267a0f9300c7ac4d2e7f14a9ba8eabc1eafcc5a7
---
 gdb/doc/gdb.texinfo | 117 ++++++++++++++++++++++++++++++++++++++++++--
 gdb/NEWS            |  23 +++++++++
 2 files changed, 135 insertions(+), 5 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d09b86cda95..a429e9299c6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2983,11 +2983,27 @@ current working directory of the debuggee.
 @cindex redirection
 @cindex i/o
 @cindex terminal
-By default, the program you run under @value{GDBN} does input and output to
-the same terminal that @value{GDBN} uses.  @value{GDBN} switches the terminal
-to its own terminal modes to interact with you, but it records the terminal
-modes your program was using and switches back to them when you continue
-running your program.
+By default, the program you run under @value{GDBN} does (or
+@value{GDBN} provides the illusion that it does) input and output to
+the same terminal that @value{GDBN} uses.
+
+Depending on the operating system and configuration, either:
+
+@itemize
+
+@item
+@value{GDBN} switches the terminal to its own terminal modes to
+interact with you, but it records the terminal modes your program was
+using and switches back to them when you continue running your
+program.  This is the default on most systems.
+
+@item
+@value{GDBN} creates a pseudo-terminal, sets your program to use it
+for standard input and standard output, and forwards input and output
+to and from @value{GDBN}'s terminal at appropriate times.  This is the
+default on GNU/Linux.
+
+@end itemize
 
 @table @code
 @kindex info terminal
@@ -3023,6 +3039,30 @@ directs that processes started with subsequent @code{run} commands
 default to do input and output on the terminal @file{/dev/ttyb} and have
 that as their controlling terminal.
 
+On some operating systems, by default, @value{GDBN} creates a
+pseudo-terminal, and sets your program to use it for standard input
+and standard output.  @value{GDBN} takes care of forwarding input and
+output to and from @value{GDBN}'s terminal at appropriate times, so
+this is largely transparent.
+
+On such systems, in some cases, like for example if you need to run
+your program and then detach it, and you want the program to remain
+associated with a terminal, you may prefer that @value{GDBN} starts
+your program using the same device for standard input and output as
+@value{GDBN} is using.
+
+Specifying the special @file{/dev/tty} file as input and output device
+is interpreted as a request for doing I/O to the same terminal that
+@value{GDBN} uses directly, skipping creation of an intermediary
+pseudo-terminal on systems where @value{GDBN} creates one by default.
+
+Note that on GNU/Linux at least, a consequence of using the same
+terminal as @value{GDBN} directly is that programs that block or
+ignore the @code{SIGINT} signal (with e.g., the @code{sigprocmask}
+function), or use the @code{sigwait} function to wait for
+@code{SIGINT} signals can not be interrupted by typing the interrupt
+character (often @kbd{Ctrl-c}).
+
 An explicit redirection in @code{run} overrides the @code{tty} command's
 effect on the input/output device, but not its effect on the controlling
 terminal.
@@ -3050,6 +3090,21 @@ restores the default behavior, which is to use the same terminal as
 Show the current tty for the program being debugged.
 @end table
 
+The management of @value{GDBN}-created terminals can be inspected
+using:
+
+@table @code
+@kindex set debug managed-tty
+@item set debug managed-tty @r{[}on|off@r{]}
+Set whether to print debug output about @value{GDBN}-managed
+terminals.
+
+@kindex show debug managed-tty
+@item show debug managed-tty
+Show whether the debug output about @value{GDBN}-managed terminals is
+printed.
+@end table
+
 @node Attach
 @section Debugging an Already-running Process
 @kindex attach
@@ -3132,6 +3187,16 @@ things; you can control whether or not you need to confirm by using the
 @code{set confirm} command (@pxref{Messages/Warnings, ,Optional Warnings and
 Messages}).
 
+On systems where @value{GDBN} creates a pseudo-terminal for spawned
+inferiors, detaching from a process that was started by @value{GDBN}
+(for example with the @code{run} command) results in the process
+losing its terminal right after detaching, because @value{GDBN}
+destroys the pseudo-terminal device pair.  If you prefer, you can use
+the @code{tty} command command to instruct @value{GDBN} to start your
+program doing I/O to the same terminal that @value{GDBN} uses or to
+some other terminal.  @xref{Input/Output, ,Your Program's Input and
+Output}.
+
 @node Kill Process
 @section Killing the Child Process
 
@@ -4224,6 +4289,7 @@ running or not, what process it is, and why it stopped.
 @menu
 * Breakpoints::                 Breakpoints, watchpoints, and catchpoints
 * Continuing and Stepping::     Resuming execution
+* Interrupting::                Interrupting execution
 * Skipping Over Functions and Files::
                                 Skipping over functions and files
 * Signals::                     Signals
@@ -6376,6 +6442,45 @@ default is @code{on}.
 
 @end table
 
+@node Interrupting
+@section Interrupting
+
+You can stop the program while it is running in the foreground by
+pressing the interrupt character (often @kbd{Ctrl-c}).  If the program
+is executing in the background (@pxref{Background Execution}), you can
+use the @code{interrupt} command (@pxref{interrupt}).
+
+Depending on operating system and configuration, this results in
+interrupting the program with either a @code{SIGINT} signal:
+
+@smallexample
+Program received signal SIGINT, Interrupt.
+@end smallexample
+
+@noindent
+or plainly suspending the program:
+
+@smallexample
+Program stopped.
+@end smallexample
+
+The terminal the program was started with affects whether you get the
+former or the latter.  @xref{Input/Output, ,Your Program's Input and
+Output}.
+
+On systems where interrupting the program results in a plain
+suspension instead of the program receiving a @code{SIGINT} signal,
+you can still pass a @code{SIGINT} signal to the program after it
+stops, using either the @code{signal SIGINT} or @code{queue-signal
+SIGINT} commands.  @xref{Signaling,,Giving your Program a Signal}.
+
+The remote target supports a number of options to configure how the
+remote program is interrupted.  @xref{Remote Configuration}.
+
+@value{GDBN} on MS-Windows supports @kbd{C-@key{BREAK}} as an
+alternative interrupt key sequence.  @xref{interrupt debuggee on
+MS-Windows}.
+
 @node Skipping Over Functions and Files
 @section Skipping Over Functions and Files
 @cindex skipping over functions and files
@@ -7090,6 +7195,7 @@ using the @code{interrupt} command.
 
 @table @code
 @kindex interrupt
+@anchor{interrupt}
 @item interrupt
 @itemx interrupt -a
 
@@ -24219,6 +24325,7 @@ DLLs with and without symbolic debugging information.
 
 @cindex Ctrl-BREAK, MS-Windows
 @cindex interrupt debuggee on MS-Windows
+@anchor{interrupt debuggee on MS-Windows}
 MS-Windows programs that call @code{SetConsoleMode} to switch off the
 special meaning of the @samp{Ctrl-C} keystroke cannot be interrupted
 by typing @kbd{C-c}.  For this reason, @value{GDBN} on MS-Windows
diff --git a/gdb/NEWS b/gdb/NEWS
index 56743fc9aea..90361d3d330 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -6,6 +6,25 @@
 * The 'set disassembler-options' command now supports specifying options
   for the ARC target.
 
+* On GNU/Linux, by default GDB now starts programs associated with a
+  pseudo-terminal slave device created and managed by GDB, instead of
+  having the inferior use the same terminal as GDB directly.  GDB
+  takes care of forwarding input and output to and from GDB's terminal
+  at appropriate times, so this is largely transparent.
+
+  In addition, by default, Ctrl-C no longer stops the program with
+  SIGINT.  Instead you'll see for example:
+
+     Thread 1 "main" stopped.
+
+  With these changes it is now possible to interrupt programs that
+  block or ignore the SIGINT signal (with e.g., the sigprocmask
+  function), or use the sigwait function to wait for SIGINT signal.
+
+  You can use the "tty /dev/tty" command to restore the previous
+  behavior of spawning programs associated with the same terminal as
+  GDB.
+
 * GDB now supports general memory tagging functionality if the underlying
   architecture supports the proper primitives and hooks.  Currently this is
   enabled only for AArch64 MTE.
@@ -84,6 +103,10 @@ set debug event-loop
 show debug event-loop
   Control the display of debug output about GDB's event loop.
 
+set debug managed-tty
+show debug managed-tty
+  Control the display of debug output about GDB-managed terminals.
+
 set print memory-tag-violations
 show print memory-tag-violations
   Control whether to display additional information about memory tag violations
-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (15 preceding siblings ...)
  2021-06-14 21:24 ` [PATCH v2 16/16] Document pseudo-terminal and interrupting changes Pedro Alves
@ 2021-06-15 12:34 ` Eli Zaretskii via Gdb-patches
  2021-06-16 11:27   ` Pedro Alves
  2021-06-18 10:12 ` Andrew Burgess
  2021-06-24 18:12 ` Konstantin Kharlamov via Gdb-patches
  18 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii via Gdb-patches @ 2021-06-15 12:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 14 Jun 2021 22:23:54 +0100
> 
> Currently, on GNU/Linux, it is not possible to interrupt with Ctrl-C
> programs that block or ignore SIGINT, with e.g., sigprocmask or
> signal(SIGINT, SIG_IGN).  You type Ctrl-C, but nothing happens.

For the reasons I explained in my other message, can we please talk
about "stopping a program" instead of "interrupting"?  Both in the log
message (and the discussion) and in the documentation, please.  At
least for me, the use of "interrupt" was confusing.

> This series address the problem by turning Ctrl-C handling around such
> that the SIGINT always reaches GDB first, not the inferior.  That is
> done by making GDB put inferiors in their own terminal/session created
> by GDB.  I.e., GDB creates a pseudo-terminal master/slave pair, makes
> the inferior run with the slave as its terminal, and pumps
> output/input on the master end.

Is this "pumping" really-truly 100% transparent to the program being
debugged?  I understand how a program that uses stdio will simply read
the same stuff that was "pumped" from GDB's terminal, but what about
other kinds of input and settings?

For example, some text-mode programs change the settings of the
terminal via termios -- will those settings be reflected in the GDB's
terminal when the debuggee reads or writes from its terminal?  Or what
about support for terminfo functions -- will the commands be passed
back and forth between the terminals and the user will see the effects
he/she expects?  Some terminal emulators support advanced features
like cut/paste -- will that also work with this pumping?  And if the
program changes the character that causes SIGINT (Emacs does that when
it runs on a text-mode terminal), will that change the interrupt
character for GDB as well?

> The series will then make GDB interrupt the program with SIGSTOP
> instead of SIGINT, which always works even if the inferior
> blocks/ignores SIGINT -- SIGSTOP can't be ignored.

And if the user then types "continue", will SIGINT be delivered to the
program, or will the Ctrl-C keystroke be "swallowed" by GDB?

> Having the inferior in its own terminal also means that GDB is in
> control of when inferior output is flushed to the screen.  When
> debugging with the CLI, this means that inferior output is now never
> interpersed with GDB's output in an unreadable fashion.

I guess this will be done on the screen line level?  Because if a
program uses terminfo or curses to write to different parts of the
screen, the outputs of the program and GDB will mix anyhow, just not
on the same screen line, right?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-14 21:24 ` [PATCH v2 16/16] Document pseudo-terminal and interrupting changes Pedro Alves
@ 2021-06-15 12:56   ` Eli Zaretskii via Gdb-patches
  2021-06-16  9:31     ` Pedro Alves
  2021-06-16 10:15     ` Pedro Alves
  0 siblings, 2 replies; 34+ messages in thread
From: Eli Zaretskii via Gdb-patches @ 2021-06-15 12:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 14 Jun 2021 22:24:10 +0100
> 
> +Depending on the operating system and configuration, either:
> +
> +@itemize
> +
> +@item
> +@value{GDBN} switches the terminal to its own terminal modes to
> +interact with you, but it records the terminal modes your program was
> +using and switches back to them when you continue running your
> +program.  This is the default on most systems.
> +
> +@item
> +@value{GDBN} creates a pseudo-terminal, sets your program to use it
> +for standard input and standard output, and forwards input and output
> +to and from @value{GDBN}'s terminal at appropriate times.  This is the
> +default on GNU/Linux.

This doesn't read well.  I suggest to make "Either" and "Or" part of
the text, like this:

  Depending on the operating system and configuration:

  @itemize

  @item
  Either @value{GDBN} switches the terminal ...

  @item
  Or @value{GDB} creates a pseudo-terminal ...

> +@value{GDBN} creates a pseudo-terminal, sets your program to use it
> +for standard input and standard output
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This begs the question about the standard error stream.

> +On some operating systems, by default, @value{GDBN} creates a
> +pseudo-terminal, and sets your program to use it for standard input
> +and standard output.  @value{GDBN} takes care of forwarding input and
> +output to and from @value{GDBN}'s terminal at appropriate times, so
> +this is largely transparent.

The 2 different behaviors are described somewhat inconsistently: in
one place you say "most systems" vs "GNU/Linux", in another place
"some operating systems" instead of "GNU/Linux", and in yet another
place you say "on systems where GDB creates a pseudo-terminal".  Can
you describe the pseudo-terminal feature consistently as a GNU/Linux
feature?

Also, describing the feature in a single place, and just having a
cross-reference to there in other places will go a long way towards
making the description clear and complete.

> +On such systems, in some cases, like for example if you need to run
> +your program and then detach it, and you want the program to remain
> +associated with a terminal, you may prefer that @value{GDBN} starts
> +your program using the same device for standard input and output as
> +@value{GDBN} is using.

IMHO, this is a misfeature.  If the terminal from which GDB was run
remains on the system, it would be an unpleasant surprise for the user
that the program gets hit by SIGHUP when you detach.  I think we
should try to avoid this side effect, if that's feasible.

> +@node Interrupting
> +@section Interrupting

Once again, I'd prefer to talk about "stopping" instead.  Or maybe
even "getting control to GDB".

> +Depending on operating system and configuration, this results in
> +interrupting the program with either a @code{SIGINT} signal:
> +
> +@smallexample
> +Program received signal SIGINT, Interrupt.
> +@end smallexample
> +
> +@noindent
> +or plainly suspending the program:
> +
> +@smallexample
> +Program stopped.
> +@end smallexample

This seems to be inaccurate.  On MS-Windows, for example, I see
something different:

  Thread 4 received signal SIGTRAP, Trace/breakpoint trap.

So maybe you want to say that the above is only true for Posix hosts.
Or maybe just be more vague and don't try quoting the messages?

> +On systems where interrupting the program results in a plain
> +suspension instead of the program receiving a @code{SIGINT} signal,
> +you can still pass a @code{SIGINT} signal to the program after it
> +stops, using either the @code{signal SIGINT} or @code{queue-signal
> +SIGINT} commands.  @xref{Signaling,,Giving your Program a Signal}.

This begs a question I already asked elsewhere: I'd expect that
continuing the program after it was stopped like that will deliver
SIGINT to the program, without any special commands.  Isn't that so?
Your text seems to imply that it isn't, which I find surprising --
after all, the user pressed Ctrl-C, so "normally" the debuggee should
be hit with SIGINT, as if we were not debugging it.

> +@value{GDBN} on MS-Windows supports @kbd{C-@key{BREAK}} as an
> +alternative interrupt key sequence.  @xref{interrupt debuggee on
> +MS-Windows}.

I'm not sure I understand the significance of this note: after all, a
Windows program can install a Ctrl-BREAK handler exactly like it does
with Ctrl-C.  Is this only about SetConsoleMode?

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-15 12:56   ` Eli Zaretskii via Gdb-patches
@ 2021-06-16  9:31     ` Pedro Alves
  2021-06-16 12:29       ` Eli Zaretskii via Gdb-patches
  2021-06-16 10:15     ` Pedro Alves
  1 sibling, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-16  9:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Hi!

On 2021-06-15 1:56 p.m., Eli Zaretskii wrote:
>> +On such systems, in some cases, like for example if you need to run
>> +your program and then detach it, and you want the program to remain
>> +associated with a terminal, you may prefer that @value{GDBN} starts
>> +your program using the same device for standard input and output as
>> +@value{GDBN} is using.
> IMHO, this is a misfeature.  If the terminal from which GDB was run
> remains on the system, it would be an unpleasant surprise for the user
> that the program gets hit by SIGHUP when you detach.  I think we
> should try to avoid this side effect, if that's feasible.
> 

Hmm, I'm not sure I follow what you're saying here, maybe I'm misunderstanding.

We no longer get a SIGHUP on detach, that was fixed on v2.  What the text is saying now is
different: because when you start your program with "run", the terminal the inferior is
associated with was created by GDB, when you "detach", GDB destroys the terminal.  The
result is that the program you detached from loses its terminal.  From the detached
program's perspective, it's the same as with current GDB when you do "run", then "detach",
and then close the GDB terminal.  The difference is that with my patches the program
loses its terminal a step earlier.  In v1, the detached program would also get a SIGHUP, 
but that was fixed in v2, it no longer does.

If you really need to do "run" and then "detach" for some reason, _and_ you want
your program to have a terminal, you should start the program with a specified
terminal, like:

 - start the program outside gdb, and then use "attach".  Make sure the terminal
   you started the program at remains alive.

 - use the "tty" command to tell GDB to start the program on some specified terminal 
   (which will outlive gdb and gdb's terminal)

If you're sure gdb's terminal will stay around, and you want to tell GDB to start
the program on that terminal, you can use the "tty /dev/tty" command before starting
the program.

Is this clearer?  I can try to adjust the documentation to explain this better and be more
descriptive like above. 

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-15 12:56   ` Eli Zaretskii via Gdb-patches
  2021-06-16  9:31     ` Pedro Alves
@ 2021-06-16 10:15     ` Pedro Alves
  2021-06-16 12:15       ` Eli Zaretskii via Gdb-patches
  1 sibling, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-16 10:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Hi Eli,

It will take me a bit to actually go implement changes, but here are some responses.

On 2021-06-15 1:56 p.m., Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Date: Mon, 14 Jun 2021 22:24:10 +0100
>>
>> +Depending on the operating system and configuration, either:
>> +
>> +@itemize
>> +
>> +@item
>> +@value{GDBN} switches the terminal to its own terminal modes to
>> +interact with you, but it records the terminal modes your program was
>> +using and switches back to them when you continue running your
>> +program.  This is the default on most systems.
>> +
>> +@item
>> +@value{GDBN} creates a pseudo-terminal, sets your program to use it
>> +for standard input and standard output, and forwards input and output
>> +to and from @value{GDBN}'s terminal at appropriate times.  This is the
>> +default on GNU/Linux.
> 
> This doesn't read well.  I suggest to make "Either" and "Or" part of
> the text, like this:
> 
>   Depending on the operating system and configuration:
> 
>   @itemize
> 
>   @item
>   Either @value{GDBN} switches the terminal ...
> 
>   @item
>   Or @value{GDB} creates a pseudo-terminal ...
> 
>> +@value{GDBN} creates a pseudo-terminal, sets your program to use it
>> +for standard input and standard output
>        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This begs the question about the standard error stream.

Indeed.  I think I'll drop the word "standard".

> 
>> +On some operating systems, by default, @value{GDBN} creates a
>> +pseudo-terminal, and sets your program to use it for standard input
>> +and standard output.  @value{GDBN} takes care of forwarding input and
>> +output to and from @value{GDBN}'s terminal at appropriate times, so
>> +this is largely transparent.
> 
> The 2 different behaviors are described somewhat inconsistently: in
> one place you say "most systems" vs "GNU/Linux", in another place
> "some operating systems" instead of "GNU/Linux", and in yet another
> place you say "on systems where GDB creates a pseudo-terminal".  Can
> you describe the pseudo-terminal feature consistently as a GNU/Linux
> feature?

Yeah, I'll take a fresh look.  I was thinking that in the future other Unix
hosts will be switched to use this feature as well, and we tend to forget updating
such references in the manual.

> 
> Also, describing the feature in a single place, and just having a
> cross-reference to there in other places will go a long way towards
> making the description clear and complete.
> 
>> +On such systems, in some cases, like for example if you need to run
>> +your program and then detach it, and you want the program to remain
>> +associated with a terminal, you may prefer that @value{GDBN} starts
>> +your program using the same device for standard input and output as
>> +@value{GDBN} is using.
> 
> IMHO, this is a misfeature.  If the terminal from which GDB was run
> remains on the system, it would be an unpleasant surprise for the user
> that the program gets hit by SIGHUP when you detach.  I think we
> should try to avoid this side effect, if that's feasible.
> 
>> +@node Interrupting
>> +@section Interrupting
> 
> Once again, I'd prefer to talk about "stopping" instead.  Or maybe
> even "getting control to GDB".
> 
>> +Depending on operating system and configuration, this results in
>> +interrupting the program with either a @code{SIGINT} signal:
>> +
>> +@smallexample
>> +Program received signal SIGINT, Interrupt.
>> +@end smallexample
>> +
>> +@noindent
>> +or plainly suspending the program:
>> +
>> +@smallexample
>> +Program stopped.
>> +@end smallexample
> 
> This seems to be inaccurate.  On MS-Windows, for example, I see
> something different:
> 
>   Thread 4 received signal SIGTRAP, Trace/breakpoint trap.
> 
> So maybe you want to say that the above is only true for Posix hosts.
> Or maybe just be more vague and don't try quoting the messages?
> 
>> +On systems where interrupting the program results in a plain
>> +suspension instead of the program receiving a @code{SIGINT} signal,
>> +you can still pass a @code{SIGINT} signal to the program after it
>> +stops, using either the @code{signal SIGINT} or @code{queue-signal
>> +SIGINT} commands.  @xref{Signaling,,Giving your Program a Signal}.
> 
> This begs a question I already asked elsewhere: I'd expect that
> continuing the program after it was stopped like that will deliver
> SIGINT to the program, without any special commands.  Isn't that so?
> Your text seems to imply that it isn't, which I find surprising --
> after all, the user pressed Ctrl-C, so "normally" the debuggee should
> be hit with SIGINT, as if we were not debugging it.

This was discussed in the other thread, but what you're saying really isn't
correct -- continuing the program after it was stopped doesn't normally
make the the debuggee receive the SIGINT.  Only if you explicitly pass it,
with special commands.

> 
>> +@value{GDBN} on MS-Windows supports @kbd{C-@key{BREAK}} as an
>> +alternative interrupt key sequence.  @xref{interrupt debuggee on
>> +MS-Windows}.
> 
> I'm not sure I understand the significance of this note: after all, a
> Windows program can install a Ctrl-BREAK handler exactly like it does
> with Ctrl-C.  Is this only about SetConsoleMode?

I was creating a new "Interrupting" node in the manual, so I thought it
was a good place to put cross references to other places in the manual
that talk about interruption.  That's all this is.  Just like the
reference to the remote debug section.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
  2021-06-15 12:34 ` [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Eli Zaretskii via Gdb-patches
@ 2021-06-16 11:27   ` Pedro Alves
  2021-06-16 12:45     ` Eli Zaretskii via Gdb-patches
  0 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-16 11:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 2021-06-15 1:34 p.m., Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Date: Mon, 14 Jun 2021 22:23:54 +0100
>>
>> Currently, on GNU/Linux, it is not possible to interrupt with Ctrl-C
>> programs that block or ignore SIGINT, with e.g., sigprocmask or
>> signal(SIGINT, SIG_IGN).  You type Ctrl-C, but nothing happens.
> 
> For the reasons I explained in my other message, can we please talk
> about "stopping a program" instead of "interrupting"?  Both in the log
> message (and the discussion) and in the documentation, please.  At
> least for me, the use of "interrupt" was confusing.
> 
>> This series address the problem by turning Ctrl-C handling around such
>> that the SIGINT always reaches GDB first, not the inferior.  That is
>> done by making GDB put inferiors in their own terminal/session created
>> by GDB.  I.e., GDB creates a pseudo-terminal master/slave pair, makes
>> the inferior run with the slave as its terminal, and pumps
>> output/input on the master end.
> 
> Is this "pumping" really-truly 100% transparent to the program being
> debugged?  I understand how a program that uses stdio will simply read
> the same stuff that was "pumped" from GDB's terminal, but what about
> other kinds of input and settings?
> 
> For example, some text-mode programs change the settings of the
> terminal via termios -- will those settings be reflected in the GDB's
> terminal when the debuggee reads or writes from its terminal?  Or what
> about support for terminfo functions -- will the commands be passed
> back and forth between the terminals and the user will see the effects
> he/she expects?  Some terminal emulators support advanced features
> like cut/paste -- will that also work with this pumping?  And if the
> program changes the character that causes SIGINT (Emacs does that when
> it runs on a text-mode terminal), will that change the interrupt
> character for GDB as well?

Debugging curses programs works fine, because we "pump" all input/output between
the terminals, escape sequences and all.  E.g., I use the TUI frequently, and
debugging gdb with itself and enabling the TUI in the inferior gdb works fine.  
No other pumping is done, only input/output.

When the new terminal is created, it inherits the terminal settings of the
GDB terminal, but otherwise, the inferior's terminal settings are not
reflected in GDB's terminal settings.  I mean, if the program changes \n -> \r\n
translation in the inferior, that changes the inferior's terminal, and what ends
up output to the pseudo-terminal slave end.  When GDB flushes that into its
own terminal, GDB puts its own terminal in raw mode, so that exactly whatever
characters/bytes the inferior wanted to send to the screen, they end up in
GDB's terminal.  Similarly for the input direction.

Cut/paste works fine for me, but I'm not sure I'm trying what you are
suggesting.  E.g., I debug a GDB that is running in TUI mode, and then I can use
the mouse to select text (shift click drag) and then paste it back (shift middle click).
This is all implemented in the terminal emulator, the application has no idea about it,
it's just text grabbed from the screen, and then input back into the application,
as if you typed it.

Changing the interrupt character does not work.  If I debug emacs in text-mode, and
then press ctrl-c, it drops you back into GDB, like:

 Thread 1 "emacs" stopped.
 0x00007ffff5197246 in __pselect (nfds=9, readfds=0x7fffffffc8a0, writefds=0x7fffffffc920, exceptfds=0x0, timeout=<optimized out>, sigmask=<optimized out>) at ../sysdeps/unix/sysv/linux/pselect.c:48                                                           |
 48      ../sysdeps/unix/sysv/linux/pselect.c: No such file or directory.
 (gdb)

If I press "ctrl-g", I then get "Program stopped by SIGINT":

 Thread 1 "emacs" received signal SIGINT, Interrupt.
 0x00007ffff5197246 in __pselect (nfds=9, readfds=0x7fffffffc8a0, writefds=0x7fffffffc920, exceptfds=0x0, timeout=<optimized out>, sigmask=<optimized out>) at ../sysdeps/unix/sysv/linux/pselect.c:48                                                           |
 48      in ../sysdeps/unix/sysv/linux/pselect.c
 (gdb)

I see this as a feature and it goes back to the "supervisor" idea discussed
in the other thread.  To change the "supervisor" interrupt key, you have to
change the interrupt key in the supervisor's terminal, with e.g., "$ stty intr ^G".
or even "(gdb) shell stty intr ^G".  Not unlike if you had attached to emacs
instead of spawned it.  I guess the emacs's emacs-gdb.gdb file could run that
stty command, so it is run automatically when debugging emacs.  The altered intr key will
remain in effect after gdb exits, though.  (I can reproduce it with current GDB.)
I guess GDB could grow a built-in command for that to avoid it.  

But, shouldn't GDB restore terminal settings on exit?  That's easy to do, GDB
already saves the initial terminal settings when it starts up, to pass them down
to inferiors when their started.  See patch at the bottom.  That does restore
the intr key.

> 
>> The series will then make GDB interrupt the program with SIGSTOP
>> instead of SIGINT, which always works even if the inferior
>> blocks/ignores SIGINT -- SIGSTOP can't be ignored.
> 
> And if the user then types "continue", will SIGINT be delivered to the
> program, or will the Ctrl-C keystroke be "swallowed" by GDB?

Already answered elsewhere, but yes, it will be swallowed, just like today
by default.

> 
>> Having the inferior in its own terminal also means that GDB is in
>> control of when inferior output is flushed to the screen.  When
>> debugging with the CLI, this means that inferior output is now never
>> interpersed with GDB's output in an unreadable fashion.
> 
> I guess this will be done on the screen line level?  Because if a
> program uses terminfo or curses to write to different parts of the
> screen, the outputs of the program and GDB will mix anyhow, just not
> on the same screen line, right?
> 

Yeah, all I meant is that the inferior and GDB won't be concurrently writing
to the screen at the same time,  E.g., if you continue the program in the
background (continue&), and the program prints to the screen, if you do e.g. "help all",
the inferior output won't be flushed until all the help text is printed.
I should probably just drop that sentence, it's not that important.

Pedro Alves

From c98f0866f70b473399193f45186cd7fbdb43e3aa Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 16 Jun 2021 12:12:52 +0100
Subject: [PATCH] restore terminal settings on exit

Change-Id: I7afb648d6a17ee7588b1963518a5a79b20ebb8c9
---
 gdb/inflow.c   | 11 ++++++++++-
 gdb/terminal.h |  6 +++++-
 gdb/top.c      |  4 +++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/gdb/inflow.c b/gdb/inflow.c
index 35a51b26fb5..f100bd58dfc 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -207,7 +207,7 @@ static bool input_fd_redirected = false;
 /* See terminal.h.  */
 
 void
-set_initial_gdb_ttystate (void)
+save_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.  */
@@ -227,6 +227,15 @@ set_initial_gdb_ttystate (void)
     }
 }
 
+/* See terminal.h.  */
+
+void
+restore_initial_gdb_ttystate ()
+{
+  if (initial_gdb_ttystate != NULL)
+    serial_set_tty_state (stdin_serial, initial_gdb_ttystate);
+}
+
 /* Does GDB have a terminal (on stdin)?  */
 
 static int
diff --git a/gdb/terminal.h b/gdb/terminal.h
index f8ef29bc2a9..8661a8fc519 100644
--- a/gdb/terminal.h
+++ b/gdb/terminal.h
@@ -44,6 +44,10 @@ 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);
+extern void save_initial_gdb_ttystate (void);
+
+/* Restore the terminal settings back to what they were at GDB
+   startup.  */
+extern void restore_initial_gdb_ttystate ();
 
 #endif /* !defined (TERMINAL_H) */
diff --git a/gdb/top.c b/gdb/top.c
index 6e0f43d2fd9..ee6b45774a4 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1841,6 +1841,8 @@ quit_force (int *exit_arg, int from_tty)
       exception_print (gdb_stderr, ex);
     }
 
+  restore_initial_gdb_ttystate ();
+
   exit (exit_code);
 }
 
@@ -2401,7 +2403,7 @@ gdb_init ()
 
   /* Take a snapshot of our tty state before readline/ncurses have had a chance
      to alter it.  */
-  set_initial_gdb_ttystate ();
+  save_initial_gdb_ttystate ();
 
   async_init_signals ();
 

-- 
2.26.2


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-16 10:15     ` Pedro Alves
@ 2021-06-16 12:15       ` Eli Zaretskii via Gdb-patches
  2021-06-16 12:26         ` Pedro Alves
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii via Gdb-patches @ 2021-06-16 12:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@palves.net>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 16 Jun 2021 11:15:38 +0100
> 
> >> +On systems where interrupting the program results in a plain
> >> +suspension instead of the program receiving a @code{SIGINT} signal,
> >> +you can still pass a @code{SIGINT} signal to the program after it
> >> +stops, using either the @code{signal SIGINT} or @code{queue-signal
> >> +SIGINT} commands.  @xref{Signaling,,Giving your Program a Signal}.
> > 
> > This begs a question I already asked elsewhere: I'd expect that
> > continuing the program after it was stopped like that will deliver
> > SIGINT to the program, without any special commands.  Isn't that so?
> > Your text seems to imply that it isn't, which I find surprising --
> > after all, the user pressed Ctrl-C, so "normally" the debuggee should
> > be hit with SIGINT, as if we were not debugging it.
> 
> This was discussed in the other thread, but what you're saying really isn't
> correct -- continuing the program after it was stopped doesn't normally
> make the the debuggee receive the SIGINT.  Only if you explicitly pass it,
> with special commands.

If you are talking about the default value of "handle SIGINT", then it
is still possible (and IMO advisable) to mention that the signal will
be passed to the debuggee, except that the default setting of SIGINT
handling purposefully prevents that.

> >> +@value{GDBN} on MS-Windows supports @kbd{C-@key{BREAK}} as an
> >> +alternative interrupt key sequence.  @xref{interrupt debuggee on
> >> +MS-Windows}.
> > 
> > I'm not sure I understand the significance of this note: after all, a
> > Windows program can install a Ctrl-BREAK handler exactly like it does
> > with Ctrl-C.  Is this only about SetConsoleMode?
> 
> I was creating a new "Interrupting" node in the manual, so I thought it
> was a good place to put cross references to other places in the manual
> that talk about interruption.  That's all this is.  Just like the
> reference to the remote debug section.

My point was more general: why are we taking such a great care
publishing the Ctrl-BREAK support?  What's the big deal?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-16 12:15       ` Eli Zaretskii via Gdb-patches
@ 2021-06-16 12:26         ` Pedro Alves
  2021-06-16 13:05           ` Eli Zaretskii via Gdb-patches
  0 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-16 12:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 2021-06-16 1:15 p.m., Eli Zaretskii wrote:
>>>> +@value{GDBN} on MS-Windows supports @kbd{C-@key{BREAK}} as an
>>>> +alternative interrupt key sequence.  @xref{interrupt debuggee on
>>>> +MS-Windows}.
>>> I'm not sure I understand the significance of this note: after all, a
>>> Windows program can install a Ctrl-BREAK handler exactly like it does
>>> with Ctrl-C.  Is this only about SetConsoleMode?
>> I was creating a new "Interrupting" node in the manual, so I thought it
>> was a good place to put cross references to other places in the manual
>> that talk about interruption.  That's all this is.  Just like the
>> reference to the remote debug section.
> My point was more general: why are we taking such a great care
> publishing the Ctrl-BREAK support?  What's the big deal?
> 

The text that I was xrefing reads:

 @cindex Ctrl-BREAK, MS-Windows
 @cindex interrupt debuggee on MS-Windows
 @anchor{interrupt debuggee on MS-Windows}
 MS-Windows programs that call @code{SetConsoleMode} to switch off the
 special meaning of the @samp{Ctrl-C} keystroke cannot be interrupted
 by typing @kbd{C-c}.  For this reason, @value{GDBN} on MS-Windows
 supports @kbd{C-@key{BREAK}} as an alternative interrupt key
 sequence, which can be used to interrupt the debuggee even if it
 ignores @kbd{C-c}.

And the sentence I wrote was just my attempt to summarize this information
so that the xref made any sense.  How would you go about this?  

- Say something different in the xref?  What?

- Remove the xref completely?  

- Move the whole text to the new Interrupt section (removing it from its original place)?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-16  9:31     ` Pedro Alves
@ 2021-06-16 12:29       ` Eli Zaretskii via Gdb-patches
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii via Gdb-patches @ 2021-06-16 12:29 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@palves.net>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 16 Jun 2021 10:31:27 +0100
> 
> >> +On such systems, in some cases, like for example if you need to run
> >> +your program and then detach it, and you want the program to remain
> >> +associated with a terminal, you may prefer that @value{GDBN} starts
> >> +your program using the same device for standard input and output as
> >> +@value{GDBN} is using.
> > IMHO, this is a misfeature.  If the terminal from which GDB was run
> > remains on the system, it would be an unpleasant surprise for the user
> > that the program gets hit by SIGHUP when you detach.  I think we
> > should try to avoid this side effect, if that's feasible.
> > 
> 
> Hmm, I'm not sure I follow what you're saying here, maybe I'm misunderstanding.
> 
> We no longer get a SIGHUP on detach, that was fixed on v2.  What the text is saying now is
> different: because when you start your program with "run", the terminal the inferior is
> associated with was created by GDB, when you "detach", GDB destroys the terminal.  The
> result is that the program you detached from loses its terminal.

I understood that, and this is what I was calling "a misfeature"
above.

> From the detached
> program's perspective, it's the same as with current GDB when you do "run", then "detach",
> and then close the GDB terminal.

_Technically_ it is the same.  But from the user POV, it isn't, not
unless the user closes the terminal from which he/she invoked GDB.  As
long as that terminal stays, the user might be surprised to see the
program behaving as if the terminal was closed.

> If you really need to do "run" and then "detach" for some reason, _and_ you want
> your program to have a terminal, you should start the program with a specified
> terminal, like:
> 
>  - start the program outside gdb, and then use "attach".  Make sure the terminal
>    you started the program at remains alive.
> 
>  - use the "tty" command to tell GDB to start the program on some specified terminal 
>    (which will outlive gdb and gdb's terminal)
> 
> If you're sure gdb's terminal will stay around, and you want to tell GDB to start
> the program on that terminal, you can use the "tty /dev/tty" command before starting
> the program.

I understand.  Your description is clear.  I'm saying that it would be
good to avoid this side effect, if possible.  If not, users will have
to live with it.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
  2021-06-16 11:27   ` Pedro Alves
@ 2021-06-16 12:45     ` Eli Zaretskii via Gdb-patches
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii via Gdb-patches @ 2021-06-16 12:45 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@palves.net>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 16 Jun 2021 12:27:28 +0100
> 
> Debugging curses programs works fine, because we "pump" all input/output between
> the terminals, escape sequences and all.  E.g., I use the TUI frequently, and
> debugging gdb with itself and enabling the TUI in the inferior gdb works fine.  
> No other pumping is done, only input/output.
> 
> When the new terminal is created, it inherits the terminal settings of the
> GDB terminal, but otherwise, the inferior's terminal settings are not
> reflected in GDB's terminal settings.  I mean, if the program changes \n -> \r\n
> translation in the inferior, that changes the inferior's terminal, and what ends
> up output to the pseudo-terminal slave end.  When GDB flushes that into its
> own terminal, GDB puts its own terminal in raw mode, so that exactly whatever
> characters/bytes the inferior wanted to send to the screen, they end up in
> GDB's terminal.  Similarly for the input direction.

That's good to hear, but it sounds like we aren't entirely sure which
features won't work with this arrangement, and are still going by
trial and error?  I hope by the time we release GDB 12, the
restrictions and other issues caused by this change will be
well-known, because we will need to document them, so users could know
in advance when they need to go back to the original behavior.

Or maybe I can persuade you to turn this feature off by default, at
least for GDB 12?

> Cut/paste works fine for me, but I'm not sure I'm trying what you are
> suggesting.

I meant the so-called "bracketed paste mode", see

  https://cirw.in/blog/bracketed-paste

> E.g., I debug a GDB that is running in TUI mode, and then I can use
> the mouse to select text (shift click drag) and then paste it back (shift middle click).
> This is all implemented in the terminal emulator, the application has no idea about it,
> it's just text grabbed from the screen, and then input back into the application,
> as if you typed it.

Yes, but some of these features need to be turned on before they can
be used.  The application (think: Emacs) does that for its terminal,
but not for the GDB terminal.

> Changing the interrupt character does not work.  If I debug emacs in text-mode, and
> then press ctrl-c, it drops you back into GDB, like:
> 
>  Thread 1 "emacs" stopped.
>  0x00007ffff5197246 in __pselect (nfds=9, readfds=0x7fffffffc8a0, writefds=0x7fffffffc920, exceptfds=0x0, timeout=<optimized out>, sigmask=<optimized out>) at ../sysdeps/unix/sysv/linux/pselect.c:48                                                           |
>  48      ../sysdeps/unix/sysv/linux/pselect.c: No such file or directory.
>  (gdb)

This means that no Emacs key sequence that uses C-c will work under
GDB in this new mode, right?  In particular, "C-x C-c", which exits
Emacs?  That's why Emacs reprograms the terminal to use C-g instead.

> If I press "ctrl-g", I then get "Program stopped by SIGINT":
> 
>  Thread 1 "emacs" received signal SIGINT, Interrupt.
>  0x00007ffff5197246 in __pselect (nfds=9, readfds=0x7fffffffc8a0, writefds=0x7fffffffc920, exceptfds=0x0, timeout=<optimized out>, sigmask=<optimized out>) at ../sysdeps/unix/sysv/linux/pselect.c:48                                                           |
>  48      in ../sysdeps/unix/sysv/linux/pselect.c
>  (gdb)

Is this with the default setting in src/.gdbinit?  It says

  handle 2 noprint pass

And it does that for a reason: C-g is handled by the Emacs signal
handler.

> I see this as a feature and it goes back to the "supervisor" idea discussed
> in the other thread.  To change the "supervisor" interrupt key, you have to
> change the interrupt key in the supervisor's terminal, with e.g., "$ stty intr ^G".
> or even "(gdb) shell stty intr ^G".  Not unlike if you had attached to emacs
> instead of spawned it.  I guess the emacs's emacs-gdb.gdb file could run that
> stty command, so it is run automatically when debugging emacs.  The altered intr key will
> remain in effect after gdb exits, though.  (I can reproduce it with current GDB.)
> I guess GDB could grow a built-in command for that to avoid it.  

But that is only needed (or useful) if this new pseudo-tty mode is
being used, right?  How can this be expressed in a .gdbinit file, to
avoid sending unnecessary and potentially disruptive stty commands?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 16/16] Document pseudo-terminal and interrupting changes
  2021-06-16 12:26         ` Pedro Alves
@ 2021-06-16 13:05           ` Eli Zaretskii via Gdb-patches
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii via Gdb-patches @ 2021-06-16 13:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@palves.net>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 16 Jun 2021 13:26:33 +0100
> 
> The text that I was xrefing reads:
> 
>  @cindex Ctrl-BREAK, MS-Windows
>  @cindex interrupt debuggee on MS-Windows
>  @anchor{interrupt debuggee on MS-Windows}
>  MS-Windows programs that call @code{SetConsoleMode} to switch off the
>  special meaning of the @samp{Ctrl-C} keystroke cannot be interrupted
>  by typing @kbd{C-c}.  For this reason, @value{GDBN} on MS-Windows
>  supports @kbd{C-@key{BREAK}} as an alternative interrupt key
>  sequence, which can be used to interrupt the debuggee even if it
>  ignores @kbd{C-c}.
> 
> And the sentence I wrote was just my attempt to summarize this information
> so that the xref made any sense.  How would you go about this?  
> 
> - Say something different in the xref?  What?
> 
> - Remove the xref completely?  
> 
> - Move the whole text to the new Interrupt section (removing it from its original place)?

Your text and cross-reference are okay, no need to change anything.
Sorry for the noise.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 11/16] Move scoped_ignore_sigttou to gdbsupport/
  2021-06-14 21:24 ` [PATCH v2 11/16] Move scoped_ignore_sigttou to gdbsupport/ Pedro Alves
@ 2021-06-17 21:49   ` Pedro Alves
  0 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-06-17 21:49 UTC (permalink / raw)
  To: gdb-patches

FYI, this patch has already been merged, as part of a different patch set:

 https://sourceware.org/pipermail/gdb-patches/2021-June/179982.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (16 preceding siblings ...)
  2021-06-15 12:34 ` [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Eli Zaretskii via Gdb-patches
@ 2021-06-18 10:12 ` Andrew Burgess
  2021-06-24 18:12 ` Konstantin Kharlamov via Gdb-patches
  18 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-06-18 10:12 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro,

This change looks awesome!  I've wanted this for a long time now, so
glad it might finally arrive.

I have a question for a slightly related topic.  I don't think this
needs resolving prior to this series landing, but it might be worth
having a discussion about what the solution might be within this new
setup.

I was recently looking at signal handling in general, and started
looking at how SIGTSTP is handled, for example, as might be delivered
by Ctrl-Z from a terminal.

My interest in this started by looking at async_sigtstp_handler in
event-top.c, I noticed at the end of this function there is code to
reprint the prompt, which should be run after GDB is resumed, after a
stop.

However, right now, this is not the case.  When the terminal is
claimed by GDB then, when GDB is stopped and resumed, no prompt is
printed.

On digging into this, the problem is that the SIGTSTP handler is only
installed for the duration of command_line_input, which is the
non-async command line parser.

So, my initial plan was to removed the SIGTSTP handling from
command_line_input, and instead have the code which switches terminal
ownership install/remove the SIGTSTP handler...

...but then I realised, that if GDB doesn't own the terminal the
SIGTSTP should be sent directly to the inferior anyway, right?  So,
then my plan became, lets just install the SIGTSTP handler right from
the start, and this seems to work fine before your changes (there are
still some bugs to work out relating to printing the correct prompt in
all cases, and reprinting any partial read line input that might have
been in place, but these issues can be ignored for this discussion).

The problem then is what should happen after your work is applied?

The patch below applies on top of your work, and demonstrates the
problem, when GDB owns the terminal stop/resume works fine:

  (gdb)
  [1]+  Stopped                 ./gdb/gdb --data-directory ./gdb/data-directory/ ~/tmp/loop.x
  $ fg
  ./gdb/gdb --data-directory ./gdb/data-directory/ ~/tmp/loop.x
  (gdb)

But as the inferior never really takes over the terminal any more,
it's not clear to me what we should do?

One idea I've had is to just make SIGTSTP interrupt the inferior just
like SIGINT does.  This would drop the user back to a prompt, at which
point the user could send SIGTSTP again if they then wanted GDB itself
to stop.

Look forward to hearing your thoughts,

Thanks,
Andrew

---

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 9741b23576a..d609d73a016 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -962,6 +962,7 @@ async_init_signals (void)
 #ifdef SIGTSTP
   sigtstp_token =
     create_async_signal_handler (async_sigtstp_handler, NULL, "sigtstp");
+  signal (SIGTSTP, handle_sigtstp);
 #endif
 
   install_handle_sigsegv ();
@@ -1182,27 +1183,34 @@ handle_sigtstp (int sig)
 static void
 async_sigtstp_handler (gdb_client_data arg)
 {
-  char *prompt = get_prompt ();
+  if (!target_terminal::is_ours ())
+    {
+      fprintf (stderr, "TODO: Not really sure what we should do here.\n");
+    }
+  else
+    {
+      char *prompt = get_prompt ();
 
-  signal (SIGTSTP, SIG_DFL);
+      signal (SIGTSTP, SIG_DFL);
 #if HAVE_SIGPROCMASK
-  {
-    sigset_t zero;
+      {
+	sigset_t zero;
 
-    sigemptyset (&zero);
-    gdb_sigmask (SIG_SETMASK, &zero, 0);
-  }
+	sigemptyset (&zero);
+	gdb_sigmask (SIG_SETMASK, &zero, 0);
+      }
 #elif HAVE_SIGSETMASK
-  sigsetmask (0);
+      sigsetmask (0);
 #endif
-  raise (SIGTSTP);
-  signal (SIGTSTP, handle_sigtstp);
-  printf_unfiltered ("%s", prompt);
-  gdb_flush (gdb_stdout);
+      raise (SIGTSTP);
+      signal (SIGTSTP, handle_sigtstp);
+      fprintf_unfiltered (gdb_stdout, "%s%s", prompt, rl_line_buffer);
+      gdb_flush (gdb_stdout);
 
-  /* Forget about any previous command -- null line now will do
-     nothing.  */
-  dont_repeat ();
+      /* Forget about any previous command -- null line now will do
+	 nothing.  */
+      dont_repeat ();
+    }
 }
 #endif /* SIGTSTP */
 
diff --git a/gdb/top.c b/gdb/top.c
index 6e0f43d2fd9..876546c023f 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1325,11 +1325,6 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix)
   /* Starting a new command line.  */
   cmd_line_buffer.used_size = 0;
 
-#ifdef SIGTSTP
-  if (job_control)
-    signal (SIGTSTP, handle_sigtstp);
-#endif
-
   while (1)
     {
       gdb::unique_xmalloc_ptr<char> rl;
@@ -1385,11 +1380,6 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix)
       prompt = NULL;
     }
 
-#ifdef SIGTSTP
-  if (job_control)
-    signal (SIGTSTP, SIG_DFL);
-#endif
-
   return cmd;
 }
 \f

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
  2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
                   ` (17 preceding siblings ...)
  2021-06-18 10:12 ` Andrew Burgess
@ 2021-06-24 18:12 ` Konstantin Kharlamov via Gdb-patches
  2021-06-24 18:55   ` Pedro Alves
  18 siblings, 1 reply; 34+ messages in thread
From: Konstantin Kharlamov via Gdb-patches @ 2021-06-24 18:12 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches, Eli Zaretskii

Hello! I was referred to your patchset from #gdb IRC channel. Presumably,
someone asked for real world usecases, but I haven't found that question. So I'm
replying to the top message.

I was recently fixing some bug in Pipewire. To be exact, I was working on
`pipewire-media-session` (one of daemons that Pipewire is comprised of). Process
of debugging basically included setting breakpoints somewhere, then hopefully
triggering the breakpoint, then inspecting the state. Nothing unusual.

So, the thing that was slowing me down and annoying is that I couldn't just
pause the debuggee through the usual means of ^C, then add a breakpoint.
Pressing ^C was causing the process to exit!

Pipewire project seems to have a number of various daemons, and all of them set
SIGINT handlers (judging by git-grepping for SIGINT). So basically, it is hard
to debug Pipewire with GDB due to GDB not being able to interactively pause the
process.

Now, I tested the patchset, and I confirm it solves the problem! GDB built from
the palves' branch pauses pipewire-media-session correctly on ^C!

With that said, I'm not sure if you need/use that in GDB project, but in any
case, the series is:

	Tested-by: Konstantin Kharlamov <hi-angel@yandex.ru>

FYI, Tom Tromey mentioned¹ that the problem arises when debuggee uses either
sigwait and signalfd. With git-grep I haven't found matches for sigwait in
Pipewire, but I found ones for signalfd. I see your top message does not mention
that your patchset also fixes situation for signalfd usage — but it seems it
does. So, just for your interest, your patchset might fix even more problems
than you expected ☺

1: https://bugzilla.kernel.org/show_bug.cgi?id=9039#c8

On Mon, 2021-06-14 at 22:23 +0100, Pedro Alves wrote:
> New in v2:
>   - run + detach no longer results in the detached process dying with
>     SIGHUP.  GDB no longer queries about it on detach.
>   - the testsuite gets fewer "tty /dev/tty" as a result.
>   - documentation updated accordingly.
>   - documentation now explicitly mentions that you can use "signal
>     SIGINT" after Ctrl-C.
>   - Added missing "@noindent" per Eli's review.
>   - the gdb.base/annota1.exp patch has been meanwhile merged
> 
> For review convenience, I've put this in the users/palves/ctrl-c
> branch.
> 
> Currently, on GNU/Linux, it is not possible to interrupt with Ctrl-C
> programs that block or ignore SIGINT, with e.g., sigprocmask or
> signal(SIGINT, SIG_IGN).  You type Ctrl-C, but nothing happens.
> 
> Similarly, if a program uses sigwait to wait for SIGINT, and the
> program receives a SIGINT, the SIGINT is _not_ intercepted by ptrace,
> it goes straight to the inferior.  These problems have been known for
> years, and recorded in gdb/9425, gdb/14559.
> 
> This is a consequence of how GDB implements interrupting programs with
> Ctrl-C -- when GDB spawns a new process, it makes the process use the
> same terminal as GDB, and then makes the process's process group be
> the foreground process group in GDB's terminal.  This means that when
> the process is running in the foreground, after e.g. "continue", when
> the user types Ctrl-C, the kernel sends a SIGINT to the foreground
> process group, which is the inferior.  GDB then intercepts the SIGINT
> via ptrace, the same way it intercepts any other signal, stops all the
> other threads of the inferior if in all-stop, and presents the
> "Program received SIGINT" stop to the user.
> 
> This series address the problem by turning Ctrl-C handling around such
> that the SIGINT always reaches GDB first, not the inferior.  That is
> done by making GDB put inferiors in their own terminal/session created
> by GDB.  I.e., GDB creates a pseudo-terminal master/slave pair, makes
> the inferior run with the slave as its terminal, and pumps
> output/input on the master end.  Because the inferior is run with its
> own session/terminal, GDB is free to remain as the foreground process
> in its own terminal, which means that the Ctrl-C SIGINT always reaches
> GDB first, instead of reaching the inferior first, and then GDB
> reacting to the ptrace-intercepted SIGINT.  Because GDB gets the
> SIGINT first, GDB is then free to handle it by interrupting the
> program any way it sees fit.
> 
> The series will then make GDB interrupt the program with SIGSTOP
> instead of SIGINT, which always works even if the inferior
> blocks/ignores SIGINT -- SIGSTOP can't be ignored.  (In the future GDB
> may even switch to PTRACE_INTERRUPT, though that's a project of its
> own.)
> 
> Having the inferior in its own terminal also means that GDB is in
> control of when inferior output is flushed to the screen.  When
> debugging with the CLI, this means that inferior output is now never
> interpersed with GDB's output in an unreadable fashion.  This will
> also allow fixing the problem of inferior output really messing up the
> screen in the TUI, forcing users to Ctrl-L to refresh the screen.
> This series does not address the TUI part, but it shouldn't be too
> hard -- I wrote a quick&dirty prototype patch doing that a few years
> back, so I know it works.
> 
> Tested on GNU/Linux native, gdbserver and gdbserver + "maint target
> set-non-stop on".  Also build-tested on mingw32-w64, Solaris 11, and
> OpenBSD.
> 
> More details in each of the patches in the series.
> 
> The first patch adds tests that fail currently, but will pass at the
> end of the series.
> 
> The main GDB changes are in patches #12 and #15.
> 
> Patch #1-#5, #9-#11, #13-#14 could go in without the main changes.
> 
> All documentation (manual, NEWS) is in the last patch -- patch #16.
> 
> Pedro Alves (16):
>   Test interrupting programs that block SIGINT [gdb/9425, gdb/14559]
>   prefork_hook: Remove 'args' parameter
>   Make gdb.base/long-inferior-output.exp fail fast
>   Fix gdb.multi/multi-term-settings.exp race
>   Don't check parent pid in
>     gdb.threads/{ia64-sigill,siginfo-threads,watchthreads-reorder}.c
>   Special-case "set inferior-tty /dev/tty"
>   Make inferior/GDB share terminal in tests expecting output after
>     detach
>   Make inferior/GDB share terminal in tests that exercise GDB/inferior
>     reading same input
>   gdb.mi/mi-logging.exp, don't send input to GDB while the inferior is
>     running
>   target_terminal::ours_for_output before printing signal received
>   Move scoped_ignore_sigttou to gdbsupport/
>   Always put inferiors in their own terminal/session [gdb/9425,
>     gdb/14559]
>   exists_non_stop_target: Avoid flushing frames
>   convert previous_inferior_ptid to strong reference to thread_info
>   GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR
>     gdb/9425, PR gdb/14559]
>   Document pseudo-terminal and interrupting changes
> 
>  gdb/doc/gdb.texinfo                           | 117 ++-
>  gdb/NEWS                                      |  23 +
>  gdb/Makefile.in                               |   1 -
>  gdb/event-top.c                               |  10 +-
>  gdb/fork-child.c                              |  10 +-
>  gdb/inf-ptrace.c                              |  24 +-
>  gdb/inf-ptrace.h                              |   8 +
>  gdb/infcmd.c                                  |  39 +-
>  gdb/inferior.h                                |   4 +
>  gdb/inflow.c                                  | 719 +++++++++++++++---
>  gdb/infrun.c                                  | 179 ++++-
>  gdb/infrun.h                                  |  16 +
>  gdb/linux-nat.c                               | 115 ++-
>  gdb/linux-nat.h                               |   2 +
>  gdb/nat/fork-inferior.c                       | 120 ++-
>  gdb/nat/fork-inferior.h                       |  30 +-
>  gdb/procfs.c                                  |   1 -
>  gdb/ser-unix.c                                |   2 +-
>  gdb/target.c                                  |  40 +-
>  gdb/terminal.h                                |   3 +
>  .../gdb.base/annota-input-while-running.exp   |   4 +
>  gdb/testsuite/gdb.base/annota1.exp            |   3 +-
>  .../gdb.base/bp-cmds-continue-ctrl-c.exp      |   3 +
>  .../gdb.base/continue-all-already-running.exp |   3 +
>  gdb/testsuite/gdb.base/infcall-input.exp      |  12 +-
>  .../gdb.base/interrupt-daemon-attach.exp      |   2 +-
>  gdb/testsuite/gdb.base/interrupt-daemon.exp   |   4 +-
>  gdb/testsuite/gdb.base/interrupt-noterm.exp   |   2 +-
>  gdb/testsuite/gdb.base/interrupt.exp          |   4 +-
>  .../gdb.base/long-inferior-output.exp         |  31 +-
>  gdb/testsuite/gdb.base/random-signal.exp      |   3 +-
>  gdb/testsuite/gdb.base/range-stepping.exp     |   2 +-
>  gdb/testsuite/gdb.base/sigint-masked-out.c    |  43 ++
>  gdb/testsuite/gdb.base/sigint-masked-out.exp  | 109 +++
>  gdb/testsuite/gdb.base/sigint-sigwait.c       |  80 ++
>  gdb/testsuite/gdb.base/sigint-sigwait.exp     | 113 +++
>  gdb/testsuite/gdb.gdb/selftest.exp            |   4 +-
>  gdb/testsuite/gdb.mi/mi-logging.exp           |  94 ++-
>  gdb/testsuite/gdb.mi/new-ui-mi-sync.exp       |   2 +-
>  .../gdb.multi/multi-target-interrupt.exp      |   2 +-
>  .../gdb.multi/multi-target-no-resumed.exp     |   2 +-
>  .../gdb.multi/multi-term-settings.exp         |  92 ++-
>  gdb/testsuite/gdb.server/reconnect-ctrl-c.exp |   4 +-
>  gdb/testsuite/gdb.threads/async.exp           |   2 +-
>  .../gdb.threads/continue-pending-status.exp   |   2 +-
>  gdb/testsuite/gdb.threads/ia64-sigill.c       |   5 -
>  gdb/testsuite/gdb.threads/leader-exit.exp     |   2 +-
>  gdb/testsuite/gdb.threads/manythreads.exp     |   4 +-
>  .../process-dies-while-detaching.exp          |   3 +
>  gdb/testsuite/gdb.threads/pthreads.exp        |   2 +-
>  gdb/testsuite/gdb.threads/schedlock.exp       |  11 +-
>  gdb/testsuite/gdb.threads/siginfo-threads.c   |   5 -
>  gdb/testsuite/gdb.threads/sigthread.exp       |   2 +-
>  .../gdb.threads/watchthreads-reorder.c        |   5 -
>  gdb/testsuite/lib/gdb.exp                     |  19 +
>  gdb/tui/tui-win.c                             |   3 +
>  gdbserver/fork-child.cc                       |  15 +-
>  gdbsupport/Makefile.am                        |   1 +
>  gdbsupport/Makefile.in                        |  15 +-
>  gdbsupport/managed-tty.cc                     |  22 +
>  gdbsupport/managed-tty.h                      |  42 +
>  .../scoped_ignore_sigttou.h                   |   8 +-
>  62 files changed, 1994 insertions(+), 255 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/sigint-masked-out.c
>  create mode 100644 gdb/testsuite/gdb.base/sigint-masked-out.exp
>  create mode 100644 gdb/testsuite/gdb.base/sigint-sigwait.c
>  create mode 100644 gdb/testsuite/gdb.base/sigint-sigwait.exp
>  create mode 100644 gdbsupport/managed-tty.cc
>  create mode 100644 gdbsupport/managed-tty.h
>  rename gdb/inflow.h => gdbsupport/scoped_ignore_sigttou.h (90%)
> 
> 
> base-commit: c9923e71ff57ce6e824833560aae59057c6f5783



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
  2021-06-24 18:12 ` Konstantin Kharlamov via Gdb-patches
@ 2021-06-24 18:55   ` Pedro Alves
  2021-06-29  1:15     ` Eldar Abusalimov via Gdb-patches
  0 siblings, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-06-24 18:55 UTC (permalink / raw)
  To: Konstantin Kharlamov, gdb-patches, Eli Zaretskii

On 2021-06-24 7:12 p.m., Konstantin Kharlamov wrote:
> Hello! I was referred to your patchset from #gdb IRC channel. Presumably,
> someone asked for real world usecases, but I haven't found that question. So I'm
> replying to the top message.

Hi!

> 
> I was recently fixing some bug in Pipewire. To be exact, I was working on
> `pipewire-media-session` (one of daemons that Pipewire is comprised of). Process
> of debugging basically included setting breakpoints somewhere, then hopefully
> triggering the breakpoint, then inspecting the state. Nothing unusual.
> 
> So, the thing that was slowing me down and annoying is that I couldn't just
> pause the debuggee through the usual means of ^C, then add a breakpoint.
> Pressing ^C was causing the process to exit!
> 
> Pipewire project seems to have a number of various daemons, and all of them set
> SIGINT handlers (judging by git-grepping for SIGINT). So basically, it is hard
> to debug Pipewire with GDB due to GDB not being able to interactively pause the
> process.
> 
> Now, I tested the patchset, and I confirm it solves the problem! GDB built from
> the palves' branch pauses pipewire-media-session correctly on ^C!

Awesome!

> With that said, I'm not sure if you need/use that in GDB project, but in any
> case, the series is:
> 
> 	Tested-by: Konstantin Kharlamov <hi-angel@yandex.ru>

We don't typically use it, but it doesn't hurt either!

> FYI, Tom Tromey mentioned¹ that the problem arises when debuggee uses either
> sigwait and signalfd. With git-grep I haven't found matches for sigwait in
> Pipewire, but I found ones for signalfd. I see your top message does not mention
> that your patchset also fixes situation for signalfd usage — but it seems it
> does. So, just for your interest, your patchset might fix even more problems
> than you expected ☺

Ahah, yeah.  Thanks, I didn't remember signalfd.  :-)

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT
  2021-06-24 18:55   ` Pedro Alves
@ 2021-06-29  1:15     ` Eldar Abusalimov via Gdb-patches
  0 siblings, 0 replies; 34+ messages in thread
From: Eldar Abusalimov via Gdb-patches @ 2021-06-29  1:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Konstantin Kharlamov

Hi Pedro,

Let me chime in to say that the issue addressed is indeed relevant, in
case you need any reassurance. :-)  This being addressed is great news!

A related problem[1] was reported in the CLion IDE issue tracker, and
the discussion also mentions the Kernel PR linked by Konstantin.  Since
CLion signals the inferior process directly, we ended up changing the
signal sent by the IDE from SIGINT to SIGSTOP so that there's no way the
inferior program can intercept the signal.  Apparently, this is also how
LLDB handles that as well.

However, that didn't cover[2] the target-async case, that is, when the
signal is still being sent by GDB (using the "interrupt" command), and
remote targets, where it's the gdbserver who ultimately supervises the
inferior process, not GDB.

From the patchset, I gather that the target-async case has now been
fixed as well, but what about remote targets?  Do you think it makes
sense to also change gdbserver to use SIGSTOP instead of SIGINT?  As far
as I understand, unlike for a locally spawned process, this shouldn't
require waltzing with pseudo-terminals for the inferior on the remote
gdbserver side.  At the same time consistency w.r.t. signal handling on
suspending the target would definitely be a nice thing to have
eventually.

  [1]: https://youtrack.jetbrains.com/issue/CPP-14435#focus=Comments-27-3105589.0-0
  [2]: https://youtrack.jetbrains.com/issue/CPP-21975#focus=Comments-27-4635249.0-0


--
Regards,
Eldar

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 15/16] GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR gdb/9425, PR gdb/14559]
  2021-06-14 21:24 ` [PATCH v2 15/16] GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR gdb/9425, PR gdb/14559] Pedro Alves
@ 2021-07-08 23:05   ` Maciej W. Rozycki
  2021-07-13 15:26     ` Pedro Alves
  0 siblings, 1 reply; 34+ messages in thread
From: Maciej W. Rozycki @ 2021-07-08 23:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Mon, 14 Jun 2021, Pedro Alves wrote:

> This commit has a user-visible behavior change in all-stop mode --
> when you interrupt the program with Ctrl-C, instead of:
> 
>   Thread 1 "threads" received signal SIGINT, Interrupt.
> 
> You'll get:
> 
>   Thread 1 "threads" stopped.
> 
> Which is what you already got with the "interrupt" command in non-stop
> mode.
> 
> If you really want to pass a SIGINT to the program, you can then issue:
> 
>   (gdb) signal SIGINT

 Can you still do:

(gdb) handle SIGINT nostop noprint noignore

instead to make SIGINT go through to the debuggee right away without GDB
getting bothered in any way?  I've had a need to use such a setup numerous
times (normally you can still hit ^Z or ^\ to get GDB interrupted with
SIGTSTP or SIGQUIT respectively instead).

  Maciej

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 15/16] GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR gdb/9425, PR gdb/14559]
  2021-07-08 23:05   ` Maciej W. Rozycki
@ 2021-07-13 15:26     ` Pedro Alves
  0 siblings, 0 replies; 34+ messages in thread
From: Pedro Alves @ 2021-07-13 15:26 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches

Hi Maciej,

On 2021-07-09 12:05 a.m., Maciej W. Rozycki wrote:
> On Mon, 14 Jun 2021, Pedro Alves wrote:
> 
>> This commit has a user-visible behavior change in all-stop mode --
>> when you interrupt the program with Ctrl-C, instead of:
>>
>>   Thread 1 "threads" received signal SIGINT, Interrupt.
>>
>> You'll get:
>>
>>   Thread 1 "threads" stopped.
>>
>> Which is what you already got with the "interrupt" command in non-stop
>> mode.
>>
>> If you really want to pass a SIGINT to the program, you can then issue:
>>
>>   (gdb) signal SIGINT
> 
>  Can you still do:
> 
> (gdb) handle SIGINT nostop noprint noignore
> 
> instead to make SIGINT go through to the debuggee right away without GDB
> getting bothered in any way?  I've had a need to use such a setup numerous
> times (normally you can still hit ^Z or ^\ to get GDB interrupted with
> SIGTSTP or SIGQUIT respectively instead).
> 

Yes, in the sense that if a SIGINT it sent to the inferior, it will go
through to the debuggee right away.  However, if you press Ctrl-C in
GDB (which is probably what you're asking), then it will stop the
program regardless of "handle SIGINT".  I've been thinking that to handle
scenarios like these and Eli's emacs scenario, GDB could gain some
command to configure it's own interrupt key.  The inferior's
terminal would still inherit the terminal settings that GDB had when it
was started up, so the terminal's interrupt key as configured when GDB
started (usually ^c) would still raise a SIGINT in the inferior.    So in
your scenario you'd do "set interrupt-key ^g" or some such, like:

 (gdb) handle SIGINT nostop noprint noignore
 (gdb) set interrupt-key ^g

With the above, ctrl-c would result in a SIGINT in the inferior
and ctrl-g would always stop the program, no matter what the program
does with signal masks/dispositions.

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2021-07-13 15:26 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 21:23 [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Pedro Alves
2021-06-14 21:23 ` [PATCH v2 01/16] Test interrupting programs that block SIGINT [gdb/9425, gdb/14559] Pedro Alves
2021-06-14 21:23 ` [PATCH v2 02/16] prefork_hook: Remove 'args' parameter Pedro Alves
2021-06-14 21:23 ` [PATCH v2 03/16] Make gdb.base/long-inferior-output.exp fail fast Pedro Alves
2021-06-14 21:23 ` [PATCH v2 04/16] Fix gdb.multi/multi-term-settings.exp race Pedro Alves
2021-06-14 21:23 ` [PATCH v2 05/16] Don't check parent pid in gdb.threads/{ia64-sigill, siginfo-threads, watchthreads-reorder}.c Pedro Alves
2021-06-14 21:24 ` [PATCH v2 06/16] Special-case "set inferior-tty /dev/tty" Pedro Alves
2021-06-14 21:24 ` [PATCH v2 07/16] Make inferior/GDB share terminal in tests expecting output after detach Pedro Alves
2021-06-14 21:24 ` [PATCH v2 08/16] Make inferior/GDB share terminal in tests that exercise GDB/inferior reading same input Pedro Alves
2021-06-14 21:24 ` [PATCH v2 09/16] gdb.mi/mi-logging.exp, don't send input to GDB while the inferior is running Pedro Alves
2021-06-14 21:24 ` [PATCH v2 10/16] target_terminal::ours_for_output before printing signal received Pedro Alves
2021-06-14 21:24 ` [PATCH v2 11/16] Move scoped_ignore_sigttou to gdbsupport/ Pedro Alves
2021-06-17 21:49   ` Pedro Alves
2021-06-14 21:24 ` [PATCH v2 12/16] Always put inferiors in their own terminal/session [gdb/9425, gdb/14559] Pedro Alves
2021-06-14 21:24 ` [PATCH v2 13/16] exists_non_stop_target: Avoid flushing frames Pedro Alves
2021-06-14 21:24 ` [PATCH v2 14/16] convert previous_inferior_ptid to strong reference to thread_info Pedro Alves
2021-06-14 21:24 ` [PATCH v2 15/16] GNU/Linux: Interrupt/Ctrl-C with SIGSTOP instead of SIGINT [PR gdb/9425, PR gdb/14559] Pedro Alves
2021-07-08 23:05   ` Maciej W. Rozycki
2021-07-13 15:26     ` Pedro Alves
2021-06-14 21:24 ` [PATCH v2 16/16] Document pseudo-terminal and interrupting changes Pedro Alves
2021-06-15 12:56   ` Eli Zaretskii via Gdb-patches
2021-06-16  9:31     ` Pedro Alves
2021-06-16 12:29       ` Eli Zaretskii via Gdb-patches
2021-06-16 10:15     ` Pedro Alves
2021-06-16 12:15       ` Eli Zaretskii via Gdb-patches
2021-06-16 12:26         ` Pedro Alves
2021-06-16 13:05           ` Eli Zaretskii via Gdb-patches
2021-06-15 12:34 ` [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT Eli Zaretskii via Gdb-patches
2021-06-16 11:27   ` Pedro Alves
2021-06-16 12:45     ` Eli Zaretskii via Gdb-patches
2021-06-18 10:12 ` Andrew Burgess
2021-06-24 18:12 ` Konstantin Kharlamov via Gdb-patches
2021-06-24 18:55   ` Pedro Alves
2021-06-29  1:15     ` Eldar Abusalimov via Gdb-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox