[pushed][PATCH 4/7] Arch-specific remote follow fork

Vidya Praveen vidyapraveen@arm.com
Thu May 14 10:25:00 GMT 2015


Don,

There are several instances of xyz->private->arch_private where xyz
is (struct process_info*). Assume it is a typo (should've been priv
instead of private)? 

I atleast see aarch64 build of this fail.

Regards
VP.


On Tue, May 12, 2015 at 06:28:12PM +0100, Don Breazeal wrote:
> This is the version of the patch that I pushed.
> Thanks
> --Don
> 
> 
> This patch implements the architecture-specific pieces of follow-fork
> for remote and extended-remote Linux targets, which in the current
> implementation copyies the parent's debug register state into the new
> child's data structures.  This is required for x86, arm, aarch64, and
> mips.
> 
> This follows the native implementation as closely as possible by
> implementing a new linux_target_ops function 'new_fork', which is
> analogous to 'linux_nat_new_fork' in linux-nat.c.  In gdbserver, the debug
> registers are stored in the process list, instead of an
> architecture-specific list, so the function arguments are process_info
> pointers instead of an lwp_info and a pid as in the native implementation.
> 
> In the MIPS implementation the debug register mirror is stored differently
> from x86, ARM, and aarch64, so instead of doing a simple structure assignment
> I had to clone the list of watchpoint structures.
> 
> Tested using gdb.threads/watchpoint-fork.exp on x86, and ran manual tests
> on a MIPS board and an ARM board.  Aarch64 hasn't been tested.
> 
> gdb/gdbserver/ChangeLog:
> 
>         * linux-aarch64-low.c (aarch64_linux_new_fork): New function.
>         (the_low_target) <new_fork>: Initialize new member.
>         * linux-arm-low.c (arm_new_fork): New function.
>         (the_low_target) <new_fork>: Initialize new member.
>         * linux-low.c (handle_extended_wait): Call new target function
>         new_fork.
>         * linux-low.h (struct linux_target_ops) <new_fork>: New member.
>         * linux-mips-low.c (mips_add_watchpoint): New function
>         extracted from mips_insert_point.
>         (the_low_target) <new_fork>: Initialize new member.
>         (mips_linux_new_fork): New function.
>         (mips_insert_point): Call mips_add_watchpoint.
>         * linux-x86-low.c (x86_linux_new_fork): New function.
>         (the_low_target) <new_fork>: Initialize new member.
> ---
>  gdb/gdbserver/ChangeLog           | 17 +++++++++
>  gdb/gdbserver/linux-aarch64-low.c | 28 +++++++++++++++
>  gdb/gdbserver/linux-arm-low.c     | 42 ++++++++++++++++++++++
>  gdb/gdbserver/linux-low.c         |  4 +++
>  gdb/gdbserver/linux-low.h         |  3 ++
>  gdb/gdbserver/linux-mips-low.c    | 76 ++++++++++++++++++++++++++++++++-------
>  gdb/gdbserver/linux-x86-low.c     | 29 +++++++++++++++
>  7 files changed, 187 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
> index 0ef695c..0bdcdf3 100644
> --- a/gdb/gdbserver/ChangeLog
> +++ b/gdb/gdbserver/ChangeLog
> @@ -1,5 +1,22 @@
>  2015-05-12  Don Breazeal  <donb@codesourcery.com>
> 
> +       * linux-aarch64-low.c (aarch64_linux_new_fork): New function.
> +       (the_low_target) <new_fork>: Initialize new member.
> +       * linux-arm-low.c (arm_new_fork): New function.
> +       (the_low_target) <new_fork>: Initialize new member.
> +       * linux-low.c (handle_extended_wait): Call new target function
> +       new_fork.
> +       * linux-low.h (struct linux_target_ops) <new_fork>: New member.
> +       * linux-mips-low.c (mips_add_watchpoint): New function
> +       extracted from mips_insert_point.
> +       (the_low_target) <new_fork>: Initialize new member.
> +       (mips_linux_new_fork): New function.
> +       (mips_insert_point): Call mips_add_watchpoint.
> +       * linux-x86-low.c (x86_linux_new_fork): New function.
> +       (the_low_target) <new_fork>: Initialize new member.
> +
> +2015-05-12  Don Breazeal  <donb@codesourcery.com>
> +
>         * linux-low.c (handle_extended_wait): Implement return value,
>         rename argument 'event_child' to 'event_lwp', handle
>         PTRACE_EVENT_FORK, call internal_error for unrecognized event.
> diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
> index d44175e..7e6e9ac 100644
> --- a/gdb/gdbserver/linux-aarch64-low.c
> +++ b/gdb/gdbserver/linux-aarch64-low.c
> @@ -1135,6 +1135,33 @@ aarch64_linux_new_thread (struct lwp_info *lwp)
>    lwp->arch_private = info;
>  }
> 
> +static void
> +aarch64_linux_new_fork (struct process_info *parent,
> +                       struct process_info *child)
> +{
> +  /* These are allocated by linux_add_process.  */
> +  gdb_assert (parent->private != NULL
> +             && parent->private->arch_private != NULL);
> +  gdb_assert (child->private != NULL
> +             && child->private->arch_private != NULL);
> +
> +  /* Linux kernel before 2.6.33 commit
> +     72f674d203cd230426437cdcf7dd6f681dad8b0d
> +     will inherit hardware debug registers from parent
> +     on fork/vfork/clone.  Newer Linux kernels create such tasks with
> +     zeroed debug registers.
> +
> +     GDB core assumes the child inherits the watchpoints/hw
> +     breakpoints of the parent, and will remove them all from the
> +     forked off process.  Copy the debug registers mirrors into the
> +     new process so that all breakpoints and watchpoints can be
> +     removed together.  The debug registers mirror will become zeroed
> +     in the end before detaching the forked off process, thus making
> +     this compatible with older Linux kernels too.  */
> +
> +  *child->private->arch_private = *parent->private->arch_private;
> +}
> +
>  /* Called when resuming a thread.
>     If the debug regs have changed, update the thread's copies.  */
> 
> @@ -1299,6 +1326,7 @@ struct linux_target_ops the_low_target =
>    NULL,
>    aarch64_linux_new_process,
>    aarch64_linux_new_thread,
> +  aarch64_linux_new_fork,
>    aarch64_linux_prepare_to_resume,
>  };
> 
> diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
> index d408dcd..3420dea 100644
> --- a/gdb/gdbserver/linux-arm-low.c
> +++ b/gdb/gdbserver/linux-arm-low.c
> @@ -717,6 +717,47 @@ arm_new_thread (struct lwp_info *lwp)
>    lwp->arch_private = info;
>  }
> 
> +static void
> +arm_new_fork (struct process_info *parent, struct process_info *child)
> +{
> +  struct arch_process_info *parent_proc_info = parent->private->arch_private;
> +  struct arch_process_info *child_proc_info = child->private->arch_private;
> +  struct lwp_info *child_lwp;
> +  struct arch_lwp_info *child_lwp_info;
> +  int i;
> +
> +  /* These are allocated by linux_add_process.  */
> +  gdb_assert (parent->private != NULL
> +             && parent->private->arch_private != NULL);
> +  gdb_assert (child->private != NULL
> +             && child->private->arch_private != NULL);
> +
> +  /* Linux kernel before 2.6.33 commit
> +     72f674d203cd230426437cdcf7dd6f681dad8b0d
> +     will inherit hardware debug registers from parent
> +     on fork/vfork/clone.  Newer Linux kernels create such tasks with
> +     zeroed debug registers.
> +
> +     GDB core assumes the child inherits the watchpoints/hw
> +     breakpoints of the parent, and will remove them all from the
> +     forked off process.  Copy the debug registers mirrors into the
> +     new process so that all breakpoints and watchpoints can be
> +     removed together.  The debug registers mirror will become zeroed
> +     in the end before detaching the forked off process, thus making
> +     this compatible with older Linux kernels too.  */
> +
> +  *child_proc_info = *parent_proc_info;
> +
> +  /* Mark all the hardware breakpoints and watchpoints as changed to
> +     make sure that the registers will be updated.  */
> +  child_lwp = find_lwp_pid (ptid_of (child));
> +  child_lwp_info = child_lwp->arch_private;
> +  for (i = 0; i < MAX_BPTS; i++)
> +    child_lwp_info->bpts_changed[i] = 1;
> +  for (i = 0; i < MAX_WPTS; i++)
> +    child_lwp_info->wpts_changed[i] = 1;
> +}
> +
>  /* Called when resuming a thread.
>     If the debug regs have changed, update the thread's copies.  */
>  static void
> @@ -920,6 +961,7 @@ struct linux_target_ops the_low_target = {
>    NULL, /* siginfo_fixup */
>    arm_new_process,
>    arm_new_thread,
> +  arm_new_fork,
>    arm_prepare_to_resume,
>  };
> 
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index b280c17..d9053ad 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -489,6 +489,10 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
>           child_proc->tdesc = tdesc;
>           child_lwp->must_set_ptrace_flags = 1;
> 
> +         /* Clone arch-specific process data.  */
> +         if (the_low_target.new_fork != NULL)
> +           the_low_target.new_fork (parent_proc, child_proc);
> +
>           /* Save fork info in the parent thread.  */
>           event_lwp->waitstatus.kind = TARGET_WAITKIND_FORKED;
>           event_lwp->waitstatus.value.related_pid = ptid;
> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
> index 41067d6..3300da9 100644
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -187,6 +187,9 @@ struct linux_target_ops
>       allocate it here.  */
>    void (*new_thread) (struct lwp_info *);
> 
> +  /* Hook to call, if any, when a new fork is attached.  */
> +  void (*new_fork) (struct process_info *parent, struct process_info *child);
> +
>    /* Hook to call prior to resuming a thread.  */
>    void (*prepare_to_resume) (struct lwp_info *);
> 
> diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
> index 7988d07..4601ad0 100644
> --- a/gdb/gdbserver/linux-mips-low.c
> +++ b/gdb/gdbserver/linux-mips-low.c
> @@ -344,6 +344,68 @@ mips_linux_new_thread (struct lwp_info *lwp)
>    lwp->arch_private = info;
>  }
> 
> +/* Create a new mips_watchpoint and add it to the list.  */
> +
> +static void
> +mips_add_watchpoint (struct arch_process_info *private, CORE_ADDR addr,
> +                    int len, int watch_type)
> +{
> +  struct mips_watchpoint *new_watch;
> +  struct mips_watchpoint **pw;
> +
> +  new_watch = xmalloc (sizeof (struct mips_watchpoint));
> +  new_watch->addr = addr;
> +  new_watch->len = len;
> +  new_watch->type = watch_type;
> +  new_watch->next = NULL;
> +
> +  pw = &private->current_watches;
> +  while (*pw != NULL)
> +    pw = &(*pw)->next;
> +  *pw = new_watch;
> +}
> +
> +/* Hook to call when a new fork is attached.  */
> +
> +static void
> +mips_linux_new_fork (struct process_info *parent,
> +                       struct process_info *child)
> +{
> +  struct arch_process_info *parent_private;
> +  struct arch_process_info *child_private;
> +  struct mips_watchpoint *wp;
> +
> +  /* These are allocated by linux_add_process.  */
> +  gdb_assert (parent->private != NULL
> +             && parent->private->arch_private != NULL);
> +  gdb_assert (child->private != NULL
> +             && child->private->arch_private != NULL);
> +
> +  /* Linux kernel before 2.6.33 commit
> +     72f674d203cd230426437cdcf7dd6f681dad8b0d
> +     will inherit hardware debug registers from parent
> +     on fork/vfork/clone.  Newer Linux kernels create such tasks with
> +     zeroed debug registers.
> +
> +     GDB core assumes the child inherits the watchpoints/hw
> +     breakpoints of the parent, and will remove them all from the
> +     forked off process.  Copy the debug registers mirrors into the
> +     new process so that all breakpoints and watchpoints can be
> +     removed together.  The debug registers mirror will become zeroed
> +     in the end before detaching the forked off process, thus making
> +     this compatible with older Linux kernels too.  */
> +
> +  parent_private = parent->private->arch_private;
> +  child_private = child->private->arch_private;
> +
> +  child_private->watch_readback_valid = parent_private->watch_readback_valid;
> +  child_private->watch_readback = parent_private->watch_readback;
> +
> +  for (wp = parent_private->current_watches; wp != NULL; wp = wp->next)
> +    mips_add_watchpoint (child_private, wp->addr, wp->len, wp->type);
> +
> +  child_private->watch_mirror = parent_private->watch_mirror;
> +}
>  /* This is the implementation of linux_target_ops method
>     prepare_to_resume.  If the watch regs have changed, update the
>     thread's copies.  */
> @@ -397,8 +459,6 @@ mips_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
>    struct process_info *proc = current_process ();
>    struct arch_process_info *priv = proc->priv->arch_private;
>    struct pt_watch_regs regs;
> -  struct mips_watchpoint *new_watch;
> -  struct mips_watchpoint **pw;
>    int pid;
>    long lwpid;
>    enum target_hw_bp_type watch_type;
> @@ -425,16 +485,7 @@ mips_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
>      return -1;
> 
>    /* It fit.  Stick it on the end of the list.  */
> -  new_watch = xmalloc (sizeof (struct mips_watchpoint));
> -  new_watch->addr = addr;
> -  new_watch->len = len;
> -  new_watch->type = watch_type;
> -  new_watch->next = NULL;
> -
> -  pw = &priv->current_watches;
> -  while (*pw != NULL)
> -    pw = &(*pw)->next;
> -  *pw = new_watch;
> +  mips_add_watchpoint (priv, addr, len, watch_type);
> 
>    priv->watch_mirror = regs;
> 
> @@ -845,6 +896,7 @@ struct linux_target_ops the_low_target = {
>    NULL, /* siginfo_fixup */
>    mips_linux_new_process,
>    mips_linux_new_thread,
> +  mips_linux_new_fork,
>    mips_linux_prepare_to_resume
>  };
> 
> diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
> index 763df08..4aef7b7 100644
> --- a/gdb/gdbserver/linux-x86-low.c
> +++ b/gdb/gdbserver/linux-x86-low.c
> @@ -634,6 +634,34 @@ x86_linux_new_process (void)
>    return info;
>  }
> 
> +/* Target routine for linux_new_fork.  */
> +
> +static void
> +x86_linux_new_fork (struct process_info *parent, struct process_info *child)
> +{
> +  /* These are allocated by linux_add_process.  */
> +  gdb_assert (parent->priv != NULL
> +             && parent->priv->arch_private != NULL);
> +  gdb_assert (child->priv != NULL
> +             && child->priv->arch_private != NULL);
> +
> +  /* Linux kernel before 2.6.33 commit
> +     72f674d203cd230426437cdcf7dd6f681dad8b0d
> +     will inherit hardware debug registers from parent
> +     on fork/vfork/clone.  Newer Linux kernels create such tasks with
> +     zeroed debug registers.
> +
> +     GDB core assumes the child inherits the watchpoints/hw
> +     breakpoints of the parent, and will remove them all from the
> +     forked off process.  Copy the debug registers mirrors into the
> +     new process so that all breakpoints and watchpoints can be
> +     removed together.  The debug registers mirror will become zeroed
> +     in the end before detaching the forked off process, thus making
> +     this compatible with older Linux kernels too.  */
> +
> +  *child->priv->arch_private = *parent->priv->arch_private;
> +}
> +
>  /* See nat/x86-dregs.h.  */
> 
>  struct x86_debug_reg_state *
> @@ -3263,6 +3291,7 @@ struct linux_target_ops the_low_target =
>    x86_siginfo_fixup,
>    x86_linux_new_process,
>    x86_linux_new_thread,
> +  x86_linux_new_fork,
>    x86_linux_prepare_to_resume,
>    x86_linux_process_qsupported,
>    x86_supports_tracepoints,
> --
> 1.8.1.1
> 



More information about the Gdb-patches mailing list