At issue here 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, and DR_CONTROL_LENGTH (dreg_state.dbg_regs[i].ctrl)
got changed to a non-zero default value, when the PTRACE_SETREGSET request
was made in aarch64_linux_set_debug_regs(), in preparation for resuming
the thread.
I sent out a patch upstream for review last year. Yao Qi commented that
he needed to take a look at ptrace implementation to better understand
why this was happening. Unfortunately he didn't get a chance to do so.
This is a revised patch which calls ptrace only if any debug register
has changed, either address or control containing a non-zero value,
in aarch64_linux_set_debug_regs() and is similar to what x86 does -
x86_linux_update_debug_registers() checks if a debug register has
non-zero reference count before setting it.
Tested on aarch64-linux-gnu. No regressions.
---
gdb/ChangeLog | 6 +
gdb/nat/aarch64-linux-hw-point.c | 7 +
gdb/testsuite/ChangeLog | 20 ++-
gdb/testsuite/gdb.arch/aarch64-dbreg-contents.c | 179 ++++++++++++++++++++++
gdb/testsuite/gdb.arch/aarch64-dbreg-contents.exp | 40 +++++
5 files changed, 244 insertions(+), 8 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-dbreg-contents.c
create mode 100644 gdb/testsuite/gdb.arch/aarch64-dbreg-contents.exp
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7b108bf..d5bf15f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-06-27 Weimin Pan <weimin.pan@oracle.com>
+
+ PR gdb/21870
+ * nat/aarch64-linux-hw-point.c (aarch64_linux_set_debug_regs):
+ Validate debug register contents before calling ptrace.
+
2018-06-18 Tom Tromey <tom@tromey.com>
* solib-aix.c (solib_aix_get_section_offsets): Return
diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index a3931ea..41ac304 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -694,11 +694,18 @@ aarch64_linux_set_debug_regs (struct aarch64_debug_reg_state *state,
iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs)
+ count * sizeof (regs.dbg_regs[0]));
+ int changed_regs = 0;
for (i = 0; i < count; i++)
{
regs.dbg_regs[i].addr = addr[i];
regs.dbg_regs[i].ctrl = ctrl[i];
+ /* Check to see if any of these debug registers contains valid data,
+ e.g. non-zero, before calling ptrace. See PR gdb/21870. */
+ if (ctrl[i] || addr[i])
+ changed_regs++;
}
+ if (changed_regs == 0)
+ return;
if (ptrace (PTRACE_SETREGSET, tid,
watchpoint ? NT_ARM_HW_WATCH : NT_ARM_HW_BREAK,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index a812061..0ce2d34 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,4 +1,9 @@
+2018-06-27 Weimin Pan <weimin.pan@oracle.com>
+
+ PR gdb/21870
+ * gdb.arch/aarch64-dbreg-contents.c: New file.
+ * gdb.arch/aarch64-dbreg-contents.exp: New file.
+
2018-06-18 Tom de Vries <tdevries@suse.de>
* gdb.ada/bp_inlined_func.exp: Allow 5 breakpoint locations.
diff --git a/gdb/testsuite/gdb.arch/aarch64-dbreg-contents.c b/gdb/testsuite/gdb.arch/aarch64-dbreg-contents.c
new file mode 100644
index 0000000..85d4a03
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-dbreg-contents.c
@@ -0,0 +1,179 @@
+/* Test case for setting a memory-write unaligned watchpoint on aarch64.
+
+ This software is provided 'as-is', without any express or implied
+ warranty. In no event will the authors be held liable for any damages
+ arising from the use of this software.
+
+ Permission is granted to anyone to use this software for any purpose,
+ including commercial applications, and to alter it and redistribute it
+ freely. */
+
+#define _GNU_SOURCE 1
+#ifdef __ia64__
+#define ia64_fpreg ia64_fpreg_DISABLE
+#define pt_all_user_regs pt_all_user_regs_DISABLE
+#endif /* __ia64__ */
+#include <sys/ptrace.h>
+#ifdef __ia64__
+#undef ia64_fpreg
+#undef pt_all_user_regs
+#endif /* __ia64__ */
+#include <linux/ptrace.h>
+#include <sys/types.h>
+#include <sys/user.h>
+#if defined __i386__ || defined __x86_64__
+#include <sys/debugreg.h>
+#endif
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <errno.h>
+#include <sys/uio.h>
+#include <elf.h>
+#include <error.h>
+
+static __attribute__((unused)) pid_t child;
+
+static __attribute__((unused)) void
+cleanup (void)
+{
+ if (child > 0)
+ kill (child, SIGKILL);
+ child = 0;
+}
+
+static __attribute__((unused)) void
+handler_fail (int signo)
+{
+ cleanup ();
+ signal (signo, SIG_DFL);
+ raise (signo);
+}
+
+#ifdef __aarch64__
+
+#define SET_WATCHPOINT set_watchpoint
+
+/* Macros to extract fields from the hardware debug information word. */
+#define AARCH64_DEBUG_NUM_SLOTS(x) ((x) & 0xff)
+#define AARCH64_DEBUG_ARCH(x) (((x) >> 8) & 0xff)
+/* Macro for the expected version of the ARMv8-A debug architecture. */
+#define AARCH64_DEBUG_ARCH_V8 0x6
+#define DR_CONTROL_ENABLED(ctrl) (((ctrl) & 0x1) == 1)
+#define DR_CONTROL_LENGTH(ctrl) (((ctrl) >> 5) & 0xff)
+
+static void
+set_watchpoint (pid_t pid, volatile void *addr, unsigned len_mask)
+{
+ struct user_hwdebug_state dreg_state;
+ struct iovec iov;
+ long l;
+
+ assert (len_mask >= 0x01);
+ assert (len_mask <= 0xff);
+
+ iov.iov_base = &dreg_state;
+ iov.iov_len = sizeof (dreg_state);
+ errno = 0;
+ l = ptrace (PTRACE_GETREGSET, pid, NT_ARM_HW_WATCH, &iov);
+ assert (l == 0);
+ assert (AARCH64_DEBUG_ARCH (dreg_state.dbg_info) == AARCH64_DEBUG_ARCH_V8);
+ assert (AARCH64_DEBUG_NUM_SLOTS (dreg_state.dbg_info) >= 1);
+
+ assert (!DR_CONTROL_ENABLED (dreg_state.dbg_regs[0].ctrl));
+ dreg_state.dbg_regs[0].ctrl |= 1;
+ assert ( DR_CONTROL_ENABLED (dreg_state.dbg_regs[0].ctrl));
+
+ assert (DR_CONTROL_LENGTH (dreg_state.dbg_regs[0].ctrl) == 0);
+ dreg_state.dbg_regs[0].ctrl |= len_mask << 5;
+ assert (DR_CONTROL_LENGTH (dreg_state.dbg_regs[0].ctrl) == len_mask);
+
+ dreg_state.dbg_regs[0].ctrl |= 2 << 3; // write
+ dreg_state.dbg_regs[0].ctrl |= 2 << 1; // GDB: ???: enabled at el0
+ //printf("ctrl=0x%x\n",dreg_state.dbg_regs[0].ctrl);
+ dreg_state.dbg_regs[0].addr = (uintptr_t) addr;
+
+ iov.iov_base = &dreg_state;
+ iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs)
+ + sizeof (dreg_state.dbg_regs[0]));
+ errno = 0;
+ l = ptrace (PTRACE_SETREGSET, pid, NT_ARM_HW_WATCH, &iov);
+ if (errno != 0)
+ error (1, errno, "PTRACE_SETREGSET: NT_ARM_HW_WATCH");
+ assert (l == 0);
+}
+
+#endif
+
+#ifndef SET_WATCHPOINT
+
+int
+main (void)
+{
+ return 77;
+}
+#else
+
+static volatile long long check;
+
+int
+main (void)
+{
+ pid_t got_pid;
+ int i, status;
+ long l;
+
+ atexit (cleanup);
+ signal (SIGABRT, handler_fail);
+ signal (SIGINT, handler_fail);
+
+ child = fork ();
+ assert (child >= 0);
+ if (child == 0)
+ {
+ l = ptrace (PTRACE_TRACEME, 0, NULL, NULL);
+ assert (l == 0);
+ i = raise (SIGUSR1);
+ assert (i == 0);
+ check = -1;
+ i = raise (SIGUSR2);
+ /* NOTREACHED */
+ assert (0);
+ }
+
+ got_pid = waitpid (child, &status, 0);
+ assert (got_pid == child);
+ assert (WIFSTOPPED (status));
+ assert (WSTOPSIG (status) == SIGUSR1);
+
+ // PASS:
+ //SET_WATCHPOINT (child, &check, 0xff);
+ // FAIL:
+ SET_WATCHPOINT (child, &check, 0x02);
+
+ errno = 0;
+ l = ptrace (PTRACE_CONT, child, 0l, 0l);
+ assert_perror (errno);
+ assert (l == 0);
+
+ got_pid = waitpid (child, &status, 0);
+ assert (got_pid == child);
+ assert (WIFSTOPPED (status));
+ if (WSTOPSIG (status) == SIGUSR2)
+ {
+ /* We missed the watchpoint - unsupported by hardware? */
+ cleanup ();
+ return 2;
+ }
+ assert (WSTOPSIG (status) == SIGTRAP);
+
+ cleanup ();
+ return 0;
+}
+
+#endif
+
diff --git a/gdb/testsuite/gdb.arch/aarch64-dbreg-contents.exp b/gdb/testsuite/gdb.arch/aarch64-dbreg-contents.exp
new file mode 100644
index 0000000..6aa23b0
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-dbreg-contents.exp
@@ -0,0 +1,40 @@
+# Copyright 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
+# 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/>.
+#
+# Make sure that the inferior doesn't assert and exits successfully.
+
+if {![is_aarch64_target]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile .c
+
+if { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile} executable {debug}] != "" } {
+ untested "failed to compile"
+ return -1
+}
+
+clean_restart $testfile
+
+set test "run to exit"
+gdb_test_multiple "run" "$test" {
+ -re "exited with code 01.*$gdb_prompt $" {
+ pass "$test"
+ }
+ -re "exited normally.*$gdb_prompt $" {
+ pass "$test"
+ }
+}