[PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Brian Vandenberg]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

In Solaris:

If 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

[-- Attachment #2: gdb-8527-patch, revised.txt --]
[-- Type: text/plain, Size: 784 bytes --]

gdb/Changelog:
2018-08-07  Brian Vandenberg  <phantall@gmail.com>

	PR gdb/8527
	* procfs.c (proc_wait_for_stop): calls to set_sigint_trap and
	clear_sigint_trap.

diff --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.
+     */
+    set_sigint_trap();
+
     win = (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;

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Joel Brobecker]

Hi Brian,

> In Solaris:
> 
> If 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

> gdb/Changelog:
> 2018-08-07  Brian Vandenberg  <phantall@gmail.com>
> 
> 	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 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
you 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.

In the meantime, I have a trivial coding style comment:

> 
> diff --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
a little more precise:

       /* 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.  */

> +    set_sigint_trap();
> +
>      win = (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;


-- 
Joel

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Brian Vandenberg]

Greetings,

Did you run the testsuite before and after the patch, by any chance?


Nope.  In my work environment I don't have much flexibility on
getting/installing software.  If I run the test suite I would probably have
to setup an IllumOS VM at home to run it, but that'd be x86 not SPARC.


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
> a little more precise:
>        /* 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.  */


I've no qualms with those changes.  Thanks for your feedback.

-brian


On Thu, Nov 1, 2018 at 3:19 PM Joel Brobecker <brobecker@adacore.com> wrote:

> Hi Brian,
>
> > In Solaris:
> >
> > If 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
>
> > gdb/Changelog:
> > 2018-08-07  Brian Vandenberg  <phantall@gmail.com>
> >
> >       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 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
> you 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.
>
> In the meantime, I have a trivial coding style comment:
>
> >
> > diff --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
> a little more precise:
>
>        /* 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.  */
>
> > +    set_sigint_trap();
> > +
> >      win = (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;
>
>
> --
> Joel
>

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Simon Marchi]

On 2018-11-01 5:45 p.m., Brian Vandenberg wrote:
> Greetings,
> 
> Did you run the testsuite before and after the patch, by any chance?
> 
> 
> Nope.  In my work environment I don't have much flexibility on
> getting/installing software.  If I run the test suite I would probably have
> to setup an IllumOS VM at home to run it, but that'd be x86 not SPARC.
> 
> 
> 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
>> a little more precise:
>>        /* 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.  */
> 
> 
> I've no qualms with those changes.  Thanks for your feedback.

Asking because it's ambiguous... do you plan on sending an updated patch?

As for the patch content and its testing, perhaps Rainer can give some feedback.

Simon

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Rainer Orth]

Hi Simon,

> On 2018-11-01 5:45 p.m., Brian Vandenberg wrote:
>> Greetings,
>> 
>> Did you run the testsuite before and after the patch, by any chance?
>> 
>> 
>> Nope.  In my work environment I don't have much flexibility on
>> getting/installing software.  If I run the test suite I would probably have
>> to setup an IllumOS VM at home to run it, but that'd be x86 not SPARC.
>> 
>> 
>> 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
>>> a little more precise:
>>>        /* 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.  */
>> 
>> 
>> I've no qualms with those changes.  Thanks for your feedback.
>
> Asking because it's ambiguous... do you plan on sending an updated patch?

I don't think this is necessary: I'm going to take care of that.

> As for the patch content and its testing, perhaps Rainer can give some feedback.

Sorry for the delay in replying: I've both been very busy with
end-of-stage1 gcc stuff and unwell lately.  I hope to get to this soon.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 6146 bytes --]

Hi Joel, Brian,

>> In Solaris:
>> 
>> If 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
>
>> gdb/Changelog:
>> 2018-08-07  Brian Vandenberg  <phantall@gmail.com>
>> 
>> 	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.

> I 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.

> you 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.

> In the meantime, I have a trivial coding style comment:
>
>> 
>> diff --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
> a little more precise:
>
>        /* 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.

>> +    set_sigint_trap();

Another nit: blank before (), here and below.

>>      win = (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  <phantall@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-attach-sigint.patch --]
[-- Type: text/x-patch, Size: 4132 bytes --]

# HG changeset patch
# Parent  ba9899dff4a02b0de8cd73acc977a524b3921581
Interrupt not functional in Eclipse/CDT on Solaris (PR gdb/8527)

diff --git a/gdb/procfs.c b/gdb/procfs.c
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -909,7 +909,12 @@ proc_wait_for_stop (procinfo *pi)
 
   procfs_ctl_t cmd = PCWSTOP;
 
+  set_sigint_trap ();
+
   win = (write (pi->ctl_fd, (char *) &cmd, sizeof (cmd)) == sizeof (cmd));
+
+  clear_sigint_trap ();
+
   /* We been runnin' and we stopped -- need to update status.  */
   pi->status_valid = 0;
 
diff --git a/gdb/testsuite/gdb.base/signal-no-ctty.c b/gdb/testsuite/gdb.base/signal-no-ctty.c
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.base/signal-no-ctty.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 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>
+
+int main()
+{
+  /* Detach from controlling terminal.  */
+  if (setpgrp () == (pid_t) -1)
+    return 1;
+
+  /* Don't let the test case run forever.  */
+  alarm (60);
+
+  for (;;)
+    ;
+}
diff --git a/gdb/testsuite/gdb.base/signal-no-ctty.exp b/gdb/testsuite/gdb.base/signal-no-ctty.exp
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.base/signal-no-ctty.exp
@@ -0,0 +1,71 @@
+# Copyright 2013-2018 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/>.
+
+if [target_info exists gdb,nosignals] {
+    verbose "Skipping signal-no-ctty.exp because of nosignals."
+    continue
+}
+
+# This test requires sending ^C to interrupt the running target.
+if [target_info exists gdb,nointerrupts] {
+    verbose "Skipping signal-no-ctty.exp because of nointerrupts."
+    return
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Wait a bit and stop the target with ctrl-c.
+proc do_test {} {
+    global binfile
+
+    # 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|Thread .*) received signal SIGINT.*" "stop with control-c"
+}
+
+# With native debugging and "run" (with job control), the ctrl-c always
+# reaches the inferior, not gdb.  With remote debugging, the ctrl-c reaches
+# GDB first.
+with_test_prefix "run" {
+    clean_restart $binfile
+
+    gdb_run_cmd
+
+    do_test
+}
+
+# With "attach" however, even with native debugging, the ctrl-c always
+# reaches GDB first.  Test that as well.
+with_test_prefix "attach" {
+    if {[can_spawn_for_attach]} {
+	clean_restart $binfile
+
+	set test_spawn_id [spawn_wait_for_attach $binfile]
+	set testpid [spawn_id_get_pid $test_spawn_id]
+
+	gdb_test "attach $testpid" "Attaching to.*process $testpid.*libc.*" "attach"
+
+	send_gdb "continue\n"
+
+	do_test
+
+	kill_wait_spawned_process $test_spawn_id
+    }
+}

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 6701 bytes --]

Hi Joel,

>>> In Solaris:
>>> 
>>> If 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
>>
>>> gdb/Changelog:
>>> 2018-08-07  Brian Vandenberg  <phantall@gmail.com>
>>> 
>>> 	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.
>
>> I 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.
>
>> you 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.
>
>> In the meantime, I have a trivial coding style comment:
>>
>>> 
>>> diff --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
>> a little more precise:
>>
>>        /* 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.
>
>>> +    set_sigint_trap();
>
> Another nit: blank before (), here and below.
>
>>>      win = (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.

I never got a reply to this one, but I think I just figured out the
testcase part myself.  Tested on amd64-pc-solaris2.11 and
sparcv9-sun-solaris2.11 (test fails before with

FAIL: gdb.base/signal-no-ctty.exp: child: stop with control-c (timeout)

and passes after) and x86_64-pc-linux-gnu.

Ok for master?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2018-08-07  Brian Vandenberg  <phantall@gmail.com>
	    Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	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/sigint-no-ctty.c, gdb.base/sigint-no-ctty.exp: New
	test.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-attach-sigint.patch --]
[-- Type: text/x-diff, Size: 4981 bytes --]

# HG changeset patch
# Parent  14950de898700dec7316fb3a5691413d651ddf42
Can't interrupt process without controlling terminal on Solaris (PR gdb/8527)

2018-08-07  Brian Vandenberg  <phantall@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/sigint-no-ctty.c, gdb.base/sigint-no-ctty.exp: New
	test.

diff --git a/gdb/procfs.c b/gdb/procfs.c
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -909,7 +909,12 @@ proc_wait_for_stop (procinfo *pi)
 
   procfs_ctl_t cmd = PCWSTOP;
 
+  set_sigint_trap ();
+
   win = (write (pi->ctl_fd, (char *) &cmd, sizeof (cmd)) == sizeof (cmd));
+
+  clear_sigint_trap ();
+
   /* We been runnin' and we stopped -- need to update status.  */
   pi->status_valid = 0;
 
diff --git a/gdb/testsuite/gdb.base/sigint-no-ctty.c b/gdb/testsuite/gdb.base/sigint-no-ctty.c
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sigint-no-ctty.c
@@ -0,0 +1,61 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017-2019 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 <unistd.h>
+
+/* GDB reads this to figure out the child's PID.  */
+pid_t child_pid;
+
+void
+marker (void)
+{
+}
+
+int
+main ()
+{
+  /* Don't let the test case run forever.  */
+  alarm (60);
+
+  child_pid = fork ();
+
+  switch (child_pid)
+    {
+    case -1:
+      return 1;
+
+    case 0:
+      break;
+
+    default:
+      while (1)
+	{
+	  marker ();
+	  usleep (1000);
+	}
+    }
+
+  /* Detach from controlling terminal.  */
+  if (setsid () == (pid_t) -1)
+    return 1;
+
+  for (;;)
+    ;
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/sigint-no-ctty.exp b/gdb/testsuite/gdb.base/sigint-no-ctty.exp
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sigint-no-ctty.exp
@@ -0,0 +1,87 @@
+# Copyright 2019 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/>.  */
+
+if [target_info exists gdb,nosignals] {
+    verbose "Skipping signal-no-ctty.exp because of nosignals."
+    continue
+}
+
+# This test requires sending ^C to interrupt the running target.
+if [target_info exists gdb,nointerrupts] {
+    verbose "Skipping signal-no-ctty.exp because of nointerrupts."
+    return
+}
+
+standard_testfile
+
+if { [build_executable ${testfile}.exp ${testfile} $srcfile {debug}] == -1 } {
+    return -1
+}
+
+proc do_test {} {
+    global binfile
+    global decimal
+
+    set test_spawn_id [spawn_wait_for_attach $binfile]
+    set parent_pid [spawn_id_get_pid $test_spawn_id]
+
+    # Attach to the parent, run it to a known point, extract the
+    # child's PID, and detach.
+    with_test_prefix "parent" {
+	clean_restart ${binfile}
+
+	gdb_test "attach $parent_pid" \
+	    "Attaching to program.*, process $parent_pid.*" \
+	    "attach"
+
+	gdb_breakpoint "marker"
+	gdb_continue_to_breakpoint "marker"
+
+	set child_pid [get_integer_valueof "child_pid" -1]
+	if {$child_pid == -1} {
+	    return
+	}
+
+	gdb_test "detach" \
+	    "Detaching from program: .*process $parent_pid\r\n\\\[Inferior $decimal \\(.*\\) detached\\\]"
+
+	remote_exec host "kill -9 $parent_pid"
+    }
+
+    # Start over, and attach to the child this time.
+    with_test_prefix "child" {
+	global gdb_prompt
+
+	clean_restart $binfile
+
+	gdb_test "attach $child_pid" \
+	    "Attaching to program.*, process $child_pid.*" \
+	    "attach"
+
+	gdb_test_multiple "continue" "continue" {
+	    -re "Continuing" {
+		pass "continue"
+	    }
+	}
+
+	after 500 {send_gdb "\003"}
+	gdb_test "" "(Program|Thread .*) received signal SIGINT.*" \
+	    "stop with control-c"
+
+	remote_exec host "kill -9 $child_pid"
+    }
+}
+
+do_test

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Pedro Alves]

Hi Rainer,

On 02/26/2019 03:14 PM, Rainer Orth wrote:

>> 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.

It's a shame that the Solaris port doesn't support follow-fork.  I don't
suppose there's anything fundamentally impossible.  I'm sure it must
be possible to intercept fork/vfork/exec events with procfs.

>> 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.
> I never got a reply to this one, but I think I just figured out the
> testcase part myself. 

I'm curious -- what was the issue on Linux?

> +++ b/gdb/testsuite/gdb.base/sigint-no-ctty.exp
> @@ -0,0 +1,87 @@
> +# Copyright 2019 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/>.  */
> +

Please add a small intro comment mentioning what the testcase is about.

AFAICT, this is basically testing the same thing that
gdb.base/interrupt-daemon.exp is testing, with the difference that it
exercises inferiors started with "attach" instead of "run".  I'd suggest
renaming the testcase to interrupt-daemon-attach.exp, so that it sits
alongside interrupt-daemon.exp.

> +if [target_info exists gdb,nosignals] {
> +    verbose "Skipping signal-no-ctty.exp because of nosignals."
> +    continue
> +}
> +
> +# This test requires sending ^C to interrupt the running target.
> +if [target_info exists gdb,nointerrupts] {
> +    verbose "Skipping signal-no-ctty.exp because of nointerrupts."
> +    return
> +}
> +
> +standard_testfile
> +
> +if { [build_executable ${testfile}.exp ${testfile} $srcfile {debug}] == -1 } {
> +    return -1
> +}
> +
> +proc do_test {} {
> +    global binfile
> +    global decimal
> +
> +    set test_spawn_id [spawn_wait_for_attach $binfile]


This is missing a can_wait_for_attach check:

$ make check TESTS="gdb.base/sigint-no-ctty.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
...
ERROR: tcl error sourcing src/gdb/testsuite/gdb.base/sigint-no-ctty.exp.
ERROR: can't spawn for attach with this target/board
    while executing
"error "can't spawn for attach with this target/board""
    invoked from within
"if ![can_spawn_for_attach] {
        # The caller should have checked can_spawn_for_attach itself
        # before getting here.
        error "can't spawn for attach with..."
    (procedure "spawn_wait_for_attach" line 4)
    invoked from within

Otherwise, this is fine with me.

Thanks,
Pedro Alves

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 4429 bytes --]

Hi Pedro,

> On 02/26/2019 03:14 PM, Rainer Orth wrote:
>
>>> 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.
>
> It's a shame that the Solaris port doesn't support follow-fork.  I don't
> suppose there's anything fundamentally impossible.  I'm sure it must
> be possible to intercept fork/vfork/exec events with procfs.

certainly: that's just one of many warts of the port.  However, before
looking into adding missing features, I need to spend some time
investigating the large number of tests that fail (often timeouts) that
make the testsuite impossible to run usefully in the buildbots, taking
at least half an hour to complete and being flaky as hell in some areas.

>>> 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.
>> I never got a reply to this one, but I think I just figured out the
>> testcase part myself. 
>
> I'm curious -- what was the issue on Linux?

The initial testcase was just misguided: I found no reliable way to
really detach from the controlling tty without fork (which I'd have
liked to avoid in order not to have to jump through hoops to determine
the child pid).  I haven't looked closer after several false attempts to
make this work, but just started afresh from attach-non-pgrp-leader.exp
instead and modified that.

>> +++ b/gdb/testsuite/gdb.base/sigint-no-ctty.exp
[...]
> Please add a small intro comment mentioning what the testcase is about.

Done now.

> AFAICT, this is basically testing the same thing that
> gdb.base/interrupt-daemon.exp is testing, with the difference that it
> exercises inferiors started with "attach" instead of "run".  I'd suggest

More or less so, yes.  Just without the double fork and the bg variant
that isn't supported on Solaris.

> renaming the testcase to interrupt-daemon-attach.exp, so that it sits
> alongside interrupt-daemon.exp.

Fine with me.

>> +proc do_test {} {
>> +    global binfile
>> +    global decimal
>> +
>> +    set test_spawn_id [spawn_wait_for_attach $binfile]
>
>
> This is missing a can_wait_for_attach check:
>
> $ make check TESTS="gdb.base/sigint-no-ctty.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
> ...
> ERROR: tcl error sourcing src/gdb/testsuite/gdb.base/sigint-no-ctty.exp.
> ERROR: can't spawn for attach with this target/board
>     while executing
> "error "can't spawn for attach with this target/board""
>     invoked from within
> "if ![can_spawn_for_attach] {
>         # The caller should have checked can_spawn_for_attach itself
>         # before getting here.
>         error "can't spawn for attach with..."
>     (procedure "spawn_wait_for_attach" line 4)
>     invoked from within

Fixed now: didn't happen for me since I'm only testing on Unix targets.

> Otherwise, this is fine with me.

Here's the revised version, successfully tested as before.  Ok for
master now?

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2018-08-07  Brian Vandenberg  <phantall@gmail.com>
	    Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	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/interrupt-daemon-attach.c,
	gdb.base/interrupt-daemon-attach.exp: New test.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-attach-sigint.patch --]
[-- Type: text/x-patch, Size: 5172 bytes --]

# HG changeset patch
# Parent  14950de898700dec7316fb3a5691413d651ddf42
Can't interrupt process without controlling terminal on Solaris (PR gdb/8527)

2019-02-26  Brian Vandenberg  <phantall@gmail.com>
	    Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	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/interrupt-daemon-attach.c,
	gdb.base/interrupt-daemon-attach.exp: New test.

diff --git a/gdb/procfs.c b/gdb/procfs.c
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -909,7 +909,12 @@ proc_wait_for_stop (procinfo *pi)
 
   procfs_ctl_t cmd = PCWSTOP;
 
+  set_sigint_trap ();
+
   win = (write (pi->ctl_fd, (char *) &cmd, sizeof (cmd)) == sizeof (cmd));
+
+  clear_sigint_trap ();
+
   /* We been runnin' and we stopped -- need to update status.  */
   pi->status_valid = 0;
 
diff --git a/gdb/testsuite/gdb.base/interrupt-daemon-attach.c b/gdb/testsuite/gdb.base/interrupt-daemon-attach.c
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.base/interrupt-daemon-attach.c
@@ -0,0 +1,61 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017-2019 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 <unistd.h>
+
+/* GDB reads this to figure out the child's PID.  */
+pid_t child_pid;
+
+void
+marker (void)
+{
+}
+
+int
+main ()
+{
+  /* Don't let the test case run forever.  */
+  alarm (60);
+
+  child_pid = fork ();
+
+  switch (child_pid)
+    {
+    case -1:
+      return 1;
+
+    case 0:
+      break;
+
+    default:
+      while (1)
+	{
+	  marker ();
+	  usleep (1000);
+	}
+    }
+
+  /* Detach from controlling terminal.  */
+  if (setsid () == (pid_t) -1)
+    return 1;
+
+  for (;;)
+    ;
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/interrupt-daemon-attach.exp b/gdb/testsuite/gdb.base/interrupt-daemon-attach.exp
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.base/interrupt-daemon-attach.exp
@@ -0,0 +1,91 @@
+# Copyright 2019 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/>.  */
+
+if [target_info exists gdb,nosignals] {
+    verbose "Skipping interrupt-daemon-attach.exp because of nosignals."
+    continue
+}
+
+# This test requires sending ^C to interrupt the running target.
+if [target_info exists gdb,nointerrupts] {
+    verbose "Skipping interrupt-daemon-attach.exp because of nointerrupts."
+    return
+}
+
+if { ![can_spawn_for_attach] } {
+    return 0
+}
+
+standard_testfile
+
+if { [build_executable ${testfile}.exp ${testfile} $srcfile {debug}] == -1 } {
+    return -1
+}
+
+proc do_test {} {
+    global binfile
+    global decimal
+
+    set test_spawn_id [spawn_wait_for_attach $binfile]
+    set parent_pid [spawn_id_get_pid $test_spawn_id]
+
+    # Attach to the parent, run it to a known point, extract the
+    # child's PID, and detach.
+    with_test_prefix "parent" {
+	clean_restart ${binfile}
+
+	gdb_test "attach $parent_pid" \
+	    "Attaching to program.*, process $parent_pid.*" \
+	    "attach"
+
+	gdb_breakpoint "marker"
+	gdb_continue_to_breakpoint "marker"
+
+	set child_pid [get_integer_valueof "child_pid" -1]
+	if {$child_pid == -1} {
+	    return
+	}
+
+	gdb_test "detach" \
+	    "Detaching from program: .*process $parent_pid\r\n\\\[Inferior $decimal \\(.*\\) detached\\\]"
+
+	remote_exec host "kill -9 $parent_pid"
+    }
+
+    # Start over, and attach to the child this time.
+    with_test_prefix "child" {
+	global gdb_prompt
+
+	clean_restart $binfile
+
+	gdb_test "attach $child_pid" \
+	    "Attaching to program.*, process $child_pid.*" \
+	    "attach"
+
+	gdb_test_multiple "continue" "continue" {
+	    -re "Continuing" {
+		pass "continue"
+	    }
+	}
+
+	after 500 {send_gdb "\003"}
+	gdb_test "" "(Program|Thread .*) received signal SIGINT.*" \
+	    "stop with control-c"
+
+	remote_exec host "kill -9 $child_pid"
+    }
+}
+
+do_test

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Rainer Orth]

Hi Pedro,

>> On 02/26/2019 03:14 PM, Rainer Orth wrote:
>>
>>>> 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.
>>
>> It's a shame that the Solaris port doesn't support follow-fork.  I don't
>> suppose there's anything fundamentally impossible.  I'm sure it must
>> be possible to intercept fork/vfork/exec events with procfs.
>
> certainly: that's just one of many warts of the port.  However, before
> looking into adding missing features, I need to spend some time
> investigating the large number of tests that fail (often timeouts) that
> make the testsuite impossible to run usefully in the buildbots, taking
> at least half an hour to complete and being flaky as hell in some areas.
>
>>>> 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.
>>> I never got a reply to this one, but I think I just figured out the
>>> testcase part myself. 
>>
>> I'm curious -- what was the issue on Linux?
>
> The initial testcase was just misguided: I found no reliable way to
> really detach from the controlling tty without fork (which I'd have
> liked to avoid in order not to have to jump through hoops to determine
> the child pid).  I haven't looked closer after several false attempts to
> make this work, but just started afresh from attach-non-pgrp-leader.exp
> instead and modified that.
>
>>> +++ b/gdb/testsuite/gdb.base/sigint-no-ctty.exp
> [...]
>> Please add a small intro comment mentioning what the testcase is about.
>
> Done now.
>
>> AFAICT, this is basically testing the same thing that
>> gdb.base/interrupt-daemon.exp is testing, with the difference that it
>> exercises inferiors started with "attach" instead of "run".  I'd suggest
>
> More or less so, yes.  Just without the double fork and the bg variant
> that isn't supported on Solaris.
>
>> renaming the testcase to interrupt-daemon-attach.exp, so that it sits
>> alongside interrupt-daemon.exp.
>
> Fine with me.
>
>>> +proc do_test {} {
>>> +    global binfile
>>> +    global decimal
>>> +
>>> +    set test_spawn_id [spawn_wait_for_attach $binfile]
>>
>>
>> This is missing a can_wait_for_attach check:
>>
>> $ make check TESTS="gdb.base/sigint-no-ctty.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
>> ...
>> ERROR: tcl error sourcing src/gdb/testsuite/gdb.base/sigint-no-ctty.exp.
>> ERROR: can't spawn for attach with this target/board
>>     while executing
>> "error "can't spawn for attach with this target/board""
>>     invoked from within
>> "if ![can_spawn_for_attach] {
>>         # The caller should have checked can_spawn_for_attach itself
>>         # before getting here.
>>         error "can't spawn for attach with..."
>>     (procedure "spawn_wait_for_attach" line 4)
>>     invoked from within
>
> Fixed now: didn't happen for me since I'm only testing on Unix targets.
>
>> Otherwise, this is fine with me.
>
> Here's the revised version, successfully tested as before.  Ok for
> master now?

I've commited the patch to master now, taking the above as approval.
Also ok for the 8.3 branch after a couple of days once it's clear the
testcase works everywhere?

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Tom Tromey]

>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

Rainer> I've commited the patch to master now, taking the above as approval.
Rainer> Also ok for the 8.3 branch after a couple of days once it's clear the
Rainer> testcase works everywhere?

I think that would be fine.  Thank you for doing this.

Tom

Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Pedro Alves]

On 03/05/2019 03:47 PM, Tom Tromey wrote:
>>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
> 
> Rainer> I've commited the patch to master now, taking the above as approval.
> Rainer> Also ok for the 8.3 branch after a couple of days once it's clear the
> Rainer> testcase works everywhere?
> 
> I think that would be fine.  Thank you for doing this.

Agreed, thanks.

Pedro Alves