Re: [PATCH,v2] [AArch64] MTE corefile support

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

On 5/31/21 11:49 AM, Simon Marchi wrote:
> On 2021-05-31 10:12 a.m., Luis Machado wrote:
>>>> @@ -71,4 +72,18 @@ extern CORE_ADDR aarch64_mte_set_ltag (CORE_ADDR address, CORE_ADDR tag);
>>>>       It is always possible to get the logical tag.  */
>>>>    extern CORE_ADDR aarch64_mte_get_ltag (CORE_ADDR address);
>>>>    +/* MTE-specific NT_MEMTAG header.  */
>>>> +struct tag_dump_mte
>>>> +{
>>>> +  /* Size of the tag granule in bytes.  */
>>>> +  uint16_t granule_byte_size;
>>>> +  /* Size of the tag in bits.  */
>>>> +  uint16_t tag_bit_size;
>>>> +  /* Reserved field for the future.  */
>>>> +  uint16_t __unused;
>>>> +};
>>>
>>> This struct is unused.
>>>
>>
>> I'm inclined to leave that as documentation instead, so it is clear what the header sizes are referring to.
> 
> The thing is that somebody reading the code in the .c seeing the
> arbitrary sizeofs  will note have a clue to go look at
> arch/aarch64-mte-linux.h for explanations.  So another option would be
> to write it as a comment near the code.  Something like:
> 
>    /* The header follows the following format:
> 
>       struct
>       {
>         /* Size of the tag granule in bytes.  */
>         uint16_t granule_byte_size;
>         /* Size of the tag in bits.  */
>         uint16_t tag_bit_size;
>         /* Reserved field for the future.  */
>         uint16_t __unused;
>       };
>     */
> 
> Another option would be to have the structure and refer to its fields in
> sizeof:
> 
>      store_unsigned_integer (buf, sizeof (tag_dump_mte::granule_byte_size),
> 			    byte_order, AARCH64_MTE_GRANULE_SIZE);

I think that would work as well. I can change it for the next iteration.