Re: [PATCH v3 05/24] GDBserver remote packet support for memory tagging

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

> diff --git a/gdbserver/server.cc b/gdbserver/server.cc
> index 95db9807a9..7eddd0e01f 100644
> --- a/gdbserver/server.cc
> +++ b/gdbserver/server.cc
> @@ -547,12 +547,71 @@ handle_btrace_conf_general_set (char *own_buf)
>    return 1;
>  }
>  
> +/* Create the qMemTags packet reply given TAGS.  */
> +
> +static int
> +create_fmemtags_reply (char *reply, const gdb::byte_vector &tags)
> +{
> +  /* It is an error to pass a zero-sized tag vector.  */
> +  if (tags.size () == 0)
> +    return 1;

If this catches a logic error in GDBserver, this should be an assert.

> +
> +  std::string packet ("m");
> +
> +  /* Write the tag data.  */
> +  packet += bin2hex (tags.data (), tags.size ());
> +
> +  /* Check if the reply is too big for the packet to handle.  */
> +  if (PBUFSIZ < packet.size ())
> +    return 1;
> +
> +  strcpy (reply, packet.c_str ());
> +  return 0;
> +}
> +
> +/* Parse the QMemTags request into ADDR, LEN and TAGS.
> +
> +   Return 0 if successful, non-zero otherwise.  */
> +
> +static int
> +parse_smemtags_request (char *request, CORE_ADDR *addr, size_t *len,
> +			gdb::byte_vector &tags, int *type)
> +{
> +  if (!startswith (request, "QMemTags:"))
> +    return 1;

That check seems unnecessary, or maybe I'd convert it to an assert.

> diff --git a/gdbserver/server.h b/gdbserver/server.h
> index 22228050a8..3d4a086e18 100644
> --- a/gdbserver/server.h
> +++ b/gdbserver/server.h
> @@ -190,6 +190,9 @@ struct client_state
>  
>    int current_traceframe = -1;
>  
> +  /* If true, memory tagging features are supported.  */
> +  bool memory_tagging_feature = false;

Just a note that as of this patch, this field is unnecessary.  If it gets
used later, that's fine.

> @@ -499,6 +500,19 @@ class process_stratum_target
>  
>    /* Return tdesc index for IPA.  */
>    virtual int get_ipa_tdesc_idx ();
> +
> +  /* Returns true if the target supports memory tagging facilities.  */
> +  virtual bool supports_memory_tagging ();
> +
> +  /* Return the allocated memory tags of type TYPE associated with
> +     [ADDRESS, ADDRESS + LEN) in TAGS.  */
> +  virtual int fetch_memtags (CORE_ADDR address, size_t len,
> +			     gdb::byte_vector &tags, int type);
> +
> +  /* Write the allocation tags of type TYPE contained in TAGS to the
> +     memory range [ADDRESS, ADDRESS + LEN).  */
> +  virtual int store_memtags (CORE_ADDR address, size_t len,
> +			     const gdb::byte_vector &tags, int type);

I suppose that these should return bool.

Simon