File gdb-tdep-fix-arm-thumb2-hw-breakpoint.patch of Package gdb
From 64dc13a4571b4092726291f3ee30bf5c2166fa13 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Sat, 27 Jul 2024 10:05:20 +0200
Subject: [PATCH 09/46] [gdb/tdep] Fix arm thumb2 hw breakpoint
On an aarch64-linux system with 32-bit userland running in a chroot, and using
target board unix/mthumb I get:
...
(gdb) hbreak hbreak.c:27^M
Hardware assisted breakpoint 2 at 0x4004e2: file hbreak.c, line 27.^M
(gdb) PASS: gdb.base/hbreak.exp: hbreak
continue^M
Continuing.^M
Unexpected error setting breakpoint: Invalid argument.^M
(gdb) XFAIL: gdb.base/hbreak.exp: continue to break-at-exit after hbreak
...
due to this call in arm_linux_nat_target::low_prepare_to_resume:
...
if (ptrace (PTRACE_SETHBPREGS, pid,
(PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
perror_with_name (_("Unexpected error setting breakpoint"));
...
This problem does not happen if instead we use a 4-byte aligned address.
This may or may not be a kernel bug.
Work around this by first using an inoffensive address bpts[i].address & ~0x7.
Likewise in arm_target::low_prepare_to_resume, which fixes the same fail on
target board native-gdbserver/mthumb.
While we're at it:
- use arm_hwbp_control_is_initialized in
arm_linux_nat_target::low_prepare_to_resume,
- handle the !arm_hwbp_control_is_initialized case explicitly,
- add missing '_()' in arm_target::low_prepare_to_resume,
- make error messages identical between arm_target::low_prepare_to_resume and
arm_linux_nat_target::low_prepare_to_resume,
- factor out sethbpregs_hwbp_address and sethbpregs_hwbp_control to
make the implementation more readable.
Remove the tentative xfail added in d0af16d5a10 ("[gdb/testsuite] Add xfail in
gdb.base/hbreak.exp") by simply reverting the commit.
Tested on arm-linux.
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
---
gdb/arm-linux-nat.c | 98 ++++++++++++++++++++++++++++++++++----
gdbserver/linux-arm-low.cc | 65 ++++++++++++++++++++-----
2 files changed, 141 insertions(+), 22 deletions(-)
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index ac53bed72d7..7b4faacd601 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -876,6 +876,14 @@ arm_hwbp_control_is_enabled (arm_hwbp_control_t control)
return control & 0x1;
}
+/* Is the breakpoint control value CONTROL initialized? */
+
+static int
+arm_hwbp_control_is_initialized (arm_hwbp_control_t control)
+{
+ return control != 0;
+}
+
/* Change a breakpoint control word so that it is in the disabled state. */
static arm_hwbp_control_t
arm_hwbp_control_disable (arm_hwbp_control_t control)
@@ -1234,6 +1242,34 @@ arm_linux_nat_target::low_delete_thread (struct arch_lwp_info *arch_lwp)
xfree (arch_lwp);
}
+/* For PID, set the address register of hardware breakpoint pair I to
+ ADDRESS. */
+
+static void
+sethbpregs_hwbp_address (int pid, int i, unsigned int address)
+{
+ PTRACE_TYPE_ARG3 address_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 1);
+
+ errno = 0;
+
+ if (ptrace (PTRACE_SETHBPREGS, pid, address_reg, &address) < 0)
+ perror_with_name (_("Unexpected error updating breakpoint address"));
+}
+
+/* For PID, set the control register of hardware breakpoint pair I to
+ CONTROL. */
+
+static void
+sethbpregs_hwbp_control (int pid, int i, arm_hwbp_control_t control)
+{
+ PTRACE_TYPE_ARG3 control_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 2);
+
+ errno = 0;
+
+ if (ptrace (PTRACE_SETHBPREGS, pid, control_reg, &control) < 0)
+ perror_with_name (_("Unexpected error setting breakpoint control"));
+}
+
/* Called when resuming a thread.
The hardware debug registers are updated when there is any change. */
@@ -1257,16 +1293,58 @@ arm_linux_nat_target::low_prepare_to_resume (struct lwp_info *lwp)
for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
if (arm_lwp_info->bpts_changed[i])
{
- errno = 0;
- if (arm_hwbp_control_is_enabled (bpts[i].control))
- if (ptrace (PTRACE_SETHBPREGS, pid,
- (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
- perror_with_name (_("Unexpected error setting breakpoint"));
-
- if (bpts[i].control != 0)
- if (ptrace (PTRACE_SETHBPREGS, pid,
- (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
- perror_with_name (_("Unexpected error setting breakpoint"));
+ unsigned int address = bpts[i].address;
+ arm_hwbp_control_t control = bpts[i].control;
+
+ if (!arm_hwbp_control_is_initialized (control))
+ {
+ /* Nothing to do. */
+ }
+ else if (!arm_hwbp_control_is_enabled (control))
+ {
+ /* Disable hardware breakpoint, just write the control
+ register. */
+ sethbpregs_hwbp_control (pid, i, control);
+ }
+ else
+ {
+ /* We used to do here simply:
+ 1. address_reg = address
+ 2. control_reg = control
+ but the write to address_reg can fail for thumb2 instructions if
+ the address is not 4-byte aligned.
+
+ It's not clear whether this is a kernel bug or not, partly
+ because PTRACE_SETHBPREGS is undocumented.
+
+ The context is that we're using two ptrace calls to set the two
+ halves of a register pair. For each ptrace call, the kernel must
+ check the arguments, and return -1 and set errno appropriately if
+ something is wrong. One of the aspects that needs validation is
+ whether, in terms of hw_breakpoint_arch_parse, the breakpoint
+ address matches the breakpoint length. This aspect can only be
+ checked by looking in both registers, which only makes sense
+ once a pair is written in full.
+
+ The problem is that the kernel checks this aspect after each
+ ptrace call, and consequently for the first call it may be
+ checking this aspect using a default or previous value for the
+ part of the pair not written by the call. A possible fix for
+ this would be to only check this aspect when writing the
+ control reg.
+
+ Work around this by first using an inoffensive address, which is
+ guaranteed to hit the offset == 0 case in
+ hw_breakpoint_arch_parse. */
+ unsigned int aligned_address = address & ~0x7U;
+ if (aligned_address != address)
+ {
+ sethbpregs_hwbp_address (pid, i, aligned_address);
+ sethbpregs_hwbp_control (pid, i, control);
+ }
+ sethbpregs_hwbp_address (pid, i, address);
+ sethbpregs_hwbp_control (pid, i, control);
+ }
arm_lwp_info->bpts_changed[i] = 0;
}
diff --git a/gdbserver/linux-arm-low.cc b/gdbserver/linux-arm-low.cc
index eec4649b235..ee89949a2a2 100644
--- a/gdbserver/linux-arm-low.cc
+++ b/gdbserver/linux-arm-low.cc
@@ -819,6 +819,34 @@ arm_target::low_new_fork (process_info *parent, process_info *child)
child_lwp_info->wpts_changed[i] = 1;
}
+/* For PID, set the address register of hardware breakpoint pair I to
+ ADDRESS. */
+
+static void
+sethbpregs_hwbp_address (int pid, int i, unsigned int address)
+{
+ PTRACE_TYPE_ARG3 address_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 1);
+
+ errno = 0;
+
+ if (ptrace (PTRACE_SETHBPREGS, pid, address_reg, &address) < 0)
+ perror_with_name (_("Unexpected error updating breakpoint address"));
+}
+
+/* For PID, set the control register of hardware breakpoint pair I to
+ CONTROL. */
+
+static void
+sethbpregs_hwbp_control (int pid, int i, arm_hwbp_control_t control)
+{
+ PTRACE_TYPE_ARG3 control_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 2);
+
+ errno = 0;
+
+ if (ptrace (PTRACE_SETHBPREGS, pid, control_reg, &control) < 0)
+ perror_with_name (_("Unexpected error setting breakpoint control"));
+}
+
/* Called when resuming a thread.
If the debug regs have changed, update the thread's copies. */
void
@@ -834,19 +862,32 @@ arm_target::low_prepare_to_resume (lwp_info *lwp)
for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
if (lwp_info->bpts_changed[i])
{
- errno = 0;
+ unsigned int address = proc_info->bpts[i].address;
+ arm_hwbp_control_t control = proc_info->bpts[i].control;
- if (arm_hwbp_control_is_enabled (proc_info->bpts[i].control))
- if (ptrace (PTRACE_SETHBPREGS, pid,
- (PTRACE_TYPE_ARG3) ((i << 1) + 1),
- &proc_info->bpts[i].address) < 0)
- perror_with_name ("Unexpected error setting breakpoint address");
-
- if (arm_hwbp_control_is_initialized (proc_info->bpts[i].control))
- if (ptrace (PTRACE_SETHBPREGS, pid,
- (PTRACE_TYPE_ARG3) ((i << 1) + 2),
- &proc_info->bpts[i].control) < 0)
- perror_with_name ("Unexpected error setting breakpoint");
+ if (!arm_hwbp_control_is_initialized (control))
+ {
+ /* Nothing to do. */
+ }
+ else if (!arm_hwbp_control_is_enabled (control))
+ {
+ /* Disable hardware breakpoint, just write the control
+ register. */
+ sethbpregs_hwbp_control (pid, i, control);
+ }
+ else
+ {
+ /* See arm_linux_nat_target::low_prepare_to_resume for detailed
+ comment. */
+ unsigned int aligned_address = address & ~0x7U;
+ if (aligned_address != address)
+ {
+ sethbpregs_hwbp_address (pid, i, aligned_address);
+ sethbpregs_hwbp_control (pid, i, control);
+ }
+ sethbpregs_hwbp_address (pid, i, address);
+ sethbpregs_hwbp_control (pid, i, control);
+ }
lwp_info->bpts_changed[i] = 0;
}
--
2.43.0
