mirror of
https://github.com/bminor/binutils-gdb.git
synced 2025-12-26 09:08:59 +00:00
b0196f0c87
At Red Hat we have an out of tree AArch64 watchpoint test which broke
after this commit:
commit cf16ab724a
Date: Tue Mar 12 17:08:18 2024 +0100
[gdb/tdep] Fix gdb.base/watch-bitfields.exp on aarch64
The problem with AArch64 hardware watchpoints is that they (as I
understand it) are restricted to a minimum of 8 bytes.
The problem is that current AArch64 hardware has imprecise hardware
watchpoint events due to unaligned accesses. The address reported for
the watchpoint event will depend on the access size. As a result, it
is possible that multiple watchpoints could potentially account for a
single watchpoint event, which is the case in the RH test. GDB can
then miss-identify which watchpoint actually triggered.
Prior to the above commit the RH test was passing. However, the test
was relying on, in the case of ambiguity, GDB selecting the first
created watchpoint. That behaviour changed with the above commit.
Now GDB favours reporting non write breakpoints, and will only report
a write breakpoint if no non-write breakpoint exists in the same
region.
I originally posted a patch to try and tweak the existing logic to
restore enough of the original behaviour that the RH test would pass,
this can be found here (2 iterations):
https://inbox.sourceware.org/gdb-patches/65e746b6394f04faa027e778f733eda95d20f368.1753115072.git.aburgess@redhat.com
https://inbox.sourceware.org/gdb-patches/638cbe9b738c0c529f6370f90ba4a395711f63ae.1753971315.git.aburgess@redhat.com
Neither of these really resolved the problem, they fixed some cases,
but broke others.
Ultimately, the problem on AArch64 is that for a single watchpoint
trap, there could be multiple watchpoints that are potentially
responsible. The existing API defined by the target_ops methods
stopped_by_watchpoint() and stopped_data_address() only allow for two
possible options:
1. If stopped_by_watchpoint() is true then stopped_data_address()
can return true and a single address which identifies all
watchpoints at that single address, or
2. If stopped_by_watchpoint() is true then stopped_data_address()
can return false, in which case GDB will check all write
watchpoints to see if any have changed, if they have, then GDB
tells the user that that was the triggering watchpoint.
If we are in a situation where we have to choose between multiple
write and read watchpoints then the current API doesn't allow the
architecture specific code to tell GDB core about this case.
In this commit I propose that we change the target_ops API,
specifically, the method:
bool target_ops::stopped_data_address (CORE_ADDR *);
will change to:
std::vector<CORE_ADDR> target_ops::stopped_data_addresses ();
The architecture specific code can now return a set of watchpoint
addresses, allowing GDB to identify a set of watchpoints that might
have triggered. GDB core can then select the most likely watchpoint,
and present that to the user.
As with the old API, target_ops::stopped_data_addresses should only be
called when target_ops::stopped_by_watchpoint is true, in which case
it's return values can be interpreted like this:
a. An empty vector; this replaces the old case where false was
returned. GDB should check all the write watchpoints and select
the one that changed as the responsible watchpoint.
b. A single entry vector; all targets except AArch64 currently
return at most a single entry vector. The single address
indicates the watchpoint(s) that triggered.
c. A multi-entry vector; currently AArch64 only. These addresses
indicate the set of watchpoints that might have triggered. GDB
will check the write watchpoints to see which (if any) changed,
and if no write watchpoints changed, GDB will present the first
access watchpoint.
In the future, we might want to improve the handling of (c) so that
GDB tells the user that multiple access watchpoints might have
triggered, and then list all of them. This might clear up some
confusion. But I think that can be done in the future (I don't have
an immediate plan to work on this). I think this change is already a
good improvement.
The changes for this are pretty extensive, but here's a basic summary:
* Within gdb/ changing the API name from stopped_data_address to
stopped_data_addresses throughout. Comments are updated too where
needed.
* For targets other than AArch64, the existing code is retained with
as few changes as possible, we only allow for a single address to
be returned, the address is now wrapped in a vector. Where we
used to return false, we now return the empty vector.
* For AArch64, the return a vector logic is pushed through to
gdb/nat/aarch64-hw-point.{c,h}, and aarch64_stopped_data_address
changes to aarch64_stopped_data_addresses, and is updated to
return a vector of addresses.
* In infrun.c there's some updates to some debug output.
* In breakpoint.c the interesting changes are in
watchpoints_triggered. The existing code has three cases to
handle:
(i) target_stopped_by_watchpoint returns false. This case is
unchanged.
(ii) target_stopped_data_address returns false. This case is now
calling target_stopped_data_addresses, and checks for the
empty vector, but otherwise is unchanged.
(iii) target_stopped_data_address returns true, and a single
address. This code calls target_stopped_data_addresses, and
now handles the possibility of a vector containing multiple
entries. We need to first loop over every watchpoint
setting its triggered status to 'no', then we check every
address in the vector setting matching watchpoint's
triggered status to 'yes'. But the actual logic for if a
watchpoint matches an address or not is unchanged.
The important thing to notice here is that in case (iii), before
this patch, GDB could already set _multiple_ watchpoints to
triggered. For example, setting a read and write watchpoint on
the same address would result in multiple watchpoints being marked
as triggered. This patch just extends this so that multiple
watchpoints, at multiple addresses, can now be marked as
triggered.
* In remote.c there is an interesting change. We need to allow
gdbserver to pass the multiple addresses back to GDB. To achieve
this, I now allow multiple 'watch', 'rwatch', and 'awatch' tokens
in a 'T' stop reply packet. There's a new feature multi-wp-addr
which is passed in the qSupported packet to determine if the
remote is allowed to pass back multiple watchpoint stop reasons.
If the remote passed multiple watchpoint addresses then these are
collected and returned from the target_ops::stopped_data_addresses
call.
If a new GDB connects to an old gdbserver that doesn't understand
the multi-wp-addr feature, then gdbserver will continue to return
a single watchpoint address in the 'T' packet, which is what
happens before this patch.
* In gdbserver/ the changes are pretty similar. The API is renamed
from ::stopped_data_address to ::stopped_data_addresses, and
::low_stopped_data_address to ::low_stopped_data_addresses.
There's also code added to detect the new multi-wp-addr feature.
If this feature is not advertised from GDB then only a single
watchpoint address will be returned in the 'T' stop reply packet.
* In GDB and gdbserver, for all targets except AArch64, the existing
code to figure out a watchpoint address is retained, we just wrap
the single address into a vector.
* For AArch64, we call aarch64_stopped_data_addresses, which returns
the required vector.
For testing, I've built GDB on GNU/Linux for i386, x86-64, PPC64le,
ARM, and AArch64. That still leaves a lot of targets possibly
impacted by this change as untested. Which is a risk. I certainly
wouldn't want to push this patch until after GDB 17 branches so we
have time to find and fix any regressions that are introduced.
I've run a full regression test on AArch64 and x86-64 (both GNU/Linux)
with no regressions. As I said above, for other targets nothing
should really have changed, all non-AArch64 targets just return a
single watchpoint address from target_ops::stopped_data_addresses(),
so, as long as the target builds, it should run unchanged.
I also sent the branch through the sourceware CI, and everything
passed.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33240
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33252
Acked-By: Tom de Vries <tdevries@suse.de>
145 lines
5.7 KiB
C++
145 lines
5.7 KiB
C++
/* Copyright (C) 2009-2025 Free Software Foundation, Inc.
|
|
|
|
This file is part of GDB.
|
|
|
|
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/>. */
|
|
|
|
#ifndef GDB_NAT_AARCH64_HW_POINT_H
|
|
#define GDB_NAT_AARCH64_HW_POINT_H
|
|
|
|
/* Macro definitions, data structures, and code for the hardware
|
|
breakpoint and hardware watchpoint support follow. We use the
|
|
following abbreviations throughout the code:
|
|
|
|
hw - hardware
|
|
bp - breakpoint
|
|
wp - watchpoint */
|
|
|
|
/* Maximum number of hardware breakpoint and watchpoint registers.
|
|
Neither of these values may exceed the width of dr_changed_t
|
|
measured in bits. */
|
|
|
|
#define AARCH64_HBP_MAX_NUM 16
|
|
#define AARCH64_HWP_MAX_NUM 16
|
|
|
|
/* Alignment requirement in bytes for addresses written to
|
|
hardware breakpoint and watchpoint value registers.
|
|
|
|
A ptrace call attempting to set an address that does not meet the
|
|
alignment criteria will fail. Limited support has been provided in
|
|
this port for unaligned watchpoints, such that from a GDB user
|
|
perspective, an unaligned watchpoint may be requested.
|
|
|
|
This is achieved by minimally enlarging the watched area to meet the
|
|
alignment requirement, and if necessary, splitting the watchpoint
|
|
over several hardware watchpoint registers. */
|
|
|
|
#define AARCH64_HBP_ALIGNMENT 4
|
|
#define AARCH64_HWP_ALIGNMENT 8
|
|
|
|
/* The maximum length of a memory region that can be watched by one
|
|
hardware watchpoint register. */
|
|
|
|
#define AARCH64_HWP_MAX_LEN_PER_REG 8
|
|
|
|
/* Macro for the expected version of the ARMv8-A debug architecture. */
|
|
#define AARCH64_DEBUG_ARCH_V8 0x6
|
|
#define AARCH64_DEBUG_ARCH_V8_1 0x7
|
|
#define AARCH64_DEBUG_ARCH_V8_2 0x8
|
|
#define AARCH64_DEBUG_ARCH_V8_4 0x9
|
|
#define AARCH64_DEBUG_ARCH_V8_8 0xa
|
|
/* Armv8.9 debug architecture. */
|
|
#define AARCH64_DEBUG_ARCH_V8_9 0xb
|
|
|
|
/* ptrace expects control registers to be formatted as follows:
|
|
|
|
31 13 5 3 1 0
|
|
+--------------------------------+----------+------+------+----+
|
|
| RESERVED (SBZ) | MASK | TYPE | PRIV | EN |
|
|
+--------------------------------+----------+------+------+----+
|
|
|
|
The TYPE field is ignored for breakpoints. */
|
|
|
|
#define DR_CONTROL_ENABLED(ctrl) (((ctrl) & 0x1) == 1)
|
|
#define DR_CONTROL_MASK(ctrl) (((ctrl) >> 5) & 0xff)
|
|
#define DR_CONTROL_TYPE(ctrl) (((ctrl) >> 3) & 0x3)
|
|
|
|
/* Structure for managing the hardware breakpoint/watchpoint resources.
|
|
DR_ADDR_* stores the address, DR_CTRL_* stores the control register
|
|
content, and DR_REF_COUNT_* counts the numbers of references to the
|
|
corresponding bp/wp, by which way the limited hardware resources
|
|
are not wasted on duplicated bp/wp settings (though so far gdb has
|
|
done a good job by not sending duplicated bp/wp requests). */
|
|
|
|
struct aarch64_debug_reg_state
|
|
{
|
|
/* hardware breakpoint */
|
|
CORE_ADDR dr_addr_bp[AARCH64_HBP_MAX_NUM];
|
|
unsigned int dr_ctrl_bp[AARCH64_HBP_MAX_NUM];
|
|
unsigned int dr_ref_count_bp[AARCH64_HBP_MAX_NUM];
|
|
|
|
/* hardware watchpoint */
|
|
/* Address aligned down to AARCH64_HWP_ALIGNMENT. */
|
|
CORE_ADDR dr_addr_wp[AARCH64_HWP_MAX_NUM];
|
|
/* Address as entered by user without any forced alignment. */
|
|
CORE_ADDR dr_addr_orig_wp[AARCH64_HWP_MAX_NUM];
|
|
unsigned int dr_ctrl_wp[AARCH64_HWP_MAX_NUM];
|
|
unsigned int dr_ref_count_wp[AARCH64_HWP_MAX_NUM];
|
|
};
|
|
|
|
extern int aarch64_num_bp_regs;
|
|
extern int aarch64_num_wp_regs;
|
|
|
|
/* Invoked when IDXth breakpoint/watchpoint register pair needs to be
|
|
updated. */
|
|
void aarch64_notify_debug_reg_change (ptid_t ptid, int is_watchpoint,
|
|
unsigned int idx);
|
|
|
|
unsigned int aarch64_watchpoint_offset (unsigned int ctrl);
|
|
unsigned int aarch64_watchpoint_length (unsigned int ctrl);
|
|
enum target_hw_bp_type aarch64_watchpoint_type (unsigned int ctrl);
|
|
|
|
/* Helper for the "stopped_data_addresses" target method. Returns a vector
|
|
containing the addresses of all hardware watchpoints that could account
|
|
for a watchpoint trap at ADDR_TRAP. Return an empty vector if no
|
|
suitable watchpoint addresses can be identified.
|
|
|
|
It is possible that multiple watchpoints could account for a trap at
|
|
ADDR_TRAP, in which case all possible addresses are returned, and GDB
|
|
core is responsible for selecting a suitable watchpoint, or otherwise
|
|
letting the user know that there is some ambiguity. */
|
|
|
|
extern std::vector<CORE_ADDR> aarch64_stopped_data_addresses
|
|
(const struct aarch64_debug_reg_state *state, CORE_ADDR addr_trap);
|
|
|
|
int aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr,
|
|
int len, int is_insert, ptid_t ptid,
|
|
struct aarch64_debug_reg_state *state);
|
|
int aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr,
|
|
int len, int is_insert, ptid_t ptid,
|
|
struct aarch64_debug_reg_state *state);
|
|
|
|
/* Return TRUE if there are any hardware breakpoints. If WATCHPOINT is TRUE,
|
|
check hardware watchpoints instead. */
|
|
bool aarch64_any_set_debug_regs_state (aarch64_debug_reg_state *state,
|
|
bool watchpoint);
|
|
|
|
void aarch64_show_debug_reg_state (struct aarch64_debug_reg_state *state,
|
|
const char *func, CORE_ADDR addr,
|
|
int len, enum target_hw_bp_type type);
|
|
|
|
int aarch64_region_ok_for_watchpoint (CORE_ADDR addr, int len);
|
|
|
|
#endif /* GDB_NAT_AARCH64_HW_POINT_H */
|