Re: [PATCH v3 1/4] GDB: Add gdb/arch/aarch64-pauth-linux.h

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Simon Marchi <simark@simark.ca> writes:

> On 2026-02-22 10:15, Luis wrote:
>> That's ok by me. Would you like to clarify what Thiago needs to do so we don't send mixed signals then?
>
> I think we can summarize it like this:
>
> - Definitions in arch/ should be namespaced / prefixed with AARCH64_
>   (e.g. AARCH64_HWCAP_FOO)
> - Definitions in nat/, which are there to allow building natively
>   against an older, should use the original name (e.g HWCAP_FOO)
> - -tdep files should use the AARCH64_-prefixed versions (e.g.
>   AARCH64_HWCAP_FOO)
> - -nat (in gdb) and -low (in gdbserver) should use the original name
>   (e.g. HWCAP_FOO)

Thank you for the clarification. I've done all of the above in v4.

Regarding the first point, I found some definitions in arch/ besides the
HWCAP ones which should have the AARCH64_ prefix, so I did that too:

- SEGV_MTEAERR → AARCH64_SEGV_MTEAERR
- SEGV_MTESERR → AARCH64_SEGV_MTESERR
- SEGV_CPERR → AARCH64_SEGV_CPERR
- PR_SHADOW_STACK_ENABLE → AARCH64_PR_SHADOW_STACK_ENABLE
- PR_SHADOW_STACK_WRITE → AARCH64_PR_SHADOW_STACK_WRITE
- PR_SHADOW_STACK_PUSH → AARCH64_PR_SHADOW_STACK_PUSH

Actually, the last four of them weren't even in a gdb/arch/ header, but
in gdb/linux-tdep.h. I moved them into gdb/arch/.

From your other email in this thread:

> On 2026-02-22 05:06, Luis wrote:
>> I don´t recall the discussions touching renaming/moving of these
>> constants. But I see your point. I just don´t think we should block
>> this series from going in on that particular snag. We should try a
>> refactoring as a follow on.
>> 
>
> This is what I was referring to, from Thiago:
>
>     Ok. While fixing this, I noticed that gdb/arch/aarch64-gcs-linux.h also
>     contains:
>
>       /* Feature check for Guarded Control Stack.  */
>       #ifndef HWCAP_GCS
>       #define HWCAP_GCS (1ULL << 32)
>       #endif
>
>     which can be said to be an implicit dependency on an arch-specific
>     include file (since it relies on such file defining or not HWCAP_GCS).
>
>     Also, on non-AArch64 systems it relies on them not having by coincidence
>     an unrelated hardware capability bit also named HWCAP_GCS...
>
>     I fixed this in v2 by renaming the macro to AARCH64_HWCAP_GCS and
>     defining it unconditionally (following the existing example of
>     AARCH64_HWCAP_PACA).
>
> From: https://inbox.sourceware.org/gdb-patches/87qzqkx8l1.fsf@linaro.org/T/#mbe0d3d5a2802f4f88abe907874ed44c063666639
>
> I think that was a good idea, and it seems like we went backwards from
> there?

I misunderstood this comment you made when you reviewed v2¹:

>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
>> index 028de981588b..b2dd192a7b56 100644
>> --- a/gdb/aarch64-linux-nat.c
>> +++ b/gdb/aarch64-linux-nat.c
>> @@ -51,7 +51,7 @@
>>  #include "gdb_proc_service.h"
>>  #include "arch-utils.h"
>>  
>> -#include "arch/aarch64-gcs-linux.h"
>> +#include "arch/aarch64-linux.h"
>>  #include "arch/aarch64-mte-linux.h"
>>  
>>  #include "nat/aarch64-mte-linux-ptrace.h"
>> @@ -1013,7 +1013,7 @@ aarch64_linux_nat_target::read_description ()
>>       active or not.  */
>>    features.vq = aarch64_sve_get_vq (tid);
>>    features.pauth = hwcap & AARCH64_HWCAP_PACA;
>> -  features.gcs = features.gcs_linux = hwcap & HWCAP_GCS;
>
> This one could stay HWCAP_GCS, but I don't have a preference.
>
> If we want to ensure / prove they are the same, we could add asserts
> like this in the nat/ header:
>
>   static_assert (AARCH64_HWCAP_GCS == HWCAP_GCS);
>
> That assert would only be checked when compiling natively for an aarch64
> system (where HWCAP_GCS is available), but it's better than nothing.

to mean that you didn't have a preference on whether to use
AARCH64_-prefixed definitions. I didn't notice that the comment was made
in a hunk from a -nat.c file. Sorry for the confusion.

-- 
Thiago

¹ https://inbox.sourceware.org/gdb-patches/5cebfbfa-a192-4359-97d9-83455d40fdc6@simark.ca/