| Message ID | 20240722144128.1597747-1-gustavo.romero@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gdb: aarch64: Support MTE on baremetal | expand |
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.
Hi Luis! On 7/23/24 6:46 AM, Luis Machado wrote: > Hi, > > Thanks for the patch. Thanks a lot for the review :) > 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. tdep->has_mte() does not work in aarch64-tdep.c because the XML advertises the 'tag_ctl' pseudo-register, meant to change the Tag Check Fault behavior at runtime. It doesn't make sense to have it in baremetal, since it's not a real register and was introduced to emulate Linux's prctl() PR_SET_TAGGED_ADDR_CTRL option. In that sense, QEMU system mode doesn't advertise it for this reason either. As you said, all memory-tag subcommands check if MTE is supported by the target calling target_supports_memory_tagging() and so all goes well in aarch64-tdep.c I don't have any idea on how to address what you're asking for. Actually, I was inclined to remove the if (tdep->has_mte()) check in aarch64-linux-tdep.c when registering the hooks. I don't see any harm on always registering them and with has_mte() in place we are kind duplicating the check: one check is done via XML advertisement of 'tag_ctl' and the other check (in the hooks) uses "memory-tagging+" feature in qSupported. >> /* 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. Thanks! I've addressed all your comments in v2, except the has_mte() check, since we are still discussing it. Cheers, Gustavo
On 7/24/24 01:45, Gustavo Romero wrote: > Hi Luis! > > On 7/23/24 6:46 AM, Luis Machado wrote: >> Hi, >> >> Thanks for the patch. > > Thanks a lot for the review :) > > >> 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. > > tdep->has_mte() does not work in aarch64-tdep.c because the XML advertises the 'tag_ctl' > pseudo-register, meant to change the Tag Check Fault behavior at runtime. It doesn't > make sense to have it in baremetal, since it's not a real register and was introduced > to emulate Linux's prctl() PR_SET_TAGGED_ADDR_CTRL option. In that sense, QEMU system > mode doesn't advertise it for this reason either. Yeah, since the feature was initially designed with Linux in mind, we have the tag_ctl register and its associated register set that we need to read from/dump to core files. With the feature being supported for baremetal now, that might need to become a Linux-specific thing. Which, incidentally, as I mentioned above about the header file, is a bit of an intrusion of Linux-specific knowledge on an otherwise generic aarch64-tdep.c. > > As you said, all memory-tag subcommands check if MTE is supported by the target calling > target_supports_memory_tagging() and so all goes well in aarch64-tdep.c > > I don't have any idea on how to address what you're asking for. Actually, I was inclined > to remove the if (tdep->has_mte()) check in aarch64-linux-tdep.c when registering the > hooks. I don't see any harm on always registering them and with has_mte() in place we > are kind duplicating the check: one check is done via XML advertisement of 'tag_ctl' and > the other check (in the hooks) uses "memory-tagging+" feature in qSupported. It might be possible to do the same check with the target method I suppose? The remote target would advertise memory-tagging+, the native and linux layers would advertise the presence of HWCAP2_MTE. The question is if the target is properly setup at the point where we are checking these XML features. We'd need to check. > > >>> /* 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. > > Thanks! I've addressed all your comments in v2, except the has_mte() check, since we are > still discussing it. > > > Cheers, > Gustavo
Hi Luis, On 7/24/24 6:17 AM, Luis Machado wrote: > On 7/24/24 01:45, Gustavo Romero wrote: >> Hi Luis! >> >> On 7/23/24 6:46 AM, Luis Machado wrote: >>> Hi, >>> >>> Thanks for the patch. >> >> Thanks a lot for the review :) >> >> >>> 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. >> >> tdep->has_mte() does not work in aarch64-tdep.c because the XML advertises the 'tag_ctl' >> pseudo-register, meant to change the Tag Check Fault behavior at runtime. It doesn't >> make sense to have it in baremetal, since it's not a real register and was introduced >> to emulate Linux's prctl() PR_SET_TAGGED_ADDR_CTRL option. In that sense, QEMU system >> mode doesn't advertise it for this reason either. > > Yeah, since the feature was initially designed with Linux in mind, we have the tag_ctl > register and its associated register set that we need to read from/dump to core files. > > With the feature being supported for baremetal now, that might need to become a Linux-specific > thing. Which, incidentally, as I mentioned above about the header file, is a bit of an intrusion > of Linux-specific knowledge on an otherwise generic aarch64-tdep.c. > >> >> As you said, all memory-tag subcommands check if MTE is supported by the target calling >> target_supports_memory_tagging() and so all goes well in aarch64-tdep.c >> >> I don't have any idea on how to address what you're asking for. Actually, I was inclined >> to remove the if (tdep->has_mte()) check in aarch64-linux-tdep.c when registering the >> hooks. I don't see any harm on always registering them and with has_mte() in place we >> are kind duplicating the check: one check is done via XML advertisement of 'tag_ctl' and >> the other check (in the hooks) uses "memory-tagging+" feature in qSupported. > > It might be possible to do the same check with the target method I suppose? > > The remote target would advertise memory-tagging+, the native and linux layers would > advertise the presence of HWCAP2_MTE. The question is if the target is properly setup at > the point where we are checking these XML features. We'd need to check Do you mean something like: diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 865b1c0b13b..bd4baf2632a 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -4655,6 +4655,8 @@ 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); + if (tdep->has_mte ()) + { /* Set the allocation tag granule size to 16 bytes. */ set_gdbarch_memtag_granule_size (gdbarch, AARCH64_MTE_GRANULE_SIZE); @@ -4671,6 +4673,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) /* Register a hook for converting a memory tag to a string. */ set_gdbarch_memtag_to_string (gdbarch, aarch64_memtag_to_string); + } /* ABI */ set_gdbarch_short_bit (gdbarch, 16); diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index 50166fb4f24..989ccc7a7b9 100644 --- a/gdb/aarch64-tdep.h +++ b/gdb/aarch64-tdep.h @@ -26,6 +26,7 @@ #include "displaced-stepping.h" #include "infrun.h" #include "gdbarch.h" +#include "target.h" /* Forward declarations. */ struct gdbarch; @@ -126,7 +127,7 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base /* Returns true if the target supports MTE. */ bool has_mte () const { - return mte_reg_base != -1; + return target_supports_memory_tagging (); } /* TLS registers. This is -1 if the TLS registers are not available. */ ? Yes, this works because in remote_target::start_remote_1() (e.g. invoked on 'target remote :1234') remote_query_supported(), which parses the qSupported string, is called before target_find_description() is called (which then calls aarch64_gdbarch_init()), hence when has_mte() is called from aarch64_gdbarch_init() qSupported string is already parsed correctly. I.e, in remote_target::start_remote_1() we have: [...] /* The first packet we send to the target is the optional "supported packets" request. If the target can answer this, it will tell us which later probes to skip. */ remote_query_supported (); [...] /* Next, if the target can specify a description, read it. We do this before anything involving memory or registers. */ target_find_description (); Is this your suggestion? Thanks. Cheers, Gustavo
On 7/25/24 01:54, Gustavo Romero wrote: > Hi Luis, > > On 7/24/24 6:17 AM, Luis Machado wrote: >> On 7/24/24 01:45, Gustavo Romero wrote: >>> Hi Luis! >>> >>> On 7/23/24 6:46 AM, Luis Machado wrote: >>>> Hi, >>>> >>>> Thanks for the patch. >>> >>> Thanks a lot for the review :) >>> >>> >>>> 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. >>> >>> tdep->has_mte() does not work in aarch64-tdep.c because the XML advertises the 'tag_ctl' >>> pseudo-register, meant to change the Tag Check Fault behavior at runtime. It doesn't >>> make sense to have it in baremetal, since it's not a real register and was introduced >>> to emulate Linux's prctl() PR_SET_TAGGED_ADDR_CTRL option. In that sense, QEMU system >>> mode doesn't advertise it for this reason either. >> >> Yeah, since the feature was initially designed with Linux in mind, we have the tag_ctl >> register and its associated register set that we need to read from/dump to core files. >> >> With the feature being supported for baremetal now, that might need to become a Linux-specific >> thing. Which, incidentally, as I mentioned above about the header file, is a bit of an intrusion >> of Linux-specific knowledge on an otherwise generic aarch64-tdep.c. >> >>> >>> As you said, all memory-tag subcommands check if MTE is supported by the target calling >>> target_supports_memory_tagging() and so all goes well in aarch64-tdep.c >>> >>> I don't have any idea on how to address what you're asking for. Actually, I was inclined >>> to remove the if (tdep->has_mte()) check in aarch64-linux-tdep.c when registering the >>> hooks. I don't see any harm on always registering them and with has_mte() in place we >>> are kind duplicating the check: one check is done via XML advertisement of 'tag_ctl' and >>> the other check (in the hooks) uses "memory-tagging+" feature in qSupported. >> >> It might be possible to do the same check with the target method I suppose? >> >> The remote target would advertise memory-tagging+, the native and linux layers would >> advertise the presence of HWCAP2_MTE. The question is if the target is properly setup at >> the point where we are checking these XML features. We'd need to check > > Do you mean something like: > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index 865b1c0b13b..bd4baf2632a 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -4655,6 +4655,8 @@ 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); > > + if (tdep->has_mte ()) > + { > /* Set the allocation tag granule size to 16 bytes. */ > set_gdbarch_memtag_granule_size (gdbarch, AARCH64_MTE_GRANULE_SIZE); > > @@ -4671,6 +4673,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > > /* Register a hook for converting a memory tag to a string. */ > set_gdbarch_memtag_to_string (gdbarch, aarch64_memtag_to_string); > + } > > /* ABI */ > set_gdbarch_short_bit (gdbarch, 16); > diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h > index 50166fb4f24..989ccc7a7b9 100644 > --- a/gdb/aarch64-tdep.h > +++ b/gdb/aarch64-tdep.h > @@ -26,6 +26,7 @@ > #include "displaced-stepping.h" > #include "infrun.h" > #include "gdbarch.h" > +#include "target.h" > > /* Forward declarations. */ > struct gdbarch; > @@ -126,7 +127,7 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base > /* Returns true if the target supports MTE. */ > bool has_mte () const > { > - return mte_reg_base != -1; > + return target_supports_memory_tagging (); > } > > /* TLS registers. This is -1 if the TLS registers are not available. */ > > ? > > Yes, this works because in remote_target::start_remote_1() (e.g. invoked on 'target remote :1234') > remote_query_supported(), which parses the qSupported string, is called before > target_find_description() is called (which then calls aarch64_gdbarch_init()), hence when has_mte() > is called from aarch64_gdbarch_init() qSupported string is already parsed correctly. > > I.e, in remote_target::start_remote_1() we have: > > [...] > > /* The first packet we send to the target is the optional "supported > packets" request. If the target can answer this, it will tell us > which later probes to skip. */ > remote_query_supported (); > > [...] > > /* Next, if the target can specify a description, read it. We do > this before anything involving memory or registers. */ > target_find_description (); > > > Is this your suggestion? I was thinking more about your suggestion to drop has_mte. Though that is mostly used for register sets. Upon further analysis, I think it is fine to do registering of these generic hooks even if the tag_ctl is not available. Let me check v2.
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" #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) +{ + 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) +{ + 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) +{ + 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); + /* 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); + #endif /* aarch64-tdep.h */
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(-)