Patchwork [v4,4/7] Target remote follow fork

login
register
mail settings
Submitter Don Breazeal
Date Jan. 25, 2015, 9:46 p.m.
Message ID <1422222420-25421-5-git-send-email-donb@codesourcery.com>
Download mbox | patch
Permalink /patch/4813/
State New
Headers show

Comments

Don Breazeal - Jan. 25, 2015, 9:46 p.m.
This patch enables follow fork in gdbserver for remote (as in 'target
remote') targets.  In addition, it enables some multiprocess functionality
that was previously only available in extended-remote targets, and
prevents gdbserver from exiting after an inferior exit or detach if there
is another active inferior.

This patch also implements a mechanism in linux_low_enable_events to
disable for events if GDB did not inquire about them in the qSupported
packet.

Thanks,
--Don

gdb/gdbserver/
2015-01-25  Don Breazeal  <donb@codesourcery.com>

	* linux-low.c (linux_low_enable_events): Change function
	type from void to int.
	(linux_low_filter_event): Handle return value from
	linux_low_enable_events, only set must_set_ptrace_flags on success.
	* server.c (process_serial_event): Do not exit when target remote
	if there is still an active inferior.

gdb/
2015-01-25  Don Breazeal  <donb@codesourcery.com>

	* remote.c (set_general_process): Remove requirement for 
	extended mode.
	(remote_query_supported): Remove requirement for extended
	mode for fork and vfork event support.
	(remote_detach_1): If target remote, prevent final cleanup
	and exit if there is still an active inferior.
	(remote_kill): Call new function remote_kill_1, move below
	definition of remote_vkill.
	(remote_kill_1): New function.
	(extended_remote_kill): Call new function remote_kill_1.
	(remote_pid_to_str): Remove requirement for extended mode
	for multiprocess-style IDs.
	(remote_supports_multi_process): Remove requirement for
	extended mode.

gdb/testsuite/
2015-01-25  Don Breazeal  <donb@codesourcery.com>

	* gdb.base/multi-forks.exp: Remove restriction on remote target.
	* gdb.threads/linux-dp.exp: Accept multiprocess-style thread IDs.
	* gdb.trace/report.exp: Accept multiprocess-style thread IDs.

---
 gdb/gdbserver/linux-low.c              |   21 ++++-
 gdb/gdbserver/server.c                 |    6 +-
 gdb/remote.c                           |  137 ++++++++++++++++++++-----------
 gdb/testsuite/gdb.base/multi-forks.exp |    4 -
 gdb/testsuite/gdb.threads/linux-dp.exp |    4 +-
 gdb/testsuite/gdb.trace/report.exp     |    2 +-
 6 files changed, 111 insertions(+), 63 deletions(-)

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 9fda25d..6feebd0 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1865,16 +1865,25 @@  check_stopped_by_watchpoint (struct lwp_info *child)
 
 /* Wrapper for linux_enable_event_reporting that supports disabling
    supported events if we have determined we don't want to report
-   them.  This will not be needed once follow fork is implemented
-   for target remote as well as extended-remote.  */
+   them.  Return 0 if this function needs to be called again, 1 if
+   it doesn't.  */
 
-static void
+static int
 linux_low_enable_events (pid_t pid, int attached)
 {
+  /* If GDB has not connected yet, then we don't know if it wants
+     us to report fork events.  Try again later.  */
+  if (!gdb_connected ())
+    return 0;
+      
+  /* If GDB doesn't want fork events, don't enable them.  */
   if (!report_fork_events)
     linux_ptrace_clear_additional_flags (PTRACE_O_TRACEFORK);
 
+  /* Enable all supported and requested events.  */
   linux_enable_event_reporting (pid, attached);
+
+  return 1;
 }
 
 /* Do low-level handling of the event, and check if we should go on
@@ -1964,9 +1973,11 @@  linux_low_filter_event (int lwpid, int wstat)
   if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
     {
       struct process_info *proc = find_process_pid (pid_of (thread));
+      int done;
 
-      linux_low_enable_events (lwpid, proc->attached);
-      child->must_set_ptrace_flags = 0;
+      done = linux_low_enable_events (lwpid, proc->attached);
+      if (done)
+	child->must_set_ptrace_flags = 0;
     }
 
   /* Be careful to not overwrite stop_pc until
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 48cc363..57190f6 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3535,9 +3535,11 @@  process_serial_event (void)
 	  discard_queued_stop_replies (pid);
 	  write_ok (own_buf);
 
-	  if (extended_protocol)
+	  if (extended_protocol || get_first_inferior (&all_threads) != NULL)
 	    {
-	      /* Treat this like a normal program exit.  */
+	      /* There is still at least one inferior remaining, so
+		 don't terminate gdbserver and treat this like a
+		 normal program exit.  */
 	      last_status.kind = TARGET_WAITKIND_EXITED;
 	      last_status.value.integer = 0;
 	      last_ptid = pid_to_ptid (pid);
diff --git a/gdb/remote.c b/gdb/remote.c
index eb540d9..9e48205 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -121,6 +121,8 @@  static void remote_serial_write (const char *str, int len);
 
 static void remote_kill (struct target_ops *ops);
 
+static void extended_remote_kill (struct target_ops *ops);
+
 static int remote_can_async_p (struct target_ops *);
 
 static int remote_is_async_p (struct target_ops *);
@@ -1904,7 +1906,7 @@  set_general_process (void)
   struct remote_state *rs = get_remote_state ();
 
   /* If the remote can't handle multiple processes, don't bother.  */
-  if (!rs->extended || !remote_multi_process_p (rs))
+  if (!remote_multi_process_p (rs))
     return;
 
   /* We only need to change the remote current thread if it's pointing
@@ -4160,11 +4162,8 @@  remote_query_supported (void)
 
       q = remote_query_supported_append (q, "qRelocInsn+");
 
-      if (rs->extended)
-	{
-	  q = remote_query_supported_append (q, "fork-events+");
-	  q = remote_query_supported_append (q, "vfork-events+");
-	}
+      q = remote_query_supported_append (q, "fork-events+");
+      q = remote_query_supported_append (q, "vfork-events+");
 
       q = reconcat (q, "qSupported:", q, (char *) NULL);
       putpkt (q);
@@ -4489,12 +4488,15 @@  remote_detach_1 (struct target_ops *ops, const char *args,
   else
     error (_("Can't detach process."));
 
-  if (from_tty && !extended)
+  /* Are we detaching the last inferior in remote mode?  */
+  if (from_tty && !extended && (number_of_inferiors () == 1))
     puts_filtered (_("Ending remote debugging.\n"));
 
+  /* XXX */
   /* If doing detach-on-fork, we don't mourn, because that will delete
      breakpoints that should be available for the child.  */
-  if (tp->pending_follow.kind != TARGET_WAITKIND_FORKED)
+  if ((tp->pending_follow.kind != TARGET_WAITKIND_FORKED)
+      && (number_of_inferiors () == 1))
     target_mourn_inferior ();
   else
     {
@@ -7862,41 +7864,7 @@  getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever,
 }
 
 
-static void
-remote_kill (struct target_ops *ops)
-{
-  volatile struct gdb_exception ex;
-
-  /* Catch errors so the user can quit from gdb even when we
-     aren't on speaking terms with the remote system.  */
-  TRY_CATCH (ex, RETURN_MASK_ERROR)
-    {
-      putpkt ("k");
-    }
-  if (ex.reason < 0)
-    {
-      if (ex.error == TARGET_CLOSE_ERROR)
-	{
-	  /* If we got an (EOF) error that caused the target
-	     to go away, then we're done, that's what we wanted.
-	     "k" is susceptible to cause a premature EOF, given
-	     that the remote server isn't actually required to
-	     reply to "k", and it can happen that it doesn't
-	     even get to reply ACK to the "k".  */
-	  return;
-	}
-
-	/* Otherwise, something went wrong.  We didn't actually kill
-	   the target.  Just propagate the exception, and let the
-	   user or higher layers decide what to do.  */
-	throw_exception (ex);
-    }
-
-  /* We've killed the remote end, we get to mourn it.  Since this is
-     target remote, single-process, mourning the inferior also
-     unpushes remote_ops.  */
-  target_mourn_inferior ();
-}
+/* Use the vKill packet to perform a 'kill' operation.  */
 
 static int
 remote_vkill (int pid, struct remote_state *rs)
@@ -7923,15 +7891,17 @@  remote_vkill (int pid, struct remote_state *rs)
     }
 }
 
+/* Worker function for remote_kill and extended_remote_kill.  */
+
 static void
-extended_remote_kill (struct target_ops *ops)
+remote_kill_1 (struct target_ops *ops)
 {
   int res;
   int pid = ptid_get_pid (inferior_ptid);
   struct remote_state *rs = get_remote_state ();
 
   res = remote_vkill (pid, rs);
-  if (res == -1 && !(rs->extended && remote_multi_process_p (rs)))
+  if (res == -1 && !remote_multi_process_p (rs))
     {
       /* Don't try 'k' on a multi-process aware stub -- it has no way
 	 to specify the pid.  */
@@ -7950,8 +7920,77 @@  extended_remote_kill (struct target_ops *ops)
 
   if (res != 0)
     error (_("Can't kill process"));
+}
+
+/* Remote target hook for 'kill'.  */
+
+static void
+remote_kill (struct target_ops *ops)
+{
+  volatile struct gdb_exception ex;
+  int pid = ptid_get_pid (inferior_ptid);
+  struct thread_info *tp = first_thread_of_process (pid);
+
+  /* Catch errors so the user can quit from gdb even when we
+     aren't on speaking terms with the remote system.  */
+  TRY_CATCH (ex, RETURN_MASK_ERROR)
+    {
+      remote_kill_1 (ops);
+    }
+  if (ex.reason < 0)
+    {
+      if (ex.error == TARGET_CLOSE_ERROR)
+	{
+	  /* If we got an (EOF) error that caused the target
+	     to go away, then we're done, that's what we wanted.
+	     "k" is susceptible to cause a premature EOF, given
+	     that the remote server isn't actually required to
+	     reply to "k", and it can happen that it doesn't
+	     even get to reply ACK to the "k".  */
+	  return;
+	}
+
+	/* Otherwise, something went wrong.  We didn't actually kill
+	   the target.  Just propagate the exception, and let the
+	   user or higher layers decide what to do.  */
+	throw_exception (ex);
+    }
+
+  /* If doing detach-on-fork, we don't mourn, because that will delete
+     breakpoints that should be available for the child.  */
+  if ((tp->pending_follow.kind != TARGET_WAITKIND_FORKED)
+      && (number_of_inferiors () == 1))
+    target_mourn_inferior ();
+  else
+    {
+      inferior_ptid = null_ptid;
+      detach_inferior (pid);
+      inf_child_maybe_unpush_target (ops);
+    }
+}
+
+/* Extended-remote target hook for 'kill'.  */
+
+static void
+extended_remote_kill (struct target_ops *ops)
+{
+  int pid = ptid_get_pid (inferior_ptid);
+  struct thread_info *tp = first_thread_of_process (pid);
 
-  target_mourn_inferior ();
+  remote_kill_1 (ops);
+
+  /* If doing detach-on-fork, we don't mourn, because that will delete
+     breakpoints that should be available for the child.  */
+  if ((tp->pending_follow.kind != TARGET_WAITKIND_FORKED)
+      && (number_of_inferiors () == 1))
+    target_mourn_inferior ();
+  else
+    {
+      /* Don't unpush the target here (as in remote_kill) since in
+	 extended mode gdbserver continues running without any inferior.  */
+      inferior_ptid = null_ptid;
+      detach_inferior (pid);
+    }
 }
 
 static void
@@ -9477,7 +9516,7 @@  remote_pid_to_str (struct target_ops *ops, ptid_t ptid)
     {
       if (ptid_equal (magic_null_ptid, ptid))
 	xsnprintf (buf, sizeof buf, "Thread <main>");
-      else if (rs->extended && remote_multi_process_p (rs))
+      else if (remote_multi_process_p (rs))
 	if (ptid_get_lwp (ptid) == 0)
 	  return normal_pid_to_str (ptid);
 	else
@@ -10483,7 +10522,7 @@  remote_supports_multi_process (struct target_ops *self)
      processes, even though plain remote can use the multi-process
      thread id extensions, so that GDB knows the target process's
      PID.  */
-  return rs->extended && remote_multi_process_p (rs);
+  return remote_multi_process_p (rs);
 }
 
 static int
@@ -11705,6 +11744,7 @@  Specify the serial device it is connected to\n\
   remote_ops.to_read_btrace = remote_read_btrace;
   remote_ops.to_augmented_libraries_svr4_read =
     remote_augmented_libraries_svr4_read;
+  remote_ops.to_follow_fork = remote_follow_fork;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
@@ -11730,7 +11770,6 @@  Specify the serial device it is connected to (e.g. /dev/ttya).";
   extended_remote_ops.to_kill = extended_remote_kill;
   extended_remote_ops.to_supports_disable_randomization
     = extended_remote_supports_disable_randomization;
-  extended_remote_ops.to_follow_fork = remote_follow_fork;
 }
 
 static int
diff --git a/gdb/testsuite/gdb.base/multi-forks.exp b/gdb/testsuite/gdb.base/multi-forks.exp
index 5682730..5f64410 100644
--- a/gdb/testsuite/gdb.base/multi-forks.exp
+++ b/gdb/testsuite/gdb.base/multi-forks.exp
@@ -13,10 +13,6 @@ 
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-if { [is_remote target] || ![isnative] } then {
-    continue
-}
-
 # Until "set follow-fork-mode" and "catch fork" are implemented on
 # other targets...
 #
diff --git a/gdb/testsuite/gdb.threads/linux-dp.exp b/gdb/testsuite/gdb.threads/linux-dp.exp
index e612978..cb93706 100644
--- a/gdb/testsuite/gdb.threads/linux-dp.exp
+++ b/gdb/testsuite/gdb.threads/linux-dp.exp
@@ -64,7 +64,7 @@  for {set i 0} {$i < 5} {incr i} {
 	-re "^ *Id.*Frame *\[\r\n\]+" {
 	    exp_continue
 	}
-	-re "^. +(\[0-9\]+ *Thread \[-0-9a-fx\]+) \[^\n\]*\n" {
+	-re "^. +(\[0-9\]+ *Thread \[-0-9a-fx\.\]+) \[^\n\]*\n" {
 	    verbose -log "found thread $expect_out(1,string)" 2
 	    lappend threads_before $expect_out(1,string)
 	    exp_continue
@@ -126,7 +126,7 @@  for {set i 0} {$i < 5} {incr i} {
 	-re "^ *Id.*Frame *\[\r\n\]+" {
 	    exp_continue
 	}
-	-re "^. +(\[0-9\]+ *Thread \[-0-9a-fx\]+) \[^\n\]*\n" {
+	-re "^. +(\[0-9\]+ *Thread \[-0-9a-fx\.\]+) \[^\n\]*\n" {
 	    set name $expect_out(1,string)
 	    for {set j 0} {$j != [llength $threads_before] } {incr j} {
 		if {$name == [lindex $threads_before $j]} {
diff --git a/gdb/testsuite/gdb.trace/report.exp b/gdb/testsuite/gdb.trace/report.exp
index 2fa676b..c1de422 100644
--- a/gdb/testsuite/gdb.trace/report.exp
+++ b/gdb/testsuite/gdb.trace/report.exp
@@ -400,7 +400,7 @@  proc use_collected_data { data_source } {
 
 	# There is always a thread of an inferior, either a live one or
 	# a faked one.
-	gdb_test "info threads" "\\* ${decimal}    (process|Thread) ${decimal}\[ \t\].*"
+	gdb_test "info threads" "\\* ${decimal}    (process|Thread) \[0-9\.]*\[ \t\].*"
 	gdb_test "info inferiors" "\\* 1    process ${decimal} \[ \t\]+${binfile}.*"
     }
 }