public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Move foreach_with_prefix to lib/gdb.exp
  2016-01-19 17:52 [PATCH 0/3] Fix fork bugs Pedro Alves
  2016-01-19 17:52 ` [PATCH 2/3] Fix PR 19494: hang when killing unfollowed fork children Pedro Alves
  2016-01-19 17:52 ` [PATCH 3/3] Fix PR 19461: strange "info thread" behavior in non-stop Pedro Alves
@ 2016-01-19 17:52 ` Pedro Alves
  2016-01-25 13:22 ` [PATCH 0/3] Fix fork bugs Pedro Alves
  3 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2016-01-19 17:52 UTC (permalink / raw)
  To: gdb-patches

gdb/testsuite/ChangeLog:
2016-01-19  Pedro Alves  <palves@redhat.com>

	* gdb.base/step-sw-breakpoint-adjust-pc.exp (foreach_with_prefix):
	Delete, moved to lib/gdb.exp.
	* gdb.threads/forking-threads-plus-breakpoint.exp
	(foreach_with_prefix): Likewise.
	* gdb.threads/process-dies-while-handling-bp.exp
	(foreach_with_prefix): Likewise.
	* lib/gdb.exp (foreach_with_prefix): New procedure.
---
 gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp      | 12 ------------
 .../gdb.threads/forking-threads-plus-breakpoint.exp          | 12 ------------
 gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp | 12 ------------
 gdb/testsuite/lib/gdb.exp                                    | 12 ++++++++++++
 4 files changed, 12 insertions(+), 36 deletions(-)

diff --git a/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp b/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp
index 378d71c..28ffbe7 100644
--- a/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp
+++ b/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp
@@ -73,18 +73,6 @@ proc test {non_stop displaced always_inserted} {
     }
 }
 
-# Wrapper for foreach that calls with_test_prefix on each iteration,
-# including the iterator's current value in the prefix.
-
-proc foreach_with_prefix {var list body} {
-    upvar 1 $var myvar
-    foreach myvar $list {
-	with_test_prefix "$var=$myvar" {
-	    uplevel 1 $body
-	}
-    }
-}
-
 foreach_with_prefix non_stop { "off" "on" } {
     foreach_with_prefix displaced_step { "off" "on" } {
 	foreach_with_prefix always_inserted { "off" "on" } {
diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
index d204125..b5f7c21 100644
--- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
+++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
@@ -92,18 +92,6 @@ proc do_test { cond_bp_target detach_on_fork } {
 	"only inferior 1 left"
 }
 
-# Wrapper for foreach that calls with_test_prefix on each iteration,
-# including the iterator's current value in the prefix.
-
-proc foreach_with_prefix {var list body} {
-    upvar 1 $var myvar
-    foreach myvar $list {
-	with_test_prefix "$var=$myvar" {
-	    uplevel 1 $body
-	}
-    }
-}
-
 foreach_with_prefix cond_bp_target {1 0} {
     foreach_with_prefix detach_on_fork {"on" "off"} {
 	do_test $cond_bp_target $detach_on_fork
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp b/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
index 8605a38..2db31af 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
@@ -128,18 +128,6 @@ proc do_test { non_stop cond_bp_target } {
 	"no threads left"
 }
 
-# Wrapper for foreach that calls with_test_prefix on each iteration,
-# including the iterator's current value in the prefix.
-
-proc foreach_with_prefix {var list body} {
-    upvar 1 $var myvar
-    foreach myvar $list {
-	with_test_prefix "$var=$myvar" {
-	    uplevel 1 $body
-	}
-    }
-}
-
 foreach_with_prefix non_stop {"on" "off"} {
     foreach_with_prefix cond_bp_target {1 0} {
 	do_test $non_stop $cond_bp_target
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e6fe62c..66821eb 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1906,6 +1906,18 @@ proc with_test_prefix { prefix body } {
   }
 }
 
+# Wrapper for foreach that calls with_test_prefix on each iteration,
+# including the iterator's name and current value in the prefix.
+
+proc foreach_with_prefix {var list body} {
+    upvar 1 $var myvar
+    foreach myvar $list {
+	with_test_prefix "$var=$myvar" {
+	    uplevel 1 $body
+	}
+    }
+}
+
 # Run BODY in the context of the caller.  After BODY is run, the variables
 # listed in VARS will be reset to the values they had before BODY was run.
 #
-- 
1.9.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] Fix PR 19494: hang when killing unfollowed fork children
  2016-01-19 17:52 [PATCH 0/3] Fix fork bugs Pedro Alves
@ 2016-01-19 17:52 ` Pedro Alves
  2016-01-19 17:52 ` [PATCH 3/3] Fix PR 19461: strange "info thread" behavior in non-stop Pedro Alves
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2016-01-19 17:52 UTC (permalink / raw)
  To: gdb-patches

linux_nat_kill relies on get_last_target_status to determine whether
the current inferior is stopped at a unfollowed fork/vfork event.
This is bad because many things can happen ever since we caught the
fork/vfork event...  This commit rewrites that code to instead walk
the thread list looking for unfollowed fork events, similarly to what
was done for remote.c.

New test included.  The main idea of the test is make sure that when
the program stops for a fork catchpoint, and the user kills the
parent, gdb also kills the unfollowed fork child.  Since the child
hasn't been added as an inferior at that point, we need some other
portable way to detect that the child is gone.  The test uses a pipe
for that.  The program forks twice, so you have grandparent, child and
grandchild.  The grandchild inherits the write side of the pipe.  The
grandparent hangs reading from the pipe, since nothing ever writes to
it.  If, when GDB kills the child, it also kills the grandchild, then
the grandparent's pipe read returns 0/EOF and the test passes.
Otherwise, if GDB doesn't kill the grandchild, then the pipe read
never returns and the test times out, like:

 FAIL: gdb.base/catch-fork-kill.exp: fork-kind=fork: exit-kind=kill: fork: kill parent (timeout)
 FAIL: gdb.base/catch-fork-kill.exp: fork-kind=vfork: exit-kind=kill: vfork: kill parent (timeout)

No regressions on x86_64 Fedora 20.  New test passes with gdbserver as
well.

gdb/ChangeLog:
2016-01-19  Pedro Alves  <palves@redhat.com>

	PR gdb/19494
	* linux-nat.c (kill_one_lwp): New, factored out from ...
	(kill_callback): ... this.
	(kill_wait_callback): New, factored out from ...
	(kill_wait_one_lwp): ... this.
	(kill_unfollowed_fork_children): New function.
	(linux_nat_kill): Use it.

gdb/testsuite/ChangeLog:
2016-01-19  Pedro Alves  <palves@redhat.com>

	PR gdb/19494
	* gdb.base/catch-fork-kill.c: New file.
	* gdb.base/catch-fork-kill.exp: New file.
---
 gdb/linux-nat.c                            | 104 +++++++++++++++++++----------
 gdb/testsuite/gdb.base/catch-fork-kill.c   |  97 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/catch-fork-kill.exp |  99 +++++++++++++++++++++++++++
 3 files changed, 265 insertions(+), 35 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/catch-fork-kill.c
 create mode 100644 gdb/testsuite/gdb.base/catch-fork-kill.exp

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index eb609c8..6ded38d 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3466,44 +3466,44 @@ linux_nat_wait (struct target_ops *ops,
   return event_ptid;
 }
 
-static int
-kill_callback (struct lwp_info *lp, void *data)
+/* Kill one LWP.  */
+
+static void
+kill_one_lwp (pid_t pid)
 {
   /* PTRACE_KILL may resume the inferior.  Send SIGKILL first.  */
 
   errno = 0;
-  kill_lwp (ptid_get_lwp (lp->ptid), SIGKILL);
+  kill_lwp (pid, SIGKILL);
   if (debug_linux_nat)
     {
       int save_errno = errno;
 
       fprintf_unfiltered (gdb_stdlog,
-			  "KC:  kill (SIGKILL) %s, 0, 0 (%s)\n",
-			  target_pid_to_str (lp->ptid),
+			  "KC:  kill (SIGKILL) %ld, 0, 0 (%s)\n", (long) pid,
 			  save_errno ? safe_strerror (save_errno) : "OK");
     }
 
   /* Some kernels ignore even SIGKILL for processes under ptrace.  */
 
   errno = 0;
-  ptrace (PTRACE_KILL, ptid_get_lwp (lp->ptid), 0, 0);
+  ptrace (PTRACE_KILL, pid, 0, 0);
   if (debug_linux_nat)
     {
       int save_errno = errno;
 
       fprintf_unfiltered (gdb_stdlog,
-			  "KC:  PTRACE_KILL %s, 0, 0 (%s)\n",
-			  target_pid_to_str (lp->ptid),
+			  "KC:  PTRACE_KILL %ld, 0, 0 (%s)\n", (long) pid,
 			  save_errno ? safe_strerror (save_errno) : "OK");
     }
-
-  return 0;
 }
 
-static int
-kill_wait_callback (struct lwp_info *lp, void *data)
+/* Wait for an LWP to die.  */
+
+static void
+kill_wait_one_lwp (pid_t pid)
 {
-  pid_t pid;
+  pid_t res;
 
   /* We must make sure that there are no pending events (delayed
      SIGSTOPs, pending SIGTRAPs, etc.) to make sure the current
@@ -3511,49 +3511,83 @@ kill_wait_callback (struct lwp_info *lp, void *data)
 
   do
     {
-      pid = my_waitpid (ptid_get_lwp (lp->ptid), NULL, __WALL);
-      if (pid != (pid_t) -1)
+      res = my_waitpid (pid, NULL, __WALL);
+      if (res != (pid_t) -1)
 	{
 	  if (debug_linux_nat)
 	    fprintf_unfiltered (gdb_stdlog,
-				"KWC: wait %s received unknown.\n",
-				target_pid_to_str (lp->ptid));
+				"KWC: wait %ld received unknown.\n",
+				(long) pid);
 	  /* The Linux kernel sometimes fails to kill a thread
 	     completely after PTRACE_KILL; that goes from the stop
 	     point in do_fork out to the one in get_signal_to_deliver
 	     and waits again.  So kill it again.  */
-	  kill_callback (lp, NULL);
+	  kill_one_lwp (pid);
 	}
     }
-  while (pid == ptid_get_lwp (lp->ptid));
+  while (res == pid);
+
+  gdb_assert (res == -1 && errno == ECHILD);
+}
+
+/* Callback for iterate_over_lwps.  */
 
-  gdb_assert (pid == -1 && errno == ECHILD);
+static int
+kill_callback (struct lwp_info *lp, void *data)
+{
+  kill_one_lwp (ptid_get_lwp (lp->ptid));
   return 0;
 }
 
+/* Callback for iterate_over_lwps.  */
+
+static int
+kill_wait_callback (struct lwp_info *lp, void *data)
+{
+  kill_wait_one_lwp (ptid_get_lwp (lp->ptid));
+  return 0;
+}
+
+/* Kill the fork children of any threads of inferior INF that are
+   stopped at a fork event.  */
+
+static void
+kill_unfollowed_fork_children (struct inferior *inf)
+{
+  struct thread_info *thread;
+
+  ALL_NON_EXITED_THREADS (thread)
+    if (thread->inf == inf)
+      {
+	struct target_waitstatus *ws = &thread->pending_follow;
+
+	if (ws->kind == TARGET_WAITKIND_FORKED
+	    || ws->kind == TARGET_WAITKIND_VFORKED)
+	  {
+	    ptid_t child_ptid = ws->value.related_pid;
+	    int child_pid = ptid_get_pid (child_ptid);
+	    int child_lwp = ptid_get_lwp (child_ptid);
+	    int status;
+
+	    kill_one_lwp (child_lwp);
+	    kill_wait_one_lwp (child_lwp);
+
+	    /* Let the arch-specific native code know this process is
+	       gone.  */
+	    linux_nat_forget_process (child_pid);
+	  }
+      }
+}
+
 static void
 linux_nat_kill (struct target_ops *ops)
 {
   struct target_waitstatus last;
-  ptid_t last_ptid;
-  int status;
 
   /* If we're stopped while forking and we haven't followed yet,
      kill the other task.  We need to do this first because the
      parent will be sleeping if this is a vfork.  */
-
-  get_last_target_status (&last_ptid, &last);
-
-  if (last.kind == TARGET_WAITKIND_FORKED
-      || last.kind == TARGET_WAITKIND_VFORKED)
-    {
-      ptrace (PT_KILL, ptid_get_pid (last.value.related_pid), 0, 0);
-      wait (&status);
-
-      /* Let the arch-specific native code know this process is
-	 gone.  */
-      linux_nat_forget_process (ptid_get_pid (last.value.related_pid));
-    }
+  kill_unfollowed_fork_children (current_inferior ());
 
   if (forks_exist_p ())
     linux_fork_killall ();
diff --git a/gdb/testsuite/gdb.base/catch-fork-kill.c b/gdb/testsuite/gdb.base/catch-fork-kill.c
new file mode 100644
index 0000000..607df2e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/catch-fork-kill.c
@@ -0,0 +1,97 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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 <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <assert.h>
+
+int fds[2] = { -1, -1 };
+
+static void
+grandparent_done (void)
+{
+}
+
+/* The exp file overrides this in order to test both fork and
+   vfork.  */
+#ifndef FORK
+#define FORK fork
+#endif
+
+int
+main (void)
+{
+  int pid;
+  int nbytes;
+  const char string[] = "Hello, world!\n";
+  char readbuffer[80];
+
+  /* Don't run forever.  */
+  alarm (300);
+
+  /* Create a pipe.  The write side will be inherited all the way to
+     the grandchild.  The grandparent will read this, expecting to see
+     EOF (meaning the grandchild closed the pipe).  */
+  pipe (fds);
+
+  pid = FORK ();
+  if (pid < 0)
+    {
+      perror ("fork");
+      exit (1);
+    }
+  else if (pid == 0)
+    {
+      /* Close input side of pipe.  */
+      close (fds[0]);
+
+      pid = FORK ();
+      if (pid == 0)
+	{
+	  printf ("I'm the grandchild!\n");
+
+	  /* Don't explicitly close the pipe.  If GDB fails to kill
+	     this process, then the grandparent will hang in the pipe
+	     read below.  */
+#if 0
+	  close (fds[1]);
+#endif
+	  while (1)
+	    sleep (1);
+	}
+      else
+	{
+	  close (fds[1]);
+	  printf ("I'm the proud parent of child #%d!\n", pid);
+	  wait (NULL);
+	}
+    }
+  else if (pid > 0)
+    {
+      close (fds[1]);
+      printf ("I'm the proud parent of child #%d!\n", pid);
+      nbytes = read (fds[0], readbuffer, sizeof (readbuffer));
+      assert (nbytes == 0);
+      printf ("read returned nbytes=%d\n", nbytes);
+      wait (NULL);
+
+      grandparent_done ();
+    }
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/catch-fork-kill.exp b/gdb/testsuite/gdb.base/catch-fork-kill.exp
new file mode 100644
index 0000000..29d2077
--- /dev/null
+++ b/gdb/testsuite/gdb.base/catch-fork-kill.exp
@@ -0,0 +1,99 @@
+# Copyright 2016 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/>.
+
+# When we intercept a fork/vfork with a catchpoint, the child is left
+# stopped.  At that point, if we kill the parent, we should kill the
+# child as well.  This test makes sure that works.  See PR gdb/19494.
+
+# The main idea of the test is make sure that when the program stops
+# for a fork catchpoint, and the user kills the parent, gdb also kills
+# the unfollowed fork child.  Since the child hasn't been added as an
+# inferior at that point, we need some other portable way to detect
+# that the child is gone.  The test uses a pipe for that.  The program
+# forks twice, so you have grandparent, child and grandchild.  The
+# grandchild inherits the write side of the pipe.  The grandparent
+# hangs reading from the pipe, since nothing ever writes to it.  If,
+# when GDB kills the child, it also kills the grandchild, then the
+# grandparent's pipe read returns 0/EOF and the test passes.
+# Otherwise, if GDB doesn't kill the grandchild, then the pipe read
+# never returns and the test times out.
+
+standard_testfile
+
+# Build two programs -- one for fork, and another for vfork.
+set testfile_fork "${testfile}-fork"
+set testfile_vfork "${testfile}-vfork"
+
+foreach kind {"fork" "vfork"} {
+    set compile_options "debug additional_flags=-DFORK=$kind"
+    set testfile [set testfile_$kind]
+    if {[build_executable $testfile.exp $testfile ${srcfile} \
+	     ${compile_options}] == -1} {
+	untested "failed to compile $testfile"
+	return -1
+    }
+}
+
+# The test proper.  FORK_KIND is either "fork" or "vfork".  EXIT_KIND
+# is either "exit" (run the parent to exit) or "kill" (kill parent).
+
+proc do_test {fork_kind exit_kind} {
+    global testfile testfile_$fork_kind
+
+    set testfile [set testfile_$fork_kind]
+
+    with_test_prefix "$fork_kind" {
+	clean_restart $testfile
+
+	if ![runto_main] {
+	    untested "could not run to main"
+	    return -1
+	}
+
+	gdb_test_no_output "set follow-fork child"
+	gdb_test_no_output "set detach-on-fork off"
+
+	gdb_test "catch $fork_kind" "Catchpoint .*($fork_kind).*"
+
+	gdb_test "continue" \
+	    "Catchpoint \[0-9\]* \\(${fork_kind}ed process \[0-9\]*\\),.*" \
+	    "continue to child ${fork_kind}"
+
+	gdb_test "continue" \
+	    "Catchpoint \[0-9\]* \\(${fork_kind}ed process \[0-9\]*\\),.*" \
+	    "continue to grandchild ${fork_kind}"
+
+	gdb_test "kill inferior 2" "" "kill child" \
+	    "Kill the program being debugged.*y or n. $" "y"
+
+	gdb_test "inferior 1" "Switching to inferior 1 .*" "switch to parent"
+
+	if {$exit_kind == "exit"} {
+	    gdb_test "break grandparent_done" "Breakpoint .*"
+	    gdb_test "continue" "hit Breakpoint .*, grandparent_done.*"
+	} elseif {$exit_kind == "kill"} {
+	    gdb_test "kill" "" "kill parent" \
+		"Kill the program being debugged.*y or n. $" "y"
+	} else {
+	    perror "unreachable"
+	}
+    }
+}
+
+foreach_with_prefix fork-kind {"fork" "vfork"} {
+    foreach_with_prefix exit-kind {"exit" "kill"} {
+	do_test ${fork-kind} ${exit-kind}
+    }
+}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 0/3] Fix fork bugs
@ 2016-01-19 17:52 Pedro Alves
  2016-01-19 17:52 ` [PATCH 2/3] Fix PR 19494: hang when killing unfollowed fork children Pedro Alves
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Pedro Alves @ 2016-01-19 17:52 UTC (permalink / raw)
  To: gdb-patches

My intention was to fix PR 19461 (strange "info thread" behavior in
non-stop), which looked like an easy one-liner.  As usual, the tests
are the hardest part, and reveal other problems...

In this case, the test I wrote for PR 19461 tripped on a gdb hang,
which I filed as PR 19494 (hang when killing unfollowed fork
children), and fixed.  I then decided to write a test specific for
that bug too, for better coverage.

Since I was going to add two more copies of foreach_with_prefix, one
per test, I instead thought I'd finally propose moving it to common
code.

Pedro Alves (3):
  Move foreach_with_prefix to lib/gdb.exp
  Fix PR 19494: hang when killing unfollowed fork children
  Fix PR 19461: strange "info thread" behavior in non-stop

 gdb/infrun.c                                       |  11 ++
 gdb/linux-nat.c                                    | 104 ++++++++-----
 gdb/testsuite/gdb.base/catch-fork-kill.c           |  97 ++++++++++++
 gdb/testsuite/gdb.base/catch-fork-kill.exp         |  99 +++++++++++++
 gdb/testsuite/gdb.base/fork-running-state.c        |  83 +++++++++++
 gdb/testsuite/gdb.base/fork-running-state.exp      | 163 +++++++++++++++++++++
 .../gdb.base/step-sw-breakpoint-adjust-pc.exp      |  12 --
 .../forking-threads-plus-breakpoint.exp            |  12 --
 .../gdb.threads/process-dies-while-handling-bp.exp |  12 --
 gdb/testsuite/lib/gdb.exp                          |  12 ++
 10 files changed, 534 insertions(+), 71 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/catch-fork-kill.c
 create mode 100644 gdb/testsuite/gdb.base/catch-fork-kill.exp
 create mode 100644 gdb/testsuite/gdb.base/fork-running-state.c
 create mode 100644 gdb/testsuite/gdb.base/fork-running-state.exp

-- 
1.9.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] Fix PR 19461: strange "info thread" behavior in non-stop
  2016-01-19 17:52 [PATCH 0/3] Fix fork bugs Pedro Alves
  2016-01-19 17:52 ` [PATCH 2/3] Fix PR 19494: hang when killing unfollowed fork children Pedro Alves
@ 2016-01-19 17:52 ` Pedro Alves
  2016-01-19 17:52 ` [PATCH 1/3] Move foreach_with_prefix to lib/gdb.exp Pedro Alves
  2016-01-25 13:22 ` [PATCH 0/3] Fix fork bugs Pedro Alves
  3 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2016-01-19 17:52 UTC (permalink / raw)
  To: gdb-patches

If you have "set follow-fork child" set, then if you do "info threads"
right after a fork, and before the child reports any other event to
GDB core, you'll see:

(gdb) info threads
  Id   Target Id         Frame
* 1.1  Thread 0x7ffff7fc1740 (LWP 31875) "fork-plus-threa" (running)
  2.1  process 31879 "fork-plus-threa" Selected thread is running.
(gdb)

The "Selected thread is running." bit is a bogus error.  That was GDB
trying to fetch the current frame of thread 2.1, because the external
runnning state is "stopped", and then throwing an error because the
thread is actually running.

This actually affects all-stop + schedule-multiple as well.

The problem here is that on a fork event, GDB doesn't update the
external parent/child running states.

New comprehensive test included.  The "kill inferior 1" / "kill
inferior 2" bits also trip on PR gdb/19494 (hang killing unfollowed
fork children), which was fixed by the previous patch.

gdb/ChangeLog:
2016-01-19  Pedro Alves  <palves@redhat.com>

	PR threads/19461
	* infrun.c (handle_inferior_event_1) <fork/vfork>: Update
	parent/child running states.

gdb/testsuite/ChangeLog:
2016-01-19  Pedro Alves  <palves@redhat.com>

	PR threads/19461
	* gdb.base/fork-running-state.c: New file.
	* gdb.base/fork-running-state.exp: New file.
---
 gdb/infrun.c                                  |  11 ++
 gdb/testsuite/gdb.base/fork-running-state.c   |  83 +++++++++++++
 gdb/testsuite/gdb.base/fork-running-state.exp | 163 ++++++++++++++++++++++++++
 3 files changed, 257 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/fork-running-state.c
 create mode 100644 gdb/testsuite/gdb.base/fork-running-state.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 33981d2..15210c9 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5198,6 +5198,17 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	  parent = ecs->ptid;
 	  child = ecs->ws.value.related_pid;
 
+	  /* At this point, the parent is marked running, and the
+	     child is marked stopped.  */
+
+	  /* If not resuming the parent, mark it stopped.  */
+	  if (follow_child && !detach_fork && !non_stop && !sched_multi)
+	    set_running (parent, 0);
+
+	  /* If resuming the child, mark it running.  */
+	  if (follow_child || (!detach_fork && (non_stop || sched_multi)))
+	    set_running (child, 1);
+
 	  /* In non-stop mode, also resume the other branch.  */
 	  if (!detach_fork && (non_stop
 			       || (sched_multi && target_is_non_stop_p ())))
diff --git a/gdb/testsuite/gdb.base/fork-running-state.c b/gdb/testsuite/gdb.base/fork-running-state.c
new file mode 100644
index 0000000..c7103fd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fork-running-state.c
@@ -0,0 +1,83 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015-2016 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>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+int save_parent;
+
+/* The fork child.  Just runs forever.  */
+
+static int
+fork_child (void)
+{
+  while (1)
+    {
+      sleep (1);
+
+      /* Exit if GDB kills the parent.  */
+      if (getppid () != save_parent)
+	break;
+      if (kill (getppid (), 0) != 0)
+	break;
+    }
+
+  return 0;
+}
+
+/* The fork parent.  Just runs forever waiting for the child to
+   exit.  */
+
+static int
+fork_parent (void)
+{
+  if (wait (NULL) == -1)
+    {
+      perror ("wait");
+      return 1;
+    }
+
+  return 0;
+}
+
+int
+main (void)
+{
+  pid_t pid;
+
+  save_parent = getpid ();
+
+  /* Don't run forever.  */
+  alarm (180);
+
+  /* The parent and child should basically run forever without
+     tripping on any debug event.  We want to check that GDB updates
+     the parent and child running states correctly right after the
+     fork.  */
+  pid = fork ();
+  if (pid > 0)
+    return fork_parent ();
+  else if (pid == 0)
+    return fork_child ();
+  else
+    {
+      perror ("fork");
+      return 1;
+    }
+}
diff --git a/gdb/testsuite/gdb.base/fork-running-state.exp b/gdb/testsuite/gdb.base/fork-running-state.exp
new file mode 100644
index 0000000..be81be6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fork-running-state.exp
@@ -0,0 +1,163 @@
+# Copyright (C) 2016 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/>.
+
+# Regression test for PR threads/19461 (strange "info thread" behavior
+# in non-stop).  GDB used to miss updating the parent/child running
+# states after a fork.
+
+standard_testfile
+
+# The test proper.
+
+proc do_test { detach_on_fork follow_fork non_stop schedule_multiple } {
+    global GDBFLAGS
+    global srcfile testfile
+    global gdb_prompt
+
+    save_vars { GDBFLAGS } {
+	append GDBFLAGS " -ex \"set non-stop $non_stop\""
+
+	if {[prepare_for_testing "failed to prepare" \
+		 $testfile $srcfile {debug}] == -1} {
+	    return -1
+	}
+    }
+
+    if ![runto_main] then {
+	fail "Can't run to main"
+	return 0
+    }
+
+    # If debugging with target remote, check whether the all-stop
+    # variant of the RSP is being used.  If so, we can't run the
+    # all-stop tests.
+    if { [target_info exists gdb_protocol]
+	 && ([target_info gdb_protocol] == "remote"
+	     || [target_info gdb_protocol] == "extended-remote")} {
+
+	set test "maint show target-non-stop"
+	gdb_test_multiple "maint show target-non-stop" $test {
+	    -re "(is|currently) on.*$gdb_prompt $" {
+	    }
+	    -re "(is|currently) off.*$gdb_prompt $" {
+		unsupported "can't issue info threads while target is running"
+		return 0
+	    }
+	}
+    }
+
+    # We want to catch "[New inferior ...]" below, to avoid sleeping.
+    if {$detach_on_fork == "off" || $follow_fork == "child"} {
+	gdb_test_no_output "set print inferior-events on"
+    }
+
+    gdb_test_no_output "set detach-on-fork $detach_on_fork"
+
+    gdb_test_no_output "set follow-fork $follow_fork"
+    if {$non_stop == "off"} {
+	gdb_test_no_output "set schedule-multiple $schedule_multiple"
+    }
+
+    set test "continue &"
+    gdb_test_multiple $test $test {
+	-re "$gdb_prompt " {
+	    pass $test
+	}
+    }
+
+    if {$detach_on_fork == "off" || $follow_fork == "child"} {
+	set test "fork child appears"
+	gdb_test_multiple "" $test {
+	    -re "\\\[New inferior " {
+		pass $test
+	    }
+	}
+    } else {
+	# All we can do is wait a little bit for the parent to fork.
+	sleep 1
+    }
+
+    set not_nl "\[^\r\n\]*"
+
+    if {$detach_on_fork == "on" && $non_stop == "on" && $follow_fork == "child"} {
+	gdb_test "info threads" \
+	    "  2.1 ${not_nl}\\\(running\\\).*No selected thread.*"
+    } elseif {$detach_on_fork == "on" && $follow_fork == "child"} {
+	gdb_test "info threads" \
+	    "\\\* 2.1 ${not_nl}\\\(running\\\)"
+    } elseif {$detach_on_fork == "on"} {
+	gdb_test "info threads" \
+	    "\\\* 1 ${not_nl}\\\(running\\\)"
+    } elseif {$non_stop == "on"
+	      || ($schedule_multiple == "on" && $follow_fork == "parent")} {
+	# Both parent and child should be marked running, and the
+	# parent should be selected.
+	gdb_test "info threads" \
+	    [multi_line \
+		 "\\\* 1.1 ${not_nl} \\\(running\\\)${not_nl}" \
+		 "  2.1 ${not_nl} \\\(running\\\)"]
+    } elseif {$schedule_multiple == "on" && $follow_fork == "child"} {
+	# Both parent and child should be marked running, and the
+	# child should be selected.
+	gdb_test "info threads" \
+	    [multi_line \
+		 "  1.1 ${not_nl} \\\(running\\\)${not_nl}" \
+		 "\\\* 2.1 ${not_nl} \\\(running\\\)"]
+    } else {
+	set test "only $follow_fork marked running"
+	gdb_test_multiple "info threads" $test {
+	    -re "\\\(running\\\)${not_nl}\\\(running\\\)\r\n$gdb_prompt $" {
+		fail $test
+	    }
+	    -re "\\\* 1.1 ${not_nl}\\\(running\\\)\r\n  2.1 ${not_nl}\r\n$gdb_prompt $" {
+		gdb_assert [string eq $follow_fork "parent"] $test
+	    }
+	    -re "1.1 ${not_nl}\r\n\\\* 2.1 ${not_nl}\\\(running\\\)\r\n$gdb_prompt $" {
+		gdb_assert [string eq $follow_fork "child"] $test
+	    }
+	}
+    }
+
+    # We don't want to see "Inferior exited" in reaction to the kills.
+    gdb_test_no_output "set print inferior-events off"
+
+    # Kill both parent and child.
+    if {$detach_on_fork == "off" || $follow_fork == "parent"} {
+	gdb_test_no_output "kill inferior 1" "kill parent"
+    }
+    if {$detach_on_fork == "off" || $follow_fork == "child"} {
+	gdb_test_no_output "kill inferior 2" "kill child"
+    }
+}
+
+# Exercise all permutations of:
+#
+#  set detach-on-fork off|on
+#  set follow-fork parent|child
+#  set non-stop on|off
+#  set schedule-multiple on|off
+
+foreach_with_prefix detach-on-fork {"off" "on"} {
+    foreach_with_prefix follow-fork {"parent" "child"} {
+	with_test_prefix "non-stop" {
+	    do_test ${detach-on-fork} ${follow-fork} "on" "-"
+	}
+	with_test_prefix "all-stop" {
+	    foreach_with_prefix schedule-multiple {"on" "off"} {
+		do_test ${detach-on-fork} ${follow-fork} "off" ${schedule-multiple}
+	    }
+	}
+    }
+}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] Fix fork bugs
  2016-01-19 17:52 [PATCH 0/3] Fix fork bugs Pedro Alves
                   ` (2 preceding siblings ...)
  2016-01-19 17:52 ` [PATCH 1/3] Move foreach_with_prefix to lib/gdb.exp Pedro Alves
@ 2016-01-25 13:22 ` Pedro Alves
  3 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2016-01-25 13:22 UTC (permalink / raw)
  To: gdb-patches

On 01/19/2016 05:51 PM, Pedro Alves wrote:

> Pedro Alves (3):
>   Move foreach_with_prefix to lib/gdb.exp
>   Fix PR 19494: hang when killing unfollowed fork children
>   Fix PR 19461: strange "info thread" behavior in non-stop

I've pushed these in now.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-01-25 13:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-19 17:52 [PATCH 0/3] Fix fork bugs Pedro Alves
2016-01-19 17:52 ` [PATCH 2/3] Fix PR 19494: hang when killing unfollowed fork children Pedro Alves
2016-01-19 17:52 ` [PATCH 3/3] Fix PR 19461: strange "info thread" behavior in non-stop Pedro Alves
2016-01-19 17:52 ` [PATCH 1/3] Move foreach_with_prefix to lib/gdb.exp Pedro Alves
2016-01-25 13:22 ` [PATCH 0/3] Fix fork bugs Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).