Hi Joel, Brian,
Post by Joel BrobeckerPost by Brian VandenbergIf gdb attaches to a process that either has no controlling terminal,
or the controlling terminal differs from the one gdb is running under,
break/^C doesn't interrupt the debugged process.
This is a fix that was proposed for this problem quite awhile ago but
never implemented; it's been in the Adacore GDB branch for quite
awhile.
Without going into unnecessary details I cannot easily run the test
suite against this change right now. If this patch gets rejected
based on that, when I have time I'll see about getting IllumOS
installed in a VM and test it there, but the problem was originally
found in sparc Solaris.
----
note: this patch was tested against 8.1.1. It looks like it probably
still applies on the 8.2 branch, but I won't be able to test it until
8.2 is released.
-brian
ps, my assignment/release forms were completed/received 10/30/2017
PR gdb/8527
* procfs.c (proc_wait_for_stop): calls to set_sigint_trap and
clear_sigint_trap.
I'm not sure anyone took the time to answer this message; if not,
I apologize.
I'm sorry it took me so long, too, first to get well again and then to
get to this.
Post by Joel BrobeckerI can testify that, for as long as we got the patch in the AdaCore
sources, we never noticed any ill effect. We never got to validate
it against the official testsuite, unfortunately, because for some
reason, when I did so, our Solaris machines would badly crash. Did
This is weird: the only time I saw something like this was with a few
Solaris 12/11.4 Beta builds.
Post by Joel Brobeckeryou run the testsuite before and after the patch, by any chance?
Rainer (in Cc:) is our maintainer for Solaris, so he may have an opinion.
I've now regtested the patch on both amd64-pc-solaris2.11 and
sparcv9-sun-solaris2.11. Initially, gdb.base/attach.exp seemed to
regress on 32-bit Solaris/SPARC, but that turned out to be due to
flakyness of parts of the testsuite on Solaris.
Post by Joel BrobeckerPost by Brian Vandenbergdiff --git a/gdb/procfs.c b/gdb/procfs.c
index 7b7ff45..7cd870c 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -913,7 +913,12 @@ proc_wait_for_stop (procinfo *pi)
procfs_ctl_t cmd = PCWSTOP;
+ /* PR gdb/8527
+ * Was not correctly interrupting the inferior process
+ * when ^C was pressed in the debug terminal.
+ */
For multiline comments like the above, we do not repeat the '*'
at the beginning of each line.
/* PR gdb/8527: Was not correctly interrupting the inferior process
when ^C was pressed in the debug terminal. */
And if I may, reading this sentence, it's a bit hard to understand
what the comment is trying to explain. The following might be
/* PR gdb/8527: Call set_sigint_trap to make sure that a ctrl-c
pressed in the debugger terminal gets passed down to the
inferior, thus causing it to be interrupted. */
TBH, I'd just leave off the comments: this is just what
set_sigint_trap/clear_sigint_trap are supposed to do. No other use
comments on this, and the PR reference can easily be found in the commit
message and the ChangeLog.
Another nit: blank before (), here and below.
Post by Joel BrobeckerPost by Brian Vandenbergwin = (write (pi->ctl_fd, (char *) &cmd, sizeof (cmd)) == sizeof (cmd));
+
+ /* PR gdb/8527 */
+ clear_sigint_trap();
+
/* We been runnin' and we stopped -- need to update status. */
pi->status_valid = 0;
When I noticed that we don't have any test for this issue, I started to
develop one. This turned out to be a rough trip and my first foray into
the gdb testsuite, so please bear with me.
Looking for possible testcases to modify, I first came
gdb.base/interrupt-daemon.exp. However, there turned out to be two
issues: I'd needed the pid of the grandchild process to attach to, and
this wasn't emitted to gdb.log when printed.
Besides, when I checked the test as is, it already FAILs on Solaris.
This seems to happen because set follow-fork-mode child isn't
implemented, but fails silently and the list of targets supporting it is
is either incomplete or completely missing in the tests that use it.
Next, I started with a copy of gdb.base/random-signal.c, adding a call
to setpgrp to detach it from its controling terminal.
Initially, that test FAILed, too, because it expected a
Program received signal SIGINT.*
message while on Solaris I get
Thread N received signal SIGINT.*
I suppose that's because starting with Solaris 10, every process is
multithreaded. Once I allowed for that form, too, the test PASSed on
Solaris, both for the run and attach cases. I'll go through the
testsuite and allow for both cases there, too, probably causing several
tests to magically PASS now ;-)
As expected, with unmodified gdb, I get the expected
FAIL: gdb.base/signal-no-ctty.exp: attach: stop with control-c (timeout)
However, when I tested the testcase on Linux/x86_64, it FAILs:
attach 113292
Attaching to program: /vol/gcc/obj/gdb/gdb/dist/gdb/testsuite/outputs/gdb.base/signal-no-ctty/signal-no-ctty, process 113292
warning: process 113292 is a zombie - the process has already terminated
ptrace: Operation not permitted.
(gdb) FAIL: gdb.base/signal-no-ctty.exp: attach: attach
The weird thing is that both with the setpgrp call and when run like
$ ./signal-no-ctty < /dev/null >& /dev/null &
ps still shows a controlling terminal for the process. Don't yet know
what's going on here.
Current patch attached for reference.
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-08-07 Brian Vandenberg <***@gmail.com>
gdb:
PR gdb/8527
* procfs.c (proc_wait_for_stop): Wrap write of PCWSTOP in
set_sigint_trap, clear_sigint_trap.
gdb/testsuite:
PR gdb/8527
* gdb.base/signal-no-ctty.c, gdb.base/signal-no-ctty.exp: New
test.