Re: [PATCH v5 03/25] Add GDB-side remote target support for memory tagging

Simon Marchi via Gdb-patches
2021-02-05T02:48:26Z

On 2021-01-27 3:20 p.m., Luis Machado via Gdb-patches wrote:
> Updates for v4:
>
> - Rename RSP helper functions.
> - Fixup parameter and return types.
>
> Updates for v2:
>
> - Add type field to target hooks.
> - Add type data to qMemTags and QMemTags. The packets now look like this:
>
> qMemTags: ,<length>:<type>
> QMemTags: ,<length>:<type>:<uninterpreted tag bytes>
>
> --
>
> This patch adds memory tagging support to GDB's remote side, with
> packet string checks, new packet support and an implementation of
> the two new tags methods fetch_atags and store_atags.

fetch_memtags and store_memtags

>
> GDBserver needs to know how to read/write allocation tags, since that is
> done via ptrace. It doesn't need to know about logical tags.
>
> The new packets are:
>
> qMemTags: ,<length>:<type>
> --
>
> Reads tags of the specified type from the address range
> [ , )
>
> QMemTags: ,<length>:<type>:<uninterpreted tag bytes>
> --
> Writes the tags of specified type represented by the uninterpreted bytes to
> the address range [ , ).
>
> The interpretation of what to do with the tag bytes is up to the arch-specific
> code.
>
> Note that these new packets consider the case of packet size overflow as an
> error, given the common use case is to read/write only a few memory tags at
> a time. Having to use a couple new packets for multi-part transfers wouldn't
> make sense for the little use it would have.
>
> gdb/ChangeLog:
>
> YYYY-MM-DD Luis Machado <luis.machado@linaro.org>
>
> * remote.c (PACKET_memory_tagging_feature): New enum.
> (remote_memory_tagging_p): New function.
> (remote_protocol_features): New "memory-tagging" entry.
> (remote_target::remote_query_supported): Handle memory tagging
> support.
> (remote_target::supports_memory_tagging): Implement.
> (create_fetch_memtags_request, parse_fetch_memtags_reply)
> (create_store_memtags_request): New functions.
> (remote_target::fetch_memtags): Implement.
> (remote_target::store_memtags): Implement.
> (_initialize_remote): Add new "memory-tagging-feature"
> config command.
> ---
> gdb/remote.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 103 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index b130f1ddae..bb3473b04c 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -2175,6 +2175,10 @@ enum {
> /* Support TARGET_WAITKIND_NO_RESUMED. */
> PACKET_no_resumed,
>
> + /* Support for memory tagging, allocation tag fetch/store
> + packets and the tag violation stop replies. */
> + PACKET_memory_tagging_feature,
> +
> PACKET_MAX
> };
>
> @@ -2316,6 +2320,14 @@ remote_exec_event_p (struct remote_state *rs)
> return packet_support (PACKET_exec_event_feature) == PACKET_ENABLE;
> }
>
> +/* Returns true if memory tagging is supported. */
> +
> +static bool
> +remote_memory_tagging_p (void)

Remove `void`.

Otherwise, LGTM.

Simon