AArch64: Racy: Don't set empty set of hardware BPs/WPs on new thread

On some heavily loaded AArch64 boxes, GDB will sometimes hang forever when
the inferior creates a thread.  This hang happens inside the kernel during
the ptrace call to set hardware watchpoints or hardware breakpoints.
Currently, GDB will always set hw wp/bp at the start of each thread even if
there are none set in the process.

This patch works around the issue by avoiding setting hw wp/bp if there
are none set for the process.

On an effected machine, this fix drastically reduces the racy nature of the
gdb.threads test set.  I ran the entire gdb test suite across all processors
for 100 iterations, then ran the results through the racy tests script.
Without the patch, 58 .exp files in gdb.threads were marked as racy.  After
the patch this reduced to the same ~14 tests as the non effected boxes.

Clearly GDB will still be subject to hangs on an effect box if hw wp/bp's are
used prior to creating inferior threads on a heavily loaded system.

To enable this in gdbserver, the sequence in gdbserver add_lwp() is switched
to the same as gdb order as gdb, to ensure the thread is registered before
calling new_thread().  This allows aarch64_linux_new_thread() to read the
ptid.

gdb/ChangeLog:

	* nat/aarch64-linux-hw-point.c
	(aarch64_linux_any_set_debug_regs_state): New function.
	* nat/aarch64-linux-hw-point.h
	(aarch64_linux_any_set_debug_regs_state): New declaration.
	* nat/aarch64-linux.c (aarch64_linux_new_thread): Check if any
	BPs or WPs are set.

gdb/gdbserver/ChangeLog:

	* linux-low.c (add_lwp): Switch ordering.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 83eabc43c566ffb0a0005862b9e56515a3fbb2bb..d1f583df013468b17a9bc9df2b308882d43c2fc2 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2018-12-05  Alan Hayward  <alan.hayward@arm.com>
+
+       * nat/aarch64-linux-hw-point.c
+       (aarch64_linux_any_set_debug_regs_state): New function.
+       * nat/aarch64-linux-hw-point.h
+       (aarch64_linux_any_set_debug_regs_state): New declaration.
+       * nat/aarch64-linux.c (aarch64_linux_new_thread): Check if any
+       BPs or WPs are set.
+
 2018-11-30  John Baldwin  <jhb@FreeBSD.org>
 
        * common/filestuff.c [HAVE_KINFO_GETFILE]: Include headers.

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index ec4d72f81f8738b9afdbc162fb96d4e01d682157..1e28ed363b581c27cbbd41232737a3c18e5c8bf9 100644 (file)
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2018-12-05  Alan Hayward  <alan.hayward@arm.com>
+
+       * linux-low.c (add_lwp): Switch ordering.
+
 2018-11-29  Tom Tromey  <tom@tromey.com>
 
        * win32-low.c (win32_join): Take pid, not process.

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 4d849279ca3d3e6199b360b4fb45efeb9991989d..9e1faf39b3bd12108179ad08457976b1520c1cd8 100644 (file)
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -951,11 +951,11 @@ add_lwp (ptid_t ptid)
 
   lwp->waitstatus.kind = TARGET_WAITKIND_IGNORE;
 
+  lwp->thread = add_thread (ptid, lwp);
+
   if (the_low_target.new_thread != NULL)
     the_low_target.new_thread (lwp);
 
-  lwp->thread = add_thread (ptid, lwp);
-
   return lwp;
 }
 

diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index 18b5af2d49b7989c7d339bd964c942c1aca7da67..b446e198310160d5200ac6e69f91007de135d6c2 100644 (file)
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -717,6 +717,26 @@ aarch64_linux_set_debug_regs (struct aarch64_debug_reg_state *state,
     }
 }
 
+/* See nat/aarch64-linux-hw-point.h.  */
+
+bool
+aarch64_linux_any_set_debug_regs_state (aarch64_debug_reg_state *state,
+                                       bool watchpoint)
+{
+  int count = watchpoint ? aarch64_num_wp_regs : aarch64_num_bp_regs;
+  if (count == 0)
+    return false;
+
+  const CORE_ADDR *addr = watchpoint ? state->dr_addr_wp : state->dr_addr_bp;
+  const unsigned int *ctrl = watchpoint ? state->dr_ctrl_wp : state->dr_ctrl_bp;
+
+  for (int i = 0; i < count; i++)
+    if (addr[i] != 0 || ctrl[i] != 0)
+      return true;
+
+  return false;
+}
+
 /* Print the values of the cached breakpoint/watchpoint registers.  */
 
 void

diff --git a/gdb/nat/aarch64-linux-hw-point.h b/gdb/nat/aarch64-linux-hw-point.h
index 1940b06a89e025b45df7c9aa4a4c6e51a438989d..c6fdf787ee98c32eb47f7dc02e7f64382addc572 100644 (file)
--- a/gdb/nat/aarch64-linux-hw-point.h
+++ b/gdb/nat/aarch64-linux-hw-point.h
@@ -182,6 +182,11 @@ int aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr,
 void aarch64_linux_set_debug_regs (struct aarch64_debug_reg_state *state,
                                   int tid, int watchpoint);
 
+/* Return TRUE if there are any hardware breakpoints.  If WATCHPOINT is TRUE,
+   check hardware watchpoints instead.  */
+bool aarch64_linux_any_set_debug_regs_state (aarch64_debug_reg_state *state,
+                                            bool watchpoint);
+
 void aarch64_show_debug_reg_state (struct aarch64_debug_reg_state *state,
                                   const char *func, CORE_ADDR addr,
                                   int len, enum target_hw_bp_type type);

diff --git a/gdb/nat/aarch64-linux.c b/gdb/nat/aarch64-linux.c
index 0f2c8011eab361fe69c30ddebaad5160b8b18d41..48c59f6288920291fb0caef9039164e609e6afc7 100644 (file)
--- a/gdb/nat/aarch64-linux.c
+++ b/gdb/nat/aarch64-linux.c
@@ -73,13 +73,18 @@ aarch64_linux_prepare_to_resume (struct lwp_info *lwp)
 void
 aarch64_linux_new_thread (struct lwp_info *lwp)
 {
+  ptid_t ptid = ptid_of_lwp (lwp);
+  struct aarch64_debug_reg_state *state
+    = aarch64_get_debug_reg_state (ptid.pid ());
   struct arch_lwp_info *info = XNEW (struct arch_lwp_info);
 
-  /* Mark that all the hardware breakpoint/watchpoint register pairs
-     for this thread need to be initialized (with data from
-     aarch_process_info.debug_reg_state).  */
-  DR_MARK_ALL_CHANGED (info->dr_changed_bp, aarch64_num_bp_regs);
-  DR_MARK_ALL_CHANGED (info->dr_changed_wp, aarch64_num_wp_regs);
+  /* If there are hardware breakpoints/watchpoints in the process then mark that
+     all the hardware breakpoint/watchpoint register pairs for this thread need
+     to be initialized (with data from aarch_process_info.debug_reg_state).  */
+  if (aarch64_linux_any_set_debug_regs_state (state, false))
+    DR_MARK_ALL_CHANGED (info->dr_changed_bp, aarch64_num_bp_regs);
+  if (aarch64_linux_any_set_debug_regs_state (state, true))
+    DR_MARK_ALL_CHANGED (info->dr_changed_wp, aarch64_num_wp_regs);
 
   lwp_set_arch_private_info (lwp, info);
 }