[PATCH] gdb: aarch64: Support MTE on baremetal
Luis Machado
luis.machado@arm.com
Tue Jul 23 09:46:16 GMT 2024
Hi,
Thanks for the patch.
On 7/22/24 15:41, Gustavo Romero wrote:
> This commit moves aarch64_linux_memtag_matches_p,
> aarch64_linux_set_memtags, aarch64_linux_get_memtag, and
> aarch64_linux_memtag_to_string hooks (plus aarch64_mte_get_atag
> function used by them), along with the setting of the memtag granule
> size, from aarch64-linux-tdep.c to aarch64-tdep.c, making MTE available
> on baremetal targets. Since the aarch64-linux-tdep.c layer inherits
> these hooks from aarch64-tdep.c, there is no effective change for
> aarch64-linux targets.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
> gdb/aarch64-linux-tdep.c | 167 --------------------------------------
> gdb/aarch64-tdep.c | 168 +++++++++++++++++++++++++++++++++++++++
> gdb/aarch64-tdep.h | 3 +
> 3 files changed, 171 insertions(+), 167 deletions(-)
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index a1296a8f0c7..63defd5a31f 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -2427,29 +2427,6 @@ aarch64_linux_gcc_target_options (struct gdbarch *gdbarch)
> return {};
> }
>
> -/* Helper to get the allocation tag from a 64-bit ADDRESS.
> -
> - Return the allocation tag if successful and nullopt otherwise. */
> -
> -static std::optional<CORE_ADDR>
> -aarch64_mte_get_atag (CORE_ADDR address)
> -{
> - gdb::byte_vector tags;
> -
> - /* Attempt to fetch the allocation tag. */
> - if (!target_fetch_memtags (address, 1, tags,
> - static_cast<int> (memtag_type::allocation)))
> - return {};
> -
> - /* Only one tag should've been returned. Make sure we got exactly that. */
> - if (tags.size () != 1)
> - error (_("Target returned an unexpected number of tags."));
> -
> - /* Although our tags are 4 bits in size, they are stored in a
> - byte. */
> - return tags[0];
> -}
> -
> /* Implement the tagged_address_p gdbarch method. */
>
> static bool
> @@ -2466,132 +2443,6 @@ aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR address)
> return true;
> }
>
> -/* Implement the memtag_matches_p gdbarch method. */
> -
> -static bool
> -aarch64_linux_memtag_matches_p (struct gdbarch *gdbarch,
> - struct value *address)
> -{
> - gdb_assert (address != nullptr);
> -
> - CORE_ADDR addr = value_as_address (address);
> -
> - /* Fetch the allocation tag for ADDRESS. */
> - std::optional<CORE_ADDR> atag
> - = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch, addr));
> -
> - if (!atag.has_value ())
> - return true;
> -
> - /* Fetch the logical tag for ADDRESS. */
> - gdb_byte ltag = aarch64_mte_get_ltag (addr);
> -
> - /* Are the tags the same? */
> - return ltag == *atag;
> -}
> -
> -/* Implement the set_memtags gdbarch method. */
> -
> -static bool
> -aarch64_linux_set_memtags (struct gdbarch *gdbarch, struct value *address,
> - size_t length, const gdb::byte_vector &tags,
> - memtag_type tag_type)
> -{
> - gdb_assert (!tags.empty ());
> - gdb_assert (address != nullptr);
> -
> - CORE_ADDR addr = value_as_address (address);
> -
> - /* Set the logical tag or the allocation tag. */
> - if (tag_type == memtag_type::logical)
> - {
> - /* When setting logical tags, we don't care about the length, since
> - we are only setting a single logical tag. */
> - addr = aarch64_mte_set_ltag (addr, tags[0]);
> -
> - /* Update the value's content with the tag. */
> - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> - gdb_byte *srcbuf = address->contents_raw ().data ();
> - store_unsigned_integer (srcbuf, sizeof (addr), byte_order, addr);
> - }
> - else
> - {
> - /* Remove the top byte. */
> - addr = gdbarch_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:
> -
> - 1 - G == N: Store all the N tags to memory.
> -
> - 2 - G < N : Warn about having more tags than granules, but write G
> - tags.
> -
> - 3 - G > N : This is a "fill tags" operation. We should use the tags
> - as a pattern to fill the granules repeatedly until we have
> - written G tags to memory.
> - */
> -
> - size_t g = aarch64_mte_get_tag_granules (addr, length,
> - AARCH64_MTE_GRANULE_SIZE);
> - size_t n = tags.size ();
> -
> - if (g < n)
> - warning (_("Got more tags than memory granules. Tags will be "
> - "truncated."));
> - else if (g > n)
> - warning (_("Using tag pattern to fill memory range."));
> -
> - if (!target_store_memtags (addr, length, tags,
> - static_cast<int> (memtag_type::allocation)))
> - return false;
> - }
> - return true;
> -}
> -
> -/* Implement the get_memtag gdbarch method. */
> -
> -static struct value *
> -aarch64_linux_get_memtag (struct gdbarch *gdbarch, struct value *address,
> - memtag_type tag_type)
> -{
> - gdb_assert (address != nullptr);
> -
> - CORE_ADDR addr = value_as_address (address);
> - CORE_ADDR tag = 0;
> -
> - /* Get the logical tag or the allocation tag. */
> - if (tag_type == memtag_type::logical)
> - tag = aarch64_mte_get_ltag (addr);
> - else
> - {
> - /* Remove the top byte. */
> - addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> - std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
> -
> - if (!atag.has_value ())
> - return nullptr;
> -
> - tag = *atag;
> - }
> -
> - /* Convert the tag to a value. */
> - return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int,
> - tag);
> -}
> -
> -/* Implement the memtag_to_string gdbarch method. */
> -
> -static std::string
> -aarch64_linux_memtag_to_string (struct gdbarch *gdbarch, struct value *tag_value)
> -{
> - if (tag_value == nullptr)
> - return "";
> -
> - CORE_ADDR tag = value_as_address (tag_value);
> -
> - return string_printf ("0x%s", phex_nz (tag, sizeof (tag)));
> -}
>
> /* AArch64 Linux implementation of the report_signal_info gdbarch
> hook. Displays information about possible memory tag violations. */
> @@ -2900,24 +2751,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> /* Register a hook for checking if an address is tagged or not. */
> set_gdbarch_tagged_address_p (gdbarch, aarch64_linux_tagged_address_p);
>
> - /* Register a hook for checking if there is a memory tag match. */
> - set_gdbarch_memtag_matches_p (gdbarch,
> - aarch64_linux_memtag_matches_p);
> -
> - /* Register a hook for setting the logical/allocation tags for
> - a range of addresses. */
> - set_gdbarch_set_memtags (gdbarch, aarch64_linux_set_memtags);
> -
> - /* Register a hook for extracting the logical/allocation tag from an
> - address. */
> - set_gdbarch_get_memtag (gdbarch, aarch64_linux_get_memtag);
> -
> - /* Set the allocation tag granule size to 16 bytes. */
> - set_gdbarch_memtag_granule_size (gdbarch, AARCH64_MTE_GRANULE_SIZE);
> -
> - /* Register a hook for converting a memory tag to a string. */
> - set_gdbarch_memtag_to_string (gdbarch, aarch64_linux_memtag_to_string);
> -
> set_gdbarch_report_signal_info (gdbarch,
> aarch64_linux_report_signal_info);
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index e4bca6c6632..f6227a5b82d 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -45,6 +45,7 @@
>
> #include "aarch64-tdep.h"
> #include "aarch64-ravenscar-thread.h"
> +#include "arch/aarch64-mte-linux.h"
Including a more specific scope into a generic one is incorrect. In this
particular case we're including a *linux* header into a supposedly generic
aarch64-tdep.c file.
If we need to use things contained in the Linux files, we should make sure they can be made
generic and then extract those out from arch/aarch64-mte-linux.[c|h] and then move them
to, say, arch/aarch64-mte.[c|h].
>
> #include "record.h"
> #include "record-full.h"
> @@ -4088,6 +4089,156 @@ aarch64_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> return streq (inst.opcode->name, "ret");
> }
>
> +/* Helper to get the allocation tag from a 64-bit ADDRESS.
> +
> + Return the allocation tag if successful and nullopt otherwise. */
> +
> +std::optional<CORE_ADDR>
> +aarch64_mte_get_atag (CORE_ADDR address)
> +{
> + gdb::byte_vector tags;
> +
> + /* Attempt to fetch the allocation tag. */
> + if (!target_fetch_memtags (address, 1, tags,
> + static_cast<int> (memtag_type::allocation)))
> + return {};
> +
> + /* Only one tag should've been returned. Make sure we got exactly that. */
> + if (tags.size () != 1)
> + error (_("Target returned an unexpected number of tags."));
> +
> + /* Although our tags are 4 bits in size, they are stored in a
> + byte. */
> + return tags[0];
> +}
> +
> +/* Implement the memtag_matches_p gdbarch method. */
> +
> +static bool
> +aarch64_memtag_matches_p (struct gdbarch *gdbarch,
> + struct value *address)
There seems to be something off with indentation above.
> +{
> + gdb_assert (address != nullptr);
> +
> + CORE_ADDR addr = value_as_address (address);
> +
> + /* Fetch the allocation tag for ADDRESS. */
> + std::optional<CORE_ADDR> atag
> + = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch, addr));
> +
> + if (!atag.has_value ())
> + return true;
> +
> + /* Fetch the logical tag for ADDRESS. */
> + gdb_byte ltag = aarch64_mte_get_ltag (addr);
> +
> + /* Are the tags the same? */
> + return ltag == *atag;
> +}
> +
> +/* Implement the set_memtags gdbarch method. */
> +
> +static bool
> +aarch64_set_memtags (struct gdbarch *gdbarch, struct value *address,
> + size_t length, const gdb::byte_vector &tags,
> + memtag_type tag_type)
Indentation above looks off too.
> +{
> + gdb_assert (!tags.empty ());
> + gdb_assert (address != nullptr);
> +
> + CORE_ADDR addr = value_as_address (address);
> +
> + /* Set the logical tag or the allocation tag. */
> + if (tag_type == memtag_type::logical)
> + {
> + /* When setting logical tags, we don't care about the length, since
> + we are only setting a single logical tag. */
> + addr = aarch64_mte_set_ltag (addr, tags[0]);
> +
> + /* Update the value's content with the tag. */
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> + gdb_byte *srcbuf = address->contents_raw ().data ();
> + store_unsigned_integer (srcbuf, sizeof (addr), byte_order, addr);
> + }
> + else
> + {
> + /* Remove the top byte. */
> + addr = gdbarch_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:
> +
> + 1 - G == N: Store all the N tags to memory.
> +
> + 2 - G < N : Warn about having more tags than granules, but write G
> + tags.
> +
> + 3 - G > N : This is a "fill tags" operation. We should use the tags
> + as a pattern to fill the granules repeatedly until we have
> + written G tags to memory.
> + */
> +
> + size_t g = aarch64_mte_get_tag_granules (addr, length,
> + AARCH64_MTE_GRANULE_SIZE);
> + size_t n = tags.size ();
> +
> + if (g < n)
> + warning (_("Got more tags than memory granules. Tags will be "
> + "truncated."));
> + else if (g > n)
> + warning (_("Using tag pattern to fill memory range."));
> +
> + if (!target_store_memtags (addr, length, tags,
> + static_cast<int> (memtag_type::allocation)))
> + return false;
> + }
> + return true;
> +}
> +
> +/* Implement the get_memtag gdbarch method. */
> +
> +static struct value *
> +aarch64_get_memtag (struct gdbarch *gdbarch, struct value *address,
> + memtag_type tag_type)
Indentation is off above.
> +{
> + gdb_assert (address != nullptr);
> +
> + CORE_ADDR addr = value_as_address (address);
> + CORE_ADDR tag = 0;
> +
> + /* Get the logical tag or the allocation tag. */
> + if (tag_type == memtag_type::logical)
> + tag = aarch64_mte_get_ltag (addr);
> + else
> + {
> + /* Remove the top byte. */
> + addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> + std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
> +
> + if (!atag.has_value ())
> + return nullptr;
> +
> + tag = *atag;
> + }
> +
> + /* Convert the tag to a value. */
> + return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int,
> + tag);
> +}
> +
> +/* Implement the memtag_to_string gdbarch method. */
> +
> +static std::string
> +aarch64_memtag_to_string (struct gdbarch *gdbarch, struct value *tag_value)
> +{
> + if (tag_value == nullptr)
> + return "";
> +
> + CORE_ADDR tag = value_as_address (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. */
>
> @@ -4504,6 +4655,23 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> aarch64_pseudo_register_reggroup_p);
> set_gdbarch_cannot_store_register (gdbarch, aarch64_cannot_store_register);
>
> + /* Set the allocation tag granule size to 16 bytes. */
> + set_gdbarch_memtag_granule_size (gdbarch, AARCH64_MTE_GRANULE_SIZE);
> +
> + /* Register a hook for checking if there is a memory tag match. */
> + set_gdbarch_memtag_matches_p (gdbarch, aarch64_memtag_matches_p);
> +
> + /* Register a hook for setting the logical/allocation tags for
> + a range of addresses. */
> + set_gdbarch_set_memtags (gdbarch, aarch64_set_memtags);
> +
> + /* Register a hook for extracting the logical/allocation tag from an
> + address. */
> + set_gdbarch_get_memtag (gdbarch, aarch64_get_memtag);
> +
> + /* Register a hook for converting a memory tag to a string. */
> + set_gdbarch_memtag_to_string (gdbarch, aarch64_memtag_to_string);
> +
These gdbarch hooks are being registered regardless of whether the target supports
MTE or not. In aarch64-linux-tdep.c they were registered if MTE support was confirmed
by the presence of the XML feature. I think this should be the same, even if we have
some safety checks prior to using these hooks.
> /* ABI */
> set_gdbarch_short_bit (gdbarch, 16);
> set_gdbarch_int_bit (gdbarch, 32);
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index 0e6024bfcbc..0072834be9c 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -203,4 +203,7 @@ void aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
>
> bool aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch);
>
> +std::optional<CORE_ADDR>
> +aarch64_mte_get_atag (CORE_ADDR address);
> +
The above declaration should fit in one line.
> #endif /* aarch64-tdep.h */
I'm running some tests on my end, but these are things I spotted while going through the
changes.
More information about the Gdb-patches
mailing list