Re: [PATCH v2 1/2] GDB: aarch64-linux: Reorganize gdb/arch/aarch64-gcs-linux.h

[Luis <luis.machado.foss@gmail.com>]

Hi,

First, aarch64's code could really use a refactoring in terms of 
cleaning up the headers and splitting things into their own files. With 
that said, we don´t need to do that now, but would be nice to do it 
eventually.

On 14/02/2026 04:54, Thiago Jung Bauermann wrote:
> This file implicitly depends on aarch64-linux's <asm/sigcontext.h> for
> the definition of GCS_MAGIC and on its <bits/hwcap.h> for the definition
> of HWCAP_GCS, but files on gdb/arch/ shouldn't depend on include files
> specific to the architecture.
> 
> Regarding GCS_MAGIC and struct user_gcs, move that code section to
> gdb/nat/aarch64-linux.h which can rely on <asm/sigcontext.h> and
> <asm/ptrace.h>.
> 
> To fix the use of struct user_gcs in gdb/aarch64-linuxt-dep.c, define a
> macro with the size of the GCS regset in aarch64-linux-tdep.h and use it
> in aarch64-linux-tdep.c, as is done for other regsets and following a
> suggestion from Simon Marchi.
> 
> Regarding HWCAP_GCS, rename it to AARCH64_HWCAP_GCS to ensure that it
> won't conflict with any system header definition and define it
> unconditionally, following the example of AARCH64_HWCAP_PACA.
> 
> Renaming HWCAP_GCS also avoids a potential (and I would say unlikely)
> problem on non-AArch64 systems: naming conflict if by coincidence they have
> an unrelated hardware capability bit also named HWCAP_GCS.
> 
> Finally, I noticed that AARCH64_HWCAP_PACA is duplicated in GDB and
> gdbserver files, so consolidate them in the same header and rename it to
> gdb/arch/aarch64-linux.h since it isn't just about GCS anymore.
> ---
> 
> As I mentioned on v1's email thread, this patch opens a small can of worms,
> since other gdb/arch/aarch64*.h do the same kind of thing (mostly with
> HWCAP macros but also a few others).
> 
> Perhaps I'm going too far with the HWCAP_GCS renaming and the current
> handling of it is fine?
> 
>   gdb/Makefile.in                               |  2 +-
>   gdb/aarch64-linux-nat.c                       |  4 +--
>   gdb/aarch64-linux-tdep.c                      |  9 +++---
>   gdb/aarch64-linux-tdep.h                      |  6 ++--
>   .../{aarch64-gcs-linux.h => aarch64-linux.h}  | 30 +++++--------------
>   gdb/nat/aarch64-linux.h                       | 16 ++++++++++
>   gdbserver/linux-aarch64-low.cc                |  7 ++---
>   7 files changed, 36 insertions(+), 38 deletions(-)
>   rename gdb/arch/{aarch64-gcs-linux.h => aarch64-linux.h} (61%)
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 2aa95be968ac..fd47c613966a 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1301,7 +1301,7 @@ HFILES_NO_SRCDIR = \
>   	amdgpu-tdep.h \
>   	annotate.h \
>   	arch/aarch32.h \
> -	arch/aarch64-gcs-linux.h \
> +	arch/aarch64-linux.h \

If there is anything feature-specific about things in aarch64-linux.h, 
I´d rather have a new featute-specific file here.

>   	arch/aarch64.h \
>   	arch/aarch64-insn.h \
>   	arch/aarch64-mte.h \
> 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"

Likewise above.

>   #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;
> +  features.gcs = features.gcs_linux = hwcap & AARCH64_HWCAP_GCS;
>     features.mte = hwcap2 & HWCAP2_MTE;
>     features.tls = aarch64_tls_register_count (tid);
>     /* SME feature check.  */
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index b85c25ecae1d..33e1b411ff1d 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -51,7 +51,7 @@
>   #include "record-full.h"
>   #include "linux-record.h"
>   
> -#include "arch/aarch64-gcs-linux.h"
> +#include "arch/aarch64-linux.h"

Likewise here.

 >   #include "arch/aarch64-mte.h">   #include "arch/aarch64-mte-linux.h"
>   #include "arch/aarch64-scalable-linux.h"
> @@ -1734,8 +1734,9 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
>   	  gcs_regmap, regcache_supply_regset, regcache_collect_regset
>   	};
>   
> -      cb (".reg-aarch-gcs", sizeof (user_gcs), sizeof (user_gcs),
> -	  &aarch64_linux_gcs_regset, "GCS registers", cb_data);
> +      cb (".reg-aarch-gcs", AARCH64_LINUX_SIZEOF_GCS_REGSET,
> +	  AARCH64_LINUX_SIZEOF_GCS_REGSET, &aarch64_linux_gcs_regset,
> +	  "GCS registers", cb_data);
>       }
>   }
>   
> @@ -1761,7 +1762,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
>        length.  */
>     features.vq = aarch64_linux_core_read_vq_from_sections (gdbarch, abfd);
>     features.pauth = hwcap & AARCH64_HWCAP_PACA;
> -  features.gcs = features.gcs_linux = hwcap & HWCAP_GCS;
> +  features.gcs = features.gcs_linux = hwcap & AARCH64_HWCAP_GCS;
>     features.mte = hwcap2 & HWCAP2_MTE;
>     features.fpmr = hwcap2 & HWCAP2_FPMR;
>   
> diff --git a/gdb/aarch64-linux-tdep.h b/gdb/aarch64-linux-tdep.h
> index e926687ff6eb..f247d6aa72cd 100644
> --- a/gdb/aarch64-linux-tdep.h
> +++ b/gdb/aarch64-linux-tdep.h
> @@ -39,10 +39,10 @@
>   /* The MTE regset consists of a 64-bit register.  */
>   #define AARCH64_LINUX_SIZEOF_MTE_REGSET (8)
>   
> +/* The GCS regset consists of 3 64-bit registers.  */
> +#define AARCH64_LINUX_SIZEOF_GCS_REGSET (3 * 8)
> +

The above is the kind of feature-specific data we should put into 
arch/aarch64-linux-<feature>.[c|h]. At the moment not all of it follows 
this particular pattern, and that's annoying. But we should strive to 
clean it up.

>   extern const struct regset aarch64_linux_gregset;
>   extern const struct regset aarch64_linux_fpregset;
>   
> -/* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
> -#define AARCH64_HWCAP_PACA (1 << 30)
> -

The above is older code, and that bit is likely available out there. We 
should do the same treatment to it as the others. That is, if it is not 
defined, define it. Otherwise use whatever is defined.

>   #endif /* GDB_AARCH64_LINUX_TDEP_H */
> diff --git a/gdb/arch/aarch64-gcs-linux.h b/gdb/arch/aarch64-linux.h
> similarity index 61%
> rename from gdb/arch/aarch64-gcs-linux.h
> rename to gdb/arch/aarch64-linux.h
> index b31fc32daa03..a33c082a0318 100644
> --- a/gdb/arch/aarch64-gcs-linux.h
> +++ b/gdb/arch/aarch64-linux.h
> @@ -1,4 +1,4 @@
> -/* Common Linux target-dependent definitions for AArch64 GCS
> +/* Common Linux target-dependent definitions for AArch64 running Linux.
 >   >      Copyright (C) 2025-2026 Free Software Foundation, Inc.
>   
> @@ -17,28 +17,12 @@
>      You should have received a copy of the GNU General Public License
>      along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>   
> -#ifndef GDB_ARCH_AARCH64_GCS_LINUX_H
> -#define GDB_ARCH_AARCH64_GCS_LINUX_H
> -
> -#include <stdint.h>
> +#ifndef GDB_ARCH_AARCH64_LINUX_H
> +#define GDB_ARCH_AARCH64_LINUX_H
>   
> +/* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
> +#define AARCH64_HWCAP_PACA (1 << 30)

We should either move all of the definitions to arch/aarch64-linux.h or 
move them to their own separate feature files. See mte/sve/sme for instance.

>   /* Feature check for Guarded Control Stack.  */
> -#ifndef HWCAP_GCS
> -#define HWCAP_GCS (1ULL << 32)
> -#endif
> -
> -/* Make sure we only define these if the kernel header doesn't.  */
> -#ifndef GCS_MAGIC
> -
> -/* GCS state (NT_ARM_GCS).  */
> -
> -struct user_gcs
> -{
> -  uint64_t features_enabled;
> -  uint64_t features_locked;
> -  uint64_t gcspr_el0;
> -};
> -
> -#endif /* GCS_MAGIC */
> +#define AARCH64_HWCAP_GCS (1ULL << 32)
>   
> -#endif /* GDB_ARCH_AARCH64_GCS_LINUX_H */
> +#endif /* GDB_ARCH_AARCH64_LINUX_H */
> diff --git a/gdb/nat/aarch64-linux.h b/gdb/nat/aarch64-linux.h
> index bb5381eba3ef..f7c09aa112bf 100644
> --- a/gdb/nat/aarch64-linux.h
> +++ b/gdb/nat/aarch64-linux.h
> @@ -19,7 +19,9 @@
>   #ifndef GDB_NAT_AARCH64_LINUX_H
>   #define GDB_NAT_AARCH64_LINUX_H
>   
> +#include <stdint.h>
>   #include <signal.h>
> +#include <asm/ptrace.h>
>   
>   /* Defines ps_err_e, struct ps_prochandle.  */
>   #include "gdb_proc_service.h"
> @@ -113,6 +115,20 @@ typedef struct compat_siginfo
>   #define cpt_si_band _sifields._sigpoll._band
>   #define cpt_si_fd _sifields._sigpoll._fd
>   
> +/* Make sure we only define these if the kernel header doesn't.  */
> +#ifndef GCS_MAGIC
> +
> +/* GCS state (NT_ARM_GCS).  */
> +
> +struct user_gcs
> +{
> +  uint64_t features_enabled;
> +  uint64_t features_locked;
> +  uint64_t gcspr_el0;
> +};
> +
> +#endif /* GCS_MAGIC */
> +

To keep things clean I´d define the above in a separare file for the 
feature. See gdb/nat/aarch64-mte-linux-ptrace.[h|c] for instance.


>   void aarch64_siginfo_from_compat_siginfo (siginfo_t *to,
>   					    compat_siginfo_t *from);
>   void aarch64_compat_siginfo_from_siginfo (compat_siginfo_t *to,
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index b19e605f55d6..60ea50f609cc 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -39,7 +39,7 @@
>   
>   #include "gdb_proc_service.h"
>   #include "arch/aarch64.h"
> -#include "arch/aarch64-gcs-linux.h"
> +#include "arch/aarch64-linux.h"

See the comment below about having per-feature files. I think having 
per-feature files is less work at this point.

>   #include "arch/aarch64-mte-linux.h"
>   #include "arch/aarch64-scalable-linux.h"
>   #include "linux-aarch32-tdesc.h"
> @@ -984,9 +984,6 @@ aarch64_adjust_register_sets (const struct aarch64_features &features)
>       }
>   }
>   
> -/* Matches HWCAP_PACA in kernel header arch/arm64/include/uapi/asm/hwcap.h.  */
> -#define AARCH64_HWCAP_PACA (1 << 30)
> -
>   /* Implementation of linux target ops method "low_arch_setup".  */
>   
>   void
> @@ -1009,7 +1006,7 @@ aarch64_target::low_arch_setup ()
>         /* A-profile MTE is 64-bit only.  */
>         features.mte = linux_get_hwcap2 (pid, 8) & HWCAP2_MTE;
>         features.tls = aarch64_tls_register_count (tid);
> -      features.gcs = features.gcs_linux = linux_get_hwcap (pid, 8) & HWCAP_GCS;
> +      features.gcs = features.gcs_linux = linux_get_hwcap (pid, 8) & AARCH64_HWCAP_GCS;
>         features.fpmr = linux_get_hwcap2 (pid, 8) & HWCAP2_FPMR;
>   
>         /* Scalable Matrix Extension feature and size check.  */

Checking arch/aarch64.h, HWCAP2_FPMR looks out of place as well. So 
yeah, overall these files need a bit of a cleanup.