From: Luis Machado <luis.machado@arm.com>
To: gdb-patches@sourceware.org
Cc: lsix@lancelotsix.com
Subject: [Ping v2][PATCH,v2] [aarch64] Fix removal of non-address bits for PAuth
Date: Mon, 1 Aug 2022 12:09:13 +0100 [thread overview]
Message-ID: <bf2265ad-4631-e774-aa6b-b504a4b289bb@arm.com> (raw)
In-Reply-To: <0a729c27-3f70-181a-f26d-14cbc0a2fdab@arm.com>
On 7/18/22 09:16, Luis Machado wrote:
> On 7/11/22 12:55, Luis Machado via Gdb-patches wrote:
>> v2:
>>
>> - Renamed aarch64_remove_top_bytes to aarch64_remove_top_bits
>> - Formatting issues.
>>
>> --
>>
>> The address_significant gdbarch setting was introduced as a way to remove
>> non-address bits from pointers, and it is specified by a constant. This
>> constant represents the number of address bits in a pointer.
>>
>> Right now AArch64 is the only architecture that uses it, and 56 was a
>> correct option so far.
>>
>> But if we are using Pointer Authentication (PAuth), we might use up to 2 bytes
>> from the address space to store the required information. We could also have
>> cases where we're using both PAuth and MTE.
>>
>> We could adjust the constant to 48 to cover those cases, but this doesn't
>> cover the case where GDB needs to sign-extend kernel addresses after removal
>> of the non-address bits.
>>
>> This has worked so far because bit 55 is used to select between kernel-space
>> and user-space addresses. But trying to clear a range of bits crossing the
>> bit 55 boundary requires the hook to be smarter.
>>
>> The following patch renames the gdbarch hook from significant_addr_bit to
>> remove_non_address_bits and passes a pointer as opposed to the number of
>> bits. The hook is now responsible for removing the required non-address bits
>> and sign-extending the address if needed.
>>
>> While at it, make GDB and GDBServer share some more code for aarch64 and add a
>> new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.
>>
>> Bug-url: https://sourceware.org/bugzilla/show_bug.cgi?id=28947
>> ---
>> gdb/aarch64-linux-nat.c | 2 +-
>> gdb/aarch64-linux-tdep.c | 57 ++++++++-
>> gdb/arch-utils.c | 8 ++
>> gdb/arch-utils.h | 4 +
>> gdb/arch/aarch64.c | 19 +++
>> gdb/arch/aarch64.h | 9 ++
>> gdb/breakpoint.c | 6 +-
>> gdb/gdbarch-components.py | 20 ++-
>> gdb/gdbarch-gen.h | 19 ++-
>> gdb/gdbarch.c | 25 ++--
>> gdb/target.c | 2 +-
>> .../gdb.arch/aarch64-non-address-bits.c | 25 ++++
>> .../gdb.arch/aarch64-non-address-bits.exp | 121 ++++++++++++++++++
>> gdb/utils.c | 22 ----
>> gdb/utils.h | 3 -
>> gdbserver/linux-aarch64-low.cc | 40 ++++--
>> 16 files changed, 309 insertions(+), 73 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-non-address-bits.c
>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp
>>
>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
>> index d58ad0143a2..3f41aae188e 100644
>> --- a/gdb/aarch64-linux-nat.c
>> +++ b/gdb/aarch64-linux-nat.c
>> @@ -843,7 +843,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
>> - = address_significant (gdbarch, (CORE_ADDR) siginfo.si_addr);
>> + = gdbarch_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 453692df2f4..fffeb6f3df7 100644
>> --- a/gdb/aarch64-linux-tdep.c
>> +++ b/gdb/aarch64-linux-tdep.c
>> @@ -1586,7 +1586,7 @@ aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, struct value *address)
>> CORE_ADDR addr = value_as_address (address);
>> /* Remove the top byte for the memory range check. */
>> - addr = address_significant (gdbarch, addr);
>> + addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>> /* Check if the page that contains ADDRESS is mapped with PROT_MTE. */
>> if (!linux_address_in_memtag_page (addr))
>> @@ -1612,7 +1612,7 @@ aarch64_linux_memtag_matches_p (struct gdbarch *gdbarch,
>> /* Fetch the allocation tag for ADDRESS. */
>> gdb::optional<CORE_ADDR> atag
>> - = aarch64_mte_get_atag (address_significant (gdbarch, addr));
>> + = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch, addr));
>> if (!atag.has_value ())
>> return true;
>> @@ -1651,7 +1651,7 @@ aarch64_linux_set_memtags (struct gdbarch *gdbarch, struct value *address,
>> else
>> {
>> /* Remove the top byte. */
>> - addr = address_significant (gdbarch, addr);
>> + addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>> /* Make sure we are dealing with a tagged address to begin with. */
>> if (!aarch64_linux_tagged_address_p (gdbarch, address))
>> @@ -1708,7 +1708,7 @@ aarch64_linux_get_memtag (struct gdbarch *gdbarch, struct value *address,
>> return nullptr;
>> /* Remove the top byte. */
>> - addr = address_significant (gdbarch, addr);
>> + addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>> gdb::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
>> if (!atag.has_value ())
>> @@ -1782,7 +1782,8 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>> uiout->text ("\n");
>> gdb::optional<CORE_ADDR> atag
>> - = aarch64_mte_get_atag (address_significant (gdbarch, fault_addr));
>> + = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
>> + fault_addr));
>> gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
>> if (!atag.has_value ())
>> @@ -1803,6 +1804,49 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>> }
>> }
>> +/* AArch64 implementation of the remove_non_address_bits gdbarch hook. Remove
>> + non address bits from a pointer value. */
>> +
>> +static CORE_ADDR
>> +aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
>> +{
>> + aarch64_gdbarch_tdep *tdep
>> + = (aarch64_gdbarch_tdep *) gdbarch_tdep (gdbarch);
>> +
>> + /* By default, we assume TBI and discard the top 8 bits plus the VA range
>> + select bit (55). */
>> + CORE_ADDR mask = (((CORE_ADDR) 1) << (64 - 9)) - 1;
>> + mask = ~mask;
>> +
>> + if (tdep->has_pauth ())
>> + {
>> + /* Fetch the PAC masks. These masks are per-process, so we can just
>> + fetch data from whatever thread we have at the moment.
>> +
>> + Also, we have both a code mask and a data mask. For now they are the
>> + same, but this may change in the future. */
>> + struct regcache *regs = get_current_regcache ();
>> + CORE_ADDR cmask, dmask;
>> +
>> + if (regs->cooked_read (tdep->pauth_reg_base, &dmask) != REG_VALID)
>> + dmask = mask;
>> +
>> + if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) != REG_VALID)
>> + cmask = mask;
>> +
>> + if (dmask != cmask)
>> + {
>> + /* Warn if the masks are different. */
>> + warning (_("C mask and D mask differ"));
>> + mask |= dmask > cmask? dmask : cmask;
>> + }
>> + else
>> + mask |= cmask;
>> + }
>> +
>> + return aarch64_remove_top_bits (pointer, mask);
>> +}
>> +
>> static void
>> aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> {
>> @@ -1858,7 +1902,8 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> /* The top byte of a user space address known as the "tag",
>> is ignored by the kernel and can be regarded as additional
>> data associated with the address. */
>> - set_gdbarch_significant_addr_bit (gdbarch, 56);
>> + set_gdbarch_remove_non_address_bits (gdbarch,
>> + aarch64_remove_non_address_bits);
>> /* MTE-specific settings and hooks. */
>> if (tdep->has_mte ())
>> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
>> index ff946ee3767..db77dc44c74 100644
>> --- a/gdb/arch-utils.c
>> +++ b/gdb/arch-utils.c
>> @@ -82,6 +82,14 @@ legacy_register_sim_regno (struct gdbarch *gdbarch, int regnum)
>> return LEGACY_SIM_REGNO_IGNORE;
>> }
>> +/* See arch-utils.h */
>> +
>> +CORE_ADDR
>> +default_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
>> +{
>> + /* By default, just return the pointer value. */
>> + return pointer;
>> +}
>> /* See arch-utils.h */
>> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
>> index f850e5fd6e7..e117599b171 100644
>> --- a/gdb/arch-utils.h
>> +++ b/gdb/arch-utils.h
>> @@ -132,6 +132,10 @@ extern const struct floatformat **
>> default_floatformat_for_type (struct gdbarch *gdbarch,
>> const char *name, int len);
>> +/* Default implementation of gdbarch_remove_non_address_bits. */
>> +CORE_ADDR default_remove_non_address_bits (struct gdbarch *gdbarch,
>> + CORE_ADDR pointer);
>> +
>> /* Default implementation of gdbarch_memtag_to_string. */
>> extern std::string default_memtag_to_string (struct gdbarch *gdbarch,
>> struct value *tag);
>> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
>> index 0f73286f145..120664b5fdf 100644
>> --- a/gdb/arch/aarch64.c
>> +++ b/gdb/arch/aarch64.c
>> @@ -58,3 +58,22 @@ aarch64_create_target_description (const aarch64_features &features)
>> return tdesc.release ();
>> }
>> +
>> +/* See arch/aarch64.h. */
>> +
>> +CORE_ADDR
>> +aarch64_remove_top_bits (CORE_ADDR pointer, CORE_ADDR mask)
>> +{
>> + /* The VA range select bit is 55. This bit tells us if we have a
>> + kernel-space address or a user-space address. */
>> + bool kernel_address = (pointer & VA_RANGE_SELECT_BIT_MASK) != 0;
>> +
>> + /* Remove the top non-address bits. */
>> + pointer &= ~mask;
>> +
>> + /* Sign-extend if we have a kernel-space address. */
>> + if (kernel_address)
>> + pointer |= mask;
>> +
>> + return pointer;
>> +}
>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>> index 8e3fd36726a..f2589666a1e 100644
>> --- a/gdb/arch/aarch64.h
>> +++ b/gdb/arch/aarch64.h
>> @@ -67,6 +67,12 @@ namespace std
>> target_desc *
>> aarch64_create_target_description (const aarch64_features &features);
>> +/* Given a pointer value POINTER and a MASK of non-address bits, remove the
>> + non-address bits from the pointer and sign-extend the result if required.
>> + The sign-extension is required so we can handle kernel addresses
>> + correctly. */
>> +CORE_ADDR aarch64_remove_top_bits (CORE_ADDR pointer, CORE_ADDR mask);
>> +
>> /* Register numbers of various important registers.
>> Note that on SVE, the Z registers reuse the V register numbers and the V
>> registers become pseudo registers. */
>> @@ -96,6 +102,9 @@ enum aarch64_regnum
>> AARCH64_LAST_V_ARG_REGNUM = AARCH64_V0_REGNUM + 7
>> };
>> +/* Bit 55 is used to select between a kernel-space and user-space address. */
>> +#define VA_RANGE_SELECT_BIT_MASK 0x80000000000000
>> +
>> #define V_REGISTER_SIZE 16
>> /* Pseudo register base numbers. */
>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> index a3be12557f6..c7ccb6c4f0d 100644
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -2113,7 +2113,8 @@ update_watchpoint (struct watchpoint *b, int reparse)
>> loc->gdbarch = value_type (v)->arch ();
>> loc->pspace = frame_pspace;
>> - loc->address = address_significant (loc->gdbarch, addr);
>> + loc->address
>> + = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
>> if (bitsize != 0)
>> {
>> @@ -7136,7 +7137,8 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
>> adjusted_bpaddr = gdbarch_adjust_breakpoint_address (gdbarch, bpaddr);
>> }
>> - adjusted_bpaddr = address_significant (gdbarch, adjusted_bpaddr);
>> + adjusted_bpaddr
>> + = gdbarch_remove_non_address_bits (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-components.py b/gdb/gdbarch-components.py
>> index fc10e8600ba..c6836b63c50 100644
>> --- a/gdb/gdbarch-components.py
>> +++ b/gdb/gdbarch-components.py
>> @@ -1116,15 +1116,21 @@ possible it should be in TARGET_READ_PC instead).
>> invalid=False,
>> )
>> -Value(
>> +Method(
>> comment="""
>> -On some machines, not all bits of an address word are significant.
>> -For example, on AArch64, the top bits of an address known as the "tag"
>> -are ignored by the kernel, the hardware, etc. and can be regarded as
>> -additional data associated with the address.
>> +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.
>> +
>> +Given a pointer for the architecture, this hook removes all the
>> +non-significant bits and sign-extends things as needed. It is used mostly
>> +for data pointers, as opposed to code pointers.
>> """,
>> - type="int",
>> - name="significant_addr_bit",
>> + type="CORE_ADDR",
>> + name="remove_non_address_bits",
>> + params=[("CORE_ADDR", "pointer")],
>> + predefault="default_remove_non_address_bits",
>> invalid=False,
>> )
>> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
>> index ddcb4c55615..64930f7f239 100644
>> --- a/gdb/gdbarch-gen.h
>> +++ b/gdb/gdbarch-gen.h
>> @@ -609,13 +609,18 @@ typedef CORE_ADDR (gdbarch_addr_bits_remove_ftype) (struct gdbarch *gdbarch, COR
>> extern CORE_ADDR gdbarch_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr);
>> extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_bits_remove_ftype *addr_bits_remove);
>> -/* On some machines, not all bits of an address word are significant.
>> - For example, on AArch64, the top bits of an address known as the "tag"
>> - are ignored by the kernel, the hardware, etc. and can be regarded as
>> - additional data associated with the address. */
>> -
>> -extern int gdbarch_significant_addr_bit (struct gdbarch *gdbarch);
>> -extern void set_gdbarch_significant_addr_bit (struct gdbarch *gdbarch, int significant_addr_bit);
>> +/* 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.
>> +
>> + Given a pointer for the architecture, this hook removes all the
>> + non-significant bits and sign-extends things as needed. It is used mostly
>> + for data pointers, as opposed to code pointers. */
>> +
>> +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);
>> /* Return a string representation of the memory tag TAG. */
>> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
>> index 68ef0480219..d536d8578af 100644
>> --- a/gdb/gdbarch.c
>> +++ b/gdb/gdbarch.c
>> @@ -140,7 +140,7 @@ struct gdbarch
>> int frame_red_zone_size;
>> gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr;
>> gdbarch_addr_bits_remove_ftype *addr_bits_remove;
>> - int significant_addr_bit;
>> + gdbarch_remove_non_address_bits_ftype *remove_non_address_bits;
>> gdbarch_memtag_to_string_ftype *memtag_to_string;
>> gdbarch_tagged_address_p_ftype *tagged_address_p;
>> gdbarch_memtag_matches_p_ftype *memtag_matches_p;
>> @@ -327,6 +327,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
>> gdbarch->stabs_argument_has_addr = default_stabs_argument_has_addr;
>> gdbarch->convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity;
>> gdbarch->addr_bits_remove = core_addr_identity;
>> + gdbarch->remove_non_address_bits = default_remove_non_address_bits;
>> gdbarch->memtag_to_string = default_memtag_to_string;
>> gdbarch->tagged_address_p = default_tagged_address_p;
>> gdbarch->memtag_matches_p = default_memtag_matches_p;
>> @@ -496,7 +497,7 @@ 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 significant_addr_bit, invalid_p == 0 */
>> + /* Skip verify of remove_non_address_bits, 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 */
>> @@ -974,8 +975,8 @@ 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: significant_addr_bit = %s\n",
>> - plongest (gdbarch->significant_addr_bit));
>> + "gdbarch_dump: remove_non_address_bits = <%s>\n",
>> + host_address_to_string (gdbarch->remove_non_address_bits));
>> gdb_printf (file,
>> "gdbarch_dump: memtag_to_string = <%s>\n",
>> host_address_to_string (gdbarch->memtag_to_string));
>> @@ -3147,21 +3148,21 @@ set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
>> gdbarch->addr_bits_remove = addr_bits_remove;
>> }
>> -int
>> -gdbarch_significant_addr_bit (struct gdbarch *gdbarch)
>> +CORE_ADDR
>> +gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
>> {
>> gdb_assert (gdbarch != NULL);
>> - /* Skip verify of significant_addr_bit, invalid_p == 0 */
>> + gdb_assert (gdbarch->remove_non_address_bits != NULL);
>> if (gdbarch_debug >= 2)
>> - gdb_printf (gdb_stdlog, "gdbarch_significant_addr_bit called\n");
>> - return gdbarch->significant_addr_bit;
>> + gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
>> + return gdbarch->remove_non_address_bits (gdbarch, pointer);
>> }
>> void
>> -set_gdbarch_significant_addr_bit (struct gdbarch *gdbarch,
>> - int significant_addr_bit)
>> +set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
>> + gdbarch_remove_non_address_bits_ftype remove_non_address_bits)
>> {
>> - gdbarch->significant_addr_bit = significant_addr_bit;
>> + gdbarch->remove_non_address_bits = remove_non_address_bits;
>> }
>> std::string
>> diff --git a/gdb/target.c b/gdb/target.c
>> index 18e53aa5d27..4ce1b9451a0 100644
>> --- a/gdb/target.c
>> +++ b/gdb/target.c
>> @@ -1644,7 +1644,7 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
>> if (len == 0)
>> return TARGET_XFER_EOF;
>> - memaddr = address_significant (target_gdbarch (), memaddr);
>> + memaddr = gdbarch_remove_non_address_bits (target_gdbarch (), memaddr);
>> /* Fill in READBUF with breakpoint shadows, or WRITEBUF with
>> breakpoint insns, thus hiding out from higher layers whether
>> diff --git a/gdb/testsuite/gdb.arch/aarch64-non-address-bits.c b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.c
>> new file mode 100644
>> index 00000000000..3cf6d63da17
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.c
>> @@ -0,0 +1,25 @@
>> +/* This file is part of GDB, the GNU debugger.
>> +
>> + Copyright 2022 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/>. */
>> +
>> +static long l = 0;
>> +static long *l_ptr = &l;
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> + return *l_ptr;
>> +}
>> diff --git a/gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp
>> new file mode 100644
>> index 00000000000..2b5a7b2f517
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp
>> @@ -0,0 +1,121 @@
>> +# Copyright 2022 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/>.
>> +#
>> +# This file is part of the gdb testsuite.
>> +#
>> +# Test that GDB for AArch64/Linux can properly handle pointers with
>> +# the upper 16 bits (PAC) or 8 bits (Tag) set, as well as the
>> +# VA_RANGE_SELECT bit (55).
>> +
>> +if {![is_aarch64_target]} {
>> + verbose "Skipping ${gdb_test_file_name}."
>> + return
>> +}
>> +
>> +global hex
>> +global decimal
>> +
>> +standard_testfile
>> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
>> + return -1
>> +}
>> +
>> +if ![runto_main] {
>> + return -1
>> +}
>> +
>> +# We need to iterate over two distinct ranges, separated by a single bit.
>> +# This bit is 55 (VA_RANGE_SELECT) which tells us if we have a kernel-space
>> +# address or a user-space address.
>> +
>> +# The tag field has 8 bits.
>> +set tag_bits_count 8
>> +
>> +# The pac field has 7 bits.
>> +set pac_bits_count 7
>> +
>> +# A couple patterns that we reuse for the tests later. One is for a successful
>> +# memory read and the other is for a memory read failure.
>> +set memory_read_ok_pattern "$hex\( <l>\)?:\[ \t\]+$hex"
>> +set memory_read_fail_pattern "$hex:\[ \t\]+Cannot access memory at address $hex"
>> +
>> +set pac_enabled 0
>> +
>> +# Check if PAC is enabled.
>> +gdb_test_multiple "ptype \$pauth_cmask" "fetch PAC cmask" {
>> + -re "type = long\r\n$gdb_prompt" {
>> + set pac_enabled 1
>> + }
>> + -re "type = void\r\n$gdb_prompt" {
>> + }
>> + -re ".*$gdb_prompt $" {
>> + fail $gdb_test_name
>> + return 1
>> + }
>> +}
>> +
>> +# Value of the cmask register.
>> +set cmask 0
>> +
>> +# If there are PAC registers, GDB uses those to unmask the PAC bits.
>> +if {$pac_enabled} {
>> + set cmask [get_valueof "" "\$pauth_cmask >> 48" "0" "fetch PAC cmask"]
>> +}
>> +
>> +# Cycle through the tag and pac bit ranges and check how GDB
>> +# behaves when trying to access these addresses.
>> +foreach upper_bits {"0x0" "0x1" "0x2" "0x4" "0x8" "0x10" "0x20" "0x40" "0x80"} {
>> + foreach lower_bits {"0x0" "0x1" "0x2" "0x4" "0x8" "0x10" "0x20" "0x40"} {
>> +
>> + # A successful memory read pattern
>> + set pattern $memory_read_ok_pattern
>> +
>> + if {!$pac_enabled} {
>> + # If PAC is not supported, memory reads will fail if
>> + # lower_bits != 0x0
>> + if {$lower_bits != "0x0"} {
>> + set pattern $memory_read_fail_pattern
>> + }
>> + } else {
>> + # Otherwise, figure out if the memory read will succeed or not by
>> + # checking cmask.
>> + gdb_test_multiple "p/x (~${cmask}ULL & (${lower_bits}ULL))" "" {
>> + -re "= 0x0\r\n$gdb_prompt" {
>> + # Either cmask is 0x7F or lower_bits is 0x0. Either way, the
>> + # memory read should succeed.
>> + }
>> + -re "= $hex\r\n$gdb_prompt" {
>> + if {$lower_bits != "0x0"} {
>> + # cmask doesn't mask off all the PAC bits, which
>> + # results in a memory read failure, with the actual
>> + # address being accessed differing from the one we
>> + # passed.
>> + set pattern $memory_read_fail_pattern
>> + }
>> + }
>> + }
>> + }
>> +
>> + # Test without the VA_RANGE_SELECT bit set.
>> + gdb_test "x/gx ((unsigned long) l_ptr | ((${upper_bits}ULL << 56) | (${lower_bits}ULL << 48)))" \
>> + $pattern \
>> + "user-space memory access tag bits $upper_bits and pac bits $lower_bits"
>> +
>> + # Now test with the VA_RANGE_SELECT bit set.
>> + gdb_test "x/gx ((unsigned long) l_ptr | ((${upper_bits}ULL << 56) | (${lower_bits}ULL << 48) | (1ULL << 55))) " \
>> + $memory_read_fail_pattern \
>> + "kernel-space memory access tag bits $upper_bits and pac bits $lower_bits"
>> + }
>> +}
>> diff --git a/gdb/utils.c b/gdb/utils.c
>> index 413a4f4d53b..f88d669c016 100644
>> --- a/gdb/utils.c
>> +++ b/gdb/utils.c
>> @@ -3139,28 +3139,6 @@ show_debug_timestamp (struct ui_file *file, int from_tty,
>> }
>> \f
>> -/* See utils.h. */
>> -
>> -CORE_ADDR
>> -address_significant (gdbarch *gdbarch, CORE_ADDR addr)
>> -{
>> - /* Clear insignificant bits of a target address and sign extend resulting
>> - address, avoiding shifts larger or equal than the width of a CORE_ADDR.
>> - The local variable ADDR_BIT stops the compiler reporting a shift overflow
>> - when it won't occur. Skip updating of target address if current target
>> - has not set gdbarch significant_addr_bit. */
>> - int addr_bit = gdbarch_significant_addr_bit (gdbarch);
>> -
>> - if (addr_bit && (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT)))
>> - {
>> - CORE_ADDR sign = (CORE_ADDR) 1 << (addr_bit - 1);
>> - addr &= ((CORE_ADDR) 1 << addr_bit) - 1;
>> - addr = (addr ^ sign) - sign;
>> - }
>> -
>> - return addr;
>> -}
>> -
>> const char *
>> paddress (struct gdbarch *gdbarch, CORE_ADDR addr)
>> {
>> diff --git a/gdb/utils.h b/gdb/utils.h
>> index d2acf899ba2..237ef0a5d99 100644
>> --- a/gdb/utils.h
>> +++ b/gdb/utils.h
>> @@ -278,9 +278,6 @@ extern void fputs_styled (const char *linebuffer,
>> extern void fputs_highlighted (const char *str, const compiled_regex &highlight,
>> struct ui_file *stream);
>> -/* Return the address only having significant bits. */
>> -extern CORE_ADDR address_significant (gdbarch *gdbarch, CORE_ADDR addr);
>> -
>> /* Convert CORE_ADDR to string in platform-specific manner.
>> This is usually formatted similar to 0x%lx. */
>> extern const char *paddress (struct gdbarch *gdbarch, CORE_ADDR addr);
>> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
>> index db508696261..8a6077b6c76 100644
>> --- a/gdbserver/linux-aarch64-low.cc
>> +++ b/gdbserver/linux-aarch64-low.cc
>> @@ -508,21 +508,37 @@ aarch64_target::low_remove_point (raw_bkpt_type type, CORE_ADDR addr,
>> return ret;
>> }
>> -/* Return the address only having significant bits. This is used to ignore
>> - the top byte (TBI). */
>> -
>> static CORE_ADDR
>> -address_significant (CORE_ADDR addr)
>> +aarch64_remove_non_address_bits (CORE_ADDR pointer)
>> {
>> - /* Clear insignificant bits of a target address and sign extend resulting
>> - address. */
>> - int addr_bit = 56;
>> + /* By default, we assume TBI and discard the top 8 bits plus the
>> + VA range select bit (55). */
>> + CORE_ADDR mask = (((CORE_ADDR) 1) << (64 - 9)) - 1;
>> + mask = ~mask;
>> +
>> + /* Check if PAC is available for this target. */
>> + if (tdesc_contains_feature (current_process ()->tdesc,
>> + "org.gnu.gdb.aarch64.pauth"))
>> + {
>> + /* Fetch the PAC masks. These masks are per-process, so we can just
>> + fetch data from whatever thread we have at the moment.
>> +
>> + Also, we have both a code mask and a data mask. For now they are the
>> + same, but this may change in the future. */
>> - CORE_ADDR sign = (CORE_ADDR) 1 << (addr_bit - 1);
>> - addr &= ((CORE_ADDR) 1 << addr_bit) - 1;
>> - addr = (addr ^ sign) - sign;
>> + struct regcache *regs = get_thread_regcache (current_thread, 1);
>> + CORE_ADDR dmask = regcache_raw_get_unsigned_by_name (regs, "pauth_dmask");
>> + CORE_ADDR cmask = regcache_raw_get_unsigned_by_name (regs, "pauth_cmask");
>> +
>> + if (dmask != cmask && dmask != 0 && cmask != 0)
>> + {
>> + /* Warn if the masks are different. */
>> + warning (_("C mask and D mask differ"));
>> + mask |= (dmask > cmask)? dmask : cmask;
>> + }
>> + }
>> - return addr;
>> + return aarch64_remove_top_bits (pointer, mask);
>> }
>> /* Implementation of linux target ops method "low_stopped_data_address". */
>> @@ -549,7 +565,7 @@ aarch64_target::low_stopped_data_address ()
>> hardware watchpoint hit. The stopped data addresses coming from the
>> kernel can potentially be tagged addresses. */
>> const CORE_ADDR addr_trap
>> - = address_significant ((CORE_ADDR) siginfo.si_addr);
>> + = aarch64_remove_non_address_bits ((CORE_ADDR) siginfo.si_addr);
>> /* Check if the address matches any watched address. */
>> state = aarch64_get_debug_reg_state (pid_of (current_thread));
>
next prev parent reply other threads:[~2022-08-01 11:09 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-05 14:00 [PATCH] [AArch64] " Luis Machado
2022-07-05 18:12 ` John Baldwin
2022-07-06 11:38 ` Lancelot SIX
2022-07-08 11:36 ` Luis Machado
2022-07-11 11:55 ` [PATCH,v2] [aarch64] " Luis Machado
2022-07-18 8:16 ` [Ping v1][PATCH,v2] " Luis Machado
2022-08-01 11:09 ` Luis Machado [this message]
2022-08-08 11:34 ` [Ping v3][PATCH,v2] " Luis Machado
2022-08-18 15:49 ` [Ping v4][PATCH,v2] " Luis Machado
2022-08-18 23:47 ` [PATCH,v2] " Thiago Jung Bauermann
2022-08-19 9:52 ` Luis Machado
2022-08-19 14:06 ` Thiago Jung Bauermann
2022-08-23 20:29 ` [PATCH,v3] " Luis Machado
2022-08-24 18:44 ` Thiago Jung Bauermann
2022-09-01 9:29 ` [PING][PATCH,v3] " Luis Machado
2022-09-07 8:21 ` Luis Machado
2022-09-12 12:47 ` Luis Machado
2022-09-20 12:26 ` Luis Machado
2022-09-22 12:59 ` [PATCH,v3] " Lancelot SIX
2022-09-22 16:39 ` Luis Machado
2022-09-23 7:58 ` Lancelot SIX
2022-10-03 11:37 ` [PING][PATCH,v3] " Luis Machado
2022-10-10 12:18 ` Luis Machado
2022-10-17 10:04 ` Luis Machado
2022-10-25 13:52 ` Luis Machado
2022-11-10 1:00 ` Luis Machado
2022-11-29 22:19 ` Luis Machado
2022-12-09 16:42 ` Luis Machado
2022-12-09 19:14 ` [PATCH,v3] " Simon Marchi
2022-12-12 14:21 ` Luis Machado
2022-12-12 15:07 ` Simon Marchi
2022-12-12 17:13 ` [PATCH v4] " Luis Machado
2022-12-12 18:54 ` Simon Marchi
2022-12-13 9:18 ` Luis Machado
2022-12-13 10:27 ` [PATCH v5] " Luis Machado
2022-12-16 10:57 ` [PATCH v6] " Luis Machado
2022-12-16 11:20 ` Luis Machado
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bf2265ad-4631-e774-aa6b-b504a4b289bb@arm.com \
--to=luis.machado@arm.com \
--cc=gdb-patches@sourceware.org \
--cc=lsix@lancelotsix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).