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

[Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>]

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