This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PING][PATCH PR gdb/21870] aarch64: Leftover uncleared debug registers


On 10/6/2017 3:18 PM, Weimin Pan wrote:
     The root cause is that ptrace() does not validate either
     address or size when setting a hardware watchpoint/breakpoint.
     As a result, watchpoints were set at address 0, the initial value of
     aarch64_debug_reg_state in aarch64_process_info, when the
     PTRACE_SETREGSET request was first made in
     aarch64_linux_set_debug_regs(), in preparation for resuming the thread.

     Other than changing the kernel ptrace() implementation, first attempt to
     fix this problem in gdb was to focus on aarch64_linux_new_thread().
     Instead of marking all hardware breakpoint/watchpoint register pairs for
     the new thread that have changed, tried to reflect the state by using
     either DR_MARK_ALL_CHANGED() if they have really been changed or
     DR_CLEAR_CHANGED() otherwise. But finding whether or not the registers
     have been changed by using parent's lwp_info or aarch64_process_info
     proved to be hard or incorrect, especially the latter which caused
     gdbserver to crash in the middle of the ptid_of_lwp() call.

     Another approach was then taken - add function initial_control_length()
     to validate the contents in the control registers, basing on the fact that
     the kernel only supports Byte Address Select (BAS) values of 0x1, 0x3, 0xf
     and 0xff, before calling ptrace() in aarch64_linux_set_debug_regs().

     Tested on aarch64-linux-gnu. No regressions.
---
  gdb/ChangeLog                    |  7 +++++++
  gdb/nat/aarch64-linux-hw-point.c | 18 +++++++++++++++++-
  2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c4f55a8137..543e1a0487 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-10-06  Weimin Pan  <weimin.pan@oracle.com>
+
+	PR gdb/21870
+	* nat/aarch64-linux-hw-point.c (aarch64_linux_set_debug_regs):
+	Call new function to validate the length in control registers.
+	(initial_control_length): New function.
+
  2017-09-15  Pedro Alves  <palves@redhat.com>
* compile/compile-c-types.c (convert_enum, convert_int)
diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index 9800d9a59c..22c0a48c14 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -548,6 +548,22 @@ aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr,
  						state);
  }
+/* Validate the lengths of breakpoints/watchpoints, according to the
+   contents of these hardware debug control registers, and return
+   true if all these registers contain zero length.  */
+
+static bool
+initial_control_length (const unsigned int *ctrl, int count)
+{
+  for (int i = 0; i < count; i++)
+    {
+      if (DR_CONTROL_LENGTH (ctrl[i]))
+        return false;
+    }
+
+  return true;
+}
+
  /* Call ptrace to set the thread TID's hardware breakpoint/watchpoint
     registers with data from *STATE.  */
@@ -566,7 +582,7 @@ aarch64_linux_set_debug_regs (const struct aarch64_debug_reg_state *state,
    count = watchpoint ? aarch64_num_wp_regs : aarch64_num_bp_regs;
    addr = watchpoint ? state->dr_addr_wp : state->dr_addr_bp;
    ctrl = watchpoint ? state->dr_ctrl_wp : state->dr_ctrl_bp;
-  if (count == 0)
+  if (count == 0 || initial_control_length (ctrl, count))
      return;
    iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs)
  		 + count * sizeof (regs.dbg_regs[0]));


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]