Bug 14559 - The 'interrupt' command does not work if sigwait is in use
Summary: The 'interrupt' command does not work if sigwait is in use
Status: NEW
Alias: None
Product: gdb
Classification: Unclassified
Component: gdb (show other bugs)
Version: unknown
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on: 9425
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-07 19:09 UTC by Andy Lutomirski
Modified: 2021-10-21 07:03 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Lutomirski 2012-09-07 19:09:02 UTC
Take a program that uses sigwait (on Linux).  Invoke gdb.  Start the program with r&, so it's running asynchronously.  Then type 'interrupt'.  It doesn't work -- sigwait reports SIGINT.  IMO this is a separate gdb bug from the usual bugs about SIGINT and sigwait not getting along (e.g. bug 9425): the interrupt command should be directly stopping the program, not sending SIGINT.
Comment 1 Pedro Alves 2012-09-11 15:27:36 UTC
Could you do a little test for us, and try "interrupt" with non-stop enabled
(set non-stop on)?  That goes through a different path and stops the inferior
with SIGSTOP instead of SIGINT.  Thanks.
Comment 2 Andy Lutomirski 2012-09-11 21:46:16 UTC
That works.  interrupt -a also appears to work.
Comment 3 Tom Tromey 2013-11-26 20:41:16 UTC
Moved out of waiting state -- seems legit to me.

See also bug #15250.
Comment 4 dje 2014-07-30 17:10:57 UTC
(In reply to Andy Lutomirski from comment #2)
> That works.  interrupt -a also appears to work.

Clarification (for my own purposes): This is still a non-stop vs all-stop distinction.  "interrupt -a" is an error in all-stop mode (though there's a plan to treat it like a plain "interrupt" in all-stop).
Comment 5 Sourceware Commits 2018-01-30 15:38:18 UTC
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e671cd59d74cec9f53e110ce887128d1eeadb7f2

commit e671cd59d74cec9f53e110ce887128d1eeadb7f2
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Jan 30 14:23:51 2018 +0000

    Per-inferior target_terminal state, fix PR gdb/13211, more
    
    In my multi-target branch I ran into problems with GDB's terminal
    handling that exist in master as well, with multi-inferior debugging.
    
    This patch adds a testcase for said problems
    (gdb.multi/multi-term-settings.exp), fixes the problems, fixes PR
    gdb/13211 as well (and adds a testcase for that too,
    gdb.base/interrupt-daemon.exp).
    
    The basis of the problem I ran into is the following.  Consider a
    scenario where you have:
    
     - inferior 1 - started with "attach", process is running on some
       other terminal.
    
     - inferior 2 - started with "run", process is sharing gdb's terminal.
    
    In this scenario, when you stop/resume both inferiors, you want GDB to
    save/restore the terminal settings of inferior 2, the one that is
    sharing GDB's terminal.  I.e., you want inferior 2 to "own" the
    terminal (in target_terminal::is_ours/target_terminal::is_inferior
    sense).
    
    Unfortunately, that's not what you get currently.  Because GDB doesn't
    know whether an attached inferior is actually sharing GDB's terminal,
    it tries to save/restore its settings anyway, ignoring errors.  In
    this case, this is pointless, because inferior 1 is running on a
    different terminal, but GDB doesn't know better.
    
    And then, because it is only possible to have the terminal settings of
    a single inferior be in effect at a time, or make one inferior/pgrp be
    the terminal's foreground pgrp (aka, only one inferior can "own" the
    terminal, ignoring fork children here), if GDB happens to try to
    restore the terminal settings of inferior 1 first, then GDB never
    restores the terminal settings of inferior 2.
    
    This patch fixes that and a few things more along the way:
    
     - Moves enum target_terminal::terminal_state out of the
       target_terminal class (it's currently private) and makes it a
       scoped enum so that it can be easily used elsewhere.
    
     - Replaces the inflow.c:terminal_is_ours boolean with a
       target_terminal_state variable.  This allows distinguishing is_ours
       and is_ours_for_output states.  This allows finally making
       child_terminal_ours_1 do something with its "output_only"
       parameter.
    
     - Makes each inferior have its own copy of the
       is_ours/is_ours_for_output/is_inferior state.
    
     - Adds a way for GDB to tell whether the inferior is sharing GDB's
       terminal.  Works best on Linux and Solaris; the fallback works just
       as well as currently.
    
     - With that, we can remove the inf->attach_flag tests from
       child_terminal_inferior/child_terminal_ours.
    
     - Currently target_ops.to_ours is responsible for both saving the
       current inferior's terminal state, and restoring gdb's state.
       Because each inferior has its own terminal state (possibly handled
       by different targets in a multi-target world, even), we need to
       split the inferior-saving part from the gdb-restoring part.  The
       patch adds a new target_ops.to_save_inferior target method for
       that.
    
     - Adds a new target_terminal::save_inferior() function, so that
       sequences like:
    
         scoped_restore_terminal_state save_state;
         target_terminal::ours_for_output ();
    
       ... restore back inferiors that were
       target_terminal_state::is_inferior before back to is_inferior, and
       leaves inferiors that were is_ours alone.
    
     - Along the way, this adds a default implementation of
       target_pass_ctrlc to inflow.c (for inf-child.c), that handles
       passing the Ctrl-C to a process running on GDB's terminal or to
       some other process otherwise.
    
     - Similarly, adds a new target default implementation of
       target_interrupt, for the "interrupt" command.  The current
       implementation of this hook in inf-ptrace.c kills the whole process
       group, but that's incorrect/undesirable because we may not be
       attached to all processes in the process group.  And also, it's
       incorrect because inferior_process_group() doesn't really return
       the inferior's real process group id if the inferior is not a
       process group leader...  This is the cause of PR gdb/13211 [1],
       which this patch fixes.  While at it, that target method's "ptid"
       parameter is eliminated, because it's not really used.
    
     - A new test is included that exercises and fixes PR gdb/13211, and
       also fixes a GDB issue reported on stackoverflow that I ran into
       while working on this [2].  The problem is similar to PR gdb/13211,
       except that it also triggers with Ctrl-C.  When debugging a daemon
       (i.e., a process that disconnects from the controlling terminal and
       is not a process group leader, then Ctrl-C doesn't work, you just
       can't interrupt the inferior at all, resulting in a hung debug
       session.  The problem is that since the inferior is no longer
       associated with gdb's session / controlling terminal, then trying
       to put the inferior in the foreground fails.  And so Ctrl-C never
       reaches the inferior directly.  pass_signal is only used when the
       inferior is attached, but that is not the case here.  This is fixed
       by the new child_pass_ctrlc.  Without the fix, the new
       interrupt-daemon.exp testcase fails with timeout waiting for a
       SIGINT that never arrives.
    
    [1] PR gdb/13211 - Async / Process group and interrupt not working
    https://sourceware.org/bugzilla/show_bug.cgi?id=13211
    
    [2] GDB not reacting Ctrl-C when after fork() and setsid()
    https://stackoverflow.com/questions/46101292/gdb-not-reacting-ctrl-c-when-after-fork-and-setsid
    
    Note this patch does _not_ fix:
    
     - PR gdb/14559 - The 'interrupt' command does not work if sigwait is in use
       https://sourceware.org/bugzilla/show_bug.cgi?id=14559
    
     - PR gdb/9425 - When using "sigwait" GDB doesn't trap SIGINT. Ctrl+C terminates program when should break gdb.
       https://sourceware.org/bugzilla/show_bug.cgi?id=9425
    
    The only way to fix that that I know of (without changing the kernel)
    is to make GDB put inferiors in a separate session (create a
    pseudo-tty master/slave pair, make the inferior run with the slave as
    its terminal, and have gdb pump output/input on the master end).
    
    gdb/ChangeLog:
    2018-01-30  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/13211
    	* config.in, configure: Regenerate.
    	* configure.ac: Check for getpgid.
    	* go32-nat.c (go32_pass_ctrlc): New.
    	(go32_target): Install it.
    	* inf-child.c (inf_child_target): Install
    	child_terminal_save_inferior, child_pass_ctrlc and
    	child_interrupt.
    	* inf-ptrace.c (inf_ptrace_interrupt): Delete.
    	(inf_ptrace_target): No longer install it.
    	* infcmd.c (interrupt_target_1): Adjust.
    	* inferior.h (child_terminal_save_inferior, child_pass_ctrlc)
    	(child_interrupt): Declare.
    	(inferior::terminal_state): New.
    	* inflow.c (struct terminal_info): Update comments.
    	(inferior_process_group): Delete.
    	(terminal_is_ours): Delete.
    	(gdb_tty_state): New.
    	(child_terminal_init): Adjust.
    	(is_gdb_terminal, sharing_input_terminal_1)
    	(sharing_input_terminal): New functions.
    	(child_terminal_inferior): Adjust.  Use sharing_input_terminal.
    	Set the process's actual process group in the foreground if
    	possible.  Handle is_ours_for_output/is_ours distinction.  Don't
    	mark terminal as the inferior's if not sharing GDB's terminal.
    	Don't check attach_flag.
    	(child_terminal_ours_for_output, child_terminal_ours): Adjust to
    	pass down a target_terminal_state.
    	(child_terminal_save_inferior): New, factored out from ...
    	(child_terminal_ours_1): ... this.  Handle
    	target_terminal_state::is_ours_for_output.
    	(child_interrupt, child_pass_ctrlc): New.
    	(inflow_inferior_exit): Clear the inferior's terminal_state.
    	(copy_terminal_info): Copy the inferior's terminal state.
    	(_initialize_inflow): Remove reference to terminal_is_ours.
    	* inflow.h (inferior_process_group): Delete.
    	* nto-procfs.c (nto_handle_sigint, procfs_interrupt): Adjust.
    	* procfs.c (procfs_target): Don't install procfs_interrupt.
    	(procfs_interrupt): Delete.
    	* remote.c (remote_serial_quit_handler): Adjust.
    	(remote_interrupt): Remove ptid parameter.  Adjust.
    	* target-delegates.c: Regenerate.
    	* target.c: Include "terminal.h".
    	(target_terminal::terminal_state): Rename to ...
    	(target_terminal::m_terminal_state): ... this.
    	(target_terminal::init): Adjust.
    	(target_terminal::inferior): Adjust to per-inferior
    	terminal_state.
    	(target_terminal::restore_inferior, target_terminal_is_ours_kind): New.
    	(target_terminal::ours, target_terminal::ours_for_output): Use
    	target_terminal_is_ours_kind.
    	(target_interrupt): Remove ptid parameter.  Adjust.
    	(default_target_pass_ctrlc): Adjust.
    	* target.h (target_ops::to_terminal_save_inferior): New field.
    	(target_ops::to_interrupt): Remove ptid_t parameter.
    	(target_interrupt): Remove ptid_t parameter.  Update comment.
    	(target_pass_ctrlc): Update comment.
    	* target/target.h (target_terminal_state): New scoped enum,
    	factored out of ...
    	(target_terminal::terminal_state): ... here.
    	(target_terminal::inferior): Update comments.
    	(target_terminal::restore_inferior): New.
    	(target_terminal::is_inferior, target_terminal::is_ours)
    	(target_terminal::is_ours_for_output): Adjust.
    	(target_terminal::scoped_restore_terminal_state): Adjust to
    	rename, and call restore_inferior() instead of inferior().
    	(target_terminal::scoped_restore_terminal_state::m_state): Change
    	type.
    	(target_terminal::terminal_state): Rename to ...
    	(target_terminal::m_terminal_state): ... this and change type.
    
    gdb/gdbserver/ChangeLog:
    2018-01-30  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/13211
    	* target.c (target_terminal::terminal_state): Rename to ...
    	(target_terminal::m_terminal_state): ... this.
    
    gdb/testsuite/ChangeLog:
    2018-01-30  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/13211
    	* gdb.base/interrupt-daemon.c: New.
    	* gdb.base/interrupt-daemon.exp: New.
    	* gdb.multi/multi-term-settings.c: New.
    	* gdb.multi/multi-term-settings.exp: New.
Comment 6 Thi Milton 2021-10-21 07:03:26 UTC Comment hidden (spam)