[v4,6/7] Remote follow vfork

Message ID 1422222420-25421-7-git-send-email-donb@codesourcery.com
State New, archived
Headers

Commit Message

Don Breazeal Jan. 25, 2015, 9:46 p.m. UTC
  This patch implements follow-fork for vfork on remote and extended-remote
Linux targets.

The implementation follows the native implementation as much as possible.
Most of the work is done on the GDB side in the existing code now in
infrun.c.  GDBserver just has to report the events and do a little
bookkeeping.

Implementation was almost entirely in gdbserver, excepting changes to
gdb/remote.c, and included:

 * enabling VFORK events
   - by adding ptrace options for VFORK and VFORK_DONE as 'additional
     options' in linux-low.c:initialize_low, so that they are available for
     extended-mode.

   - by adding ptrace options for VFORK and VFORK_DONE as 'base options' in
     linux-low.c:linux_enable_extended_features.  In this function we know
     we are in extended-mode, so we enable these options if they are
     supported.
 
 * handling VFORK and VFORK_DONE events in linux-low.c:handle_extended_wait
   by saving the event information for event reporting and for the
   follow_fork request expected to come from GDB.
 
 * including VFORK and VFORK_DONE events in the predicate
   linux-low.c:extended_event_reported.

 * adding support for VFORK and VFORK_DONE events in RSP by adding stop
   reasons "vfork" and "vforkdone" to the 'T' Stop Reply Packet in both
   gdbserver/remote-utils.c and gdb/remote.c.

Tested on x64 Ubuntu Lucid, native, remote, extended-remote.

Thanks
--Don

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

	* linux-low.c (handle_extended_wait): Handle PTRACE_EVENT_FORK and
	PTRACE_EVENT_VFORK_DONE.
	(linux_low_enable_events): Prevent enablement of VFORK events if
	GDB has not requested them.
	(extended_event_reported): Add vfork and vfork-done to the list
	of extended events.
	(initialize_low): Enable PTRACE_EVENT_FORK and PTRACE_EVENT_VFORK_DONE.
	* remote-utils.c (prepare_resume_reply): New stop reasons "vfork"
	and "vforkdone" for RSP 'T' Stop Reply Packet.
	* server.h (report_vfork_events): Declare
	global variable.

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

	* remote.c (remote_follow_fork): Add vfork event type to assert.
	(remote_detach_1): Don't call inf_child_maybe_unpush_target for
	extended-mode targets.
	(remote_parse_stop_reply): New stop reasons "vfork" and
	"vforkdone" for RSP 'T' Stop Reply Packet.
	(remote_kill): Don't call inf_child_maybe_unpush_target for
	extended-mode targets.

---
 gdb/gdbserver/linux-low.c    |   28 ++++++++++++++++++++++------
 gdb/gdbserver/remote-utils.c |   16 ++++++++++++++--
 gdb/gdbserver/server.h       |    1 +
 gdb/remote.c                 |   27 ++++++++++++++++++++++++---
 4 files changed, 61 insertions(+), 11 deletions(-)
  

Comments

Pedro Alves Feb. 10, 2015, 4:39 p.m. UTC | #1
Hi Don,

No major comments here, other than what's been said in
previous patches.  A couple remarks only.

On 01/25/2015 09:46 PM, Don Breazeal wrote:
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1481,7 +1481,10 @@ remote_follow_fork (struct target_ops *target, int follow_child,
>  	  pid_t child_pid;
>  
>  	  gdb_assert ((inferior_thread ()->pending_follow.kind
> -		       == TARGET_WAITKIND_FORKED));
> +		       == TARGET_WAITKIND_FORKED)
> +		      || (inferior_thread ()->pending_follow.kind
> +			  == TARGET_WAITKIND_VFORKED));
> +

Please use a temporary variable, something like:

          kind = inferior_thread ()->pending_follow.kind
 	  gdb_assert (kind == TARGET_WAITKIND_FORKED)
		      || (kind == TARGET_WAITKIND_VFORKED));

Makes it easier to debug a core dump caused by this assert.

>  	  child_ptid = inferior_thread ()->pending_follow.value.related_pid;
>  	  child_pid = ptid_get_pid (child_ptid);
>  
> @@ -4502,7 +4505,8 @@ remote_detach_1 (struct target_ops *ops, const char *args,
>      {
>        inferior_ptid = null_ptid;
>        detach_inferior (pid);
> -      inf_child_maybe_unpush_target (ops);
> +      if (!extended)
> +	inf_child_maybe_unpush_target (ops);

Hmm, I'm afraid I don't understand why is this part of this patch?

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 5af02ac..66b1729 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -381,7 +381,8 @@  handle_extended_wait (struct lwp_info *event_child, int wstat)
   struct thread_info *event_thr = get_lwp_thread (event_child);
   struct lwp_info *new_lwp;
 
-  if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_CLONE))
+  if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_VFORK)
+      || (event == PTRACE_EVENT_CLONE))
     {
       ptid_t ptid;
       unsigned long new_pid;
@@ -407,7 +408,7 @@  handle_extended_wait (struct lwp_info *event_child, int wstat)
 	    warning ("wait returned unexpected status 0x%x", status);
 	}
 
-      if (event == PTRACE_EVENT_FORK)
+      if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK)
 	{
 	  struct process_info *parent_proc;
 	  struct process_info *child_proc;
@@ -449,8 +450,12 @@  handle_extended_wait (struct lwp_info *event_child, int wstat)
 	  if (the_low_target.new_fork != NULL)
 	    the_low_target.new_fork (parent_proc, child_proc);
 
-	  /* Save fork info in the parent thread.  */
-	  event_thr->pending_follow.kind = TARGET_WAITKIND_FORKED;
+	  /* Save fork info for target processing and reporting to GDB.  */
+	  if (event == PTRACE_EVENT_FORK)
+	    event_thr->pending_follow.kind = TARGET_WAITKIND_FORKED;
+	  else if (event == PTRACE_EVENT_VFORK)
+	    event_thr->pending_follow.kind = TARGET_WAITKIND_VFORKED;
+
 	  event_thr->pending_follow.value.related_pid = ptid;
 
 	  /* Report the event.  */
@@ -507,6 +512,11 @@  handle_extended_wait (struct lwp_info *event_child, int wstat)
       /* Don't report the event.  */
       return 1;
     }
+  else if (event == PTRACE_EVENT_VFORK_DONE)
+    {
+      event_thr->pending_follow.kind = TARGET_WAITKIND_VFORK_DONE;
+      return 0;
+    }
 
   internal_error (__FILE__, __LINE__, _("unknown ptrace event %d"), event);
 }
@@ -1883,6 +1893,8 @@  linux_low_enable_events (pid_t pid, int attached)
   /* If GDB doesn't want fork events, don't enable them.  */
   if (!report_fork_events)
     linux_ptrace_clear_additional_flags (PTRACE_O_TRACEFORK);
+  if (!report_vfork_events)
+    linux_ptrace_clear_additional_flags (PTRACE_O_TRACEVFORK);
 
   /* Enable all supported and requested events.  */
   linux_enable_event_reporting (pid, attached);
@@ -2541,7 +2553,9 @@  extended_event_reported (const struct target_waitstatus *waitstatus)
   if (waitstatus == NULL)
     return 0;
 
-  return (waitstatus->kind == TARGET_WAITKIND_FORKED);
+  return (waitstatus->kind == TARGET_WAITKIND_FORKED
+	  || waitstatus->kind == TARGET_WAITKIND_VFORKED
+	  || waitstatus->kind == TARGET_WAITKIND_VFORK_DONE);
 }
 
 /* Wait for process, returns status.  */
@@ -6246,6 +6260,8 @@  initialize_low (void)
   initialize_low_arch ();
 
   /* Enable any extended ptrace events that are supported.  */
-  linux_ptrace_set_additional_flags (PTRACE_O_TRACEFORK);
+  linux_ptrace_set_additional_flags (PTRACE_O_TRACEFORK
+				     | PTRACE_O_TRACEVFORK
+				     | PTRACE_O_TRACEVFORKDONE);
   linux_test_for_event_reporting ();
 }
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index f0cc882..0a4b405 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1115,15 +1115,18 @@  prepare_resume_reply (char *buf, ptid_t ptid,
     {
     case TARGET_WAITKIND_STOPPED:
     case TARGET_WAITKIND_FORKED:
+    case TARGET_WAITKIND_VFORKED:
       {
 	struct thread_info *saved_thread;
 	const char **regp;
 	struct regcache *regcache;
 
-	if (status->kind == TARGET_WAITKIND_FORKED && multi_process)
+	if ((status->kind == TARGET_WAITKIND_FORKED
+	     || status->kind == TARGET_WAITKIND_VFORKED) && multi_process)
 	  {
 	    enum gdb_signal signal = GDB_SIGNAL_TRAP;
-	    const char *event = "fork";
+	    const char *event = (status->kind == TARGET_WAITKIND_FORKED
+				 ? "fork" : "vfork");
 
 	    sprintf (buf, "T%02x%s:p%x.%lx;", signal, event,
 		     ptid_get_pid (status->value.related_pid),
@@ -1234,6 +1237,15 @@  prepare_resume_reply (char *buf, ptid_t ptid,
       else
 	sprintf (buf, "X%02x", status->value.sig);
       break;
+    case TARGET_WAITKIND_VFORK_DONE:
+      if (multi_process)
+	{
+	  enum gdb_signal signal = GDB_SIGNAL_TRAP;
+	  const char *event = "vforkdone";
+
+	  sprintf (buf, "T%02x%s:;", signal, event);
+	}
+      break;
     default:
       error ("unhandled waitkind");
       break;
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index ce6ffb9..5284dac 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -85,6 +85,7 @@  extern int disable_packet_qfThreadInfo;
 extern int run_once;
 extern int multi_process;
 extern int report_fork_events;
+extern int report_vfork_events;
 extern int non_stop;
 
 extern int disable_randomization;
diff --git a/gdb/remote.c b/gdb/remote.c
index 9e48205..6ea3c60 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1481,7 +1481,10 @@  remote_follow_fork (struct target_ops *target, int follow_child,
 	  pid_t child_pid;
 
 	  gdb_assert ((inferior_thread ()->pending_follow.kind
-		       == TARGET_WAITKIND_FORKED));
+		       == TARGET_WAITKIND_FORKED)
+		      || (inferior_thread ()->pending_follow.kind
+			  == TARGET_WAITKIND_VFORKED));
+
 	  child_ptid = inferior_thread ()->pending_follow.value.related_pid;
 	  child_pid = ptid_get_pid (child_ptid);
 
@@ -4502,7 +4505,8 @@  remote_detach_1 (struct target_ops *ops, const char *args,
     {
       inferior_ptid = null_ptid;
       detach_inferior (pid);
-      inf_child_maybe_unpush_target (ops);
+      if (!extended)
+	inf_child_maybe_unpush_target (ops);
     }
 }
 
@@ -5669,6 +5673,21 @@  Packet: '%s'\n"),
 		  event->ws.value.related_pid = read_ptid (++p1, &p);
 		  event->ws.kind = TARGET_WAITKIND_FORKED;
 		}
+	      else if (strncmp (p, "vfork", p1 - p) == 0)
+		{
+		  event->ws.value.related_pid = read_ptid (++p1, &p);
+		  event->ws.kind = TARGET_WAITKIND_VFORKED;
+		}
+	      else if (strncmp (p, "vforkdone", p1 - p) == 0)
+		{
+		  p1++;
+		  p_temp = p1;
+		  while (*p_temp && *p_temp != ';')
+		    p_temp++;
+
+		  event->ws.kind = TARGET_WAITKIND_VFORK_DONE;
+		  p = p_temp;
+		}
 	      else
 		{
 		  /* Silently skip unknown optional info.  */
@@ -7929,6 +7948,7 @@  remote_kill (struct target_ops *ops)
 {
   volatile struct gdb_exception ex;
   int pid = ptid_get_pid (inferior_ptid);
+  struct remote_state *rs = get_remote_state ();
   struct thread_info *tp = first_thread_of_process (pid);
 
   /* Catch errors so the user can quit from gdb even when we
@@ -7965,7 +7985,8 @@  remote_kill (struct target_ops *ops)
     {
       inferior_ptid = null_ptid;
       detach_inferior (pid);
-      inf_child_maybe_unpush_target (ops);
+      if (!rs->extended)
+	inf_child_maybe_unpush_target (ops);
     }
 }