| Message ID | 20240404064819.2848899-8-gustavo.romero@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add another way to check tagged addresses on remote targets | expand |
> From: Gustavo Romero <gustavo.romero@linaro.org> > Cc: luis.machado@arm.com, thiago.bauermann@linaro.org, > gustavo.romero@linaro.org > Date: Thu, 4 Apr 2024 06:48:19 +0000 > > This commit documents the 'qMemTagCheckAddr' packet and the associated > feature 'memory-tagging-check-addr' that informs if this packet is > supported by the target. > > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> > --- > gdb/NEWS | 9 ++++++++ > gdb/doc/gdb.texinfo | 50 ++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 56 insertions(+), 3 deletions(-) Thanks. > diff --git a/gdb/NEWS b/gdb/NEWS > index feb3a37393a..1158715f41c 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -192,6 +192,15 @@ QThreadOptions in qSupported > QThreadOptions packet, and the qSupported response can contain the > set of thread options the remote stub supports. > > +qMemTagCheckAddr > + This new packet allows GDB to query the stub about a given address to check if > + it is tagged or not. Many memory tagging-related GDB commands need to perform > + this check before they read/write the allocation tag related to an address. > + Currently, however, this is done through a 'vFile' request to read the file > + /proc/<PID>/smaps and check if the address is in a region reported as memory > + tagged. Since not all targets have a notion of what the smaps file is about, > + this new packet provides a more generic way to perform such a check. Please reformat this text so that each line is shorter than 78 columns. Also, please leave two spaces between sentences, per our conventions. > +@item qMemTagCheckAddr:@var{address} > +@anchor {qMemTagCheckAddr} > +@cindex check if a given address is in a memory tagged region. > +@cindex @samp{qMemTagCheckAddr} packet > +Check if address @var{address} is in a memory tagged region; if it is, it's said > +to be tagged. The target is responsible for checking it, as this is > +architecture-specific. Likewise here: please make the lines shorter. Also, the @cindex lines should come _before_ the @item line, not after it, so that the index entry records the location before the @item. > +Reply: > +@table @samp > +Replies are in a 2 hex digits format. This should be before @table, like this: Replies to this packet should all be in two hex digit format, as follows: @item @var{01} > +stub. Access to the @file{/proc/@var{pid}/smaps} file is done via @samp{vFile} ^^ Two spaces between sentences, please. Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>>>>> "Gustavo" == Gustavo Romero <gustavo.romero@linaro.org> writes:
Gustavo> +@item @w{}
Gustavo> +An empty reply indicates that @samp{qMemTagCheckAddr} is not supported by the
Gustavo> +stub. This situation should not occur because @value{GDBN} will only send this
Gustavo> +packet if the stub has advertised support for memory tagging check via the
Gustavo> +@samp{memory-tagging-check-addr} feature in @samp{qSupported}.
Is querying really needed in this case?
Like, if there is some user feature that requires knowing whether this
work before ever trying it, then I guess that would be a good
justification. In other cases, it seems to me that simply trying to use
a packet is better than a qSupported response; or at least I don't know
why it wouldn't be.
Tom
Hi Tom, On 4/8/24 4:37 PM, Tom Tromey wrote: >>>>>> "Gustavo" == Gustavo Romero <gustavo.romero@linaro.org> writes: > > Gustavo> +@item @w{} > Gustavo> +An empty reply indicates that @samp{qMemTagCheckAddr} is not supported by the > Gustavo> +stub. This situation should not occur because @value{GDBN} will only send this > Gustavo> +packet if the stub has advertised support for memory tagging check via the > Gustavo> +@samp{memory-tagging-check-addr} feature in @samp{qSupported}. > > Is querying really needed in this case? > Like, if there is some user feature that requires knowing whether this > work before ever trying it, then I guess that would be a good > justification. In other cases, it seems to me that simply trying to use > a packet is better than a qSupported response; or at least I don't know > why it wouldn't be. That's right, I think we are in sync here. Luis communicated to me last week (private conversation) about this possibility, hence for v4 we just try to send the qMemTagCheckAddr packet, and if it fails (empty reply) there is a fallback to the current code path, which reads the smaps. So I'm dropping the memory-tagging-check-addr feature. Thanks for review! Cheers, Gustavo
>>>>> "Gustavo" == Gustavo Romero <gustavo.romero@linaro.org> writes: >> Is querying really needed in this case? >> Like, if there is some user feature that requires knowing whether this >> work before ever trying it, then I guess that would be a good >> justification. In other cases, it seems to me that simply trying to use >> a packet is better than a qSupported response; or at least I don't know >> why it wouldn't be. Gustavo> That's right, I think we are in sync here. Luis communicated to me last week Gustavo> (private conversation) about this possibility, hence for v4 we just try to Gustavo> send the qMemTagCheckAddr packet, and if it fails (empty reply) there is a Gustavo> fallback to the current code path, which reads the smaps. So I'm dropping Gustavo> the memory-tagging-check-addr feature. Great, thank you. TBH I am not sure if my view on the desirability of probing versus qSupported is the correct one. Like, there may be counterexamples I'm unaware of. Tom
On 4/9/24 16:33, Tom Tromey wrote: >>>>>> "Gustavo" == Gustavo Romero <gustavo.romero@linaro.org> writes: > >>> Is querying really needed in this case? >>> Like, if there is some user feature that requires knowing whether this >>> work before ever trying it, then I guess that would be a good >>> justification. In other cases, it seems to me that simply trying to use >>> a packet is better than a qSupported response; or at least I don't know >>> why it wouldn't be. > > Gustavo> That's right, I think we are in sync here. Luis communicated to me last week > Gustavo> (private conversation) about this possibility, hence for v4 we just try to > Gustavo> send the qMemTagCheckAddr packet, and if it fails (empty reply) there is a > Gustavo> fallback to the current code path, which reads the smaps. So I'm dropping > Gustavo> the memory-tagging-check-addr feature. > > Great, thank you. > > TBH I am not sure if my view on the desirability of probing versus > qSupported is the correct one. Like, there may be counterexamples I'm > unaware of. We already have a qSupported string that reports if memory tagging is supported by the remote (memory-tagging+). So before we send this new packet, gdb can check if it makes sense to send it. If the target supports memory tagging, then the answer is yes. Otherwise no. If we didn't have "memory-tagging+", I think it would make sense to have a feature string in place, because then a remote can support the packet itself, but not necessarily support checking if a particular address is within a tagged memory range. So probing seems fine in this case. In the future gdbserver may implement it as well. Until then, an empty reply from gdbserver will tell gdb this method of querying is not supported.
diff --git a/gdb/NEWS b/gdb/NEWS index feb3a37393a..1158715f41c 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -192,6 +192,15 @@ QThreadOptions in qSupported QThreadOptions packet, and the qSupported response can contain the set of thread options the remote stub supports. +qMemTagCheckAddr + This new packet allows GDB to query the stub about a given address to check if + it is tagged or not. Many memory tagging-related GDB commands need to perform + this check before they read/write the allocation tag related to an address. + Currently, however, this is done through a 'vFile' request to read the file + /proc/<PID>/smaps and check if the address is in a region reported as memory + tagged. Since not all targets have a notion of what the smaps file is about, + this new packet provides a more generic way to perform such a check. + *** Changes in GDB 14 * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 82a617e9ad3..afd57ff874c 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -44093,6 +44093,37 @@ although this should not happen given @value{GDBN} will only send this packet if the stub has advertised support for memory tagging via @samp{qSupported}. @end table +@item qMemTagCheckAddr:@var{address} +@anchor {qMemTagCheckAddr} +@cindex check if a given address is in a memory tagged region. +@cindex @samp{qMemTagCheckAddr} packet +Check if address @var{address} is in a memory tagged region; if it is, it's said +to be tagged. The target is responsible for checking it, as this is +architecture-specific. + +@var{address} is the address to be checked. + +Reply: +@table @samp +Replies are in a 2 hex digits format. + +@item @var{01} +Address @var{address} is tagged. + +@item @var{00} +Address @var{address} is not tagged. + +@item E @var{nn} +An error occurred. This means that address could not be checked for some +reason. + +@item @w{} +An empty reply indicates that @samp{qMemTagCheckAddr} is not supported by the +stub. This situation should not occur because @value{GDBN} will only send this +packet if the stub has advertised support for memory tagging check via the +@samp{memory-tagging-check-addr} feature in @samp{qSupported}. +@end table + @item QMemTags:@var{start address},@var{length}:@var{type}:@var{tag bytes} @anchor{QMemTags} @cindex store memory tags @@ -44914,6 +44945,11 @@ These are the currently defined stub features and their properties: @tab @samp{-} @tab No +@item @samp{memory-tagging-check-addr} +@tab No +@tab @samp{-} +@tab No + @end multitable These are the currently defined stub features, in more detail: @@ -45141,9 +45177,17 @@ The remote stub supports and implements the required memory tagging functionality and understands the @samp{qMemTags} (@pxref{qMemTags}) and @samp{QMemTags} (@pxref{QMemTags}) packets. -For AArch64 GNU/Linux systems, this feature also requires access to the -@file{/proc/@var{pid}/smaps} file so memory mapping page flags can be inspected. -This is done via the @samp{vFile} requests. +For AArch64 GNU/Linux systems, this feature can require access to the +@file{/proc/@var{pid}/smaps} file so memory mapping page flags can be inspected, +if @samp{memory-tagging-check-addr} feature (see below) is not supported by the +stub. Access to the @file{/proc/@var{pid}/smaps} file is done via @samp{vFile} +requests. + +@item memory-tagging-check-addr +The remote stub supports and implements the required memory tagging address +check functionality and understands the @samp{qMemTagCheckAddr} +(@pxref{qMemTagCheckAddr}) packet, which is used for address checking instead +of accessing the @file{/proc/@var{pid}/smaps} file. @end table
This commit documents the 'qMemTagCheckAddr' packet and the associated feature 'memory-tagging-check-addr' that informs if this packet is supported by the target. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> --- gdb/NEWS | 9 ++++++++ gdb/doc/gdb.texinfo | 50 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 3 deletions(-)