[aarch64] Check region OK for HW watchpoint in GDBserver
authorYao Qi <yao.qi@linaro.org>
Thu, 3 Sep 2015 13:01:49 +0000 (14:01 +0100)
committerYao Qi <yao.qi@linaro.org>
Thu, 3 Sep 2015 13:01:49 +0000 (14:01 +0100)
Nowadays, if user requests HW watchpoint to monitor a large memory area
or unaligned area, aarch64 GDB will split into multiple aligned areas,
and use multiple debugging registers to watch them.  However, the
registers are not updated in a transaction way.  GDBserver doesn't revert
updates in previous iterations if some debugging registers fail to update
due to some reason, like no free debugging registers available, in the
latter iteration.  For example, if we have a char buf[34], and watch buf
in gdb,

(gdb) watch buf
Hardware watchpoint 2: buf
(gdb) c
Continuing.
infrun: clear_proceed_status_thread (Thread 13466)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: step-over queue now empty
infrun: resuming [Thread 13466] for step-over
Sending packet: $m410838,22#35...Packet received: 00000000000000000000000000000000000000000000000000000000000000000000
infrun: skipping breakpoint: stepping past insn at: 0x400524
infrun: skipping breakpoint: stepping past insn at: 0x400524
Sending packet: $Z2,410838,22#80...Packet received: E01 <----- [1]
Packet Z2 (write-watchpoint) is supported
Sending packet: $Z0,7fb7fe0a8c,4#43...Packet received: OK
Warning:
Could not insert hardware watchpoint 2.
Could not insert hardware breakpoints:
You may have requested too many hardware breakpoints/watchpoints.

GDB receives E01 for Z2 packet [1] but GDBserver updates the debugging
register status,

insert_point (addr=0x00410838, len=34, type=hw-write-watchpoint):
BREAKPOINTs:
BP0: addr=0x0, ctrl=0x00000000, ref.count=0
BP1: addr=0x0, ctrl=0x00000000, ref.count=0
BP2: addr=0x0, ctrl=0x00000000, ref.count=0
BP3: addr=0x0, ctrl=0x00000000, ref.count=0
BP4: addr=0x0, ctrl=0x00000000, ref.count=0
BP5: addr=0x0, ctrl=0x00000000, ref.count=0
WATCHPOINTs:
WP0: addr=0x410850, ctrl=0x00001ff5, ref.count=1
WP1: addr=0x410848, ctrl=0x00001ff5, ref.count=1
WP2: addr=0x410840, ctrl=0x00001ff5, ref.count=1
WP3: addr=0x410838, ctrl=0x00001ff5, ref.count=1

four debugging registers can not monitor 34-byte long area, so the last
iteration of updating debugging register state fails but previous
iterations succeed.  This makes GDB think no HW watchpoint is inserted
but some debugging registers are used.

This problem was exposed by "watch buf" gdb.base/watchpoint.exp with
aarch64 GDBserver debugging arm 32-bit program.  The buf is 30-byte long
but 4-byte aligned, and four debugging registers can't cover 34-byte
(extend 4 bytes to be 8-byte aligned) area.  However, this problem
does exist on non-multi-arch debugging scenario as well.

This patch moves code in aarch64_linux_region_ok_for_hw_watchpoint to
aarch64_linux_region_ok_for_watchpoint in nat/aarch64-linux-hw-point.c.
Then, checks with aarch64_linux_region_ok_for_watchpoint, like what we
are doing in GDB.  If the region is OK, call aarch64_handle_watchpoint.

Regression tested on aarch64 with both 64-bit program and 32-bit
program.  Some fails in gdb.base/watchpoint.exp are fixed.

gdb:

2015-09-03  Yao Qi  <yao.qi@linaro.org>

* aarch64-linux-nat.c (aarch64_linux_region_ok_for_hw_watchpoint):
Move code to aarch64_linux_region_ok_for_watchpoint.  Call
aarch64_linux_region_ok_for_watchpoint.
* nat/aarch64-linux-hw-point.c (aarch64_linux_region_ok_for_watchpoint):
New function.
* nat/aarch64-linux-hw-point.h (aarch64_linux_region_ok_for_watchpoint):
Declare it.

gdb/gdbserver:

2015-09-03  Yao Qi  <yao.qi@linaro.org>

* linux-aarch64-low.c (aarch64_insert_point): Call
aarch64_handle_watchpoint if aarch64_linux_region_ok_for_watchpoint
returns true.

gdb/ChangeLog
gdb/aarch64-linux-nat.c
gdb/gdbserver/ChangeLog
gdb/gdbserver/linux-aarch64-low.c
gdb/nat/aarch64-linux-hw-point.c
gdb/nat/aarch64-linux-hw-point.h

index f8b68747bd248475777c72558018e149b35db996..f03a2fd06ba306eea19e37fb0ea38061d9c6a2cc 100644 (file)
@@ -1,3 +1,13 @@
+2015-09-03  Yao Qi  <yao.qi@linaro.org>
+
+       * aarch64-linux-nat.c (aarch64_linux_region_ok_for_hw_watchpoint):
+       Move code to aarch64_linux_region_ok_for_watchpoint.  Call
+       aarch64_linux_region_ok_for_watchpoint.
+       * nat/aarch64-linux-hw-point.c (aarch64_linux_region_ok_for_watchpoint):
+       New function.
+       * nat/aarch64-linux-hw-point.h (aarch64_linux_region_ok_for_watchpoint):
+       Declare it.
+
 2015-09-02  Patrick Palka  <patrick@parcs.ath.cx>
 
        * gdb_obstack.h (obstack_strdup): Declare.
index 9747461c9d896c42222e308db9fede819b577751..f2ef41bb7a9ef3a34d924a1aa16f11a2a52137a4 100644 (file)
@@ -717,38 +717,7 @@ static int
 aarch64_linux_region_ok_for_hw_watchpoint (struct target_ops *self,
                                           CORE_ADDR addr, int len)
 {
-  CORE_ADDR aligned_addr;
-
-  /* Can not set watchpoints for zero or negative lengths.  */
-  if (len <= 0)
-    return 0;
-
-  /* Must have hardware watchpoint debug register(s).  */
-  if (aarch64_num_wp_regs == 0)
-    return 0;
-
-  /* We support unaligned watchpoint address and arbitrary length,
-     as long as the size of the whole watched area after alignment
-     doesn't exceed size of the total area that all watchpoint debug
-     registers can watch cooperatively.
-
-     This is a very relaxed rule, but unfortunately there are
-     limitations, e.g. false-positive hits, due to limited support of
-     hardware debug registers in the kernel.  See comment above
-     aarch64_align_watchpoint for more information.  */
-
-  aligned_addr = addr & ~(AARCH64_HWP_MAX_LEN_PER_REG - 1);
-  if (aligned_addr + aarch64_num_wp_regs * AARCH64_HWP_MAX_LEN_PER_REG
-      < addr + len)
-    return 0;
-
-  /* All tests passed so we are likely to be able to set the watchpoint.
-     The reason that it is 'likely' rather than 'must' is because
-     we don't check the current usage of the watchpoint registers, and
-     there may not be enough registers available for this watchpoint.
-     Ideally we should check the cached debug register state, however
-     the checking is costly.  */
-  return 1;
+  return aarch64_linux_region_ok_for_watchpoint (addr, len);
 }
 
 /* Implement the "to_stopped_data_address" target_ops method.  */
index 28c041cac94049744617854b2d346c5f41f21abf..58a46b7e5cbe35c8cbc616cfe48f0adb946e0fe7 100644 (file)
@@ -1,3 +1,9 @@
+2015-09-03  Yao Qi  <yao.qi@linaro.org>
+
+       * linux-aarch64-low.c (aarch64_insert_point): Call
+       aarch64_handle_watchpoint if aarch64_linux_region_ok_for_watchpoint
+       returns true.
+
 2015-08-27  Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
 
        * linux-low.c (check_stopped_by_breakpoint): Use
index da0ea9b89464f8a0f313b6ed992848a152be7cf4..aebf1e312310dc645099dd34b6da85f27e0318c5 100644 (file)
@@ -302,9 +302,13 @@ aarch64_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
   targ_type = raw_bkpt_type_to_target_hw_bp_type (type);
 
   if (targ_type != hw_execute)
-    ret =
-      aarch64_handle_watchpoint (targ_type, addr, len, 1 /* is_insert */,
-                                state);
+    {
+      if (aarch64_linux_region_ok_for_watchpoint (addr, len))
+       ret = aarch64_handle_watchpoint (targ_type, addr, len,
+                                        1 /* is_insert */, state);
+      else
+       ret = -1;
+    }
   else
     ret =
       aarch64_handle_breakpoint (targ_type, addr, len, 1 /* is_insert */,
index a3c923ad055db5eef43299fd766c2862878c4c9f..bca6ec1ce7e7bdfe2818162dcd63d22140bec56d 100644 (file)
@@ -645,3 +645,43 @@ aarch64_linux_get_debug_reg_capacity (int tid)
       aarch64_num_bp_regs = 0;
     }
 }
+
+/* Return true if we can watch a memory region that starts address
+   ADDR and whose length is LEN in bytes.  */
+
+int
+aarch64_linux_region_ok_for_watchpoint (CORE_ADDR addr, int len)
+{
+  CORE_ADDR aligned_addr;
+
+  /* Can not set watchpoints for zero or negative lengths.  */
+  if (len <= 0)
+    return 0;
+
+  /* Must have hardware watchpoint debug register(s).  */
+  if (aarch64_num_wp_regs == 0)
+    return 0;
+
+  /* We support unaligned watchpoint address and arbitrary length,
+     as long as the size of the whole watched area after alignment
+     doesn't exceed size of the total area that all watchpoint debug
+     registers can watch cooperatively.
+
+     This is a very relaxed rule, but unfortunately there are
+     limitations, e.g. false-positive hits, due to limited support of
+     hardware debug registers in the kernel.  See comment above
+     aarch64_align_watchpoint for more information.  */
+
+  aligned_addr = addr & ~(AARCH64_HWP_MAX_LEN_PER_REG - 1);
+  if (aligned_addr + aarch64_num_wp_regs * AARCH64_HWP_MAX_LEN_PER_REG
+      < addr + len)
+    return 0;
+
+  /* All tests passed so we are likely to be able to set the watchpoint.
+     The reason that it is 'likely' rather than 'must' is because
+     we don't check the current usage of the watchpoint registers, and
+     there may not be enough registers available for this watchpoint.
+     Ideally we should check the cached debug register state, however
+     the checking is costly.  */
+  return 1;
+}
index a27a2018570a4b2a2fd710244a0ad250a0b66a7c..a3cf7d9d4bcf316bf10b22c9e2caa0a6a97ee07c 100644 (file)
@@ -182,4 +182,6 @@ void aarch64_linux_get_debug_reg_capacity (int tid);
 
 struct aarch64_debug_reg_state *aarch64_get_debug_reg_state (pid_t pid);
 
+int aarch64_linux_region_ok_for_watchpoint (CORE_ADDR addr, int len);
+
 #endif /* AARCH64_LINUX_HW_POINT_H */