[rfc,gdbserver] Support hardware watchpoints on ARM

Message ID 201109121723.p8CHN03n017032@d06av02.portsmouth.uk.ibm.com
State Accepted
Headers show

Commit Message

Ulrich Weigand Sept. 12, 2011, 5:23 p.m.
Hello,

this adds hardware breakpoint/watchpoint support for the arm-linux-gnueabi
target to gdbserver.  (Such support has been present in GDB itself for a
while.)  Functionality should be equivalent to what is available in GDB.

[ Note that there are the usual limitations w.r.t. resource accounting
with the remote target.  ARM Linux currently supports only one watchpoint
at a time; native GDB is able to automatically detect that fact, but
with gdbserver you need to let GDB know about this manually via
   set remote hardware-watchpoint-limit 1
This is fundamentally due to the fact that the remote protocol does not
provide for watchpoint resource accounting packets.  Hopefully with the
work currently under way to remove resource accounting (and instead
simply attempt to insert watchpoints until the target reports failure),
this problem should go away. ]

Also note that while the implementation is modeled after code in
arm-linux-nat.c, there is no attempt at sharing code in common/.
The interfaces to target code in GDB and linux-low in gdbserver
w.r.t. watchpoints are still too different for this to make much
sense.  (Again, after the resource accounting rework mentioned
above, some of those differences might go away.)

Remote tested on arm-linux-gnueabi from a i386-linux host, using a
modification to the test suite that provides the above-mentioned
set remote hardware-watchpoint-limit command (similar to how the
can-use-hardware-watchpoint mechanism works).  No regressions.

Any comments?  I'm planning on committing this soon ...

Bye,
Ulrich


ChangeLog:

	* linux-arm-low.c: Include <signal.h>.
	(PTRACE_GETHBPREGS, PTRACE_SETHBPREGS): Define if necessary.
	(struct arm_linux_hwbp_cap): New data type.
	(arm_hwbp_type, arm_hwbp_control_t): New typedefs.
	(struct arm_linux_hw_breakpoint): New data type.
	(MAX_BPTS, MAX_WPTS): Define.
	(struct arch_process_info, struct arch_lwp_info): New data types.
	(arm_linux_get_hwbp_cap): New function.
	(arm_linux_get_hw_breakpoint_count): Likewise.
	(arm_linux_get_hw_watchpoint_count): Likewise.
	(arm_linux_get_hw_watchpoint_max_length): Likewise.
	(arm_hwbp_control_initialize): Likewise.
	(arm_hwbp_control_is_enabled): Likewise.
	(arm_hwbp_control_is_initialized): Likewise.
	(arm_hwbp_control_disable): Likewise.
	(arm_linux_hw_breakpoint_equal): Likewise.
	(arm_linux_hw_point_initialize): Likewise.
	(struct update_registers_data): New data structure.
	(update_registers_callback: New function.
	(arm_insert_point): Likewise.
	(arm_remove_point): Likewise.
	(arm_stopped_by_watchpoint): Likewise.
	(arm_stopped_data_address): Likewise.
	(arm_new_process): Likewise.
	(arm_new_thread): Likewise.
	(arm_prepare_to_resume): Likewise.
	(the_low_target): Register arm_insert_point, arm_remove_point,
	arm_stopped_by_watchpoint, arm_stopped_data_address, arm_new_process,
	arm_new_thread, and arm_prepare_to_resume.

Comments

Pedro Alves Sept. 21, 2011, 1:29 p.m. | #1
Hi Ulrich,

I was just looking over the patch before lunch, and
meanwhile you've committed it.  :-)  It looks fine to me in any
case.  :-)  I just had a couple minor remarks.

On Monday 12 September 2011 18:23:00, Ulrich Weigand wrote:
> +  if (hwbp_type == arm_hwbp_break)
> +    {
> +      /* For breakpoints, the length field encodes the mode.  */
> +      switch (len)
> +       {
> +       case 2:  /* 16-bit Thumb mode breakpoint */
> +       case 3:  /* 32-bit Thumb mode breakpoint */
> +         mask = 0x3 << (addr & 2);
> +         break;
> +       case 4:  /* 32-bit ARM mode breakpoint */
> +         mask = 0xf;
> +         break;
> +       default:
> +         /* Unsupported. */
> +         return -1;
> +       }
> +
> +      addr &= ~3;

Is this ~3 correct for 16-bit Thumb?

> +    }


> +static void
> +arm_prepare_to_resume (struct lwp_info *lwp)
> +{
> +  int pid = lwpid_of (lwp);
> +  struct process_info *proc = find_process_pid (pid_of (lwp));
> +  struct arch_process_info *proc_info = proc->private->arch_private;
> +  struct arch_lwp_info *lwp_info = lwp->arch_private;
> +  int i;
> +
> +  for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)

It's a bit unfortunate that arm_linux_get_hw_breakpoint_count 
relies on the current_inferior global having been set to LWP by
the callers.  We try to avoid that when we have an LWP handy.
Can we make arm_linux_get_hw_breakpoint_count take an LWP argument?

On Monday 12 September 2011 18:23:00, Ulrich Weigand wrote:
> +       if (arm_hwbp_control_is_enabled (proc_info->bpts[i].control))
> +         if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 1),
> +             &proc_info->bpts[i].address) < 0)
> +           error (_("Unexpected error setting breakpoint address"));
> +
> +       if (arm_hwbp_control_is_initialized (proc_info->bpts[i].control))
> +         if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 2),
> +             &proc_info->bpts[i].control) < 0)
> +           error (_("Unexpected error setting breakpoint"));

I think perror_with_name would be better, so we can see on the logs
the errno ptrace set on error.

Patch

Index: gdb/gdbserver/linux-arm-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-arm-low.c,v
retrieving revision 1.27
diff -u -p -r1.27 linux-arm-low.c
--- gdb/gdbserver/linux-arm-low.c	2 Mar 2011 23:40:42 -0000	1.27
+++ gdb/gdbserver/linux-arm-low.c	12 Sep 2011 13:54:41 -0000
@@ -26,6 +26,7 @@ 
 #include <elf.h>
 #endif
 #include <sys/ptrace.h>
+#include <signal.h>
 
 /* Defined in auto-generated files.  */
 void init_registers_arm (void);
@@ -48,6 +49,65 @@  void init_registers_arm_with_neon (void)
 # define PTRACE_SETVFPREGS 28
 #endif
 
+#ifndef PTRACE_GETHBPREGS
+#define PTRACE_GETHBPREGS 29
+#define PTRACE_SETHBPREGS 30
+#endif
+
+/* Information describing the hardware breakpoint capabilities.  */
+struct arm_linux_hwbp_cap
+{
+  unsigned char arch;
+  unsigned char max_wp_length;
+  unsigned char wp_count;
+  unsigned char bp_count;
+};
+
+/* Enum describing the different types of ARM hardware break-/watch-points.  */
+typedef enum
+{
+  arm_hwbp_break = 0,
+  arm_hwbp_load = 1,
+  arm_hwbp_store = 2,
+  arm_hwbp_access = 3
+} arm_hwbp_type;
+
+/* Type describing an ARM Hardware Breakpoint Control register value.  */
+typedef unsigned int arm_hwbp_control_t;
+
+/* Structure used to keep track of hardware break-/watch-points.  */
+struct arm_linux_hw_breakpoint
+{
+  /* Address to break on, or being watched.  */
+  unsigned int address;
+  /* Control register for break-/watch- point.  */
+  arm_hwbp_control_t control;
+};
+
+/* Since we cannot dynamically allocate subfields of arch_process_info,
+   assume a maximum number of supported break-/watchpoints.  */
+#define MAX_BPTS 32
+#define MAX_WPTS 32
+
+/* Per-process arch-specific data we want to keep.  */
+struct arch_process_info
+{
+  /* 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
+{
+  /* Non-zero if our copy differs from what's recorded in the thread.  */
+  char bpts_changed[MAX_BPTS];
+  char wpts_changed[MAX_WPTS];
+  /* Cached stopped data address.  */
+  CORE_ADDR stopped_data_address;
+};
+
 static unsigned long arm_hwcap;
 
 /* These are in <asm/elf.h> in current kernels.  */
@@ -280,6 +340,431 @@  ps_get_thread_area (const struct ps_proc
   return PS_OK;
 }
 
+
+/* Get hold of the Hardware Breakpoint information for the target we are
+   attached to.  Returns NULL if the kernel doesn't support Hardware
+   breakpoints at all, or a pointer to the information structure.  */
+static const struct arm_linux_hwbp_cap *
+arm_linux_get_hwbp_cap (void)
+{
+  /* The info structure we return.  */
+  static struct arm_linux_hwbp_cap info;
+
+  /* Is INFO in a good state?  -1 means that no attempt has been made to
+     initialize INFO; 0 means an attempt has been made, but it failed; 1
+     means INFO is in an initialized state.  */
+  static int available = -1;
+
+  if (available == -1)
+    {
+      int pid = lwpid_of (get_thread_lwp (current_inferior));
+      unsigned int val;
+
+      if (ptrace (PTRACE_GETHBPREGS, pid, 0, &val) < 0)
+	available = 0;
+      else
+	{
+	  info.arch = (unsigned char)((val >> 24) & 0xff);
+	  info.max_wp_length = (unsigned char)((val >> 16) & 0xff);
+	  info.wp_count = (unsigned char)((val >> 8) & 0xff);
+	  info.bp_count = (unsigned char)(val & 0xff);
+	  available = (info.arch != 0);
+	}
+
+      if (available)
+	{
+	  if (info.wp_count > MAX_WPTS)
+	    internal_error (__FILE__, __LINE__,
+			    "Unsupported number of watchpoints");
+	  if (info.bp_count > MAX_BPTS)
+	    internal_error (__FILE__, __LINE__,
+			    "Unsupported number of breakpoints");
+	}
+    }
+
+  return available == 1 ? &info : NULL;
+}
+
+/* How many hardware breakpoints are available?  */
+static int
+arm_linux_get_hw_breakpoint_count (void)
+{
+  const struct arm_linux_hwbp_cap *cap = arm_linux_get_hwbp_cap ();
+  return cap != NULL ? cap->bp_count : 0;
+}
+
+/* How many hardware watchpoints are available?  */
+static int
+arm_linux_get_hw_watchpoint_count (void)
+{
+  const struct arm_linux_hwbp_cap *cap = arm_linux_get_hwbp_cap ();
+  return cap != NULL ? cap->wp_count : 0;
+}
+
+/* Maximum length of area watched by hardware watchpoint.  */
+static int
+arm_linux_get_hw_watchpoint_max_length (void)
+{
+  const struct arm_linux_hwbp_cap *cap = arm_linux_get_hwbp_cap ();
+  return cap != NULL ? cap->max_wp_length : 0;
+}
+
+/* Initialize an ARM hardware break-/watch-point control register value.
+   BYTE_ADDRESS_SELECT is the mask of bytes to trigger on; HWBP_TYPE is the
+   type of break-/watch-point; ENABLE indicates whether the point is enabled.
+   */
+static arm_hwbp_control_t
+arm_hwbp_control_initialize (unsigned byte_address_select,
+			     arm_hwbp_type hwbp_type,
+			     int enable)
+{
+  gdb_assert ((byte_address_select & ~0xffU) == 0);
+  gdb_assert (hwbp_type != arm_hwbp_break
+	      || ((byte_address_select & 0xfU) != 0));
+
+  return (byte_address_select << 5) | (hwbp_type << 3) | (3 << 1) | enable;
+}
+
+/* Does the breakpoint control value CONTROL have the enable bit set?  */
+static int
+arm_hwbp_control_is_enabled (arm_hwbp_control_t control)
+{
+  return control & 0x1;
+}
+
+/* Is the breakpoint control value CONTROL initialized?  */
+static int
+arm_hwbp_control_is_initialized (arm_hwbp_control_t control)
+{
+  return control != 0;
+}
+
+/* Change a breakpoint control word so that it is in the disabled state.  */
+static arm_hwbp_control_t
+arm_hwbp_control_disable (arm_hwbp_control_t control)
+{
+  return control & ~0x1;
+}
+
+/* Are two break-/watch-points equal?  */
+static int
+arm_linux_hw_breakpoint_equal (const struct arm_linux_hw_breakpoint *p1,
+			       const struct arm_linux_hw_breakpoint *p2)
+{
+  return p1->address == p2->address && p1->control == p2->control;
+}
+
+/* Initialize the hardware breakpoint structure P for a breakpoint or
+   watchpoint at ADDR to LEN.  The type of watchpoint is given in TYPE.
+   Returns -1 if TYPE is unsupported, 0 if TYPE represents a breakpoint,
+   and 1 if type represents a watchpoint.  */
+static int
+arm_linux_hw_point_initialize (char type, CORE_ADDR addr, int len,
+			       struct arm_linux_hw_breakpoint *p)
+{
+  arm_hwbp_type hwbp_type;
+  unsigned mask;
+
+  /* Breakpoint/watchpoint types (GDB terminology):
+     0 = memory breakpoint for instructions
+     (not supported; done via memory write instead)
+     1 = hardware breakpoint for instructions (supported)
+     2 = write watchpoint (supported)
+     3 = read watchpoint (supported)
+     4 = access watchpoint (supported).  */
+  switch (type)
+    {
+    case '1':
+      hwbp_type = arm_hwbp_break;
+      break;
+    case '2':
+      hwbp_type = arm_hwbp_store;
+      break;
+    case '3':
+      hwbp_type = arm_hwbp_load;
+      break;
+    case '4':
+      hwbp_type = arm_hwbp_access;
+      break;
+    default:
+      /* Unsupported.  */
+      return -1;
+    }
+
+  if (hwbp_type == arm_hwbp_break)
+    {
+      /* For breakpoints, the length field encodes the mode.  */
+      switch (len)
+	{
+	case 2:	 /* 16-bit Thumb mode breakpoint */
+	case 3:  /* 32-bit Thumb mode breakpoint */
+	  mask = 0x3 << (addr & 2);
+	  break;
+	case 4:  /* 32-bit ARM mode breakpoint */
+	  mask = 0xf;
+	  break;
+	default:
+	  /* Unsupported. */
+	  return -1;
+	}
+
+      addr &= ~3;
+    }
+  else
+    {
+      CORE_ADDR max_wp_length = arm_linux_get_hw_watchpoint_max_length ();
+      CORE_ADDR aligned_addr;
+
+      /* Can not set watchpoints for zero or negative lengths.  */
+      if (len <= 0)
+	return -1;
+      /* The current ptrace interface can only handle watchpoints that are a
+	 power of 2.  */
+      if ((len & (len - 1)) != 0)
+	return -1;
+
+      /* Test that the range [ADDR, ADDR + LEN) fits into the largest address
+	 range covered by a watchpoint.  */
+      aligned_addr = addr & ~(max_wp_length - 1);
+      if (aligned_addr + max_wp_length < addr + len)
+	return -1;
+
+      mask = (1 << len) - 1;
+    }
+
+  p->address = (unsigned int) addr;
+  p->control = arm_hwbp_control_initialize (mask, hwbp_type, 1);
+
+  return hwbp_type != arm_hwbp_break;
+}
+
+/* Callback to mark a watch-/breakpoint to be updated in all threads of
+   the current process.  */
+
+struct update_registers_data
+{
+  int watch;
+  int i;
+};
+
+static int
+update_registers_callback (struct inferior_list_entry *entry, void *arg)
+{
+  struct lwp_info *lwp = (struct lwp_info *) entry;
+  struct update_registers_data *data = (struct update_registers_data *) arg;
+
+  /* Only update the threads of the current process.  */
+  if (pid_of (lwp) == pid_of (get_thread_lwp (current_inferior)))
+    {
+      /* 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->i] = 1;
+      else
+	lwp->arch_private->bpts_changed[data->i] = 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 hardware break-/watchpoint.  */
+static int
+arm_insert_point (char type, CORE_ADDR addr, int len)
+{
+  struct process_info *proc = current_process ();
+  struct arm_linux_hw_breakpoint p, *pts;
+  int watch, i, count;
+
+  watch = arm_linux_hw_point_initialize (type, addr, len, &p);
+  if (watch < 0)
+    {
+      /* Unsupported.  */
+      return 1;
+    }
+
+  if (watch)
+    {
+      count = arm_linux_get_hw_watchpoint_count ();
+      pts = proc->private->arch_private->wpts;
+    }
+  else
+    {
+      count = arm_linux_get_hw_breakpoint_count ();
+      pts = proc->private->arch_private->bpts;
+    }
+
+  for (i = 0; i < count; i++)
+    if (!arm_hwbp_control_is_enabled (pts[i].control))
+      {
+	struct update_registers_data data = { watch, i };
+	pts[i] = p;
+	find_inferior (&all_lwps, update_registers_callback, &data);
+	return 0;
+      }
+
+  /* We're out of watchpoints.  */
+  return -1;
+}
+
+/* Remove hardware break-/watchpoint.  */
+static int
+arm_remove_point (char type, CORE_ADDR addr, int len)
+{
+  struct process_info *proc = current_process ();
+  struct arm_linux_hw_breakpoint p, *pts;
+  int watch, i, count;
+
+  watch = arm_linux_hw_point_initialize (type, addr, len, &p);
+  if (watch < 0)
+    {
+      /* Unsupported.  */
+      return -1;
+    }
+
+  if (watch)
+    {
+      count = arm_linux_get_hw_watchpoint_count ();
+      pts = proc->private->arch_private->wpts;
+    }
+  else
+    {
+      count = arm_linux_get_hw_breakpoint_count ();
+      pts = proc->private->arch_private->bpts;
+    }
+
+  for (i = 0; i < count; i++)
+    if (arm_linux_hw_breakpoint_equal (&p, pts + i))
+      {
+	struct update_registers_data data = { watch, i };
+	pts[i].control = arm_hwbp_control_disable (pts[i].control);
+	find_inferior (&all_lwps, update_registers_callback, &data);
+	return 0;
+      }
+
+  /* No watchpoint matched.  */
+  return -1;
+}
+
+/* Return whether current thread is stopped due to a watchpoint.  */
+static int
+arm_stopped_by_watchpoint (void)
+{
+  struct lwp_info *lwp = get_thread_lwp (current_inferior);
+  struct siginfo siginfo;
+
+  /* We must be able to set hardware watchpoints.  */
+  if (arm_linux_get_hw_watchpoint_count () == 0)
+    return 0;
+
+  /* Retrieve siginfo.  */
+  errno = 0;
+  ptrace (PTRACE_GETSIGINFO, lwpid_of (lwp), 0, &siginfo);
+  if (errno != 0)
+    return 0;
+
+  /* This must be a hardware breakpoint.  */
+  if (siginfo.si_signo != SIGTRAP
+      || (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
+    return 0;
+
+  /* If we are in a positive slot then we're looking at a breakpoint and not
+     a watchpoint.  */
+  if (siginfo.si_errno >= 0)
+    return 0;
+
+  /* Cache stopped data address for use by arm_stopped_data_address.  */
+  lwp->arch_private->stopped_data_address
+    = (CORE_ADDR) (uintptr_t) siginfo.si_addr;
+
+  return 1;
+}
+
+/* Return data address that triggered watchpoint.  Called only if
+   arm_stopped_by_watchpoint returned true.  */
+static CORE_ADDR
+arm_stopped_data_address (void)
+{
+  struct lwp_info *lwp = get_thread_lwp (current_inferior);
+  return lwp->arch_private->stopped_data_address;
+}
+
+/* Called when a new process is created.  */
+static struct arch_process_info *
+arm_new_process (void)
+{
+  struct arch_process_info *info = xcalloc (1, sizeof (*info));
+  return info;
+}
+
+/* Called when a new thread is detected.  */
+static struct arch_lwp_info *
+arm_new_thread (void)
+{
+  struct arch_lwp_info *info = xcalloc (1, sizeof (*info));
+  int i;
+
+  for (i = 0; i < MAX_BPTS; i++)
+    info->bpts_changed[i] = 1;
+  for (i = 0; i < MAX_WPTS; i++)
+    info->wpts_changed[i] = 1;
+
+  return info;
+}
+
+/* Called when resuming a thread.
+   If the debug regs have changed, update the thread's copies.  */
+static void
+arm_prepare_to_resume (struct lwp_info *lwp)
+{
+  int pid = lwpid_of (lwp);
+  struct process_info *proc = find_process_pid (pid_of (lwp));
+  struct arch_process_info *proc_info = proc->private->arch_private;
+  struct arch_lwp_info *lwp_info = lwp->arch_private;
+  int i;
+
+  for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
+    if (lwp_info->bpts_changed[i])
+      {
+	errno = 0;
+
+	if (arm_hwbp_control_is_enabled (proc_info->bpts[i].control))
+	  if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 1),
+	      &proc_info->bpts[i].address) < 0)
+	    error (_("Unexpected error setting breakpoint address"));
+
+	if (arm_hwbp_control_is_initialized (proc_info->bpts[i].control))
+	  if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 2),
+	      &proc_info->bpts[i].control) < 0)
+	    error (_("Unexpected error setting breakpoint"));
+
+	lwp_info->bpts_changed[i] = 0;
+      }
+
+  for (i = 0; i < arm_linux_get_hw_watchpoint_count (); i++)
+    if (lwp_info->wpts_changed[i])
+      {
+	errno = 0;
+
+	if (arm_hwbp_control_is_enabled (proc_info->wpts[i].control))
+	  if (ptrace (PTRACE_SETHBPREGS, pid, -((i << 1) + 1),
+	      &proc_info->wpts[i].address) < 0)
+	    error (_("Unexpected error setting watchpoint address"));
+
+	if (arm_hwbp_control_is_initialized (proc_info->wpts[i].control))
+	  if (ptrace (PTRACE_SETHBPREGS, pid, -((i << 1) + 2),
+	      &proc_info->wpts[i].control) < 0)
+	    error (_("Unexpected error setting watchpoint"));
+
+	lwp_info->wpts_changed[i] = 0;
+      }
+}
+
+
 static int
 arm_get_hwcap (unsigned long *valp)
 {
@@ -389,4 +874,14 @@  struct linux_target_ops the_low_target =
   arm_reinsert_addr,
   0,
   arm_breakpoint_at,
+  arm_insert_point,
+  arm_remove_point,
+  arm_stopped_by_watchpoint,
+  arm_stopped_data_address,
+  NULL, /* collect_ptrace_register */
+  NULL, /* supply_ptrace_register */
+  NULL, /* siginfo_fixup */
+  arm_new_process,
+  arm_new_thread,
+  arm_prepare_to_resume,
 };