[PATCH v4 1/3] Factor out "Detaching from program" message printing

[PATCH v4 1/3] Factor out "Detaching from program" message printing

[Pedro Alves]

Several targets have a copy of the same code that prints

 "Detaching from program ..."

in their target_detach implementation.  Factor that out to a common
function.

(For now, I left the couple targets that print this a bit differently
alone.  Maybe this could be further pulled out into infcmd.c.  If we
did that, and those targets want to continue printing differently,
this new function could be converted to a target method.)

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* darwin-nat.c (darwin_detach): Use target_announce_detach.
	* inf-ptrace.c (inf_ptrace_detach): Likewise.
	* nto-procfs.c (procfs_detach): Likewise.
	* remote.c (remote_detach_1): Likewise.
	* target.c (target_announce_detach): New function.
	* target.h (target_announce_detach): New declaration.
---
 gdb/darwin-nat.c | 10 +---------
 gdb/inf-ptrace.c | 10 +---------
 gdb/nto-procfs.c | 11 ++---------
 gdb/remote.c     | 10 +---------
 gdb/target.c     | 22 ++++++++++++++++++++++
 gdb/target.h     |  5 +++++
 6 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 54c430f..590c2ad 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1739,15 +1739,7 @@ darwin_detach (struct target_ops *ops, const char *args, int from_tty)
   int res;
 
   /* Display message.  */
-  if (from_tty)
-    {
-      char *exec_file = get_exec_file (0);
-      if (exec_file == 0)
-	exec_file = "";
-      printf_unfiltered (_("Detaching from program: %s, %s\n"), exec_file,
-			 target_pid_to_str (pid_to_ptid (pid)));
-      gdb_flush (gdb_stdout);
-    }
+  target_announce_detach (from_tty);
 
   /* If ptrace() is in use, stop the process.  */
   if (!inf->priv->no_ptrace)
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 329d8fb..dd11043 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -240,15 +240,7 @@ inf_ptrace_detach (struct target_ops *ops, const char *args, int from_tty)
   pid_t pid = ptid_get_pid (inferior_ptid);
   int sig = 0;
 
-  if (from_tty)
-    {
-      char *exec_file = get_exec_file (0);
-      if (exec_file == 0)
-	exec_file = "";
-      printf_unfiltered (_("Detaching from program: %s, %s\n"), exec_file,
-			 target_pid_to_str (pid_to_ptid (pid)));
-      gdb_flush (gdb_stdout);
-    }
+  target_announce_detach (from_tty);
   if (args)
     sig = atoi (args);
 
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index eb7dcfe..f49453d 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -962,15 +962,8 @@ procfs_detach (struct target_ops *ops, const char *args, int from_tty)
   int siggnal = 0;
   int pid;
 
-  if (from_tty)
-    {
-      char *exec_file = get_exec_file (0);
-      if (exec_file == 0)
-	exec_file = "";
-      printf_unfiltered ("Detaching from program: %s %s\n",
-			 exec_file, target_pid_to_str (inferior_ptid));
-      gdb_flush (gdb_stdout);
-    }
+  target_announce_detach ();
+
   if (args)
     siggnal = atoi (args);
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 1f0d67c..48d914e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5134,15 +5134,7 @@ remote_detach_1 (const char *args, int from_tty)
   if (!target_has_execution)
     error (_("No process to detach from."));
 
-  if (from_tty)
-    {
-      char *exec_file = get_exec_file (0);
-      if (exec_file == NULL)
-	exec_file = "";
-      printf_unfiltered (_("Detaching from program: %s, %s\n"), exec_file,
-			 target_pid_to_str (pid_to_ptid (pid)));
-      gdb_flush (gdb_stdout);
-    }
+  target_announce_detach (from_tty);
 
   /* Tell the remote target to detach.  */
   remote_detach_pid (pid);
diff --git a/gdb/target.c b/gdb/target.c
index c0ce46d..3aa9500 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3212,6 +3212,28 @@ find_target_at (enum strata stratum)
 }
 
 \f
+
+/* See target.h  */
+
+void
+target_announce_detach (int from_tty)
+{
+  pid_t pid;
+  char *exec_file;
+
+  if (!from_tty)
+    return;
+
+  exec_file = get_exec_file (0);
+  if (exec_file == NULL)
+    exec_file = "";
+
+  pid = ptid_get_pid (inferior_ptid);
+  printf_unfiltered (_("Detaching from program: %s, %s\n"), exec_file,
+		     target_pid_to_str (pid_to_ptid (pid)));
+  gdb_flush (gdb_stdout);
+}
+
 /* The inferior process has died.  Long live the inferior!  */
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index 6b5b6e0..fc317e3 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1302,6 +1302,11 @@ extern struct target_ops *find_run_target (void);
 #define target_post_attach(pid) \
      (*current_target.to_post_attach) (&current_target, pid)
 
+/* Display a message indicating we're about to detach from the current
+   inferior process.  */
+
+extern void target_announce_detach (int from_tty);
+
 /* Takes a program previously attached to and detaches it.
    The program may resume execution (some targets do, some don't) and will
    no longer stop on signals, etc.  We better not have left any breakpoints
-- 
2.5.5

[PATCH v4 0/3] Fix failure to detach if process exits while detaching on Linux

[Pedro Alves]

This series picks up where
 https://sourceware.org/ml/gdb-patches/2016-06/msg00068.html
left off.

New in v4:

 - Handles failure to remove watchpoints / (prepare to resume).

 - Reworked to reap zombie lwps on failure to detach.  Without this,
   gdb actually remains attached to the process.  This can result in
   the process's parent hanging, if it is waiting for the child to
   exit.
   
 - Tests much extended.

 - Commit log tweaked / extended.

Also pushed to users/palves/detach-gone-thread-v4, for convenience.

( Antoine, I rewrote the test substantially, and renamed it to
process-dies-while-detaching.exp to mirror
process-dies-while-handling-bp.exp, which is somewhat similar in
spirit.  You can easily see the differences from what we were working on
earlier with:
git diff -M origin/users/palves/detach-gone-thread-wip..origin/users/palves/detach-gone-thread-v4
)

 gdb/breakpoint.c                                   |  25 +-
 gdb/darwin-nat.c                                   |  10 +-
 gdb/gdbserver/linux-low.c                          | 116 ++++++--
 gdb/inf-ptrace.c                                   |  20 +-
 gdb/inf-ptrace.h                                   |   4 +
 gdb/infcmd.c                                       |   7 +
 gdb/linux-nat.c                                    | 152 ++++++----
 gdb/nto-procfs.c                                   |  11 +-
 gdb/remote.c                                       |  10 +-
 gdb/target.c                                       |  22 ++
 gdb/target.h                                       |   5 +
 gdb/testsuite/gdb.multi/watchpoint-multi-exit.c    |  66 +++++
 gdb/testsuite/gdb.multi/watchpoint-multi-exit.exp  |  87 ++++++
 .../gdb.threads/process-dies-while-detaching.c     | 116 ++++++++
 .../gdb.threads/process-dies-while-detaching.exp   | 327 +++++++++++++++++++++
 15 files changed, 864 insertions(+), 114 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/watchpoint-multi-exit.c
 create mode 100644 gdb/testsuite/gdb.multi/watchpoint-multi-exit.exp
 create mode 100644 gdb/testsuite/gdb.threads/process-dies-while-detaching.c
 create mode 100644 gdb/testsuite/gdb.threads/process-dies-while-detaching.exp

-- 
2.5.5

[PATCH v4 2/3] Forget watchpoint locations when inferior exits or is killed/detached

[Pedro Alves]

If you have two inferiors (or more), set watchpoints in one of the
inferiors, and then that inferior exits, until you manually delete the
watchpoint (or something forces a breakpoint re-set), you can't resume
the other inferior.

This is exercised by the test added by this commit.  Without the GDB
fix, this test fails like this:

 FAIL: gdb.multi/watchpoint-multi-exit.exp: dispose=kill: continue to marker in inferior 1
 FAIL: gdb.multi/watchpoint-multi-exit.exp: dispose=detach: continue to marker in inferior 1
 FAIL: gdb.multi/watchpoint-multi-exit.exp: dispose=exit: continue to marker in inferior 1

and gdb.log shows (in all three cases):

 (gdb) continue
 Continuing.
 Warning:
 Could not insert hardware watchpoint 2.
 Could not insert hardware breakpoints:
 You may have requested too many hardware breakpoints/watchpoints.

 Command aborted.
 (gdb) FAIL: gdb.multi/watchpoint-multi-exit.exp: dispose=kill: continue to marker in inferior 1

The problem is that GDB doesn't forget about the locations of
watchpoints set in the inferior that is now dead.  When we try to
continue the inferior that is still alive, we reach
insert_breakpoint_locations, which has the the loop that triggers the
error:

  /* If we failed to insert all locations of a watchpoint, remove
     them, as half-inserted watchpoint is of limited use.  */

That loop finds locations that are not marked inserted, but which
according to should_be_inserted should have been inserted, and so
errors out.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (breakpoint_init_inferior): Discard watchpoint
	locations.
	* infcmd.c (detach_command): Call breakpoint_init_inferior.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.multi/watchpoint-multi-exit.c: New file.
	* gdb.multi/watchpoint-multi-exit.exp: New file.
---
 gdb/breakpoint.c                                  | 25 ++++---
 gdb/infcmd.c                                      |  7 ++
 gdb/testsuite/gdb.multi/watchpoint-multi-exit.c   | 66 +++++++++++++++++
 gdb/testsuite/gdb.multi/watchpoint-multi-exit.exp | 87 +++++++++++++++++++++++
 4 files changed, 177 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/watchpoint-multi-exit.c
 create mode 100644 gdb/testsuite/gdb.multi/watchpoint-multi-exit.exp

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d2dafef..8c358b5 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4204,15 +4204,24 @@ breakpoint_init_inferior (enum inf_context context)
 	  /* Likewise for watchpoints on local expressions.  */
 	  if (w->exp_valid_block != NULL)
 	    delete_breakpoint (b);
-	  else if (context == inf_starting)
+	  else
 	    {
-	      /* Reset val field to force reread of starting value in
-		 insert_breakpoints.  */
-	      if (w->val)
-		value_free (w->val);
-	      w->val = NULL;
-	      w->val_valid = 0;
-	  }
+	      /* Get rid of existing locations, which are no longer
+		 valid.  New ones will be created in
+		 update_watchpoint, when the inferior is
+		 restarted.  */
+	      b->loc = NULL;
+
+	      if (context == inf_starting)
+		{
+		  /* Reset val field to force reread of starting value in
+		     insert_breakpoints.  */
+		  if (w->val)
+		    value_free (w->val);
+		  w->val = NULL;
+		  w->val_valid = 0;
+		}
+	    }
 	}
 	break;
       default:
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 1defb13..ad73bf9 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2988,6 +2988,13 @@ detach_command (char *args, int from_tty)
 
   target_detach (args, from_tty);
 
+  /* The current inferior process was just detached successfully.  Get
+     rid of breakpoints that no longer make sense.  Note we don't do
+     this within target_detach because that is also used when
+     following child forks, and in that case we will want to transfer
+     breakpoints to the child, not delete them.  */
+  breakpoint_init_inferior (inf_exited);
+
   /* If the solist is global across inferiors, don't clear it when we
      detach from a single inferior.  */
   if (!gdbarch_has_global_solist (target_gdbarch ()))
diff --git a/gdb/testsuite/gdb.multi/watchpoint-multi-exit.c b/gdb/testsuite/gdb.multi/watchpoint-multi-exit.c
new file mode 100644
index 0000000..fa672ba
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/watchpoint-multi-exit.c
@@ -0,0 +1,66 @@
+/* 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 <unistd.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+/* GDB sets watchpoint here.  */
+static volatile int globalvar;
+
+/* Whether it's expected that the child exits with a signal, vs
+   exiting normally.  GDB sets this.  */
+static volatile int expect_signaled;
+
+static void
+marker (void)
+{
+}
+
+static void
+child_function (void)
+{
+}
+
+int
+main (void)
+{
+  pid_t child;
+
+  child = fork ();
+  if (child == -1)
+    exit (1);
+  else if (child != 0)
+    {
+      int status, ret;
+
+      ret = waitpid (child, &status, 0);
+      if (ret == -1)
+	exit (2);
+      else if (expect_signaled && !WIFSIGNALED (status))
+	exit (3);
+      else if (!expect_signaled && !WIFEXITED (status))
+	exit (4);
+      else
+	marker ();
+    }
+  else
+    child_function ();
+
+  exit (0);
+}
diff --git a/gdb/testsuite/gdb.multi/watchpoint-multi-exit.exp b/gdb/testsuite/gdb.multi/watchpoint-multi-exit.exp
new file mode 100644
index 0000000..e811422
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/watchpoint-multi-exit.exp
@@ -0,0 +1,87 @@
+# 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/>.
+
+# Make sure that if an inferior exits or is detached/killed, its
+# watchpoints don't end up with stale locations, preventing resumption
+# of other inferiors.
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile {debug}]} {
+    return -1
+}
+
+# The test proper.  DISPOSE indicates how to dispose of the fork
+# child.  Can be either "kill", "detach", or "exit" (to continue it to
+# normal exit).
+proc do_test {dispose} {
+    global binfile
+
+    clean_restart $binfile
+
+    gdb_test_no_output "set follow-fork child"
+    gdb_test_no_output "set detach-on-fork off"
+
+    if ![runto "child_function"] {
+	fail "Can't run to child_function"
+	return
+    }
+
+    gdb_test "watch globalvar" "atchpoint \[0-9\]+: globalvar" \
+	"set watchpoint on inferior 2"
+
+    # Dispose of the inferior.  This should get rid of this inferior's
+    # watchpoint locations.
+    if {$dispose == "kill"} {
+	gdb_test "kill" "" "kill inferior 2" \
+	    "Kill the program being debugged.*y or n. $" "y"
+    } elseif {$dispose == "detach"} {
+	gdb_test "detach" ".*" "detach inferior 2"
+    } elseif {$dispose == "exit"} {
+	gdb_test "continue" ".*exited normally.*" "run to exit inferior 2"
+    } else {
+	perror "unhandled dispose: $dispose"
+    }
+
+    gdb_test "inferior 1" "witching to inferior 1 .*" \
+	"switch back to inferior 1"
+
+    if {$dispose == "kill"} {
+	gdb_test "print expect_signaled = 1" " = 1"
+    }
+
+    gdb_breakpoint "marker"
+
+    # Now continue inferior 1.  Before GDB was fixed, watchpoints for
+    # inferior 2 managed to be left behind.  When GDB realized that
+    # they hadn't been inserted, the continue command was aborted:
+    #
+    #  (gdb) continue
+    #  Continuing.
+    #  Warning:
+    #  Could not insert hardware watchpoint 2.
+    #  Could not insert hardware breakpoints:
+    #  You may have requested too many hardware breakpoints/watchpoints.
+    #
+    #  Command aborted.
+    #  (gdb)
+    #
+    gdb_test "continue" "Breakpoint \[0-9\]+, marker .*" \
+	"continue in inferior 1"
+}
+
+foreach_with_prefix dispose {"kill" "detach" "exit"} {
+    do_test $dispose
+}
-- 
2.5.5

[PATCH v4 3/3] Fix failure to detach if process exits while detaching on Linux

[Pedro Alves]

This commit fixes detaching on Linux when some thread exits the whole
thread group (process) just while we're detaching.

On Linux, a ptracer must detach from each LWP individually, with
PTRACE_DETACH.  Since PTRACE_DETACH sets the thread running free, if
one of the already-detached threads causes the whole thread group to
exit (e.g., simply calls exit), the kernel force-kills the other
threads in the group, making them zombie, just as we're still
detaching them.  Since PTRACE_DETACH against a zombie thread fails
with ESRCH, and gdb/gdbserver are not expecting this, the detach fails
with an error like: "Can't detach process: No such process.".

This patch detects this detach failure as normal, and instead of
erroring out, reaps the now-dead thread.

New test included, that exercises several different scenarios that
cause GDB/GDBserver to error out when it should not.

Tested on x86-64 GNU/Linux with {unix, native-gdbserver,
native-extended-gdbserver}

Note: without the previous fix, the "single-process + continue"
variant of the new test would fail with:

 (gdb) PASS: gdb.threads/process-dies-while-detaching.exp: single-process: continue: watchpoint: switch to parent
 continue
 Continuing.
 Warning:
 Could not insert hardware watchpoint 3.
 Could not insert hardware breakpoints:
 You may have requested too many hardware breakpoints/watchpoints.

 Command aborted.
 (gdb) FAIL: gdb.threads/process-dies-while-detaching.exp: single-process: continue: watchpoint: continue

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
	    Antoine Tremblay  <antoine.tremblay@ericsson.com>

	* linux-low.c: Change interface to take the target lwp_info
	pointer directly and return void.  Handle detaching from a zombie
	thread.
	(linux_detach_lwp_callback): New function.
	(linux_detach): Detach from the leader thread after detaching from
	the clone threads.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
	    Antoine Tremblay  <antoine.tremblay@ericsson.com>

	* inf-ptrace.c (inf_ptrace_detach_success): New function, factored
	out from ...
	(inf_ptrace_detach): ... here.
	* inf-ptrace.h (inf_ptrace_detach_success): New declaration.
	* linux-nat.c (get_pending_status): Rename to ...
	(get_detach_signal): ... this, and return a host signal instead of
	filling in a wait status.
	(detach_one_lwp): New function, factored out from detach_callback
	and adjusted to handle detaching from a zombie thread.
	(detach_callback): Skip the leader thread.
	(linux_nat_detach): No longer defer to inf_ptrace_detach to detach
	the leader thread, nor build a signal string to pass down.
	Instead, use target_announce_detach, detach_one_lwp and
	inf_ptrace_detach_success.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
	    Antoine Tremblay  <antoine.tremblay@ericsson.com>

	* gdb.threads/process-dies-while-detaching.c: New file.
	* gdb.threads/process-dies-while-detaching.exp: New file.
---
 gdb/gdbserver/linux-low.c                          | 116 ++++++--
 gdb/inf-ptrace.c                                   |  10 +
 gdb/inf-ptrace.h                                   |   4 +
 gdb/linux-nat.c                                    | 152 ++++++----
 .../gdb.threads/process-dies-while-detaching.c     | 116 ++++++++
 .../gdb.threads/process-dies-while-detaching.exp   | 327 +++++++++++++++++++++
 6 files changed, 655 insertions(+), 70 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/process-dies-while-detaching.c
 create mode 100644 gdb/testsuite/gdb.threads/process-dies-while-detaching.exp

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 81134b0..7ce37d9 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -266,6 +266,7 @@ static int kill_lwp (unsigned long lwpid, int signo);
 static void enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info);
 static void complete_ongoing_step_over (void);
 static int linux_low_ptrace_options (int attached);
+static int check_ptrace_stopped_lwp_gone (struct lwp_info *lp);
 
 /* When the event-loop is doing a step-over, this points at the thread
    being stepped.  */
@@ -1447,16 +1448,14 @@ get_detach_signal (struct thread_info *thread)
     }
 }
 
-static int
-linux_detach_one_lwp (struct inferior_list_entry *entry, void *args)
+/* Detach from LWP.  */
+
+static void
+linux_detach_one_lwp (struct lwp_info *lwp)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
-  struct lwp_info *lwp = get_thread_lwp (thread);
-  int pid = * (int *) args;
+  struct thread_info *thread = get_lwp_thread (lwp);
   int sig;
-
-  if (ptid_get_pid (entry->id) != pid)
-    return 0;
+  int lwpid;
 
   /* If there is a pending SIGSTOP, get rid of it.  */
   if (lwp->stop_expected)
@@ -1469,22 +1468,94 @@ linux_detach_one_lwp (struct inferior_list_entry *entry, void *args)
       lwp->stop_expected = 0;
     }
 
-  /* Flush any pending changes to the process's registers.  */
-  regcache_invalidate_thread (thread);
-
   /* Pass on any pending signal for this thread.  */
   sig = get_detach_signal (thread);
 
-  /* Finally, let it resume.  */
-  if (the_low_target.prepare_to_resume != NULL)
-    the_low_target.prepare_to_resume (lwp);
-  if (ptrace (PTRACE_DETACH, lwpid_of (thread), (PTRACE_TYPE_ARG3) 0,
+  /* Preparing to resume may try to write registers, and fail if the
+     lwp is zombie.  If that happens, ignore the error.  We'll handle
+     it below, when detach fails with ESRCH.  */
+  TRY
+    {
+      /* Flush any pending changes to the process's registers.  */
+      regcache_invalidate_thread (thread);
+
+      /* Finally, let it resume.  */
+      if (the_low_target.prepare_to_resume != NULL)
+	the_low_target.prepare_to_resume (lwp);
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (!check_ptrace_stopped_lwp_gone (lwp))
+	throw_exception (ex);
+    }
+  END_CATCH
+
+  lwpid = lwpid_of (thread);
+  if (ptrace (PTRACE_DETACH, lwpid, (PTRACE_TYPE_ARG3) 0,
 	      (PTRACE_TYPE_ARG4) (long) sig) < 0)
-    error (_("Can't detach %s: %s"),
-	   target_pid_to_str (ptid_of (thread)),
-	   strerror (errno));
+    {
+      int save_errno = errno;
+
+      /* We know the thread exists, so ESRCH must mean the lwp is
+	 zombie.  This can happen if one of the already-detached
+	 threads exits the whole thread group.  In that case we're
+	 still attached, and must reap the lwp.  */
+      if (save_errno == ESRCH)
+	{
+	  int ret, status;
+
+	  ret = my_waitpid (lwpid, &status, __WALL);
+	  if (ret == -1)
+	    {
+	      warning (_("Couldn't reap LWP %d while detaching: %s"),
+		       lwpid, strerror (errno));
+	    }
+	  else if (!WIFEXITED (status) && !WIFSIGNALED (status))
+	    {
+	      warning (_("Reaping LWP %d while detaching "
+			 "returned unexpected status 0x%x"),
+		       lwpid, status);
+	    }
+	}
+      else
+	{
+	  error (_("Can't detach %s: %s"),
+		 target_pid_to_str (ptid_of (thread)),
+		 strerror (save_errno));
+	}
+    }
+  else if (debug_threads)
+    {
+      debug_printf ("PTRACE_DETACH (%s, %s, 0) (OK)\n",
+		    target_pid_to_str (ptid_of (thread)),
+		    strsignal (sig));
+    }
 
   delete_lwp (lwp);
+}
+
+/* Callback for find_inferior.  Detaches from non-leader threads of a
+   given process.  */
+
+static int
+linux_detach_lwp_callback (struct inferior_list_entry *entry, void *args)
+{
+  struct thread_info *thread = (struct thread_info *) entry;
+  struct lwp_info *lwp = get_thread_lwp (thread);
+  int pid = *(int *) args;
+  int lwpid = lwpid_of (thread);
+
+  /* Skip other processes.  */
+  if (ptid_get_pid (entry->id) != pid)
+    return 0;
+
+  /* We don't actually detach from the thread group leader just yet.
+     If the thread group exits, we must reap the zombie clone lwps
+     before we're able to reap the leader.  */
+  if (ptid_get_pid (entry->id) == lwpid)
+    return 0;
+
+  linux_detach_one_lwp (lwp);
   return 0;
 }
 
@@ -1492,6 +1563,7 @@ static int
 linux_detach (int pid)
 {
   struct process_info *process;
+  struct lwp_info *main_lwp;
 
   process = find_process_pid (pid);
   if (process == NULL)
@@ -1515,7 +1587,13 @@ linux_detach (int pid)
   /* Stabilize threads (move out of jump pads).  */
   stabilize_threads ();
 
-  find_inferior (&all_threads, linux_detach_one_lwp, &pid);
+  /* Detach from the clone lwps first.  If the thread group exits just
+     while we're detaching, we must reap the clone lwps before we're
+     able to reap the leader.  */
+  find_inferior (&all_threads, linux_detach_lwp_callback, &pid);
+
+  main_lwp = find_lwp_pid (pid_to_ptid (pid));
+  linux_detach_one_lwp (main_lwp);
 
   the_target->mourn (process);
 
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index dd11043..0896cff 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -257,6 +257,16 @@ inf_ptrace_detach (struct target_ops *ops, const char *args, int from_tty)
   error (_("This system does not support detaching from a process"));
 #endif
 
+  inf_ptrace_detach_success (ops);
+}
+
+/* See inf-ptrace.h.  */
+
+void
+inf_ptrace_detach_success (struct target_ops *ops)
+{
+  pid_t pid = ptid_get_pid (inferior_ptid);
+
   inferior_ptid = null_ptid;
   detach_inferior (pid);
 
diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h
index 0a26720..f1fc111 100644
--- a/gdb/inf-ptrace.h
+++ b/gdb/inf-ptrace.h
@@ -38,4 +38,8 @@ extern struct target_ops *
 
 extern pid_t get_ptrace_pid (ptid_t);
 
+
+/* Cleanup the inferior after a successful ptrace detach.  */
+extern void inf_ptrace_detach_success (struct target_ops *ops);
+
 #endif
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index e6d525f..7ab0eee 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -820,6 +820,7 @@ linux_nat_pass_signals (struct target_ops *self,
 static int stop_wait_callback (struct lwp_info *lp, void *data);
 static char *linux_child_pid_to_exec_file (struct target_ops *self, int pid);
 static int resume_stopped_resumed_lwps (struct lwp_info *lp, void *data);
+static int check_ptrace_stopped_lwp_gone (struct lwp_info *lp);
 
 \f
 
@@ -1295,9 +1296,13 @@ linux_nat_attach (struct target_ops *ops, const char *args, int from_tty)
     target_async (1);
 }
 
-/* Get pending status of LP.  */
+/* Get pending signal of THREAD as a host signal number, for detaching
+   purposes.  This is the signal the thread last stopped for, which we
+   need to deliver to the thread when detaching, otherwise, it'd be
+   suppressed/lost.  */
+
 static int
-get_pending_status (struct lwp_info *lp, int *status)
+get_detach_signal (struct lwp_info *lp)
 {
   enum gdb_signal signo = GDB_SIGNAL_0;
 
@@ -1350,8 +1355,6 @@ get_pending_status (struct lwp_info *lp, int *status)
 	}
     }
 
-  *status = 0;
-
   if (signo == GDB_SIGNAL_0)
     {
       if (debug_linux_nat)
@@ -1370,21 +1373,28 @@ get_pending_status (struct lwp_info *lp, int *status)
     }
   else
     {
-      *status = W_STOPCODE (gdb_signal_to_host (signo));
-
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,
 			    "GPT: lwp %s has pending signal %s\n",
 			    target_pid_to_str (lp->ptid),
 			    gdb_signal_to_string (signo));
+
+      return gdb_signal_to_host (signo);
     }
 
   return 0;
 }
 
-static int
-detach_callback (struct lwp_info *lp, void *data)
+/* Detach from LP.  If SIGNO_P is non-NULL, then it points to the
+   signal number that should be passed to the LWP when detaching.
+   Otherwise pass any pending signal the LWP may have, if any.  */
+
+static void
+detach_one_lwp (struct lwp_info *lp, int *signo_p)
 {
+  int lwpid = ptid_get_lwp (lp->ptid);
+  int signo;
+
   gdb_assert (lp->status == 0 || WIFSTOPPED (lp->status));
 
   if (debug_linux_nat && lp->status)
@@ -1400,36 +1410,83 @@ detach_callback (struct lwp_info *lp, void *data)
 			    "DC: Sending SIGCONT to %s\n",
 			    target_pid_to_str (lp->ptid));
 
-      kill_lwp (ptid_get_lwp (lp->ptid), SIGCONT);
+      kill_lwp (lwpid, SIGCONT);
       lp->signalled = 0;
     }
 
-  /* We don't actually detach from the LWP that has an id equal to the
-     overall process id just yet.  */
-  if (ptid_get_lwp (lp->ptid) != ptid_get_pid (lp->ptid))
+  if (signo_p == NULL)
     {
-      int status = 0;
-
       /* Pass on any pending signal for this LWP.  */
-      get_pending_status (lp, &status);
+      signo = get_detach_signal (lp);
+    }
+  else
+    signo = *signo_p;
 
+  /* Preparing to resume may try to write registers, and fail if the
+     lwp is zombie.  If that happens, ignore the error.  We'll handle
+     it below, when detach fails with ESRCH.  */
+  TRY
+    {
       if (linux_nat_prepare_to_resume != NULL)
 	linux_nat_prepare_to_resume (lp);
-      errno = 0;
-      if (ptrace (PTRACE_DETACH, ptid_get_lwp (lp->ptid), 0,
-		  WSTOPSIG (status)) < 0)
-	error (_("Can't detach %s: %s"), target_pid_to_str (lp->ptid),
-	       safe_strerror (errno));
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (!check_ptrace_stopped_lwp_gone (lp))
+	throw_exception (ex);
+    }
+  END_CATCH
 
-      if (debug_linux_nat)
-	fprintf_unfiltered (gdb_stdlog,
-			    "PTRACE_DETACH (%s, %s, 0) (OK)\n",
-			    target_pid_to_str (lp->ptid),
-			    strsignal (WSTOPSIG (status)));
+  if (ptrace (PTRACE_DETACH, lwpid, 0, signo) < 0)
+    {
+      int save_errno = errno;
+
+      /* We know the thread exists, so ESRCH must mean the lwp is
+	 zombie.  This can happen if one of the already-detached
+	 threads exits the whole thread group.  In that case we're
+	 still attached, and must reap the lwp.  */
+      if (save_errno == ESRCH)
+	{
+	  int ret, status;
 
-      delete_lwp (lp->ptid);
+	  ret = my_waitpid (lwpid, &status, __WALL);
+	  if (ret == -1)
+	    {
+	      warning (_("Couldn't reap LWP %d while detaching: %s"),
+		       lwpid, strerror (errno));
+	    }
+	  else if (!WIFEXITED (status) && !WIFSIGNALED (status))
+	    {
+	      warning (_("Reaping LWP %d while detaching "
+			 "returned unexpected status 0x%x"),
+		       lwpid, status);
+	    }
+	}
+      else
+	{
+	  error (_("Can't detach %s: %s"), target_pid_to_str (lp->ptid),
+		 safe_strerror (save_errno));
+	}
+    }
+  else if (debug_linux_nat)
+    {
+      fprintf_unfiltered (gdb_stdlog,
+			  "PTRACE_DETACH (%s, %s, 0) (OK)\n",
+			  target_pid_to_str (lp->ptid),
+			  strsignal (signo));
     }
 
+  delete_lwp (lp->ptid);
+}
+
+static int
+detach_callback (struct lwp_info *lp, void *data)
+{
+  /* We don't actually detach from the thread group leader just yet.
+     If the thread group exits, we must reap the zombie clone lwps
+     before we're able to reap the leader.  */
+  if (ptid_get_lwp (lp->ptid) != ptid_get_pid (lp->ptid))
+    detach_one_lwp (lp, NULL);
   return 0;
 }
 
@@ -1437,7 +1494,6 @@ static void
 linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
 {
   int pid;
-  int status;
   struct lwp_info *main_lwp;
 
   pid = ptid_get_pid (inferior_ptid);
@@ -1459,29 +1515,6 @@ linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
 
   main_lwp = find_lwp_pid (pid_to_ptid (pid));
 
-  /* Pass on any pending signal for the last LWP.  */
-  if ((args == NULL || *args == '\0')
-      && get_pending_status (main_lwp, &status) != -1
-      && WIFSTOPPED (status))
-    {
-      char *tem;
-
-      /* Put the signal number in ARGS so that inf_ptrace_detach will
-	 pass it along with PTRACE_DETACH.  */
-      tem = (char *) alloca (8);
-      xsnprintf (tem, 8, "%d", (int) WSTOPSIG (status));
-      args = tem;
-      if (debug_linux_nat)
-	fprintf_unfiltered (gdb_stdlog,
-			    "LND: Sending signal %s to %s\n",
-			    args,
-			    target_pid_to_str (main_lwp->ptid));
-    }
-
-  if (linux_nat_prepare_to_resume != NULL)
-    linux_nat_prepare_to_resume (main_lwp);
-  delete_lwp (main_lwp->ptid);
-
   if (forks_exist_p ())
     {
       /* Multi-fork case.  The current inferior_ptid is being detached
@@ -1491,7 +1524,24 @@ linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
       linux_fork_detach (args, from_tty);
     }
   else
-    linux_ops->to_detach (ops, args, from_tty);
+    {
+      int signo;
+
+      target_announce_detach (from_tty);
+
+      /* Pass on any pending signal for the last LWP, unless the user
+	 requested detaching with a different signal (most likely 0,
+	 meaning, discard the signal).  */
+      if (args != NULL)
+	signo = atoi (args);
+      else
+	signo = get_detach_signal (main_lwp);
+
+      detach_one_lwp (main_lwp, &signo);
+
+      inf_ptrace_detach_success (ops);
+    }
+  delete_lwp (main_lwp->ptid);
 }
 
 /* Resume execution of the inferior process.  If STEP is nonzero,
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.c b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
new file mode 100644
index 0000000..a28e804
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
@@ -0,0 +1,116 @@
+/* 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 <pthread.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <assert.h>
+
+/* This barrier ensures we only reach the initial breakpoint after all
+   threads have started.  */
+pthread_barrier_t start_threads_barrier;
+
+/* Many threads in order to be fairly sure the process exits while GDB
+   is detaching from each thread in the process, on targets that need
+   to detach from each thread individually.  */
+#define NTHREADS 256
+
+/* GDB sets a watchpoint here.  */
+int globalvar = 1;
+
+/* GDB reads this.  */
+int mypid;
+
+/* Threads' entry point.  */
+
+void *
+thread_function (void *arg)
+{
+  pthread_barrier_wait (&start_threads_barrier);
+  _exit (0);
+}
+
+/* The fork child's entry point.  */
+
+void
+child_function (void)
+{
+  pthread_t threads[NTHREADS];
+  int i;
+
+  pthread_barrier_init (&start_threads_barrier, NULL, NTHREADS + 1);
+
+  for (i = 0; i < NTHREADS; i++)
+    pthread_create (&threads[i], NULL, thread_function, NULL);
+  pthread_barrier_wait (&start_threads_barrier);
+
+  exit (0);
+}
+
+/* This is defined by the .exp file if testing the multi-process
+   variant.  */
+#ifdef MULTIPROCESS
+
+/* The fork parent's entry point.  */
+
+void
+parent_function (pid_t child)
+{
+  int status, ret;
+
+  alarm (300);
+
+  ret = waitpid (child, &status, 0);
+  if (ret == -1)
+    exit (1);
+  else if (!WIFEXITED (status))
+    exit (2);
+  else
+    {
+      printf ("exited, status=%d\n", WEXITSTATUS (status));
+      exit (0);
+    }
+}
+
+#endif
+
+int
+main (void)
+{
+#ifdef MULTIPROCESS
+  pid_t child;
+
+  child = fork ();
+  if (child == -1)
+    return 1;
+#endif
+
+  mypid = getpid ();
+
+#ifdef MULTIPROCESS
+  if (child != 0)
+    parent_function (child);
+  else
+#endif
+    child_function ();
+
+  /* Not reached.  */
+  abort ();
+}
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
new file mode 100644
index 0000000..1ab3820
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
@@ -0,0 +1,327 @@
+# 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/>.
+
+# This test spawns a few threads that immediately exit the whole
+# process.  On targets where the debugger needs to detach from each
+# thread individually (such as on the Linux kernel), the debugger must
+# handle the case of the process exiting while the detach is ongoing.
+#
+# Similarly, the process can also be killed from outside the debugger
+# (e.g., with SIGKILL), _before_ the user requests a detach.  The
+# debugger must likewise detach gracefully.
+#
+# The testcase actually builds two variants of the test program:
+# single-process, and multi-process.  In the multi-process variant,
+# the test program forks, and it's the fork child that spawns threads
+# that exit just while the process is being detached from.  The fork
+# parent waits for its child to exit, so if GDB fails to detach from
+# the child correctly, the parent hangs.  Because continuing the
+# parent can mask failure to detach from the child correctly (e.g.,
+# due to waitpid(-1,...) calls deep in the target layers managing to
+# reap the child), we try immediately detaching from the parent too,
+# and observing whether the parent exits via standard output.
+#
+# Normally, if testing with "target remote" against gdbserver, then
+# after detaching from all attached processes, gdbserver exits.
+# However, when gdbserver detaches from a process that is its own
+# direct child, gdbserver does not exit immediately.  Instead it
+# "joins" (waits for) the child, only exiting when the child itself
+# exits too.  Thus, on Linux, if gdbserver fails to detach from the
+# zombie child's threads correctly (or rather, reap them), it'll hang,
+# because the leader thread will only return an exit status after all
+# threads are reaped.  We test that as well.
+
+standard_testfile
+
+# Test that GDBserver exits.
+
+proc test_server_exit {} {
+    global server_spawn_id
+
+    set test "server exits"
+    gdb_expect {
+	-i $server_spawn_id
+	eof {
+	    pass $test
+	    wait -i $server_spawn_id
+	    unset server_spawn_id
+	}
+	timeout {
+	    fail "$test (timeout)"
+	}
+    }
+}
+
+# If RESULT is not zero, make the caller return.
+
+proc return_if_fail { result } {
+    if {$result != 0} {
+	return -code return
+    }
+}
+
+# Detach from a process, and ensure that it exits after detaching.
+# This relies on inferior I/O.
+
+proc detach_and_expect_exit {test} {
+    global decimal
+    global gdb_spawn_id
+    global inferior_spawn_id
+    global gdb_prompt
+
+    return_if_fail [gdb_test_multiple "detach" $test {
+	-re "Detaching from .*, process $decimal" {
+	}
+    }]
+
+    set saw_prompt 0
+    set saw_inf_exit 0
+    while { !$saw_prompt && ! $saw_inf_exit } {
+	# We don't know what order the interesting things will arrive in.
+	# Using a pattern of the form 'x|y|z' instead of -re x ... -re y
+	# ... -re z ensures that expect always chooses the match that
+	# occurs leftmost in the input, and not the pattern appearing
+	# first in the script that occurs anywhere in the input, so that
+	# we don't skip anything.
+	return_if_fail [gdb_test_multiple "" $test {
+	    -i "$inferior_spawn_id $gdb_spawn_id"
+	    -re "(exited, status=0)|($gdb_prompt )" {
+		if {[info exists expect_out(1,string)]} {
+		    verbose -log "saw inferior exit"
+		    set saw_inf_exit 1
+		} elseif {[info exists expect_out(2,string)]} {
+		    verbose -log "saw prompt"
+		    set saw_prompt 1
+		}
+		array unset expect_out
+	    }
+	}]
+    }
+
+    pass $test
+}
+
+# Run to _exit in the child.
+
+proc continue_to_exit_bp {} {
+    gdb_breakpoint "_exit" temporary
+    gdb_continue_to_breakpoint "_exit" ".*_exit.*"
+}
+
+# If testing single-process, simply detach from the process.
+#
+# If testing multi-process, first detach from the child, then detach
+# from the parent and confirm that the parent exits, thus unsuring
+# we've detached from the child successfully, as the parent hangs in
+# its waitpid call otherwise.
+#
+# If connected with "target remote", make sure gdbserver exits.
+#
+# CMD indicates what to do with the parent after detaching the child.
+# Can be either "detach" to detach, or "continue", to continue to
+# exit.  If "continue", then CONTINUE_RE is the regexp to expect.
+# Defaults to normal exit output.
+#
+proc do_detach {multi_process cmd {continue_re ""}} {
+    global decimal
+    global server_spawn_id
+
+    if {$continue_re == ""} {
+	set continue_re "exited normally.*"
+    }
+
+    set is_remote [expr {[target_info exists gdb_protocol]
+			 && [target_info gdb_protocol] == "remote"}]
+
+    if {$multi_process} {
+	gdb_test "detach" "Detaching from .*, process $decimal" \
+	    "detach child"
+
+	gdb_test "inferior 1" "\[Switching to inferior $decimal\].*" \
+	    "switch to parent"
+
+	if {$cmd == "detach"} {
+	    # Make sure that detach works and that the parent process
+	    # exits cleanly.
+	    detach_and_expect_exit "detach parent"
+	} elseif {$cmd == "continue"} {
+	    # Make sure that continuing works and that the parent process
+	    # exits cleanly.
+	    gdb_test "continue" $continue_re
+	} else {
+	    perror "unhandled command: $mode: $cmd"
+	}
+    } else {
+	if $is_remote {
+	    set extra "\r\nEnding remote debugging\."
+	} else {
+	    set extra ""
+	}
+	if {$cmd == "detach"} {
+	    gdb_test "detach" "Detaching from .*, process $decimal$extra"
+	} elseif {$cmd == "continue"} {
+	    gdb_test "continue" $continue_re
+	} else {
+	    perror "unhandled command: $mode: $cmd"
+	}
+    }
+
+    # When connected in "target remote" mode, the server should exit
+    # when there are no processes left to debug.
+    if { $is_remote && [info exists server_spawn_id]} {
+	test_server_exit
+    }
+}
+
+# Test detaching from a process that dies just while GDB is detaching.
+
+proc test_detach {multi_process cmd} {
+    with_test_prefix "detach" {
+	global binfile
+
+	clean_restart ${binfile}
+
+	if ![runto_main] {
+	    fail "Can't run to main"
+	    return -1
+	}
+
+	if {$multi_process} {
+	    gdb_test_no_output "set detach-on-fork off"
+	    gdb_test_no_output "set follow-fork-mode child"
+	}
+
+	# Run to _exit in the child.
+	continue_to_exit_bp
+
+	do_detach $multi_process $cmd
+    }
+}
+
+# Same as test_detach, except set a watchpoint before detaching.
+
+proc test_detach_watch {multi_process cmd} {
+    with_test_prefix "watchpoint" {
+	global binfile decimal
+
+	clean_restart ${binfile}
+
+	if ![runto_main] {
+	    fail "Can't run to main"
+	    return -1
+	}
+
+	if {$multi_process} {
+	    gdb_test_no_output "set detach-on-fork off"
+	    gdb_test_no_output "set follow-fork-mode child"
+
+	    gdb_breakpoint "child_function" temporary
+	    gdb_continue_to_breakpoint "child_function" ".*"
+	}
+
+	# Set a watchpoint in the child.
+	gdb_test "watch globalvar" ".* watchpoint $decimal: globalvar"
+
+	# Continue to the _exit breakpoint.  This arms the watchpoint
+	# registers in all threads.  Detaching will thus need to clear
+	# them out, and handle the case of the thread disappearing
+	# while doing that (on targets that need to detach from each
+	# thread individually).
+	continue_to_exit_bp
+
+	do_detach $multi_process $cmd
+    }
+}
+
+# Test detaching from a process that dies _before_ GDB starts
+# detaching.
+
+proc test_detach_killed_outside {multi_process cmd} {
+    with_test_prefix "killed outside" {
+	global binfile
+
+	clean_restart ${binfile}
+
+	if ![runto_main] {
+	    fail "Can't run to main"
+	    return -1
+	}
+
+	gdb_test_no_output "set breakpoint always-inserted on"
+
+	if {$multi_process} {
+	    gdb_test_no_output "set detach-on-fork off"
+	    gdb_test_no_output "set follow-fork-mode child"
+	}
+
+	# Run to _exit in the child.
+	continue_to_exit_bp
+
+	set childpid [get_integer_valueof "mypid" -1]
+	if { $childpid == -1 } {
+	    untested "failed to extract child pid"
+	    return -1
+	}
+
+	remote_exec target "kill -9 ${childpid}"
+
+	# Give it some time to die.
+	sleep 2
+
+	if {$multi_process} {
+	    set continue_re "exited with code 02.*"
+	} else {
+	    set continue_re "terminated with signal SIGKILL.*"
+	}
+	do_detach $multi_process $cmd $continue_re
+    }
+}
+
+# The test proper.  MULTI_PROCESS is true if testing the multi-process
+# variant.
+
+proc do_test {multi_process cmd} {
+    global testfile srcfile binfile
+
+    if {$multi_process && $cmd == "detach"
+	&& [target_info exists gdb,noinferiorio]} {
+	# This requires inferior I/O to tell whether both the parent
+	# and child exit successfully.
+	return
+    }
+
+    set binfile [standard_output_file ${testfile}-$multi_process-$cmd]
+    set options {debug pthreads}
+    if {$multi_process} {
+	lappend options "additional_flags=-DMULTIPROCESS"
+    }
+
+    if {[build_executable "failed to build" \
+	     $testfile-$multi_process-$cmd $srcfile $options] == -1} {
+	return -1
+    }
+
+    test_detach $multi_process $cmd
+    test_detach_watch $multi_process $cmd
+    test_detach_killed_outside $multi_process $cmd
+}
+
+foreach multi_process {0 1} {
+    set mode [expr {$multi_process ? "single-process" : "multi-process"}]
+    foreach cmd {"detach" "continue"} {
+	with_test_prefix "$mode: $cmd" {
+	    do_test $multi_process $cmd
+	}
+    }
+}
-- 
2.5.5

Re: [PATCH v4 1/3] Factor out "Detaching from program" message printing

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Several targets have a copy of the same code that prints
>
>  "Detaching from program ..."
>
> in their target_detach implementation.  Factor that out to a common
> function.
>
> (For now, I left the couple targets that print this a bit differently
> alone.  Maybe this could be further pulled out into infcmd.c.  If we
> did that, and those targets want to continue printing differently,
> this new function could be converted to a target method.)
>
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
> 	* darwin-nat.c (darwin_detach): Use target_announce_detach.
> 	* inf-ptrace.c (inf_ptrace_detach): Likewise.
> 	* nto-procfs.c (procfs_detach): Likewise.
> 	* remote.c (remote_detach_1): Likewise.
> 	* target.c (target_announce_detach): New function.
> 	* target.h (target_announce_detach): New declaration.

Patch is good to me.

-- 
Yao (齐尧)

Re: [PATCH v4 2/3] Forget watchpoint locations when inferior exits or is killed/detached

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> +	      /* Get rid of existing locations, which are no longer
> +		 valid.  New ones will be created in
> +		 update_watchpoint, when the inferior is
> +		 restarted.  */
> +	      b->loc = NULL;

Use decref_bp_location or is it intended?

-- 
Yao (齐尧)

Re: [PATCH v4 3/3] Fix failure to detach if process exits while detaching on Linux

[Antoine Tremblay]

Pedro Alves writes:

> +# If testing single-process, simply detach from the process.
> +#
> +# If testing multi-process, first detach from the child, then detach
> +# from the parent and confirm that the parent exits, thus unsuring

Typo: ensuring...

Otherwise, wow really nice work and quite well commented, I learned a
lot about the testsuite reading that.

Thanks!

Antoine

Re: [PATCH v4 3/3] Fix failure to detach if process exits while detaching on Linux

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> +# If testing single-process, simply detach from the process.
> +#
> +# If testing multi-process, first detach from the child, then detach
> +# from the parent and confirm that the parent exits, thus unsuring

s/unsuring/ensuring/ ?

> +# we've detached from the child successfully, as the parent hangs in
> +# its waitpid call otherwise.
> +#
> +# If connected with "target remote", make sure gdbserver exits.
> +#
> +# CMD indicates what to do with the parent after detaching the child.
> +# Can be either "detach" to detach, or "continue", to continue to
> +# exit.  If "continue", then CONTINUE_RE is the regexp to expect.
> +# Defaults to normal exit output.
> +#
> +proc do_detach {multi_process cmd {continue_re ""}} {
> +    global decimal
> +    global server_spawn_id
> +
> +    if {$continue_re == ""} {
> +	set continue_re "exited normally.*"
> +    }
> +
> +    set is_remote [expr {[target_info exists gdb_protocol]
> +			 && [target_info gdb_protocol] == "remote"}]
> +
> +    if {$multi_process} {
> +	gdb_test "detach" "Detaching from .*, process $decimal" \
> +	    "detach child"
> +
> +	gdb_test "inferior 1" "\[Switching to inferior $decimal\].*" \
> +	    "switch to parent"
> +
> +	if {$cmd == "detach"} {
> +	    # Make sure that detach works and that the parent process
> +	    # exits cleanly.
> +	    detach_and_expect_exit "detach parent"
> +	} elseif {$cmd == "continue"} {
> +	    # Make sure that continuing works and that the parent process
> +	    # exits cleanly.
> +	    gdb_test "continue" $continue_re
> +	} else {
> +	    perror "unhandled command: $mode: $cmd"

no variable "mode".

> +	}
> +    } else {
> +	if $is_remote {
> +	    set extra "\r\nEnding remote debugging\."
> +	} else {
> +	    set extra ""
> +	}
> +	if {$cmd == "detach"} {
> +	    gdb_test "detach" "Detaching from .*, process $decimal$extra"
> +	} elseif {$cmd == "continue"} {
> +	    gdb_test "continue" $continue_re
> +	} else {
> +	    perror "unhandled command: $mode: $cmd"
> +	}
> +    }
> +

-- 
Yao (齐尧)

Re: [PATCH v4 2/3] Forget watchpoint locations when inferior exits or is killed/detached

[Pedro Alves]

On 06/17/2016 12:55 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> +	      /* Get rid of existing locations, which are no longer
>> +		 valid.  New ones will be created in
>> +		 update_watchpoint, when the inferior is
>> +		 restarted.  */
>> +	      b->loc = NULL;
> 
> Use decref_bp_location or is it intended?
> 

It's intended.  The b->loc chain doesn't count for incr/decref.
It really holds weak references.  The strong references are either
in  the global location chain or the bpstat chains.  So the next time
update_global_location_list is called, locations that are found
to not be referenced by any breakpoint are garbage collected then,
via decref_bp_location.

See how update_breakpoint_locations simply hoists old locations
out of the breakpoint, but doesn't decref them.  [Used to clearer
before c2f4122d5cc2 ("Limit breakpoint re-set to the current
program space") though].

Thanks,
Pedro Alves

Re: [PATCH v4 3/3] Fix failure to detach if process exits while detaching on Linux

[Pedro Alves]

On 06/17/2016 04:07 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> +# If testing single-process, simply detach from the process.
>> +#
>> +# If testing multi-process, first detach from the child, then detach
>> +# from the parent and confirm that the parent exits, thus unsuring
> 
> s/unsuring/ensuring/ ?

Indeed.

>> +	} else {
>> +	    perror "unhandled command: $mode: $cmd"
> 
> no variable "mode".

Whoops, there used to be one.  Fixed now (the two instances):

-           perror "unhandled command: $mode: $cmd"
+           perror "unhandled command: $cmd"

Thanks,
Pedro Alves

Re: [PATCH v4 3/3] Fix failure to detach if process exits while detaching on Linux

[Pedro Alves]

On 06/17/2016 02:03 PM, Antoine Tremblay wrote:
> 
> Pedro Alves writes:
> 
>> +# If testing single-process, simply detach from the process.
>> +#
>> +# If testing multi-process, first detach from the child, then detach
>> +# from the parent and confirm that the parent exits, thus unsuring
> 
> Typo: ensuring...

I fixed this locally.

> Otherwise, wow really nice work and quite well commented, I learned a
> lot about the testsuite reading that.

Thanks!  Glad to hear that.

-- 
Pedro Alves

Re: [PATCH v4 2/3] Forget watchpoint locations when inferior exits or is killed/detached

[Pedro Alves]

On 06/24/2016 08:10 PM, Pedro Alves wrote:
> On 06/17/2016 12:55 PM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> +	      /* Get rid of existing locations, which are no longer
>>> +		 valid.  New ones will be created in
>>> +		 update_watchpoint, when the inferior is
>>> +		 restarted.  */
>>> +	      b->loc = NULL;
>>
>> Use decref_bp_location or is it intended?
>>
> 
> It's intended.  The b->loc chain doesn't count for incr/decref.
> It really holds weak references.  The strong references are either
> in  the global location chain or the bpstat chains.  So the next time
> update_global_location_list is called, locations that are found
> to not be referenced by any breakpoint are garbage collected then,
> via decref_bp_location.
> 
> See how update_breakpoint_locations simply hoists old locations
> out of the breakpoint, but doesn't decref them.  [Used to clearer
> before c2f4122d5cc2 ("Limit breakpoint re-set to the current
> program space") though].

I've pushed this with the comment updated to say:

	      /* Get rid of existing locations, which are no longer
		 valid.  New ones will be created in
		 update_watchpoint, when the inferior is restarted.
		 The next update_global_location_list call will
		 garbage collect them.  */

Thanks,
Pedro Alves