| Message ID | 20240416140728.198163-9-gustavo.romero@linaro.org |
|---|---|
| State | Superseded |
| 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, > eliz@gnu.org, > tom@tromey.com, > gustavo.romero@linaro.org > Date: Tue, 16 Apr 2024 14:07:28 +0000 > > This commit documents the qIsAddressTagged packet. Thanks. > diff --git a/gdb/NEWS b/gdb/NEWS > index feb3a37393a..1693a7a15f8 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -192,6 +192,16 @@ QThreadOptions in qSupported > QThreadOptions packet, and the qSupported response can contain the > set of thread options the remote stub supports. > > +qIsAddressTagged > + 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 This part is okay. > +@item qIsAddressTagged:@var{address} > +@cindex check if a given address is in a memory tagged region. ^ Index entries should not end in a period. > +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 I suggest to use @dfn{tagged}, since this is new terminology. It will then look nicer in print (and will be quoted in Info). > +@item @var{01} > +Address @var{address} is tagged. > + > +@item @var{00} > +Address @var{address} is not tagged. I think 01 and 00 should not be in @var here, since they are literal strings. The @samp markup of the @table will do here. > +@item E @var{nn} > +An error occurred. This means that address could not be checked for some > +reason. Here "nn" is the error value, right? If so, I suggest to say @item E @var{nn} An error occurred whose code is @var{nn}. Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Hi Eli, Thanks a lot for the review. I just have one question. On 4/16/24 11:34 AM, Eli Zaretskii wrote: >> From: Gustavo Romero <gustavo.romero@linaro.org> >> Cc: luis.machado@arm.com, >> thiago.bauermann@linaro.org, >> eliz@gnu.org, >> tom@tromey.com, >> gustavo.romero@linaro.org >> Date: Tue, 16 Apr 2024 14:07:28 +0000 >> >> This commit documents the qIsAddressTagged packet. > > Thanks. > >> diff --git a/gdb/NEWS b/gdb/NEWS >> index feb3a37393a..1693a7a15f8 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -192,6 +192,16 @@ QThreadOptions in qSupported >> QThreadOptions packet, and the qSupported response can contain the >> set of thread options the remote stub supports. >> >> +qIsAddressTagged >> + 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 > > This part is okay. > >> +@item qIsAddressTagged:@var{address} >> +@cindex check if a given address is in a memory tagged region. > ^ > Index entries should not end in a period. > >> +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 > > I suggest to use @dfn{tagged}, since this is new terminology. It will > then look nicer in print (and will be quoted in Info). > >> +@item @var{01} >> +Address @var{address} is tagged. >> + >> +@item @var{00} >> +Address @var{address} is not tagged. > > I think 01 and 00 should not be in @var here, since they are literal > strings. The @samp markup of the @table will do here. > >> +@item E @var{nn} >> +An error occurred. This means that address could not be checked for some >> +reason. > > Here "nn" is the error value, right? If so, I suggest to say > > @item E @var{nn} > An error occurred whose code is @var{nn}. Do you mean remove the "This means that address could not be checked for some reason." text completely or just s/An error occurred/An error occurred whose code is @var{nn}/? I'd like to keep the text because the error codes actually don't tell much about the reason of the error -- usually -- in this context, so I'd like to inform users that in any case the address could not be checked if an error (any error) occurs. For example, in the gdbserver we can find comments like: gdbserver/remote-utils.cc-1020-void gdbserver/remote-utils.cc-1021-write_enn (char *buf) gdbserver/remote-utils.cc-1022-{ gdbserver/remote-utils.cc:1023: /* Some day, we should define the meanings of the error codes... */ gdbserver/remote-utils.cc-1024- buf[0] = 'E'; gdbserver/remote-utils.cc-1025- buf[1] = '0'; gdbserver/remote-utils.cc-1026- buf[2] = '1'; gdbserver/remote-utils.cc-1027- buf[3] = '\0'; gdbserver/remote-utils.cc-1028-} Cheers, Gustavo
> Cc: gdb-patches@sourceware.org, luis.machado@arm.com, > thiago.bauermann@linaro.org, tom@tromey.com > From: Gustavo Romero <gustavo.romero@linaro.org> > Date: Tue, 16 Apr 2024 20:10:43 -0300 > > Hi Eli, > > Thanks a lot for the review. I just have one question. > > >> +@item E @var{nn} > >> +An error occurred. This means that address could not be checked for some > >> +reason. > > > > Here "nn" is the error value, right? If so, I suggest to say > > > > @item E @var{nn} > > An error occurred whose code is @var{nn}. > > Do you mean remove the "This means that address could not be checked for some reason." > text completely or just s/An error occurred/An error occurred whose code is @var{nn}/? The latter.
On 4/17/24 9:09 AM, Eli Zaretskii wrote: >> Cc: gdb-patches@sourceware.org, luis.machado@arm.com, >> thiago.bauermann@linaro.org, tom@tromey.com >> From: Gustavo Romero <gustavo.romero@linaro.org> >> Date: Tue, 16 Apr 2024 20:10:43 -0300 >> >> Hi Eli, >> >> Thanks a lot for the review. I just have one question. >> >>>> +@item E @var{nn} >>>> +An error occurred. This means that address could not be checked for some >>>> +reason. >>> >>> Here "nn" is the error value, right? If so, I suggest to say >>> >>> @item E @var{nn} >>> An error occurred whose code is @var{nn}. >> >> Do you mean remove the "This means that address could not be checked for some reason." >> text completely or just s/An error occurred/An error occurred whose code is @var{nn}/? > > The latter. Thanks for the clarification! Cheers, Gustavo
diff --git a/gdb/NEWS b/gdb/NEWS index feb3a37393a..1693a7a15f8 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -192,6 +192,16 @@ QThreadOptions in qSupported QThreadOptions packet, and the qSupported response can contain the set of thread options the remote stub supports. +qIsAddressTagged + 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..cb6033bbbee 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -44093,6 +44093,35 @@ 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 qIsAddressTagged:@var{address} +@cindex check if a given address is in a memory tagged region. +@cindex @samp{qIsAddressTagged} packet +@anchor {qIsAddressTagged} +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 to this packet should all be in two hex digit format, as follows: + +@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{qIsAddressTagged} is not supported by the +stub. +@end table + @item QMemTags:@var{start address},@var{length}:@var{type}:@var{tag bytes} @anchor{QMemTags} @cindex store memory tags @@ -45141,9 +45170,11 @@ 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{qIsAddressTagged} (@pxref{qIsAddressTagged}) packet +is not supported by the stub. Access to the @file{/proc/@var{pid}/smaps} +file is done via @samp{vFile} requests. @end table