diff mbox

aarch64: PR 19806: watchpoints: false negatives + PR 20207 contiguous ones

Message ID 20170327210753.GA29656@host1.jankratochvil.net
State New
Headers show

Commit Message

Jan Kratochvil March 27, 2017, 9:07 p.m. UTC
Hi,

obsoletes:
	[patch] aarch64: PR 19806: watchpoints: false negatives -> false positives
	https://sourceware.org/ml/gdb-patches/2016-06/msg00078.html
	Message-ID: <20160606075945.GA19395@host1.jankratochvil.net>

Therefore on old kernels (see below) it will now rather report watchpoint
which was hit only next to it (for rwatch/awatch).  But it will never miss
a watchpoint.

Additionally it now supports any watchpoint masks as described in:
	kernel RFE: aarch64: ptrace: BAS: Support any contiguous range (edit) 
	https://sourceware.org/bugzilla/show_bug.cgi?id=20207
which is already fixed in recent Linux kernels.

Tested on RHEL-7.{3,4} for no regressions on:
	kernel-4.10.0-6.el7.aarch64 (contiguous watchpoints supported)
	kernel-4.5.0-15.el7.aarch64 (contiguous watchpoints unsupported)
I have seen (not investigated):
	-FAIL: gdb.threads/thread-specific-bp.exp: non-stop: thread-specific breakpoint was deleted (timeout)
	+PASS: gdb.threads/thread-specific-bp.exp: non-stop: thread-specific breakpoint was deleted

There may be a regresion that it now less merges watchpoints so that with
multiple overlapping watchpoints it may run out of the 4 hardware watchpoint
registers.  But as discussed in the original thread GDB needs some generic
watchpoints merging framework to be used by all the target specific code.
Even current FSF GDB code does not merge it perfectly.  Also with the more
precise watchpoints one can technically merge them less.  And I do not think
it matters too much to improve mergeability only for old kernels.
Still even on new kernels some better merging logic would make sense.

OK for check-in?


Thanks,
Jan
gdb/ChangeLog
2017-03-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR breakpoints/19806 and support for PR external/20207.
	* aarch64-linux-nat.c (aarch64_linux_stopped_data_address): Fix missed
	watchpoints and PR external/20207 watchpoints.
	* gdbserver/linux-aarch64-low.c (aarch64_stopped_data_address):
	Likewise.
	* nat/aarch64-linux-hw-point.c (have_any_contiguous): New.
	(aarch64_watchpoint_offset): New.
	(aarch64_watchpoint_length): Support PR external/20207 watchpoints.
	(aarch64_point_encode_ctrl_reg): New parameter offset, new asserts.
	(aarch64_point_is_aligned): Support PR external/20207 watchpoints.
	(aarch64_align_watchpoint): New parameters aligned_offset_p and
	next_addr_orig_p.  Support PR external/20207 watchpoints.
	(aarch64_downgrade_regs): New.
	(aarch64_dr_state_insert_one_point): New parameters offset and
	addr_orig.
	(aarch64_dr_state_remove_one_point): Likewise.
	(aarch64_handle_breakpoint): Update caller.
	(aarch64_handle_aligned_watchpoint): Likewise.
	(aarch64_handle_unaligned_watchpoint): Support addr_orig and
	aligned_offset.
	(aarch64_linux_set_debug_regs): Remove const from state.  Call
	aarch64_downgrade_regs.
	(aarch64_show_debug_reg_state): Print also dr_addr_orig_wp.
	* nat/aarch64-linux-hw-point.h (DR_CONTROL_LENGTH): Rename to ...
	(DR_CONTROL_MASK): ... here.
	(struct aarch64_debug_reg_state): New field dr_addr_orig_wp.
	(unsigned int aarch64_watchpoint_offset): New prototype.
	(aarch64_linux_set_debug_regs): Remove const from state.

gdb/testsuite/ChangeLog
2017-03-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR breakpoints/19806 and support for PR external/20207.
	* gdb.base/watchpoint-unaligned.c: New file.
	* gdb.base/watchpoint-unaligned.exp: New file.

Comments

Jan Kratochvil June 19, 2017, 1:43 p.m. UTC | #1
There is a Red Hat internal deadline for this Bug 1228549.
Jan Kratochvil June 19, 2017, 1:44 p.m. UTC | #2
On Mon, 19 Jun 2017 15:43:28 +0200, Jan Kratochvil wrote:
> There is a Red Hat internal deadline for this Bug 1228549.

My mistake, no, neither it is this Bug nor there is no near deadline for it.
diff mbox

Patch

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 3f5b30e..b26d0a2 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -735,16 +735,29 @@  aarch64_linux_stopped_data_address (struct target_ops *target,
   state = aarch64_get_debug_reg_state (ptid_get_pid (inferior_ptid));
   for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
     {
+      const unsigned int offset = aarch64_watchpoint_offset
+							 (state->dr_ctrl_wp[i]);
       const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
       const CORE_ADDR addr_trap = (CORE_ADDR) siginfo.si_addr;
-      const CORE_ADDR addr_watch = state->dr_addr_wp[i];
+      const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
+      const CORE_ADDR addr_watch_aligned = (state->dr_addr_wp[i]
+					    & -(CORE_ADDR) 8);
+      const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
 
       if (state->dr_ref_count_wp[i]
 	  && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
-	  && addr_trap >= addr_watch
+	  && addr_trap >= addr_watch_aligned
 	  && addr_trap < addr_watch + len)
 	{
-	  *addr_p = addr_trap;
+	  /* ADDR_TRAP reports first address of the range touched by CPU.
+	     ADDR_TRAP may be before the ADDR_WATCH..ADDR_WATCH+LEN range
+	     for larger CPU access hitting the watchpoint by its tail part.
+	     ADDR_TRAP must be in the ADDR_WATCH_ALIGNED..ADDR_WATCH+LEN range.
+	     Never report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
+	     range (not matching any known watchpoint range).
+	     ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false positive due to
+	     PR external/20207 buggy kernels.  */
+	  *addr_p = addr_orig;
 	  return 1;
 	}
     }
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 334310b..abbb43a 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -372,16 +372,33 @@  aarch64_stopped_data_address (void)
 
   /* Check if the address matches any watched address.  */
   state = aarch64_get_debug_reg_state (pid_of (current_thread));
+  CORE_ADDR retval (0);
   for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
     {
+      const unsigned int offset = aarch64_watchpoint_offset
+							 (state->dr_ctrl_wp[i]);
       const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
       const CORE_ADDR addr_trap = (CORE_ADDR) siginfo.si_addr;
-      const CORE_ADDR addr_watch = state->dr_addr_wp[i];
+      const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
+      const CORE_ADDR addr_watch_aligned = (state->dr_addr_wp[i]
+					    & -(CORE_ADDR) 8);
+      const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
+
       if (state->dr_ref_count_wp[i]
 	  && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
-	  && addr_trap >= addr_watch
+	  && addr_trap >= addr_watch_aligned
 	  && addr_trap < addr_watch + len)
-	return addr_trap;
+	{
+	  /* ADDR_TRAP reports first address of the range touched by CPU.
+	     ADDR_TRAP may be before the ADDR_WATCH..ADDR_WATCH+LEN range
+	     for larger CPU access hitting the watchpoint by its tail part.
+	     ADDR_TRAP must be in the ADDR_WATCH_ALIGNED..ADDR_WATCH+LEN range.
+	     Never report RETVAL outside of any ADDR_WATCH..ADDR_WATCH+LEN
+	     range (not matching any known watchpoint range).
+	     ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false positive due to
+	     PR external/20207 buggy kernels.  */
+	  return addr_orig;
+	}
     }
 
   return (CORE_ADDR) 0;
diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index 9800d9a..f8a7938 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -34,6 +34,26 @@ 
 int aarch64_num_bp_regs;
 int aarch64_num_wp_regs;
 
+// TRUE means this kernel has fixed PR external/20207.
+// Fixed kernel supports any contiguous range of bits in 8-bit byte
+// DR_CONTROL_MASK.  Buggy kernel supports only 0x01, 0x03, 0x0f and 0xff.
+static bool have_any_contiguous (true);
+
+// Return starting byte (0..7) of a watchpoint encoded by CTRL.
+
+unsigned int
+aarch64_watchpoint_offset (unsigned int ctrl)
+{
+  uint8_t mask = DR_CONTROL_MASK (ctrl);
+  unsigned retval;
+
+  // Shift out bottom zeroes.
+  for (retval = 0; mask && (mask & 1) == 0; ++retval)
+    mask >>= 1;
+
+  return retval;
+}
+
 /* Utility function that returns the length in bytes of a watchpoint
    according to the content of a hardware debug control register CTRL.
    Note that the kernel currently only supports the following Byte
@@ -44,19 +64,21 @@  int aarch64_num_wp_regs;
 unsigned int
 aarch64_watchpoint_length (unsigned int ctrl)
 {
-  switch (DR_CONTROL_LENGTH (ctrl))
-    {
-    case 0x01:
-      return 1;
-    case 0x03:
-      return 2;
-    case 0x0f:
-      return 4;
-    case 0xff:
-      return 8;
-    default:
-      return 0;
-    }
+  uint8_t mask = DR_CONTROL_MASK (ctrl);
+  unsigned retval;
+
+  // Shift out bottom zeroes.
+  mask >>= aarch64_watchpoint_offset (ctrl);
+
+  // Count bottom ones;
+  for (retval = 0; (mask & 1) != 0; ++retval)
+    mask >>= 1;
+
+  if (mask)
+    error (_("Unexpected hardware watchpoint length register value 0x%x"),
+      DR_CONTROL_MASK (ctrl));
+
+  return retval;
 }
 
 /* Given the hardware breakpoint or watchpoint type TYPE and its
@@ -64,10 +86,13 @@  aarch64_watchpoint_length (unsigned int ctrl)
    breakpoint/watchpoint control register.  */
 
 static unsigned int
-aarch64_point_encode_ctrl_reg (enum target_hw_bp_type type, int len)
+aarch64_point_encode_ctrl_reg (enum target_hw_bp_type type, int offset, int len)
 {
   unsigned int ctrl, ttype;
 
+  gdb_assert (offset == 0 || have_any_contiguous);
+  gdb_assert (offset + len <= AARCH64_HWP_MAX_LEN_PER_REG);
+
   /* type */
   switch (type)
     {
@@ -89,8 +114,8 @@  aarch64_point_encode_ctrl_reg (enum target_hw_bp_type type, int len)
 
   ctrl = ttype << 3;
 
-  /* length bitmask */
-  ctrl |= ((1 << len) - 1) << 5;
+  /* offset and length bitmask */
+  ctrl |= ((1 << len) - 1) << (5 + offset);
   /* enabled at el0 */
   ctrl |= (2 << 1) | 1;
 
@@ -134,7 +159,8 @@  aarch64_point_is_aligned (int is_watchpoint, CORE_ADDR addr, int len)
   if (addr & (alignment - 1))
     return 0;
 
-  if (len != 8 && len != 4 && len != 2 && len != 1)
+  if ((!have_any_contiguous && len != 8 && len != 4 && len != 2 && len != 1)
+      || (have_any_contiguous && (len < 1 || len > 8)))
     return 0;
 
   return 1;
@@ -181,11 +207,12 @@  aarch64_point_is_aligned (int is_watchpoint, CORE_ADDR addr, int len)
 
 static void
 aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
-			  int *aligned_len_p, CORE_ADDR *next_addr_p,
-			  int *next_len_p)
+			  int *aligned_offset_p, int *aligned_len_p,
+			  CORE_ADDR *next_addr_p, int *next_len_p,
+			  CORE_ADDR *next_addr_orig_p)
 {
   int aligned_len;
-  unsigned int offset;
+  unsigned int offset, aligned_offset;
   CORE_ADDR aligned_addr;
   const unsigned int alignment = AARCH64_HWP_ALIGNMENT;
   const unsigned int max_wp_len = AARCH64_HWP_MAX_LEN_PER_REG;
@@ -200,6 +227,7 @@  aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
      must be aligned.  */
   offset = addr & (alignment - 1);
   aligned_addr = addr - offset;
+  aligned_offset = have_any_contiguous ? addr & (alignment - 1) : 0;
 
   gdb_assert (offset >= 0 && offset < alignment);
   gdb_assert (aligned_addr >= 0 && aligned_addr <= addr);
@@ -209,7 +237,7 @@  aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
     {
       /* Need more than one watchpoint registers; truncate it at the
 	 alignment boundary.  */
-      aligned_len = max_wp_len;
+      aligned_len = max_wp_len - (have_any_contiguous ? offset : 0);
       len -= (max_wp_len - offset);
       addr += (max_wp_len - offset);
       gdb_assert ((addr & (alignment - 1)) == 0);
@@ -222,19 +250,25 @@  aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
 	aligned_len_array[AARCH64_HWP_MAX_LEN_PER_REG] =
 	{ 1, 2, 4, 4, 8, 8, 8, 8 };
 
-      aligned_len = aligned_len_array[offset + len - 1];
+      aligned_len = (have_any_contiguous
+		     ? len : aligned_len_array[offset + len - 1]);
       addr += len;
       len = 0;
     }
 
   if (aligned_addr_p)
     *aligned_addr_p = aligned_addr;
+  if (aligned_offset_p)
+    *aligned_offset_p = aligned_offset;
   if (aligned_len_p)
     *aligned_len_p = aligned_len;
   if (next_addr_p)
     *next_addr_p = addr;
   if (next_len_p)
     *next_len_p = len;
+  if (next_addr_orig_p)
+    *next_addr_orig_p = (((*next_addr_orig_p) + alignment)
+			 & -(CORE_ADDR) alignment);
 }
 
 struct aarch64_dr_update_callback_param
@@ -324,17 +358,70 @@  aarch64_notify_debug_reg_change (const struct aarch64_debug_reg_state *state,
   iterate_over_lwps (pid_ptid, debug_reg_change_callback, (void *) &param);
 }
 
+/* Reconfigure STATE to be compatible with Linux kernel with buggy
+   PR external/20207 during HAVE_ANY_CONTIGUOUS true->false change.
+   A regression for buggy kernels is that originally GDB could support
+   more watchpoint combinations that had matching (and thus shared)
+   masks.  On such buggy kernels new GDB will try to first setup the
+   perfect matching ranges which will run out of registers (before this
+   function can merge them).  */
+
+static void
+aarch64_downgrade_regs (struct aarch64_debug_reg_state *state)
+{
+  for (int i = 0; i < aarch64_num_wp_regs; ++i)
+    if ((state->dr_ctrl_wp[i] & 1) != 0)
+      {
+	gdb_assert (state->dr_ref_count_wp[i] != 0);
+	uint8_t mask_orig = (state->dr_ctrl_wp[i] >> 5) & 0xff;
+	gdb_assert (mask_orig != 0);
+	const std::array<uint8_t, 4> old_valid ({ 0x01, 0x03, 0x0f, 0xff });
+	uint8_t mask (0);
+	for (const uint8_t old_mask:old_valid)
+	  if (mask_orig <= old_mask)
+	    {
+	      mask = old_mask;
+	      break;
+	    }
+	gdb_assert (mask != 0);
+
+	// No update needed for this watchpoint?
+	if (mask == mask_orig)
+	  continue;
+	state->dr_ctrl_wp[i] |= mask << 5;
+	state->dr_addr_wp[i] &= -(CORE_ADDR) AARCH64_HWP_ALIGNMENT;
+
+	// Try to match duplicate entries.
+	for (int j = 0; j < i; ++j)
+	  if ((state->dr_ctrl_wp[j] & 1) != 0
+	      && state->dr_addr_wp[j] == state->dr_addr_wp[i]
+	      && state->dr_addr_orig_wp[j] == state->dr_addr_orig_wp[i]
+	      && state->dr_ctrl_wp[j] == state->dr_ctrl_wp[i])
+	    {
+	      state->dr_ref_count_wp[j] += state->dr_ref_count_wp[i];
+	      state->dr_ref_count_wp[i] = 0;
+	      state->dr_addr_wp[i] = 0;
+	      state->dr_addr_orig_wp[i] = 0;
+	      state->dr_ctrl_wp[i] &= ~1;
+	      break;
+	    }
+
+	aarch64_notify_debug_reg_change (state, 1 /* is_watchpoint */, i);
+      }
+}
+
 /* Record the insertion of one breakpoint/watchpoint, as represented
    by ADDR and CTRL, in the process' arch-specific data area *STATE.  */
 
 static int
 aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
 				   enum target_hw_bp_type type,
-				   CORE_ADDR addr, int len)
+				   CORE_ADDR addr, int offset, int len,
+				   CORE_ADDR addr_orig)
 {
   int i, idx, num_regs, is_watchpoint;
   unsigned int ctrl, *dr_ctrl_p, *dr_ref_count;
-  CORE_ADDR *dr_addr_p;
+  CORE_ADDR *dr_addr_p, *dr_addr_orig_p;
 
   /* Set up state pointers.  */
   is_watchpoint = (type != hw_execute);
@@ -343,6 +430,7 @@  aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
     {
       num_regs = aarch64_num_wp_regs;
       dr_addr_p = state->dr_addr_wp;
+      dr_addr_orig_p = state->dr_addr_orig_wp;
       dr_ctrl_p = state->dr_ctrl_wp;
       dr_ref_count = state->dr_ref_count_wp;
     }
@@ -350,11 +438,12 @@  aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
     {
       num_regs = aarch64_num_bp_regs;
       dr_addr_p = state->dr_addr_bp;
+      dr_addr_orig_p = nullptr;
       dr_ctrl_p = state->dr_ctrl_bp;
       dr_ref_count = state->dr_ref_count_bp;
     }
 
-  ctrl = aarch64_point_encode_ctrl_reg (type, len);
+  ctrl = aarch64_point_encode_ctrl_reg (type, offset, len);
 
   /* Find an existing or free register in our cache.  */
   idx = -1;
@@ -366,7 +455,9 @@  aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
 	  idx = i;
 	  /* no break; continue hunting for an exising one.  */
 	}
-      else if (dr_addr_p[i] == addr && dr_ctrl_p[i] == ctrl)
+      else if (dr_addr_p[i] == addr
+	       && (dr_addr_orig_p == nullptr || dr_addr_orig_p[i] == addr_orig)
+	       && dr_ctrl_p[i] == ctrl)
 	{
 	  gdb_assert (dr_ref_count[i] != 0);
 	  idx = i;
@@ -383,6 +474,8 @@  aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
     {
       /* new entry */
       dr_addr_p[idx] = addr;
+      if (dr_addr_orig_p != nullptr)
+	dr_addr_orig_p[idx] = addr_orig;
       dr_ctrl_p[idx] = ctrl;
       dr_ref_count[idx] = 1;
       /* Notify the change.  */
@@ -403,11 +496,12 @@  aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
 static int
 aarch64_dr_state_remove_one_point (struct aarch64_debug_reg_state *state,
 				   enum target_hw_bp_type type,
-				   CORE_ADDR addr, int len)
+				   CORE_ADDR addr, int offset, int len,
+				   CORE_ADDR addr_orig)
 {
   int i, num_regs, is_watchpoint;
   unsigned int ctrl, *dr_ctrl_p, *dr_ref_count;
-  CORE_ADDR *dr_addr_p;
+  CORE_ADDR *dr_addr_p, *dr_addr_orig_p;
 
   /* Set up state pointers.  */
   is_watchpoint = (type != hw_execute);
@@ -415,6 +509,7 @@  aarch64_dr_state_remove_one_point (struct aarch64_debug_reg_state *state,
     {
       num_regs = aarch64_num_wp_regs;
       dr_addr_p = state->dr_addr_wp;
+      dr_addr_orig_p = state->dr_addr_orig_wp;
       dr_ctrl_p = state->dr_ctrl_wp;
       dr_ref_count = state->dr_ref_count_wp;
     }
@@ -422,15 +517,18 @@  aarch64_dr_state_remove_one_point (struct aarch64_debug_reg_state *state,
     {
       num_regs = aarch64_num_bp_regs;
       dr_addr_p = state->dr_addr_bp;
+      dr_addr_orig_p = nullptr;
       dr_ctrl_p = state->dr_ctrl_bp;
       dr_ref_count = state->dr_ref_count_bp;
     }
 
-  ctrl = aarch64_point_encode_ctrl_reg (type, len);
+  ctrl = aarch64_point_encode_ctrl_reg (type, offset, len);
 
   /* Find the entry that matches the ADDR and CTRL.  */
   for (i = 0; i < num_regs; ++i)
-    if (dr_addr_p[i] == addr && dr_ctrl_p[i] == ctrl)
+    if (dr_addr_p[i] == addr
+	&& (dr_addr_orig_p == nullptr || dr_addr_orig_p[i] == addr_orig)
+	&& dr_ctrl_p[i] == ctrl)
       {
 	gdb_assert (dr_ref_count[i] != 0);
 	break;
@@ -446,6 +544,8 @@  aarch64_dr_state_remove_one_point (struct aarch64_debug_reg_state *state,
       /* Clear the enable bit.  */
       ctrl &= ~1;
       dr_addr_p[i] = 0;
+      if (dr_addr_orig_p != nullptr)
+	dr_addr_orig_p[i] = 0;
       dr_ctrl_p[i] = ctrl;
       /* Notify the change.  */
       aarch64_notify_debug_reg_change (state, is_watchpoint, i);
@@ -472,10 +572,10 @@  aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr,
       if (!aarch64_point_is_aligned (0 /* is_watchpoint */ , addr, len))
 	return -1;
 
-      return aarch64_dr_state_insert_one_point (state, type, addr, len);
+      return aarch64_dr_state_insert_one_point (state, type, addr, 0, len, -1);
     }
   else
-    return aarch64_dr_state_remove_one_point (state, type, addr, len);
+    return aarch64_dr_state_remove_one_point (state, type, addr, 0, len, -1);
 }
 
 /* This is essentially the same as aarch64_handle_breakpoint, apart
@@ -487,9 +587,9 @@  aarch64_handle_aligned_watchpoint (enum target_hw_bp_type type,
 				   struct aarch64_debug_reg_state *state)
 {
   if (is_insert)
-    return aarch64_dr_state_insert_one_point (state, type, addr, len);
+    return aarch64_dr_state_insert_one_point (state, type, addr, 0, len, addr);
   else
-    return aarch64_dr_state_remove_one_point (state, type, addr, len);
+    return aarch64_dr_state_remove_one_point (state, type, addr, 0, len, addr);
 }
 
 /* Insert/remove unaligned watchpoint by calling
@@ -504,29 +604,42 @@  aarch64_handle_unaligned_watchpoint (enum target_hw_bp_type type,
 				     CORE_ADDR addr, int len, int is_insert,
 				     struct aarch64_debug_reg_state *state)
 {
+  CORE_ADDR addr_orig = addr;
+
   while (len > 0)
     {
       CORE_ADDR aligned_addr;
-      int aligned_len, ret;
+      int aligned_offset, aligned_len, ret;
+      CORE_ADDR addr_orig_next = addr_orig;
 
-      aarch64_align_watchpoint (addr, len, &aligned_addr, &aligned_len,
-				&addr, &len);
+      aarch64_align_watchpoint (addr, len, &aligned_addr, &aligned_offset,
+				&aligned_len, &addr, &len, &addr_orig_next);
 
       if (is_insert)
 	ret = aarch64_dr_state_insert_one_point (state, type, aligned_addr,
-						 aligned_len);
+						 aligned_offset,
+						 aligned_len, addr_orig);
       else
 	ret = aarch64_dr_state_remove_one_point (state, type, aligned_addr,
-						 aligned_len);
+						 aligned_offset,
+						 aligned_len, addr_orig);
 
       if (show_debug_regs)
 	debug_printf ("handle_unaligned_watchpoint: is_insert: %d\n"
 		      "                             "
 		      "aligned_addr: %s, aligned_len: %d\n"
 		      "                                "
-		      "next_addr: %s,    next_len: %d\n",
+		      "addr_orig: %s\n"
+		      "                                "
+		      "next_addr: %s,    next_len: %d\n"
+		      "                           "
+		      "addr_orig_next: %s\n",
 		      is_insert, core_addr_to_string_nz (aligned_addr),
-		      aligned_len, core_addr_to_string_nz (addr), len);
+		      aligned_len, core_addr_to_string_nz (addr_orig),
+		      core_addr_to_string_nz (addr), len,
+		      core_addr_to_string_nz (addr_orig_next));
+
+      addr_orig = addr_orig_next;
 
       if (ret != 0)
 	return ret;
@@ -552,7 +665,7 @@  aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr,
    registers with data from *STATE.  */
 
 void
-aarch64_linux_set_debug_regs (const struct aarch64_debug_reg_state *state,
+aarch64_linux_set_debug_regs (struct aarch64_debug_reg_state *state,
 			      int tid, int watchpoint)
 {
   int i, count;
@@ -580,7 +693,17 @@  aarch64_linux_set_debug_regs (const struct aarch64_debug_reg_state *state,
   if (ptrace (PTRACE_SETREGSET, tid,
 	      watchpoint ? NT_ARM_HW_WATCH : NT_ARM_HW_BREAK,
 	      (void *) &iov))
-    error (_("Unexpected error setting hardware debug registers"));
+    {
+      // Handle PR external/20207 buggy kernels?
+      if (watchpoint && errno == EINVAL && have_any_contiguous)
+	{
+	  have_any_contiguous = false;
+	  aarch64_downgrade_regs (state);
+	  aarch64_linux_set_debug_regs (state, tid, watchpoint);
+	  return;
+	}
+      error (_("Unexpected error setting hardware debug registers"));
+    }
 }
 
 /* Print the values of the cached breakpoint/watchpoint registers.  */
@@ -611,8 +734,9 @@  aarch64_show_debug_reg_state (struct aarch64_debug_reg_state *state,
 
   debug_printf ("\tWATCHPOINTs:\n");
   for (i = 0; i < aarch64_num_wp_regs; i++)
-    debug_printf ("\tWP%d: addr=%s, ctrl=0x%08x, ref.count=%d\n",
+    debug_printf ("\tWP%d: addr=%s (orig=%s), ctrl=0x%08x, ref.count=%d\n",
 		  i, core_addr_to_string_nz (state->dr_addr_wp[i]),
+		  core_addr_to_string_nz (state->dr_addr_orig_wp[i]),
 		  state->dr_ctrl_wp[i], state->dr_ref_count_wp[i]);
 }
 
diff --git a/gdb/nat/aarch64-linux-hw-point.h b/gdb/nat/aarch64-linux-hw-point.h
index 610a5f1..de2206d 100644
--- a/gdb/nat/aarch64-linux-hw-point.h
+++ b/gdb/nat/aarch64-linux-hw-point.h
@@ -77,13 +77,13 @@ 
 
    31                             13          5      3      1     0
    +--------------------------------+----------+------+------+----+
-   |         RESERVED (SBZ)         |  LENGTH  | TYPE | PRIV | EN |
+   |         RESERVED (SBZ)         |   MASK   | TYPE | PRIV | EN |
    +--------------------------------+----------+------+------+----+
 
    The TYPE field is ignored for breakpoints.  */
 
 #define DR_CONTROL_ENABLED(ctrl)	(((ctrl) & 0x1) == 1)
-#define DR_CONTROL_LENGTH(ctrl)		(((ctrl) >> 5) & 0xff)
+#define DR_CONTROL_MASK(ctrl)		(((ctrl) >> 5) & 0xff)
 
 /* Each bit of a variable of this type is used to indicate whether a
    hardware breakpoint or watchpoint setting has been changed since
@@ -148,6 +148,7 @@  struct aarch64_debug_reg_state
 
   /* hardware watchpoint */
   CORE_ADDR dr_addr_wp[AARCH64_HWP_MAX_NUM];
+  CORE_ADDR dr_addr_orig_wp[AARCH64_HWP_MAX_NUM];
   unsigned int dr_ctrl_wp[AARCH64_HWP_MAX_NUM];
   unsigned int dr_ref_count_wp[AARCH64_HWP_MAX_NUM];
 };
@@ -166,6 +167,7 @@  struct arch_lwp_info
 extern int aarch64_num_bp_regs;
 extern int aarch64_num_wp_regs;
 
+unsigned int aarch64_watchpoint_offset (unsigned int ctrl);
 unsigned int aarch64_watchpoint_length (unsigned int ctrl);
 
 int aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr,
@@ -175,7 +177,7 @@  int aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr,
 			       int len, int is_insert,
 			       struct aarch64_debug_reg_state *state);
 
-void aarch64_linux_set_debug_regs (const struct aarch64_debug_reg_state *state,
+void aarch64_linux_set_debug_regs (struct aarch64_debug_reg_state *state,
 				   int tid, int watchpoint);
 
 void aarch64_show_debug_reg_state (struct aarch64_debug_reg_state *state,
diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.c b/gdb/testsuite/gdb.base/watchpoint-unaligned.c
new file mode 100644
index 0000000..f653e77
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.c
@@ -0,0 +1,68 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+#include <assert.h>
+
+static int again;
+
+static volatile struct
+{
+  uint64_t alignment;
+  union
+    {
+      uint64_t size8[1];
+      uint32_t size4[2];
+      uint16_t size2[4];
+      uint8_t size1[8];
+    }
+  u;
+} data;
+
+static int size = 0;
+static int offset;
+
+int
+main (void)
+{
+  volatile uint64_t local;
+
+  assert (sizeof (data) == 8 + 8);
+  while (size)
+    {
+      switch (size)
+	{
+	case 8:
+	  local = data.u.size8[offset];
+	  break;
+	case 4:
+	  local = data.u.size4[offset];
+	  break;
+	case 2:
+	  local = data.u.size2[offset];
+	  break;
+	case 1:
+	  local = data.u.size1[offset];
+	  break;
+	default:
+	  assert (0);
+	}
+      size = 0;
+      size = size; /* again_start */
+    }
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
new file mode 100644
index 0000000..ea8b9e3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
@@ -0,0 +1,84 @@ 
+# Copyright 2017 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+#
+# This file is part of the gdb testsuite.
+
+if {![is_aarch64_target] && ![istarget "x86_64-*-*"] && ![istarget i?86-*]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+standard_testfile
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_breakpoint "[gdb_get_line_number "again_start"]" "Breakpoint $decimal at $hex" "again_start"
+
+set sizes {1 2 4 8}
+array set alignedend {1 1  2 2  3 4  4 4  5 8  6 8  7 8  8 8}
+
+foreach wpsize $sizes {
+    for {set wpoffset 0} {$wpoffset < 8/$wpsize} {incr wpoffset} {
+	set wpstart [expr $wpoffset * $wpsize]
+	set wpend [expr ($wpoffset + 1) * $wpsize]
+	set wpendaligned $alignedend($wpend)
+	foreach rdsize $sizes {
+	    for {set rdoffset 0} {$rdoffset < 8/$rdsize} {incr rdoffset} {
+		set rdstart [expr $rdoffset * $rdsize]
+		set rdend [expr ($rdoffset + 1) * $rdsize]
+		set expect_hit [expr max($wpstart,$rdstart) < min($wpend,$rdend)]
+		set test "rwatch data.u.size$wpsize\[$wpoffset\]"
+		set wpnum ""
+		gdb_test_multiple $test $test {
+		    -re "Hardware read watchpoint (\[0-9\]+): .*\r\n$gdb_prompt $" {
+			set wpnum $expect_out(1,string)
+		    }
+		}
+		gdb_test_no_output "set variable size = $rdsize" ""
+		gdb_test_no_output "set variable offset = $rdoffset" ""
+		set test "continue"
+		set did_hit 0
+		gdb_test_multiple $test $test {
+		    -re "Hardware read watchpoint $wpnum:.*Value = .*\r\n$gdb_prompt $" {
+			set did_hit 1
+			send_gdb "continue\n"
+			exp_continue
+		    }
+		    -re " again_start .*\r\n$gdb_prompt $" {
+		    }
+		}
+		gdb_test_no_output "delete $wpnum" ""
+		set test "wp(size=$wpsize offset=$wpoffset) rd(size=$rdsize offset=$rdoffset) expect=$expect_hit"
+		if {$expect_hit == $did_hit} {
+		    pass $test
+		} else {
+		    # We do not know if we run on a fixed kernel or not.
+		    # Report XFAIL only in the FAIL case.
+		    if {$expect_hit == 0 && $rdstart < $wpendaligned} {
+			setup_xfail external/20207 "aarch64-*-*"
+		    }
+		    fail $test
+		}
+	    }
+	}
+    }
+}