[commit+7.6] testsuite: Add more valgrind kills on cleanup

[commit+7.6] testsuite: Add more valgrind kills on cleanup

[Jan Kratochvil]

Hi,

I have already checked in a similar stale valgrind cleanup:
	[patch] testsuite: valgrind-infcall.exp: explicit kill
	http://sourceware.org/ml/gdb-patches/2013-02/msg00459.html
	Message-ID: <20130218154552.GA16416@host2.jankratochvil.net>

I have found today that it still does not solve everything, the missed case(s)
are patched below.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-03/msg00156.html
and also for 7.6 as this is a very safe patch:
	http://sourceware.org/ml/gdb-cvs/2013-03/msg00157.html

As Tom asked off-list before this case was not caught by 'orphanripper'
	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-orphanripper.c
present in Fedora GDB as this tool got stale waiting on fd EOF with stuck
valgrind had this fd open for writing.  This was exactly the reason why
I wrote 'orphanripper' and its SIGCHLD handling should have caught that, I do
not yet understand why 'orphanripper' failed in this case.


Jan


http://sourceware.org/ml/gdb-cvs/2013-03/msg00156.html

--- src/gdb/testsuite/ChangeLog	2013/03/15 17:10:43	1.3588
+++ src/gdb/testsuite/ChangeLog	2013/03/17 20:37:30	1.3589
@@ -1,3 +1,10 @@
+2013-03-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.base/valgrind-infcall.exp
+	(continue #$continue_count) <remote connection closed>
+	(continue #$continue_count) <valgrind vgdb has terminated>: Add kill of
+	$valgrind_pid.
+
 2013-03-15  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.cp/overload.cc (intintfunc): New.
--- src/gdb/testsuite/gdb.base/valgrind-infcall.exp	2013/02/27 18:46:52	1.7
+++ src/gdb/testsuite/gdb.base/valgrind-infcall.exp	2013/03/17 20:37:32	1.8
@@ -101,10 +101,14 @@
 	}
 	-re "Remote connection closed.*\r\n$gdb_prompt $" {
 	    fail "$test (remote connection closed)"
+	    # Only if valgrind got stuck.
+	    remote_exec host "kill -9 ${valgrind_pid}"
 	    return -1
 	}
 	-re "The program is not being run\\.\r\n$gdb_prompt $" {
 	    fail "$test (valgrind vgdb has terminated)"
+	    # Only if valgrind got stuck.
+	    remote_exec host "kill -9 ${valgrind_pid}"
 	    return -1
 	}
 	-re "\r\n$gdb_prompt $" {

Re: [commit+7.6] testsuite: Add more valgrind kills on cleanup

[Jan Kratochvil]

On Sun, 17 Mar 2013 21:47:46 +0100, Jan Kratochvil wrote:
> As Tom asked off-list before this case was not caught by 'orphanripper'
> 	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-orphanripper.c
> present in Fedora GDB as this tool got stale waiting on fd EOF with stuck
> valgrind had this fd open for writing.  This was exactly the reason why
> I wrote 'orphanripper' and its SIGCHLD handling should have caught that, I do
> not yet understand why 'orphanripper' failed in this case.

I found a race that kernel reports Z (Zombie) state for a process but
'kill (child, 0)' still returns 0 at that moment.  So the code parses
/proc/CHILD/stat now.

It is now unrelated to FSF GDB but it was discussed the 'orphanripper'
testsuite wrapper could be upstreamed.


Jan


diff --git a/gdb-orphanripper.c b/gdb-orphanripper.c
index f8e3f49..d79d93c 100644
--- a/gdb-orphanripper.c
+++ b/gdb-orphanripper.c
@@ -47,13 +47,10 @@
 
 static const char *progname;
 
-static volatile int signal_chld_hit = 0;
 static volatile pid_t child;
 
 static void signal_chld (int signo)
 {
-  if (child && kill (child, 0) != 0)
-    signal_chld_hit = 1;
 }
 
 static volatile int signal_alrm_hit = 0;
@@ -104,6 +101,44 @@ static int read_out (int amaster)
   return 1;
 }
 
+/* kill (child, 0) == 0 sometimes even when CHILD's state is already "Z".  */
+
+static int child_exited (void)
+{
+  char buf[200];
+  int fd, i, retval;
+  ssize_t got;
+  char *state;
+
+  snprintf (buf, sizeof (buf), "/proc/%ld/stat", (long) child);
+  fd = open (buf, O_RDONLY);
+  if (fd == -1)
+    {
+      perror ("open (/proc/CHILD/stat)");
+      exit (EXIT_FAILURE);
+    }
+  got = read (fd, buf, sizeof(buf));
+  if (got <= 0)
+    {
+      perror ("read (/proc/CHILD/stat)");
+      exit (EXIT_FAILURE);
+    }
+  if (close (fd) != 0)
+    {
+      perror ("close (/proc/CHILD/stat)");
+      exit (EXIT_FAILURE);
+    }
+  i = sscanf (buf, "%*d%*s%ms", &state);
+  if (i != 1)
+    {
+      perror ("sscanf (/proc/CHILD/stat)");
+      exit (EXIT_FAILURE);
+    }
+  retval = strcmp (state, "Z") == 0;
+  free (state);
+  return retval;
+}
+
 static int spawn (char **argv, int timeout)
 {
   pid_t child_got;
@@ -157,6 +192,11 @@ static int spawn (char **argv, int timeout)
 	assert (i == STDIN_FILENO);
 #endif
 
+	i = sigemptyset (&set);
+	assert (i == 0);
+	i = sigprocmask (SIG_SETMASK, &set, NULL);
+	assert (i == 0);
+
 	/* Do not setpgrp(2) in the parent process as the process-group
 	   is shared for the whole sh(1) pipeline we could be a part
 	   of.  The process-group is set according to PID of the first
@@ -206,7 +246,7 @@ static int spawn (char **argv, int timeout)
       i = ppoll (&pollfd, 1, NULL, &set);
       if (i == -1 && errno == EINTR)
 	{
-	  if (signal_chld_hit)
+	  if (child_exited ())
 	    break;
 	  /* Non-CHILD child may have exited.  */
 	  continue;
@@ -230,7 +270,7 @@ static int spawn (char **argv, int timeout)
 	  exit (EXIT_FAILURE);
 	}
       /* Child exited?  */
-      if (signal_chld_hit)
+      if (child_exited ())
 	break;
     }
 
@@ -279,12 +319,10 @@ static int spawn (char **argv, int timeout)
       exit (EXIT_FAILURE);
     }
 
-  /* In the POLLHUP case we may not have seen SIGCHLD so far.  */
+  /* Not used in fact.  */
   i = sigprocmask (SIG_SETMASK, &set, NULL);
   assert (i == 0);
 
-  assert (signal_chld_hit != 0);
-
   /* Do not unset O_NONBLOCK as a stale child (the whole purpose of this
      program) having open its output pty would block us in read_out.  */
 #if 0