ping: [patch] aarch64: PR 19806: watchpoints: false negatives + PR 20207 contiguous ones

diff --git a/gdb/NEWS b/gdb/NEWS
index 25c404bfc37..38043c4ff2e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -35,6 +35,16 @@  SH-5/SH64 ELF			sh64-*-elf*, SH-5/SH64 support in sh*
 SH-5/SH64 running GNU/Linux	SH-5/SH64 support in sh*-*-linux*
 SH-5/SH64 running OpenBSD 	SH-5/SH64 support in sh*-*-openbsd*
 
+* Aarch64/Linux hardware watchpoints improvements
+
+  Hardware watchpoints on unaligned addresses are now properly
+  supported when running Linux kernel 4.10 or higher: read and access
+  watchpoints are no longer spuriously missed, and all watchpoints
+  lengths between 1 and 8 bytes are supported.  On older kernels,
+  watchpoints set on unaligned addresses are no longer missed, with
+  the tradeoff that there is a possibility of false hits being
+  reported.
+
 *** Changes in GDB 8.1
 
 * GDB now supports dynamically creating arbitrary register groups specified
@@ -148,11 +158,6 @@  SH-5/SH64 running OpenBSD 	SH-5/SH64 support in sh*-*-openbsd*
 
   Tab completion was adjusted accordingly as well.
 
-* Unaligned hardware watchpoints on aarch64-linux are now properly supported
-  with Linux kernel 4.10 and higher.  On older kernels some unaligned
-  watchpoints are no longer missed - but other unaligned watchpoints may make
-  false hits now.
-
 * Python Scripting
 
   ** New events gdb.new_inferior, gdb.inferior_deleted, and
diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index b69dfb2fce7..421d044e108 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -735,13 +735,12 @@  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 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] + offset;
-      const CORE_ADDR addr_watch_aligned = (state->dr_addr_wp[i]
-					    & -(CORE_ADDR) 8);
+      const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
       const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
 
       if (state->dr_ref_count_wp[i]
@@ -749,14 +748,24 @@  aarch64_linux_stopped_data_address (struct target_ops *target,
 	  && addr_trap >= addr_watch_aligned
 	  && addr_trap < addr_watch + len)
 	{
-	  /* 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_TRAP reports the first address of the memory range
+	     accessed by the CPU, regardless of what was the memory
+	     range watched.  Thus, a large CPU access that straddles
+	     the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
+	     ADDR_TRAP that is lower than the
+	     ADDR_WATCH..ADDR_WATCH+LEN range.  E.g.:
+
+	     addr: |   4   |   5   |   6   |   7   |   8   |
+				   |---- range watched ----|
+		   |----------- range accessed ------------|
+
+	     In this case, ADDR_TRAP will be 4.
+
+	     To match a watchpoint known to GDB core, we must never
+	     report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
+	     range.  ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
+	     positive on kernels older than 4.10.  See PR
+	     external/20207.  */
 	  *addr_p = addr_orig;
 	  return 1;
 	}
diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index 80de826ba78..8d839d10fa8 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -440,3 +440,23 @@  is_regular_file (const char *name, int *errno_ptr)
     *errno_ptr = EINVAL;
   return false;
 }
+
+/* See common/common-utils.h.  */
+
+ULONGEST
+align_up (ULONGEST v, int n)
+{
+  /* Check that N is really a power of two.  */
+  gdb_assert (n && (n & (n-1)) == 0);
+  return (v + n - 1) & -n;
+}
+
+/* See common/common-utils.h.  */
+
+ULONGEST
+align_down (ULONGEST v, int n)
+{
+  /* Check that N is really a power of two.  */
+  gdb_assert (n && (n & (n-1)) == 0);
+  return (v & -n);
+}
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 5408c354693..7bc6e90f05c 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -151,4 +151,36 @@  in_inclusive_range (T value, T low, T high)
    we're expecting a regular file.  */
 extern bool is_regular_file (const char *name, int *errno_ptr);
 
+/* Ensure that V is aligned to an N byte boundary (B's assumed to be a
+   power of 2).  Round up/down when necessary.  Examples of correct
+   use include:
+
+    addr = align_up (addr, 8); -- VALUE needs 8 byte alignment
+    write_memory (addr, value, len);
+    addr += len;
+
+   and:
+
+    sp = align_down (sp - len, 16); -- Keep SP 16 byte aligned
+    write_memory (sp, value, len);
+
+   Note that uses such as:
+
+    write_memory (addr, value, len);
+    addr += align_up (len, 8);
+
+   and:
+
+    sp -= align_up (len, 8);
+    write_memory (sp, value, len);
+
+   are typically not correct as they don't ensure that the address (SP
+   or ADDR) is correctly aligned (relying on previous alignment to
+   keep things right).  This is also why the methods are called
+   "align_..." instead of "round_..." as the latter reads better with
+   this incorrect coding style.  */
+
+extern ULONGEST align_up (ULONGEST v, int n);
+extern ULONGEST align_down (ULONGEST v, int n);
+
 #endif
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 133572ef0c5..7ea24c23634 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -360,13 +360,12 @@  aarch64_stopped_data_address (void)
   state = aarch64_get_debug_reg_state (pid_of (current_thread));
   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 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] + offset;
-      const CORE_ADDR addr_watch_aligned = (state->dr_addr_wp[i]
-					    & -(CORE_ADDR) 8);
+      const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
       const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
 
       if (state->dr_ref_count_wp[i]
@@ -374,14 +373,24 @@  aarch64_stopped_data_address (void)
 	  && addr_trap >= addr_watch_aligned
 	  && addr_trap < addr_watch + len)
 	{
-	  /* 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.  */
+	  /* ADDR_TRAP reports the first address of the memory range
+	     accessed by the CPU, regardless of what was the memory
+	     range watched.  Thus, a large CPU access that straddles
+	     the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
+	     ADDR_TRAP that is lower than the
+	     ADDR_WATCH..ADDR_WATCH+LEN range.  E.g.:
+
+	     addr: |   4   |   5   |   6   |   7   |   8   |
+				   |---- range watched ----|
+		   |----------- range accessed ------------|
+
+	     In this case, ADDR_TRAP will be 4.
+
+	     To match a watchpoint known to GDB core, we must never
+	     report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
+	     range.  ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
+	     positive on kernels older than 4.10.  See PR
+	     external/20207.  */
 	  return addr_orig;
 	}
     }
diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index ef0022ac0e1..a3931ea6a94 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -34,10 +34,13 @@ 
 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);
+/* True if this kernel does not have the bug described by PR
+   external/20207 (Linux >= 4.10).  A fixed kernel supports any
+   contiguous range of bits in 8-bit byte DR_CONTROL_MASK.  A buggy
+   kernel supports only 0x01, 0x03, 0x0f and 0xff.  We start by
+   assuming the bug is fixed, and then detect the bug at
+   PTRACE_SETREGSET time.  */
+static bool kernel_supports_any_contiguous_range = true;
 
 /* Return starting byte 0..7 incl. of a watchpoint encoded by CTRL.  */
 
@@ -47,7 +50,7 @@  aarch64_watchpoint_offset (unsigned int ctrl)
   uint8_t mask = DR_CONTROL_MASK (ctrl);
   unsigned retval;
 
-  // Shift out bottom zeroes.
+  /* Shift out bottom zeros.  */
   for (retval = 0; mask && (mask & 1) == 0; ++retval)
     mask >>= 1;
 
@@ -56,8 +59,8 @@  aarch64_watchpoint_offset (unsigned int ctrl)
 
 /* Utility function that returns the length in bytes of a watchpoint
    according to the content of a hardware debug control register CTRL.
-   Any contiguous range of bytes in CTRL is supported.  Returned value
-   can be between 0..8 incl.  */
+   Any contiguous range of bytes in CTRL is supported.  The returned
+   value can be between 0..8 (inclusive).  */
 
 unsigned int
 aarch64_watchpoint_length (unsigned int ctrl)
@@ -65,16 +68,16 @@  aarch64_watchpoint_length (unsigned int ctrl)
   uint8_t mask = DR_CONTROL_MASK (ctrl);
   unsigned retval;
 
-  // Shift out bottom zeroes.
+  /* Shift out bottom zeros.  */
   mask >>= aarch64_watchpoint_offset (ctrl);
 
-  // Count bottom ones;
+  /* Count bottom ones.  */
   for (retval = 0; (mask & 1) != 0; ++retval)
     mask >>= 1;
 
-  if (mask)
+  if (mask != 0)
     error (_("Unexpected hardware watchpoint length register value 0x%x"),
-      DR_CONTROL_MASK (ctrl));
+	   DR_CONTROL_MASK (ctrl));
 
   return retval;
 }
@@ -88,7 +91,7 @@  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 == 0 || kernel_supports_any_contiguous_range);
   gdb_assert (offset + len <= AARCH64_HWP_MAX_LEN_PER_REG);
 
   /* type */
@@ -157,51 +160,56 @@  aarch64_point_is_aligned (int is_watchpoint, CORE_ADDR addr, int len)
   if (addr & (alignment - 1))
     return 0;
 
-  if ((!have_any_contiguous && len != 8 && len != 4 && len != 2 && len != 1)
-      || (have_any_contiguous && (len < 1 || len > 8)))
+  if ((!kernel_supports_any_contiguous_range
+       && len != 8 && len != 4 && len != 2 && len != 1)
+      || (kernel_supports_any_contiguous_range
+	  && (len < 1 || len > 8)))
     return 0;
 
   return 1;
 }
 
 /* Given the (potentially unaligned) watchpoint address in ADDR and
-   length in LEN, return the aligned address and aligned length in
-   *ALIGNED_ADDR_P and *ALIGNED_LEN_P, respectively.  The returned
-   aligned address and length will be valid values to write to the
-   hardware watchpoint value and control registers.
+   length in LEN, return the aligned address, offset from that base
+   address, and aligned length in *ALIGNED_ADDR_P, *ALIGNED_OFFSET_P
+   and *ALIGNED_LEN_P, respectively.  The returned values will be
+   valid values to write to the hardware watchpoint value and control
+   registers.
 
    The given watchpoint may get truncated if more than one hardware
    register is needed to cover the watched region.  *NEXT_ADDR_P
    and *NEXT_LEN_P, if non-NULL, will return the address and length
    of the remaining part of the watchpoint (which can be processed
-   by calling this routine again to generate another aligned address
-   and length pair.
+   by calling this routine again to generate another aligned address,
+   offset and length tuple.
 
    Essentially, unaligned watchpoint is achieved by minimally
    enlarging the watched area to meet the alignment requirement, and
    if necessary, splitting the watchpoint over several hardware
-   watchpoint registers.  The trade-off is that there will be
-   false-positive hits for the read-type or the access-type hardware
-   watchpoints; for the write type, which is more commonly used, there
-   will be no such issues, as the higher-level breakpoint management
-   in gdb always examines the exact watched region for any content
-   change, and transparently resumes a thread from a watchpoint trap
-   if there is no change to the watched region.
+   watchpoint registers.
+
+   On kernels that predate the support for Byte Address Select (BAS)
+   in the hardware watchpoint control register, the offset from the
+   base address is always zero, and so in that case the trade-off is
+   that there will be false-positive hits for the read-type or the
+   access-type hardware watchpoints; for the write type, which is more
+   commonly used, there will be no such issues, as the higher-level
+   breakpoint management in gdb always examines the exact watched
+   region for any content change, and transparently resumes a thread
+   from a watchpoint trap if there is no change to the watched region.
 
    Another limitation is that because the watched region is enlarged,
-   the watchpoint fault address returned by
+   the watchpoint fault address discovered by
    aarch64_stopped_data_address may be outside of the original watched
    region, especially when the triggering instruction is accessing a
    larger region.  When the fault address is not within any known
    range, watchpoints_triggered in gdb will get confused, as the
    higher-level watchpoint management is only aware of original
    watched regions, and will think that some unknown watchpoint has
-   been triggered.  In such a case, gdb may stop without displaying
-   any detailed information.
-
-   Once the kernel provides the full support for Byte Address Select
-   (BAS) in the hardware watchpoint control register, these
-   limitations can be largely relaxed with some further work.  */
+   been triggered.  To prevent such a case,
+   aarch64_stopped_data_address implementations in gdb and gdbserver
+   try to match the trapped address with a watched region, and return
+   an address within the latter. */
 
 static void
 aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
@@ -221,11 +229,12 @@  aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
   if (len <= 0)
     return;
 
-  /* Address to be put into the hardware watchpoint value register
-     must be aligned.  */
+  /* The address put into the hardware watchpoint value register must
+     be aligned.  */
   offset = addr & (alignment - 1);
   aligned_addr = addr - offset;
-  aligned_offset = have_any_contiguous ? addr & (alignment - 1) : 0;
+  aligned_offset
+    = kernel_supports_any_contiguous_range ? addr & (alignment - 1) : 0;
 
   gdb_assert (offset >= 0 && offset < alignment);
   gdb_assert (aligned_addr >= 0 && aligned_addr <= addr);
@@ -233,9 +242,10 @@  aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
 
   if (offset + len >= max_wp_len)
     {
-      /* Need more than one watchpoint registers; truncate it at the
+      /* Need more than one watchpoint register; truncate at the
 	 alignment boundary.  */
-      aligned_len = max_wp_len - (have_any_contiguous ? offset : 0);
+      aligned_len
+	= max_wp_len - (kernel_supports_any_contiguous_range ? offset : 0);
       len -= (max_wp_len - offset);
       addr += (max_wp_len - offset);
       gdb_assert ((addr & (alignment - 1)) == 0);
@@ -248,7 +258,7 @@  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 = (have_any_contiguous
+      aligned_len = (kernel_supports_any_contiguous_range
 		     ? len : aligned_len_array[offset + len - 1]);
       addr += len;
       len = 0;
@@ -265,8 +275,7 @@  aarch64_align_watchpoint (CORE_ADDR addr, int len, CORE_ADDR *aligned_addr_p,
   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);
+    *next_addr_orig_p = align_down (*next_addr_orig_p + alignment, alignment);
 }
 
 struct aarch64_dr_update_callback_param
@@ -356,13 +365,15 @@  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).  */
+/* Reconfigure STATE to be compatible with Linux kernels with the PR
+   external/20207 bug.  This is called when
+   KERNEL_SUPPORTS_ANY_CONTIGUOUS_RANGE transitions to false.  Note we
+   don't try to support combining watchpoints with matching (and thus
+   shared) masks, as it's too late when we get here.  On buggy
+   kernels, GDB will try to first setup the perfect matching ranges,
+   which will run out of registers before this function can merge
+   them.  It doesn't look like worth the effort to improve that, given
+   eventually buggy kernels will be phased out.  */
 
 static void
 aarch64_downgrade_regs (struct aarch64_debug_reg_state *state)
@@ -373,9 +384,9 @@  aarch64_downgrade_regs (struct aarch64_debug_reg_state *state)
 	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)
+	static const uint8_t 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;
@@ -383,13 +394,14 @@  aarch64_downgrade_regs (struct aarch64_debug_reg_state *state)
 	    }
 	gdb_assert (mask != 0);
 
-	// No update needed for this watchpoint?
+	/* 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;
+	state->dr_addr_wp[i]
+	  = align_down (state->dr_addr_wp[i], AARCH64_HWP_ALIGNMENT);
 
-	// Try to match duplicate entries.
+	/* 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]
@@ -692,10 +704,11 @@  aarch64_linux_set_debug_regs (struct aarch64_debug_reg_state *state,
 	      watchpoint ? NT_ARM_HW_WATCH : NT_ARM_HW_BREAK,
 	      (void *) &iov))
     {
-      // Handle PR external/20207 buggy kernels?
-      if (watchpoint && errno == EINVAL && have_any_contiguous)
+      /* Handle Linux kernels with the PR external/20207 bug.  */
+      if (watchpoint && errno == EINVAL
+	  && kernel_supports_any_contiguous_range)
 	{
-	  have_any_contiguous = false;
+	  kernel_supports_any_contiguous_range = false;
 	  aarch64_downgrade_regs (state);
 	  aarch64_linux_set_debug_regs (state, tid, watchpoint);
 	  return;
diff --git a/gdb/nat/aarch64-linux-hw-point.h b/gdb/nat/aarch64-linux-hw-point.h
index 7c3513e301e..1940b06a89e 100644
--- a/gdb/nat/aarch64-linux-hw-point.h
+++ b/gdb/nat/aarch64-linux-hw-point.h
@@ -147,9 +147,9 @@  struct aarch64_debug_reg_state
   unsigned int dr_ref_count_bp[AARCH64_HBP_MAX_NUM];
 
   /* hardware watchpoint */
-  // Address aligned down to AARCH64_HWP_ALIGNMENT.
+  /* Address aligned down to AARCH64_HWP_ALIGNMENT.  */
   CORE_ADDR dr_addr_wp[AARCH64_HWP_MAX_NUM];
-  // Address as entered by user without any alignment.
+  /* Address as entered by user without any forced alignment.  */
   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];
diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.c b/gdb/testsuite/gdb.base/watchpoint-unaligned.c
index f653e77529d..ea844e947f6 100644
--- a/gdb/testsuite/gdb.base/watchpoint-unaligned.c
+++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.c
@@ -1,6 +1,6 @@ 
 /* This testcase is part of GDB, the GNU debugger.
 
-   Copyright 2017 Free Software Foundation, Inc.
+   Copyright 2017-2018 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
@@ -62,7 +62,7 @@  main (void)
 	  assert (0);
 	}
       size = 0;
-      size = size; /* again_start */
+      size = size; /* start_again */
     }
   return 0;
 }
diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
index 8d463b12fad..833997b04d7 100644
--- a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
@@ -1,4 +1,4 @@ 
-# Copyright 2017 Free Software Foundation, Inc.
+# Copyright 2017-2018 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
@@ -11,11 +11,12 @@ 
 # 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.
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 # This file is part of the gdb testsuite.
 
+# Test inserting read watchpoints on unaligned addresses.
+
 standard_testfile
 if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
     return -1
@@ -26,21 +27,21 @@  if ![runto_main] {
     return -1
 }
 
-gdb_breakpoint "[gdb_get_line_number "again_start"]" "Breakpoint $decimal at $hex" "again_start"
+gdb_breakpoint [gdb_get_line_number "start_again"] "Breakpoint $decimal at $hex" "start_again"
 
 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} {
+    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} {
+	    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 expect_hit [expr max ($wpstart, $rdstart) < min ($wpend, $rdend)]
 		set test "rwatch data.u.size$wpsize\[$wpoffset\]"
 		set wpnum ""
 		gdb_test_multiple $test $test {
@@ -51,25 +52,25 @@  foreach wpsize $sizes {
 		gdb_test_no_output "set variable size = $rdsize" ""
 		gdb_test_no_output "set variable offset = $rdoffset" ""
 		set test "continue"
-		set did_hit 0
+		set got_hit 0
 		gdb_test_multiple $test $test {
 		    -re "Hardware read watchpoint $wpnum:.*Value = .*\r\n$gdb_prompt $" {
-			set did_hit 1
+			set got_hit 1
 			send_gdb "continue\n"
 			exp_continue
 		    }
-		    -re " again_start .*\r\n$gdb_prompt $" {
+		    -re " start_again .*\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} {
+		if {$expect_hit == $got_hit} {
 		    pass $test
 		} else {
-		    # We do not know if we run on a fixed kernel or not.
-		    # Report XFAIL only in the FAIL case.
+		    # We do not know if we run on a fixed Linux kernel
+		    # or not.  Report XFAIL only in the FAIL case.
 		    if {$expect_hit == 0 && $rdstart < $wpendaligned} {
-			setup_xfail external/20207 "aarch64-*-*"
+			setup_xfail external/20207 "aarch64*-*-linux*"
 		    }
 		    fail $test
 		}
@@ -92,16 +93,16 @@  foreach wpcount {4 7} {
     gdb_test_no_output "set variable size = 1" ""
     gdb_test_no_output "set variable offset = 1" ""
     set test "continue"
-    set did_hit 0
+    set got_hit 0
     gdb_test_multiple $test $test {
 	-re "\r\nCould not insert hardware watchpoint .*\r\n$gdb_prompt $" {
 	}
 	-re "Hardware read watchpoint $wpoffset_to_wpnum(1):.*Value = .*\r\n$gdb_prompt $" {
-	    set did_hit 1
+	    set got_hit 1
 	    send_gdb "continue\n"
 	    exp_continue
 	}
-	-re " again_start .*\r\n$gdb_prompt $" {
+	-re " start_again .*\r\n$gdb_prompt $" {
 	}
     }
     for {set wpoffset 1} {$wpoffset <= $wpcount} {incr wpoffset} {
@@ -111,9 +112,5 @@  foreach wpcount {4 7} {
     if {$wpcount > 4} {
 	setup_kfail tdep/22389 *-*-*
     }
-    if {$did_hit} {
-	pass $test
-    } else {
-	fail $test
-    }
+    gdb_assert $got_hit $test
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index b957b0dc5c2..e274f026764 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2849,22 +2849,6 @@  gdb_realpath_tests ()
 
 #endif /* GDB_SELF_TEST */
 
-ULONGEST
-align_up (ULONGEST v, int n)
-{
-  /* Check that N is really a power of two.  */
-  gdb_assert (n && (n & (n-1)) == 0);
-  return (v + n - 1) & -n;
-}
-
-ULONGEST
-align_down (ULONGEST v, int n)
-{
-  /* Check that N is really a power of two.  */
-  gdb_assert (n && (n & (n-1)) == 0);
-  return (v & -n);
-}
-
 /* Allocation function for the libiberty hash table which uses an
    obstack.  The obstack is passed as DATA.  */
 
diff --git a/gdb/utils.h b/gdb/utils.h
index 4dec889d83e..c728449429e 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -494,38 +494,6 @@  extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout);
 
 extern int myread (int, char *, int);
 
-/* Ensure that V is aligned to an N byte boundary (B's assumed to be a
-   power of 2).  Round up/down when necessary.  Examples of correct
-   use include:
-
-   addr = align_up (addr, 8); -- VALUE needs 8 byte alignment
-   write_memory (addr, value, len);
-   addr += len;
-
-   and:
-
-   sp = align_down (sp - len, 16); -- Keep SP 16 byte aligned
-   write_memory (sp, value, len);
-
-   Note that uses such as:
-
-   write_memory (addr, value, len);
-   addr += align_up (len, 8);
-
-   and:
-
-   sp -= align_up (len, 8);
-   write_memory (sp, value, len);
-
-   are typically not correct as they don't ensure that the address (SP
-   or ADDR) is correctly aligned (relying on previous alignment to
-   keep things right).  This is also why the methods are called
-   "align_..." instead of "round_..." as the latter reads better with
-   this incorrect coding style.  */
-
-extern ULONGEST align_up (ULONGEST v, int n);
-extern ULONGEST align_down (ULONGEST v, int n);
-
 /* Resource limits used by getrlimit and setrlimit.  */
 
 enum resource_limit_kind