* [PATCH v6 0/2] Add amd64 LAM watchpoint support
@ 2024-10-28 8:46 Schimpe, Christina
2024-10-28 8:46 ` [PATCH v6 1/2] gdb: Make tagged pointer support configurable Schimpe, Christina
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Schimpe, Christina @ 2024-10-28 8:46 UTC (permalink / raw)
To: gdb-patches
Hi all,
this is my v6 of the series "Add amd64 LAM watchpoint support".
I just rebased it on latest master since the posting of v5 has been a
while and there were some conflicts in gdb/aarch64-tdep.c and
loongarch-linux-nat.c.
Besides that only the review for patch 1 is open, patch 2 has already
been approved by Felix and Eli for the docs parts.
v5 of this series can be found here:
https://sourceware.org/pipermail/gdb-patches/2024-July/210414.html
Best Regards
Christina
Christina Schimpe (2):
gdb: Make tagged pointer support configurable.
LAM: Enable tagged pointer support for watchpoints.
gdb/NEWS | 5 +++
gdb/aarch64-linux-nat.c | 2 +-
gdb/aarch64-linux-tdep.c | 7 +--
gdb/aarch64-tdep.c | 23 ++++++----
gdb/aarch64-tdep.h | 6 +++
gdb/amd64-linux-tdep.c | 64 +++++++++++++++++++++++++++
gdb/breakpoint.c | 5 ++-
gdb/gdbarch-gen.c | 66 +++++++++++++++++++++++-----
gdb/gdbarch-gen.h | 49 ++++++++++++++++-----
gdb/gdbarch_components.py | 53 ++++++++++++++++++----
gdb/loongarch-linux-nat.c | 2 +-
gdb/target.c | 3 +-
gdb/testsuite/gdb.arch/amd64-lam.c | 49 +++++++++++++++++++++
gdb/testsuite/gdb.arch/amd64-lam.exp | 46 +++++++++++++++++++
gdb/testsuite/lib/gdb.exp | 63 ++++++++++++++++++++++++++
15 files changed, 396 insertions(+), 47 deletions(-)
create mode 100755 gdb/testsuite/gdb.arch/amd64-lam.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-lam.exp
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-10-28 8:46 [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
@ 2024-10-28 8:46 ` Schimpe, Christina
2024-11-04 11:17 ` Luis Machado
2024-10-28 8:46 ` [PATCH v6 2/2] LAM: Enable tagged pointer support for watchpoints Schimpe, Christina
2024-11-04 8:37 ` [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
2 siblings, 1 reply; 12+ messages in thread
From: Schimpe, Christina @ 2024-10-28 8:46 UTC (permalink / raw)
To: gdb-patches
From: Christina Schimpe <christina.schimpe@intel.com>
The gdbarch function gdbarch_remove_non_address_bits adjusts addresses to
enable debugging of programs with tagged pointers on Linux, for instance for
ARM's feature top byte ignore (TBI).
Once the function is implemented for an architecture, it adjusts addresses for
memory access, breakpoints and watchpoints.
Linear address masking (LAM) is Intel's (R) implementation of tagged
pointer support. It requires certain adaptions to GDB's tagged pointer
support due to the following:
- LAM supports address tagging for data accesses only. Thus, specifying
breakpoints on tagged addresses is not a valid use case.
- In contrast to the implementation for ARM's TBI, the Linux kernel supports
tagged pointers for memory access.
This patch makes GDB's tagged pointer support configurable such that it is
possible to enable the address adjustment for a specific feature only (e.g
memory access, breakpoints or watchpoints). This way, one can make sure
that addresses are only adjusted when necessary. In case of LAM, this
avoids unnecessary parsing of the /proc/<pid>/status file to get the
untag mask.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
---
gdb/aarch64-linux-nat.c | 2 +-
gdb/aarch64-linux-tdep.c | 7 +++--
gdb/aarch64-tdep.c | 23 ++++++++------
gdb/aarch64-tdep.h | 6 ++++
gdb/breakpoint.c | 5 +--
gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++-------
gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
gdb/loongarch-linux-nat.c | 2 +-
gdb/target.c | 3 +-
10 files changed, 169 insertions(+), 47 deletions(-)
diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 0fa5bee500b..48ab765d880 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
kernel can potentially be tagged addresses. */
struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
const CORE_ADDR addr_trap
- = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
+ = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
/* Check if the address matches any watched address. */
state = aarch64_get_debug_reg_state (inferior_ptid.pid ());
diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index c608a84bc71..61c3be8b6f0 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -2433,7 +2433,7 @@ static bool
aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR address)
{
/* Remove the top byte for the memory range check. */
- address = gdbarch_remove_non_address_bits (gdbarch, address);
+ address = aarch64_remove_non_address_bits (gdbarch, address);
/* Check if the page that contains ADDRESS is mapped with PROT_MTE. */
if (!linux_address_in_memtag_page (address))
@@ -2491,8 +2491,9 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
uiout->text ("\n");
std::optional<CORE_ADDR> atag
- = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
- fault_addr));
+ = aarch64_mte_get_atag (
+ aarch64_remove_non_address_bits (gdbarch, fault_addr));
+
gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
if (!atag.has_value ())
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 8a2a9b1e23c..91fec7879ed 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch *gdbarch,
/* Fetch the allocation tag for ADDRESS. */
std::optional<CORE_ADDR> atag
- = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch, addr));
+ = aarch64_mte_get_atag (aarch64_remove_non_address_bits (gdbarch, addr));
if (!atag.has_value ())
return true;
@@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch *gdbarch, struct value *address,
else
{
/* Remove the top byte. */
- addr = gdbarch_remove_non_address_bits (gdbarch, addr);
+ addr = aarch64_remove_non_address_bits (gdbarch, addr);
/* With G being the number of tag granules and N the number of tags
passed in, we can have the following cases:
@@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch *gdbarch, struct value *address,
else
{
/* Remove the top byte. */
- addr = gdbarch_remove_non_address_bits (gdbarch, addr);
+ addr = aarch64_remove_non_address_bits (gdbarch, addr);
std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
if (!atag.has_value ())
@@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch *gdbarch, struct value *tag_value)
return string_printf ("0x%s", phex_nz (tag, sizeof (tag)));
}
-/* AArch64 implementation of the remove_non_address_bits gdbarch hook. Remove
- non address bits from a pointer value. */
+/* See aarch64-tdep.h. */
-static CORE_ADDR
+CORE_ADDR
aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
{
/* By default, we assume TBI and discard the top 8 bits plus the VA range
@@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
/* Architecture hook to remove bits of a pointer that are not part of the
- address, like memory tags (MTE) and pointer authentication signatures. */
- set_gdbarch_remove_non_address_bits (gdbarch,
- aarch64_remove_non_address_bits);
+ address, like memory tags (MTE) and pointer authentication signatures.
+ Configure address adjustment for watch-, breakpoints and memory
+ transfer. */
+ set_gdbarch_remove_non_address_bits_watchpoint
+ (gdbarch, aarch64_remove_non_address_bits);
+ set_gdbarch_remove_non_address_bits_breakpoint
+ (gdbarch, aarch64_remove_non_address_bits);
+ set_gdbarch_remove_non_address_bits_memory
+ (gdbarch, aarch64_remove_non_address_bits);
/* SME pseudo-registers. */
if (tdep->has_sme ())
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index 50166fb4f24..13ea37de7a3 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch);
std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR address);
+/* AArch64 implementation of the remove_non_address_bits gdbarch hooks.
+ Remove non address bits from a pointer value. */
+
+CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
+ CORE_ADDR pointer);
+
#endif /* aarch64-tdep.h */
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b7e4f5d0a45..9983e905e1b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b, bool reparse)
loc->gdbarch = v->type ()->arch ();
loc->pspace = wp_pspace;
loc->address
- = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
+ = gdbarch_remove_non_address_bits_watchpoint (loc->gdbarch,
+ addr);
b->add_location (*loc);
if (bitsize != 0)
@@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
}
adjusted_bpaddr
- = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
+ = gdbarch_remove_non_address_bits_breakpoint (gdbarch, adjusted_bpaddr);
/* An adjusted breakpoint address can significantly alter
a user's expectations. Print a warning if an adjustment
diff --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c
index 0d00cd7c993..d05c7a3cbdf 100644
--- a/gdb/gdbarch-gen.c
+++ b/gdb/gdbarch-gen.c
@@ -143,7 +143,9 @@ struct gdbarch
int frame_red_zone_size = 0;
gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity;
gdbarch_addr_bits_remove_ftype *addr_bits_remove = core_addr_identity;
- gdbarch_remove_non_address_bits_ftype *remove_non_address_bits = default_remove_non_address_bits;
+ gdbarch_remove_non_address_bits_watchpoint_ftype *remove_non_address_bits_watchpoint = default_remove_non_address_bits;
+ gdbarch_remove_non_address_bits_breakpoint_ftype *remove_non_address_bits_breakpoint = default_remove_non_address_bits;
+ gdbarch_remove_non_address_bits_memory_ftype *remove_non_address_bits_memory = default_remove_non_address_bits;
gdbarch_memtag_to_string_ftype *memtag_to_string = default_memtag_to_string;
gdbarch_tagged_address_p_ftype *tagged_address_p = default_tagged_address_p;
gdbarch_memtag_matches_p_ftype *memtag_matches_p = default_memtag_matches_p;
@@ -407,7 +409,9 @@ verify_gdbarch (struct gdbarch *gdbarch)
/* Skip verify of frame_red_zone_size, invalid_p == 0. */
/* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
/* Skip verify of addr_bits_remove, invalid_p == 0. */
- /* Skip verify of remove_non_address_bits, invalid_p == 0. */
+ /* Skip verify of remove_non_address_bits_watchpoint, invalid_p == 0. */
+ /* Skip verify of remove_non_address_bits_breakpoint, invalid_p == 0. */
+ /* Skip verify of remove_non_address_bits_memory, invalid_p == 0. */
/* Skip verify of memtag_to_string, invalid_p == 0. */
/* Skip verify of tagged_address_p, invalid_p == 0. */
/* Skip verify of memtag_matches_p, invalid_p == 0. */
@@ -910,8 +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
"gdbarch_dump: addr_bits_remove = <%s>\n",
host_address_to_string (gdbarch->addr_bits_remove));
gdb_printf (file,
- "gdbarch_dump: remove_non_address_bits = <%s>\n",
- host_address_to_string (gdbarch->remove_non_address_bits));
+ "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
+ host_address_to_string (gdbarch->remove_non_address_bits_watchpoint));
+ gdb_printf (file,
+ "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
+ host_address_to_string (gdbarch->remove_non_address_bits_breakpoint));
+ gdb_printf (file,
+ "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
+ host_address_to_string (gdbarch->remove_non_address_bits_memory));
gdb_printf (file,
"gdbarch_dump: memtag_to_string = <%s>\n",
host_address_to_string (gdbarch->memtag_to_string));
@@ -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
}
CORE_ADDR
-gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
+gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch, CORE_ADDR pointer)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->remove_non_address_bits_watchpoint != NULL);
+ if (gdbarch_debug >= 2)
+ gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_watchpoint called\n");
+ return gdbarch->remove_non_address_bits_watchpoint (gdbarch, pointer);
+}
+
+void
+set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
+ gdbarch_remove_non_address_bits_watchpoint_ftype remove_non_address_bits_watchpoint)
+{
+ gdbarch->remove_non_address_bits_watchpoint = remove_non_address_bits_watchpoint;
+}
+
+CORE_ADDR
+gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pointer)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->remove_non_address_bits_breakpoint != NULL);
+ if (gdbarch_debug >= 2)
+ gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_breakpoint called\n");
+ return gdbarch->remove_non_address_bits_breakpoint (gdbarch, pointer);
+}
+
+void
+set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
+ gdbarch_remove_non_address_bits_breakpoint_ftype remove_non_address_bits_breakpoint)
+{
+ gdbarch->remove_non_address_bits_breakpoint = remove_non_address_bits_breakpoint;
+}
+
+CORE_ADDR
+gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch, CORE_ADDR pointer)
{
gdb_assert (gdbarch != NULL);
- gdb_assert (gdbarch->remove_non_address_bits != NULL);
+ gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
if (gdbarch_debug >= 2)
- gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
- return gdbarch->remove_non_address_bits (gdbarch, pointer);
+ gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_memory called\n");
+ return gdbarch->remove_non_address_bits_memory (gdbarch, pointer);
}
void
-set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
- gdbarch_remove_non_address_bits_ftype remove_non_address_bits)
+set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
+ gdbarch_remove_non_address_bits_memory_ftype remove_non_address_bits_memory)
{
- gdbarch->remove_non_address_bits = remove_non_address_bits;
+ gdbarch->remove_non_address_bits_memory = remove_non_address_bits_memory;
}
std::string
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index b982fd7cd09..9fda85f860f 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR ad
extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_bits_remove_ftype *addr_bits_remove);
/* On some architectures, not all bits of a pointer are significant.
- On AArch64, for example, the top bits of a pointer may carry a "tag", which
- can be ignored by the kernel and the hardware. The "tag" can be regarded as
- additional data associated with the pointer, but it is not part of the address.
+ On AArch64 and amd64, for example, the top bits of a pointer may carry a
+ "tag", which can be ignored by the kernel and the hardware. The "tag" can be
+ regarded as additional data associated with the pointer, but it is not part
+ of the address.
Given a pointer for the architecture, this hook removes all the
- non-significant bits and sign-extends things as needed. It gets used to remove
- non-address bits from data pointers (for example, removing the AArch64 MTE tag
- bits from a pointer) and from code pointers (removing the AArch64 PAC signature
- from a pointer containing the return address). */
-
-typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
-extern CORE_ADDR gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer);
-extern void set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype *remove_non_address_bits);
+ non-significant bits and sign-extends things as needed. It gets used to
+ remove non-address bits from pointers used for watchpoints. */
+
+typedef CORE_ADDR (gdbarch_remove_non_address_bits_watchpoint_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
+extern CORE_ADDR gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch, CORE_ADDR pointer);
+extern void set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype *remove_non_address_bits_watchpoint);
+
+/* On some architectures, not all bits of a pointer are significant.
+ On AArch64 and amd64, for example, the top bits of a pointer may carry a
+ "tag", which can be ignored by the kernel and the hardware. The "tag" can be
+ regarded as additional data associated with the pointer, but it is not part
+ of the address.
+
+ Given a pointer for the architecture, this hook removes all the
+ non-significant bits and sign-extends things as needed. It gets used to
+ remove non-address bits from pointers used for breakpoints. */
+
+typedef CORE_ADDR (gdbarch_remove_non_address_bits_breakpoint_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
+extern CORE_ADDR gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pointer);
+extern void set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype *remove_non_address_bits_breakpoint);
+
+/* On some architectures, not all bits of a pointer are significant.
+ On AArch64 and amd64, for example, the top bits of a pointer may carry a
+ "tag", which can be ignored by the kernel and the hardware. The "tag" can be
+ regarded as additional data associated with the pointer, but it is not part
+ of the address.
+
+ Given a pointer for the architecture, this hook removes all the
+ non-significant bits and sign-extends things as needed. It gets used to
+ remove non-address bits from any pointer used to access memory. */
+
+typedef CORE_ADDR (gdbarch_remove_non_address_bits_memory_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
+extern CORE_ADDR gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch, CORE_ADDR pointer);
+extern void set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_memory_ftype *remove_non_address_bits_memory);
/* Return a string representation of the memory tag TAG. */
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index 4006380076d..cc7c6d8677b 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC instead).
Method(
comment="""
On some architectures, not all bits of a pointer are significant.
-On AArch64, for example, the top bits of a pointer may carry a "tag", which
-can be ignored by the kernel and the hardware. The "tag" can be regarded as
-additional data associated with the pointer, but it is not part of the address.
+On AArch64 and amd64, for example, the top bits of a pointer may carry a
+"tag", which can be ignored by the kernel and the hardware. The "tag" can be
+regarded as additional data associated with the pointer, but it is not part
+of the address.
Given a pointer for the architecture, this hook removes all the
-non-significant bits and sign-extends things as needed. It gets used to remove
-non-address bits from data pointers (for example, removing the AArch64 MTE tag
-bits from a pointer) and from code pointers (removing the AArch64 PAC signature
-from a pointer containing the return address).
+non-significant bits and sign-extends things as needed. It gets used to
+remove non-address bits from pointers used for watchpoints.
""",
type="CORE_ADDR",
- name="remove_non_address_bits",
+ name="remove_non_address_bits_watchpoint",
+ params=[("CORE_ADDR", "pointer")],
+ predefault="default_remove_non_address_bits",
+ invalid=False,
+)
+
+Method(
+ comment="""
+On some architectures, not all bits of a pointer are significant.
+On AArch64 and amd64, for example, the top bits of a pointer may carry a
+"tag", which can be ignored by the kernel and the hardware. The "tag" can be
+regarded as additional data associated with the pointer, but it is not part
+of the address.
+
+Given a pointer for the architecture, this hook removes all the
+non-significant bits and sign-extends things as needed. It gets used to
+remove non-address bits from pointers used for breakpoints.
+""",
+ type="CORE_ADDR",
+ name="remove_non_address_bits_breakpoint",
+ params=[("CORE_ADDR", "pointer")],
+ predefault="default_remove_non_address_bits",
+ invalid=False,
+)
+
+Method(
+ comment="""
+On some architectures, not all bits of a pointer are significant.
+On AArch64 and amd64, for example, the top bits of a pointer may carry a
+"tag", which can be ignored by the kernel and the hardware. The "tag" can be
+regarded as additional data associated with the pointer, but it is not part
+of the address.
+
+Given a pointer for the architecture, this hook removes all the
+non-significant bits and sign-extends things as needed. It gets used to
+remove non-address bits from any pointer used to access memory.
+""",
+ type="CORE_ADDR",
+ name="remove_non_address_bits_memory",
params=[("CORE_ADDR", "pointer")],
predefault="default_remove_non_address_bits",
invalid=False,
diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
index bc9927dd751..dc4a019614a 100644
--- a/gdb/loongarch-linux-nat.c
+++ b/gdb/loongarch-linux-nat.c
@@ -613,7 +613,7 @@ loongarch_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
kernel can potentially be tagged addresses. */
struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
const CORE_ADDR addr_trap
- = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
+ = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
/* Check if the address matches any watched address. */
state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
diff --git a/gdb/target.c b/gdb/target.c
index 4378c0572d2..b6d1abe82db 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1608,7 +1608,8 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
if (len == 0)
return TARGET_XFER_EOF;
- memaddr = gdbarch_remove_non_address_bits (current_inferior ()->arch (),
+ memaddr
+ = gdbarch_remove_non_address_bits_memory (current_inferior ()->arch (),
memaddr);
/* Fill in READBUF with breakpoint shadows, or WRITEBUF with
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 2/2] LAM: Enable tagged pointer support for watchpoints.
2024-10-28 8:46 [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
2024-10-28 8:46 ` [PATCH v6 1/2] gdb: Make tagged pointer support configurable Schimpe, Christina
@ 2024-10-28 8:46 ` Schimpe, Christina
2024-11-04 8:37 ` [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
2 siblings, 0 replies; 12+ messages in thread
From: Schimpe, Christina @ 2024-10-28 8:46 UTC (permalink / raw)
To: gdb-patches
From: Christina Schimpe <christina.schimpe@intel.com>
The Intel (R) linear address masking (LAM) feature modifies the checking
applied to 64-bit linear addresses. With this so-called "modified
canonicality check" the processor masks the metadata bits in a pointer
before using it as a linear address. LAM supports two different modes that
differ regarding which pointer bits are masked and can be used for
metadata: LAM 48 resulting in a LAM width of 15 and LAM 57 resulting in a
LAM width of 6.
This patch adjusts watchpoint addresses based on the currently enabled
LAM mode using the untag mask provided in the /proc/<pid>/status file.
As LAM can be enabled at runtime or as the configuration may change
when entering an enclave, GDB checks enablement state each time a watchpoint
is updated.
In contrast to the patch implemented for ARM's Top Byte Ignore "Clear
non-significant bits of address on memory access", it is not necessary to
adjust addresses before they are passed to the target layer cache, as
for LAM tagged pointers are supported by the system call to read memory.
Additionally, LAM applies only to addresses used for data accesses.
Thus, it is sufficient to mask addresses used for watchpoints.
The following examples are based on a LAM57 enabled program.
Before this patch tagged pointers were not supported for watchpoints:
~~~
(gdb) print pi_tagged
$2 = (int *) 0x10007ffffffffe004
(gdb) watch *pi_tagged
Hardware watchpoint 2: *pi_tagged
(gdb) c
Continuing.
Couldn't write debug register: Invalid argument.
~~~~
Once LAM 48 or LAM 57 is enabled for the current program, GDB can now
specify watchpoints for tagged addresses with LAM width 15 or 6,
respectively.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
---
gdb/NEWS | 5 +++
gdb/amd64-linux-tdep.c | 64 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.arch/amd64-lam.c | 49 +++++++++++++++++++++
gdb/testsuite/gdb.arch/amd64-lam.exp | 46 ++++++++++++++++++++
gdb/testsuite/lib/gdb.exp | 63 +++++++++++++++++++++++++++
5 files changed, 227 insertions(+)
create mode 100755 gdb/testsuite/gdb.arch/amd64-lam.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-lam.exp
diff --git a/gdb/NEWS b/gdb/NEWS
index 42b8a88fd8a..e25b6c414c9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,11 @@
*** Changes since GDB 15
+* GDB now supports watchpoints for tagged data pointers (see
+ https://en.wikipedia.org/wiki/Tagged_pointer) on amd64, such as the
+ one used by the Linear Address Masking (LAM) feature provided by
+ Intel.
+
* Debugging support for Intel MPX has been removed. This includes the
removal of
** MPX register support
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 77de8211d86..2c76a1de5a8 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -43,6 +43,7 @@
#include "target-descriptions.h"
#include "expop.h"
#include "arch/amd64-linux-tdesc.h"
+#include "inferior.h"
/* The syscall's XML filename for i386. */
#define XML_SYSCALL_FILENAME_AMD64 "syscalls/amd64-linux.xml"
@@ -50,6 +51,10 @@
#include "record-full.h"
#include "linux-record.h"
+#include <string_view>
+
+#define DEFAULT_TAG_MASK 0xffffffffffffffffULL
+
/* Mapping between the general-purpose registers in `struct user'
format and GDB's register cache layout. */
@@ -1767,6 +1772,62 @@ amd64_dtrace_parse_probe_argument (struct gdbarch *gdbarch,
}
}
+/* Extract the untagging mask based on the currently active linear address
+ masking (LAM) mode, which is stored in the /proc/<pid>/status file.
+ If we cannot extract the untag mask (for example, if we don't have
+ execution), we assume address tagging is not enabled and return the
+ DEFAULT_TAG_MASK. */
+
+static CORE_ADDR
+amd64_linux_lam_untag_mask ()
+{
+ if (!target_has_execution ())
+ return DEFAULT_TAG_MASK;
+
+ inferior *inf = current_inferior ();
+ if (inf->fake_pid_p)
+ return DEFAULT_TAG_MASK;
+
+ const std::string filename = string_printf ("/proc/%d/status", inf->pid);
+ gdb::unique_xmalloc_ptr<char> status_file
+ = target_fileio_read_stralloc (nullptr, filename.c_str ());
+
+ if (status_file == nullptr)
+ return DEFAULT_TAG_MASK;
+
+ std::string_view status_file_view (status_file.get ());
+ constexpr std::string_view untag_mask_str = "untag_mask:\t";
+ const size_t found = status_file_view.find (untag_mask_str);
+ if (found != std::string::npos)
+ {
+ const char* start = status_file_view.data() + found
+ + untag_mask_str.length ();
+ char* endptr;
+ errno = 0;
+ unsigned long long result = std::strtoul (start, &endptr, 0);
+ if (errno != 0 || endptr == start)
+ error (_("Failed to parse untag_mask from file %s."),
+ std::string (filename).c_str ());
+
+ return result;
+ }
+
+ return DEFAULT_TAG_MASK;
+}
+
+/* Adjust watchpoint address based on the currently active linear address
+ masking (LAM) mode using the untag mask. Check each time for a new
+ mask, as LAM is enabled at runtime. */
+
+static CORE_ADDR
+amd64_linux_remove_non_address_bits_watchpoint (gdbarch *gdbarch,
+ CORE_ADDR addr)
+{
+ /* Clear insignificant bits of a target address using the untag
+ mask. */
+ return (addr & amd64_linux_lam_untag_mask ());
+}
+
static void
amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
int num_disp_step_buffers)
@@ -1818,6 +1879,9 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_process_record (gdbarch, i386_process_record);
set_gdbarch_process_record_signal (gdbarch, amd64_linux_record_signal);
+
+ set_gdbarch_remove_non_address_bits_watchpoint
+ (gdbarch, amd64_linux_remove_non_address_bits_watchpoint);
}
static void
diff --git a/gdb/testsuite/gdb.arch/amd64-lam.c b/gdb/testsuite/gdb.arch/amd64-lam.c
new file mode 100755
index 00000000000..0fe2bc6c2ad
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-lam.c
@@ -0,0 +1,49 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2023-2024 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/>. */
+
+#define _GNU_SOURCE
+#include <stdint.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/syscall.h>
+#include <assert.h>
+#include <errno.h>
+#include <asm/prctl.h>
+
+int
+main (int argc, char **argv)
+{
+ int i;
+ int* pi = &i;
+ int* pi_tagged;
+
+ /* Enable LAM 57. */
+ errno = 0;
+ syscall (SYS_arch_prctl, ARCH_ENABLE_TAGGED_ADDR, 6);
+ assert_perror (errno);
+
+ /* Add tagging at bit 61. */
+ pi_tagged = (int *) ((uintptr_t) pi | (1LL << 60));
+
+ i = 0; /* Breakpoint here. */
+ *pi = 1;
+ *pi_tagged = 2;
+ *pi = 3;
+ *pi_tagged = 4;
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-lam.exp b/gdb/testsuite/gdb.arch/amd64-lam.exp
new file mode 100644
index 00000000000..0bcbb639b66
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-lam.exp
@@ -0,0 +1,46 @@
+# Copyright 2023-2024 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/>.
+
+# Test Linear Address Masking (LAM) support.
+
+require allow_lam_tests
+
+standard_testfile amd64-lam.c
+
+# Test LAM 57.
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+ return -1
+}
+
+if { ![runto_main] } {
+ return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "Breakpoint here"]
+gdb_continue_to_breakpoint "Breakpoint here"
+
+# Test hw watchpoint for a tagged and an untagged address with hit on a
+# tagged and an untagged address each.
+
+foreach symbol {"pi" "pi_tagged"} {
+ gdb_test "watch *${symbol}"
+ gdb_test "continue" \
+ "Continuing\\..*Hardware watchpoint \[0-9\]+.*" \
+ "run until watchpoint on ${symbol}"
+ gdb_test "continue" \
+ "Continuing\\..*Hardware watchpoint \[0-9\]+.*" \
+ "run until watchpoint on ${symbol}, 2nd hit"
+ delete_breakpoints
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a438a101fc9..0ab29f35393 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4295,6 +4295,69 @@ gdb_caching_proc allow_avx512fp16_tests {} {
return $allow_avx512fp16_tests
}
+# Run a test on the target to see if it supports LAM 57. Return 1 if so,
+# 0 if it does not. Based on the arch_prctl() handle ARCH_ENABLE_TAGGED_ADDR
+# to enable LAM which fails if the hardware or the OS does not support LAM.
+
+gdb_caching_proc allow_lam_tests {} {
+ global gdb_prompt inferior_exited_re
+
+ set me "allow_lam_tests"
+ if { ![istarget "x86_64-*-*"] } {
+ verbose "$me: target does not support LAM, returning 1" 2
+ return 0
+ }
+
+ # Compile a test program.
+ set src {
+ #define _GNU_SOURCE
+ #include <sys/syscall.h>
+ #include <assert.h>
+ #include <errno.h>
+ #include <asm/prctl.h>
+
+ int configure_lam ()
+ {
+ errno = 0;
+ syscall (SYS_arch_prctl, ARCH_ENABLE_TAGGED_ADDR, 6);
+ assert_perror (errno);
+ return errno;
+ }
+
+ int
+ main () { return configure_lam (); }
+ }
+
+ if {![gdb_simple_compile $me $src executable ""]} {
+ return 0
+ }
+ # No error message, compilation succeeded so now run it via gdb.
+
+ set allow_lam_tests 0
+ clean_restart $obj
+ gdb_run_cmd
+ gdb_expect {
+ -re ".*$inferior_exited_re with code.*${gdb_prompt} $" {
+ verbose -log "$me: LAM support not detected."
+ }
+ -re ".*Program received signal SIGABRT, Aborted.*${gdb_prompt} $" {
+ verbose -log "$me: LAM support not detected."
+ }
+ -re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
+ verbose -log "$me: LAM support detected."
+ set allow_lam_tests 1
+ }
+ default {
+ warning "\n$me: default case taken."
+ }
+ }
+ gdb_exit
+ remote_file build delete $obj
+
+ verbose "$me: returning $allow_lam_tests" 2
+ return $allow_lam_tests
+}
+
# Run a test on the target to see if it supports btrace hardware. Return 1 if so,
# 0 if it does not. Based on 'check_vmx_hw_available' from the GCC testsuite.
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 0/2] Add amd64 LAM watchpoint support
2024-10-28 8:46 [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
2024-10-28 8:46 ` [PATCH v6 1/2] gdb: Make tagged pointer support configurable Schimpe, Christina
2024-10-28 8:46 ` [PATCH v6 2/2] LAM: Enable tagged pointer support for watchpoints Schimpe, Christina
@ 2024-11-04 8:37 ` Schimpe, Christina
2 siblings, 0 replies; 12+ messages in thread
From: Schimpe, Christina @ 2024-11-04 8:37 UTC (permalink / raw)
To: gdb-patches, Luis Machado; +Cc: Schimpe, Christina
Hi Luis,
Is it possible that you would have a look at the remaining non-architecture specific parts of patch 1?
Note: There were also some conflicts for the aarch64* parts of patch 1 during the latest rebase. So that could would need a re-review as well, I think.
Best Regards + thank you very much in advance,
Christina
> -----Original Message-----
> From: Schimpe, Christina <christina.schimpe@intel.com>
> Sent: Monday, October 28, 2024 9:46 AM
> To: gdb-patches@sourceware.org
> Subject: [PATCH v6 0/2] Add amd64 LAM watchpoint support
>
> Hi all,
>
> this is my v6 of the series "Add amd64 LAM watchpoint support".
> I just rebased it on latest master since the posting of v5 has been a while and
> there were some conflicts in gdb/aarch64-tdep.c and loongarch-linux-nat.c.
>
> Besides that only the review for patch 1 is open, patch 2 has already been
> approved by Felix and Eli for the docs parts.
>
> v5 of this series can be found here:
> https://sourceware.org/pipermail/gdb-patches/2024-July/210414.html
>
> Best Regards
> Christina
>
> Christina Schimpe (2):
> gdb: Make tagged pointer support configurable.
> LAM: Enable tagged pointer support for watchpoints.
>
> gdb/NEWS | 5 +++
> gdb/aarch64-linux-nat.c | 2 +-
> gdb/aarch64-linux-tdep.c | 7 +--
> gdb/aarch64-tdep.c | 23 ++++++----
> gdb/aarch64-tdep.h | 6 +++
> gdb/amd64-linux-tdep.c | 64 +++++++++++++++++++++++++++
> gdb/breakpoint.c | 5 ++-
> gdb/gdbarch-gen.c | 66 +++++++++++++++++++++++-----
> gdb/gdbarch-gen.h | 49 ++++++++++++++++-----
> gdb/gdbarch_components.py | 53 ++++++++++++++++++----
> gdb/loongarch-linux-nat.c | 2 +-
> gdb/target.c | 3 +-
> gdb/testsuite/gdb.arch/amd64-lam.c | 49 +++++++++++++++++++++
> gdb/testsuite/gdb.arch/amd64-lam.exp | 46 +++++++++++++++++++
> gdb/testsuite/lib/gdb.exp | 63 ++++++++++++++++++++++++++
> 15 files changed, 396 insertions(+), 47 deletions(-) create mode 100755
> gdb/testsuite/gdb.arch/amd64-lam.c
> create mode 100644 gdb/testsuite/gdb.arch/amd64-lam.exp
>
> --
> 2.34.1
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-10-28 8:46 ` [PATCH v6 1/2] gdb: Make tagged pointer support configurable Schimpe, Christina
@ 2024-11-04 11:17 ` Luis Machado
2024-11-05 14:07 ` Schimpe, Christina
0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2024-11-04 11:17 UTC (permalink / raw)
To: Schimpe, Christina, gdb-patches
Hi Christina,
On 10/28/24 08:46, Schimpe, Christina wrote:
> From: Christina Schimpe <christina.schimpe@intel.com>
>
> The gdbarch function gdbarch_remove_non_address_bits adjusts addresses to
> enable debugging of programs with tagged pointers on Linux, for instance for
> ARM's feature top byte ignore (TBI).
> Once the function is implemented for an architecture, it adjusts addresses for
> memory access, breakpoints and watchpoints.
>
> Linear address masking (LAM) is Intel's (R) implementation of tagged
> pointer support. It requires certain adaptions to GDB's tagged pointer
> support due to the following:
> - LAM supports address tagging for data accesses only. Thus, specifying
> breakpoints on tagged addresses is not a valid use case.
> - In contrast to the implementation for ARM's TBI, the Linux kernel supports
> tagged pointers for memory access.
>
> This patch makes GDB's tagged pointer support configurable such that it is
> possible to enable the address adjustment for a specific feature only (e.g
> memory access, breakpoints or watchpoints). This way, one can make sure
> that addresses are only adjusted when necessary. In case of LAM, this
> avoids unnecessary parsing of the /proc/<pid>/status file to get the
> untag mask.
>
> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
> ---
> gdb/aarch64-linux-nat.c | 2 +-
> gdb/aarch64-linux-tdep.c | 7 +++--
> gdb/aarch64-tdep.c | 23 ++++++++------
> gdb/aarch64-tdep.h | 6 ++++
> gdb/breakpoint.c | 5 +--
> gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++-------
> gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
> gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
> gdb/loongarch-linux-nat.c | 2 +-
> gdb/target.c | 3 +-
> 10 files changed, 169 insertions(+), 47 deletions(-)
>
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 0fa5bee500b..48ab765d880 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
> kernel can potentially be tagged addresses. */
> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> const CORE_ADDR addr_trap
> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
>
> /* Check if the address matches any watched address. */
> state = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index c608a84bc71..61c3be8b6f0 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -2433,7 +2433,7 @@ static bool
> aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR address)
> {
> /* Remove the top byte for the memory range check. */
> - address = gdbarch_remove_non_address_bits (gdbarch, address);
> + address = aarch64_remove_non_address_bits (gdbarch, address);
>
> /* Check if the page that contains ADDRESS is mapped with PROT_MTE. */
> if (!linux_address_in_memtag_page (address))
> @@ -2491,8 +2491,9 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
> uiout->text ("\n");
>
> std::optional<CORE_ADDR> atag
> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> - fault_addr));
> + = aarch64_mte_get_atag (
> + aarch64_remove_non_address_bits (gdbarch, fault_addr));
> +
> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
>
> if (!atag.has_value ())
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 8a2a9b1e23c..91fec7879ed 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch *gdbarch,
>
> /* Fetch the allocation tag for ADDRESS. */
> std::optional<CORE_ADDR> atag
> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch, addr));
> + = aarch64_mte_get_atag (aarch64_remove_non_address_bits (gdbarch, addr));
>
> if (!atag.has_value ())
> return true;
> @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch *gdbarch, struct value *address,
> else
> {
> /* Remove the top byte. */
> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
>
> /* With G being the number of tag granules and N the number of tags
> passed in, we can have the following cases:
> @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch *gdbarch, struct value *address,
> else
> {
> /* Remove the top byte. */
> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
>
> if (!atag.has_value ())
> @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch *gdbarch, struct value *tag_value)
> return string_printf ("0x%s", phex_nz (tag, sizeof (tag)));
> }
>
> -/* AArch64 implementation of the remove_non_address_bits gdbarch hook. Remove
> - non address bits from a pointer value. */
> +/* See aarch64-tdep.h. */
>
> -static CORE_ADDR
> +CORE_ADDR
> aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> {
> /* By default, we assume TBI and discard the top 8 bits plus the VA range
> @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
>
> /* Architecture hook to remove bits of a pointer that are not part of the
> - address, like memory tags (MTE) and pointer authentication signatures. */
> - set_gdbarch_remove_non_address_bits (gdbarch,
> - aarch64_remove_non_address_bits);
> + address, like memory tags (MTE) and pointer authentication signatures.
> + Configure address adjustment for watch-, breakpoints and memory
s/watch-/watchpoints
> + transfer. */
> + set_gdbarch_remove_non_address_bits_watchpoint
> + (gdbarch, aarch64_remove_non_address_bits);
> + set_gdbarch_remove_non_address_bits_breakpoint
> + (gdbarch, aarch64_remove_non_address_bits);
> + set_gdbarch_remove_non_address_bits_memory
> + (gdbarch, aarch64_remove_non_address_bits);
>
> /* SME pseudo-registers. */
> if (tdep->has_sme ())
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index 50166fb4f24..13ea37de7a3 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch);
>
> std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR address);
>
> +/* AArch64 implementation of the remove_non_address_bits gdbarch hooks.
> + Remove non address bits from a pointer value. */
> +
> +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
> + CORE_ADDR pointer);
> +
> #endif /* aarch64-tdep.h */
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index b7e4f5d0a45..9983e905e1b 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b, bool reparse)
> loc->gdbarch = v->type ()->arch ();
> loc->pspace = wp_pspace;
> loc->address
> - = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
> + = gdbarch_remove_non_address_bits_watchpoint (loc->gdbarch,
> + addr);
> b->add_location (*loc);
>
> if (bitsize != 0)
> @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
> }
>
> adjusted_bpaddr
> - = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
> + = gdbarch_remove_non_address_bits_breakpoint (gdbarch, adjusted_bpaddr);
>
> /* An adjusted breakpoint address can significantly alter
> a user's expectations. Print a warning if an adjustment
> diff --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c
> index 0d00cd7c993..d05c7a3cbdf 100644
> --- a/gdb/gdbarch-gen.c
> +++ b/gdb/gdbarch-gen.c
> @@ -143,7 +143,9 @@ struct gdbarch
> int frame_red_zone_size = 0;
> gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity;
> gdbarch_addr_bits_remove_ftype *addr_bits_remove = core_addr_identity;
> - gdbarch_remove_non_address_bits_ftype *remove_non_address_bits = default_remove_non_address_bits;
> + gdbarch_remove_non_address_bits_watchpoint_ftype *remove_non_address_bits_watchpoint = default_remove_non_address_bits;
> + gdbarch_remove_non_address_bits_breakpoint_ftype *remove_non_address_bits_breakpoint = default_remove_non_address_bits;
> + gdbarch_remove_non_address_bits_memory_ftype *remove_non_address_bits_memory = default_remove_non_address_bits;
> gdbarch_memtag_to_string_ftype *memtag_to_string = default_memtag_to_string;
> gdbarch_tagged_address_p_ftype *tagged_address_p = default_tagged_address_p;
> gdbarch_memtag_matches_p_ftype *memtag_matches_p = default_memtag_matches_p;
> @@ -407,7 +409,9 @@ verify_gdbarch (struct gdbarch *gdbarch)
> /* Skip verify of frame_red_zone_size, invalid_p == 0. */
> /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
> /* Skip verify of addr_bits_remove, invalid_p == 0. */
> - /* Skip verify of remove_non_address_bits, invalid_p == 0. */
> + /* Skip verify of remove_non_address_bits_watchpoint, invalid_p == 0. */
> + /* Skip verify of remove_non_address_bits_breakpoint, invalid_p == 0. */
> + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0. */
> /* Skip verify of memtag_to_string, invalid_p == 0. */
> /* Skip verify of tagged_address_p, invalid_p == 0. */
> /* Skip verify of memtag_matches_p, invalid_p == 0. */
> @@ -910,8 +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
> "gdbarch_dump: addr_bits_remove = <%s>\n",
> host_address_to_string (gdbarch->addr_bits_remove));
> gdb_printf (file,
> - "gdbarch_dump: remove_non_address_bits = <%s>\n",
> - host_address_to_string (gdbarch->remove_non_address_bits));
> + "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
> + host_address_to_string (gdbarch->remove_non_address_bits_watchpoint));
> + gdb_printf (file,
> + "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
> + host_address_to_string (gdbarch->remove_non_address_bits_breakpoint));
> + gdb_printf (file,
> + "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
> + host_address_to_string (gdbarch->remove_non_address_bits_memory));
> gdb_printf (file,
> "gdbarch_dump: memtag_to_string = <%s>\n",
> host_address_to_string (gdbarch->memtag_to_string));
> @@ -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
> }
>
> CORE_ADDR
> -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +{
> + gdb_assert (gdbarch != NULL);
> + gdb_assert (gdbarch->remove_non_address_bits_watchpoint != NULL);
> + if (gdbarch_debug >= 2)
> + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_watchpoint called\n");
> + return gdbarch->remove_non_address_bits_watchpoint (gdbarch, pointer);
> +}
> +
> +void
> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
> + gdbarch_remove_non_address_bits_watchpoint_ftype remove_non_address_bits_watchpoint)
> +{
> + gdbarch->remove_non_address_bits_watchpoint = remove_non_address_bits_watchpoint;
> +}
> +
> +CORE_ADDR
> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +{
> + gdb_assert (gdbarch != NULL);
> + gdb_assert (gdbarch->remove_non_address_bits_breakpoint != NULL);
> + if (gdbarch_debug >= 2)
> + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_breakpoint called\n");
> + return gdbarch->remove_non_address_bits_breakpoint (gdbarch, pointer);
> +}
> +
> +void
> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
> + gdbarch_remove_non_address_bits_breakpoint_ftype remove_non_address_bits_breakpoint)
> +{
> + gdbarch->remove_non_address_bits_breakpoint = remove_non_address_bits_breakpoint;
> +}
> +
> +CORE_ADDR
> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch, CORE_ADDR pointer)
> {
> gdb_assert (gdbarch != NULL);
> - gdb_assert (gdbarch->remove_non_address_bits != NULL);
> + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
> if (gdbarch_debug >= 2)
> - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
> - return gdbarch->remove_non_address_bits (gdbarch, pointer);
> + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_memory called\n");
> + return gdbarch->remove_non_address_bits_memory (gdbarch, pointer);
> }
>
> void
> -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> - gdbarch_remove_non_address_bits_ftype remove_non_address_bits)
> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> + gdbarch_remove_non_address_bits_memory_ftype remove_non_address_bits_memory)
> {
> - gdbarch->remove_non_address_bits = remove_non_address_bits;
> + gdbarch->remove_non_address_bits_memory = remove_non_address_bits_memory;
> }
>
> std::string
> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index b982fd7cd09..9fda85f860f 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR ad
> extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_bits_remove_ftype *addr_bits_remove);
>
> /* On some architectures, not all bits of a pointer are significant.
> - On AArch64, for example, the top bits of a pointer may carry a "tag", which
> - can be ignored by the kernel and the hardware. The "tag" can be regarded as
> - additional data associated with the pointer, but it is not part of the address.
> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> + "tag", which can be ignored by the kernel and the hardware. The "tag" can be
> + regarded as additional data associated with the pointer, but it is not part
> + of the address.
>
> Given a pointer for the architecture, this hook removes all the
> - non-significant bits and sign-extends things as needed. It gets used to remove
> - non-address bits from data pointers (for example, removing the AArch64 MTE tag
> - bits from a pointer) and from code pointers (removing the AArch64 PAC signature
> - from a pointer containing the return address). */
> -
> -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
> -extern CORE_ADDR gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer);
> -extern void set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype *remove_non_address_bits);
> + non-significant bits and sign-extends things as needed. It gets used to
> + remove non-address bits from pointers used for watchpoints. */
> +
> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_watchpoint_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern CORE_ADDR gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern void set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype *remove_non_address_bits_watchpoint);
> +
> +/* On some architectures, not all bits of a pointer are significant.
> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> + "tag", which can be ignored by the kernel and the hardware. The "tag" can be
> + regarded as additional data associated with the pointer, but it is not part
> + of the address.
> +
> + Given a pointer for the architecture, this hook removes all the
> + non-significant bits and sign-extends things as needed. It gets used to
> + remove non-address bits from pointers used for breakpoints. */
> +
> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_breakpoint_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern CORE_ADDR gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern void set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype *remove_non_address_bits_breakpoint);
> +
> +/* On some architectures, not all bits of a pointer are significant.
> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> + "tag", which can be ignored by the kernel and the hardware. The "tag" can be
> + regarded as additional data associated with the pointer, but it is not part
> + of the address.
> +
> + Given a pointer for the architecture, this hook removes all the
> + non-significant bits and sign-extends things as needed. It gets used to
> + remove non-address bits from any pointer used to access memory. */
> +
> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_memory_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern CORE_ADDR gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern void set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_memory_ftype *remove_non_address_bits_memory);
>
> /* Return a string representation of the memory tag TAG. */
>
> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> index 4006380076d..cc7c6d8677b 100644
> --- a/gdb/gdbarch_components.py
> +++ b/gdb/gdbarch_components.py
> @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC instead).
> Method(
> comment="""
> On some architectures, not all bits of a pointer are significant.
> -On AArch64, for example, the top bits of a pointer may carry a "tag", which
> -can be ignored by the kernel and the hardware. The "tag" can be regarded as
> -additional data associated with the pointer, but it is not part of the address.
> +On AArch64 and amd64, for example, the top bits of a pointer may carry a
> +"tag", which can be ignored by the kernel and the hardware. The "tag" can be
> +regarded as additional data associated with the pointer, but it is not part
> +of the address.
>
> Given a pointer for the architecture, this hook removes all the
> -non-significant bits and sign-extends things as needed. It gets used to remove
> -non-address bits from data pointers (for example, removing the AArch64 MTE tag
> -bits from a pointer) and from code pointers (removing the AArch64 PAC signature
> -from a pointer containing the return address).
> +non-significant bits and sign-extends things as needed. It gets used to
> +remove non-address bits from pointers used for watchpoints.
> """,
> type="CORE_ADDR",
> - name="remove_non_address_bits",
> + name="remove_non_address_bits_watchpoint",
> + params=[("CORE_ADDR", "pointer")],
> + predefault="default_remove_non_address_bits",
> + invalid=False,
> +)
> +
> +Method(
> + comment="""
> +On some architectures, not all bits of a pointer are significant.
> +On AArch64 and amd64, for example, the top bits of a pointer may carry a
> +"tag", which can be ignored by the kernel and the hardware. The "tag" can be
> +regarded as additional data associated with the pointer, but it is not part
> +of the address.
> +
> +Given a pointer for the architecture, this hook removes all the
> +non-significant bits and sign-extends things as needed. It gets used to
> +remove non-address bits from pointers used for breakpoints.
> +""",
> + type="CORE_ADDR",
> + name="remove_non_address_bits_breakpoint",
> + params=[("CORE_ADDR", "pointer")],
> + predefault="default_remove_non_address_bits",
> + invalid=False,
> +)
> +
> +Method(
> + comment="""
> +On some architectures, not all bits of a pointer are significant.
> +On AArch64 and amd64, for example, the top bits of a pointer may carry a
> +"tag", which can be ignored by the kernel and the hardware. The "tag" can be
> +regarded as additional data associated with the pointer, but it is not part
> +of the address.
> +
> +Given a pointer for the architecture, this hook removes all the
> +non-significant bits and sign-extends things as needed. It gets used to
> +remove non-address bits from any pointer used to access memory.
> +""",
> + type="CORE_ADDR",
> + name="remove_non_address_bits_memory",
> params=[("CORE_ADDR", "pointer")],
> predefault="default_remove_non_address_bits",
> invalid=False,
> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> index bc9927dd751..dc4a019614a 100644
> --- a/gdb/loongarch-linux-nat.c
> +++ b/gdb/loongarch-linux-nat.c
> @@ -613,7 +613,7 @@ loongarch_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
> kernel can potentially be tagged addresses. */
> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> const CORE_ADDR addr_trap
> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
>
> /* Check if the address matches any watched address. */
> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
I think the loongarch-linux-nat.c change is spurious. Could you please address that?
> diff --git a/gdb/target.c b/gdb/target.c
> index 4378c0572d2..b6d1abe82db 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1608,7 +1608,8 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
> if (len == 0)
> return TARGET_XFER_EOF;
>
> - memaddr = gdbarch_remove_non_address_bits (current_inferior ()->arch (),
> + memaddr
> + = gdbarch_remove_non_address_bits_memory (current_inferior ()->arch (),
> memaddr);
>
> /* Fill in READBUF with breakpoint shadows, or WRITEBUF with
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-11-04 11:17 ` Luis Machado
@ 2024-11-05 14:07 ` Schimpe, Christina
2024-11-05 14:22 ` Luis Machado
0 siblings, 1 reply; 12+ messages in thread
From: Schimpe, Christina @ 2024-11-05 14:07 UTC (permalink / raw)
To: Luis Machado, gdb-patches
Hi Luis,
Thanks a lot for the review. I have one questions to your comment, please see below.
> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Monday, November 4, 2024 12:18 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>
> Hi Christina,
>
> On 10/28/24 08:46, Schimpe, Christina wrote:
> > From: Christina Schimpe <christina.schimpe@intel.com>
> >
> > The gdbarch function gdbarch_remove_non_address_bits adjusts addresses
> > to enable debugging of programs with tagged pointers on Linux, for
> > instance for ARM's feature top byte ignore (TBI).
> > Once the function is implemented for an architecture, it adjusts
> > addresses for memory access, breakpoints and watchpoints.
> >
> > Linear address masking (LAM) is Intel's (R) implementation of tagged
> > pointer support. It requires certain adaptions to GDB's tagged
> > pointer support due to the following:
> > - LAM supports address tagging for data accesses only. Thus, specifying
> > breakpoints on tagged addresses is not a valid use case.
> > - In contrast to the implementation for ARM's TBI, the Linux kernel supports
> > tagged pointers for memory access.
> >
> > This patch makes GDB's tagged pointer support configurable such that
> > it is possible to enable the address adjustment for a specific feature
> > only (e.g memory access, breakpoints or watchpoints). This way, one
> > can make sure that addresses are only adjusted when necessary. In
> > case of LAM, this avoids unnecessary parsing of the /proc/<pid>/status
> > file to get the untag mask.
> >
> > Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
> > ---
> > gdb/aarch64-linux-nat.c | 2 +-
> > gdb/aarch64-linux-tdep.c | 7 +++--
> > gdb/aarch64-tdep.c | 23 ++++++++------
> > gdb/aarch64-tdep.h | 6 ++++
> > gdb/breakpoint.c | 5 +--
> > gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++-------
> > gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
> > gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
> > gdb/loongarch-linux-nat.c | 2 +-
> > gdb/target.c | 3 +-
> > 10 files changed, 169 insertions(+), 47 deletions(-)
> >
> > diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index
> > 0fa5bee500b..48ab765d880 100644
> > --- a/gdb/aarch64-linux-nat.c
> > +++ b/gdb/aarch64-linux-nat.c
> > @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address
> (CORE_ADDR *addr_p)
> > kernel can potentially be tagged addresses. */
> > struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> > const CORE_ADDR addr_trap
> > - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> siginfo.si_addr);
> > + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> > + siginfo.si_addr);
> >
> > /* Check if the address matches any watched address. */
> > state = aarch64_get_debug_reg_state (inferior_ptid.pid ()); diff
> > --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index
> > c608a84bc71..61c3be8b6f0 100644
> > --- a/gdb/aarch64-linux-tdep.c
> > +++ b/gdb/aarch64-linux-tdep.c
> > @@ -2433,7 +2433,7 @@ static bool
> > aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR
> > address) {
> > /* Remove the top byte for the memory range check. */
> > - address = gdbarch_remove_non_address_bits (gdbarch, address);
> > + address = aarch64_remove_non_address_bits (gdbarch, address);
> >
> > /* Check if the page that contains ADDRESS is mapped with PROT_MTE. */
> > if (!linux_address_in_memtag_page (address)) @@ -2491,8 +2491,9 @@
> > aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
> > uiout->text ("\n");
> >
> > std::optional<CORE_ADDR> atag
> > - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> > - fault_addr));
> > + = aarch64_mte_get_atag (
> > + aarch64_remove_non_address_bits (gdbarch, fault_addr));
> > +
> > gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
> >
> > if (!atag.has_value ())
> > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> > 8a2a9b1e23c..91fec7879ed 100644
> > --- a/gdb/aarch64-tdep.c
> > +++ b/gdb/aarch64-tdep.c
> > @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch
> > *gdbarch,
> >
> > /* Fetch the allocation tag for ADDRESS. */
> > std::optional<CORE_ADDR> atag
> > - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> addr));
> > + = aarch64_mte_get_atag (aarch64_remove_non_address_bits (gdbarch,
> > + addr));
> >
> > if (!atag.has_value ())
> > return true;
> > @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch *gdbarch,
> struct value *address,
> > else
> > {
> > /* Remove the top byte. */
> > - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> > + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >
> > /* With G being the number of tag granules and N the number of tags
> > passed in, we can have the following cases:
> > @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch *gdbarch,
> struct value *address,
> > else
> > {
> > /* Remove the top byte. */
> > - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> > + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> > std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
> >
> > if (!atag.has_value ())
> > @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
> *gdbarch, struct value *tag_value)
> > return string_printf ("0x%s", phex_nz (tag, sizeof (tag))); }
> >
> > -/* AArch64 implementation of the remove_non_address_bits gdbarch hook.
> Remove
> > - non address bits from a pointer value. */
> > +/* See aarch64-tdep.h. */
> >
> > -static CORE_ADDR
> > +CORE_ADDR
> > aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> > pointer) {
> > /* By default, we assume TBI and discard the top 8 bits plus the VA
> > range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct gdbarch_info
> info, struct gdbarch_list *arches)
> > tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
> >
> > /* Architecture hook to remove bits of a pointer that are not part of the
> > - address, like memory tags (MTE) and pointer authentication signatures. */
> > - set_gdbarch_remove_non_address_bits (gdbarch,
> > - aarch64_remove_non_address_bits);
> > + address, like memory tags (MTE) and pointer authentication signatures.
> > + Configure address adjustment for watch-, breakpoints and memory
>
> s/watch-/watchpoints
Fill fix, thanks.
> > + transfer. */
> > + set_gdbarch_remove_non_address_bits_watchpoint
> > + (gdbarch, aarch64_remove_non_address_bits);
> > + set_gdbarch_remove_non_address_bits_breakpoint
> > + (gdbarch, aarch64_remove_non_address_bits);
> > + set_gdbarch_remove_non_address_bits_memory
> > + (gdbarch, aarch64_remove_non_address_bits);
> >
> > /* SME pseudo-registers. */
> > if (tdep->has_sme ())
> > diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
> > 50166fb4f24..13ea37de7a3 100644
> > --- a/gdb/aarch64-tdep.h
> > +++ b/gdb/aarch64-tdep.h
> > @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep (struct
> > gdbarch *gdbarch);
> >
> > std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR address);
> >
> > +/* AArch64 implementation of the remove_non_address_bits gdbarch hooks.
> > + Remove non address bits from a pointer value. */
> > +
> > +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
> > + CORE_ADDR pointer);
> > +
> > #endif /* aarch64-tdep.h */
> > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
> > b7e4f5d0a45..9983e905e1b 100644
> > --- a/gdb/breakpoint.c
> > +++ b/gdb/breakpoint.c
> > @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b, bool
> reparse)
> > loc->gdbarch = v->type ()->arch ();
> > loc->pspace = wp_pspace;
> > loc->address
> > - = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
> > + = gdbarch_remove_non_address_bits_watchpoint (loc-
> >gdbarch,
> > + addr);
> > b->add_location (*loc);
> >
> > if (bitsize != 0)
> > @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
> *gdbarch,
> > }
> >
> > adjusted_bpaddr
> > - = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
> > + = gdbarch_remove_non_address_bits_breakpoint (gdbarch,
> > +adjusted_bpaddr);
> >
> > /* An adjusted breakpoint address can significantly alter
> > a user's expectations. Print a warning if an adjustment diff --git
> > a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index 0d00cd7c993..d05c7a3cbdf
> > 100644
> > --- a/gdb/gdbarch-gen.c
> > +++ b/gdb/gdbarch-gen.c
> > @@ -143,7 +143,9 @@ struct gdbarch
> > int frame_red_zone_size = 0;
> > gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr =
> convert_from_func_ptr_addr_identity;
> > gdbarch_addr_bits_remove_ftype *addr_bits_remove =
> > core_addr_identity;
> > - gdbarch_remove_non_address_bits_ftype *remove_non_address_bits =
> > default_remove_non_address_bits;
> > + gdbarch_remove_non_address_bits_watchpoint_ftype
> > + *remove_non_address_bits_watchpoint =
> > + default_remove_non_address_bits;
> > + gdbarch_remove_non_address_bits_breakpoint_ftype
> > + *remove_non_address_bits_breakpoint =
> > + default_remove_non_address_bits;
> > + gdbarch_remove_non_address_bits_memory_ftype
> > + *remove_non_address_bits_memory = default_remove_non_address_bits;
> > gdbarch_memtag_to_string_ftype *memtag_to_string =
> default_memtag_to_string;
> > gdbarch_tagged_address_p_ftype *tagged_address_p =
> default_tagged_address_p;
> > gdbarch_memtag_matches_p_ftype *memtag_matches_p =
> > default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch (struct
> gdbarch *gdbarch)
> > /* Skip verify of frame_red_zone_size, invalid_p == 0. */
> > /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
> > /* Skip verify of addr_bits_remove, invalid_p == 0. */
> > - /* Skip verify of remove_non_address_bits, invalid_p == 0. */
> > + /* Skip verify of remove_non_address_bits_watchpoint, invalid_p ==
> > + 0. */
> > + /* Skip verify of remove_non_address_bits_breakpoint, invalid_p ==
> > + 0. */
> > + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
> > + */
> > /* Skip verify of memtag_to_string, invalid_p == 0. */
> > /* Skip verify of tagged_address_p, invalid_p == 0. */
> > /* Skip verify of memtag_matches_p, invalid_p == 0. */ @@ -910,8
> > +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
> > "gdbarch_dump: addr_bits_remove = <%s>\n",
> > host_address_to_string (gdbarch->addr_bits_remove));
> > gdb_printf (file,
> > - "gdbarch_dump: remove_non_address_bits = <%s>\n",
> > - host_address_to_string (gdbarch->remove_non_address_bits));
> > + "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
> > + host_address_to_string
> > +(gdbarch->remove_non_address_bits_watchpoint));
> > + gdb_printf (file,
> > + "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
> > + host_address_to_string
> > +(gdbarch->remove_non_address_bits_breakpoint));
> > + gdb_printf (file,
> > + "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
> > + host_address_to_string
> > +(gdbarch->remove_non_address_bits_memory));
> > gdb_printf (file,
> > "gdbarch_dump: memtag_to_string = <%s>\n",
> > host_address_to_string (gdbarch->memtag_to_string)); @@
> > -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch
> > *gdbarch, }
> >
> > CORE_ADDR
> > -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> > pointer)
> > +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer) {
> > + gdb_assert (gdbarch != NULL);
> > + gdb_assert (gdbarch->remove_non_address_bits_watchpoint != NULL);
> > + if (gdbarch_debug >= 2)
> > + gdb_printf (gdb_stdlog,
> > +"gdbarch_remove_non_address_bits_watchpoint called\n");
> > + return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
> > +pointer); }
> > +
> > +void
> > +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> *gdbarch,
> > +
> gdbarch_remove_non_address_bits_watchpoint_ftype
> > +remove_non_address_bits_watchpoint)
> > +{
> > + gdbarch->remove_non_address_bits_watchpoint =
> > +remove_non_address_bits_watchpoint;
> > +}
> > +
> > +CORE_ADDR
> > +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer) {
> > + gdb_assert (gdbarch != NULL);
> > + gdb_assert (gdbarch->remove_non_address_bits_breakpoint != NULL);
> > + if (gdbarch_debug >= 2)
> > + gdb_printf (gdb_stdlog,
> > +"gdbarch_remove_non_address_bits_breakpoint called\n");
> > + return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
> > +pointer); }
> > +
> > +void
> > +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
> > +
> gdbarch_remove_non_address_bits_breakpoint_ftype
> > +remove_non_address_bits_breakpoint)
> > +{
> > + gdbarch->remove_non_address_bits_breakpoint =
> > +remove_non_address_bits_breakpoint;
> > +}
> > +
> > +CORE_ADDR
> > +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer)
> > {
> > gdb_assert (gdbarch != NULL);
> > - gdb_assert (gdbarch->remove_non_address_bits != NULL);
> > + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
> > if (gdbarch_debug >= 2)
> > - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
> > - return gdbarch->remove_non_address_bits (gdbarch, pointer);
> > + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_memory
> > + called\n"); return gdbarch->remove_non_address_bits_memory
> > + (gdbarch, pointer);
> > }
> >
> > void
> > -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> > - gdbarch_remove_non_address_bits_ftype
> remove_non_address_bits)
> > +set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +
> gdbarch_remove_non_address_bits_memory_ftype
> > +remove_non_address_bits_memory)
> > {
> > - gdbarch->remove_non_address_bits = remove_non_address_bits;
> > + gdbarch->remove_non_address_bits_memory =
> > + remove_non_address_bits_memory;
> > }
> >
> > std::string
> > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
> > b982fd7cd09..9fda85f860f 100644
> > --- a/gdb/gdbarch-gen.h
> > +++ b/gdb/gdbarch-gen.h
> > @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove
> > (struct gdbarch *gdbarch, CORE_ADDR ad extern void
> > set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
> > gdbarch_addr_bits_remove_ftype *addr_bits_remove);
> >
> > /* On some architectures, not all bits of a pointer are significant.
> > - On AArch64, for example, the top bits of a pointer may carry a "tag", which
> > - can be ignored by the kernel and the hardware. The "tag" can be regarded as
> > - additional data associated with the pointer, but it is not part of the address.
> > + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> > + "tag", which can be ignored by the kernel and the hardware. The "tag" can
> be
> > + regarded as additional data associated with the pointer, but it is not part
> > + of the address.
> >
> > Given a pointer for the architecture, this hook removes all the
> > - non-significant bits and sign-extends things as needed. It gets used to
> remove
> > - non-address bits from data pointers (for example, removing the AArch64 MTE
> tag
> > - bits from a pointer) and from code pointers (removing the AArch64 PAC
> signature
> > - from a pointer containing the return address). */
> > -
> > -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct
> > gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
> > gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> > pointer); -extern void set_gdbarch_remove_non_address_bits (struct
> > gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype
> > *remove_non_address_bits);
> > + non-significant bits and sign-extends things as needed. It gets used to
> > + remove non-address bits from pointers used for watchpoints. */
> > +
> > +typedef CORE_ADDR (gdbarch_remove_non_address_bits_watchpoint_ftype)
> > +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> > +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer); extern void
> > +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> > +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
> > +*remove_non_address_bits_watchpoint);
> > +
> > +/* On some architectures, not all bits of a pointer are significant.
> > + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> > + "tag", which can be ignored by the kernel and the hardware. The "tag" can
> be
> > + regarded as additional data associated with the pointer, but it is not part
> > + of the address.
> > +
> > + Given a pointer for the architecture, this hook removes all the
> > + non-significant bits and sign-extends things as needed. It gets used to
> > + remove non-address bits from pointers used for breakpoints. */
> > +
> > +typedef CORE_ADDR (gdbarch_remove_non_address_bits_breakpoint_ftype)
> > +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> > +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer); extern void
> > +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> > +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
> > +*remove_non_address_bits_breakpoint);
> > +
> > +/* On some architectures, not all bits of a pointer are significant.
> > + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> > + "tag", which can be ignored by the kernel and the hardware. The "tag" can
> be
> > + regarded as additional data associated with the pointer, but it is not part
> > + of the address.
> > +
> > + Given a pointer for the architecture, this hook removes all the
> > + non-significant bits and sign-extends things as needed. It gets used to
> > + remove non-address bits from any pointer used to access memory. */
> > +
> > +typedef CORE_ADDR (gdbarch_remove_non_address_bits_memory_ftype)
> > +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> > +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer); extern void
> > +set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +gdbarch_remove_non_address_bits_memory_ftype
> > +*remove_non_address_bits_memory);
> >
> > /* Return a string representation of the memory tag TAG. */
> >
> > diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> > index 4006380076d..cc7c6d8677b 100644
> > --- a/gdb/gdbarch_components.py
> > +++ b/gdb/gdbarch_components.py
> > @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
> instead).
> > Method(
> > comment="""
> > On some architectures, not all bits of a pointer are significant.
> > -On AArch64, for example, the top bits of a pointer may carry a "tag",
> > which -can be ignored by the kernel and the hardware. The "tag" can
> > be regarded as -additional data associated with the pointer, but it is not part of
> the address.
> > +On AArch64 and amd64, for example, the top bits of a pointer may
> > +carry a "tag", which can be ignored by the kernel and the hardware.
> > +The "tag" can be regarded as additional data associated with the
> > +pointer, but it is not part of the address.
> >
> > Given a pointer for the architecture, this hook removes all the
> > -non-significant bits and sign-extends things as needed. It gets used
> > to remove -non-address bits from data pointers (for example, removing
> > the AArch64 MTE tag -bits from a pointer) and from code pointers
> > (removing the AArch64 PAC signature -from a pointer containing the return
> address).
> > +non-significant bits and sign-extends things as needed. It gets used
> > +to remove non-address bits from pointers used for watchpoints.
> > """,
> > type="CORE_ADDR",
> > - name="remove_non_address_bits",
> > + name="remove_non_address_bits_watchpoint",
> > + params=[("CORE_ADDR", "pointer")],
> > + predefault="default_remove_non_address_bits",
> > + invalid=False,
> > +)
> > +
> > +Method(
> > + comment="""
> > +On some architectures, not all bits of a pointer are significant.
> > +On AArch64 and amd64, for example, the top bits of a pointer may
> > +carry a "tag", which can be ignored by the kernel and the hardware.
> > +The "tag" can be regarded as additional data associated with the
> > +pointer, but it is not part of the address.
> > +
> > +Given a pointer for the architecture, this hook removes all the
> > +non-significant bits and sign-extends things as needed. It gets used
> > +to remove non-address bits from pointers used for breakpoints.
> > +""",
> > + type="CORE_ADDR",
> > + name="remove_non_address_bits_breakpoint",
> > + params=[("CORE_ADDR", "pointer")],
> > + predefault="default_remove_non_address_bits",
> > + invalid=False,
> > +)
> > +
> > +Method(
> > + comment="""
> > +On some architectures, not all bits of a pointer are significant.
> > +On AArch64 and amd64, for example, the top bits of a pointer may
> > +carry a "tag", which can be ignored by the kernel and the hardware.
> > +The "tag" can be regarded as additional data associated with the
> > +pointer, but it is not part of the address.
> > +
> > +Given a pointer for the architecture, this hook removes all the
> > +non-significant bits and sign-extends things as needed. It gets used
> > +to remove non-address bits from any pointer used to access memory.
> > +""",
> > + type="CORE_ADDR",
> > + name="remove_non_address_bits_memory",
> > params=[("CORE_ADDR", "pointer")],
> > predefault="default_remove_non_address_bits",
> > invalid=False,
> > diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> > index bc9927dd751..dc4a019614a 100644
> > --- a/gdb/loongarch-linux-nat.c
> > +++ b/gdb/loongarch-linux-nat.c
> > @@ -613,7 +613,7 @@ loongarch_linux_nat_target::stopped_data_address
> (CORE_ADDR *addr_p)
> > kernel can potentially be tagged addresses. */
> > struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> > const CORE_ADDR addr_trap
> > - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> siginfo.si_addr);
> > + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> > + siginfo.si_addr);
> >
> > /* Check if the address matches any watched address. */
> > state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
>
> I think the loongarch-linux-nat.c change is spurious. Could you please address
> that?
Why is it spurious? What do you mean exactly?
Thanks,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-11-05 14:07 ` Schimpe, Christina
@ 2024-11-05 14:22 ` Luis Machado
2024-11-05 14:40 ` Schimpe, Christina
0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2024-11-05 14:22 UTC (permalink / raw)
To: Schimpe, Christina, gdb-patches; +Cc: Tiezhu Yang
On 11/5/24 14:07, Schimpe, Christina wrote:
> Hi Luis,
>
> Thanks a lot for the review. I have one questions to your comment, please see below.
>
>> -----Original Message-----
>> From: Luis Machado <luis.machado@arm.com>
>> Sent: Monday, November 4, 2024 12:18 PM
>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>> patches@sourceware.org
>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>>
>> Hi Christina,
>>
>> On 10/28/24 08:46, Schimpe, Christina wrote:
>>> From: Christina Schimpe <christina.schimpe@intel.com>
>>>
>>> The gdbarch function gdbarch_remove_non_address_bits adjusts addresses
>>> to enable debugging of programs with tagged pointers on Linux, for
>>> instance for ARM's feature top byte ignore (TBI).
>>> Once the function is implemented for an architecture, it adjusts
>>> addresses for memory access, breakpoints and watchpoints.
>>>
>>> Linear address masking (LAM) is Intel's (R) implementation of tagged
>>> pointer support. It requires certain adaptions to GDB's tagged
>>> pointer support due to the following:
>>> - LAM supports address tagging for data accesses only. Thus, specifying
>>> breakpoints on tagged addresses is not a valid use case.
>>> - In contrast to the implementation for ARM's TBI, the Linux kernel supports
>>> tagged pointers for memory access.
>>>
>>> This patch makes GDB's tagged pointer support configurable such that
>>> it is possible to enable the address adjustment for a specific feature
>>> only (e.g memory access, breakpoints or watchpoints). This way, one
>>> can make sure that addresses are only adjusted when necessary. In
>>> case of LAM, this avoids unnecessary parsing of the /proc/<pid>/status
>>> file to get the untag mask.
>>>
>>> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
>>> ---
>>> gdb/aarch64-linux-nat.c | 2 +-
>>> gdb/aarch64-linux-tdep.c | 7 +++--
>>> gdb/aarch64-tdep.c | 23 ++++++++------
>>> gdb/aarch64-tdep.h | 6 ++++
>>> gdb/breakpoint.c | 5 +--
>>> gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++-------
>>> gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
>>> gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
>>> gdb/loongarch-linux-nat.c | 2 +-
>>> gdb/target.c | 3 +-
>>> 10 files changed, 169 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index
>>> 0fa5bee500b..48ab765d880 100644
>>> --- a/gdb/aarch64-linux-nat.c
>>> +++ b/gdb/aarch64-linux-nat.c
>>> @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address
>> (CORE_ADDR *addr_p)
>>> kernel can potentially be tagged addresses. */
>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
>>> const CORE_ADDR addr_trap
>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
>> siginfo.si_addr);
>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>> + siginfo.si_addr);
>>>
>>> /* Check if the address matches any watched address. */
>>> state = aarch64_get_debug_reg_state (inferior_ptid.pid ()); diff
>>> --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index
>>> c608a84bc71..61c3be8b6f0 100644
>>> --- a/gdb/aarch64-linux-tdep.c
>>> +++ b/gdb/aarch64-linux-tdep.c
>>> @@ -2433,7 +2433,7 @@ static bool
>>> aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR
>>> address) {
>>> /* Remove the top byte for the memory range check. */
>>> - address = gdbarch_remove_non_address_bits (gdbarch, address);
>>> + address = aarch64_remove_non_address_bits (gdbarch, address);
>>>
>>> /* Check if the page that contains ADDRESS is mapped with PROT_MTE. */
>>> if (!linux_address_in_memtag_page (address)) @@ -2491,8 +2491,9 @@
>>> aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>>> uiout->text ("\n");
>>>
>>> std::optional<CORE_ADDR> atag
>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
>>> - fault_addr));
>>> + = aarch64_mte_get_atag (
>>> + aarch64_remove_non_address_bits (gdbarch, fault_addr));
>>> +
>>> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
>>>
>>> if (!atag.has_value ())
>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
>>> 8a2a9b1e23c..91fec7879ed 100644
>>> --- a/gdb/aarch64-tdep.c
>>> +++ b/gdb/aarch64-tdep.c
>>> @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch
>>> *gdbarch,
>>>
>>> /* Fetch the allocation tag for ADDRESS. */
>>> std::optional<CORE_ADDR> atag
>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
>> addr));
>>> + = aarch64_mte_get_atag (aarch64_remove_non_address_bits (gdbarch,
>>> + addr));
>>>
>>> if (!atag.has_value ())
>>> return true;
>>> @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch *gdbarch,
>> struct value *address,
>>> else
>>> {
>>> /* Remove the top byte. */
>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
>>>
>>> /* With G being the number of tag granules and N the number of tags
>>> passed in, we can have the following cases:
>>> @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch *gdbarch,
>> struct value *address,
>>> else
>>> {
>>> /* Remove the top byte. */
>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
>>> std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
>>>
>>> if (!atag.has_value ())
>>> @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
>> *gdbarch, struct value *tag_value)
>>> return string_printf ("0x%s", phex_nz (tag, sizeof (tag))); }
>>>
>>> -/* AArch64 implementation of the remove_non_address_bits gdbarch hook.
>> Remove
>>> - non address bits from a pointer value. */
>>> +/* See aarch64-tdep.h. */
>>>
>>> -static CORE_ADDR
>>> +CORE_ADDR
>>> aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
>>> pointer) {
>>> /* By default, we assume TBI and discard the top 8 bits plus the VA
>>> range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct gdbarch_info
>> info, struct gdbarch_list *arches)
>>> tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
>>>
>>> /* Architecture hook to remove bits of a pointer that are not part of the
>>> - address, like memory tags (MTE) and pointer authentication signatures. */
>>> - set_gdbarch_remove_non_address_bits (gdbarch,
>>> - aarch64_remove_non_address_bits);
>>> + address, like memory tags (MTE) and pointer authentication signatures.
>>> + Configure address adjustment for watch-, breakpoints and memory
>>
>> s/watch-/watchpoints
>
> Fill fix, thanks.
>
>>> + transfer. */
>>> + set_gdbarch_remove_non_address_bits_watchpoint
>>> + (gdbarch, aarch64_remove_non_address_bits);
>>> + set_gdbarch_remove_non_address_bits_breakpoint
>>> + (gdbarch, aarch64_remove_non_address_bits);
>>> + set_gdbarch_remove_non_address_bits_memory
>>> + (gdbarch, aarch64_remove_non_address_bits);
>>>
>>> /* SME pseudo-registers. */
>>> if (tdep->has_sme ())
>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
>>> 50166fb4f24..13ea37de7a3 100644
>>> --- a/gdb/aarch64-tdep.h
>>> +++ b/gdb/aarch64-tdep.h
>>> @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep (struct
>>> gdbarch *gdbarch);
>>>
>>> std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR address);
>>>
>>> +/* AArch64 implementation of the remove_non_address_bits gdbarch hooks.
>>> + Remove non address bits from a pointer value. */
>>> +
>>> +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
>>> + CORE_ADDR pointer);
>>> +
>>> #endif /* aarch64-tdep.h */
>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
>>> b7e4f5d0a45..9983e905e1b 100644
>>> --- a/gdb/breakpoint.c
>>> +++ b/gdb/breakpoint.c
>>> @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b, bool
>> reparse)
>>> loc->gdbarch = v->type ()->arch ();
>>> loc->pspace = wp_pspace;
>>> loc->address
>>> - = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
>>> + = gdbarch_remove_non_address_bits_watchpoint (loc-
>>> gdbarch,
>>> + addr);
>>> b->add_location (*loc);
>>>
>>> if (bitsize != 0)
>>> @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
>> *gdbarch,
>>> }
>>>
>>> adjusted_bpaddr
>>> - = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
>>> + = gdbarch_remove_non_address_bits_breakpoint (gdbarch,
>>> +adjusted_bpaddr);
>>>
>>> /* An adjusted breakpoint address can significantly alter
>>> a user's expectations. Print a warning if an adjustment diff --git
>>> a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index 0d00cd7c993..d05c7a3cbdf
>>> 100644
>>> --- a/gdb/gdbarch-gen.c
>>> +++ b/gdb/gdbarch-gen.c
>>> @@ -143,7 +143,9 @@ struct gdbarch
>>> int frame_red_zone_size = 0;
>>> gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr =
>> convert_from_func_ptr_addr_identity;
>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove =
>>> core_addr_identity;
>>> - gdbarch_remove_non_address_bits_ftype *remove_non_address_bits =
>>> default_remove_non_address_bits;
>>> + gdbarch_remove_non_address_bits_watchpoint_ftype
>>> + *remove_non_address_bits_watchpoint =
>>> + default_remove_non_address_bits;
>>> + gdbarch_remove_non_address_bits_breakpoint_ftype
>>> + *remove_non_address_bits_breakpoint =
>>> + default_remove_non_address_bits;
>>> + gdbarch_remove_non_address_bits_memory_ftype
>>> + *remove_non_address_bits_memory = default_remove_non_address_bits;
>>> gdbarch_memtag_to_string_ftype *memtag_to_string =
>> default_memtag_to_string;
>>> gdbarch_tagged_address_p_ftype *tagged_address_p =
>> default_tagged_address_p;
>>> gdbarch_memtag_matches_p_ftype *memtag_matches_p =
>>> default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch (struct
>> gdbarch *gdbarch)
>>> /* Skip verify of frame_red_zone_size, invalid_p == 0. */
>>> /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
>>> /* Skip verify of addr_bits_remove, invalid_p == 0. */
>>> - /* Skip verify of remove_non_address_bits, invalid_p == 0. */
>>> + /* Skip verify of remove_non_address_bits_watchpoint, invalid_p ==
>>> + 0. */
>>> + /* Skip verify of remove_non_address_bits_breakpoint, invalid_p ==
>>> + 0. */
>>> + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
>>> + */
>>> /* Skip verify of memtag_to_string, invalid_p == 0. */
>>> /* Skip verify of tagged_address_p, invalid_p == 0. */
>>> /* Skip verify of memtag_matches_p, invalid_p == 0. */ @@ -910,8
>>> +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
>>> "gdbarch_dump: addr_bits_remove = <%s>\n",
>>> host_address_to_string (gdbarch->addr_bits_remove));
>>> gdb_printf (file,
>>> - "gdbarch_dump: remove_non_address_bits = <%s>\n",
>>> - host_address_to_string (gdbarch->remove_non_address_bits));
>>> + "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
>>> + host_address_to_string
>>> +(gdbarch->remove_non_address_bits_watchpoint));
>>> + gdb_printf (file,
>>> + "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
>>> + host_address_to_string
>>> +(gdbarch->remove_non_address_bits_breakpoint));
>>> + gdb_printf (file,
>>> + "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
>>> + host_address_to_string
>>> +(gdbarch->remove_non_address_bits_memory));
>>> gdb_printf (file,
>>> "gdbarch_dump: memtag_to_string = <%s>\n",
>>> host_address_to_string (gdbarch->memtag_to_string)); @@
>>> -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch
>>> *gdbarch, }
>>>
>>> CORE_ADDR
>>> -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
>>> pointer)
>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
>>> +CORE_ADDR pointer) {
>>> + gdb_assert (gdbarch != NULL);
>>> + gdb_assert (gdbarch->remove_non_address_bits_watchpoint != NULL);
>>> + if (gdbarch_debug >= 2)
>>> + gdb_printf (gdb_stdlog,
>>> +"gdbarch_remove_non_address_bits_watchpoint called\n");
>>> + return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
>>> +pointer); }
>>> +
>>> +void
>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>> *gdbarch,
>>> +
>> gdbarch_remove_non_address_bits_watchpoint_ftype
>>> +remove_non_address_bits_watchpoint)
>>> +{
>>> + gdbarch->remove_non_address_bits_watchpoint =
>>> +remove_non_address_bits_watchpoint;
>>> +}
>>> +
>>> +CORE_ADDR
>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
>>> +CORE_ADDR pointer) {
>>> + gdb_assert (gdbarch != NULL);
>>> + gdb_assert (gdbarch->remove_non_address_bits_breakpoint != NULL);
>>> + if (gdbarch_debug >= 2)
>>> + gdb_printf (gdb_stdlog,
>>> +"gdbarch_remove_non_address_bits_breakpoint called\n");
>>> + return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
>>> +pointer); }
>>> +
>>> +void
>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
>>> +
>> gdbarch_remove_non_address_bits_breakpoint_ftype
>>> +remove_non_address_bits_breakpoint)
>>> +{
>>> + gdbarch->remove_non_address_bits_breakpoint =
>>> +remove_non_address_bits_breakpoint;
>>> +}
>>> +
>>> +CORE_ADDR
>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>> +CORE_ADDR pointer)
>>> {
>>> gdb_assert (gdbarch != NULL);
>>> - gdb_assert (gdbarch->remove_non_address_bits != NULL);
>>> + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
>>> if (gdbarch_debug >= 2)
>>> - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
>>> - return gdbarch->remove_non_address_bits (gdbarch, pointer);
>>> + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_memory
>>> + called\n"); return gdbarch->remove_non_address_bits_memory
>>> + (gdbarch, pointer);
>>> }
>>>
>>> void
>>> -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
>>> - gdbarch_remove_non_address_bits_ftype
>> remove_non_address_bits)
>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>> +
>> gdbarch_remove_non_address_bits_memory_ftype
>>> +remove_non_address_bits_memory)
>>> {
>>> - gdbarch->remove_non_address_bits = remove_non_address_bits;
>>> + gdbarch->remove_non_address_bits_memory =
>>> + remove_non_address_bits_memory;
>>> }
>>>
>>> std::string
>>> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
>>> b982fd7cd09..9fda85f860f 100644
>>> --- a/gdb/gdbarch-gen.h
>>> +++ b/gdb/gdbarch-gen.h
>>> @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove
>>> (struct gdbarch *gdbarch, CORE_ADDR ad extern void
>>> set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove);
>>>
>>> /* On some architectures, not all bits of a pointer are significant.
>>> - On AArch64, for example, the top bits of a pointer may carry a "tag", which
>>> - can be ignored by the kernel and the hardware. The "tag" can be regarded as
>>> - additional data associated with the pointer, but it is not part of the address.
>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
>>> + "tag", which can be ignored by the kernel and the hardware. The "tag" can
>> be
>>> + regarded as additional data associated with the pointer, but it is not part
>>> + of the address.
>>>
>>> Given a pointer for the architecture, this hook removes all the
>>> - non-significant bits and sign-extends things as needed. It gets used to
>> remove
>>> - non-address bits from data pointers (for example, removing the AArch64 MTE
>> tag
>>> - bits from a pointer) and from code pointers (removing the AArch64 PAC
>> signature
>>> - from a pointer containing the return address). */
>>> -
>>> -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct
>>> gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
>>> gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
>>> pointer); -extern void set_gdbarch_remove_non_address_bits (struct
>>> gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype
>>> *remove_non_address_bits);
>>> + non-significant bits and sign-extends things as needed. It gets used to
>>> + remove non-address bits from pointers used for watchpoints. */
>>> +
>>> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_watchpoint_ftype)
>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
>>> +CORE_ADDR pointer); extern void
>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>> +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
>>> +*remove_non_address_bits_watchpoint);
>>> +
>>> +/* On some architectures, not all bits of a pointer are significant.
>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
>>> + "tag", which can be ignored by the kernel and the hardware. The "tag" can
>> be
>>> + regarded as additional data associated with the pointer, but it is not part
>>> + of the address.
>>> +
>>> + Given a pointer for the architecture, this hook removes all the
>>> + non-significant bits and sign-extends things as needed. It gets used to
>>> + remove non-address bits from pointers used for breakpoints. */
>>> +
>>> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_breakpoint_ftype)
>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
>>> +CORE_ADDR pointer); extern void
>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>> +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
>>> +*remove_non_address_bits_breakpoint);
>>> +
>>> +/* On some architectures, not all bits of a pointer are significant.
>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
>>> + "tag", which can be ignored by the kernel and the hardware. The "tag" can
>> be
>>> + regarded as additional data associated with the pointer, but it is not part
>>> + of the address.
>>> +
>>> + Given a pointer for the architecture, this hook removes all the
>>> + non-significant bits and sign-extends things as needed. It gets used to
>>> + remove non-address bits from any pointer used to access memory. */
>>> +
>>> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_memory_ftype)
>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>> +CORE_ADDR pointer); extern void
>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>> +gdbarch_remove_non_address_bits_memory_ftype
>>> +*remove_non_address_bits_memory);
>>>
>>> /* Return a string representation of the memory tag TAG. */
>>>
>>> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
>>> index 4006380076d..cc7c6d8677b 100644
>>> --- a/gdb/gdbarch_components.py
>>> +++ b/gdb/gdbarch_components.py
>>> @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
>> instead).
>>> Method(
>>> comment="""
>>> On some architectures, not all bits of a pointer are significant.
>>> -On AArch64, for example, the top bits of a pointer may carry a "tag",
>>> which -can be ignored by the kernel and the hardware. The "tag" can
>>> be regarded as -additional data associated with the pointer, but it is not part of
>> the address.
>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>> +The "tag" can be regarded as additional data associated with the
>>> +pointer, but it is not part of the address.
>>>
>>> Given a pointer for the architecture, this hook removes all the
>>> -non-significant bits and sign-extends things as needed. It gets used
>>> to remove -non-address bits from data pointers (for example, removing
>>> the AArch64 MTE tag -bits from a pointer) and from code pointers
>>> (removing the AArch64 PAC signature -from a pointer containing the return
>> address).
>>> +non-significant bits and sign-extends things as needed. It gets used
>>> +to remove non-address bits from pointers used for watchpoints.
>>> """,
>>> type="CORE_ADDR",
>>> - name="remove_non_address_bits",
>>> + name="remove_non_address_bits_watchpoint",
>>> + params=[("CORE_ADDR", "pointer")],
>>> + predefault="default_remove_non_address_bits",
>>> + invalid=False,
>>> +)
>>> +
>>> +Method(
>>> + comment="""
>>> +On some architectures, not all bits of a pointer are significant.
>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>> +The "tag" can be regarded as additional data associated with the
>>> +pointer, but it is not part of the address.
>>> +
>>> +Given a pointer for the architecture, this hook removes all the
>>> +non-significant bits and sign-extends things as needed. It gets used
>>> +to remove non-address bits from pointers used for breakpoints.
>>> +""",
>>> + type="CORE_ADDR",
>>> + name="remove_non_address_bits_breakpoint",
>>> + params=[("CORE_ADDR", "pointer")],
>>> + predefault="default_remove_non_address_bits",
>>> + invalid=False,
>>> +)
>>> +
>>> +Method(
>>> + comment="""
>>> +On some architectures, not all bits of a pointer are significant.
>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>> +The "tag" can be regarded as additional data associated with the
>>> +pointer, but it is not part of the address.
>>> +
>>> +Given a pointer for the architecture, this hook removes all the
>>> +non-significant bits and sign-extends things as needed. It gets used
>>> +to remove non-address bits from any pointer used to access memory.
>>> +""",
>>> + type="CORE_ADDR",
>>> + name="remove_non_address_bits_memory",
>>> params=[("CORE_ADDR", "pointer")],
>>> predefault="default_remove_non_address_bits",
>>> invalid=False,
>>> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
>>> index bc9927dd751..dc4a019614a 100644
>>> --- a/gdb/loongarch-linux-nat.c
>>> +++ b/gdb/loongarch-linux-nat.c
>>> @@ -613,7 +613,7 @@ loongarch_linux_nat_target::stopped_data_address
>> (CORE_ADDR *addr_p)
>>> kernel can potentially be tagged addresses. */
>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
>>> const CORE_ADDR addr_trap
>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
>> siginfo.si_addr);
>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>> + siginfo.si_addr);
>>>
>>> /* Check if the address matches any watched address. */
>>> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
>>
>> I think the loongarch-linux-nat.c change is spurious. Could you please address
>> that?
>
> Why is it spurious? What do you mean exactly?
The intention behind this change isn't clear to me, but the Loongson native Linux layer
using a AArch64 architecture function does not make sense.
I haven't built a LoongArch native Linux target, but I'd guess that it will run into a build
error with this change, if built for LoongArch only.
The right approach, in my mind, is to drop this change, but you have to touch this code due to your
changes. It begs the question, why is the LoongArch native code calling
gdbarch_remove_non_address_bits here without even registering a function for the gdbarch hook to call?
Without this hook, we'll invoke the dummy implementation in
gdb/arch-utils.c:default_remove_non_address_bits, and that just returns the unmodified pointer.
This is a question for the LoongArch maintainer (cc-ed). If we don't hear back, we should probably
change the LoongArch code to not call gdbarch_remove_non_address_bits. I think that was the original
intention, but there might've been a copy/paste error somewhere.
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-11-05 14:22 ` Luis Machado
@ 2024-11-05 14:40 ` Schimpe, Christina
2024-11-05 14:51 ` Luis Machado
0 siblings, 1 reply; 12+ messages in thread
From: Schimpe, Christina @ 2024-11-05 14:40 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: Tiezhu Yang
> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Tuesday, November 5, 2024 3:22 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>
> On 11/5/24 14:07, Schimpe, Christina wrote:
> > Hi Luis,
> >
> > Thanks a lot for the review. I have one questions to your comment, please see
> below.
> >
> >> -----Original Message-----
> >> From: Luis Machado <luis.machado@arm.com>
> >> Sent: Monday, November 4, 2024 12:18 PM
> >> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >> patches@sourceware.org
> >> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
> >>
> >> Hi Christina,
> >>
> >> On 10/28/24 08:46, Schimpe, Christina wrote:
> >>> From: Christina Schimpe <christina.schimpe@intel.com>
> >>>
> >>> The gdbarch function gdbarch_remove_non_address_bits adjusts
> >>> addresses to enable debugging of programs with tagged pointers on
> >>> Linux, for instance for ARM's feature top byte ignore (TBI).
> >>> Once the function is implemented for an architecture, it adjusts
> >>> addresses for memory access, breakpoints and watchpoints.
> >>>
> >>> Linear address masking (LAM) is Intel's (R) implementation of tagged
> >>> pointer support. It requires certain adaptions to GDB's tagged
> >>> pointer support due to the following:
> >>> - LAM supports address tagging for data accesses only. Thus, specifying
> >>> breakpoints on tagged addresses is not a valid use case.
> >>> - In contrast to the implementation for ARM's TBI, the Linux kernel supports
> >>> tagged pointers for memory access.
> >>>
> >>> This patch makes GDB's tagged pointer support configurable such that
> >>> it is possible to enable the address adjustment for a specific
> >>> feature only (e.g memory access, breakpoints or watchpoints). This
> >>> way, one can make sure that addresses are only adjusted when
> >>> necessary. In case of LAM, this avoids unnecessary parsing of the
> >>> /proc/<pid>/status file to get the untag mask.
> >>>
> >>> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
> >>> ---
> >>> gdb/aarch64-linux-nat.c | 2 +-
> >>> gdb/aarch64-linux-tdep.c | 7 +++--
> >>> gdb/aarch64-tdep.c | 23 ++++++++------
> >>> gdb/aarch64-tdep.h | 6 ++++
> >>> gdb/breakpoint.c | 5 +--
> >>> gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++-------
> >>> gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
> >>> gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
> >>> gdb/loongarch-linux-nat.c | 2 +-
> >>> gdb/target.c | 3 +-
> >>> 10 files changed, 169 insertions(+), 47 deletions(-)
> >>>
> >>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index
> >>> 0fa5bee500b..48ab765d880 100644
> >>> --- a/gdb/aarch64-linux-nat.c
> >>> +++ b/gdb/aarch64-linux-nat.c
> >>> @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address
> >> (CORE_ADDR *addr_p)
> >>> kernel can potentially be tagged addresses. */
> >>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >>> const CORE_ADDR addr_trap
> >>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >> siginfo.si_addr);
> >>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>> + siginfo.si_addr);
> >>>
> >>> /* Check if the address matches any watched address. */
> >>> state = aarch64_get_debug_reg_state (inferior_ptid.pid ()); diff
> >>> --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index
> >>> c608a84bc71..61c3be8b6f0 100644
> >>> --- a/gdb/aarch64-linux-tdep.c
> >>> +++ b/gdb/aarch64-linux-tdep.c
> >>> @@ -2433,7 +2433,7 @@ static bool
> >>> aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR
> >>> address) {
> >>> /* Remove the top byte for the memory range check. */
> >>> - address = gdbarch_remove_non_address_bits (gdbarch, address);
> >>> + address = aarch64_remove_non_address_bits (gdbarch, address);
> >>>
> >>> /* Check if the page that contains ADDRESS is mapped with PROT_MTE. */
> >>> if (!linux_address_in_memtag_page (address)) @@ -2491,8 +2491,9
> >>> @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
> >>> uiout->text ("\n");
> >>>
> >>> std::optional<CORE_ADDR> atag
> >>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> >>> - fault_addr));
> >>> + = aarch64_mte_get_atag (
> >>> + aarch64_remove_non_address_bits (gdbarch, fault_addr));
> >>> +
> >>> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
> >>>
> >>> if (!atag.has_value ())
> >>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> >>> 8a2a9b1e23c..91fec7879ed 100644
> >>> --- a/gdb/aarch64-tdep.c
> >>> +++ b/gdb/aarch64-tdep.c
> >>> @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch
> >>> *gdbarch,
> >>>
> >>> /* Fetch the allocation tag for ADDRESS. */
> >>> std::optional<CORE_ADDR> atag
> >>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> >> addr));
> >>> + = aarch64_mte_get_atag (aarch64_remove_non_address_bits
> >>> + (gdbarch, addr));
> >>>
> >>> if (!atag.has_value ())
> >>> return true;
> >>> @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch *gdbarch,
> >> struct value *address,
> >>> else
> >>> {
> >>> /* Remove the top byte. */
> >>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> >>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >>>
> >>> /* With G being the number of tag granules and N the number of tags
> >>> passed in, we can have the following cases:
> >>> @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch *gdbarch,
> >> struct value *address,
> >>> else
> >>> {
> >>> /* Remove the top byte. */
> >>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> >>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >>> std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
> >>>
> >>> if (!atag.has_value ())
> >>> @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
> >> *gdbarch, struct value *tag_value)
> >>> return string_printf ("0x%s", phex_nz (tag, sizeof (tag))); }
> >>>
> >>> -/* AArch64 implementation of the remove_non_address_bits gdbarch hook.
> >> Remove
> >>> - non address bits from a pointer value. */
> >>> +/* See aarch64-tdep.h. */
> >>>
> >>> -static CORE_ADDR
> >>> +CORE_ADDR
> >>> aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> >>> pointer) {
> >>> /* By default, we assume TBI and discard the top 8 bits plus the
> >>> VA range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct
> >>> gdbarch_info
> >> info, struct gdbarch_list *arches)
> >>> tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
> >>>
> >>> /* Architecture hook to remove bits of a pointer that are not part of the
> >>> - address, like memory tags (MTE) and pointer authentication signatures.
> */
> >>> - set_gdbarch_remove_non_address_bits (gdbarch,
> >>> - aarch64_remove_non_address_bits);
> >>> + address, like memory tags (MTE) and pointer authentication signatures.
> >>> + Configure address adjustment for watch-, breakpoints and
> >>> + memory
> >>
> >> s/watch-/watchpoints
> >
> > Fill fix, thanks.
> >
> >>> + transfer. */
> >>> + set_gdbarch_remove_non_address_bits_watchpoint
> >>> + (gdbarch, aarch64_remove_non_address_bits);
> >>> + set_gdbarch_remove_non_address_bits_breakpoint
> >>> + (gdbarch, aarch64_remove_non_address_bits);
> >>> + set_gdbarch_remove_non_address_bits_memory
> >>> + (gdbarch, aarch64_remove_non_address_bits);
> >>>
> >>> /* SME pseudo-registers. */
> >>> if (tdep->has_sme ())
> >>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
> >>> 50166fb4f24..13ea37de7a3 100644
> >>> --- a/gdb/aarch64-tdep.h
> >>> +++ b/gdb/aarch64-tdep.h
> >>> @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep
> >>> (struct gdbarch *gdbarch);
> >>>
> >>> std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR address);
> >>>
> >>> +/* AArch64 implementation of the remove_non_address_bits gdbarch
> hooks.
> >>> + Remove non address bits from a pointer value. */
> >>> +
> >>> +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
> >>> + CORE_ADDR pointer);
> >>> +
> >>> #endif /* aarch64-tdep.h */
> >>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
> >>> b7e4f5d0a45..9983e905e1b 100644
> >>> --- a/gdb/breakpoint.c
> >>> +++ b/gdb/breakpoint.c
> >>> @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b, bool
> >> reparse)
> >>> loc->gdbarch = v->type ()->arch ();
> >>> loc->pspace = wp_pspace;
> >>> loc->address
> >>> - = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
> >>> + = gdbarch_remove_non_address_bits_watchpoint (loc-
> >>> gdbarch,
> >>> + addr);
> >>> b->add_location (*loc);
> >>>
> >>> if (bitsize != 0)
> >>> @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
> >> *gdbarch,
> >>> }
> >>>
> >>> adjusted_bpaddr
> >>> - = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
> >>> + = gdbarch_remove_non_address_bits_breakpoint (gdbarch,
> >>> +adjusted_bpaddr);
> >>>
> >>> /* An adjusted breakpoint address can significantly alter
> >>> a user's expectations. Print a warning if an adjustment diff
> >>> --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index
> >>> 0d00cd7c993..d05c7a3cbdf
> >>> 100644
> >>> --- a/gdb/gdbarch-gen.c
> >>> +++ b/gdb/gdbarch-gen.c
> >>> @@ -143,7 +143,9 @@ struct gdbarch
> >>> int frame_red_zone_size = 0;
> >>> gdbarch_convert_from_func_ptr_addr_ftype
> >>> *convert_from_func_ptr_addr =
> >> convert_from_func_ptr_addr_identity;
> >>> gdbarch_addr_bits_remove_ftype *addr_bits_remove =
> >>> core_addr_identity;
> >>> - gdbarch_remove_non_address_bits_ftype *remove_non_address_bits =
> >>> default_remove_non_address_bits;
> >>> + gdbarch_remove_non_address_bits_watchpoint_ftype
> >>> + *remove_non_address_bits_watchpoint =
> >>> + default_remove_non_address_bits;
> >>> + gdbarch_remove_non_address_bits_breakpoint_ftype
> >>> + *remove_non_address_bits_breakpoint =
> >>> + default_remove_non_address_bits;
> >>> + gdbarch_remove_non_address_bits_memory_ftype
> >>> + *remove_non_address_bits_memory = default_remove_non_address_bits;
> >>> gdbarch_memtag_to_string_ftype *memtag_to_string =
> >> default_memtag_to_string;
> >>> gdbarch_tagged_address_p_ftype *tagged_address_p =
> >> default_tagged_address_p;
> >>> gdbarch_memtag_matches_p_ftype *memtag_matches_p =
> >>> default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch (struct
> >> gdbarch *gdbarch)
> >>> /* Skip verify of frame_red_zone_size, invalid_p == 0. */
> >>> /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
> >>> /* Skip verify of addr_bits_remove, invalid_p == 0. */
> >>> - /* Skip verify of remove_non_address_bits, invalid_p == 0. */
> >>> + /* Skip verify of remove_non_address_bits_watchpoint, invalid_p
> >>> + == 0. */
> >>> + /* Skip verify of remove_non_address_bits_breakpoint, invalid_p
> >>> + == 0. */
> >>> + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
> >>> + */
> >>> /* Skip verify of memtag_to_string, invalid_p == 0. */
> >>> /* Skip verify of tagged_address_p, invalid_p == 0. */
> >>> /* Skip verify of memtag_matches_p, invalid_p == 0. */ @@ -910,8
> >>> +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file
> >>> +*file)
> >>> "gdbarch_dump: addr_bits_remove = <%s>\n",
> >>> host_address_to_string (gdbarch->addr_bits_remove));
> >>> gdb_printf (file,
> >>> - "gdbarch_dump: remove_non_address_bits = <%s>\n",
> >>> - host_address_to_string (gdbarch->remove_non_address_bits));
> >>> + "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
> >>> + host_address_to_string
> >>> +(gdbarch->remove_non_address_bits_watchpoint));
> >>> + gdb_printf (file,
> >>> + "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
> >>> + host_address_to_string
> >>> +(gdbarch->remove_non_address_bits_breakpoint));
> >>> + gdb_printf (file,
> >>> + "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
> >>> + host_address_to_string
> >>> +(gdbarch->remove_non_address_bits_memory));
> >>> gdb_printf (file,
> >>> "gdbarch_dump: memtag_to_string = <%s>\n",
> >>> host_address_to_string (gdbarch->memtag_to_string)); @@
> >>> -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch
> >>> *gdbarch, }
> >>>
> >>> CORE_ADDR
> >>> -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> >>> pointer)
> >>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>> +*gdbarch, CORE_ADDR pointer) {
> >>> + gdb_assert (gdbarch != NULL);
> >>> + gdb_assert (gdbarch->remove_non_address_bits_watchpoint != NULL);
> >>> + if (gdbarch_debug >= 2)
> >>> + gdb_printf (gdb_stdlog,
> >>> +"gdbarch_remove_non_address_bits_watchpoint called\n");
> >>> + return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
> >>> +pointer); }
> >>> +
> >>> +void
> >>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >> *gdbarch,
> >>> +
> >> gdbarch_remove_non_address_bits_watchpoint_ftype
> >>> +remove_non_address_bits_watchpoint)
> >>> +{
> >>> + gdbarch->remove_non_address_bits_watchpoint =
> >>> +remove_non_address_bits_watchpoint;
> >>> +}
> >>> +
> >>> +CORE_ADDR
> >>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>> +*gdbarch, CORE_ADDR pointer) {
> >>> + gdb_assert (gdbarch != NULL);
> >>> + gdb_assert (gdbarch->remove_non_address_bits_breakpoint != NULL);
> >>> + if (gdbarch_debug >= 2)
> >>> + gdb_printf (gdb_stdlog,
> >>> +"gdbarch_remove_non_address_bits_breakpoint called\n");
> >>> + return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
> >>> +pointer); }
> >>> +
> >>> +void
> >>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>> +*gdbarch,
> >>> +
> >> gdbarch_remove_non_address_bits_breakpoint_ftype
> >>> +remove_non_address_bits_breakpoint)
> >>> +{
> >>> + gdbarch->remove_non_address_bits_breakpoint =
> >>> +remove_non_address_bits_breakpoint;
> >>> +}
> >>> +
> >>> +CORE_ADDR
> >>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> >>> +CORE_ADDR pointer)
> >>> {
> >>> gdb_assert (gdbarch != NULL);
> >>> - gdb_assert (gdbarch->remove_non_address_bits != NULL);
> >>> + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
> >>> if (gdbarch_debug >= 2)
> >>> - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
> >>> - return gdbarch->remove_non_address_bits (gdbarch, pointer);
> >>> + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_memory
> >>> + called\n"); return gdbarch->remove_non_address_bits_memory
> >>> + (gdbarch, pointer);
> >>> }
> >>>
> >>> void
> >>> -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>> - gdbarch_remove_non_address_bits_ftype
> >> remove_non_address_bits)
> >>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>> +*gdbarch,
> >>> +
> >> gdbarch_remove_non_address_bits_memory_ftype
> >>> +remove_non_address_bits_memory)
> >>> {
> >>> - gdbarch->remove_non_address_bits = remove_non_address_bits;
> >>> + gdbarch->remove_non_address_bits_memory =
> >>> + remove_non_address_bits_memory;
> >>> }
> >>>
> >>> std::string
> >>> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
> >>> b982fd7cd09..9fda85f860f 100644
> >>> --- a/gdb/gdbarch-gen.h
> >>> +++ b/gdb/gdbarch-gen.h
> >>> @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove
> >>> (struct gdbarch *gdbarch, CORE_ADDR ad extern void
> >>> set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
> >>> gdbarch_addr_bits_remove_ftype *addr_bits_remove);
> >>>
> >>> /* On some architectures, not all bits of a pointer are significant.
> >>> - On AArch64, for example, the top bits of a pointer may carry a "tag", which
> >>> - can be ignored by the kernel and the hardware. The "tag" can be regarded
> as
> >>> - additional data associated with the pointer, but it is not part of the
> address.
> >>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> >>> + "tag", which can be ignored by the kernel and the hardware. The
> >>> + "tag" can
> >> be
> >>> + regarded as additional data associated with the pointer, but it is not part
> >>> + of the address.
> >>>
> >>> Given a pointer for the architecture, this hook removes all the
> >>> - non-significant bits and sign-extends things as needed. It gets used to
> >> remove
> >>> - non-address bits from data pointers (for example, removing the AArch64
> MTE
> >> tag
> >>> - bits from a pointer) and from code pointers (removing the AArch64 PAC
> >> signature
> >>> - from a pointer containing the return address). */
> >>> -
> >>> -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct
> >>> gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
> >>> gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> >>> pointer); -extern void set_gdbarch_remove_non_address_bits (struct
> >>> gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype
> >>> *remove_non_address_bits);
> >>> + non-significant bits and sign-extends things as needed. It gets used to
> >>> + remove non-address bits from pointers used for watchpoints. */
> >>> +
> >>> +typedef CORE_ADDR
> >>> +(gdbarch_remove_non_address_bits_watchpoint_ftype)
> >>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>> +*gdbarch, CORE_ADDR pointer); extern void
> >>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>> +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
> >>> +*remove_non_address_bits_watchpoint);
> >>> +
> >>> +/* On some architectures, not all bits of a pointer are significant.
> >>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> >>> + "tag", which can be ignored by the kernel and the hardware. The
> >>> +"tag" can
> >> be
> >>> + regarded as additional data associated with the pointer, but it is not part
> >>> + of the address.
> >>> +
> >>> + Given a pointer for the architecture, this hook removes all the
> >>> + non-significant bits and sign-extends things as needed. It gets used to
> >>> + remove non-address bits from pointers used for breakpoints. */
> >>> +
> >>> +typedef CORE_ADDR
> >>> +(gdbarch_remove_non_address_bits_breakpoint_ftype)
> >>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>> +*gdbarch, CORE_ADDR pointer); extern void
> >>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>> +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
> >>> +*remove_non_address_bits_breakpoint);
> >>> +
> >>> +/* On some architectures, not all bits of a pointer are significant.
> >>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
> >>> + "tag", which can be ignored by the kernel and the hardware. The
> >>> +"tag" can
> >> be
> >>> + regarded as additional data associated with the pointer, but it is not part
> >>> + of the address.
> >>> +
> >>> + Given a pointer for the architecture, this hook removes all the
> >>> + non-significant bits and sign-extends things as needed. It gets used to
> >>> + remove non-address bits from any pointer used to access memory.
> >>> + */
> >>> +
> >>> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_memory_ftype)
> >>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> >>> +CORE_ADDR pointer); extern void
> >>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>> +*gdbarch, gdbarch_remove_non_address_bits_memory_ftype
> >>> +*remove_non_address_bits_memory);
> >>>
> >>> /* Return a string representation of the memory tag TAG. */
> >>>
> >>> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> >>> index 4006380076d..cc7c6d8677b 100644
> >>> --- a/gdb/gdbarch_components.py
> >>> +++ b/gdb/gdbarch_components.py
> >>> @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
> >> instead).
> >>> Method(
> >>> comment="""
> >>> On some architectures, not all bits of a pointer are significant.
> >>> -On AArch64, for example, the top bits of a pointer may carry a
> >>> "tag", which -can be ignored by the kernel and the hardware. The
> >>> "tag" can be regarded as -additional data associated with the
> >>> pointer, but it is not part of
> >> the address.
> >>> +On AArch64 and amd64, for example, the top bits of a pointer may
> >>> +carry a "tag", which can be ignored by the kernel and the hardware.
> >>> +The "tag" can be regarded as additional data associated with the
> >>> +pointer, but it is not part of the address.
> >>>
> >>> Given a pointer for the architecture, this hook removes all the
> >>> -non-significant bits and sign-extends things as needed. It gets
> >>> used to remove -non-address bits from data pointers (for example,
> >>> removing the AArch64 MTE tag -bits from a pointer) and from code
> >>> pointers (removing the AArch64 PAC signature -from a pointer
> >>> containing the return
> >> address).
> >>> +non-significant bits and sign-extends things as needed. It gets
> >>> +used to remove non-address bits from pointers used for watchpoints.
> >>> """,
> >>> type="CORE_ADDR",
> >>> - name="remove_non_address_bits",
> >>> + name="remove_non_address_bits_watchpoint",
> >>> + params=[("CORE_ADDR", "pointer")],
> >>> + predefault="default_remove_non_address_bits",
> >>> + invalid=False,
> >>> +)
> >>> +
> >>> +Method(
> >>> + comment="""
> >>> +On some architectures, not all bits of a pointer are significant.
> >>> +On AArch64 and amd64, for example, the top bits of a pointer may
> >>> +carry a "tag", which can be ignored by the kernel and the hardware.
> >>> +The "tag" can be regarded as additional data associated with the
> >>> +pointer, but it is not part of the address.
> >>> +
> >>> +Given a pointer for the architecture, this hook removes all the
> >>> +non-significant bits and sign-extends things as needed. It gets
> >>> +used to remove non-address bits from pointers used for breakpoints.
> >>> +""",
> >>> + type="CORE_ADDR",
> >>> + name="remove_non_address_bits_breakpoint",
> >>> + params=[("CORE_ADDR", "pointer")],
> >>> + predefault="default_remove_non_address_bits",
> >>> + invalid=False,
> >>> +)
> >>> +
> >>> +Method(
> >>> + comment="""
> >>> +On some architectures, not all bits of a pointer are significant.
> >>> +On AArch64 and amd64, for example, the top bits of a pointer may
> >>> +carry a "tag", which can be ignored by the kernel and the hardware.
> >>> +The "tag" can be regarded as additional data associated with the
> >>> +pointer, but it is not part of the address.
> >>> +
> >>> +Given a pointer for the architecture, this hook removes all the
> >>> +non-significant bits and sign-extends things as needed. It gets
> >>> +used to remove non-address bits from any pointer used to access memory.
> >>> +""",
> >>> + type="CORE_ADDR",
> >>> + name="remove_non_address_bits_memory",
> >>> params=[("CORE_ADDR", "pointer")],
> >>> predefault="default_remove_non_address_bits",
> >>> invalid=False,
> >>> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> >>> index bc9927dd751..dc4a019614a 100644
> >>> --- a/gdb/loongarch-linux-nat.c
> >>> +++ b/gdb/loongarch-linux-nat.c
> >>> @@ -613,7 +613,7 @@ loongarch_linux_nat_target::stopped_data_address
> >> (CORE_ADDR *addr_p)
> >>> kernel can potentially be tagged addresses. */
> >>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >>> const CORE_ADDR addr_trap
> >>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >> siginfo.si_addr);
> >>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>> + siginfo.si_addr);
> >>>
> >>> /* Check if the address matches any watched address. */
> >>> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
> >>
> >> I think the loongarch-linux-nat.c change is spurious. Could you
> >> please address that?
> >
> > Why is it spurious? What do you mean exactly?
>
> The intention behind this change isn't clear to me, but the Loongson native Linux
> layer using a AArch64 architecture function does not make sense.
>
> I haven't built a LoongArch native Linux target, but I'd guess that it will run into a
> build error with this change, if built for LoongArch only.
>
> The right approach, in my mind, is to drop this change, but you have to touch this
> code due to your changes. It begs the question, why is the LoongArch native code
> calling gdbarch_remove_non_address_bits here without even registering a
> function for the gdbarch hook to call?
Right, I agree.
> Without this hook, we'll invoke the dummy implementation in gdb/arch-
> utils.c:default_remove_non_address_bits, and that just returns the unmodified
> pointer.
True, that's probably why nobody noticed before.
> This is a question for the LoongArch maintainer (cc-ed). If we don't hear back, we
> should probably change the LoongArch code to not call
> gdbarch_remove_non_address_bits. I think that was the original intention, but
> there might've been a copy/paste error somewhere.
Ok. Let's wait a bit. Thanks for the detailed feedback.
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-11-05 14:40 ` Schimpe, Christina
@ 2024-11-05 14:51 ` Luis Machado
2024-11-05 15:09 ` Schimpe, Christina
0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2024-11-05 14:51 UTC (permalink / raw)
To: Schimpe, Christina, gdb-patches; +Cc: Tiezhu Yang
On 11/5/24 14:40, Schimpe, Christina wrote:
>> -----Original Message-----
>> From: Luis Machado <luis.machado@arm.com>
>> Sent: Tuesday, November 5, 2024 3:22 PM
>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>> patches@sourceware.org
>> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>>
>> On 11/5/24 14:07, Schimpe, Christina wrote:
>>> Hi Luis,
>>>
>>> Thanks a lot for the review. I have one questions to your comment, please see
>> below.
>>>
>>>> -----Original Message-----
>>>> From: Luis Machado <luis.machado@arm.com>
>>>> Sent: Monday, November 4, 2024 12:18 PM
>>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>>>> patches@sourceware.org
>>>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>>>>
>>>> Hi Christina,
>>>>
>>>> On 10/28/24 08:46, Schimpe, Christina wrote:
>>>>> From: Christina Schimpe <christina.schimpe@intel.com>
>>>>>
>>>>> The gdbarch function gdbarch_remove_non_address_bits adjusts
>>>>> addresses to enable debugging of programs with tagged pointers on
>>>>> Linux, for instance for ARM's feature top byte ignore (TBI).
>>>>> Once the function is implemented for an architecture, it adjusts
>>>>> addresses for memory access, breakpoints and watchpoints.
>>>>>
>>>>> Linear address masking (LAM) is Intel's (R) implementation of tagged
>>>>> pointer support. It requires certain adaptions to GDB's tagged
>>>>> pointer support due to the following:
>>>>> - LAM supports address tagging for data accesses only. Thus, specifying
>>>>> breakpoints on tagged addresses is not a valid use case.
>>>>> - In contrast to the implementation for ARM's TBI, the Linux kernel supports
>>>>> tagged pointers for memory access.
>>>>>
>>>>> This patch makes GDB's tagged pointer support configurable such that
>>>>> it is possible to enable the address adjustment for a specific
>>>>> feature only (e.g memory access, breakpoints or watchpoints). This
>>>>> way, one can make sure that addresses are only adjusted when
>>>>> necessary. In case of LAM, this avoids unnecessary parsing of the
>>>>> /proc/<pid>/status file to get the untag mask.
>>>>>
>>>>> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
>>>>> ---
>>>>> gdb/aarch64-linux-nat.c | 2 +-
>>>>> gdb/aarch64-linux-tdep.c | 7 +++--
>>>>> gdb/aarch64-tdep.c | 23 ++++++++------
>>>>> gdb/aarch64-tdep.h | 6 ++++
>>>>> gdb/breakpoint.c | 5 +--
>>>>> gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++-------
>>>>> gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
>>>>> gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
>>>>> gdb/loongarch-linux-nat.c | 2 +-
>>>>> gdb/target.c | 3 +-
>>>>> 10 files changed, 169 insertions(+), 47 deletions(-)
>>>>>
>>>>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index
>>>>> 0fa5bee500b..48ab765d880 100644
>>>>> --- a/gdb/aarch64-linux-nat.c
>>>>> +++ b/gdb/aarch64-linux-nat.c
>>>>> @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address
>>>> (CORE_ADDR *addr_p)
>>>>> kernel can potentially be tagged addresses. */
>>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
>>>>> const CORE_ADDR addr_trap
>>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>> siginfo.si_addr);
>>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>>> + siginfo.si_addr);
>>>>>
>>>>> /* Check if the address matches any watched address. */
>>>>> state = aarch64_get_debug_reg_state (inferior_ptid.pid ()); diff
>>>>> --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index
>>>>> c608a84bc71..61c3be8b6f0 100644
>>>>> --- a/gdb/aarch64-linux-tdep.c
>>>>> +++ b/gdb/aarch64-linux-tdep.c
>>>>> @@ -2433,7 +2433,7 @@ static bool
>>>>> aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR
>>>>> address) {
>>>>> /* Remove the top byte for the memory range check. */
>>>>> - address = gdbarch_remove_non_address_bits (gdbarch, address);
>>>>> + address = aarch64_remove_non_address_bits (gdbarch, address);
>>>>>
>>>>> /* Check if the page that contains ADDRESS is mapped with PROT_MTE. */
>>>>> if (!linux_address_in_memtag_page (address)) @@ -2491,8 +2491,9
>>>>> @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>>>>> uiout->text ("\n");
>>>>>
>>>>> std::optional<CORE_ADDR> atag
>>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
>>>>> - fault_addr));
>>>>> + = aarch64_mte_get_atag (
>>>>> + aarch64_remove_non_address_bits (gdbarch, fault_addr));
>>>>> +
>>>>> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
>>>>>
>>>>> if (!atag.has_value ())
>>>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
>>>>> 8a2a9b1e23c..91fec7879ed 100644
>>>>> --- a/gdb/aarch64-tdep.c
>>>>> +++ b/gdb/aarch64-tdep.c
>>>>> @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch
>>>>> *gdbarch,
>>>>>
>>>>> /* Fetch the allocation tag for ADDRESS. */
>>>>> std::optional<CORE_ADDR> atag
>>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
>>>> addr));
>>>>> + = aarch64_mte_get_atag (aarch64_remove_non_address_bits
>>>>> + (gdbarch, addr));
>>>>>
>>>>> if (!atag.has_value ())
>>>>> return true;
>>>>> @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch *gdbarch,
>>>> struct value *address,
>>>>> else
>>>>> {
>>>>> /* Remove the top byte. */
>>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
>>>>>
>>>>> /* With G being the number of tag granules and N the number of tags
>>>>> passed in, we can have the following cases:
>>>>> @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch *gdbarch,
>>>> struct value *address,
>>>>> else
>>>>> {
>>>>> /* Remove the top byte. */
>>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
>>>>> std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
>>>>>
>>>>> if (!atag.has_value ())
>>>>> @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
>>>> *gdbarch, struct value *tag_value)
>>>>> return string_printf ("0x%s", phex_nz (tag, sizeof (tag))); }
>>>>>
>>>>> -/* AArch64 implementation of the remove_non_address_bits gdbarch hook.
>>>> Remove
>>>>> - non address bits from a pointer value. */
>>>>> +/* See aarch64-tdep.h. */
>>>>>
>>>>> -static CORE_ADDR
>>>>> +CORE_ADDR
>>>>> aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
>>>>> pointer) {
>>>>> /* By default, we assume TBI and discard the top 8 bits plus the
>>>>> VA range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct
>>>>> gdbarch_info
>>>> info, struct gdbarch_list *arches)
>>>>> tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
>>>>>
>>>>> /* Architecture hook to remove bits of a pointer that are not part of the
>>>>> - address, like memory tags (MTE) and pointer authentication signatures.
>> */
>>>>> - set_gdbarch_remove_non_address_bits (gdbarch,
>>>>> - aarch64_remove_non_address_bits);
>>>>> + address, like memory tags (MTE) and pointer authentication signatures.
>>>>> + Configure address adjustment for watch-, breakpoints and
>>>>> + memory
>>>>
>>>> s/watch-/watchpoints
>>>
>>> Fill fix, thanks.
>>>
>>>>> + transfer. */
>>>>> + set_gdbarch_remove_non_address_bits_watchpoint
>>>>> + (gdbarch, aarch64_remove_non_address_bits);
>>>>> + set_gdbarch_remove_non_address_bits_breakpoint
>>>>> + (gdbarch, aarch64_remove_non_address_bits);
>>>>> + set_gdbarch_remove_non_address_bits_memory
>>>>> + (gdbarch, aarch64_remove_non_address_bits);
>>>>>
>>>>> /* SME pseudo-registers. */
>>>>> if (tdep->has_sme ())
>>>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
>>>>> 50166fb4f24..13ea37de7a3 100644
>>>>> --- a/gdb/aarch64-tdep.h
>>>>> +++ b/gdb/aarch64-tdep.h
>>>>> @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep
>>>>> (struct gdbarch *gdbarch);
>>>>>
>>>>> std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR address);
>>>>>
>>>>> +/* AArch64 implementation of the remove_non_address_bits gdbarch
>> hooks.
>>>>> + Remove non address bits from a pointer value. */
>>>>> +
>>>>> +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
>>>>> + CORE_ADDR pointer);
>>>>> +
>>>>> #endif /* aarch64-tdep.h */
>>>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
>>>>> b7e4f5d0a45..9983e905e1b 100644
>>>>> --- a/gdb/breakpoint.c
>>>>> +++ b/gdb/breakpoint.c
>>>>> @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b, bool
>>>> reparse)
>>>>> loc->gdbarch = v->type ()->arch ();
>>>>> loc->pspace = wp_pspace;
>>>>> loc->address
>>>>> - = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
>>>>> + = gdbarch_remove_non_address_bits_watchpoint (loc-
>>>>> gdbarch,
>>>>> + addr);
>>>>> b->add_location (*loc);
>>>>>
>>>>> if (bitsize != 0)
>>>>> @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
>>>> *gdbarch,
>>>>> }
>>>>>
>>>>> adjusted_bpaddr
>>>>> - = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
>>>>> + = gdbarch_remove_non_address_bits_breakpoint (gdbarch,
>>>>> +adjusted_bpaddr);
>>>>>
>>>>> /* An adjusted breakpoint address can significantly alter
>>>>> a user's expectations. Print a warning if an adjustment diff
>>>>> --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index
>>>>> 0d00cd7c993..d05c7a3cbdf
>>>>> 100644
>>>>> --- a/gdb/gdbarch-gen.c
>>>>> +++ b/gdb/gdbarch-gen.c
>>>>> @@ -143,7 +143,9 @@ struct gdbarch
>>>>> int frame_red_zone_size = 0;
>>>>> gdbarch_convert_from_func_ptr_addr_ftype
>>>>> *convert_from_func_ptr_addr =
>>>> convert_from_func_ptr_addr_identity;
>>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove =
>>>>> core_addr_identity;
>>>>> - gdbarch_remove_non_address_bits_ftype *remove_non_address_bits =
>>>>> default_remove_non_address_bits;
>>>>> + gdbarch_remove_non_address_bits_watchpoint_ftype
>>>>> + *remove_non_address_bits_watchpoint =
>>>>> + default_remove_non_address_bits;
>>>>> + gdbarch_remove_non_address_bits_breakpoint_ftype
>>>>> + *remove_non_address_bits_breakpoint =
>>>>> + default_remove_non_address_bits;
>>>>> + gdbarch_remove_non_address_bits_memory_ftype
>>>>> + *remove_non_address_bits_memory = default_remove_non_address_bits;
>>>>> gdbarch_memtag_to_string_ftype *memtag_to_string =
>>>> default_memtag_to_string;
>>>>> gdbarch_tagged_address_p_ftype *tagged_address_p =
>>>> default_tagged_address_p;
>>>>> gdbarch_memtag_matches_p_ftype *memtag_matches_p =
>>>>> default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch (struct
>>>> gdbarch *gdbarch)
>>>>> /* Skip verify of frame_red_zone_size, invalid_p == 0. */
>>>>> /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
>>>>> /* Skip verify of addr_bits_remove, invalid_p == 0. */
>>>>> - /* Skip verify of remove_non_address_bits, invalid_p == 0. */
>>>>> + /* Skip verify of remove_non_address_bits_watchpoint, invalid_p
>>>>> + == 0. */
>>>>> + /* Skip verify of remove_non_address_bits_breakpoint, invalid_p
>>>>> + == 0. */
>>>>> + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
>>>>> + */
>>>>> /* Skip verify of memtag_to_string, invalid_p == 0. */
>>>>> /* Skip verify of tagged_address_p, invalid_p == 0. */
>>>>> /* Skip verify of memtag_matches_p, invalid_p == 0. */ @@ -910,8
>>>>> +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file
>>>>> +*file)
>>>>> "gdbarch_dump: addr_bits_remove = <%s>\n",
>>>>> host_address_to_string (gdbarch->addr_bits_remove));
>>>>> gdb_printf (file,
>>>>> - "gdbarch_dump: remove_non_address_bits = <%s>\n",
>>>>> - host_address_to_string (gdbarch->remove_non_address_bits));
>>>>> + "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
>>>>> + host_address_to_string
>>>>> +(gdbarch->remove_non_address_bits_watchpoint));
>>>>> + gdb_printf (file,
>>>>> + "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
>>>>> + host_address_to_string
>>>>> +(gdbarch->remove_non_address_bits_breakpoint));
>>>>> + gdb_printf (file,
>>>>> + "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
>>>>> + host_address_to_string
>>>>> +(gdbarch->remove_non_address_bits_memory));
>>>>> gdb_printf (file,
>>>>> "gdbarch_dump: memtag_to_string = <%s>\n",
>>>>> host_address_to_string (gdbarch->memtag_to_string)); @@
>>>>> -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch
>>>>> *gdbarch, }
>>>>>
>>>>> CORE_ADDR
>>>>> -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
>>>>> pointer)
>>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>>> +*gdbarch, CORE_ADDR pointer) {
>>>>> + gdb_assert (gdbarch != NULL);
>>>>> + gdb_assert (gdbarch->remove_non_address_bits_watchpoint != NULL);
>>>>> + if (gdbarch_debug >= 2)
>>>>> + gdb_printf (gdb_stdlog,
>>>>> +"gdbarch_remove_non_address_bits_watchpoint called\n");
>>>>> + return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
>>>>> +pointer); }
>>>>> +
>>>>> +void
>>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>> *gdbarch,
>>>>> +
>>>> gdbarch_remove_non_address_bits_watchpoint_ftype
>>>>> +remove_non_address_bits_watchpoint)
>>>>> +{
>>>>> + gdbarch->remove_non_address_bits_watchpoint =
>>>>> +remove_non_address_bits_watchpoint;
>>>>> +}
>>>>> +
>>>>> +CORE_ADDR
>>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>> +*gdbarch, CORE_ADDR pointer) {
>>>>> + gdb_assert (gdbarch != NULL);
>>>>> + gdb_assert (gdbarch->remove_non_address_bits_breakpoint != NULL);
>>>>> + if (gdbarch_debug >= 2)
>>>>> + gdb_printf (gdb_stdlog,
>>>>> +"gdbarch_remove_non_address_bits_breakpoint called\n");
>>>>> + return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
>>>>> +pointer); }
>>>>> +
>>>>> +void
>>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>> +*gdbarch,
>>>>> +
>>>> gdbarch_remove_non_address_bits_breakpoint_ftype
>>>>> +remove_non_address_bits_breakpoint)
>>>>> +{
>>>>> + gdbarch->remove_non_address_bits_breakpoint =
>>>>> +remove_non_address_bits_breakpoint;
>>>>> +}
>>>>> +
>>>>> +CORE_ADDR
>>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>>>> +CORE_ADDR pointer)
>>>>> {
>>>>> gdb_assert (gdbarch != NULL);
>>>>> - gdb_assert (gdbarch->remove_non_address_bits != NULL);
>>>>> + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
>>>>> if (gdbarch_debug >= 2)
>>>>> - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
>>>>> - return gdbarch->remove_non_address_bits (gdbarch, pointer);
>>>>> + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_memory
>>>>> + called\n"); return gdbarch->remove_non_address_bits_memory
>>>>> + (gdbarch, pointer);
>>>>> }
>>>>>
>>>>> void
>>>>> -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
>>>>> - gdbarch_remove_non_address_bits_ftype
>>>> remove_non_address_bits)
>>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
>>>>> +*gdbarch,
>>>>> +
>>>> gdbarch_remove_non_address_bits_memory_ftype
>>>>> +remove_non_address_bits_memory)
>>>>> {
>>>>> - gdbarch->remove_non_address_bits = remove_non_address_bits;
>>>>> + gdbarch->remove_non_address_bits_memory =
>>>>> + remove_non_address_bits_memory;
>>>>> }
>>>>>
>>>>> std::string
>>>>> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
>>>>> b982fd7cd09..9fda85f860f 100644
>>>>> --- a/gdb/gdbarch-gen.h
>>>>> +++ b/gdb/gdbarch-gen.h
>>>>> @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove
>>>>> (struct gdbarch *gdbarch, CORE_ADDR ad extern void
>>>>> set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
>>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove);
>>>>>
>>>>> /* On some architectures, not all bits of a pointer are significant.
>>>>> - On AArch64, for example, the top bits of a pointer may carry a "tag", which
>>>>> - can be ignored by the kernel and the hardware. The "tag" can be regarded
>> as
>>>>> - additional data associated with the pointer, but it is not part of the
>> address.
>>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
>>>>> + "tag", which can be ignored by the kernel and the hardware. The
>>>>> + "tag" can
>>>> be
>>>>> + regarded as additional data associated with the pointer, but it is not part
>>>>> + of the address.
>>>>>
>>>>> Given a pointer for the architecture, this hook removes all the
>>>>> - non-significant bits and sign-extends things as needed. It gets used to
>>>> remove
>>>>> - non-address bits from data pointers (for example, removing the AArch64
>> MTE
>>>> tag
>>>>> - bits from a pointer) and from code pointers (removing the AArch64 PAC
>>>> signature
>>>>> - from a pointer containing the return address). */
>>>>> -
>>>>> -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct
>>>>> gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
>>>>> gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
>>>>> pointer); -extern void set_gdbarch_remove_non_address_bits (struct
>>>>> gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype
>>>>> *remove_non_address_bits);
>>>>> + non-significant bits and sign-extends things as needed. It gets used to
>>>>> + remove non-address bits from pointers used for watchpoints. */
>>>>> +
>>>>> +typedef CORE_ADDR
>>>>> +(gdbarch_remove_non_address_bits_watchpoint_ftype)
>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>>> +*gdbarch, CORE_ADDR pointer); extern void
>>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>>> +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
>>>>> +*remove_non_address_bits_watchpoint);
>>>>> +
>>>>> +/* On some architectures, not all bits of a pointer are significant.
>>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
>>>>> + "tag", which can be ignored by the kernel and the hardware. The
>>>>> +"tag" can
>>>> be
>>>>> + regarded as additional data associated with the pointer, but it is not part
>>>>> + of the address.
>>>>> +
>>>>> + Given a pointer for the architecture, this hook removes all the
>>>>> + non-significant bits and sign-extends things as needed. It gets used to
>>>>> + remove non-address bits from pointers used for breakpoints. */
>>>>> +
>>>>> +typedef CORE_ADDR
>>>>> +(gdbarch_remove_non_address_bits_breakpoint_ftype)
>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>> +*gdbarch, CORE_ADDR pointer); extern void
>>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>> +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
>>>>> +*remove_non_address_bits_breakpoint);
>>>>> +
>>>>> +/* On some architectures, not all bits of a pointer are significant.
>>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry a
>>>>> + "tag", which can be ignored by the kernel and the hardware. The
>>>>> +"tag" can
>>>> be
>>>>> + regarded as additional data associated with the pointer, but it is not part
>>>>> + of the address.
>>>>> +
>>>>> + Given a pointer for the architecture, this hook removes all the
>>>>> + non-significant bits and sign-extends things as needed. It gets used to
>>>>> + remove non-address bits from any pointer used to access memory.
>>>>> + */
>>>>> +
>>>>> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_memory_ftype)
>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>>>> +CORE_ADDR pointer); extern void
>>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
>>>>> +*gdbarch, gdbarch_remove_non_address_bits_memory_ftype
>>>>> +*remove_non_address_bits_memory);
>>>>>
>>>>> /* Return a string representation of the memory tag TAG. */
>>>>>
>>>>> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
>>>>> index 4006380076d..cc7c6d8677b 100644
>>>>> --- a/gdb/gdbarch_components.py
>>>>> +++ b/gdb/gdbarch_components.py
>>>>> @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
>>>> instead).
>>>>> Method(
>>>>> comment="""
>>>>> On some architectures, not all bits of a pointer are significant.
>>>>> -On AArch64, for example, the top bits of a pointer may carry a
>>>>> "tag", which -can be ignored by the kernel and the hardware. The
>>>>> "tag" can be regarded as -additional data associated with the
>>>>> pointer, but it is not part of
>>>> the address.
>>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>>>> +The "tag" can be regarded as additional data associated with the
>>>>> +pointer, but it is not part of the address.
>>>>>
>>>>> Given a pointer for the architecture, this hook removes all the
>>>>> -non-significant bits and sign-extends things as needed. It gets
>>>>> used to remove -non-address bits from data pointers (for example,
>>>>> removing the AArch64 MTE tag -bits from a pointer) and from code
>>>>> pointers (removing the AArch64 PAC signature -from a pointer
>>>>> containing the return
>>>> address).
>>>>> +non-significant bits and sign-extends things as needed. It gets
>>>>> +used to remove non-address bits from pointers used for watchpoints.
>>>>> """,
>>>>> type="CORE_ADDR",
>>>>> - name="remove_non_address_bits",
>>>>> + name="remove_non_address_bits_watchpoint",
>>>>> + params=[("CORE_ADDR", "pointer")],
>>>>> + predefault="default_remove_non_address_bits",
>>>>> + invalid=False,
>>>>> +)
>>>>> +
>>>>> +Method(
>>>>> + comment="""
>>>>> +On some architectures, not all bits of a pointer are significant.
>>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>>>> +The "tag" can be regarded as additional data associated with the
>>>>> +pointer, but it is not part of the address.
>>>>> +
>>>>> +Given a pointer for the architecture, this hook removes all the
>>>>> +non-significant bits and sign-extends things as needed. It gets
>>>>> +used to remove non-address bits from pointers used for breakpoints.
>>>>> +""",
>>>>> + type="CORE_ADDR",
>>>>> + name="remove_non_address_bits_breakpoint",
>>>>> + params=[("CORE_ADDR", "pointer")],
>>>>> + predefault="default_remove_non_address_bits",
>>>>> + invalid=False,
>>>>> +)
>>>>> +
>>>>> +Method(
>>>>> + comment="""
>>>>> +On some architectures, not all bits of a pointer are significant.
>>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>>>> +The "tag" can be regarded as additional data associated with the
>>>>> +pointer, but it is not part of the address.
>>>>> +
>>>>> +Given a pointer for the architecture, this hook removes all the
>>>>> +non-significant bits and sign-extends things as needed. It gets
>>>>> +used to remove non-address bits from any pointer used to access memory.
>>>>> +""",
>>>>> + type="CORE_ADDR",
>>>>> + name="remove_non_address_bits_memory",
>>>>> params=[("CORE_ADDR", "pointer")],
>>>>> predefault="default_remove_non_address_bits",
>>>>> invalid=False,
>>>>> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
>>>>> index bc9927dd751..dc4a019614a 100644
>>>>> --- a/gdb/loongarch-linux-nat.c
>>>>> +++ b/gdb/loongarch-linux-nat.c
>>>>> @@ -613,7 +613,7 @@ loongarch_linux_nat_target::stopped_data_address
>>>> (CORE_ADDR *addr_p)
>>>>> kernel can potentially be tagged addresses. */
>>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
>>>>> const CORE_ADDR addr_trap
>>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>> siginfo.si_addr);
>>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>>> + siginfo.si_addr);
>>>>>
>>>>> /* Check if the address matches any watched address. */
>>>>> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
>>>>
>>>> I think the loongarch-linux-nat.c change is spurious. Could you
>>>> please address that?
>>>
>>> Why is it spurious? What do you mean exactly?
>>
>> The intention behind this change isn't clear to me, but the Loongson native Linux
>> layer using a AArch64 architecture function does not make sense.
>>
>> I haven't built a LoongArch native Linux target, but I'd guess that it will run into a
>> build error with this change, if built for LoongArch only.
>>
>> The right approach, in my mind, is to drop this change, but you have to touch this
>> code due to your changes. It begs the question, why is the LoongArch native code
>> calling gdbarch_remove_non_address_bits here without even registering a
>> function for the gdbarch hook to call?
>
> Right, I agree.
>
>> Without this hook, we'll invoke the dummy implementation in gdb/arch-
>> utils.c:default_remove_non_address_bits, and that just returns the unmodified
>> pointer.
>
> True, that's probably why nobody noticed before.
>
>> This is a question for the LoongArch maintainer (cc-ed). If we don't hear back, we
>> should probably change the LoongArch code to not call
>> gdbarch_remove_non_address_bits. I think that was the original intention, but
>> there might've been a copy/paste error somewhere.
>
> Ok. Let's wait a bit. Thanks for the detailed feedback.
Alternatively, if you could replace, in the LoongArch code, gdbarch_remove_non_address_bits
with one of the three new hooks, that should work as well. I think even cleaner would be to
just drop the hook for Loongarch and assign siginfo.si_addr directly to addr_trap in the code.
Then we wouldn't need to delay this change further. How does thar sound for a v7 (and hopefully last)?
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-11-05 14:51 ` Luis Machado
@ 2024-11-05 15:09 ` Schimpe, Christina
2024-11-05 15:14 ` Luis Machado
0 siblings, 1 reply; 12+ messages in thread
From: Schimpe, Christina @ 2024-11-05 15:09 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: Tiezhu Yang
> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Tuesday, November 5, 2024 3:51 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>
> On 11/5/24 14:40, Schimpe, Christina wrote:
> >> -----Original Message-----
> >> From: Luis Machado <luis.machado@arm.com>
> >> Sent: Tuesday, November 5, 2024 3:22 PM
> >> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >> patches@sourceware.org
> >> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> >> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
> >>
> >> On 11/5/24 14:07, Schimpe, Christina wrote:
> >>> Hi Luis,
> >>>
> >>> Thanks a lot for the review. I have one questions to your comment,
> >>> please see
> >> below.
> >>>
> >>>> -----Original Message-----
> >>>> From: Luis Machado <luis.machado@arm.com>
> >>>> Sent: Monday, November 4, 2024 12:18 PM
> >>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >>>> patches@sourceware.org
> >>>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
> >>>>
> >>>> Hi Christina,
> >>>>
> >>>> On 10/28/24 08:46, Schimpe, Christina wrote:
> >>>>> From: Christina Schimpe <christina.schimpe@intel.com>
> >>>>>
> >>>>> The gdbarch function gdbarch_remove_non_address_bits adjusts
> >>>>> addresses to enable debugging of programs with tagged pointers on
> >>>>> Linux, for instance for ARM's feature top byte ignore (TBI).
> >>>>> Once the function is implemented for an architecture, it adjusts
> >>>>> addresses for memory access, breakpoints and watchpoints.
> >>>>>
> >>>>> Linear address masking (LAM) is Intel's (R) implementation of
> >>>>> tagged pointer support. It requires certain adaptions to GDB's
> >>>>> tagged pointer support due to the following:
> >>>>> - LAM supports address tagging for data accesses only. Thus, specifying
> >>>>> breakpoints on tagged addresses is not a valid use case.
> >>>>> - In contrast to the implementation for ARM's TBI, the Linux kernel
> supports
> >>>>> tagged pointers for memory access.
> >>>>>
> >>>>> This patch makes GDB's tagged pointer support configurable such
> >>>>> that it is possible to enable the address adjustment for a
> >>>>> specific feature only (e.g memory access, breakpoints or
> >>>>> watchpoints). This way, one can make sure that addresses are only
> >>>>> adjusted when necessary. In case of LAM, this avoids unnecessary
> >>>>> parsing of the /proc/<pid>/status file to get the untag mask.
> >>>>>
> >>>>> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
> >>>>> ---
> >>>>> gdb/aarch64-linux-nat.c | 2 +-
> >>>>> gdb/aarch64-linux-tdep.c | 7 +++--
> >>>>> gdb/aarch64-tdep.c | 23 ++++++++------
> >>>>> gdb/aarch64-tdep.h | 6 ++++
> >>>>> gdb/breakpoint.c | 5 +--
> >>>>> gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++------
> -
> >>>>> gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
> >>>>> gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
> >>>>> gdb/loongarch-linux-nat.c | 2 +-
> >>>>> gdb/target.c | 3 +-
> >>>>> 10 files changed, 169 insertions(+), 47 deletions(-)
> >>>>>
> >>>>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> >>>>> index
> >>>>> 0fa5bee500b..48ab765d880 100644
> >>>>> --- a/gdb/aarch64-linux-nat.c
> >>>>> +++ b/gdb/aarch64-linux-nat.c
> >>>>> @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address
> >>>> (CORE_ADDR *addr_p)
> >>>>> kernel can potentially be tagged addresses. */
> >>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >>>>> const CORE_ADDR addr_trap
> >>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>> siginfo.si_addr);
> >>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>>> + siginfo.si_addr);
> >>>>>
> >>>>> /* Check if the address matches any watched address. */
> >>>>> state = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> >>>>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> >>>>> index
> >>>>> c608a84bc71..61c3be8b6f0 100644
> >>>>> --- a/gdb/aarch64-linux-tdep.c
> >>>>> +++ b/gdb/aarch64-linux-tdep.c
> >>>>> @@ -2433,7 +2433,7 @@ static bool
> >>>>> aarch64_linux_tagged_address_p (struct gdbarch *gdbarch,
> >>>>> CORE_ADDR
> >>>>> address) {
> >>>>> /* Remove the top byte for the memory range check. */
> >>>>> - address = gdbarch_remove_non_address_bits (gdbarch, address);
> >>>>> + address = aarch64_remove_non_address_bits (gdbarch, address);
> >>>>>
> >>>>> /* Check if the page that contains ADDRESS is mapped with PROT_MTE.
> */
> >>>>> if (!linux_address_in_memtag_page (address)) @@ -2491,8 +2491,9
> >>>>> @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
> >>>>> uiout->text ("\n");
> >>>>>
> >>>>> std::optional<CORE_ADDR> atag
> >>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> >>>>> - fault_addr));
> >>>>> + = aarch64_mte_get_atag (
> >>>>> + aarch64_remove_non_address_bits (gdbarch, fault_addr));
> >>>>> +
> >>>>> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
> >>>>>
> >>>>> if (!atag.has_value ())
> >>>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> >>>>> 8a2a9b1e23c..91fec7879ed 100644
> >>>>> --- a/gdb/aarch64-tdep.c
> >>>>> +++ b/gdb/aarch64-tdep.c
> >>>>> @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch
> >>>>> *gdbarch,
> >>>>>
> >>>>> /* Fetch the allocation tag for ADDRESS. */
> >>>>> std::optional<CORE_ADDR> atag
> >>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits
> (gdbarch,
> >>>> addr));
> >>>>> + = aarch64_mte_get_atag (aarch64_remove_non_address_bits
> >>>>> + (gdbarch, addr));
> >>>>>
> >>>>> if (!atag.has_value ())
> >>>>> return true;
> >>>>> @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch
> >>>>> *gdbarch,
> >>>> struct value *address,
> >>>>> else
> >>>>> {
> >>>>> /* Remove the top byte. */
> >>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> >>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >>>>>
> >>>>> /* With G being the number of tag granules and N the number of tags
> >>>>> passed in, we can have the following cases:
> >>>>> @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch
> *gdbarch,
> >>>> struct value *address,
> >>>>> else
> >>>>> {
> >>>>> /* Remove the top byte. */
> >>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> >>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >>>>> std::optional<CORE_ADDR> atag = aarch64_mte_get_atag
> >>>>> (addr);
> >>>>>
> >>>>> if (!atag.has_value ())
> >>>>> @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
> >>>> *gdbarch, struct value *tag_value)
> >>>>> return string_printf ("0x%s", phex_nz (tag, sizeof (tag))); }
> >>>>>
> >>>>> -/* AArch64 implementation of the remove_non_address_bits gdbarch
> hook.
> >>>> Remove
> >>>>> - non address bits from a pointer value. */
> >>>>> +/* See aarch64-tdep.h. */
> >>>>>
> >>>>> -static CORE_ADDR
> >>>>> +CORE_ADDR
> >>>>> aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>> CORE_ADDR
> >>>>> pointer) {
> >>>>> /* By default, we assume TBI and discard the top 8 bits plus
> >>>>> the VA range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct
> >>>>> gdbarch_info
> >>>> info, struct gdbarch_list *arches)
> >>>>> tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
> >>>>>
> >>>>> /* Architecture hook to remove bits of a pointer that are not part of the
> >>>>> - address, like memory tags (MTE) and pointer authentication signatures.
> >> */
> >>>>> - set_gdbarch_remove_non_address_bits (gdbarch,
> >>>>> - aarch64_remove_non_address_bits);
> >>>>> + address, like memory tags (MTE) and pointer authentication
> signatures.
> >>>>> + Configure address adjustment for watch-, breakpoints and
> >>>>> + memory
> >>>>
> >>>> s/watch-/watchpoints
> >>>
> >>> Fill fix, thanks.
> >>>
> >>>>> + transfer. */
> >>>>> + set_gdbarch_remove_non_address_bits_watchpoint
> >>>>> + (gdbarch, aarch64_remove_non_address_bits);
> >>>>> + set_gdbarch_remove_non_address_bits_breakpoint
> >>>>> + (gdbarch, aarch64_remove_non_address_bits);
> >>>>> + set_gdbarch_remove_non_address_bits_memory
> >>>>> + (gdbarch, aarch64_remove_non_address_bits);
> >>>>>
> >>>>> /* SME pseudo-registers. */
> >>>>> if (tdep->has_sme ())
> >>>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
> >>>>> 50166fb4f24..13ea37de7a3 100644
> >>>>> --- a/gdb/aarch64-tdep.h
> >>>>> +++ b/gdb/aarch64-tdep.h
> >>>>> @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep
> >>>>> (struct gdbarch *gdbarch);
> >>>>>
> >>>>> std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR
> >>>>> address);
> >>>>>
> >>>>> +/* AArch64 implementation of the remove_non_address_bits gdbarch
> >> hooks.
> >>>>> + Remove non address bits from a pointer value. */
> >>>>> +
> >>>>> +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch
> *gdbarch,
> >>>>> + CORE_ADDR pointer);
> >>>>> +
> >>>>> #endif /* aarch64-tdep.h */
> >>>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
> >>>>> b7e4f5d0a45..9983e905e1b 100644
> >>>>> --- a/gdb/breakpoint.c
> >>>>> +++ b/gdb/breakpoint.c
> >>>>> @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b,
> >>>>> bool
> >>>> reparse)
> >>>>> loc->gdbarch = v->type ()->arch ();
> >>>>> loc->pspace = wp_pspace;
> >>>>> loc->address
> >>>>> - = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
> >>>>> + = gdbarch_remove_non_address_bits_watchpoint (loc-
> >>>>> gdbarch,
> >>>>> + addr);
> >>>>> b->add_location (*loc);
> >>>>>
> >>>>> if (bitsize != 0)
> >>>>> @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
> >>>> *gdbarch,
> >>>>> }
> >>>>>
> >>>>> adjusted_bpaddr
> >>>>> - = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
> >>>>> + = gdbarch_remove_non_address_bits_breakpoint (gdbarch,
> >>>>> +adjusted_bpaddr);
> >>>>>
> >>>>> /* An adjusted breakpoint address can significantly alter
> >>>>> a user's expectations. Print a warning if an adjustment diff
> >>>>> --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index
> >>>>> 0d00cd7c993..d05c7a3cbdf
> >>>>> 100644
> >>>>> --- a/gdb/gdbarch-gen.c
> >>>>> +++ b/gdb/gdbarch-gen.c
> >>>>> @@ -143,7 +143,9 @@ struct gdbarch
> >>>>> int frame_red_zone_size = 0;
> >>>>> gdbarch_convert_from_func_ptr_addr_ftype
> >>>>> *convert_from_func_ptr_addr =
> >>>> convert_from_func_ptr_addr_identity;
> >>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove =
> >>>>> core_addr_identity;
> >>>>> - gdbarch_remove_non_address_bits_ftype *remove_non_address_bits
> >>>>> = default_remove_non_address_bits;
> >>>>> + gdbarch_remove_non_address_bits_watchpoint_ftype
> >>>>> + *remove_non_address_bits_watchpoint =
> >>>>> + default_remove_non_address_bits;
> >>>>> + gdbarch_remove_non_address_bits_breakpoint_ftype
> >>>>> + *remove_non_address_bits_breakpoint =
> >>>>> + default_remove_non_address_bits;
> >>>>> + gdbarch_remove_non_address_bits_memory_ftype
> >>>>> + *remove_non_address_bits_memory =
> >>>>> + default_remove_non_address_bits;
> >>>>> gdbarch_memtag_to_string_ftype *memtag_to_string =
> >>>> default_memtag_to_string;
> >>>>> gdbarch_tagged_address_p_ftype *tagged_address_p =
> >>>> default_tagged_address_p;
> >>>>> gdbarch_memtag_matches_p_ftype *memtag_matches_p =
> >>>>> default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch
> >>>>> (struct
> >>>> gdbarch *gdbarch)
> >>>>> /* Skip verify of frame_red_zone_size, invalid_p == 0. */
> >>>>> /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
> >>>>> /* Skip verify of addr_bits_remove, invalid_p == 0. */
> >>>>> - /* Skip verify of remove_non_address_bits, invalid_p == 0. */
> >>>>> + /* Skip verify of remove_non_address_bits_watchpoint, invalid_p
> >>>>> + == 0. */
> >>>>> + /* Skip verify of remove_non_address_bits_breakpoint, invalid_p
> >>>>> + == 0. */
> >>>>> + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
> >>>>> + */
> >>>>> /* Skip verify of memtag_to_string, invalid_p == 0. */
> >>>>> /* Skip verify of tagged_address_p, invalid_p == 0. */
> >>>>> /* Skip verify of memtag_matches_p, invalid_p == 0. */ @@
> >>>>> -910,8
> >>>>> +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file
> >>>>> +*file)
> >>>>> "gdbarch_dump: addr_bits_remove = <%s>\n",
> >>>>> host_address_to_string (gdbarch->addr_bits_remove));
> >>>>> gdb_printf (file,
> >>>>> - "gdbarch_dump: remove_non_address_bits = <%s>\n",
> >>>>> - host_address_to_string (gdbarch->remove_non_address_bits));
> >>>>> + "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
> >>>>> + host_address_to_string
> >>>>> +(gdbarch->remove_non_address_bits_watchpoint));
> >>>>> + gdb_printf (file,
> >>>>> + "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
> >>>>> + host_address_to_string
> >>>>> +(gdbarch->remove_non_address_bits_breakpoint));
> >>>>> + gdb_printf (file,
> >>>>> + "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
> >>>>> + host_address_to_string
> >>>>> +(gdbarch->remove_non_address_bits_memory));
> >>>>> gdb_printf (file,
> >>>>> "gdbarch_dump: memtag_to_string = <%s>\n",
> >>>>> host_address_to_string (gdbarch->memtag_to_string)); @@
> >>>>> -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch
> >>>>> *gdbarch, }
> >>>>>
> >>>>> CORE_ADDR
> >>>>> -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>> CORE_ADDR
> >>>>> pointer)
> >>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>>> +*gdbarch, CORE_ADDR pointer) {
> >>>>> + gdb_assert (gdbarch != NULL);
> >>>>> + gdb_assert (gdbarch->remove_non_address_bits_watchpoint !=
> >>>>> +NULL);
> >>>>> + if (gdbarch_debug >= 2)
> >>>>> + gdb_printf (gdb_stdlog,
> >>>>> +"gdbarch_remove_non_address_bits_watchpoint called\n");
> >>>>> + return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
> >>>>> +pointer); }
> >>>>> +
> >>>>> +void
> >>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>> *gdbarch,
> >>>>> +
> >>>> gdbarch_remove_non_address_bits_watchpoint_ftype
> >>>>> +remove_non_address_bits_watchpoint)
> >>>>> +{
> >>>>> + gdbarch->remove_non_address_bits_watchpoint =
> >>>>> +remove_non_address_bits_watchpoint;
> >>>>> +}
> >>>>> +
> >>>>> +CORE_ADDR
> >>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>> +*gdbarch, CORE_ADDR pointer) {
> >>>>> + gdb_assert (gdbarch != NULL);
> >>>>> + gdb_assert (gdbarch->remove_non_address_bits_breakpoint !=
> >>>>> +NULL);
> >>>>> + if (gdbarch_debug >= 2)
> >>>>> + gdb_printf (gdb_stdlog,
> >>>>> +"gdbarch_remove_non_address_bits_breakpoint called\n");
> >>>>> + return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
> >>>>> +pointer); }
> >>>>> +
> >>>>> +void
> >>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>> +*gdbarch,
> >>>>> +
> >>>> gdbarch_remove_non_address_bits_breakpoint_ftype
> >>>>> +remove_non_address_bits_breakpoint)
> >>>>> +{
> >>>>> + gdbarch->remove_non_address_bits_breakpoint =
> >>>>> +remove_non_address_bits_breakpoint;
> >>>>> +}
> >>>>> +
> >>>>> +CORE_ADDR
> >>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> >>>>> +CORE_ADDR pointer)
> >>>>> {
> >>>>> gdb_assert (gdbarch != NULL);
> >>>>> - gdb_assert (gdbarch->remove_non_address_bits != NULL);
> >>>>> + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
> >>>>> if (gdbarch_debug >= 2)
> >>>>> - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
> >>>>> - return gdbarch->remove_non_address_bits (gdbarch, pointer);
> >>>>> + gdb_printf (gdb_stdlog,
> >>>>> + "gdbarch_remove_non_address_bits_memory
> >>>>> + called\n"); return gdbarch->remove_non_address_bits_memory
> >>>>> + (gdbarch, pointer);
> >>>>> }
> >>>>>
> >>>>> void
> >>>>> -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>> - gdbarch_remove_non_address_bits_ftype
> >>>> remove_non_address_bits)
> >>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>>>> +*gdbarch,
> >>>>> +
> >>>> gdbarch_remove_non_address_bits_memory_ftype
> >>>>> +remove_non_address_bits_memory)
> >>>>> {
> >>>>> - gdbarch->remove_non_address_bits = remove_non_address_bits;
> >>>>> + gdbarch->remove_non_address_bits_memory =
> >>>>> + remove_non_address_bits_memory;
> >>>>> }
> >>>>>
> >>>>> std::string
> >>>>> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
> >>>>> b982fd7cd09..9fda85f860f 100644
> >>>>> --- a/gdb/gdbarch-gen.h
> >>>>> +++ b/gdb/gdbarch-gen.h
> >>>>> @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove
> >>>>> (struct gdbarch *gdbarch, CORE_ADDR ad extern void
> >>>>> set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
> >>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove);
> >>>>>
> >>>>> /* On some architectures, not all bits of a pointer are significant.
> >>>>> - On AArch64, for example, the top bits of a pointer may carry a "tag",
> which
> >>>>> - can be ignored by the kernel and the hardware. The "tag" can be
> regarded
> >> as
> >>>>> - additional data associated with the pointer, but it is not part of the
> >> address.
> >>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry
> a
> >>>>> + "tag", which can be ignored by the kernel and the hardware.
> >>>>> + The "tag" can
> >>>> be
> >>>>> + regarded as additional data associated with the pointer, but it is not
> part
> >>>>> + of the address.
> >>>>>
> >>>>> Given a pointer for the architecture, this hook removes all the
> >>>>> - non-significant bits and sign-extends things as needed. It gets used to
> >>>> remove
> >>>>> - non-address bits from data pointers (for example, removing the AArch64
> >> MTE
> >>>> tag
> >>>>> - bits from a pointer) and from code pointers (removing the AArch64 PAC
> >>>> signature
> >>>>> - from a pointer containing the return address). */
> >>>>> -
> >>>>> -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct
> >>>>> gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
> >>>>> gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>> CORE_ADDR pointer); -extern void
> >>>>> set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>> gdbarch_remove_non_address_bits_ftype
> >>>>> *remove_non_address_bits);
> >>>>> + non-significant bits and sign-extends things as needed. It gets used to
> >>>>> + remove non-address bits from pointers used for watchpoints. */
> >>>>> +
> >>>>> +typedef CORE_ADDR
> >>>>> +(gdbarch_remove_non_address_bits_watchpoint_ftype)
> >>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>>> +*gdbarch, CORE_ADDR pointer); extern void
> >>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>>> +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
> >>>>> +*remove_non_address_bits_watchpoint);
> >>>>> +
> >>>>> +/* On some architectures, not all bits of a pointer are significant.
> >>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry
> a
> >>>>> + "tag", which can be ignored by the kernel and the hardware.
> >>>>> +The "tag" can
> >>>> be
> >>>>> + regarded as additional data associated with the pointer, but it is not
> part
> >>>>> + of the address.
> >>>>> +
> >>>>> + Given a pointer for the architecture, this hook removes all the
> >>>>> + non-significant bits and sign-extends things as needed. It gets used to
> >>>>> + remove non-address bits from pointers used for breakpoints. */
> >>>>> +
> >>>>> +typedef CORE_ADDR
> >>>>> +(gdbarch_remove_non_address_bits_breakpoint_ftype)
> >>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>> +*gdbarch, CORE_ADDR pointer); extern void
> >>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>> +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
> >>>>> +*remove_non_address_bits_breakpoint);
> >>>>> +
> >>>>> +/* On some architectures, not all bits of a pointer are significant.
> >>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry
> a
> >>>>> + "tag", which can be ignored by the kernel and the hardware.
> >>>>> +The "tag" can
> >>>> be
> >>>>> + regarded as additional data associated with the pointer, but it is not
> part
> >>>>> + of the address.
> >>>>> +
> >>>>> + Given a pointer for the architecture, this hook removes all the
> >>>>> + non-significant bits and sign-extends things as needed. It gets used to
> >>>>> + remove non-address bits from any pointer used to access memory.
> >>>>> + */
> >>>>> +
> >>>>> +typedef CORE_ADDR
> (gdbarch_remove_non_address_bits_memory_ftype)
> >>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> >>>>> +CORE_ADDR pointer); extern void
> >>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>>>> +*gdbarch, gdbarch_remove_non_address_bits_memory_ftype
> >>>>> +*remove_non_address_bits_memory);
> >>>>>
> >>>>> /* Return a string representation of the memory tag TAG. */
> >>>>>
> >>>>> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> >>>>> index 4006380076d..cc7c6d8677b 100644
> >>>>> --- a/gdb/gdbarch_components.py
> >>>>> +++ b/gdb/gdbarch_components.py
> >>>>> @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
> >>>> instead).
> >>>>> Method(
> >>>>> comment="""
> >>>>> On some architectures, not all bits of a pointer are significant.
> >>>>> -On AArch64, for example, the top bits of a pointer may carry a
> >>>>> "tag", which -can be ignored by the kernel and the hardware. The
> >>>>> "tag" can be regarded as -additional data associated with the
> >>>>> pointer, but it is not part of
> >>>> the address.
> >>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
> >>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
> >>>>> +The "tag" can be regarded as additional data associated with the
> >>>>> +pointer, but it is not part of the address.
> >>>>>
> >>>>> Given a pointer for the architecture, this hook removes all the
> >>>>> -non-significant bits and sign-extends things as needed. It gets
> >>>>> used to remove -non-address bits from data pointers (for example,
> >>>>> removing the AArch64 MTE tag -bits from a pointer) and from code
> >>>>> pointers (removing the AArch64 PAC signature -from a pointer
> >>>>> containing the return
> >>>> address).
> >>>>> +non-significant bits and sign-extends things as needed. It gets
> >>>>> +used to remove non-address bits from pointers used for watchpoints.
> >>>>> """,
> >>>>> type="CORE_ADDR",
> >>>>> - name="remove_non_address_bits",
> >>>>> + name="remove_non_address_bits_watchpoint",
> >>>>> + params=[("CORE_ADDR", "pointer")],
> >>>>> + predefault="default_remove_non_address_bits",
> >>>>> + invalid=False,
> >>>>> +)
> >>>>> +
> >>>>> +Method(
> >>>>> + comment="""
> >>>>> +On some architectures, not all bits of a pointer are significant.
> >>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
> >>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
> >>>>> +The "tag" can be regarded as additional data associated with the
> >>>>> +pointer, but it is not part of the address.
> >>>>> +
> >>>>> +Given a pointer for the architecture, this hook removes all the
> >>>>> +non-significant bits and sign-extends things as needed. It gets
> >>>>> +used to remove non-address bits from pointers used for breakpoints.
> >>>>> +""",
> >>>>> + type="CORE_ADDR",
> >>>>> + name="remove_non_address_bits_breakpoint",
> >>>>> + params=[("CORE_ADDR", "pointer")],
> >>>>> + predefault="default_remove_non_address_bits",
> >>>>> + invalid=False,
> >>>>> +)
> >>>>> +
> >>>>> +Method(
> >>>>> + comment="""
> >>>>> +On some architectures, not all bits of a pointer are significant.
> >>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
> >>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
> >>>>> +The "tag" can be regarded as additional data associated with the
> >>>>> +pointer, but it is not part of the address.
> >>>>> +
> >>>>> +Given a pointer for the architecture, this hook removes all the
> >>>>> +non-significant bits and sign-extends things as needed. It gets
> >>>>> +used to remove non-address bits from any pointer used to access
> memory.
> >>>>> +""",
> >>>>> + type="CORE_ADDR",
> >>>>> + name="remove_non_address_bits_memory",
> >>>>> params=[("CORE_ADDR", "pointer")],
> >>>>> predefault="default_remove_non_address_bits",
> >>>>> invalid=False,
> >>>>> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> >>>>> index bc9927dd751..dc4a019614a 100644
> >>>>> --- a/gdb/loongarch-linux-nat.c
> >>>>> +++ b/gdb/loongarch-linux-nat.c
> >>>>> @@ -613,7 +613,7 @@
> >>>>> loongarch_linux_nat_target::stopped_data_address
> >>>> (CORE_ADDR *addr_p)
> >>>>> kernel can potentially be tagged addresses. */
> >>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >>>>> const CORE_ADDR addr_trap
> >>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>> siginfo.si_addr);
> >>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>>> + siginfo.si_addr);
> >>>>>
> >>>>> /* Check if the address matches any watched address. */
> >>>>> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
> >>>>
> >>>> I think the loongarch-linux-nat.c change is spurious. Could you
> >>>> please address that?
> >>>
> >>> Why is it spurious? What do you mean exactly?
> >>
> >> The intention behind this change isn't clear to me, but the Loongson
> >> native Linux layer using a AArch64 architecture function does not make sense.
> >>
> >> I haven't built a LoongArch native Linux target, but I'd guess that
> >> it will run into a build error with this change, if built for LoongArch only.
> >>
> >> The right approach, in my mind, is to drop this change, but you have
> >> to touch this code due to your changes. It begs the question, why is
> >> the LoongArch native code calling gdbarch_remove_non_address_bits
> >> here without even registering a function for the gdbarch hook to call?
> >
> > Right, I agree.
> >
> >> Without this hook, we'll invoke the dummy implementation in gdb/arch-
> >> utils.c:default_remove_non_address_bits, and that just returns the
> >> unmodified pointer.
> >
> > True, that's probably why nobody noticed before.
> >
> >> This is a question for the LoongArch maintainer (cc-ed). If we don't
> >> hear back, we should probably change the LoongArch code to not call
> >> gdbarch_remove_non_address_bits. I think that was the original
> >> intention, but there might've been a copy/paste error somewhere.
> >
> > Ok. Let's wait a bit. Thanks for the detailed feedback.
>
> Alternatively, if you could replace, in the LoongArch code,
> gdbarch_remove_non_address_bits with one of the three new hooks, that should
> work as well. I think even cleaner would be to just drop the hook for Loongarch
> and assign siginfo.si_addr directly to addr_trap in the code.
I am also in favour of your second suggestion to drop the hook here.
In that case, it should be separate patch I think.
I could post it independently of this series and quickly merge before posting a v7 of my series.
LoongArch doesn't supported tagged addresses, does it? If it's not supported,
I would suggest to fix it as follows:
diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
index dc4a019614a..fd3581bbd30 100644
--- a/gdb/loongarch-linux-nat.c
+++ b/gdb/loongarch-linux-nat.c
@@ -608,17 +608,11 @@ loongarch_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != TRAP_HWBKPT)
return false;
- /* Make sure to ignore the top byte, otherwise we may not recognize a
- hardware watchpoint hit. The stopped data addresses coming from the
- kernel can potentially be tagged addresses. */
- struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
- const CORE_ADDR addr_trap
- = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
-
/* Check if the address matches any watched address. */
state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
- return loongarch_stopped_data_address (state, addr_trap, addr_p);
+ return
+ loongarch_stopped_data_address (state, (CORE_ADDR) siginfo.si_addr, addr_p);
Does that make sense to you?
> Then we wouldn't need to delay this change further. How does thar sound for a v7
> (and hopefully last)?
That sounds good to me.
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-11-05 15:09 ` Schimpe, Christina
@ 2024-11-05 15:14 ` Luis Machado
2024-11-05 15:36 ` Schimpe, Christina
0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2024-11-05 15:14 UTC (permalink / raw)
To: Schimpe, Christina, gdb-patches; +Cc: Tiezhu Yang
On 11/5/24 15:09, Schimpe, Christina wrote:
>> -----Original Message-----
>> From: Luis Machado <luis.machado@arm.com>
>> Sent: Tuesday, November 5, 2024 3:51 PM
>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>> patches@sourceware.org
>> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>>
>> On 11/5/24 14:40, Schimpe, Christina wrote:
>>>> -----Original Message-----
>>>> From: Luis Machado <luis.machado@arm.com>
>>>> Sent: Tuesday, November 5, 2024 3:22 PM
>>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>>>> patches@sourceware.org
>>>> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
>>>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>>>>
>>>> On 11/5/24 14:07, Schimpe, Christina wrote:
>>>>> Hi Luis,
>>>>>
>>>>> Thanks a lot for the review. I have one questions to your comment,
>>>>> please see
>>>> below.
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Luis Machado <luis.machado@arm.com>
>>>>>> Sent: Monday, November 4, 2024 12:18 PM
>>>>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>>>>>> patches@sourceware.org
>>>>>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>>>>>>
>>>>>> Hi Christina,
>>>>>>
>>>>>> On 10/28/24 08:46, Schimpe, Christina wrote:
>>>>>>> From: Christina Schimpe <christina.schimpe@intel.com>
>>>>>>>
>>>>>>> The gdbarch function gdbarch_remove_non_address_bits adjusts
>>>>>>> addresses to enable debugging of programs with tagged pointers on
>>>>>>> Linux, for instance for ARM's feature top byte ignore (TBI).
>>>>>>> Once the function is implemented for an architecture, it adjusts
>>>>>>> addresses for memory access, breakpoints and watchpoints.
>>>>>>>
>>>>>>> Linear address masking (LAM) is Intel's (R) implementation of
>>>>>>> tagged pointer support. It requires certain adaptions to GDB's
>>>>>>> tagged pointer support due to the following:
>>>>>>> - LAM supports address tagging for data accesses only. Thus, specifying
>>>>>>> breakpoints on tagged addresses is not a valid use case.
>>>>>>> - In contrast to the implementation for ARM's TBI, the Linux kernel
>> supports
>>>>>>> tagged pointers for memory access.
>>>>>>>
>>>>>>> This patch makes GDB's tagged pointer support configurable such
>>>>>>> that it is possible to enable the address adjustment for a
>>>>>>> specific feature only (e.g memory access, breakpoints or
>>>>>>> watchpoints). This way, one can make sure that addresses are only
>>>>>>> adjusted when necessary. In case of LAM, this avoids unnecessary
>>>>>>> parsing of the /proc/<pid>/status file to get the untag mask.
>>>>>>>
>>>>>>> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
>>>>>>> ---
>>>>>>> gdb/aarch64-linux-nat.c | 2 +-
>>>>>>> gdb/aarch64-linux-tdep.c | 7 +++--
>>>>>>> gdb/aarch64-tdep.c | 23 ++++++++------
>>>>>>> gdb/aarch64-tdep.h | 6 ++++
>>>>>>> gdb/breakpoint.c | 5 +--
>>>>>>> gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++------
>> -
>>>>>>> gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
>>>>>>> gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
>>>>>>> gdb/loongarch-linux-nat.c | 2 +-
>>>>>>> gdb/target.c | 3 +-
>>>>>>> 10 files changed, 169 insertions(+), 47 deletions(-)
>>>>>>>
>>>>>>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
>>>>>>> index
>>>>>>> 0fa5bee500b..48ab765d880 100644
>>>>>>> --- a/gdb/aarch64-linux-nat.c
>>>>>>> +++ b/gdb/aarch64-linux-nat.c
>>>>>>> @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address
>>>>>> (CORE_ADDR *addr_p)
>>>>>>> kernel can potentially be tagged addresses. */
>>>>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
>>>>>>> const CORE_ADDR addr_trap
>>>>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>>>> siginfo.si_addr);
>>>>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>>>>> + siginfo.si_addr);
>>>>>>>
>>>>>>> /* Check if the address matches any watched address. */
>>>>>>> state = aarch64_get_debug_reg_state (inferior_ptid.pid ());
>>>>>>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>>>>>>> index
>>>>>>> c608a84bc71..61c3be8b6f0 100644
>>>>>>> --- a/gdb/aarch64-linux-tdep.c
>>>>>>> +++ b/gdb/aarch64-linux-tdep.c
>>>>>>> @@ -2433,7 +2433,7 @@ static bool
>>>>>>> aarch64_linux_tagged_address_p (struct gdbarch *gdbarch,
>>>>>>> CORE_ADDR
>>>>>>> address) {
>>>>>>> /* Remove the top byte for the memory range check. */
>>>>>>> - address = gdbarch_remove_non_address_bits (gdbarch, address);
>>>>>>> + address = aarch64_remove_non_address_bits (gdbarch, address);
>>>>>>>
>>>>>>> /* Check if the page that contains ADDRESS is mapped with PROT_MTE.
>> */
>>>>>>> if (!linux_address_in_memtag_page (address)) @@ -2491,8 +2491,9
>>>>>>> @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>>>>>>> uiout->text ("\n");
>>>>>>>
>>>>>>> std::optional<CORE_ADDR> atag
>>>>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
>>>>>>> - fault_addr));
>>>>>>> + = aarch64_mte_get_atag (
>>>>>>> + aarch64_remove_non_address_bits (gdbarch, fault_addr));
>>>>>>> +
>>>>>>> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
>>>>>>>
>>>>>>> if (!atag.has_value ())
>>>>>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
>>>>>>> 8a2a9b1e23c..91fec7879ed 100644
>>>>>>> --- a/gdb/aarch64-tdep.c
>>>>>>> +++ b/gdb/aarch64-tdep.c
>>>>>>> @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch
>>>>>>> *gdbarch,
>>>>>>>
>>>>>>> /* Fetch the allocation tag for ADDRESS. */
>>>>>>> std::optional<CORE_ADDR> atag
>>>>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits
>> (gdbarch,
>>>>>> addr));
>>>>>>> + = aarch64_mte_get_atag (aarch64_remove_non_address_bits
>>>>>>> + (gdbarch, addr));
>>>>>>>
>>>>>>> if (!atag.has_value ())
>>>>>>> return true;
>>>>>>> @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch
>>>>>>> *gdbarch,
>>>>>> struct value *address,
>>>>>>> else
>>>>>>> {
>>>>>>> /* Remove the top byte. */
>>>>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>>>>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
>>>>>>>
>>>>>>> /* With G being the number of tag granules and N the number of tags
>>>>>>> passed in, we can have the following cases:
>>>>>>> @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch
>> *gdbarch,
>>>>>> struct value *address,
>>>>>>> else
>>>>>>> {
>>>>>>> /* Remove the top byte. */
>>>>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>>>>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
>>>>>>> std::optional<CORE_ADDR> atag = aarch64_mte_get_atag
>>>>>>> (addr);
>>>>>>>
>>>>>>> if (!atag.has_value ())
>>>>>>> @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
>>>>>> *gdbarch, struct value *tag_value)
>>>>>>> return string_printf ("0x%s", phex_nz (tag, sizeof (tag))); }
>>>>>>>
>>>>>>> -/* AArch64 implementation of the remove_non_address_bits gdbarch
>> hook.
>>>>>> Remove
>>>>>>> - non address bits from a pointer value. */
>>>>>>> +/* See aarch64-tdep.h. */
>>>>>>>
>>>>>>> -static CORE_ADDR
>>>>>>> +CORE_ADDR
>>>>>>> aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
>>>>>>> CORE_ADDR
>>>>>>> pointer) {
>>>>>>> /* By default, we assume TBI and discard the top 8 bits plus
>>>>>>> the VA range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct
>>>>>>> gdbarch_info
>>>>>> info, struct gdbarch_list *arches)
>>>>>>> tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
>>>>>>>
>>>>>>> /* Architecture hook to remove bits of a pointer that are not part of the
>>>>>>> - address, like memory tags (MTE) and pointer authentication signatures.
>>>> */
>>>>>>> - set_gdbarch_remove_non_address_bits (gdbarch,
>>>>>>> - aarch64_remove_non_address_bits);
>>>>>>> + address, like memory tags (MTE) and pointer authentication
>> signatures.
>>>>>>> + Configure address adjustment for watch-, breakpoints and
>>>>>>> + memory
>>>>>>
>>>>>> s/watch-/watchpoints
>>>>>
>>>>> Fill fix, thanks.
>>>>>
>>>>>>> + transfer. */
>>>>>>> + set_gdbarch_remove_non_address_bits_watchpoint
>>>>>>> + (gdbarch, aarch64_remove_non_address_bits);
>>>>>>> + set_gdbarch_remove_non_address_bits_breakpoint
>>>>>>> + (gdbarch, aarch64_remove_non_address_bits);
>>>>>>> + set_gdbarch_remove_non_address_bits_memory
>>>>>>> + (gdbarch, aarch64_remove_non_address_bits);
>>>>>>>
>>>>>>> /* SME pseudo-registers. */
>>>>>>> if (tdep->has_sme ())
>>>>>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
>>>>>>> 50166fb4f24..13ea37de7a3 100644
>>>>>>> --- a/gdb/aarch64-tdep.h
>>>>>>> +++ b/gdb/aarch64-tdep.h
>>>>>>> @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep
>>>>>>> (struct gdbarch *gdbarch);
>>>>>>>
>>>>>>> std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR
>>>>>>> address);
>>>>>>>
>>>>>>> +/* AArch64 implementation of the remove_non_address_bits gdbarch
>>>> hooks.
>>>>>>> + Remove non address bits from a pointer value. */
>>>>>>> +
>>>>>>> +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch
>> *gdbarch,
>>>>>>> + CORE_ADDR pointer);
>>>>>>> +
>>>>>>> #endif /* aarch64-tdep.h */
>>>>>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
>>>>>>> b7e4f5d0a45..9983e905e1b 100644
>>>>>>> --- a/gdb/breakpoint.c
>>>>>>> +++ b/gdb/breakpoint.c
>>>>>>> @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b,
>>>>>>> bool
>>>>>> reparse)
>>>>>>> loc->gdbarch = v->type ()->arch ();
>>>>>>> loc->pspace = wp_pspace;
>>>>>>> loc->address
>>>>>>> - = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
>>>>>>> + = gdbarch_remove_non_address_bits_watchpoint (loc-
>>>>>>> gdbarch,
>>>>>>> + addr);
>>>>>>> b->add_location (*loc);
>>>>>>>
>>>>>>> if (bitsize != 0)
>>>>>>> @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
>>>>>> *gdbarch,
>>>>>>> }
>>>>>>>
>>>>>>> adjusted_bpaddr
>>>>>>> - = gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
>>>>>>> + = gdbarch_remove_non_address_bits_breakpoint (gdbarch,
>>>>>>> +adjusted_bpaddr);
>>>>>>>
>>>>>>> /* An adjusted breakpoint address can significantly alter
>>>>>>> a user's expectations. Print a warning if an adjustment diff
>>>>>>> --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index
>>>>>>> 0d00cd7c993..d05c7a3cbdf
>>>>>>> 100644
>>>>>>> --- a/gdb/gdbarch-gen.c
>>>>>>> +++ b/gdb/gdbarch-gen.c
>>>>>>> @@ -143,7 +143,9 @@ struct gdbarch
>>>>>>> int frame_red_zone_size = 0;
>>>>>>> gdbarch_convert_from_func_ptr_addr_ftype
>>>>>>> *convert_from_func_ptr_addr =
>>>>>> convert_from_func_ptr_addr_identity;
>>>>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove =
>>>>>>> core_addr_identity;
>>>>>>> - gdbarch_remove_non_address_bits_ftype *remove_non_address_bits
>>>>>>> = default_remove_non_address_bits;
>>>>>>> + gdbarch_remove_non_address_bits_watchpoint_ftype
>>>>>>> + *remove_non_address_bits_watchpoint =
>>>>>>> + default_remove_non_address_bits;
>>>>>>> + gdbarch_remove_non_address_bits_breakpoint_ftype
>>>>>>> + *remove_non_address_bits_breakpoint =
>>>>>>> + default_remove_non_address_bits;
>>>>>>> + gdbarch_remove_non_address_bits_memory_ftype
>>>>>>> + *remove_non_address_bits_memory =
>>>>>>> + default_remove_non_address_bits;
>>>>>>> gdbarch_memtag_to_string_ftype *memtag_to_string =
>>>>>> default_memtag_to_string;
>>>>>>> gdbarch_tagged_address_p_ftype *tagged_address_p =
>>>>>> default_tagged_address_p;
>>>>>>> gdbarch_memtag_matches_p_ftype *memtag_matches_p =
>>>>>>> default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch
>>>>>>> (struct
>>>>>> gdbarch *gdbarch)
>>>>>>> /* Skip verify of frame_red_zone_size, invalid_p == 0. */
>>>>>>> /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
>>>>>>> /* Skip verify of addr_bits_remove, invalid_p == 0. */
>>>>>>> - /* Skip verify of remove_non_address_bits, invalid_p == 0. */
>>>>>>> + /* Skip verify of remove_non_address_bits_watchpoint, invalid_p
>>>>>>> + == 0. */
>>>>>>> + /* Skip verify of remove_non_address_bits_breakpoint, invalid_p
>>>>>>> + == 0. */
>>>>>>> + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
>>>>>>> + */
>>>>>>> /* Skip verify of memtag_to_string, invalid_p == 0. */
>>>>>>> /* Skip verify of tagged_address_p, invalid_p == 0. */
>>>>>>> /* Skip verify of memtag_matches_p, invalid_p == 0. */ @@
>>>>>>> -910,8
>>>>>>> +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file
>>>>>>> +*file)
>>>>>>> "gdbarch_dump: addr_bits_remove = <%s>\n",
>>>>>>> host_address_to_string (gdbarch->addr_bits_remove));
>>>>>>> gdb_printf (file,
>>>>>>> - "gdbarch_dump: remove_non_address_bits = <%s>\n",
>>>>>>> - host_address_to_string (gdbarch->remove_non_address_bits));
>>>>>>> + "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
>>>>>>> + host_address_to_string
>>>>>>> +(gdbarch->remove_non_address_bits_watchpoint));
>>>>>>> + gdb_printf (file,
>>>>>>> + "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
>>>>>>> + host_address_to_string
>>>>>>> +(gdbarch->remove_non_address_bits_breakpoint));
>>>>>>> + gdb_printf (file,
>>>>>>> + "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
>>>>>>> + host_address_to_string
>>>>>>> +(gdbarch->remove_non_address_bits_memory));
>>>>>>> gdb_printf (file,
>>>>>>> "gdbarch_dump: memtag_to_string = <%s>\n",
>>>>>>> host_address_to_string (gdbarch->memtag_to_string)); @@
>>>>>>> -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch
>>>>>>> *gdbarch, }
>>>>>>>
>>>>>>> CORE_ADDR
>>>>>>> -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
>>>>>>> CORE_ADDR
>>>>>>> pointer)
>>>>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>>>>> +*gdbarch, CORE_ADDR pointer) {
>>>>>>> + gdb_assert (gdbarch != NULL);
>>>>>>> + gdb_assert (gdbarch->remove_non_address_bits_watchpoint !=
>>>>>>> +NULL);
>>>>>>> + if (gdbarch_debug >= 2)
>>>>>>> + gdb_printf (gdb_stdlog,
>>>>>>> +"gdbarch_remove_non_address_bits_watchpoint called\n");
>>>>>>> + return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
>>>>>>> +pointer); }
>>>>>>> +
>>>>>>> +void
>>>>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>>>> *gdbarch,
>>>>>>> +
>>>>>> gdbarch_remove_non_address_bits_watchpoint_ftype
>>>>>>> +remove_non_address_bits_watchpoint)
>>>>>>> +{
>>>>>>> + gdbarch->remove_non_address_bits_watchpoint =
>>>>>>> +remove_non_address_bits_watchpoint;
>>>>>>> +}
>>>>>>> +
>>>>>>> +CORE_ADDR
>>>>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>>>> +*gdbarch, CORE_ADDR pointer) {
>>>>>>> + gdb_assert (gdbarch != NULL);
>>>>>>> + gdb_assert (gdbarch->remove_non_address_bits_breakpoint !=
>>>>>>> +NULL);
>>>>>>> + if (gdbarch_debug >= 2)
>>>>>>> + gdb_printf (gdb_stdlog,
>>>>>>> +"gdbarch_remove_non_address_bits_breakpoint called\n");
>>>>>>> + return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
>>>>>>> +pointer); }
>>>>>>> +
>>>>>>> +void
>>>>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>>>> +*gdbarch,
>>>>>>> +
>>>>>> gdbarch_remove_non_address_bits_breakpoint_ftype
>>>>>>> +remove_non_address_bits_breakpoint)
>>>>>>> +{
>>>>>>> + gdbarch->remove_non_address_bits_breakpoint =
>>>>>>> +remove_non_address_bits_breakpoint;
>>>>>>> +}
>>>>>>> +
>>>>>>> +CORE_ADDR
>>>>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>>>>>> +CORE_ADDR pointer)
>>>>>>> {
>>>>>>> gdb_assert (gdbarch != NULL);
>>>>>>> - gdb_assert (gdbarch->remove_non_address_bits != NULL);
>>>>>>> + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
>>>>>>> if (gdbarch_debug >= 2)
>>>>>>> - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
>>>>>>> - return gdbarch->remove_non_address_bits (gdbarch, pointer);
>>>>>>> + gdb_printf (gdb_stdlog,
>>>>>>> + "gdbarch_remove_non_address_bits_memory
>>>>>>> + called\n"); return gdbarch->remove_non_address_bits_memory
>>>>>>> + (gdbarch, pointer);
>>>>>>> }
>>>>>>>
>>>>>>> void
>>>>>>> -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
>>>>>>> - gdbarch_remove_non_address_bits_ftype
>>>>>> remove_non_address_bits)
>>>>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
>>>>>>> +*gdbarch,
>>>>>>> +
>>>>>> gdbarch_remove_non_address_bits_memory_ftype
>>>>>>> +remove_non_address_bits_memory)
>>>>>>> {
>>>>>>> - gdbarch->remove_non_address_bits = remove_non_address_bits;
>>>>>>> + gdbarch->remove_non_address_bits_memory =
>>>>>>> + remove_non_address_bits_memory;
>>>>>>> }
>>>>>>>
>>>>>>> std::string
>>>>>>> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
>>>>>>> b982fd7cd09..9fda85f860f 100644
>>>>>>> --- a/gdb/gdbarch-gen.h
>>>>>>> +++ b/gdb/gdbarch-gen.h
>>>>>>> @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove
>>>>>>> (struct gdbarch *gdbarch, CORE_ADDR ad extern void
>>>>>>> set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
>>>>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove);
>>>>>>>
>>>>>>> /* On some architectures, not all bits of a pointer are significant.
>>>>>>> - On AArch64, for example, the top bits of a pointer may carry a "tag",
>> which
>>>>>>> - can be ignored by the kernel and the hardware. The "tag" can be
>> regarded
>>>> as
>>>>>>> - additional data associated with the pointer, but it is not part of the
>>>> address.
>>>>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry
>> a
>>>>>>> + "tag", which can be ignored by the kernel and the hardware.
>>>>>>> + The "tag" can
>>>>>> be
>>>>>>> + regarded as additional data associated with the pointer, but it is not
>> part
>>>>>>> + of the address.
>>>>>>>
>>>>>>> Given a pointer for the architecture, this hook removes all the
>>>>>>> - non-significant bits and sign-extends things as needed. It gets used to
>>>>>> remove
>>>>>>> - non-address bits from data pointers (for example, removing the AArch64
>>>> MTE
>>>>>> tag
>>>>>>> - bits from a pointer) and from code pointers (removing the AArch64 PAC
>>>>>> signature
>>>>>>> - from a pointer containing the return address). */
>>>>>>> -
>>>>>>> -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct
>>>>>>> gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
>>>>>>> gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
>>>>>>> CORE_ADDR pointer); -extern void
>>>>>>> set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
>>>>>>> gdbarch_remove_non_address_bits_ftype
>>>>>>> *remove_non_address_bits);
>>>>>>> + non-significant bits and sign-extends things as needed. It gets used to
>>>>>>> + remove non-address bits from pointers used for watchpoints. */
>>>>>>> +
>>>>>>> +typedef CORE_ADDR
>>>>>>> +(gdbarch_remove_non_address_bits_watchpoint_ftype)
>>>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>>>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>>>>> +*gdbarch, CORE_ADDR pointer); extern void
>>>>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
>>>>>>> +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
>>>>>>> +*remove_non_address_bits_watchpoint);
>>>>>>> +
>>>>>>> +/* On some architectures, not all bits of a pointer are significant.
>>>>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry
>> a
>>>>>>> + "tag", which can be ignored by the kernel and the hardware.
>>>>>>> +The "tag" can
>>>>>> be
>>>>>>> + regarded as additional data associated with the pointer, but it is not
>> part
>>>>>>> + of the address.
>>>>>>> +
>>>>>>> + Given a pointer for the architecture, this hook removes all the
>>>>>>> + non-significant bits and sign-extends things as needed. It gets used to
>>>>>>> + remove non-address bits from pointers used for breakpoints. */
>>>>>>> +
>>>>>>> +typedef CORE_ADDR
>>>>>>> +(gdbarch_remove_non_address_bits_breakpoint_ftype)
>>>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>>>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>>>> +*gdbarch, CORE_ADDR pointer); extern void
>>>>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
>>>>>>> +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
>>>>>>> +*remove_non_address_bits_breakpoint);
>>>>>>> +
>>>>>>> +/* On some architectures, not all bits of a pointer are significant.
>>>>>>> + On AArch64 and amd64, for example, the top bits of a pointer may carry
>> a
>>>>>>> + "tag", which can be ignored by the kernel and the hardware.
>>>>>>> +The "tag" can
>>>>>> be
>>>>>>> + regarded as additional data associated with the pointer, but it is not
>> part
>>>>>>> + of the address.
>>>>>>> +
>>>>>>> + Given a pointer for the architecture, this hook removes all the
>>>>>>> + non-significant bits and sign-extends things as needed. It gets used to
>>>>>>> + remove non-address bits from any pointer used to access memory.
>>>>>>> + */
>>>>>>> +
>>>>>>> +typedef CORE_ADDR
>> (gdbarch_remove_non_address_bits_memory_ftype)
>>>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
>>>>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
>>>>>>> +CORE_ADDR pointer); extern void
>>>>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
>>>>>>> +*gdbarch, gdbarch_remove_non_address_bits_memory_ftype
>>>>>>> +*remove_non_address_bits_memory);
>>>>>>>
>>>>>>> /* Return a string representation of the memory tag TAG. */
>>>>>>>
>>>>>>> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
>>>>>>> index 4006380076d..cc7c6d8677b 100644
>>>>>>> --- a/gdb/gdbarch_components.py
>>>>>>> +++ b/gdb/gdbarch_components.py
>>>>>>> @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
>>>>>> instead).
>>>>>>> Method(
>>>>>>> comment="""
>>>>>>> On some architectures, not all bits of a pointer are significant.
>>>>>>> -On AArch64, for example, the top bits of a pointer may carry a
>>>>>>> "tag", which -can be ignored by the kernel and the hardware. The
>>>>>>> "tag" can be regarded as -additional data associated with the
>>>>>>> pointer, but it is not part of
>>>>>> the address.
>>>>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>>>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>>>>>> +The "tag" can be regarded as additional data associated with the
>>>>>>> +pointer, but it is not part of the address.
>>>>>>>
>>>>>>> Given a pointer for the architecture, this hook removes all the
>>>>>>> -non-significant bits and sign-extends things as needed. It gets
>>>>>>> used to remove -non-address bits from data pointers (for example,
>>>>>>> removing the AArch64 MTE tag -bits from a pointer) and from code
>>>>>>> pointers (removing the AArch64 PAC signature -from a pointer
>>>>>>> containing the return
>>>>>> address).
>>>>>>> +non-significant bits and sign-extends things as needed. It gets
>>>>>>> +used to remove non-address bits from pointers used for watchpoints.
>>>>>>> """,
>>>>>>> type="CORE_ADDR",
>>>>>>> - name="remove_non_address_bits",
>>>>>>> + name="remove_non_address_bits_watchpoint",
>>>>>>> + params=[("CORE_ADDR", "pointer")],
>>>>>>> + predefault="default_remove_non_address_bits",
>>>>>>> + invalid=False,
>>>>>>> +)
>>>>>>> +
>>>>>>> +Method(
>>>>>>> + comment="""
>>>>>>> +On some architectures, not all bits of a pointer are significant.
>>>>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>>>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>>>>>> +The "tag" can be regarded as additional data associated with the
>>>>>>> +pointer, but it is not part of the address.
>>>>>>> +
>>>>>>> +Given a pointer for the architecture, this hook removes all the
>>>>>>> +non-significant bits and sign-extends things as needed. It gets
>>>>>>> +used to remove non-address bits from pointers used for breakpoints.
>>>>>>> +""",
>>>>>>> + type="CORE_ADDR",
>>>>>>> + name="remove_non_address_bits_breakpoint",
>>>>>>> + params=[("CORE_ADDR", "pointer")],
>>>>>>> + predefault="default_remove_non_address_bits",
>>>>>>> + invalid=False,
>>>>>>> +)
>>>>>>> +
>>>>>>> +Method(
>>>>>>> + comment="""
>>>>>>> +On some architectures, not all bits of a pointer are significant.
>>>>>>> +On AArch64 and amd64, for example, the top bits of a pointer may
>>>>>>> +carry a "tag", which can be ignored by the kernel and the hardware.
>>>>>>> +The "tag" can be regarded as additional data associated with the
>>>>>>> +pointer, but it is not part of the address.
>>>>>>> +
>>>>>>> +Given a pointer for the architecture, this hook removes all the
>>>>>>> +non-significant bits and sign-extends things as needed. It gets
>>>>>>> +used to remove non-address bits from any pointer used to access
>> memory.
>>>>>>> +""",
>>>>>>> + type="CORE_ADDR",
>>>>>>> + name="remove_non_address_bits_memory",
>>>>>>> params=[("CORE_ADDR", "pointer")],
>>>>>>> predefault="default_remove_non_address_bits",
>>>>>>> invalid=False,
>>>>>>> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
>>>>>>> index bc9927dd751..dc4a019614a 100644
>>>>>>> --- a/gdb/loongarch-linux-nat.c
>>>>>>> +++ b/gdb/loongarch-linux-nat.c
>>>>>>> @@ -613,7 +613,7 @@
>>>>>>> loongarch_linux_nat_target::stopped_data_address
>>>>>> (CORE_ADDR *addr_p)
>>>>>>> kernel can potentially be tagged addresses. */
>>>>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
>>>>>>> const CORE_ADDR addr_trap
>>>>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>>>> siginfo.si_addr);
>>>>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
>>>>>>> + siginfo.si_addr);
>>>>>>>
>>>>>>> /* Check if the address matches any watched address. */
>>>>>>> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
>>>>>>
>>>>>> I think the loongarch-linux-nat.c change is spurious. Could you
>>>>>> please address that?
>>>>>
>>>>> Why is it spurious? What do you mean exactly?
>>>>
>>>> The intention behind this change isn't clear to me, but the Loongson
>>>> native Linux layer using a AArch64 architecture function does not make sense.
>>>>
>>>> I haven't built a LoongArch native Linux target, but I'd guess that
>>>> it will run into a build error with this change, if built for LoongArch only.
>>>>
>>>> The right approach, in my mind, is to drop this change, but you have
>>>> to touch this code due to your changes. It begs the question, why is
>>>> the LoongArch native code calling gdbarch_remove_non_address_bits
>>>> here without even registering a function for the gdbarch hook to call?
>>>
>>> Right, I agree.
>>>
>>>> Without this hook, we'll invoke the dummy implementation in gdb/arch-
>>>> utils.c:default_remove_non_address_bits, and that just returns the
>>>> unmodified pointer.
>>>
>>> True, that's probably why nobody noticed before.
>>>
>>>> This is a question for the LoongArch maintainer (cc-ed). If we don't
>>>> hear back, we should probably change the LoongArch code to not call
>>>> gdbarch_remove_non_address_bits. I think that was the original
>>>> intention, but there might've been a copy/paste error somewhere.
>>>
>>> Ok. Let's wait a bit. Thanks for the detailed feedback.
>>
>> Alternatively, if you could replace, in the LoongArch code,
>> gdbarch_remove_non_address_bits with one of the three new hooks, that should
>> work as well. I think even cleaner would be to just drop the hook for Loongarch
>> and assign siginfo.si_addr directly to addr_trap in the code.
>
> I am also in favour of your second suggestion to drop the hook here.
> In that case, it should be separate patch I think.
> I could post it independently of this series and quickly merge before posting a v7 of my series.
Yes, a separate one would be nice, please.
>
> LoongArch doesn't supported tagged addresses, does it? If it's not supported,
> I would suggest to fix it as follows:
>
> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> index dc4a019614a..fd3581bbd30 100644
> --- a/gdb/loongarch-linux-nat.c
> +++ b/gdb/loongarch-linux-nat.c
> @@ -608,17 +608,11 @@ loongarch_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
> if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != TRAP_HWBKPT)
> return false;
>
> - /* Make sure to ignore the top byte, otherwise we may not recognize a
> - hardware watchpoint hit. The stopped data addresses coming from the
> - kernel can potentially be tagged addresses. */
> - struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> - const CORE_ADDR addr_trap
> - = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
> -
> /* Check if the address matches any watched address. */
> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
>
> - return loongarch_stopped_data_address (state, addr_trap, addr_p);
> + return
> + loongarch_stopped_data_address (state, (CORE_ADDR) siginfo.si_addr, addr_p);
>
> Does that make sense to you?
>
It does. Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
2024-11-05 15:14 ` Luis Machado
@ 2024-11-05 15:36 ` Schimpe, Christina
0 siblings, 0 replies; 12+ messages in thread
From: Schimpe, Christina @ 2024-11-05 15:36 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: Tiezhu Yang
> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Tuesday, November 5, 2024 4:14 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
>
> On 11/5/24 15:09, Schimpe, Christina wrote:
> >> -----Original Message-----
> >> From: Luis Machado <luis.machado@arm.com>
> >> Sent: Tuesday, November 5, 2024 3:51 PM
> >> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >> patches@sourceware.org
> >> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> >> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
> >>
> >> On 11/5/24 14:40, Schimpe, Christina wrote:
> >>>> -----Original Message-----
> >>>> From: Luis Machado <luis.machado@arm.com>
> >>>> Sent: Tuesday, November 5, 2024 3:22 PM
> >>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >>>> patches@sourceware.org
> >>>> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> >>>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
> >>>>
> >>>> On 11/5/24 14:07, Schimpe, Christina wrote:
> >>>>> Hi Luis,
> >>>>>
> >>>>> Thanks a lot for the review. I have one questions to your comment,
> >>>>> please see
> >>>> below.
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Luis Machado <luis.machado@arm.com>
> >>>>>> Sent: Monday, November 4, 2024 12:18 PM
> >>>>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >>>>>> patches@sourceware.org
> >>>>>> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support
> configurable.
> >>>>>>
> >>>>>> Hi Christina,
> >>>>>>
> >>>>>> On 10/28/24 08:46, Schimpe, Christina wrote:
> >>>>>>> From: Christina Schimpe <christina.schimpe@intel.com>
> >>>>>>>
> >>>>>>> The gdbarch function gdbarch_remove_non_address_bits adjusts
> >>>>>>> addresses to enable debugging of programs with tagged pointers
> >>>>>>> on Linux, for instance for ARM's feature top byte ignore (TBI).
> >>>>>>> Once the function is implemented for an architecture, it adjusts
> >>>>>>> addresses for memory access, breakpoints and watchpoints.
> >>>>>>>
> >>>>>>> Linear address masking (LAM) is Intel's (R) implementation of
> >>>>>>> tagged pointer support. It requires certain adaptions to GDB's
> >>>>>>> tagged pointer support due to the following:
> >>>>>>> - LAM supports address tagging for data accesses only. Thus, specifying
> >>>>>>> breakpoints on tagged addresses is not a valid use case.
> >>>>>>> - In contrast to the implementation for ARM's TBI, the Linux
> >>>>>>> kernel
> >> supports
> >>>>>>> tagged pointers for memory access.
> >>>>>>>
> >>>>>>> This patch makes GDB's tagged pointer support configurable such
> >>>>>>> that it is possible to enable the address adjustment for a
> >>>>>>> specific feature only (e.g memory access, breakpoints or
> >>>>>>> watchpoints). This way, one can make sure that addresses are
> >>>>>>> only adjusted when necessary. In case of LAM, this avoids
> >>>>>>> unnecessary parsing of the /proc/<pid>/status file to get the untag
> mask.
> >>>>>>>
> >>>>>>> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
> >>>>>>> ---
> >>>>>>> gdb/aarch64-linux-nat.c | 2 +-
> >>>>>>> gdb/aarch64-linux-tdep.c | 7 +++--
> >>>>>>> gdb/aarch64-tdep.c | 23 ++++++++------
> >>>>>>> gdb/aarch64-tdep.h | 6 ++++
> >>>>>>> gdb/breakpoint.c | 5 +--
> >>>>>>> gdb/gdbarch-gen.c | 66 ++++++++++++++++++++++++++++++++---
> ---
> >> -
> >>>>>>> gdb/gdbarch-gen.h | 49 ++++++++++++++++++++++-------
> >>>>>>> gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
> >>>>>>> gdb/loongarch-linux-nat.c | 2 +-
> >>>>>>> gdb/target.c | 3 +-
> >>>>>>> 10 files changed, 169 insertions(+), 47 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> >>>>>>> index
> >>>>>>> 0fa5bee500b..48ab765d880 100644
> >>>>>>> --- a/gdb/aarch64-linux-nat.c
> >>>>>>> +++ b/gdb/aarch64-linux-nat.c
> >>>>>>> @@ -943,7 +943,7 @@
> >>>>>>> aarch64_linux_nat_target::stopped_data_address
> >>>>>> (CORE_ADDR *addr_p)
> >>>>>>> kernel can potentially be tagged addresses. */
> >>>>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >>>>>>> const CORE_ADDR addr_trap
> >>>>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>>>> siginfo.si_addr);
> >>>>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>>>>> + siginfo.si_addr);
> >>>>>>>
> >>>>>>> /* Check if the address matches any watched address. */
> >>>>>>> state = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> >>>>>>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> >>>>>>> index
> >>>>>>> c608a84bc71..61c3be8b6f0 100644
> >>>>>>> --- a/gdb/aarch64-linux-tdep.c
> >>>>>>> +++ b/gdb/aarch64-linux-tdep.c
> >>>>>>> @@ -2433,7 +2433,7 @@ static bool
> >>>>>>> aarch64_linux_tagged_address_p (struct gdbarch *gdbarch,
> >>>>>>> CORE_ADDR
> >>>>>>> address) {
> >>>>>>> /* Remove the top byte for the memory range check. */
> >>>>>>> - address = gdbarch_remove_non_address_bits (gdbarch, address);
> >>>>>>> + address = aarch64_remove_non_address_bits (gdbarch, address);
> >>>>>>>
> >>>>>>> /* Check if the page that contains ADDRESS is mapped with
> PROT_MTE.
> >> */
> >>>>>>> if (!linux_address_in_memtag_page (address)) @@ -2491,8
> >>>>>>> +2491,9 @@ aarch64_linux_report_signal_info (struct gdbarch
> *gdbarch,
> >>>>>>> uiout->text ("\n");
> >>>>>>>
> >>>>>>> std::optional<CORE_ADDR> atag
> >>>>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits
> (gdbarch,
> >>>>>>> -
> fault_addr));
> >>>>>>> + = aarch64_mte_get_atag (
> >>>>>>> + aarch64_remove_non_address_bits (gdbarch, fault_addr));
> >>>>>>> +
> >>>>>>> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
> >>>>>>>
> >>>>>>> if (!atag.has_value ())
> >>>>>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> >>>>>>> 8a2a9b1e23c..91fec7879ed 100644
> >>>>>>> --- a/gdb/aarch64-tdep.c
> >>>>>>> +++ b/gdb/aarch64-tdep.c
> >>>>>>> @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct
> gdbarch
> >>>>>>> *gdbarch,
> >>>>>>>
> >>>>>>> /* Fetch the allocation tag for ADDRESS. */
> >>>>>>> std::optional<CORE_ADDR> atag
> >>>>>>> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits
> >> (gdbarch,
> >>>>>> addr));
> >>>>>>> + = aarch64_mte_get_atag (aarch64_remove_non_address_bits
> >>>>>>> + (gdbarch, addr));
> >>>>>>>
> >>>>>>> if (!atag.has_value ())
> >>>>>>> return true;
> >>>>>>> @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch
> >>>>>>> *gdbarch,
> >>>>>> struct value *address,
> >>>>>>> else
> >>>>>>> {
> >>>>>>> /* Remove the top byte. */
> >>>>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> >>>>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >>>>>>>
> >>>>>>> /* With G being the number of tag granules and N the number of
> tags
> >>>>>>> passed in, we can have the following cases:
> >>>>>>> @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch
> >> *gdbarch,
> >>>>>> struct value *address,
> >>>>>>> else
> >>>>>>> {
> >>>>>>> /* Remove the top byte. */
> >>>>>>> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> >>>>>>> + addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >>>>>>> std::optional<CORE_ADDR> atag = aarch64_mte_get_atag
> >>>>>>> (addr);
> >>>>>>>
> >>>>>>> if (!atag.has_value ())
> >>>>>>> @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
> >>>>>> *gdbarch, struct value *tag_value)
> >>>>>>> return string_printf ("0x%s", phex_nz (tag, sizeof (tag)));
> >>>>>>> }
> >>>>>>>
> >>>>>>> -/* AArch64 implementation of the remove_non_address_bits
> >>>>>>> gdbarch
> >> hook.
> >>>>>> Remove
> >>>>>>> - non address bits from a pointer value. */
> >>>>>>> +/* See aarch64-tdep.h. */
> >>>>>>>
> >>>>>>> -static CORE_ADDR
> >>>>>>> +CORE_ADDR
> >>>>>>> aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>>>> CORE_ADDR
> >>>>>>> pointer) {
> >>>>>>> /* By default, we assume TBI and discard the top 8 bits plus
> >>>>>>> the VA range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct
> >>>>>>> gdbarch_info
> >>>>>> info, struct gdbarch_list *arches)
> >>>>>>> tdep->ra_sign_state_regnum = ra_sign_state_offset +
> >>>>>>> num_regs;
> >>>>>>>
> >>>>>>> /* Architecture hook to remove bits of a pointer that are not part of
> the
> >>>>>>> - address, like memory tags (MTE) and pointer authentication
> signatures.
> >>>> */
> >>>>>>> - set_gdbarch_remove_non_address_bits (gdbarch,
> >>>>>>> -
> aarch64_remove_non_address_bits);
> >>>>>>> + address, like memory tags (MTE) and pointer authentication
> >> signatures.
> >>>>>>> + Configure address adjustment for watch-, breakpoints and
> >>>>>>> + memory
> >>>>>>
> >>>>>> s/watch-/watchpoints
> >>>>>
> >>>>> Fill fix, thanks.
> >>>>>
> >>>>>>> + transfer. */
> >>>>>>> + set_gdbarch_remove_non_address_bits_watchpoint
> >>>>>>> + (gdbarch, aarch64_remove_non_address_bits);
> >>>>>>> + set_gdbarch_remove_non_address_bits_breakpoint
> >>>>>>> + (gdbarch, aarch64_remove_non_address_bits);
> >>>>>>> + set_gdbarch_remove_non_address_bits_memory
> >>>>>>> + (gdbarch, aarch64_remove_non_address_bits);
> >>>>>>>
> >>>>>>> /* SME pseudo-registers. */
> >>>>>>> if (tdep->has_sme ())
> >>>>>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
> >>>>>>> 50166fb4f24..13ea37de7a3 100644
> >>>>>>> --- a/gdb/aarch64-tdep.h
> >>>>>>> +++ b/gdb/aarch64-tdep.h
> >>>>>>> @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep
> >>>>>>> (struct gdbarch *gdbarch);
> >>>>>>>
> >>>>>>> std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR
> >>>>>>> address);
> >>>>>>>
> >>>>>>> +/* AArch64 implementation of the remove_non_address_bits
> >>>>>>> +gdbarch
> >>>> hooks.
> >>>>>>> + Remove non address bits from a pointer value. */
> >>>>>>> +
> >>>>>>> +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch
> >> *gdbarch,
> >>>>>>> + CORE_ADDR pointer);
> >>>>>>> +
> >>>>>>> #endif /* aarch64-tdep.h */
> >>>>>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
> >>>>>>> b7e4f5d0a45..9983e905e1b 100644
> >>>>>>> --- a/gdb/breakpoint.c
> >>>>>>> +++ b/gdb/breakpoint.c
> >>>>>>> @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b,
> >>>>>>> bool
> >>>>>> reparse)
> >>>>>>> loc->gdbarch = v->type ()->arch ();
> >>>>>>> loc->pspace = wp_pspace;
> >>>>>>> loc->address
> >>>>>>> - = gdbarch_remove_non_address_bits (loc->gdbarch,
> addr);
> >>>>>>> + = gdbarch_remove_non_address_bits_watchpoint
> (loc-
> >>>>>>> gdbarch,
> >>>>>>> +
> addr);
> >>>>>>> b->add_location (*loc);
> >>>>>>>
> >>>>>>> if (bitsize != 0)
> >>>>>>> @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
> >>>>>> *gdbarch,
> >>>>>>> }
> >>>>>>>
> >>>>>>> adjusted_bpaddr
> >>>>>>> - = gdbarch_remove_non_address_bits (gdbarch,
> adjusted_bpaddr);
> >>>>>>> + = gdbarch_remove_non_address_bits_breakpoint (gdbarch,
> >>>>>>> +adjusted_bpaddr);
> >>>>>>>
> >>>>>>> /* An adjusted breakpoint address can significantly alter
> >>>>>>> a user's expectations. Print a warning if an adjustment diff
> >>>>>>> --git a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index
> >>>>>>> 0d00cd7c993..d05c7a3cbdf
> >>>>>>> 100644
> >>>>>>> --- a/gdb/gdbarch-gen.c
> >>>>>>> +++ b/gdb/gdbarch-gen.c
> >>>>>>> @@ -143,7 +143,9 @@ struct gdbarch
> >>>>>>> int frame_red_zone_size = 0;
> >>>>>>> gdbarch_convert_from_func_ptr_addr_ftype
> >>>>>>> *convert_from_func_ptr_addr =
> >>>>>> convert_from_func_ptr_addr_identity;
> >>>>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove =
> >>>>>>> core_addr_identity;
> >>>>>>> - gdbarch_remove_non_address_bits_ftype
> >>>>>>> *remove_non_address_bits = default_remove_non_address_bits;
> >>>>>>> + gdbarch_remove_non_address_bits_watchpoint_ftype
> >>>>>>> + *remove_non_address_bits_watchpoint =
> >>>>>>> + default_remove_non_address_bits;
> >>>>>>> + gdbarch_remove_non_address_bits_breakpoint_ftype
> >>>>>>> + *remove_non_address_bits_breakpoint =
> >>>>>>> + default_remove_non_address_bits;
> >>>>>>> + gdbarch_remove_non_address_bits_memory_ftype
> >>>>>>> + *remove_non_address_bits_memory =
> >>>>>>> + default_remove_non_address_bits;
> >>>>>>> gdbarch_memtag_to_string_ftype *memtag_to_string =
> >>>>>> default_memtag_to_string;
> >>>>>>> gdbarch_tagged_address_p_ftype *tagged_address_p =
> >>>>>> default_tagged_address_p;
> >>>>>>> gdbarch_memtag_matches_p_ftype *memtag_matches_p =
> >>>>>>> default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch
> >>>>>>> (struct
> >>>>>> gdbarch *gdbarch)
> >>>>>>> /* Skip verify of frame_red_zone_size, invalid_p == 0. */
> >>>>>>> /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0. */
> >>>>>>> /* Skip verify of addr_bits_remove, invalid_p == 0. */
> >>>>>>> - /* Skip verify of remove_non_address_bits, invalid_p == 0.
> >>>>>>> */
> >>>>>>> + /* Skip verify of remove_non_address_bits_watchpoint,
> >>>>>>> + invalid_p == 0. */
> >>>>>>> + /* Skip verify of remove_non_address_bits_breakpoint,
> >>>>>>> + invalid_p == 0. */
> >>>>>>> + /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
> >>>>>>> + */
> >>>>>>> /* Skip verify of memtag_to_string, invalid_p == 0. */
> >>>>>>> /* Skip verify of tagged_address_p, invalid_p == 0. */
> >>>>>>> /* Skip verify of memtag_matches_p, invalid_p == 0. */ @@
> >>>>>>> -910,8
> >>>>>>> +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file
> >>>>>>> +*file)
> >>>>>>> "gdbarch_dump: addr_bits_remove = <%s>\n",
> >>>>>>> host_address_to_string (gdbarch->addr_bits_remove));
> >>>>>>> gdb_printf (file,
> >>>>>>> - "gdbarch_dump: remove_non_address_bits = <%s>\n",
> >>>>>>> - host_address_to_string (gdbarch-
> >remove_non_address_bits));
> >>>>>>> + "gdbarch_dump: remove_non_address_bits_watchpoint =
> <%s>\n",
> >>>>>>> + host_address_to_string
> >>>>>>> +(gdbarch->remove_non_address_bits_watchpoint));
> >>>>>>> + gdb_printf (file,
> >>>>>>> + "gdbarch_dump: remove_non_address_bits_breakpoint =
> <%s>\n",
> >>>>>>> + host_address_to_string
> >>>>>>> +(gdbarch->remove_non_address_bits_breakpoint));
> >>>>>>> + gdb_printf (file,
> >>>>>>> + "gdbarch_dump: remove_non_address_bits_memory =
> <%s>\n",
> >>>>>>> + host_address_to_string
> >>>>>>> +(gdbarch->remove_non_address_bits_memory));
> >>>>>>> gdb_printf (file,
> >>>>>>> "gdbarch_dump: memtag_to_string = <%s>\n",
> >>>>>>> host_address_to_string (gdbarch->memtag_to_string)); @@
> >>>>>>> -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct
> >>>>>>> gdbarch *gdbarch, }
> >>>>>>>
> >>>>>>> CORE_ADDR
> >>>>>>> -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>>>> CORE_ADDR
> >>>>>>> pointer)
> >>>>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>>>>> +*gdbarch, CORE_ADDR pointer) {
> >>>>>>> + gdb_assert (gdbarch != NULL);
> >>>>>>> + gdb_assert (gdbarch->remove_non_address_bits_watchpoint !=
> >>>>>>> +NULL);
> >>>>>>> + if (gdbarch_debug >= 2)
> >>>>>>> + gdb_printf (gdb_stdlog,
> >>>>>>> +"gdbarch_remove_non_address_bits_watchpoint called\n");
> >>>>>>> + return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
> >>>>>>> +pointer); }
> >>>>>>> +
> >>>>>>> +void
> >>>>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>>>> *gdbarch,
> >>>>>>> +
> >>>>>> gdbarch_remove_non_address_bits_watchpoint_ftype
> >>>>>>> +remove_non_address_bits_watchpoint)
> >>>>>>> +{
> >>>>>>> + gdbarch->remove_non_address_bits_watchpoint =
> >>>>>>> +remove_non_address_bits_watchpoint;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +CORE_ADDR
> >>>>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>>>> +*gdbarch, CORE_ADDR pointer) {
> >>>>>>> + gdb_assert (gdbarch != NULL);
> >>>>>>> + gdb_assert (gdbarch->remove_non_address_bits_breakpoint !=
> >>>>>>> +NULL);
> >>>>>>> + if (gdbarch_debug >= 2)
> >>>>>>> + gdb_printf (gdb_stdlog,
> >>>>>>> +"gdbarch_remove_non_address_bits_breakpoint called\n");
> >>>>>>> + return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
> >>>>>>> +pointer); }
> >>>>>>> +
> >>>>>>> +void
> >>>>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>>>> +*gdbarch,
> >>>>>>> +
> >>>>>> gdbarch_remove_non_address_bits_breakpoint_ftype
> >>>>>>> +remove_non_address_bits_breakpoint)
> >>>>>>> +{
> >>>>>>> + gdbarch->remove_non_address_bits_breakpoint =
> >>>>>>> +remove_non_address_bits_breakpoint;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +CORE_ADDR
> >>>>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>>>>>> +*gdbarch, CORE_ADDR pointer)
> >>>>>>> {
> >>>>>>> gdb_assert (gdbarch != NULL);
> >>>>>>> - gdb_assert (gdbarch->remove_non_address_bits != NULL);
> >>>>>>> + gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
> >>>>>>> if (gdbarch_debug >= 2)
> >>>>>>> - gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits
> called\n");
> >>>>>>> - return gdbarch->remove_non_address_bits (gdbarch, pointer);
> >>>>>>> + gdb_printf (gdb_stdlog,
> >>>>>>> + "gdbarch_remove_non_address_bits_memory
> >>>>>>> + called\n"); return gdbarch->remove_non_address_bits_memory
> >>>>>>> + (gdbarch, pointer);
> >>>>>>> }
> >>>>>>>
> >>>>>>> void
> >>>>>>> -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>>>> -
> gdbarch_remove_non_address_bits_ftype
> >>>>>> remove_non_address_bits)
> >>>>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>>>>>> +*gdbarch,
> >>>>>>> +
> >>>>>> gdbarch_remove_non_address_bits_memory_ftype
> >>>>>>> +remove_non_address_bits_memory)
> >>>>>>> {
> >>>>>>> - gdbarch->remove_non_address_bits = remove_non_address_bits;
> >>>>>>> + gdbarch->remove_non_address_bits_memory =
> >>>>>>> + remove_non_address_bits_memory;
> >>>>>>> }
> >>>>>>>
> >>>>>>> std::string
> >>>>>>> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
> >>>>>>> b982fd7cd09..9fda85f860f 100644
> >>>>>>> --- a/gdb/gdbarch-gen.h
> >>>>>>> +++ b/gdb/gdbarch-gen.h
> >>>>>>> @@ -684,19 +684,46 @@ extern CORE_ADDR
> gdbarch_addr_bits_remove
> >>>>>>> (struct gdbarch *gdbarch, CORE_ADDR ad extern void
> >>>>>>> set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
> >>>>>>> gdbarch_addr_bits_remove_ftype *addr_bits_remove);
> >>>>>>>
> >>>>>>> /* On some architectures, not all bits of a pointer are significant.
> >>>>>>> - On AArch64, for example, the top bits of a pointer may carry a "tag",
> >> which
> >>>>>>> - can be ignored by the kernel and the hardware. The "tag" can be
> >> regarded
> >>>> as
> >>>>>>> - additional data associated with the pointer, but it is not part of the
> >>>> address.
> >>>>>>> + On AArch64 and amd64, for example, the top bits of a pointer
> >>>>>>> + may carry
> >> a
> >>>>>>> + "tag", which can be ignored by the kernel and the hardware.
> >>>>>>> + The "tag" can
> >>>>>> be
> >>>>>>> + regarded as additional data associated with the pointer, but
> >>>>>>> + it is not
> >> part
> >>>>>>> + of the address.
> >>>>>>>
> >>>>>>> Given a pointer for the architecture, this hook removes all the
> >>>>>>> - non-significant bits and sign-extends things as needed. It gets used to
> >>>>>> remove
> >>>>>>> - non-address bits from data pointers (for example, removing the
> AArch64
> >>>> MTE
> >>>>>> tag
> >>>>>>> - bits from a pointer) and from code pointers (removing the AArch64
> PAC
> >>>>>> signature
> >>>>>>> - from a pointer containing the return address). */
> >>>>>>> -
> >>>>>>> -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype)
> >>>>>>> (struct gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
> >>>>>>> gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>>>> CORE_ADDR pointer); -extern void
> >>>>>>> set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> >>>>>>> gdbarch_remove_non_address_bits_ftype
> >>>>>>> *remove_non_address_bits);
> >>>>>>> + non-significant bits and sign-extends things as needed. It gets used
> to
> >>>>>>> + remove non-address bits from pointers used for watchpoints.
> >>>>>>> + */
> >>>>>>> +
> >>>>>>> +typedef CORE_ADDR
> >>>>>>> +(gdbarch_remove_non_address_bits_watchpoint_ftype)
> >>>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>>>>>> +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>>>>> +*gdbarch, CORE_ADDR pointer); extern void
> >>>>>>> +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> >>>>>>> +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
> >>>>>>> +*remove_non_address_bits_watchpoint);
> >>>>>>> +
> >>>>>>> +/* On some architectures, not all bits of a pointer are significant.
> >>>>>>> + On AArch64 and amd64, for example, the top bits of a pointer
> >>>>>>> +may carry
> >> a
> >>>>>>> + "tag", which can be ignored by the kernel and the hardware.
> >>>>>>> +The "tag" can
> >>>>>> be
> >>>>>>> + regarded as additional data associated with the pointer, but
> >>>>>>> + it is not
> >> part
> >>>>>>> + of the address.
> >>>>>>> +
> >>>>>>> + Given a pointer for the architecture, this hook removes all the
> >>>>>>> + non-significant bits and sign-extends things as needed. It gets used
> to
> >>>>>>> + remove non-address bits from pointers used for breakpoints.
> >>>>>>> + */
> >>>>>>> +
> >>>>>>> +typedef CORE_ADDR
> >>>>>>> +(gdbarch_remove_non_address_bits_breakpoint_ftype)
> >>>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>>>>>> +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>>>> +*gdbarch, CORE_ADDR pointer); extern void
> >>>>>>> +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> >>>>>>> +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
> >>>>>>> +*remove_non_address_bits_breakpoint);
> >>>>>>> +
> >>>>>>> +/* On some architectures, not all bits of a pointer are significant.
> >>>>>>> + On AArch64 and amd64, for example, the top bits of a pointer
> >>>>>>> +may carry
> >> a
> >>>>>>> + "tag", which can be ignored by the kernel and the hardware.
> >>>>>>> +The "tag" can
> >>>>>> be
> >>>>>>> + regarded as additional data associated with the pointer, but
> >>>>>>> + it is not
> >> part
> >>>>>>> + of the address.
> >>>>>>> +
> >>>>>>> + Given a pointer for the architecture, this hook removes all the
> >>>>>>> + non-significant bits and sign-extends things as needed. It gets used
> to
> >>>>>>> + remove non-address bits from any pointer used to access memory.
> >>>>>>> + */
> >>>>>>> +
> >>>>>>> +typedef CORE_ADDR
> >> (gdbarch_remove_non_address_bits_memory_ftype)
> >>>>>>> +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> >>>>>>> +gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>>>>>> +*gdbarch, CORE_ADDR pointer); extern void
> >>>>>>> +set_gdbarch_remove_non_address_bits_memory (struct gdbarch
> >>>>>>> +*gdbarch, gdbarch_remove_non_address_bits_memory_ftype
> >>>>>>> +*remove_non_address_bits_memory);
> >>>>>>>
> >>>>>>> /* Return a string representation of the memory tag TAG. */
> >>>>>>>
> >>>>>>> diff --git a/gdb/gdbarch_components.py
> >>>>>>> b/gdb/gdbarch_components.py index 4006380076d..cc7c6d8677b
> >>>>>>> 100644
> >>>>>>> --- a/gdb/gdbarch_components.py
> >>>>>>> +++ b/gdb/gdbarch_components.py
> >>>>>>> @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
> >>>>>> instead).
> >>>>>>> Method(
> >>>>>>> comment="""
> >>>>>>> On some architectures, not all bits of a pointer are significant.
> >>>>>>> -On AArch64, for example, the top bits of a pointer may carry a
> >>>>>>> "tag", which -can be ignored by the kernel and the hardware.
> >>>>>>> The "tag" can be regarded as -additional data associated with
> >>>>>>> the pointer, but it is not part of
> >>>>>> the address.
> >>>>>>> +On AArch64 and amd64, for example, the top bits of a pointer
> >>>>>>> +may carry a "tag", which can be ignored by the kernel and the
> hardware.
> >>>>>>> +The "tag" can be regarded as additional data associated with
> >>>>>>> +the pointer, but it is not part of the address.
> >>>>>>>
> >>>>>>> Given a pointer for the architecture, this hook removes all the
> >>>>>>> -non-significant bits and sign-extends things as needed. It
> >>>>>>> gets used to remove -non-address bits from data pointers (for
> >>>>>>> example, removing the AArch64 MTE tag -bits from a pointer) and
> >>>>>>> from code pointers (removing the AArch64 PAC signature -from a
> >>>>>>> pointer containing the return
> >>>>>> address).
> >>>>>>> +non-significant bits and sign-extends things as needed. It
> >>>>>>> +gets used to remove non-address bits from pointers used for
> watchpoints.
> >>>>>>> """,
> >>>>>>> type="CORE_ADDR",
> >>>>>>> - name="remove_non_address_bits",
> >>>>>>> + name="remove_non_address_bits_watchpoint",
> >>>>>>> + params=[("CORE_ADDR", "pointer")],
> >>>>>>> + predefault="default_remove_non_address_bits",
> >>>>>>> + invalid=False,
> >>>>>>> +)
> >>>>>>> +
> >>>>>>> +Method(
> >>>>>>> + comment="""
> >>>>>>> +On some architectures, not all bits of a pointer are significant.
> >>>>>>> +On AArch64 and amd64, for example, the top bits of a pointer
> >>>>>>> +may carry a "tag", which can be ignored by the kernel and the
> hardware.
> >>>>>>> +The "tag" can be regarded as additional data associated with
> >>>>>>> +the pointer, but it is not part of the address.
> >>>>>>> +
> >>>>>>> +Given a pointer for the architecture, this hook removes all the
> >>>>>>> +non-significant bits and sign-extends things as needed. It
> >>>>>>> +gets used to remove non-address bits from pointers used for
> breakpoints.
> >>>>>>> +""",
> >>>>>>> + type="CORE_ADDR",
> >>>>>>> + name="remove_non_address_bits_breakpoint",
> >>>>>>> + params=[("CORE_ADDR", "pointer")],
> >>>>>>> + predefault="default_remove_non_address_bits",
> >>>>>>> + invalid=False,
> >>>>>>> +)
> >>>>>>> +
> >>>>>>> +Method(
> >>>>>>> + comment="""
> >>>>>>> +On some architectures, not all bits of a pointer are significant.
> >>>>>>> +On AArch64 and amd64, for example, the top bits of a pointer
> >>>>>>> +may carry a "tag", which can be ignored by the kernel and the
> hardware.
> >>>>>>> +The "tag" can be regarded as additional data associated with
> >>>>>>> +the pointer, but it is not part of the address.
> >>>>>>> +
> >>>>>>> +Given a pointer for the architecture, this hook removes all the
> >>>>>>> +non-significant bits and sign-extends things as needed. It
> >>>>>>> +gets used to remove non-address bits from any pointer used to
> >>>>>>> +access
> >> memory.
> >>>>>>> +""",
> >>>>>>> + type="CORE_ADDR",
> >>>>>>> + name="remove_non_address_bits_memory",
> >>>>>>> params=[("CORE_ADDR", "pointer")],
> >>>>>>> predefault="default_remove_non_address_bits",
> >>>>>>> invalid=False,
> >>>>>>> diff --git a/gdb/loongarch-linux-nat.c
> >>>>>>> b/gdb/loongarch-linux-nat.c index bc9927dd751..dc4a019614a
> >>>>>>> 100644
> >>>>>>> --- a/gdb/loongarch-linux-nat.c
> >>>>>>> +++ b/gdb/loongarch-linux-nat.c
> >>>>>>> @@ -613,7 +613,7 @@
> >>>>>>> loongarch_linux_nat_target::stopped_data_address
> >>>>>> (CORE_ADDR *addr_p)
> >>>>>>> kernel can potentially be tagged addresses. */
> >>>>>>> struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >>>>>>> const CORE_ADDR addr_trap
> >>>>>>> - = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>>>> siginfo.si_addr);
> >>>>>>> + = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> >>>>>>> + siginfo.si_addr);
> >>>>>>>
> >>>>>>> /* Check if the address matches any watched address. */
> >>>>>>> state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
> >>>>>>
> >>>>>> I think the loongarch-linux-nat.c change is spurious. Could you
> >>>>>> please address that?
> >>>>>
> >>>>> Why is it spurious? What do you mean exactly?
> >>>>
> >>>> The intention behind this change isn't clear to me, but the
> >>>> Loongson native Linux layer using a AArch64 architecture function does not
> make sense.
> >>>>
> >>>> I haven't built a LoongArch native Linux target, but I'd guess that
> >>>> it will run into a build error with this change, if built for LoongArch only.
> >>>>
> >>>> The right approach, in my mind, is to drop this change, but you
> >>>> have to touch this code due to your changes. It begs the question,
> >>>> why is the LoongArch native code calling
> >>>> gdbarch_remove_non_address_bits here without even registering a
> function for the gdbarch hook to call?
> >>>
> >>> Right, I agree.
> >>>
> >>>> Without this hook, we'll invoke the dummy implementation in
> >>>> gdb/arch- utils.c:default_remove_non_address_bits, and that just
> >>>> returns the unmodified pointer.
> >>>
> >>> True, that's probably why nobody noticed before.
> >>>
> >>>> This is a question for the LoongArch maintainer (cc-ed). If we
> >>>> don't hear back, we should probably change the LoongArch code to
> >>>> not call gdbarch_remove_non_address_bits. I think that was the
> >>>> original intention, but there might've been a copy/paste error somewhere.
> >>>
> >>> Ok. Let's wait a bit. Thanks for the detailed feedback.
> >>
> >> Alternatively, if you could replace, in the LoongArch code,
> >> gdbarch_remove_non_address_bits with one of the three new hooks, that
> >> should work as well. I think even cleaner would be to just drop the
> >> hook for Loongarch and assign siginfo.si_addr directly to addr_trap in the code.
> >
> > I am also in favour of your second suggestion to drop the hook here.
> > In that case, it should be separate patch I think.
> > I could post it independently of this series and quickly merge before posting a v7
> of my series.
>
> Yes, a separate one would be nice, please.
>
> >
> > LoongArch doesn't supported tagged addresses, does it? If it's not
> > supported, I would suggest to fix it as follows:
> >
> > diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> > index dc4a019614a..fd3581bbd30 100644
> > --- a/gdb/loongarch-linux-nat.c
> > +++ b/gdb/loongarch-linux-nat.c
> > @@ -608,17 +608,11 @@ loongarch_linux_nat_target::stopped_data_address
> (CORE_ADDR *addr_p)
> > if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != TRAP_HWBKPT)
> > return false;
> >
> > - /* Make sure to ignore the top byte, otherwise we may not recognize a
> > - hardware watchpoint hit. The stopped data addresses coming from the
> > - kernel can potentially be tagged addresses. */
> > - struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> > - const CORE_ADDR addr_trap
> > - = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> siginfo.si_addr);
> > -
> > /* Check if the address matches any watched address. */
> > state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
> >
> > - return loongarch_stopped_data_address (state, addr_trap, addr_p);
> > + return
> > + loongarch_stopped_data_address (state, (CORE_ADDR)
> > + siginfo.si_addr, addr_p);
> >
> > Does that make sense to you?
> >
>
> It does. Thanks!
This is the patch for LoongArch: https://sourceware.org/pipermail/gdb-patches/2024-November/212974.html
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-11-05 15:36 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-28 8:46 [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
2024-10-28 8:46 ` [PATCH v6 1/2] gdb: Make tagged pointer support configurable Schimpe, Christina
2024-11-04 11:17 ` Luis Machado
2024-11-05 14:07 ` Schimpe, Christina
2024-11-05 14:22 ` Luis Machado
2024-11-05 14:40 ` Schimpe, Christina
2024-11-05 14:51 ` Luis Machado
2024-11-05 15:09 ` Schimpe, Christina
2024-11-05 15:14 ` Luis Machado
2024-11-05 15:36 ` Schimpe, Christina
2024-10-28 8:46 ` [PATCH v6 2/2] LAM: Enable tagged pointer support for watchpoints Schimpe, Christina
2024-11-04 8:37 ` [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).