Support for HWbreak/watchpoint accross fork/vfork on arm-native

---
 gdb/arm-linux-nat.c | 417 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 273 insertions(+), 144 deletions(-)

On 01/31/2014 03:09 AM, Omair Javaid wrote:
> ---
>  gdb/arm-linux-nat.c | 417 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 273 insertions(+), 144 deletions(-)
> 
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index 6f56634..9a756c1 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -787,70 +787,120 @@ struct arm_linux_hw_breakpoint
>    arm_hwbp_control_t control;
>  };
>  
> -/* Structure containing arrays of the break and watch points which are have
> -   active in each thread.
> -
> -   The Linux ptrace interface to hardware break-/watch-points presents the 
> -   values in a vector centred around 0 (which is used fo generic information).
> -   Positive indicies refer to breakpoint addresses/control registers, negative
> -   indices to watchpoint addresses/control registers.
> -
> -   The Linux vector is indexed as follows:
> -      -((i << 1) + 2): Control register for watchpoint i.
> -      -((i << 1) + 1): Address register for watchpoint i.
> -                    0: Information register.
> -       ((i << 1) + 1): Address register for breakpoint i.
> -       ((i << 1) + 2): Control register for breakpoint i.
> -
> -   This structure is used as a per-thread cache of the state stored by the 
> -   kernel, so that we don't need to keep calling into the kernel to find a 
> -   free breakpoint.
> -
> -   We treat break-/watch-points with their enable bit clear as being deleted.
> -   */
> -typedef struct arm_linux_thread_points
> +/* Since we cannot dynamically allocate subfields of arm_linux_process_info,
> +   assume a maximum number of supported break-/watchpoints.  */
> +#define MAX_BPTS 16
> +#define MAX_WPTS 16
> +
> +/* Per-process arch-specific data we want to keep.  */
> +struct arm_linux_process_info
>  {
> -  /* Thread ID.  */
> -  int tid;
> -  /* Breakpoints for thread.  */
> -  struct arm_linux_hw_breakpoint *bpts;
> -  /* Watchpoint for threads.  */
> -  struct arm_linux_hw_breakpoint *wpts;
> -} *arm_linux_thread_points_p;
> -DEF_VEC_P (arm_linux_thread_points_p);
> -
> -/* Vector of hardware breakpoints for each thread.  */
> -VEC(arm_linux_thread_points_p) *arm_threads = NULL;
> -
> -/* Find the list of hardware break-/watch-points for a thread with id TID.
> -   If no list exists for TID we return NULL if ALLOC_NEW is 0, otherwise we
> -   create a new list and return that.  */
> -static struct arm_linux_thread_points *
> -arm_linux_find_breakpoints_by_tid (int tid, int alloc_new)
> +  /* Linked list.  */
> +  struct arm_linux_process_info *next;
> +  /* The process identifier.  */
> +  pid_t pid;
> +  /* Hardware breakpoints for this process.  */
> +  struct arm_linux_hw_breakpoint bpts[MAX_BPTS];
> +  /* Hardware watchpoints for this process.  */
> +  struct arm_linux_hw_breakpoint wpts[MAX_WPTS];
> +};
> +
> +/* Per-thread arch-specific data we want to keep.  */
> +struct arch_lwp_info
>  {
> -  int i;
> -  struct arm_linux_thread_points *t;
> +  /* Non-zero if our copy differs from what's recorded in the thread.  */
> +  char bpts_changed[MAX_BPTS];
> +  char wpts_changed[MAX_WPTS];
> +};
>  
> -  for (i = 0; VEC_iterate (arm_linux_thread_points_p, arm_threads, i, t); ++i)
> -    {
> -      if (t->tid == tid)
> -	return t;
> -    }
> +static struct arm_linux_process_info *arm_linux_process_list = NULL;
> +
> +/* Find process data for process PID.  */
>  
> -  t = NULL;
> +static struct arm_linux_process_info *
> +arm_linux_find_process_pid (pid_t pid)
> +{
> +  struct arm_linux_process_info *proc;
> +
> +  for (proc = arm_linux_process_list; proc; proc = proc->next)
> +    if (proc->pid == pid)
> +      return proc;
>  
> -  if (alloc_new)
> +  return NULL;
> +}
> +
> +/* Add process data for process PID.  Returns newly allocated info
> +   object.  */
> +
> +static struct arm_linux_process_info *
> +arm_linux_add_process (pid_t pid)
> +{
> +  struct arm_linux_process_info *proc;
> +
> +  proc = xcalloc (1, sizeof (*proc));
> +  proc->pid = pid;
> +
> +  proc->next = arm_linux_process_list;
> +  arm_linux_process_list = proc;
> +
> +  return proc;
> +}
> +
> +/* Get data specific info for process PID, creating it if necessary.
> +   Never returns NULL.  */
> +
> +static struct arm_linux_process_info *
> +arm_linux_process_info_get (pid_t pid)
> +{
> +  struct arm_linux_process_info *proc;
> +
> +  proc = arm_linux_find_process_pid (pid);
> +  if (proc == NULL)
> +    proc = arm_linux_add_process (pid);
> +
> +  return proc;
> +}
> +
> +/* Called whenever GDB is no longer debugging process PID.  It deletes
> +   data structures that keep track of debug register state.  */
> +
> +static void
> +arm_linux_forget_process (pid_t pid)
> +{
> +  struct arm_linux_process_info *proc, **proc_link;
> +
> +  proc = arm_linux_process_list;
> +  proc_link = &arm_linux_process_list;
> +
> +  while (proc != NULL)
>      {
> -      t = xmalloc (sizeof (struct arm_linux_thread_points));
> -      t->tid = tid;
> -      t->bpts = xzalloc (arm_linux_get_hw_breakpoint_count ()
> -			 * sizeof (struct arm_linux_hw_breakpoint));
> -      t->wpts = xzalloc (arm_linux_get_hw_watchpoint_count ()
> -			 * sizeof (struct arm_linux_hw_breakpoint));
> -      VEC_safe_push (arm_linux_thread_points_p, arm_threads, t);
> +      if (proc->pid == pid)
> +    {
> +      *proc_link = proc->next;
> +
> +      xfree (proc);
> +      return;
> +    }
> +
> +      proc_link = &proc->next;
> +      proc = *proc_link;
>      }
> +}
> +
> +/* Get hardware breakpoint state for process PID.  */
>  
> -  return t;
> +static struct arm_linux_hw_breakpoint *
> +arm_linux_get_hwbreak_state (pid_t pid)
> +{
> +  return arm_linux_process_info_get (pid)->bpts;
> +}
> +
> +/* Get watchpoint state for process PID.  */
> +
> +static struct arm_linux_hw_breakpoint *
> +arm_linux_get_watch_state (pid_t pid)
> +{
> +  return arm_linux_process_info_get (pid)->wpts;
>  }
>  
>  /* Initialize an ARM hardware break-/watch-point control register value.
> @@ -950,45 +1000,76 @@ arm_linux_hw_breakpoint_equal (const struct arm_linux_hw_breakpoint *p1,
>    return p1->address == p2->address && p1->control == p2->control;
>  }
>  
> +/* Callback to mark a watch-/breakpoint to be updated in all threads of
> +   the current process.  */
> +
> +struct update_registers_data
> +{
> +  int watch;
> +  int index;
> +};
> +
> +static int
> +update_registers_callback (struct lwp_info *lwp, void *arg)
> +{
> +  struct update_registers_data *data = (struct update_registers_data *) arg;
> +
> +  /* Force iterate_over_lwps to return matched lwp_info*.  */
> +  if (arg == NULL)
> +    return 1;
> +
> +  if (lwp->arch_private == NULL)
> +    lwp->arch_private = XCNEW (struct arch_lwp_info);
> +
> +  /* The actual update is done later just before resuming the lwp,
> +     we just mark that the registers need updating.  */
> +  if (data->watch)
> +    lwp->arch_private->wpts_changed[data->index] = 1;
> +  else
> +    lwp->arch_private->bpts_changed[data->index] = 1;
> +
> +  /* If the lwp isn't stopped, force it to momentarily pause, so
> +     we can update its breakpoint registers.  */
> +  if (!lwp->stopped)
> +    linux_stop_lwp (lwp);
> +
> +  return 0;
> +}
> +
>  /* Insert the hardware breakpoint (WATCHPOINT = 0) or watchpoint (WATCHPOINT
>     =1) BPT for thread TID.  */
>  static void
>  arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt, 
> -				int tid, int watchpoint)
> +                                 int watchpoint)
>  {
> -  struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 1);
> +  int pid;
> +  ptid_t pid_ptid;
>    gdb_byte count, i;
>    struct arm_linux_hw_breakpoint* bpts;
> -  int dir;
> +  struct update_registers_data data;
>  
> -  gdb_assert (t != NULL);
> +  pid = ptid_get_pid (inferior_ptid);
> +  pid_ptid = pid_to_ptid (pid);
>  
>    if (watchpoint)
>      {
>        count = arm_linux_get_hw_watchpoint_count ();
> -      bpts = t->wpts;
> -      dir = -1;
> +      bpts = arm_linux_get_watch_state(pid);
>      }
>    else
>      {
>        count = arm_linux_get_hw_breakpoint_count ();
> -      bpts = t->bpts;
> -      dir = 1;
> +      bpts = arm_linux_get_hwbreak_state(pid);
>      }
>  
>    for (i = 0; i < count; ++i)
>      if (!arm_hwbp_control_is_enabled (bpts[i].control))
>        {
> -	errno = 0;
> -	if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 1), 
> -		    &bpt->address) < 0)
> -	  perror_with_name (_("Unexpected error setting breakpoint address"));
> -	if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2), 
> -		    &bpt->control) < 0)
> -	  perror_with_name (_("Unexpected error setting breakpoint"));
> -
> -	memcpy (bpts + i, bpt, sizeof (struct arm_linux_hw_breakpoint));
> -	break;
> +        data.watch = watchpoint;
> +        data.index = i;
> +        bpts[i] = *bpt;
> +        iterate_over_lwps (pid_ptid, update_registers_callback, &data);
> +        break;
>        }
>  
>    gdb_assert (i != count);
> @@ -998,37 +1079,36 @@ arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
>     (WATCHPOINT = 1) BPT for thread TID.  */
>  static void
>  arm_linux_remove_hw_breakpoint1 (const struct arm_linux_hw_breakpoint *bpt, 
> -				 int tid, int watchpoint)
> +                                 int watchpoint)
>  {
> -  struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 0);
> +  int pid;
>    gdb_byte count, i;
> -  struct arm_linux_hw_breakpoint *bpts;
> -  int dir;
> +  ptid_t pid_ptid;
> +  struct arm_linux_hw_breakpoint* bpts;
> +  struct update_registers_data data;
>  
> -  gdb_assert (t != NULL);
> +  pid = ptid_get_pid (inferior_ptid);
> +  pid_ptid = pid_to_ptid (pid);
>  
>    if (watchpoint)
>      {
>        count = arm_linux_get_hw_watchpoint_count ();
> -      bpts = t->wpts;
> -      dir = -1;
> +      bpts = arm_linux_get_watch_state(pid);
>      }
>    else
>      {
>        count = arm_linux_get_hw_breakpoint_count ();
> -      bpts = t->bpts;
> -      dir = 1;
> +      bpts = arm_linux_get_hwbreak_state(pid);
>      }
>  
>    for (i = 0; i < count; ++i)
>      if (arm_linux_hw_breakpoint_equal (bpt, bpts + i))
>        {
> -	errno = 0;
> -	bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
> -	if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2), 
> -		    &bpts[i].control) < 0)
> -	  perror_with_name (_("Unexpected error clearing breakpoint"));
> -	break;
> +        data.watch = watchpoint;
> +        data.index = i;
> +        bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
> +        iterate_over_lwps (pid_ptid, update_registers_callback, &data);
> +        break;
>        }
>  
>    gdb_assert (i != count);
> @@ -1046,8 +1126,8 @@ arm_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
>      return -1;
>  
>    arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
> -  ALL_LWPS (lp)
> -    arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
> +
> +  arm_linux_insert_hw_breakpoint1 (&p, 0);
>  
>    return 0;
>  }
> @@ -1064,8 +1144,8 @@ arm_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
>      return -1;
>  
>    arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
> -  ALL_LWPS (lp)
> -    arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
> +
> +  arm_linux_remove_hw_breakpoint1 (&p, 0);
>  
>    return 0;
>  }
> @@ -1115,8 +1195,8 @@ arm_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
>      return -1;
>  
>    arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
> -  ALL_LWPS (lp)
> -    arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
> +
> +  arm_linux_insert_hw_breakpoint1 (&p, 1);
>  
>    return 0;
>  }
> @@ -1133,8 +1213,8 @@ arm_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw,
>      return -1;
>  
>    arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
> -  ALL_LWPS (lp)
> -    arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
> +
> +  arm_linux_remove_hw_breakpoint1 (&p, 1);
>  
>    return 0;
>  }
> @@ -1190,63 +1270,108 @@ arm_linux_watchpoint_addr_within_range (struct target_ops *target,
>  static void
>  arm_linux_new_thread (struct lwp_info *lp)
>  {
> -  int tid = ptid_get_lwp (lp->ptid);
> -  const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
> +  int i;
> +  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
>  
> -  if (info != NULL)
> +  /* Mark that all the hardware breakpoint/watchpoint register pairs
> +     for this thread need to be initialized.  */
> +
> +  for (i = 0; i < MAX_BPTS; i++)
>      {
> -      int i;
> -      struct arm_linux_thread_points *p;
> -      struct arm_linux_hw_breakpoint *bpts;
> -
> -      if (VEC_empty (arm_linux_thread_points_p, arm_threads))
> -	return;
> -
> -      /* Get a list of breakpoints from any thread. */
> -      p = VEC_last (arm_linux_thread_points_p, arm_threads);
> -
> -      /* Copy that thread's breakpoints and watchpoints to the new thread. */
> -      for (i = 0; i < info->bp_count; i++)
> -	if (arm_hwbp_control_is_enabled (p->bpts[i].control))
> -	  arm_linux_insert_hw_breakpoint1 (p->bpts + i, tid, 0);
> -      for (i = 0; i < info->wp_count; i++)
> -	if (arm_hwbp_control_is_enabled (p->wpts[i].control))
> -	  arm_linux_insert_hw_breakpoint1 (p->wpts + i, tid, 1);
> +      info->bpts_changed[i] = 1;
> +      info->wpts_changed[i] = 1;
>      }
> +
> +  lp->arch_private = info;
>  }
>  
> -/* Handle thread exit.  Tidy up the memory that has been allocated for the
> -   thread.  */
> +/* Called when resuming a thread.
> +   The hardware debug registers are updated when there is any change.  */
> +
>  static void
> -arm_linux_thread_exit (struct thread_info *tp, int silent)
> +arm_linux_prepare_to_resume (struct lwp_info *lwp)
>  {
> -  const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
> +  int pid, i;
> +  struct arm_linux_hw_breakpoint *bpts, *wpts;
> +  struct arch_lwp_info *arm_lwp_info = lwp->arch_private;
> +
> +  /* NULL means this is the main thread still going through the shell,
> +     or, no watchpoint has been set yet.  In that case, there's
> +     nothing to do.  */
> +  if (arm_lwp_info == NULL)
> +    return;
>  
> -  if (info != NULL)
> -    {
> -      int i;
> -      int tid = ptid_get_lwp (tp->ptid);
> -      struct arm_linux_thread_points *t = NULL, *p;
> +  for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
> +    if (arm_lwp_info->bpts_changed[i])
> +      {
> +        errno = 0;
> +        pid = ptid_get_lwp (lwp->ptid);
> +        bpts = arm_linux_get_hwbreak_state (ptid_get_pid (lwp->ptid));
>  
> -      for (i = 0; 
> -	   VEC_iterate (arm_linux_thread_points_p, arm_threads, i, p); i++)
> -	{
> -	  if (p->tid == tid)
> -	    {
> -	      t = p;
> -	      break;
> -	    }
> -	}
> +        if (arm_hwbp_control_is_enabled (bpts[i].control))
> +          if (ptrace (PTRACE_SETHBPREGS, pid,
> +              (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
> +              perror_with_name ("Unexpected error setting breakpoint address");
>  
> -      if (t == NULL)
> -	return;
> +        if (bpts[i].control != 0)
> +          if (ptrace (PTRACE_SETHBPREGS, pid,
> +              (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
> +              perror_with_name ("Unexpected error setting breakpoint");
>  
> -      VEC_unordered_remove (arm_linux_thread_points_p, arm_threads, i);
> +        arm_lwp_info->bpts_changed[i] = 0;
> +      }
>  
> -      xfree (t->bpts);
> -      xfree (t->wpts);
> -      xfree (t);
> -    }
> +  for (i = 0; i < arm_linux_get_hw_watchpoint_count (); i++)
> +    if (arm_lwp_info->wpts_changed[i])
> +      {
> +        errno = 0;
> +        pid = ptid_get_lwp (lwp->ptid);
> +        wpts = arm_linux_get_watch_state (ptid_get_pid (lwp->ptid));
> +
> +        if (arm_hwbp_control_is_enabled (wpts[i].control))
> +          if (ptrace (PTRACE_SETHBPREGS, pid,
> +              (PTRACE_TYPE_ARG3) -((i << 1) + 1), &wpts[i].address) < 0)
> +              perror_with_name ("Unexpected error setting watchpoint address");
> +
> +        if (wpts[i].control != 0)
> +          if (ptrace (PTRACE_SETHBPREGS, pid,
> +              (PTRACE_TYPE_ARG3) -((i << 1) + 2), &wpts[i].control) < 0)
> +              perror_with_name ("Unexpected error setting watchpoint");
> +
> +        arm_lwp_info->wpts_changed[i] = 0;
> +      }
> +}
> +
> +/* linux_nat_new_fork hook.  */
> +
> +static void
> +arm_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
> +{
> +  pid_t parent_pid;
> +  struct arm_linux_hw_breakpoint *parent_point_state;
> +  struct arm_linux_hw_breakpoint *child_point_state;
> +
> +  /* NULL means no watchpoint has ever been set in the parent.  In
> +     that case, there's nothing to do.  */
> +  if (parent->arch_private == NULL)
> +    return;
> +
> +  /* 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.  */
> +
> +  parent_pid = ptid_get_pid (parent->ptid);
> +  parent_point_state = arm_linux_get_hwbreak_state (parent_pid);
> +  child_point_state = arm_linux_get_hwbreak_state (child_pid);
> +  memcpy (child_point_state, parent_point_state,
> +          sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
> +
> +  parent_point_state = arm_linux_get_watch_state (parent_pid);
> +  child_point_state = arm_linux_get_watch_state (child_pid);
> +  memcpy (child_point_state, parent_point_state,
> +          sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
>  }
>  
>  void _initialize_arm_linux_nat (void);
> @@ -1279,7 +1404,11 @@ _initialize_arm_linux_nat (void)
>    /* Register the target.  */
>    linux_nat_add_target (t);
>  
> -  /* Handle thread creation and exit */
> -  observer_attach_thread_exit (arm_linux_thread_exit);
> +  /* Handle thread creation and exit.  */
>    linux_nat_set_new_thread (t, arm_linux_new_thread);
> +  linux_nat_set_prepare_to_resume (t, arm_linux_prepare_to_resume);
> +
> +  /* Handle process creation and exit.  */
> +  linux_nat_set_new_fork (t, arm_linux_new_fork);
> +  linux_nat_set_forget_process (t, arm_linux_forget_process);
>  }
> 

Accidentally missed the patch description and changelog:

This patch updates arm native support for hardware breakpoints and watchpoints
to accommodate watchpoint/breakpoints across fork/vfork. Previously for arm
native a record of breakpoints at thread level was being kept which has been
changed to a process level record to come in sync with gdb-linux calls to
threads and process creation/destruction hooks.

gdb:

2014-01-31  Omair Javaid  <omair.javaid@linaro.org>

	* arm-linux-nat.c (struct arm_linux_thread_points): Removed.
	(MAX_BPTS): Define.
	(MAX_WPTS): Define.
	(struct arm_linux_process_info): New.
	(DEF_VEC_P (arm_linux_thread_points_p)): Removed.
	(VEC(arm_linux_thread_points_p) *arm_threads): Removed.
	(arm_linux_find_breakpoints_by_tid): Removed.
	(struct arch_lwp_info): New.
	(arm_linux_find_process_pid): New functions.
	(arm_linux_add_process): New functions.
	(arm_linux_process_info_get): New functions.
	(arm_linux_forget_process): New function.
	(arm_linux_get_hwbreak_state): New function.
	(arm_linux_get_watch_state): New function.
	(struct update_registers_data): New.
	(update_registers_callback): New function.
	(arm_linux_insert_hw_breakpoint1): Updated.
	(arm_linux_remove_hw_breakpoint1): Updated.
	(arm_linux_insert_hw_breakpoint): Updated.
	(arm_linux_remove_hw_breakpoint): Updated.
	(arm_linux_insert_watchpoint): Updated.
	(arm_linux_remove_watchpoint): Updated.
	(arm_linux_new_thread): Updated.
	(arm_linux_prepare_to_resume): New function.
	(arm_linux_new_fork): New function.
	(_initialize_arm_linux_nat): Updated.