From 64dc13a4571b4092726291f3ee30bf5c2166fa13 Mon Sep 17 00:00:00 2001 From: Tom de Vries 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 Tested-By: Luis Machado --- 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