Fix for gdb/PR 14808, vfork/exec inferior problem

  From: Don Breazeal <donb@codesourcery.com>

Hi,

This patch is a fix for gdb/PR14808: "vfork, follow-fork child,
detach-on-fork on, child execs, parent changes executable too (but should
not)".  The problem was that after a vfork the parent and child inferiors
shared their pspace members (program space), reflecting the reality of
the processes' shared address space.  When the child exec'd, and the
parent was detached, the pspace continued to be shared.  The executable
file name is stored in the pspace, so when the child's exec file name
was updated after the exec, the parent's was also updated.

The solution was to create a separate program space for the parent, so
that its exec file name remains intact.

This patch also includes a test for this scenario.

thanks
--Don

gdb/
2014-05-12  Don Breazeal  <donb@codesourcery.com>

	* infrun.c (handle_vfork_child_exec_or_exit): For the case
	of a vfork where we follow the child and detach the parent,
	and the child execs, create a new pspace and aspace for the
	parent inferior.

gdb/testsuite
2014-05-12  Don Breazeal  <donb@codesourcery.com>

	* gdb.base/foll-vfork.exp (vfork_relations_in_info_inferiors):
	Test that after a vfork and child exec, the parent's exec file
	name has not been changed.

---
 gdb/infrun.c                          |   42 ++++++++++++++++++++++-----------
 gdb/testsuite/gdb.base/foll-vfork.exp |   11 ++++++++
 2 files changed, 39 insertions(+), 14 deletions(-)
  

05/13/2014 07:34 AM, donb@codesourcery.com wrote:
> @@ -649,6 +649,7 @@ handle_vfork_child_exec_or_exit (int exec)
>  	  struct cleanup *old_chain;
>  	  struct program_space *pspace;
>  	  struct address_space *aspace;
> +	  struct inferior *parent_inf;
>  

Local parent_inf is only used in the "if (exec)" block below, so better
to declare it there.

>  	  /* follow-fork child, detach-on-fork on.  */
>  
> @@ -665,27 +666,39 @@ handle_vfork_child_exec_or_exit (int exec)
>  	  else
>  	    old_chain = save_current_space_and_thread ();
>  
> -	  /* We're letting loose of the parent.  */
> +	  /* Make the parent the current inferior for target_detach.  */
>  	  tp = any_live_thread_of_process (inf->vfork_parent->pid);
>  	  switch_to_thread (tp->ptid);
>  
> -	  /* We're about to detach from the parent, which implicitly
> -	     removes breakpoints from its address space.  There's a
> -	     catch here: we want to reuse the spaces for the child,
> -	     but, parent/child are still sharing the pspace at this
> -	     point, although the exec in reality makes the kernel give
> -	     the child a fresh set of new pages.  The problem here is
> -	     that the breakpoints module being unaware of this, would
> -	     likely chose the child process to write to the parent
> -	     address space.  Swapping the child temporarily away from
> -	     the spaces has the desired effect.  Yes, this is "sort
> -	     of" a hack.  */
> -
> +	  /* The child inferior INF may be dead, so avoid giving the
> +	     breakpoints module the option to write through to it
> +	     by swapping the child temporarily away from the spaces
> +	     (cloning a program space resets breakpoints).  */
>  	  pspace = inf->pspace;
>  	  aspace = inf->aspace;
>  	  inf->aspace = NULL;
>  	  inf->pspace = NULL;
>  
> +	  if (exec)
> +	    {
> +	      /* The parent and child inferiors have been sharing
> +		 program and address space structures from the point
> +		 where the parent called vfork.  Now that the child has
> +		 called exec and we are detaching from the parent, the
> +		 parent inferior needs to have its own pspace and aspace

parent inferior has its own pspace, but may not have its own aspace,
depending on gdbarch_has_shared_address_space.

> +		 so that changes in the child don't affect it.  We have
> +		 to give the new spaces to the parent since we saved the
> +		 child's spaces as the current spaces above.  Even though
> +		 we are detaching the parent, we want to keep the
> +		 corresponding entry in the inferiors list intact.  */
> +	      parent_inf = current_inferior ();
> +	      parent_inf->aspace = new_address_space ();

Rather than creating a new address space, use maybe_new_address_space, like
> +	      parent_inf->pspace = add_program_space (parent_inf->aspace);

	      parent_inf->pspace
		= add_program_space (maybe_new_address_space ());
	      parent_inf->aspace = parent_inf->pspace->aspace;

> +	      parent_inf->removable = inf->removable;

Field removable of parent inferior should be unchanged, IMO.

> +	      set_current_program_space (parent_inf->pspace);
> +	      clone_program_space (parent_inf->pspace, pspace);

Do we need to unlink parent and child?  I am not very sure.

	      /* Break the bonds.  */
	      inf->vfork_parent->vfork_child = NULL;