Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT

[Konstantin Kharlamov via Gdb-patches <gdb-patches@sourceware.org>]

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