[PATCH] [gdb/tdep] Fix arm thumb2 hw breakpoint
Tom de Vries
tdevries@suse.de
Thu Jul 25 13:18:10 GMT 2024
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.
The ptrace call is the first of two writing a register pair, an address and a
control register.
Fix or work around this by:
- first writing the address bpts[i].address & ~0x7,
- then writing the control register, and
- if necessary, updating the address to bpts[i].address.
Likewise in arm_target::low_prepare_to_resume, which fixes the same fail on
target board native-gdbserver/mthumb.
While we're at it, add missing '_()' and make error messages identical between
arm_target::low_prepare_to_resume and
arm_linux_nat_target::low_prepare_to_resume.
Remove the tentative xfail added in d0af16d5a10 ("[gdb/testsuite] Add xfail in
gdb.base/hbreak.exp") by simply reverting the commit.
Tested on aarch64-linux.
---
gdb/arm-linux-nat.c | 52 ++++++++++++++++++++++++++++---
gdb/testsuite/gdb.base/hbreak.exp | 40 ++++--------------------
gdbserver/linux-arm-low.cc | 25 ++++++++++-----
3 files changed, 71 insertions(+), 46 deletions(-)
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index ac53bed72d7..8a00c8095de 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -1257,17 +1257,59 @@ 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])
{
+ PTRACE_TYPE_ARG3 address_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 1);
+ PTRACE_TYPE_ARG3 control_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 2);
+
errno = 0;
+
+ /* We used to do here simply:
+ 1. address_reg = bpts[i].address
+ 2. control_reg = bpts[i].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 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.
+
+ Work around this by first writing an inoffensive address, which is
+ guaranteed to hit the offset == 0 case in hw_breakpoint_arch_parse,
+ then the control_reg, and then the actual address, in other words:
+ 1. address_reg = bpts[i].address & ~0x7U
+ 2. control_reg = bpts[i].control
+ 3. address_reg = bpts[i].address
+ obviously skipping the 3rd step if it's the same address. */
+ unsigned int aligned_address = bpts[i].address & ~0x7U;
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 (ptrace (PTRACE_SETHBPREGS, pid, address_reg, &aligned_address)
+ < 0)
+ perror_with_name (_("Unexpected error setting breakpoint address"));
if (bpts[i].control != 0)
- if (ptrace (PTRACE_SETHBPREGS, pid,
- (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
+ if (ptrace (PTRACE_SETHBPREGS, pid, control_reg, &bpts[i].control)
+ < 0)
perror_with_name (_("Unexpected error setting breakpoint"));
+ if (arm_hwbp_control_is_enabled (bpts[i].control)
+ && bpts[i].address != aligned_address)
+ if (ptrace (PTRACE_SETHBPREGS, pid, address_reg, &bpts[i].address)
+ < 0)
+ perror_with_name
+ (_("Unexpected error updating breakpoint address"));
+
arm_lwp_info->bpts_changed[i] = 0;
}
diff --git a/gdb/testsuite/gdb.base/hbreak.exp b/gdb/testsuite/gdb.base/hbreak.exp
index b140257a23e..73a3e2afb67 100644
--- a/gdb/testsuite/gdb.base/hbreak.exp
+++ b/gdb/testsuite/gdb.base/hbreak.exp
@@ -27,38 +27,10 @@ if ![runto_main] {
set breakline [gdb_get_line_number "break-at-exit"]
-set re_loc "file \[^\r\n\]*$srcfile, line $breakline"
-set re_dot [string_to_regexp .]
+gdb_test "hbreak ${srcfile}:${breakline}" \
+ "Hardware assisted breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*${srcfile}, line ${breakline}\\." \
+ "hbreak"
-set addr 0x0
-gdb_test_multiple "hbreak ${srcfile}:${breakline}" "hbreak" {
- -re -wrap "Hardware assisted breakpoint $decimal at ($hex): $re_loc$re_dot" {
- set addr $expect_out(1,string)
- pass $gdb_test_name
- }
-}
-
-set have_xfail 0
-if { [istarget arm*-*-*] } {
- # When running 32-bit userland on aarch64 kernel, thumb instructions that
- # are not 4-byte aligned may not be supported for setting a hardware
- # breakpoint on.
- set have_xfail [expr ($addr & 0x2) == 2]
-}
-
-set re_xfail \
- [string_to_regexp \
- "Unexpected error setting breakpoint: Invalid argument."]
-
-gdb_test_multiple "continue" "continue to break-at-exit after hbreak" {
- -re -wrap "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" {
- pass $gdb_test_name
- }
- -re -wrap $re_xfail {
- if { $have_xfail } {
- xfail $gdb_test_name
- } else {
- fail $gdb_test_name
- }
- }
-}
+gdb_test "continue" \
+ "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \
+ "continue to break-at-exit after hbreak"
diff --git a/gdbserver/linux-arm-low.cc b/gdbserver/linux-arm-low.cc
index eec4649b235..6eeb883e3b0 100644
--- a/gdbserver/linux-arm-low.cc
+++ b/gdbserver/linux-arm-low.cc
@@ -834,19 +834,30 @@ 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])
{
+ PTRACE_TYPE_ARG3 address_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 1);
+ PTRACE_TYPE_ARG3 control_reg = (PTRACE_TYPE_ARG3) ((i << 1) + 2);
+
errno = 0;
+ /* See arm_linux_nat_target::low_prepare_to_resume for detailed
+ comment. */
+ unsigned int aligned_address = proc_info->bpts[i].address & ~0x7U;
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 (ptrace (PTRACE_SETHBPREGS, pid, address_reg, &aligned_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),
+ if (ptrace (PTRACE_SETHBPREGS, pid, control_reg,
&proc_info->bpts[i].control) < 0)
- perror_with_name ("Unexpected error setting breakpoint");
+ perror_with_name (_("Unexpected error setting breakpoint"));
+
+ if (arm_hwbp_control_is_enabled (proc_info->bpts[i].control)
+ && proc_info->bpts[i].address != aligned_address)
+ if (ptrace (PTRACE_SETHBPREGS, pid, address_reg,
+ &proc_info->bpts[i].address) < 0)
+ perror_with_name
+ (_("Unexpected error updating breakpoint address"));
lwp_info->bpts_changed[i] = 0;
}
base-commit: a93faed5d46039999cb1cd8659c82ac981485666
--
2.35.3
More information about the Gdb-patches
mailing list