Re: [PATCH 19/23] Documentation for the new mtag commands

[Luis Machado <luis.machado@linaro.org>]

On 7/17/20 3:11 AM, Eli Zaretskii wrote:
>> Date: Wed, 15 Jul 2020 16:45:09 -0300
>> From: Luis Machado via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: catalin.marinas@arm.com, david.spickett@linaro.org
>>
>> 	* gdb.textinfo (Memory Tagging): New subsection.
>> 	(AArch64 Memory Tagging Extension): New subsection.
> 
> gdb.texinfo (without the "t").  Also, we usually combine functions and
> sections that have the same change description, as in
>  > 	* gdb.texinfo (Memory Tagging, AArch64 Memory Tagging Extension):
> 	 New subsections.
> 

Fixed now.

>> +Memory tagging is a memory protection technology that validates accesses
>> +through pointers via a tag.
> 
> The "via a tag" part is ambiguous: it is not clear whether it refers
> to the access or to the protection.  Suggest a slight rewording:
> 
>    Memory tagging is a memory protection technology that uses tags to
>    validate memory accesses through pointers.
> 

That sounds better. Thanks.

>>                            Both the pointer tag and the memory tag in the
>> +physical address space must match for the memory access to be validated >
> Here, it is unclear what should match what.  Do you mean that the
> pointer tag must match the memory tag? or do you mean something else?
> If the former, then where does the "physical address space" part come
> into the picture?
> 

The pointer tag must match the memory tag. The physical address space 
reference was to make it clear that the allocation tag was associated 
with the memory itself.

I've rephrased this as the following now...

"The pointer tag and the memory tag must match for the memory access to 
be validated."

How does it look?

>> +There are two types of tags: logical and allocation.  The logical tag is
>                                                           ^^^^^^^^^^^^^^^
> "A logical tag"
> 
>> +stored in the pointers themselves.  The allocation tag is the tag associated
>                                         ^^^^^^^^^^^^^^^^^^
> Ditto.
> 

Fixed both.

>> +with the physical address space, against which the logical tags from pointers
>> +are validated.
> 
> "Validated" or "compared"?  The latter is much less vague, so if it's
> accurate, I think we should prefer it.
> 

Compared really. I've changed it.

>> +If the underlying architecture supports memory tagging, like AArch64,
>                                                             ^^^^^^^^^^^^
> "like AArch64 does"
> 

How about "... like AArch64 MTE or SPARC ADI..."?

>> +@item mtag showltag @var{address_expression}
>> +Show the logical tag contained in the pointer resulting from evaluating the
>> +argument expression.
> 
> This is slightly better, IMO:
> 
>    Show the logical tag stored at the address given by
>    @var{address_expression}.
> 
> It avoids two words in a row that end in "ing", which makes it a
> mouthful.
> 

I agree. I wanted to convey the idea that the result value is treated as 
a pointer, but it may not be worth it.

I went with your suggestion.

>> +@item mtag setltag @var{address_expression} @var{tag_bytes}
>> +Print the resulting pointer from evaluating the argument expression with a
>> +logical tag of @var{tag_bytes}.
> 
> I don't understand what "print the resulting point" means in this
> context, and the sentence confused me, perhaps for this very reason.
> can you elaborate what this means?
> 

The goal of the command is to modify a particular address/pointer to 
include the specified logical tag.

It will, therefore, print a modified version of the address given by 
@var{address_expression}, but containing the specified tag.

So it doesn't "set" anything at the moment, it just prints what the 
pointer would look like with the user-passed tag. This may change based 
on reviews.

How about the following?

"Print the address given by @var{address_expression}, augmented with a 
logical tag of @var{tag_bytes}."

>> +@item mtag showatag @var{address_expression}
>> +Show the allocation tag from the memory address pointed to by the evaluation
>> +of the argument expression.
> 
> See above: I'd rephrase this similarly to showltag.
> 

Right. So this is how it reads now...

"Show the allocation tag associated with the memory address given by
@var{address_expression}."

It has a subtle difference to make it clear that the allocation tag is 
not related to the pointer, but with the memory address.

>> +@item mtag setatag @var{starting_address} @var{length} @var{tag_bytes}
>> +Set the allocation tag for memory range @r{[}@var{starting_address},
>> +@var{starting_address} + @var{length}@r{)} to @var{tag_bytes}.
> 
> So setatag _sets_ a tag, but setltag _prints_ something?  Isn't that
> inconsistent?
> 

A little, and this is something we need to address during reviews.

The reason behind this inconsistency is that we can't always modify the 
user-passed argument to contain the specified logical tag, but we can 
always print that information. Whichever is most useful is debatable.

The user can always use the "set" command to modify values, and can use 
"setltag" to see what the tagged pointer would look like before 
modifying a particular value.

For allocation tags, on the other hand, we can always modify them. Those 
tags are set in memory and are not part of pointers.

Does that make things slightly more clear?

>> +@item mtag check @var{address_expression}
>> +Given the pointer resulting from evaluating the argument expression,  check that
>> +the logical tag and the allocation tags match.
> 
> Which logical tag and which allocation tag are being tested for a
> match here?
> 

The logical tag from the address/pointer given by 
@var{address_expression} is compared to the allocation tag for the same 
address.

How about the following?

"Check that the logical tag stored at the address given by
@var{address_expression} matches the allocation tag for the same address."


>> +When @value{GDBN} is debugging the AArch64 architecture, the program is
>> +using the v8.5-A feature Memory Tagging Extension (MTE) and there is support
>> +in the kernel for MTE, @value{GDBN} will make memory tagging functionality
>> +available for inspection and editing of logical and allocation tags.
> 
> Please add here a cross-reference to "Memory Tagging" subsection.
> 

Like this?

@cindex Memory Tagging

>> +To aid debugging, @value{GDBN} will output additional information when SIGSEGV
>> +signals are generated as a result of memory tag failures.
> 
> Can you add some minimal description of the additional information?
> 

Done now. I've added a couple small examples.

>> +A new register, @code{tag_ctl}, is made available through the
> 
> In what sense is this register "new"?  Perhaps you mean "special"?
> 
> Thanks.
> 

It is a new register set the kernel exposes to GDB when MTE is 
available. It is also a pseudo register that mimics a prctl setting. So 
it doesn't map 1:1 to a real register.

I agree it is a special register, and it is more meaningful than "new". 
In a few years, it won't be new anymore.