From: Luis Machado <luis.machado@arm.com>
To: Gustavo Romero <gustavo.romero@linaro.org>, gdb-patches@sourceware.org
Cc: thiago.bauermann@linaro.org, eliz@gnu.org, tom@tromey.com
Subject: Re: [PATCH v6 6/8] gdb: Add qIsAddressTagged packet
Date: Fri, 19 Apr 2024 08:52:47 +0100 [thread overview]
Message-ID: <bf0f9f65-e985-4c02-ab33-7584160efe59@arm.com> (raw)
In-Reply-To: <20240418201039.236867-7-gustavo.romero@linaro.org>
On 4/18/24 21:10, Gustavo Romero wrote:
> This commit adds a new packet, qIsAddressTagged, allowing GDB remote
> targets to use it to query the stub if a given address is tagged.
>
> Currently, the memory tagging address check is done via a read query,
> where the contents of /proc/<PID>/smaps is read and the flags are
> inspected for memory tagging-related flags that indicate the address is
> in a memory tagged region.
>
> This is not ideal, for example, for QEMU stub and other cases, such as
> on bare-metal, where there is no notion of an OS file like 'smaps.'
> Hence, the introduction of qIsAddressTagged packet allows checking
> if an address is tagged in an agnostic way.
>
> The is_address_tagged target hook in remote.c attempts to use the
> qIsAddressTagged packet first for checking if an address is tagged and
> if the stub does not support such a packet (reply is empty) it falls
> back to using the current mechanism that reads the contents of
> /proc/<PID>/smaps via vFile requests.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
> gdb/remote.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index cd271c4b493..3d034bb1ef8 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -337,6 +337,9 @@ enum {
> packets and the tag violation stop replies. */
> PACKET_memory_tagging_feature,
>
> + /* Support for the qIsAddressTagged packet. */
> + PACKET_qIsAddressTagged,
> +
> PACKET_MAX
> };
>
> @@ -15535,6 +15538,50 @@ create_store_memtags_request (gdb::char_vector &packet, CORE_ADDR address,
> strcpy (packet.data (), request.c_str ());
> }
>
> +static void
> +create_is_address_tagged_request (gdbarch *gdbarch, gdb::char_vector &packet,
> + CORE_ADDR address)
> +{
> + int addr_size;
> + std::string request;
> +
> + addr_size = gdbarch_addr_bit (gdbarch) / 8;
> + request = string_printf ("qIsAddressTagged:%s", phex_nz (address, addr_size));
> +
> + if (packet.size () < request.length () + 1)
> + error (_("Contents too big for packet qIsAddressTagged."));
> +
> + strcpy (packet.data (), request.c_str ());
> +}
> +
> +static bool
> +check_is_address_tagged_reply (remote_target *remote, gdb::char_vector &packet,
> + bool &tagged)
> +{
> + gdb_assert (remote != nullptr);
> + /* Check reply and disable qIsAddressTagged usage if it's not supported. */
> + packet_result result = remote->m_features.packet_ok (packet,
> + PACKET_qIsAddressTagged);
> +
> + /* Return false on error (Exx), empty reply (packet not supported), or reply
> + size doesn't match 2 hex digits. */
> + if ((result.status () != PACKET_OK) || (strlen (packet.data ()) != 2))
> + return false;
> +
> + gdb_byte reply;
> + /* Convert only 2 hex digits, i.e. 1 byte in hex format. */
> + hex2bin (packet.data (), &reply, 1);
> +
> + if (reply == 0x00 || reply == 0x01)
> + {
> + tagged = !!reply;
> + return true;
> + }
> +
> + /* Invalid reply. */
> + return false;
> +}
> +
> /* Implement the "fetch_memtags" target_ops method. */
>
> bool
> @@ -15581,6 +15628,31 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
> bool
> remote_target::is_address_tagged (gdbarch *gdbarch, CORE_ADDR address)
> {
> + /* Firstly, attempt to check the address using the qIsAddressTagged
> + packet. */
> + if (m_features.packet_support (PACKET_qIsAddressTagged) != PACKET_DISABLE)
> + {
> + remote_target *remote = get_current_remote_target ();
> + struct remote_state *rs = get_remote_state ();
> + bool is_addr_tagged;
> +
> + create_is_address_tagged_request (gdbarch, rs->buf, address);
> +
> + putpkt (rs->buf);
> + getpkt (&rs->buf);
> +
> + /* If qIsAddressTagged is not supported PACKET_qIsAddressTagged will be
> + set to PACKET_DISABLE so no further attempt is made to check addresses
> + using this packet and the fallback mechanism below will be used
> + instead. Also, if the check fails due to an error (Exx reply) the
> + fallback is used too. Otherwise, the qIsAddressTagged query succeeded
> + and is_addr_tagged is valid. */
> + if (check_is_address_tagged_reply (remote, rs->buf, is_addr_tagged))
> + return is_addr_tagged;
> + }
> +
> + /* Fallback to arch-specific method of checking whether an address is tagged
> + in case check via qIsAddressTagged fails. */
> return gdbarch_tagged_address_p (gdbarch, address);
> }
>
> @@ -16070,6 +16142,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
> add_packet_config_cmd (PACKET_memory_tagging_feature,
> "memory-tagging-feature", "memory-tagging-feature", 0);
>
> + add_packet_config_cmd (PACKET_qIsAddressTagged,
> + "qIsAddressTagged", "memory-tagging-address-check", 0);
> +
> /* Assert that we've registered "set remote foo-packet" commands
> for all packet configs. */
> {
Thanks, this is OK.
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
next prev parent reply other threads:[~2024-04-19 7:53 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-18 20:10 [PATCH v6 0/8] Add another way to check tagged addresses on remote targets Gustavo Romero
2024-04-18 20:10 ` [PATCH v6 1/8] gdb: aarch64: Remove MTE address checking from get_memtag Gustavo Romero
2024-04-18 20:10 ` [PATCH v6 2/8] gdb: aarch64: Move MTE address check out of set_memtag Gustavo Romero
2024-04-18 20:10 ` [PATCH v6 3/8] gdb: aarch64: Remove MTE address checking from memtag_matches_p Gustavo Romero
2024-04-18 20:10 ` [PATCH v6 4/8] gdb: Use passed gdbarch instead of calling current_inferior Gustavo Romero
2024-04-18 20:10 ` [PATCH v6 5/8] gdb: Introduce is_address_tagged target hook Gustavo Romero
2024-04-18 20:10 ` [PATCH v6 6/8] gdb: Add qIsAddressTagged packet Gustavo Romero
2024-04-19 7:52 ` Luis Machado [this message]
2024-04-18 20:10 ` [PATCH v6 7/8] gdb/testsuite: Add unit tests for " Gustavo Romero
2024-04-19 7:53 ` Luis Machado
2024-04-19 14:00 ` Gustavo Romero
2024-04-19 14:31 ` Luis Machado
2024-04-18 20:10 ` [PATCH v6 8/8] gdb: Document " Gustavo Romero
2024-04-19 6:07 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bf0f9f65-e985-4c02-ab33-7584160efe59@arm.com \
--to=luis.machado@arm.com \
--cc=eliz@gnu.org \
--cc=gdb-patches@sourceware.org \
--cc=gustavo.romero@linaro.org \
--cc=thiago.bauermann@linaro.org \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).