[01/16,v2] Refactor native follow-fork

Message ID 1408580964-27916-2-git-send-email-donb@codesourcery.com
State New, archived
Headers

Commit Message

Don Breazeal Aug. 21, 2014, 12:29 a.m. UTC
  This patch reorganizes some of the code that implements follow-fork and
detach-on-fork in preparation for implementation of those features for the
extended-remote target.  The function linux-nat.c:linux_child_follow_fork
contains target-independent code mixed in with target-dependent code.  The
target-independent pieces need to be accessible for the host-side
implementation of follow-fork for extended-remote Linux targets.

The changes are fairly mechanical.  A new routine, follow_fork_inferior,
was implemented in infrun.c, containing those parts of
linux_child_follow_fork that manage inferiors and the inferior list.  The
parts of linux_child_follow_fork that deal with LWPs and target-specifics
were left in-place.  Once the target-independent pieces were removed from
linux_child_follow_fork, it was possible to consolidate it a bit.  Although
the order of some operations was changed, the resulting functionality was
not.  

Tested on x64 Ubuntu Lucid, native only.

Thanks,
--Don

gdb/
2014-08-20  Don Breazeal  <donb@codesourcery.com>

	* infrun.c (follow_fork): Call follow_fork_inferior.
	(follow_fork_inferior): New function.
	(follow_inferior_reset_breakpoints): Make function static.
	* infrun.h: Remove declaration of follow_inferior_reset_breakpoints.
	* linux-nat.c (linux_child_follow_fork): Move target-independent
	code to infrun.c:follow_fork_inferior.

---
 gdb/infrun.c    |  248 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 gdb/infrun.h    |    2 -
 gdb/linux-nat.c |  216 ++---------------------------------------------
 3 files changed, 254 insertions(+), 212 deletions(-)
  

Comments

Pedro Alves Sept. 5, 2014, 2:20 p.m. UTC | #1
linux_child_follow_fork ends up with:

static int
linux_child_follow_fork (struct target_ops *ops, int follow_child,
			 int detach_fork)
{
  int has_vforked;
  int parent_pid, child_pid;

  has_vforked = (inferior_thread ()->pending_follow.kind
		 == TARGET_WAITKIND_VFORKED);
  parent_pid = ptid_get_lwp (inferior_ptid);
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  if (parent_pid == 0)
    parent_pid = ptid_get_pid (inferior_ptid);
  child_pid
    = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);

  if (!follow_child)
    {
...
    }
  else
    {
      struct lwp_info *child_lp;

      child_lp = add_lwp (inferior_ptid);
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      child_lp->stopped = 1;
      child_lp->last_resume_kind = resume_stop;

      /* Let the thread_db layer learn about this new process.  */
      check_for_thread_db ();
    }
}

Nothing appears to switch inferior_ptid to the child, so seems
like we're adding the child_lp with the wrong lwp (and calling
check_for_thread_db in the wrong context) ?  Is this managing
to work by chance because follow_fork_inferior leaves inferior_ptid
pointing to the child?  Then this at the top uses the wrong
inferior_thread ():

  has_vforked = (inferior_thread ()->pending_follow.kind
		 == TARGET_WAITKIND_VFORKED);


and we're lucky that nothing end up using has_vforked in the
follow child path?

I'd much rather we don't have these assumptions in place.

These files / targets also have to_follow_fork implementations:

 inf-ptrace.c:  t->to_follow_fork = inf_ptrace_follow_fork;
 inf-ttrace.c:  t->to_follow_fork = inf_ttrace_follow_fork;

which will break if we don't adjust them as well.  Did you
check whether the refactored code (follow_fork_inferior)
makes sense for those?

Thanks,
Pedro Alves
  
Don Breazeal Sept. 5, 2014, 6:56 p.m. UTC | #2
Hi Pedro,
Thanks for reviewing this.

On 9/5/2014 7:20 AM, Pedro Alves wrote:
> linux_child_follow_fork ends up with:
> 
> static int
> linux_child_follow_fork (struct target_ops *ops, int follow_child,
> 			 int detach_fork)
> {
>   int has_vforked;
>   int parent_pid, child_pid;
> 
>   has_vforked = (inferior_thread ()->pending_follow.kind
> 		 == TARGET_WAITKIND_VFORKED);
>   parent_pid = ptid_get_lwp (inferior_ptid);
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   if (parent_pid == 0)
>     parent_pid = ptid_get_pid (inferior_ptid);
>   child_pid
>     = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
> 
>   if (!follow_child)
>     {
> ...
>     }
>   else
>     {
>       struct lwp_info *child_lp;
> 
>       child_lp = add_lwp (inferior_ptid);
>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>       child_lp->stopped = 1;
>       child_lp->last_resume_kind = resume_stop;
> 
>       /* Let the thread_db layer learn about this new process.  */
>       check_for_thread_db ();
>     }
> }
> 
> Nothing appears to switch inferior_ptid to the child, so seems
> like we're adding the child_lp with the wrong lwp (and calling
> check_for_thread_db in the wrong context) ?  Is this managing
> to work by chance because follow_fork_inferior leaves inferior_ptid
> pointing to the child?  

Yes, follow_fork_inferior always sets inferior_ptid to the followed
inferior.  On entry, linux_child_follow_fork expects inferior_ptid to be
the followed inferior.  So I think it is getting the correct inferior
from inferior_ptid in these cases.  I can change that if you prefer; see
my question below about acceptable solutions.

Regarding check_for_thread_db, there is something unrelated that I don't
understand here.  If we have reached this function, then aren't we
guaranteed that PTRACE_O_TRACECLONE is supported, and that we are using
that instead of libthread_db for detecting thread events?  If so, why do
we need to call check_for_thread_db at all?

Then this at the top uses the wrong
> inferior_thread ():
> 
>   has_vforked = (inferior_thread ()->pending_follow.kind
> 		 == TARGET_WAITKIND_VFORKED);
> 
> 
> and we're lucky that nothing end up using has_vforked in the
> follow child path?

You are right, this is incorrect and unnecessary in the case where we
are following the child.

> 
> I'd much rather we don't have these assumptions in place.

Would an acceptable solution be to move the definitions and assignments
of has_vforked, parent_pid, and child_pid into the follow-parent case,
as below?

Would you also prefer that on entry to linux_child_follow_fork,
inferior_ptid is set to the parent like it was before, or would a
comment explaining that inferior_ptid is expected to be the followed
inferior be sufficient?

static int
linux_child_follow_fork (struct target_ops *ops, int follow_child,
                         int detach_fork)
{
  if (!follow_child)
    {
      struct lwp_info *child_lp = NULL;
      int status = W_STOPCODE (0);
      struct cleanup *old_chain;
      int has_vforked;
      int parent_pid, child_pid;

      has_vforked = (inferior_thread ()->pending_follow.kind
                     == TARGET_WAITKIND_VFORKED);
      parent_pid = ptid_get_lwp (inferior_ptid);
      if (parent_pid == 0)
        parent_pid = ptid_get_pid (inferior_ptid);
      child_pid
        = ptid_get_pid (inferior_thread
()->pending_follow.value.related_pid);

> 
> These files / targets also have to_follow_fork implementations:
> 
>  inf-ptrace.c:  t->to_follow_fork = inf_ptrace_follow_fork;
>  inf-ttrace.c:  t->to_follow_fork = inf_ttrace_follow_fork;
> 
> which will break if we don't adjust them as well.  Did you
> check whether the refactored code (follow_fork_inferior)
> makes sense for those?

I completely missed these; sorry about that.  In theory I should be able
to make similar changes to these that maintains the existing
functionality.  I don't currently have a way (that I know of) to test
either of them.  Testing requires a non-Linux version of Unix and an
HP-UX system, correct?  I'll start work on the changes in spite of that.

> 
> Thanks,
> Pedro Alves
> 

Thanks,
--Don
  
Don Breazeal Sept. 5, 2014, 8:20 p.m. UTC | #3
One clarification...

On 9/5/2014 11:56 AM, Breazeal, Don wrote:
> Hi Pedro,
> Thanks for reviewing this.
> 
> On 9/5/2014 7:20 AM, Pedro Alves wrote:
>> linux_child_follow_fork ends up with:
>>
>> static int
>> linux_child_follow_fork (struct target_ops *ops, int follow_child,
>> 			 int detach_fork)
>> {
>>   int has_vforked;
>>   int parent_pid, child_pid;
>>
>>   has_vforked = (inferior_thread ()->pending_follow.kind
>> 		 == TARGET_WAITKIND_VFORKED);
>>   parent_pid = ptid_get_lwp (inferior_ptid);
>>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>   if (parent_pid == 0)
>>     parent_pid = ptid_get_pid (inferior_ptid);
>>   child_pid
>>     = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
>>
>>   if (!follow_child)
>>     {
>> ...
>>     }
>>   else
>>     {
>>       struct lwp_info *child_lp;
>>
>>       child_lp = add_lwp (inferior_ptid);
>>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>       child_lp->stopped = 1;
>>       child_lp->last_resume_kind = resume_stop;
>>
>>       /* Let the thread_db layer learn about this new process.  */
>>       check_for_thread_db ();
>>     }
>> }
>>
>> Nothing appears to switch inferior_ptid to the child, so seems
>> like we're adding the child_lp with the wrong lwp (and calling
>> check_for_thread_db in the wrong context) ?  Is this managing
>> to work by chance because follow_fork_inferior leaves inferior_ptid
>> pointing to the child?  
> 
> Yes, follow_fork_inferior always sets inferior_ptid to the followed
> inferior.  On entry, linux_child_follow_fork expects inferior_ptid to be
> the followed inferior.  So I think it is getting the correct inferior
> from inferior_ptid in these cases.  I can change that if you prefer; see
> my question below about acceptable solutions.

Er... I can change how inferior_ptid is passed to
linux_child_follow_fork, not whether the correct ptid is used. :-P

> 
> Regarding check_for_thread_db, there is something unrelated that I don't
> understand here.  If we have reached this function, then aren't we
> guaranteed that PTRACE_O_TRACECLONE is supported, and that we are using
> that instead of libthread_db for detecting thread events?  If so, why do
> we need to call check_for_thread_db at all?
> 
> Then this at the top uses the wrong
>> inferior_thread ():
>>
>>   has_vforked = (inferior_thread ()->pending_follow.kind
>> 		 == TARGET_WAITKIND_VFORKED);
>>
>>
>> and we're lucky that nothing end up using has_vforked in the
>> follow child path?
> 
> You are right, this is incorrect and unnecessary in the case where we
> are following the child.
> 
>>
>> I'd much rather we don't have these assumptions in place.
> 
> Would an acceptable solution be to move the definitions and assignments
> of has_vforked, parent_pid, and child_pid into the follow-parent case,
> as below?
> 
> Would you also prefer that on entry to linux_child_follow_fork,
> inferior_ptid is set to the parent like it was before, or would a
> comment explaining that inferior_ptid is expected to be the followed
> inferior be sufficient?
> 
> static int
> linux_child_follow_fork (struct target_ops *ops, int follow_child,
>                          int detach_fork)
> {
>   if (!follow_child)
>     {
>       struct lwp_info *child_lp = NULL;
>       int status = W_STOPCODE (0);
>       struct cleanup *old_chain;
>       int has_vforked;
>       int parent_pid, child_pid;
> 
>       has_vforked = (inferior_thread ()->pending_follow.kind
>                      == TARGET_WAITKIND_VFORKED);
>       parent_pid = ptid_get_lwp (inferior_ptid);
>       if (parent_pid == 0)
>         parent_pid = ptid_get_pid (inferior_ptid);
>       child_pid
>         = ptid_get_pid (inferior_thread
> ()->pending_follow.value.related_pid);
> 
>>
>> These files / targets also have to_follow_fork implementations:
>>
>>  inf-ptrace.c:  t->to_follow_fork = inf_ptrace_follow_fork;
>>  inf-ttrace.c:  t->to_follow_fork = inf_ttrace_follow_fork;
>>
>> which will break if we don't adjust them as well.  Did you
>> check whether the refactored code (follow_fork_inferior)
>> makes sense for those?
> 
> I completely missed these; sorry about that.  In theory I should be able
> to make similar changes to these that maintains the existing
> functionality.  I don't currently have a way (that I know of) to test
> either of them.  Testing requires a non-Linux version of Unix and an
> HP-UX system, correct?  I'll start work on the changes in spite of that.
> 
>>
>> Thanks,
>> Pedro Alves
>>
> 
> Thanks,
> --Don
> 
>
  
Pedro Alves Sept. 9, 2014, 10:57 a.m. UTC | #4
On 09/05/2014 07:56 PM, Breazeal, Don wrote:
> Hi Pedro,
> Thanks for reviewing this.
> 
> On 9/5/2014 7:20 AM, Pedro Alves wrote:
>> linux_child_follow_fork ends up with:
>>
>> static int
>> linux_child_follow_fork (struct target_ops *ops, int follow_child,
>> 			 int detach_fork)
>> {
>>   int has_vforked;
>>   int parent_pid, child_pid;
>>
>>   has_vforked = (inferior_thread ()->pending_follow.kind
>> 		 == TARGET_WAITKIND_VFORKED);
>>   parent_pid = ptid_get_lwp (inferior_ptid);
>>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>   if (parent_pid == 0)
>>     parent_pid = ptid_get_pid (inferior_ptid);
>>   child_pid
>>     = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
>>
>>   if (!follow_child)
>>     {
>> ...
>>     }
>>   else
>>     {
>>       struct lwp_info *child_lp;
>>
>>       child_lp = add_lwp (inferior_ptid);
>>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>       child_lp->stopped = 1;
>>       child_lp->last_resume_kind = resume_stop;
>>
>>       /* Let the thread_db layer learn about this new process.  */
>>       check_for_thread_db ();
>>     }
>> }
>>
>> Nothing appears to switch inferior_ptid to the child, so seems
>> like we're adding the child_lp with the wrong lwp (and calling
>> check_for_thread_db in the wrong context) ?  Is this managing
>> to work by chance because follow_fork_inferior leaves inferior_ptid
>> pointing to the child?  
> 
> Yes, follow_fork_inferior always sets inferior_ptid to the followed
> inferior.

Ah.

> On entry, linux_child_follow_fork expects inferior_ptid to be
> the followed inferior.  

I see.  It does make sense.

> So I think it is getting the correct inferior
> from inferior_ptid in these cases.  I can change that if you prefer; see
> my question below about acceptable solutions.
> 
> Regarding check_for_thread_db, there is something unrelated that I don't
> understand here.  If we have reached this function, then aren't we
> guaranteed that PTRACE_O_TRACECLONE is supported, and that we are using
> that instead of libthread_db for detecting thread events?  If so, why do
> we need to call check_for_thread_db at all?

Unlike GDBserver, GDB actually still use libthread_db's create/exit event
breakpoints even if PTRACE_O_TRACECLONE is supported.  I think this is
just historical at this point, and we could skip those event breakpoints,
optimizing out a bunch of internal stops.

The check_for_thread_db call has another effect -- as mentioned in the
comment:

      /* Let the thread_db layer learn about this new process.  */
      check_for_thread_db ();

if we don't call it, then linux-thread-db.c never adds the child
process to its global thread_db_list list:

/* List of known processes using thread_db, and the required
   bookkeeping.  */
struct thread_db_info *thread_db_list;

We don't really need to try all the available libthread_db's found
in the path, we could just try try_thread_db_load with the same
libthread_db we had loaded for the parent, or even skip that
and share/refcount the bookkeeping in 'struct thread_db_info'
(dlopen handle, functions pointers, etc.) between parent and child.

This is about the same issue as mentioned just above:

	  /* Let the shared library layer (solib-svr4) learn about
	     this new process, relocate the cloned exec, pull in
	     shared libraries, and install the solib event breakpoint.
	     If a "cloned-VM" event was propagated better throughout
	     the core, this wouldn't be required.  */

> 
> Then this at the top uses the wrong
>> inferior_thread ():
>>
>>   has_vforked = (inferior_thread ()->pending_follow.kind
>> 		 == TARGET_WAITKIND_VFORKED);
>>
>>
>> and we're lucky that nothing end up using has_vforked in the
>> follow child path?
> 
> You are right, this is incorrect and unnecessary in the case where we
> are following the child.
> 
>>
>> I'd much rather we don't have these assumptions in place.
> 
> Would an acceptable solution be to move the definitions and assignments
> of has_vforked, parent_pid, and child_pid into the follow-parent case,
> as below?

Yes.

> 
> Would you also prefer that on entry to linux_child_follow_fork,
> inferior_ptid is set to the parent like it was before, or would a
> comment explaining that inferior_ptid is expected to be the followed
> inferior be sufficient?

The comment would be sufficient.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index c18267f..a51c759 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -60,6 +60,7 @@ 
 #include "completer.h"
 #include "target-descriptions.h"
 #include "target-dcache.h"
+#include "terminal.h"
 
 /* Prototypes for local functions */
 
@@ -79,6 +80,10 @@  static int restore_selected_frame (void *);
 
 static int follow_fork (void);
 
+static int follow_fork_inferior (int follow_child, int detach_fork);
+
+static void follow_inferior_reset_breakpoints (void);
+
 static void set_schedlock_func (char *args, int from_tty,
 				struct cmd_list_element *c);
 
@@ -486,9 +491,11 @@  follow_fork (void)
 	parent = inferior_ptid;
 	child = tp->pending_follow.value.related_pid;
 
-	/* Tell the target to do whatever is necessary to follow
-	   either parent or child.  */
-	if (target_follow_fork (follow_child, detach_fork))
+	/* Set up inferior(s) as specified by the caller, and tell the
+	   target to do whatever is necessary to follow either parent
+	   or child.  */
+	if (follow_fork_inferior (follow_child, detach_fork)
+	    || target_follow_fork (follow_child, detach_fork))
 	  {
 	    /* Target refused to follow, or there's some other reason
 	       we shouldn't resume.  */
@@ -560,7 +567,240 @@  follow_fork (void)
   return should_resume;
 }
 
-void
+/* Handle changes to the inferior list based on the type of fork,
+   which process is being followed, and whether the other process
+   should be detached.  */
+
+static int
+follow_fork_inferior (int follow_child, int detach_fork)
+{
+  int has_vforked;
+  int parent_pid, child_pid;
+
+  has_vforked = (inferior_thread ()->pending_follow.kind
+		 == TARGET_WAITKIND_VFORKED);
+  parent_pid = ptid_get_lwp (inferior_ptid);
+  if (parent_pid == 0)
+    parent_pid = ptid_get_pid (inferior_ptid);
+  child_pid
+    = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
+
+  if (has_vforked
+      && !non_stop /* Non-stop always resumes both branches.  */
+      && (!target_is_async_p () || sync_execution)
+      && !(follow_child || detach_fork || sched_multi))
+    {
+      /* The parent stays blocked inside the vfork syscall until the
+	 child execs or exits.  If we don't let the child run, then
+	 the parent stays blocked.  If we're telling the parent to run
+	 in the foreground, the user will not be able to ctrl-c to get
+	 back the terminal, effectively hanging the debug session.  */
+      fprintf_filtered (gdb_stderr, _("\
+Can not resume the parent process over vfork in the foreground while\n\
+holding the child stopped.  Try \"set detach-on-fork\" or \
+\"set schedule-multiple\".\n"));
+      /* FIXME output string > 80 columns.  */
+      return 1;
+    }
+
+  if (!follow_child)
+    {
+      /* Detach new forked process?  */
+      if (detach_fork)
+	{
+	  struct cleanup *old_chain;
+
+	  /* Before detaching from the child, remove all breakpoints
+	     from it.  If we forked, then this has already been taken
+	     care of by infrun.c.  If we vforked however, any
+	     breakpoint inserted in the parent is visible in the
+	     child, even those added while stopped in a vfork
+	     catchpoint.  This will remove the breakpoints from the
+	     parent also, but they'll be reinserted below.  */
+	  if (has_vforked)
+	    {
+	      /* Keep breakpoints list in sync.  */
+	      remove_breakpoints_pid (ptid_get_pid (inferior_ptid));
+	    }
+
+	  if (info_verbose || debug_infrun)
+	    {
+	      target_terminal_ours ();
+	      fprintf_filtered (gdb_stdlog,
+				"Detaching after fork from "
+				"child process %d.\n",
+				child_pid);
+	    }
+	}
+      else
+	{
+	  struct inferior *parent_inf, *child_inf;
+	  struct cleanup *old_chain;
+
+	  /* Add process to GDB's tables.  */
+	  child_inf = add_inferior (child_pid);
+
+	  parent_inf = current_inferior ();
+	  child_inf->attach_flag = parent_inf->attach_flag;
+	  copy_terminal_info (child_inf, parent_inf);
+	  child_inf->gdbarch = parent_inf->gdbarch;
+	  copy_inferior_target_desc_info (child_inf, parent_inf);
+
+	  old_chain = save_inferior_ptid ();
+	  save_current_program_space ();
+
+	  inferior_ptid = ptid_build (child_pid, child_pid, 0);
+	  add_thread (inferior_ptid);
+	  child_inf->symfile_flags = SYMFILE_NO_READ;
+
+	  /* If this is a vfork child, then the address-space is
+	     shared with the parent.  */
+	  if (has_vforked)
+	    {
+	      child_inf->pspace = parent_inf->pspace;
+	      child_inf->aspace = parent_inf->aspace;
+
+	      /* The parent will be frozen until the child is done
+		 with the shared region.  Keep track of the
+		 parent.  */
+	      child_inf->vfork_parent = parent_inf;
+	      child_inf->pending_detach = 0;
+	      parent_inf->vfork_child = child_inf;
+	      parent_inf->pending_detach = 0;
+	    }
+	  else
+	    {
+	      child_inf->aspace = new_address_space ();
+	      child_inf->pspace = add_program_space (child_inf->aspace);
+	      child_inf->removable = 1;
+	      set_current_program_space (child_inf->pspace);
+	      clone_program_space (child_inf->pspace, parent_inf->pspace);
+
+	      /* Let the shared library layer (solib-svr4) learn about
+		 this new process, relocate the cloned exec, pull in
+		 shared libraries, and install the solib event
+		 breakpoint.  If a "cloned-VM" event was propagated
+		 better throughout the core, this wouldn't be
+		 required.  */
+	      solib_create_inferior_hook (0);
+	    }
+
+	  do_cleanups (old_chain);
+	}
+
+      if (has_vforked)
+	{
+	  struct inferior *parent_inf;
+
+	  parent_inf = current_inferior ();
+
+	  /* If we detached from the child, then we have to be careful
+	     to not insert breakpoints in the parent until the child
+	     is done with the shared memory region.  However, if we're
+	     staying attached to the child, then we can and should
+	     insert breakpoints, so that we can debug it.  A
+	     subsequent child exec or exit is enough to know when does
+	     the child stops using the parent's address space.  */
+	  parent_inf->waiting_for_vfork_done = detach_fork;
+	  parent_inf->pspace->breakpoints_not_allowed = detach_fork;
+	}
+    }
+  else
+    {
+      /* Follow the child.  */
+      struct inferior *parent_inf, *child_inf;
+      struct program_space *parent_pspace;
+
+      if (info_verbose || debug_infrun)
+	{
+	  target_terminal_ours ();
+	  if (has_vforked)
+	    fprintf_filtered (gdb_stdlog,
+			      _("Attaching after process %d "
+				"vfork to child process %d.\n"),
+			      parent_pid, child_pid);
+	  else
+	    fprintf_filtered (gdb_stdlog,
+			      _("Attaching after process %d "
+				"fork to child process %d.\n"),
+			      parent_pid, child_pid);
+	}
+
+      /* Add the new inferior first, so that the target_detach below
+	 doesn't unpush the target.  */
+
+      child_inf = add_inferior (child_pid);
+
+      parent_inf = current_inferior ();
+      child_inf->attach_flag = parent_inf->attach_flag;
+      copy_terminal_info (child_inf, parent_inf);
+      child_inf->gdbarch = parent_inf->gdbarch;
+      copy_inferior_target_desc_info (child_inf, parent_inf);
+
+      parent_pspace = parent_inf->pspace;
+
+      /* If we're vforking, we want to hold on to the parent until the
+	 child exits or execs.  At child exec or exit time we can
+	 remove the old breakpoints from the parent and detach or
+	 resume debugging it.  Otherwise, detach the parent now; we'll
+	 want to reuse it's program/address spaces, but we can't set
+	 them to the child before removing breakpoints from the
+	 parent, otherwise, the breakpoints module could decide to
+	 remove breakpoints from the wrong process (since they'd be
+	 assigned to the same address space).  */
+
+      if (has_vforked)
+	{
+	  gdb_assert (child_inf->vfork_parent == NULL);
+	  gdb_assert (parent_inf->vfork_child == NULL);
+	  child_inf->vfork_parent = parent_inf;
+	  child_inf->pending_detach = 0;
+	  parent_inf->vfork_child = child_inf;
+	  parent_inf->pending_detach = detach_fork;
+	  parent_inf->waiting_for_vfork_done = 0;
+	}
+      else if (detach_fork)
+	target_detach (NULL, 0);
+
+      /* Note that the detach above makes PARENT_INF dangling.  */
+
+      /* Add the child thread to the appropriate lists, and switch to
+	 this new thread, before cloning the program space, and
+	 informing the solib layer about this new process.  */
+
+      inferior_ptid = ptid_build (child_pid, child_pid, 0);
+      add_thread (inferior_ptid);
+
+      /* If this is a vfork child, then the address-space is shared
+	 with the parent.  If we detached from the parent, then we can
+	 reuse the parent's program/address spaces.  */
+      if (has_vforked || detach_fork)
+	{
+	  child_inf->pspace = parent_pspace;
+	  child_inf->aspace = child_inf->pspace->aspace;
+	}
+      else
+	{
+	  child_inf->aspace = new_address_space ();
+	  child_inf->pspace = add_program_space (child_inf->aspace);
+	  child_inf->removable = 1;
+	  child_inf->symfile_flags = SYMFILE_NO_READ;
+	  set_current_program_space (child_inf->pspace);
+	  clone_program_space (child_inf->pspace, parent_pspace);
+
+	  /* Let the shared library layer (solib-svr4) learn about
+	     this new process, relocate the cloned exec, pull in
+	     shared libraries, and install the solib event breakpoint.
+	     If a "cloned-VM" event was propagated better throughout
+	     the core, this wouldn't be required.  */
+	  solib_create_inferior_hook (0);
+	}
+    }
+
+  return 0;
+}
+
+static void
 follow_inferior_reset_breakpoints (void)
 {
   struct thread_info *tp = inferior_thread ();
diff --git a/gdb/infrun.h b/gdb/infrun.h
index cc9cb33..fb6276b 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -115,8 +115,6 @@  extern void insert_step_resume_breakpoint_at_sal (struct gdbarch *,
 						  struct symtab_and_line ,
 						  struct frame_id);
 
-extern void follow_inferior_reset_breakpoints (void);
-
 /* Returns true if we're trying to step past the instruction at
    ADDRESS in ASPACE.  */
 extern int stepping_past_instruction_at (struct address_space *aspace,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 1e8991d..ab287bf 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -54,7 +54,6 @@ 
 #include <sys/types.h>
 #include <dirent.h>
 #include "xml-support.h"
-#include "terminal.h"
 #include <sys/vfs.h>
 #include "solib.h"
 #include "nat/linux-osdata.h"
@@ -384,64 +383,22 @@  linux_child_follow_fork (struct target_ops *ops, int follow_child,
   child_pid
     = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
 
-  if (has_vforked
-      && !non_stop /* Non-stop always resumes both branches.  */
-      && (!target_is_async_p () || sync_execution)
-      && !(follow_child || detach_fork || sched_multi))
-    {
-      /* The parent stays blocked inside the vfork syscall until the
-	 child execs or exits.  If we don't let the child run, then
-	 the parent stays blocked.  If we're telling the parent to run
-	 in the foreground, the user will not be able to ctrl-c to get
-	 back the terminal, effectively hanging the debug session.  */
-      fprintf_filtered (gdb_stderr, _("\
-Can not resume the parent process over vfork in the foreground while\n\
-holding the child stopped.  Try \"set detach-on-fork\" or \
-\"set schedule-multiple\".\n"));
-      /* FIXME output string > 80 columns.  */
-      return 1;
-    }
-
-  if (! follow_child)
+  if (!follow_child)
     {
       struct lwp_info *child_lp = NULL;
+      int status = W_STOPCODE (0);
+      struct cleanup *old_chain;
 
       /* We're already attached to the parent, by default.  */
+      old_chain = save_inferior_ptid ();
+      inferior_ptid = ptid_build (child_pid, child_pid, 0);
+      child_lp = add_lwp (inferior_ptid);
+      child_lp->stopped = 1;
+      child_lp->last_resume_kind = resume_stop;
 
       /* Detach new forked process?  */
       if (detach_fork)
 	{
-	  struct cleanup *old_chain;
-	  int status = W_STOPCODE (0);
-
-	  /* Before detaching from the child, remove all breakpoints
-	     from it.  If we forked, then this has already been taken
-	     care of by infrun.c.  If we vforked however, any
-	     breakpoint inserted in the parent is visible in the
-	     child, even those added while stopped in a vfork
-	     catchpoint.  This will remove the breakpoints from the
-	     parent also, but they'll be reinserted below.  */
-	  if (has_vforked)
-	    {
-	      /* keep breakpoints list in sync.  */
-	      remove_breakpoints_pid (ptid_get_pid (inferior_ptid));
-	    }
-
-	  if (info_verbose || debug_linux_nat)
-	    {
-	      target_terminal_ours ();
-	      fprintf_filtered (gdb_stdlog,
-				"Detaching after fork from "
-				"child process %d.\n",
-				child_pid);
-	    }
-
-	  old_chain = save_inferior_ptid ();
-	  inferior_ptid = ptid_build (child_pid, child_pid, 0);
-
-	  child_lp = add_lwp (inferior_ptid);
-	  child_lp->stopped = 1;
-	  child_lp->last_resume_kind = resume_stop;
 	  make_cleanup (delete_lwp_cleanup, child_lp);
 
 	  if (linux_nat_prepare_to_resume != NULL)
@@ -480,82 +437,15 @@  holding the child stopped.  Try \"set detach-on-fork\" or \
 	}
       else
 	{
-	  struct inferior *parent_inf, *child_inf;
-	  struct cleanup *old_chain;
-
-	  /* Add process to GDB's tables.  */
-	  child_inf = add_inferior (child_pid);
-
-	  parent_inf = current_inferior ();
-	  child_inf->attach_flag = parent_inf->attach_flag;
-	  copy_terminal_info (child_inf, parent_inf);
-	  child_inf->gdbarch = parent_inf->gdbarch;
-	  copy_inferior_target_desc_info (child_inf, parent_inf);
-
-	  old_chain = save_inferior_ptid ();
-	  save_current_program_space ();
-
-	  inferior_ptid = ptid_build (child_pid, child_pid, 0);
-	  add_thread (inferior_ptid);
-	  child_lp = add_lwp (inferior_ptid);
-	  child_lp->stopped = 1;
-	  child_lp->last_resume_kind = resume_stop;
-	  child_inf->symfile_flags = SYMFILE_NO_READ;
-
-	  /* If this is a vfork child, then the address-space is
-	     shared with the parent.  */
-	  if (has_vforked)
-	    {
-	      child_inf->pspace = parent_inf->pspace;
-	      child_inf->aspace = parent_inf->aspace;
-
-	      /* The parent will be frozen until the child is done
-		 with the shared region.  Keep track of the
-		 parent.  */
-	      child_inf->vfork_parent = parent_inf;
-	      child_inf->pending_detach = 0;
-	      parent_inf->vfork_child = child_inf;
-	      parent_inf->pending_detach = 0;
-	    }
-	  else
-	    {
-	      child_inf->aspace = new_address_space ();
-	      child_inf->pspace = add_program_space (child_inf->aspace);
-	      child_inf->removable = 1;
-	      set_current_program_space (child_inf->pspace);
-	      clone_program_space (child_inf->pspace, parent_inf->pspace);
-
-	      /* Let the shared library layer (solib-svr4) learn about
-		 this new process, relocate the cloned exec, pull in
-		 shared libraries, and install the solib event
-		 breakpoint.  If a "cloned-VM" event was propagated
-		 better throughout the core, this wouldn't be
-		 required.  */
-	      solib_create_inferior_hook (0);
-	    }
-
 	  /* Let the thread_db layer learn about this new process.  */
 	  check_for_thread_db ();
-
-	  do_cleanups (old_chain);
 	}
 
+      do_cleanups (old_chain);
+
       if (has_vforked)
 	{
 	  struct lwp_info *parent_lp;
-	  struct inferior *parent_inf;
-
-	  parent_inf = current_inferior ();
-
-	  /* If we detached from the child, then we have to be careful
-	     to not insert breakpoints in the parent until the child
-	     is done with the shared memory region.  However, if we're
-	     staying attached to the child, then we can and should
-	     insert breakpoints, so that we can debug it.  A
-	     subsequent child exec or exit is enough to know when does
-	     the child stops using the parent's address space.  */
-	  parent_inf->waiting_for_vfork_done = detach_fork;
-	  parent_inf->pspace->breakpoints_not_allowed = detach_fork;
 
 	  parent_lp = find_lwp_pid (pid_to_ptid (parent_pid));
 	  gdb_assert (linux_supports_tracefork () >= 0);
@@ -628,98 +518,12 @@  holding the child stopped.  Try \"set detach-on-fork\" or \
     }
   else
     {
-      struct inferior *parent_inf, *child_inf;
       struct lwp_info *child_lp;
-      struct program_space *parent_pspace;
-
-      if (info_verbose || debug_linux_nat)
-	{
-	  target_terminal_ours ();
-	  if (has_vforked)
-	    fprintf_filtered (gdb_stdlog,
-			      _("Attaching after process %d "
-				"vfork to child process %d.\n"),
-			      parent_pid, child_pid);
-	  else
-	    fprintf_filtered (gdb_stdlog,
-			      _("Attaching after process %d "
-				"fork to child process %d.\n"),
-			      parent_pid, child_pid);
-	}
-
-      /* Add the new inferior first, so that the target_detach below
-	 doesn't unpush the target.  */
-
-      child_inf = add_inferior (child_pid);
-
-      parent_inf = current_inferior ();
-      child_inf->attach_flag = parent_inf->attach_flag;
-      copy_terminal_info (child_inf, parent_inf);
-      child_inf->gdbarch = parent_inf->gdbarch;
-      copy_inferior_target_desc_info (child_inf, parent_inf);
-
-      parent_pspace = parent_inf->pspace;
-
-      /* If we're vforking, we want to hold on to the parent until the
-	 child exits or execs.  At child exec or exit time we can
-	 remove the old breakpoints from the parent and detach or
-	 resume debugging it.  Otherwise, detach the parent now; we'll
-	 want to reuse it's program/address spaces, but we can't set
-	 them to the child before removing breakpoints from the
-	 parent, otherwise, the breakpoints module could decide to
-	 remove breakpoints from the wrong process (since they'd be
-	 assigned to the same address space).  */
-
-      if (has_vforked)
-	{
-	  gdb_assert (child_inf->vfork_parent == NULL);
-	  gdb_assert (parent_inf->vfork_child == NULL);
-	  child_inf->vfork_parent = parent_inf;
-	  child_inf->pending_detach = 0;
-	  parent_inf->vfork_child = child_inf;
-	  parent_inf->pending_detach = detach_fork;
-	  parent_inf->waiting_for_vfork_done = 0;
-	}
-      else if (detach_fork)
-	target_detach (NULL, 0);
 
-      /* Note that the detach above makes PARENT_INF dangling.  */
-
-      /* Add the child thread to the appropriate lists, and switch to
-	 this new thread, before cloning the program space, and
-	 informing the solib layer about this new process.  */
-
-      inferior_ptid = ptid_build (child_pid, child_pid, 0);
-      add_thread (inferior_ptid);
       child_lp = add_lwp (inferior_ptid);
       child_lp->stopped = 1;
       child_lp->last_resume_kind = resume_stop;
 
-      /* If this is a vfork child, then the address-space is shared
-	 with the parent.  If we detached from the parent, then we can
-	 reuse the parent's program/address spaces.  */
-      if (has_vforked || detach_fork)
-	{
-	  child_inf->pspace = parent_pspace;
-	  child_inf->aspace = child_inf->pspace->aspace;
-	}
-      else
-	{
-	  child_inf->aspace = new_address_space ();
-	  child_inf->pspace = add_program_space (child_inf->aspace);
-	  child_inf->removable = 1;
-	  child_inf->symfile_flags = SYMFILE_NO_READ;
-	  set_current_program_space (child_inf->pspace);
-	  clone_program_space (child_inf->pspace, parent_pspace);
-
-	  /* Let the shared library layer (solib-svr4) learn about
-	     this new process, relocate the cloned exec, pull in
-	     shared libraries, and install the solib event breakpoint.
-	     If a "cloned-VM" event was propagated better throughout
-	     the core, this wouldn't be required.  */
-	  solib_create_inferior_hook (0);
-	}
-
       /* Let the thread_db layer learn about this new process.  */
       check_for_thread_db ();
     }