Re: [PATCH v2 01/24] New target methods for memory tagging support

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 10/27/20 10:22 AM, Simon Marchi wrote:
> On 2020-10-22 3:59 p.m., Luis Machado via Gdb-patches wrote:
>> diff --git a/gdb/target.h b/gdb/target.h
>> index 039194c239..80f9b1ee92 100644
>> --- a/gdb/target.h
>> +++ b/gdb/target.h
>> @@ -1260,6 +1260,22 @@ struct target_ops
>>       /* Cleanup after generating a core file.  */
>>       virtual void done_generating_core ()
>>         TARGET_DEFAULT_IGNORE ();
>> +
>> +    /* Returns true if the target supports memory tagging.  */
>> +    virtual bool supports_memory_tagging ()
>> +      TARGET_DEFAULT_RETURN (false);
>> +
>> +    /* 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)
>> +      TARGET_DEFAULT_IGNORE ();
>> +
>> +    /* 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)
>> +      TARGET_DEFAULT_IGNORE ();
> 
> Could you precise the comment for fetch_memtags and store_memtags?  It's
> not clear to me what the tags vector contains.  Will it be of length
> LEN, with each element containing the tag of the matching memory
> location in [ADDRESS, ADDRESS + LEN)?

The TAGS vector contains bytes. The interpretation of those bytes is 
target/arch-specific.  AArch64 interprets those as 1 tag per byte. Some 
other target or arch may interpret those differently.

Length is the number of bytes in the memory range, which doesn't 
necessarily match the length of the TAGS vector. AArch64 has 1 tag for 
every 16 bytes, for example. So in a 32 bytes memory range, we'd have 2 
bytes in the TAGS vector.

Should I enhance the documentation of those methods to make it a bit 
more clear? I realize some of this is tied to details and terminology of 
the technology.

> 
> Also, what does the return values mean?

My idea was to provide meaningful error codes, as opposed to a bool that 
tells us the it succeeded/failed. Maybe I should make those a bool for 
now given I don't have a standardized list of possible error codes?

> 
> Simon
>