Re: [PATCH] [gdb/aarch64] pauth: Create new feature string for pauth to prevent crashing older gdb's

[Luis Machado <luis.machado@arm.com>]

Hi Eli,

On 4/12/23 07:04, Eli Zaretskii wrote:
>> CC: <alex.bennee@linaro.org>, <richard.henderson@linaro.org>,
>>   <peter.maydell@linaro.org>
>> Date: Tue, 11 Apr 2023 23:43:27 +0100
>> From: Luis Machado via Gdb-patches <gdb-patches@sourceware.org>
>>
>> I've added some documentation to explain the situation.
>> ---
>>   gdb/aarch64-tdep.c  | 15 ++++++++++++++-
>>   gdb/doc/gdb.texinfo | 16 ++++++++++++++++
>>   2 files changed, 30 insertions(+), 1 deletion(-)
> 
> Thanks.
> 
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 45a0580bc29..aefeb63f75c 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -47980,6 +47980,22 @@ has a signed link register value that needs to be unmasked/decoded.
>>   Extra registers are allowed in this feature, but they will not affect
>>   @value{GDBN}.
>>   
>> +Due to a bug in previous versions of @value{GDBN} (versions 9, 10, 11 and 12),
>> +a new feature string was created to prevent targets causing a @value{GDBN}
>> +crash whenever they reported support for Pointer Authentication and also
>> +reported additional system registers that were not accounted for by
>> +@value{GDBN}.  This is common when using emulators and on bare-metal debugging
>> +scenarios.
>> +
>> +The new feature string is @samp{org.gnu.gdb.aarch64.pauth_v2}, and it has
>> +the same contents as feature string @samp{org.gnu.gdb.aarch64.pauth}.
>> +
>> +Targets reporting Pointer Authentication support via the remote protocol
>> +should always use the newer feature string @samp{org.gnu.gdb.aarch64.pauth_v2}.
>> +
>> +In the future the @samp{org.gnu.gdb.aarch64.pauth} feature string might be
>> +deprecated in favor of @samp{org.gnu.gdb.aarch64.pauth_v2}.
>> +
> 
> Since this is the manual for GDB users, it should describe the feature
> first, and explain its use and rationale after that.  So this
> description is backwards: it starts with the reason for introduction
> of this feature, something that doesn't necessarily make sense to the
> reader of the manual.
> 
> Instead, we should first say that 'org.gnu.gdb.aarch64.pauth' will be
> deprecated in favor of 'org.gnu.gdb.aarch64.pauth_v2', then introduce
> 'org.gnu.gdb.aarch64.pauth_v2' saying that it has the same contents as
> the former, and finally explain that targets should use
> 'org.gnu.gdb.aarch64.pauth_v2' to avoid the danger of crashing older
> GDBs.
> 
> OK?

It sounds OK.

How about the following flow?

In the future the @samp{org.gnu.gdb.aarch64.pauth} feature string might be
deprecated in favor of feature string @samp{org.gnu.gdb.aarch64.pauth_v2}.

The @samp{org.gnu.gdb.aarch64.pauth_v2} feature has the exact same contents
as feature @samp{org.gnu.gdb.aarch64.pauth}.

Targets reporting Pointer Authentication support via the remote protocol
should always use the newer feature string
@samp{org.gnu.gdb.aarch64.pauth_v2}.

The reason for having feature @samp{org.gnu.gdb.aarch64.pauth_v2} is a bug in
previous versions of @value{GDBN} (versions 9, 10, 11 and 12).  This bug
caused @value{GDBN} to crash whenever the target reported support for Pointer
Authentication (using feature string @samp{org.gnu.gdb.aarch64.pauth}) and also
reported additional system registers that were not accounted for by
@value{GDBN}.  This is common when using emulators and on bare-metal debugging
scenarios.

> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>