diff mbox

[v6,3/6] Extended-remote Linux follow fork

Message ID 1426625788-4469-4-git-send-email-donb@codesourcery.com
State New
Headers show

Commit Message

Don Breazeal March 17, 2015, 8:56 p.m. UTC
Hi Pedro,

This patch is mostly unchanged from the previous version.  I did make
a change in handle_extended_wait where I was doing a redundant pointer
assignment.  In fixing that I changed the name of the function argument
'event_child' to 'event_lwp'.  'event_child' was ambiguous, since it
could be the fork parent.

Thanks!
--Don

This patch implements basic support for follow-fork and detach-on-fork on
extended-remote Linux targets.  Only 'fork' is supported in this patch;
'vfork' support is added n a subsequent patch.  This patch depends on 
the previous patches in the patch series.

Sufficient extended-remote functionality has been implemented here to pass
gdb.base/multi-forks.exp, as well as gdb.base/foll-fork.exp with the
catchpoint tests commented out.  Some other fork tests fail with this
patch because it doesn't provide the architecture support needed for
watchpoint inheritance or fork catchpoints.

The implementation follows the same general structure as for the native
implementation as much as possible.

This implementation included:
 * enabling fork events in linux-low.c in initialize_low and
   linux_enable_extended_features

 * handling fork events in gdbserver/linux-low.c:handle_extended_wait

   - when a fork event occurs in gdbserver, we must do the full creation
     of the new process, thread, lwp, and breakpoint lists.  This is
     required whether or not the new child is destined to be
     detached-on-fork, because GDB will make target calls that require all
     the structures.  In particular we need the breakpoint lists in order
     to remove the breakpoints from a detaching child.  If we are not
     detaching the child we will need all these structures anyway.

   - as part of this event handling we store the target_waitstatus in a new
     member of the parent thread_info structure, 'pending_follow'.  This
     is used to store extended event information for reporting to GDB.

   - handle_extended_wait is given a return value, denoting whether the
     handled event should be reported to GDB.  Previously it had only
     handled clone events, which were never reported.

 * using a new predicate in gdbserver to control handling of the fork event
   (and eventually all extended events) in linux_wait_1.  The predicate,
   extended_event_reported, checks a target_waitstatus.kind for an
   extended ptrace event.

 * implementing a new RSP 'T' Stop Reply Packet stop reason: "fork", in
   gdbserver/remote-utils.c and remote.c.

 * implementing new target and RSP support for target_follow_fork with
   target extended-remote.  (The RSP components were actually defined in
   patch 4, but they see their first use here).

   - remote target routine remote_follow_fork, which just sends the 'D;pid'
     detach packet to detach the new fork child cleanly.  We can't just
     call target_detach because the data structures for the forked child
     have not been allocated on the host side.

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

gdb/doc/
2015-03-17  Don Breazeal  <donb@codesourcery.com>

	* gdb.texinfo (Forks): List targets that have fork debugging
	support.
	(Stop Reply Packets): List new stop reason "xfork".

gdb/gdbserver/
2015-03-17  Don Breazeal  <donb@codesourcery.com>

	* linux-low.c (handle_extended_wait): Implement return value,
	handle PTRACE_EVENT_FORK.
	(linux_low_enable_events): New function.
	(linux_low_filter_event): Use return value from
	handle_extended_wait.
	(extended_event_reported): New function.
	(linux_wait_1): Call extended_event_reported.
	(linux_write_memory): Add pid to debug message.
	(reset_lwp_ptrace_options_callback): New function.
	(linux_handle_new_gdb_connection): New function.
	(linux_target_ops): Initialize new structure member.
	(initialize_low): Call linux_ptrace_set_additional_flags.
	* linux-low.h (struct lwp_info) <waitstatus>: New member.
	* lynx-low.c: Initialize new structure member.
	* remote-utils.c (prepare_resume_reply): Implement stop reason
	"xfork" for "T" stop message.
	* server.h (report_fork_events): Declare global flag.
	* target.h (struct target_ops) <handle_new_gdb_connection>:
	New member.
	* win32-low.c: Initialize new structure member.

gdb/
2015-03-17  Don Breazeal  <donb@codesourcery.com>

	* NEWS: Announce follow fork support and associated RSP
	addition.
	* nat/linux-ptrace.c (supported_additional_flags): New
	static variable.
	(linux_check_ptrace_features): Use supported_additional_flags.
	(linux_test_for_tracesysgood): Likewise.
	(linux_test_for_tracefork): Likewise.
	(linux_reset_ptrace_options): New function.
	(linux_ptrace_clear_flags): New function.
	* nat/linux-ptrace.h: (linux_ptrace_clear_flags): Declare new
	function.
	* remote.c (remote_fork_event_p): New function.
	(remote_detach_1): Add target_ops argument, don't mourn inferior
	if doing detach-on-fork.
	(remote_detach): Add target_ops argument, pass to remote_detach_1.
	(extended_remote_detach): Add target_ops argument, pass to
	remote_detach_1.
	(remote_follow_fork): New function.
	(remote_parse_stop_reply): Handle new "T" stop reason "xfork".
	(remote_pid_to_str): Print "process" strings for pid/0/0 ptids.
	(init_extended_remote_ops): Initialize to_follow_fork.

gdb/testsuite/
2015-03-17  Don Breazeal  <donb@codesourcery.com>

	* gdb.base/multi-forks.exp (continue_to_exit_bp_loc):
	Use '-i' in expect statement to include input from forked child.
	Ignore inferior output if gdb,noinferiorio.
	* lib/gdb.exp (server_spawn_id): Declare new global
	variable.
	* lib/gdbserver-support.exp (server_spawn_id): Make
	variable global.

---
 gdb/NEWS                                |  15 ++-
 gdb/doc/gdb.texinfo                     |   9 ++
 gdb/gdbserver/linux-low.c               | 170 +++++++++++++++++++++++++++++---
 gdb/gdbserver/linux-low.h               |   4 +
 gdb/gdbserver/lynx-low.c                |   1 +
 gdb/gdbserver/remote-utils.c            |  15 ++-
 gdb/gdbserver/server.c                  |   3 +
 gdb/gdbserver/server.h                  |   1 +
 gdb/gdbserver/target.h                  |   7 ++
 gdb/gdbserver/win32-low.c               |   1 +
 gdb/linux-nat.c                         |   6 +-
 gdb/nat/linux-ptrace.c                  |  59 +++++++++--
 gdb/nat/linux-ptrace.h                  |  11 +++
 gdb/remote.c                            |  98 +++++++++++++++---
 gdb/testsuite/gdb.base/multi-forks.exp  |  22 ++++-
 gdb/testsuite/lib/gdb.exp               |   3 +
 gdb/testsuite/lib/gdbserver-support.exp |   2 +
 17 files changed, 382 insertions(+), 45 deletions(-)

Comments

Pedro Alves March 24, 2015, 12:16 p.m. UTC | #1
Hi Don,

On 03/17/2015 08:56 PM, Don Breazeal wrote:

>  * using a new predicate in gdbserver to control handling of the fork event
>    (and eventually all extended events) in linux_wait_1.  The predicate,
>    extended_event_reported, checks a target_waitstatus.kind for an
>    extended ptrace event.
> 
>  * implementing a new RSP 'T' Stop Reply Packet stop reason: "fork", in
>    gdbserver/remote-utils.c and remote.c.
> 
>  * implementing new target and RSP support for target_follow_fork with
>    target extended-remote.  (The RSP components were actually defined in
>    patch 4, but they see their first use here).

This description seems to be a bit out of date.  We're in patch 3 still.  :-)

> 
>    - remote target routine remote_follow_fork, which just sends the 'D;pid'
>      detach packet to detach the new fork child cleanly.  We can't just
>      call target_detach because the data structures for the forked child
>      have not been allocated on the host side.
> 
> Tested on x64 Ubuntu Lucid, native, remote, extended-remote.
> 
> gdb/doc/
> 2015-03-17  Don Breazeal  <donb@codesourcery.com>
> 
> 	* gdb.texinfo (Forks): List targets that have fork debugging
> 	support.
> 	(Stop Reply Packets): List new stop reason "xfork".

I know I was the one who suggested avoiding "fork" because it starts
with an hex char, but thinking further, if gdb must report
support for fork events for the server to report these, it's ok
to use "fork".  I'm no thinking we can rename it back, thus
fending off questions about this in the future.  Could you
do that?  Sorry about the back and forth...

>  
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b089b1b..b62b37f 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -3142,6 +3142,9 @@ create additional processes using the @code{fork} or @code{vfork} functions.
>  Currently, the only platforms with this feature are HP-UX (11.x and later
>  only?) and @sc{gnu}/Linux (kernel version 2.5.60 and later).
>  
> +The fork debugging commands are supported in both native mode and when
> +connected to @code{gdbserver} using @kbd{target extended-remote}.
> +
>  By default, when a program forks, @value{GDBN} will continue to debug
>  the parent process and the child process will run unimpeded.
>  
> @@ -35197,6 +35200,12 @@ The packet indicates that the loaded libraries have changed.
>  @value{GDBN} should use @samp{qXfer:libraries:read} to fetch a new
>  list of loaded libraries.  The @var{r} part is ignored.
>  
> +@cindex fork events, remote reply
> +@item xfork
> +The packet indicates that @code{fork} was called, and @var{r}
> +is the ptid of the new child process.  This packet is only
> +applicable to targets that support fork events.
> +

Please mention here the need for reporting support in qSupported.
E.g., the new swbreak stop reason says:

~~~
This packet should not be sent by default; older @value{GDBN} versions
did not support it.  @value{GDBN} requests it, by supplying an
appropriate @samp{qSupported} feature (@pxref{qSupported}).  The
remote stub must also supply the appropriate @samp{qSupported} feature
indicating support.
~~~

>        if (debug_threads)
>  	debug_printf ("HEW: Got clone event "
>  		      "from LWP %ld, new child is LWP %ld\n",
> @@ -434,7 +487,12 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
>  	  new_lwp->status_pending_p = 1;
>  	  new_lwp->status_pending = status;
>  	}
> +
> +      /* Don't report the event.  */
> +      return 1;
>      }
> +
> +  internal_error (__FILE__, __LINE__, _("unknown ptrace event %d"), event);
>  }
>  
>  /* Return the PC as read from the regcache of LWP, without any
> @@ -1828,6 +1886,19 @@ check_stopped_by_watchpoint (struct lwp_info *child)
>    return child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT;
>  }
>  
> +/* Wrapper for linux_enable_event_reporting that disables any
> +   supported events that we have determined should not be
> +   reported (e.g. GDB did not request them).  */
> +
> +static void
> +linux_low_enable_events (pid_t pid, int attached)
> +{
> +  if (!report_fork_events)
> +    linux_ptrace_clear_flags (PTRACE_O_TRACEFORK);
> +
> +  linux_enable_event_reporting (pid, attached);
> +}
> +
>  /* Do low-level handling of the event, and check if we should go on
>     and pass it to caller code.  Return the affected lwp if we are, or
>     NULL otherwise.  */
> @@ -1912,11 +1983,11 @@ linux_low_filter_event (int lwpid, int wstat)
>  	}
>      }
>  
> -  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
> +  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags && gdb_connected ())
>      {

I don't really understand this.  If the flag is set, why would it matter
whether gdb is connected?

>        struct process_info *proc = find_process_pid (pid_of (thread));
>  
> -      linux_enable_event_reporting (lwpid, proc->attached);
> +      linux_low_enable_events (lwpid, proc->attached);
>        child->must_set_ptrace_flags = 0;
>      }
>  
> @@ -1926,8 +1997,12 @@ linux_low_filter_event (int lwpid, int wstat)
>        && linux_is_extended_waitstatus (wstat))
>      {
>        child->stop_pc = get_pc (child);
> -      handle_extended_wait (child, wstat);
> -      return NULL;
> +      if (handle_extended_wait (child, wstat))
> +	{
> +	  /* The event has been handled, so just return without
> +	     reporting it.  */
> +	  return NULL;
> +	}
>      }
>  
>    /* Check first whether this was a SW/HW breakpoint before checking
> @@ -2502,6 +2577,18 @@ ignore_event (struct target_waitstatus *ourstatus)
>    return null_ptid;
>  }
>  
> +/* Return non-zero if WAITSTATUS reflects an extended linux
> +   event.  Otherwise, return zero.  */
> +
> +static int
> +extended_event_reported (const struct target_waitstatus *waitstatus)
> +{
> +  if (waitstatus == NULL)
> +    return 0;
> +
> +  return (waitstatus->kind == TARGET_WAITKIND_FORKED);
> +}
> +
>  /* Wait for process, returns status.  */
>  
>  static ptid_t
> @@ -2868,7 +2955,8 @@ linux_wait_1 (ptid_t ptid,
>  		       && !bp_explains_trap && !trace_event)
>  		   || (gdb_breakpoint_here (event_child->stop_pc)
>  		       && gdb_condition_true_at_breakpoint (event_child->stop_pc)
> -		       && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
> +		       && gdb_no_commands_at_breakpoint (event_child->stop_pc))
> +		   || extended_event_reported (&event_child->waitstatus));
>  
>    run_breakpoint_commands (event_child->stop_pc);
>  
> @@ -2890,6 +2978,13 @@ linux_wait_1 (ptid_t ptid,
>  			  paddress (event_child->stop_pc),
>  			  paddress (event_child->step_range_start),
>  			  paddress (event_child->step_range_end));
> +	  if (extended_event_reported (&event_child->waitstatus))
> +	    {
> +	      char *str = target_waitstatus_to_string (ourstatus);
> +	      debug_printf ("LWP %ld: extended event with waitstatus %s\n",
> +			    lwpid_of (get_lwp_thread (event_child)), str);
> +	      xfree (str);
> +	    }
>  	}
>  
>        /* We're not reporting this breakpoint to GDB, so apply the
> @@ -2999,7 +3094,17 @@ linux_wait_1 (ptid_t ptid,
>  	unstop_all_lwps (1, event_child);
>      }
>  
> -  ourstatus->kind = TARGET_WAITKIND_STOPPED;
> +  if (extended_event_reported (&event_child->waitstatus))
> +    {
> +      /* If the reported event is a fork, vfork or exec, let GDB know.  */
> +      ourstatus->kind = event_child->waitstatus.kind;
> +      ourstatus->value = event_child->waitstatus.value;
> +
> +      /* Reset the event lwp's waitstatus since we handled it already.  */
> +      event_child->waitstatus.kind = TARGET_WAITKIND_SPURIOUS;

I'd think this should be TARGET_WAITKIND_IGNORE?  That's the usual
status for "clear".

> +    }
> +  else
> +    ourstatus->kind = TARGET_WAITKIND_STOPPED;
>  
>    /* Now that we've selected our final event LWP, un-adjust its PC if
>       it was a software breakpoint, and the client doesn't know we can
> @@ -3032,7 +3137,7 @@ linux_wait_1 (ptid_t ptid,
>  	 but, it stopped for other reasons.  */
>        ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
>      }
> -  else
> +  else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
>      {
>        ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
>      }
> @@ -4830,8 +4935,8 @@ linux_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
>  	val = val & 0xffff;
>        else if (len == 3)
>  	val = val & 0xffffff;
> -      debug_printf ("Writing %0*x to 0x%08lx\n", 2 * ((len < 4) ? len : 4),
> -		    val, (long)memaddr);
> +      debug_printf ("Writing %0*x to 0x%08lx in process %d\n",
> +		    2 * ((len < 4) ? len : 4), val, (long)memaddr, pid);
>      }
>  
>    /* Fill start and end extra bytes of buffer with existing memory data.  */
> @@ -5271,6 +5376,39 @@ linux_supports_vfork_events (void)
>    return linux_supports_tracefork ();
>  }
>  
> +/* Callback for 'find_inferior'.  Set the (possibly changed) ptrace
> +   options for the specified lwp.  */
> +
> +static int
> +reset_lwp_ptrace_options_callback (struct inferior_list_entry *entry,
> +				   void *args)
> +{
> +  struct thread_info *thread = (struct thread_info *) entry;
> +  struct lwp_info *lwp = get_thread_lwp (thread);
> +  struct process_info *proc = find_process_pid (pid_of (thread));
> +
> +  linux_low_enable_events (lwpid_of (thread), proc->attached);
> +  lwp->must_set_ptrace_flags = 0;
> +
> +  return 0;
> +}
> +
> +/* Target hook for 'handle_new_gdb_connection'.  Causes a reset of the
> +   ptrace flags for all inferiors.  This is in case the new GDB connection
> +   doesn't support the same set of events that the previous one did.  */
> +
> +static void
> +linux_handle_new_gdb_connection (void)
> +{
> +  pid_t pid;
> +
> +  /* Reset the ptrace options to enable on the inferior(s).  */
> +  linux_reset_ptrace_options ();
> +
> +  /* Request that all the lwps reset their ptrace options.  */
> +  find_inferior (&all_threads, reset_lwp_ptrace_options_callback , &pid);

Spurious space before ', &pid'.  But, you can't do this if threads
are already running, such as when you reconnect after you left the
target running for disconnected tracing.  Instead, you need to
force threads to momentarily pause and set their must_set_ptrace_flags
flag, much like when we need to change running thread's debug registers
for watchpoints.  See linux-x86-low.c:update_debug_registers_callback.

>  
> diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
> index aba3da8..3494bcf 100644
> --- a/gdb/nat/linux-ptrace.c
> +++ b/gdb/nat/linux-ptrace.c
> @@ -25,13 +25,16 @@
>  
>  #include <stdint.h>
>  
> -/* Stores the currently supported ptrace options.  A value of
> -   -1 means we did not check for features yet.  A value of 0 means
> -   there are no supported features.  */
> +/* Stores the ptrace options currently enabled by this gdbserver.

Talking about "this gdbserver" here doesn't make sense.  This file
is used by native gdb too.

> +   A value of -1 means we did not check for features yet.  A value
> +   of 0 means there are no enabled features.  */
>  static int current_ptrace_options = -1;
>  
> -/* Additional flags to test.  */
> +/* Stores the fork and exec ptrace options supported by the operating
> +   system.  */
> +static int supported_additional_flags = 0;
>  
> +/* Additional flags to test.  */
>  static int additional_flags;
>  
>  /* Find all possible reasons we could fail to attach PID and append
> @@ -344,6 +347,7 @@ linux_check_ptrace_features (void)
>  
>    /* Initialize the options.  */
>    current_ptrace_options = 0;
> +  supported_additional_flags = 0;
>  
>    /* Fork a child so we can do some testing.  The child will call
>       linux_child_function and will get traced.  The child will
> @@ -394,7 +398,7 @@ linux_test_for_tracesysgood (int child_pid)
>  		(PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
>  
>    if (ret == 0)
> -    current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
> +    supported_additional_flags |= PTRACE_O_TRACESYSGOOD;
>  }
>  
>  /* Determine if PTRACE_O_TRACEFORK can be used to follow fork
> @@ -421,7 +425,7 @@ linux_test_for_tracefork (int child_pid)
>  		    (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
>  					| PTRACE_O_TRACEVFORKDONE));
>        if (ret == 0)
> -	current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
> +	supported_additional_flags |= PTRACE_O_TRACEVFORKDONE;
>      }
>  
>    /* Setting PTRACE_O_TRACEFORK did not cause an error, however we
> @@ -459,9 +463,16 @@ linux_test_for_tracefork (int child_pid)
>  	  /* We got the PID from the grandchild, which means fork
>  	     tracing is supported.  */
>  	  current_ptrace_options |= PTRACE_O_TRACECLONE;
> -	  current_ptrace_options |= (additional_flags & (PTRACE_O_TRACEFORK
> -                                                         | PTRACE_O_TRACEVFORK
> -                                                         | PTRACE_O_TRACEEXEC));
> +
> +	  /* Save the "extended" options in case we need to reset
> +	     the options later for a connect from a different GDB.  */
> +	  supported_additional_flags |= (PTRACE_O_TRACEFORK
> +					  | PTRACE_O_TRACEVFORK
> +					  | PTRACE_O_TRACEEXEC);
> +
> +	  /* Enable only those options requested by GDB.  */
> +	  current_ptrace_options |= (supported_additional_flags
> +				     & additional_flags);
>  
>  	  /* Do some cleanup and kill the grandchild.  */
>  	  my_waitpid (second_pid, &second_status, 0);
> @@ -518,6 +529,25 @@ linux_enable_event_reporting (pid_t pid, int attached)
>  	  (PTRACE_TYPE_ARG4) (uintptr_t) ptrace_options);
>  }
>  
> +/* Reset the ptrace options using potentially different
> +   additional_flags.  */
> +
> +void
> +linux_reset_ptrace_options (void)
> +{
> +  /* Check if we have initialized the ptrace features for this
> +     target.  If not, do it now.  */
> +  if (current_ptrace_options == -1)
> +    linux_check_ptrace_features ();
> +
> +  /* Clear only the "extended" options.  */
> +  linux_ptrace_clear_flags (all_additional_flags);
> +
> +  /* Add the new requested flags back in.  */
> +  current_ptrace_options |= (supported_additional_flags
> +			     & additional_flags);
> +}
> +
>  /* Disable reporting of all currently supported ptrace events.  */
>  
>  void
> @@ -603,9 +633,20 @@ linux_ptrace_init_warnings (void)
>  void
>  linux_ptrace_set_additional_flags (int flags)
>  {
> +  int invalid_flags = (flags & ~all_additional_flags);
> +
> +  gdb_assert (invalid_flags == 0);
>    additional_flags = flags;
>  }
>  
> +/* Clear FLAGS in current_ptrace_options.  */
> +
> +void
> +linux_ptrace_clear_flags (int flags)
> +{
> +  current_ptrace_options &= ~flags;
> +}
> +

This still looks a bit messy and mostly unnecessary to me.  :-/

Why not something based on this?  In linux-ptrace.c:

/* Enable reporting of all currently supported ptrace events.
   OPTIONS is a bit mask of extended features we want enabled,
   if supported by the kernel.  PTRACE_O_TRACECLONE is always
   enabled, if supported.  */

void
linux_enable_event_reporting (pid_t pid, int options)
{
  /* Check if we have initialized the ptrace features for this
     target.  If not, do it now.  */
  if (current_ptrace_options == -1)
    linux_check_ptrace_features ();

  /* We always want clone events.  */
  options |= PTRACE_O_TRACECLONE;

  /* Filter out unsupported options.  */
  options &= current_ptrace_options;

  /* Set the options.  */
  ptrace (PTRACE_SETOPTIONS, pid, (PTRACE_TYPE_ARG3) 0,
	  (PTRACE_TYPE_ARG4) (uintptr_t) options);
}


(current_ptrace_options would be renamed to
supported_ptrace_options or some such afterwards.)

and then in linux-low.c:

  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
    {
      struct process_info *proc = find_process_pid (pid_of (thread));
      int options = 0;

      if (!proc->attached)
        options |= PTRACE_O_EXITKILL;

      if (report_fork_events)
	options |= PTRACE_O_TRACEFORK;

      if (report_vfork_events)
	options |= (PTRACE_O_TRACEVFORKDONE | PTRACE_O_TRACEVFORK);

      if (report_exec_events)
	options |= PTRACE_O_TRACEEXEC;

      linux_enable_event_reporting (pid, options);
      child->must_set_ptrace_flags = 0;
    }

That'd cope with a different gdb reconnecting and requesting
different options too.


> -/* This takes a program previously attached to and detaches it.  After
> -   this is done, GDB can be used to debug some other program.  We
> -   better not have left any breakpoints in the target program or it'll
> -   die when it hits one.  */
> +/* This detaches a program to which we previously attached, using
> +   inferior_ptid to identify the process.  After this is done, GDB
> +   can be used to debug some other program.  We better not have left
> +   any breakpoints in the target program or it'll die when it hits
> +   one.  If IS_FORK_CHILD is true, then inferior_ptid is the child
> +   of an unfollowed fork, and we need to avoid deleting breakpoints
> +   still needed by the parent.  */
>  
>  static void
> -remote_detach_1 (const char *args, int from_tty, int extended)
> +remote_detach_1 (struct target_ops *ops, const char *args,
> +		 int from_tty, int is_fork_child)
>  {
>    int pid = ptid_get_pid (inferior_ptid);
>    struct remote_state *rs = get_remote_state ();
> +  struct thread_info *tp = find_thread_ptid (inferior_ptid);
> +  int is_fork_parent;
>  
>    if (args)
>      error (_("Argument given to \"detach\" when remotely debugging."));
> @@ -4468,22 +4482,75 @@ remote_detach_1 (const char *args, int from_tty, int extended)
>    else
>      error (_("Can't detach process."));
>  
> -  if (from_tty && !extended)
> +  if (from_tty && !rs->extended)
>      puts_filtered (_("Ending remote debugging.\n"));
>  
> -  target_mourn_inferior ();
> +  /* Check to see if we are detaching a fork parent.  Note that if we
> +     are detaching a fork child, tp == NULL.  */
> +  if (tp != NULL)
> +    is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;
> +
> +  /* If doing detach-on-fork, we don't mourn, because that will delete
> +     breakpoints that should be available for the followed inferior.  */
> +  if (!is_fork_child && !is_fork_parent)
> +    target_mourn_inferior ();
> +  else
> +    {
> +      inferior_ptid = null_ptid;
> +      detach_inferior (pid);
> +    }
>  }

This looks quite confusing to me.  Please instead simply factor out
the bits that actually do the remote detach to a separate
function, like:

static void
remote_detach_pid (int pid)
{
  struct remote_state *rs = get_remote_state ();

  if (remote_multi_process_p (rs))
    xsnprintf (rs->buf, get_remote_packet_size (), "D;%x", pid);
  else
    strcpy (rs->buf, "D");

  putpkt (rs->buf);
  getpkt (&rs->buf, &rs->buf_size, 0);

  if (rs->buf[0] == 'O' && rs->buf[1] == 'K')
    ;
  else if (rs->buf[0] == '\0')
    error (_("Remote doesn't know how to detach"));
  else
    error (_("Can't detach process."));
}

And use that from both remote_detach_1 and the detach-child-fork
case.

> +static int
> +remote_follow_fork (struct target_ops *ops, int follow_child,
> +		    int detach_fork)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +
> +  if (remote_fork_event_p (rs))
> +    {
> +      if (detach_fork && !follow_child)

Aren't we missing the "detach_fork && follow_child" case?

> +	{
> +	  ptid_t parent_ptid;
> +	  ptid_t child_ptid;
> +
> +	  gdb_assert (inferior_thread ()->pending_follow.kind
> +		      == TARGET_WAITKIND_FORKED);
> +
> +	  /* remote_detach_1 detaches inferior_ptid, which is currently
> +	     the ptid of the parent.  Switch inferior_ptid to the ptid
> +	     of the child for the duration of the call.  */
> +	  parent_ptid = inferior_ptid;
> +	  child_ptid = inferior_thread ()->pending_follow.value.related_pid;
> +	  inferior_ptid = child_ptid;
> +	  gdb_assert (!ptid_equal (parent_ptid, child_ptid));
> +
> +	  remote_detach_1 (ops, NULL, 0, 1);

This then ends up being just remote_detach_pid + detach_inferior, AFAICS.


> +
> +	  /* Restore the parent ptid.  */
> +	  inferior_ptid = parent_ptid;
> +	}
> +    }
> +  return 0;
>  }
>  
>  /* Same as remote_detach, but don't send the "D" packet; just disconnect.  */
> @@ -5652,6 +5719,11 @@ Packet: '%s'\n"),
>  	      p = unpack_varlen_hex (++p1, &c);
>  	      event->core = c;
>  	    }
> +	  else if (strncmp (p, "xfork", p1 - p) == 0)
> +	    {
> +	      event->ws.value.related_pid = read_ptid (++p1, &p);
> +	      event->ws.kind = TARGET_WAITKIND_FORKED;
> +	    }
>  	  else
>  	    {
>  	      ULONGEST pnum;
> @@ -9514,8 +9586,11 @@ 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))
> -	xsnprintf (buf, sizeof buf, "Thread %d.%ld",
> -		   ptid_get_pid (ptid), ptid_get_lwp (ptid));
> +	if (ptid_get_lwp (ptid) == 0)
> +	  return normal_pid_to_str (ptid);
> +	else
> +	  xsnprintf (buf, sizeof buf, "Thread %d.%ld",
> +		     ptid_get_pid (ptid), ptid_get_lwp (ptid));
>        else
>  	xsnprintf (buf, sizeof buf, "Thread %ld",
>  		   ptid_get_lwp (ptid));
> @@ -11938,6 +12013,7 @@ 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 e95cb4b..2a71e63 100644
> --- a/gdb/testsuite/gdb.base/multi-forks.exp
> +++ b/gdb/testsuite/gdb.base/multi-forks.exp
> @@ -62,6 +62,23 @@ proc continue_to_exit_bp_loc {} {
>      set seen_break 0
>      set seen_prompt 0
>      set seen_timeout 0
> +
> +    # If we are running with a native gdbserver, the output ($decimal done)
> +    # will come via the spawn_id of gdbserver, not the spawn_id of gdb (the
> +    # default).  So we grab the spawn_id of gdbserver, if it exists, and
> +    # add it to the gdb_expect statement below using "-i", allowing us to
> +    # apply the expect statement to the output of both spawn_ids.
> +    #
> +    # If we are running with a truly remote gdbserver (gdb,noinferiorio),
> +    # the output of the program will be inaccessible.  In this case we do
> +    # not check for the ($decimal done) output, but just look for the gdb
> +    # prompt.
> +    global server_spawn_id
> +    set current_spawn_id [board_info host fileid]
> +    if {![info exists server_spawn_id]} {
> +	set server_spawn_id ""
> +    }
> +
>      while { ($seen_done < 16 || ! $seen_prompt) && ! $seen_timeout } {
>  	# 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
> @@ -70,7 +87,8 @@ proc continue_to_exit_bp_loc {} {
>  	# first in the script that occurs anywhere in the input, so that
>  	# we don't skip anything.
>  	gdb_expect {
> -	    -re "($decimal done)|(Breakpoint)|($gdb_prompt)" {
> +	    -i "$current_spawn_id $server_spawn_id" \


Note this is currently working by chance.  We need this one in:

  https://sourceware.org/ml/gdb-patches/2015-02/msg00659.html

in order to safely be able to do '-i "$server_spawn_id"'.


> +	         -re "($decimal done)|(Breakpoint)|($gdb_prompt)" {
>  		if {[info exists expect_out(1,string)]} {
>  		    incr seen_done
>  		} elseif {[info exists expect_out(2,string)]} {

Thanks,
Pedro Alves
Pedro Alves April 7, 2015, 5:35 p.m. UTC | #2
On 03/24/2015 12:16 PM, Pedro Alves wrote:
>> > diff --git a/gdb/testsuite/gdb.base/multi-forks.exp b/gdb/testsuite/gdb.base/multi-forks.exp
>> > index e95cb4b..2a71e63 100644
>> > --- a/gdb/testsuite/gdb.base/multi-forks.exp
>> > +++ b/gdb/testsuite/gdb.base/multi-forks.exp
>> > @@ -62,6 +62,23 @@ proc continue_to_exit_bp_loc {} {
>> >      set seen_break 0
>> >      set seen_prompt 0
>> >      set seen_timeout 0
>> > +
>> > +    # If we are running with a native gdbserver, the output ($decimal done)
>> > +    # will come via the spawn_id of gdbserver, not the spawn_id of gdb (the
>> > +    # default).  So we grab the spawn_id of gdbserver, if it exists, and
>> > +    # add it to the gdb_expect statement below using "-i", allowing us to
>> > +    # apply the expect statement to the output of both spawn_ids.
>> > +    #
>> > +    # If we are running with a truly remote gdbserver (gdb,noinferiorio),
>> > +    # the output of the program will be inaccessible.  In this case we do
>> > +    # not check for the ($decimal done) output, but just look for the gdb
>> > +    # prompt.
>> > +    global server_spawn_id
>> > +    set current_spawn_id [board_info host fileid]
>> > +    if {![info exists server_spawn_id]} {
>> > +	set server_spawn_id ""
>> > +    }
>> > +
>> >      while { ($seen_done < 16 || ! $seen_prompt) && ! $seen_timeout } {
>> >  	# 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
>> > @@ -70,7 +87,8 @@ proc continue_to_exit_bp_loc {} {
>> >  	# first in the script that occurs anywhere in the input, so that
>> >  	# we don't skip anything.
>> >  	gdb_expect {
>> > -	    -re "($decimal done)|(Breakpoint)|($gdb_prompt)" {
>> > +	    -i "$current_spawn_id $server_spawn_id" \
> 
> Note this is currently working by chance.  We need this one in:
> 
>   https://sourceware.org/ml/gdb-patches/2015-02/msg00659.html
> 
> in order to safely be able to do '-i "$server_spawn_id"'.

FYI, I now pushed that series in.  Please update this to
use $inferior_spawn_id instead.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index d6c8769..5aedda5 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -76,6 +76,10 @@  hwbreak stop reason
 vFile:fstat:
   Return information about files on the remote system.
 
+T Stop Reply Packet's reason
+  The T stop reply packet supports a new stop reason 'xfork', which
+  signifies that the specified inferior has executed a fork.
+
 * The info record command now shows the recording format and the
   branch tracing configuration for the current thread when using
   the btrace record target.
@@ -92,8 +96,15 @@  vFile:fstat:
 HP/PA running HP-UX           hppa*-*-hpux*
 Itanium running HP-UX         ia64-*-hpux*
 
-* The remote stub now reports support for fork and vfork events to GDB's
-qSupported query.
+* Remote fork events
+
+  The remote stub now reports support for fork and vfork events to
+  GDB's qSupported query.
+
+  GDBserver extended-remote Linux targets now provides basic support
+  for fork events.  This enables follow-fork-mode and detach-on-fork
+  for those targets with Linux kernels 2.5.60 and later.  Hardware
+  watchpoints are not inherited across a fork in this implementation.
 
 *** Changes in GDB 7.9
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b089b1b..b62b37f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3142,6 +3142,9 @@  create additional processes using the @code{fork} or @code{vfork} functions.
 Currently, the only platforms with this feature are HP-UX (11.x and later
 only?) and @sc{gnu}/Linux (kernel version 2.5.60 and later).
 
+The fork debugging commands are supported in both native mode and when
+connected to @code{gdbserver} using @kbd{target extended-remote}.
+
 By default, when a program forks, @value{GDBN} will continue to debug
 the parent process and the child process will run unimpeded.
 
@@ -35197,6 +35200,12 @@  The packet indicates that the loaded libraries have changed.
 @value{GDBN} should use @samp{qXfer:libraries:read} to fetch a new
 list of loaded libraries.  The @var{r} part is ignored.
 
+@cindex fork events, remote reply
+@item xfork
+The packet indicates that @code{fork} was called, and @var{r}
+is the ptid of the new child process.  This packet is only
+applicable to targets that support fork events.
+
 @cindex replay log events, remote reply
 @item replaylog
 The packet indicates that the target cannot continue replaying 
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index c710344..8372f36 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -20,6 +20,7 @@ 
 #include "linux-low.h"
 #include "nat/linux-osdata.h"
 #include "agent.h"
+#include "tdesc.h"
 
 #include "nat/linux-nat.h"
 #include "nat/linux-waitpid.h"
@@ -371,22 +372,23 @@  linux_add_process (int pid, int attached)
 static CORE_ADDR get_pc (struct lwp_info *lwp);
 
 /* Handle a GNU/Linux extended wait response.  If we see a clone
-   event, we need to add the new LWP to our list (and not report the
-   trap to higher layers).  */
+   event, we need to add the new LWP to our list (and return 0 so as
+   not to report the trap to higher layers).  */
 
-static void
-handle_extended_wait (struct lwp_info *event_child, int wstat)
+static int
+handle_extended_wait (struct lwp_info *event_lwp, int wstat)
 {
   int event = linux_ptrace_get_extended_event (wstat);
-  struct thread_info *event_thr = get_lwp_thread (event_child);
+  struct thread_info *event_thr = get_lwp_thread (event_lwp);
   struct lwp_info *new_lwp;
 
-  if (event == PTRACE_EVENT_CLONE)
+  if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_CLONE))
     {
       ptid_t ptid;
       unsigned long new_pid;
       int ret, status;
 
+      /* Get the pid of the new lwp.  */
       ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
 	      &new_pid);
 
@@ -406,6 +408,57 @@  handle_extended_wait (struct lwp_info *event_child, int wstat)
 	    warning ("wait returned unexpected status 0x%x", status);
 	}
 
+      if (event == PTRACE_EVENT_FORK)
+	{
+	  struct process_info *parent_proc;
+	  struct process_info *child_proc;
+	  struct lwp_info *child_lwp;
+	  struct target_desc *tdesc;
+
+	  ptid = ptid_build (new_pid, new_pid, 0);
+
+	  if (debug_threads)
+	    {
+	      debug_printf ("HEW: Got fork event from LWP %ld, "
+			    "new child is %d\n",
+			    ptid_get_lwp (ptid_of (event_thr)),
+			    ptid_get_pid (ptid));
+	    }
+
+	  /* Add the new process to the tables and clone the breakpoint
+	     lists of the parent.  We need to do this even if the new process
+	     will be detached, since we will need the process object and the
+	     breakpoints to remove any breakpoints from memory when we
+	     detach, and the host side will access registers.  */
+	  child_proc = linux_add_process (new_pid, 0);
+	  gdb_assert (child_proc != NULL);
+	  child_lwp = add_lwp (ptid);
+	  gdb_assert (child_lwp != NULL);
+	  child_lwp->stopped = 1;
+	  parent_proc = get_thread_process (event_thr);
+	  child_proc->attached = parent_proc->attached;
+	  clone_all_breakpoints (&child_proc->breakpoints,
+				 &child_proc->raw_breakpoints,
+				 parent_proc->breakpoints);
+
+	  tdesc = xmalloc (sizeof (struct target_desc));
+	  copy_target_description (tdesc, parent_proc->tdesc);
+	  child_proc->tdesc = tdesc;
+	  child_lwp->must_set_ptrace_flags = 1;
+
+	  /* Save fork info in the parent thread.  */
+	  event_lwp->waitstatus.kind = TARGET_WAITKIND_FORKED;
+	  event_lwp->waitstatus.value.related_pid = ptid;
+	  /* The status_pending field contains bits denoting the
+	     extended event, so when the pending event is handled,
+	     the handler will look at lwp->waitstatus.  */
+	  event_lwp->status_pending_p = 1;
+	  event_lwp->status_pending = wstat;
+
+	  /* Report the event.  */
+	  return 0;
+	}
+
       if (debug_threads)
 	debug_printf ("HEW: Got clone event "
 		      "from LWP %ld, new child is LWP %ld\n",
@@ -434,7 +487,12 @@  handle_extended_wait (struct lwp_info *event_child, int wstat)
 	  new_lwp->status_pending_p = 1;
 	  new_lwp->status_pending = status;
 	}
+
+      /* Don't report the event.  */
+      return 1;
     }
+
+  internal_error (__FILE__, __LINE__, _("unknown ptrace event %d"), event);
 }
 
 /* Return the PC as read from the regcache of LWP, without any
@@ -1828,6 +1886,19 @@  check_stopped_by_watchpoint (struct lwp_info *child)
   return child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT;
 }
 
+/* Wrapper for linux_enable_event_reporting that disables any
+   supported events that we have determined should not be
+   reported (e.g. GDB did not request them).  */
+
+static void
+linux_low_enable_events (pid_t pid, int attached)
+{
+  if (!report_fork_events)
+    linux_ptrace_clear_flags (PTRACE_O_TRACEFORK);
+
+  linux_enable_event_reporting (pid, attached);
+}
+
 /* Do low-level handling of the event, and check if we should go on
    and pass it to caller code.  Return the affected lwp if we are, or
    NULL otherwise.  */
@@ -1912,11 +1983,11 @@  linux_low_filter_event (int lwpid, int wstat)
 	}
     }
 
-  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
+  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags && gdb_connected ())
     {
       struct process_info *proc = find_process_pid (pid_of (thread));
 
-      linux_enable_event_reporting (lwpid, proc->attached);
+      linux_low_enable_events (lwpid, proc->attached);
       child->must_set_ptrace_flags = 0;
     }
 
@@ -1926,8 +1997,12 @@  linux_low_filter_event (int lwpid, int wstat)
       && linux_is_extended_waitstatus (wstat))
     {
       child->stop_pc = get_pc (child);
-      handle_extended_wait (child, wstat);
-      return NULL;
+      if (handle_extended_wait (child, wstat))
+	{
+	  /* The event has been handled, so just return without
+	     reporting it.  */
+	  return NULL;
+	}
     }
 
   /* Check first whether this was a SW/HW breakpoint before checking
@@ -2502,6 +2577,18 @@  ignore_event (struct target_waitstatus *ourstatus)
   return null_ptid;
 }
 
+/* Return non-zero if WAITSTATUS reflects an extended linux
+   event.  Otherwise, return zero.  */
+
+static int
+extended_event_reported (const struct target_waitstatus *waitstatus)
+{
+  if (waitstatus == NULL)
+    return 0;
+
+  return (waitstatus->kind == TARGET_WAITKIND_FORKED);
+}
+
 /* Wait for process, returns status.  */
 
 static ptid_t
@@ -2868,7 +2955,8 @@  linux_wait_1 (ptid_t ptid,
 		       && !bp_explains_trap && !trace_event)
 		   || (gdb_breakpoint_here (event_child->stop_pc)
 		       && gdb_condition_true_at_breakpoint (event_child->stop_pc)
-		       && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
+		       && gdb_no_commands_at_breakpoint (event_child->stop_pc))
+		   || extended_event_reported (&event_child->waitstatus));
 
   run_breakpoint_commands (event_child->stop_pc);
 
@@ -2890,6 +2978,13 @@  linux_wait_1 (ptid_t ptid,
 			  paddress (event_child->stop_pc),
 			  paddress (event_child->step_range_start),
 			  paddress (event_child->step_range_end));
+	  if (extended_event_reported (&event_child->waitstatus))
+	    {
+	      char *str = target_waitstatus_to_string (ourstatus);
+	      debug_printf ("LWP %ld: extended event with waitstatus %s\n",
+			    lwpid_of (get_lwp_thread (event_child)), str);
+	      xfree (str);
+	    }
 	}
 
       /* We're not reporting this breakpoint to GDB, so apply the
@@ -2999,7 +3094,17 @@  linux_wait_1 (ptid_t ptid,
 	unstop_all_lwps (1, event_child);
     }
 
-  ourstatus->kind = TARGET_WAITKIND_STOPPED;
+  if (extended_event_reported (&event_child->waitstatus))
+    {
+      /* If the reported event is a fork, vfork or exec, let GDB know.  */
+      ourstatus->kind = event_child->waitstatus.kind;
+      ourstatus->value = event_child->waitstatus.value;
+
+      /* Reset the event lwp's waitstatus since we handled it already.  */
+      event_child->waitstatus.kind = TARGET_WAITKIND_SPURIOUS;
+    }
+  else
+    ourstatus->kind = TARGET_WAITKIND_STOPPED;
 
   /* Now that we've selected our final event LWP, un-adjust its PC if
      it was a software breakpoint, and the client doesn't know we can
@@ -3032,7 +3137,7 @@  linux_wait_1 (ptid_t ptid,
 	 but, it stopped for other reasons.  */
       ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
     }
-  else
+  else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
     {
       ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
     }
@@ -4830,8 +4935,8 @@  linux_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
 	val = val & 0xffff;
       else if (len == 3)
 	val = val & 0xffffff;
-      debug_printf ("Writing %0*x to 0x%08lx\n", 2 * ((len < 4) ? len : 4),
-		    val, (long)memaddr);
+      debug_printf ("Writing %0*x to 0x%08lx in process %d\n",
+		    2 * ((len < 4) ? len : 4), val, (long)memaddr, pid);
     }
 
   /* Fill start and end extra bytes of buffer with existing memory data.  */
@@ -5271,6 +5376,39 @@  linux_supports_vfork_events (void)
   return linux_supports_tracefork ();
 }
 
+/* Callback for 'find_inferior'.  Set the (possibly changed) ptrace
+   options for the specified lwp.  */
+
+static int
+reset_lwp_ptrace_options_callback (struct inferior_list_entry *entry,
+				   void *args)
+{
+  struct thread_info *thread = (struct thread_info *) entry;
+  struct lwp_info *lwp = get_thread_lwp (thread);
+  struct process_info *proc = find_process_pid (pid_of (thread));
+
+  linux_low_enable_events (lwpid_of (thread), proc->attached);
+  lwp->must_set_ptrace_flags = 0;
+
+  return 0;
+}
+
+/* Target hook for 'handle_new_gdb_connection'.  Causes a reset of the
+   ptrace flags for all inferiors.  This is in case the new GDB connection
+   doesn't support the same set of events that the previous one did.  */
+
+static void
+linux_handle_new_gdb_connection (void)
+{
+  pid_t pid;
+
+  /* Reset the ptrace options to enable on the inferior(s).  */
+  linux_reset_ptrace_options ();
+
+  /* Request that all the lwps reset their ptrace options.  */
+  find_inferior (&all_threads, reset_lwp_ptrace_options_callback , &pid);
+}
+
 static int
 linux_supports_disable_randomization (void)
 {
@@ -6237,6 +6375,7 @@  static struct target_ops linux_target_ops = {
   linux_supports_multi_process,
   linux_supports_fork_events,
   linux_supports_vfork_events,
+  linux_handle_new_gdb_connection,
 #ifdef USE_THREAD_DB
   thread_db_handle_monitor_command,
 #else
@@ -6315,5 +6454,6 @@  initialize_low (void)
   initialize_low_arch ();
 
   /* Enable extended ptrace events.  */
+  linux_ptrace_set_additional_flags (PTRACE_O_TRACEFORK);
   linux_check_ptrace_features ();
 }
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index bbff3aa..28cccc7 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -270,6 +270,10 @@  struct lwp_info
   /* When stopped is set, the last wait status recorded for this lwp.  */
   int last_status;
 
+  /* This is used to store extended ptrace event information until
+     it is reported to GDB.  */
+  struct target_waitstatus waitstatus;
+
   /* When stopped is set, this is where the lwp last stopped, with
      decr_pc_after_break already accounted for.  If the LWP is
      running, this is the address at which the lwp was resumed.  */
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index 687cce0..38df1b7 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -760,6 +760,7 @@  static struct target_ops lynx_target_ops = {
   NULL,  /* supports_multi_process */
   NULL,  /* supports_fork_events */
   NULL,  /* supports_vfork_events */
+  NULL,  /* handle_new_gdb_connection */
   NULL,  /* handle_monitor_command */
 };
 
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 1de86be..dc43e38 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1114,12 +1114,25 @@  prepare_resume_reply (char *buf, ptid_t ptid,
   switch (status->kind)
     {
     case TARGET_WAITKIND_STOPPED:
+    case TARGET_WAITKIND_FORKED:
       {
 	struct thread_info *saved_thread;
 	const char **regp;
 	struct regcache *regcache;
 
-	sprintf (buf, "T%02x", status->value.sig);
+	if (status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
+	  {
+	    enum gdb_signal signal = GDB_SIGNAL_TRAP;
+	    const char *event = "xfork";
+
+	    sprintf (buf, "T%02x%s:", signal, event);
+	    buf += strlen (buf);
+	    buf = write_ptid (buf, status->value.related_pid);
+	    strcat (buf, ";");
+	  }
+	else
+	  sprintf (buf, "T%02x", status->value.sig);
+
 	buf += strlen (buf);
 
 	saved_thread = current_thread;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index ee865fa..8fa6f8a 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2104,6 +2104,9 @@  handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 
       strcat (own_buf, ";vFile:fstat+");
 
+      /* Reinitialize the target as needed for the new connection.  */
+      target_handle_new_gdb_connection ();
+
       return;
     }
 
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 91d4080..696a24e 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -84,6 +84,7 @@  extern int disable_packet_qfThreadInfo;
 
 extern int run_once;
 extern int multi_process;
+extern int report_fork_events;
 extern int non_stop;
 
 /* True if the "swbreak+" feature is active.  In that case, GDB wants
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index dc51627..ca010ff 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -283,6 +283,9 @@  struct target_ops
   /* Returns true if vfork events are supported.  */
   int (*supports_vfork_events) (void);
 
+  /* Allows target to re-initialize connection-specific settings.  */
+  void (*handle_new_gdb_connection) (void);
+
   /* If not NULL, target-specific routine to process monitor command.
      Returns 1 if handled, or 0 to perform default processing.  */
   int (*handle_monitor_command) (char *);
@@ -422,6 +425,10 @@  int kill_inferior (int);
   (the_target->supports_vfork_events ? \
    (*the_target->supports_vfork_events) () : 0)
 
+#define target_handle_new_gdb_connection() \
+  (the_target->handle_new_gdb_connection ? \
+   (*the_target->handle_new_gdb_connection) () : 0)
+
 #define detach_inferior(pid) \
   (*the_target->detach) (pid)
 
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 5f50e46..e7c04d6 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -1829,6 +1829,7 @@  static struct target_ops win32_target_ops = {
   NULL, /* supports_multi_process */
   NULL, /* supports_fork_events */
   NULL, /* supports_vfork_events */
+  NULL, /* handle_new_gdb_connection */
   NULL, /* handle_monitor_command */
   NULL, /* core_of_thread */
   NULL, /* read_loadmap */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 22ce842..3f8fe04 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4898,11 +4898,7 @@  Enables printf debugging output."),
 
   /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
      support read-only process state.  */
-  linux_ptrace_set_additional_flags (PTRACE_O_TRACESYSGOOD
-				     | PTRACE_O_TRACEVFORKDONE
-				     | PTRACE_O_TRACEVFORK
-				     | PTRACE_O_TRACEFORK
-				     | PTRACE_O_TRACEEXEC);
+  linux_ptrace_set_additional_flags (all_additional_flags);
 }
 
 
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index aba3da8..3494bcf 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -25,13 +25,16 @@ 
 
 #include <stdint.h>
 
-/* Stores the currently supported ptrace options.  A value of
-   -1 means we did not check for features yet.  A value of 0 means
-   there are no supported features.  */
+/* Stores the ptrace options currently enabled by this gdbserver.
+   A value of -1 means we did not check for features yet.  A value
+   of 0 means there are no enabled features.  */
 static int current_ptrace_options = -1;
 
-/* Additional flags to test.  */
+/* Stores the fork and exec ptrace options supported by the operating
+   system.  */
+static int supported_additional_flags = 0;
 
+/* Additional flags to test.  */
 static int additional_flags;
 
 /* Find all possible reasons we could fail to attach PID and append
@@ -344,6 +347,7 @@  linux_check_ptrace_features (void)
 
   /* Initialize the options.  */
   current_ptrace_options = 0;
+  supported_additional_flags = 0;
 
   /* Fork a child so we can do some testing.  The child will call
      linux_child_function and will get traced.  The child will
@@ -394,7 +398,7 @@  linux_test_for_tracesysgood (int child_pid)
 		(PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
 
   if (ret == 0)
-    current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
+    supported_additional_flags |= PTRACE_O_TRACESYSGOOD;
 }
 
 /* Determine if PTRACE_O_TRACEFORK can be used to follow fork
@@ -421,7 +425,7 @@  linux_test_for_tracefork (int child_pid)
 		    (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
 					| PTRACE_O_TRACEVFORKDONE));
       if (ret == 0)
-	current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
+	supported_additional_flags |= PTRACE_O_TRACEVFORKDONE;
     }
 
   /* Setting PTRACE_O_TRACEFORK did not cause an error, however we
@@ -459,9 +463,16 @@  linux_test_for_tracefork (int child_pid)
 	  /* We got the PID from the grandchild, which means fork
 	     tracing is supported.  */
 	  current_ptrace_options |= PTRACE_O_TRACECLONE;
-	  current_ptrace_options |= (additional_flags & (PTRACE_O_TRACEFORK
-                                                         | PTRACE_O_TRACEVFORK
-                                                         | PTRACE_O_TRACEEXEC));
+
+	  /* Save the "extended" options in case we need to reset
+	     the options later for a connect from a different GDB.  */
+	  supported_additional_flags |= (PTRACE_O_TRACEFORK
+					  | PTRACE_O_TRACEVFORK
+					  | PTRACE_O_TRACEEXEC);
+
+	  /* Enable only those options requested by GDB.  */
+	  current_ptrace_options |= (supported_additional_flags
+				     & additional_flags);
 
 	  /* Do some cleanup and kill the grandchild.  */
 	  my_waitpid (second_pid, &second_status, 0);
@@ -518,6 +529,25 @@  linux_enable_event_reporting (pid_t pid, int attached)
 	  (PTRACE_TYPE_ARG4) (uintptr_t) ptrace_options);
 }
 
+/* Reset the ptrace options using potentially different
+   additional_flags.  */
+
+void
+linux_reset_ptrace_options (void)
+{
+  /* Check if we have initialized the ptrace features for this
+     target.  If not, do it now.  */
+  if (current_ptrace_options == -1)
+    linux_check_ptrace_features ();
+
+  /* Clear only the "extended" options.  */
+  linux_ptrace_clear_flags (all_additional_flags);
+
+  /* Add the new requested flags back in.  */
+  current_ptrace_options |= (supported_additional_flags
+			     & additional_flags);
+}
+
 /* Disable reporting of all currently supported ptrace events.  */
 
 void
@@ -603,9 +633,20 @@  linux_ptrace_init_warnings (void)
 void
 linux_ptrace_set_additional_flags (int flags)
 {
+  int invalid_flags = (flags & ~all_additional_flags);
+
+  gdb_assert (invalid_flags == 0);
   additional_flags = flags;
 }
 
+/* Clear FLAGS in current_ptrace_options.  */
+
+void
+linux_ptrace_clear_flags (int flags)
+{
+  current_ptrace_options &= ~flags;
+}
+
 /* Extract extended ptrace event from wait status.  */
 
 int
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 03d98c9..c466a19 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -139,6 +139,15 @@  struct buffer;
 # define TRAP_HWBKPT 4
 #endif
 
+/* All of the additional ptrace flags.  The flags passed to
+   linux_ptrace_set_additional_flags must be in
+   this set of flags.  */
+static const int all_additional_flags = (PTRACE_O_TRACEFORK
+					 | PTRACE_O_TRACEVFORK
+					 | PTRACE_O_TRACEVFORKDONE
+					 | PTRACE_O_TRACEEXEC
+					 | PTRACE_O_TRACESYSGOOD);
+
 extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer);
 
 /* Find all possible reasons we could have failed to attach to PTID
@@ -151,12 +160,14 @@  extern char *linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err);
 extern void linux_ptrace_init_warnings (void);
 extern void linux_check_ptrace_features (void);
 extern void linux_enable_event_reporting (pid_t pid, int attached);
+extern void linux_reset_ptrace_options (void);
 extern void linux_disable_event_reporting (pid_t pid);
 extern int linux_supports_tracefork (void);
 extern int linux_supports_traceclone (void);
 extern int linux_supports_tracevforkdone (void);
 extern int linux_supports_tracesysgood (void);
 extern void linux_ptrace_set_additional_flags (int);
+extern void linux_ptrace_clear_flags (int flags);
 extern int linux_ptrace_get_extended_event (int wstat);
 extern int linux_is_extended_waitstatus (int wstat);
 extern int linux_wstatus_maybe_breakpoint (int wstat);
diff --git a/gdb/remote.c b/gdb/remote.c
index 85ced68..899c7a2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1470,6 +1470,14 @@  remote_multi_process_p (struct remote_state *rs)
   return packet_support (PACKET_multiprocess_feature) == PACKET_ENABLE;
 }
 
+/* Returns true if fork events are supported.  */
+
+static int
+remote_fork_event_p (struct remote_state *rs)
+{
+  return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE;
+}
+
 /* Tokens for use by the asynchronous signal handlers for SIGINT.  */
 static struct async_signal_handler *async_sigint_remote_twice_token;
 static struct async_signal_handler *async_sigint_remote_token;
@@ -4425,16 +4433,22 @@  remote_open_1 (const char *name, int from_tty,
     wait_forever_enabled_p = 1;
 }
 
-/* This takes a program previously attached to and detaches it.  After
-   this is done, GDB can be used to debug some other program.  We
-   better not have left any breakpoints in the target program or it'll
-   die when it hits one.  */
+/* This detaches a program to which we previously attached, using
+   inferior_ptid to identify the process.  After this is done, GDB
+   can be used to debug some other program.  We better not have left
+   any breakpoints in the target program or it'll die when it hits
+   one.  If IS_FORK_CHILD is true, then inferior_ptid is the child
+   of an unfollowed fork, and we need to avoid deleting breakpoints
+   still needed by the parent.  */
 
 static void
-remote_detach_1 (const char *args, int from_tty, int extended)
+remote_detach_1 (struct target_ops *ops, const char *args,
+		 int from_tty, int is_fork_child)
 {
   int pid = ptid_get_pid (inferior_ptid);
   struct remote_state *rs = get_remote_state ();
+  struct thread_info *tp = find_thread_ptid (inferior_ptid);
+  int is_fork_parent;
 
   if (args)
     error (_("Argument given to \"detach\" when remotely debugging."));
@@ -4468,22 +4482,75 @@  remote_detach_1 (const char *args, int from_tty, int extended)
   else
     error (_("Can't detach process."));
 
-  if (from_tty && !extended)
+  if (from_tty && !rs->extended)
     puts_filtered (_("Ending remote debugging.\n"));
 
-  target_mourn_inferior ();
+  /* Check to see if we are detaching a fork parent.  Note that if we
+     are detaching a fork child, tp == NULL.  */
+  if (tp != NULL)
+    is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;
+
+  /* If doing detach-on-fork, we don't mourn, because that will delete
+     breakpoints that should be available for the followed inferior.  */
+  if (!is_fork_child && !is_fork_parent)
+    target_mourn_inferior ();
+  else
+    {
+      inferior_ptid = null_ptid;
+      detach_inferior (pid);
+    }
 }
 
 static void
 remote_detach (struct target_ops *ops, const char *args, int from_tty)
 {
-  remote_detach_1 (args, from_tty, 0);
+  remote_detach_1 (ops, args, from_tty, 0);
 }
 
 static void
 extended_remote_detach (struct target_ops *ops, const char *args, int from_tty)
 {
-  remote_detach_1 (args, from_tty, 1);
+  remote_detach_1 (ops, args, from_tty, 0);
+}
+
+/* Target follow-fork function for remote targets.  On entry, and
+   at return, the current inferior is the fork parent.
+
+   Note that although this is currently only used for extended-remote,
+   it is named remote_follow_fork in anticipation of using it for the
+   remote target as well.  */
+
+static int
+remote_follow_fork (struct target_ops *ops, int follow_child,
+		    int detach_fork)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  if (remote_fork_event_p (rs))
+    {
+      if (detach_fork && !follow_child)
+	{
+	  ptid_t parent_ptid;
+	  ptid_t child_ptid;
+
+	  gdb_assert (inferior_thread ()->pending_follow.kind
+		      == TARGET_WAITKIND_FORKED);
+
+	  /* remote_detach_1 detaches inferior_ptid, which is currently
+	     the ptid of the parent.  Switch inferior_ptid to the ptid
+	     of the child for the duration of the call.  */
+	  parent_ptid = inferior_ptid;
+	  child_ptid = inferior_thread ()->pending_follow.value.related_pid;
+	  inferior_ptid = child_ptid;
+	  gdb_assert (!ptid_equal (parent_ptid, child_ptid));
+
+	  remote_detach_1 (ops, NULL, 0, 1);
+
+	  /* Restore the parent ptid.  */
+	  inferior_ptid = parent_ptid;
+	}
+    }
+  return 0;
 }
 
 /* Same as remote_detach, but don't send the "D" packet; just disconnect.  */
@@ -5652,6 +5719,11 @@  Packet: '%s'\n"),
 	      p = unpack_varlen_hex (++p1, &c);
 	      event->core = c;
 	    }
+	  else if (strncmp (p, "xfork", p1 - p) == 0)
+	    {
+	      event->ws.value.related_pid = read_ptid (++p1, &p);
+	      event->ws.kind = TARGET_WAITKIND_FORKED;
+	    }
 	  else
 	    {
 	      ULONGEST pnum;
@@ -9514,8 +9586,11 @@  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))
-	xsnprintf (buf, sizeof buf, "Thread %d.%ld",
-		   ptid_get_pid (ptid), ptid_get_lwp (ptid));
+	if (ptid_get_lwp (ptid) == 0)
+	  return normal_pid_to_str (ptid);
+	else
+	  xsnprintf (buf, sizeof buf, "Thread %d.%ld",
+		     ptid_get_pid (ptid), ptid_get_lwp (ptid));
       else
 	xsnprintf (buf, sizeof buf, "Thread %ld",
 		   ptid_get_lwp (ptid));
@@ -11938,6 +12013,7 @@  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 e95cb4b..2a71e63 100644
--- a/gdb/testsuite/gdb.base/multi-forks.exp
+++ b/gdb/testsuite/gdb.base/multi-forks.exp
@@ -62,6 +62,23 @@  proc continue_to_exit_bp_loc {} {
     set seen_break 0
     set seen_prompt 0
     set seen_timeout 0
+
+    # If we are running with a native gdbserver, the output ($decimal done)
+    # will come via the spawn_id of gdbserver, not the spawn_id of gdb (the
+    # default).  So we grab the spawn_id of gdbserver, if it exists, and
+    # add it to the gdb_expect statement below using "-i", allowing us to
+    # apply the expect statement to the output of both spawn_ids.
+    #
+    # If we are running with a truly remote gdbserver (gdb,noinferiorio),
+    # the output of the program will be inaccessible.  In this case we do
+    # not check for the ($decimal done) output, but just look for the gdb
+    # prompt.
+    global server_spawn_id
+    set current_spawn_id [board_info host fileid]
+    if {![info exists server_spawn_id]} {
+	set server_spawn_id ""
+    }
+
     while { ($seen_done < 16 || ! $seen_prompt) && ! $seen_timeout } {
 	# 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
@@ -70,7 +87,8 @@  proc continue_to_exit_bp_loc {} {
 	# first in the script that occurs anywhere in the input, so that
 	# we don't skip anything.
 	gdb_expect {
-	    -re "($decimal done)|(Breakpoint)|($gdb_prompt)" {
+	    -i "$current_spawn_id $server_spawn_id" \
+	         -re "($decimal done)|(Breakpoint)|($gdb_prompt)" {
 		if {[info exists expect_out(1,string)]} {
 		    incr seen_done
 		} elseif {[info exists expect_out(2,string)]} {
@@ -90,7 +108,7 @@  proc continue_to_exit_bp_loc {} {
 	fail "run to exit 2 (no prompt)"
     } elseif { ! $seen_break } {
 	fail "run to exit 2 (no breakpoint hit)"
-    } elseif { $seen_done != 16 } {
+    } elseif {![target_info exists gdb,noinferiorio] && $seen_done != 16 } {
 	fail "run to exit 2 (missing done messages)"
     } else {
 	pass "run to exit 2"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f274b64..2aadd2d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -29,6 +29,9 @@  load_lib libgloss.exp
 load_lib cache.exp
 load_lib gdb-utils.exp
 
+# Stores the expect spawn ID of gdbserver.
+global server_spawn_id
+
 global GDB
 
 if [info exists TOOL_EXECUTABLE] {
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 4a59154..86c6e35 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -270,6 +270,8 @@  proc gdbserver_start { options arguments } {
 	    append gdbserver_command " $arguments"
 	}
 
+	# Save the gdbserver spawn ID.
+	global server_spawn_id
 	set server_spawn_id [remote_spawn target $gdbserver_command]
 
 	# Wait for the server to open its TCP socket, so that GDB can connect.