Re: [PATCH v5] gdb: align siginfo_t with the Linux kernel definition

[Simon Marchi <simark@simark.ca>]

On 7/28/26 8:32 AM, Matthieu Longo wrote:
> GDB's current definition of siginfo_t is missing many fields present in
> the Linux kernel definition [1].
> 
> These fields are useful for providing detailed, user-friendly diagnostics
> when a fault occurs. Some new AArch64 extensions, such as Permission
> Overlay Enhancement used to implement Protection Keys [2], require the
> debugger to inspect 'si_pkey' alongside 'si_addr' to help the user identify
> the problematic key.
> 
> This patch aligns GDB's definition of the __sifields._sigfault member of
> siginfo_t with the definition from the Linux kernel master branch.
> 
> To avoid hardcoding the field access paths throughout the codebase, this
> patch also introduces compile-time accessors for the siginfo_t attributes,
> centralizing their definitions in a single location and making future
> updates easier.
> 
> Finally, extend the testsuite to verify access to the new si_pkey field
> and its preservation when modifying $_siginfo and when reading core files.
> The tests in siginfo-obj.exp rely on the siginfo_t definition provided by
> glibc's <signal.h>, which does not yet expose all of the fields present in
> the kernel definition. As a result, the tests cannot exercise every newly
> added field and therefore focus on si_pkey, the field motivating this change.
> The test validates that GDB can read and modify the field correctly; it does
> not attempt to generate a real protection-key fault.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
>      tree/include/uapi/asm-generic/siginfo.h#n69
> [2]: https://lore.kernel.org/all/20160212210213.ABC488FA@viggo.jf.intel.com/
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> ---
>  gdb/aarch64-linux-tdep.c               |  8 ++--
>  gdb/linux-tdep.c                       | 52 +++++++++++++++++++---
>  gdb/linux-tdep.h                       | 60 ++++++++++++++++++++++++++
>  gdb/sparc64-linux-tdep.c               |  6 ++-
>  gdb/testsuite/gdb.base/siginfo-obj.c   |  1 +
>  gdb/testsuite/gdb.base/siginfo-obj.exp | 14 ++++++
>  6 files changed, 131 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index f11eccc1bc1..235b35bcfb4 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -2683,13 +2683,15 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>  
>    try
>      {
> +      using gdb_si = gdb::siginfo_type;
> +      using si_key = gdb::siginfo_type::key;
>        /* Sigcode tells us if the segfault is actually a memory tag
>  	 violation.  */
> -      si_code = parse_and_eval_long ("$_siginfo.si_code");
> -      si_errno = parse_and_eval_long ("$_siginfo.si_errno");
> +      si_code = parse_and_eval_long (gdb_si::get (si_key::siginfo_code));
> +      si_errno = parse_and_eval_long (gdb_si::get (si_key::siginfo_errno));
>  
>        fault_addr
> -	= parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
> +	= parse_and_eval_long (gdb_si::get (si_key::siginfo_addr));
>      }
>    catch (const gdb_exception_error &exception)
>      {
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 25d625db595..740043a9292 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -272,10 +272,9 @@ static struct type *
>  linux_get_siginfo_type (struct gdbarch *gdbarch)
>  {
>    struct linux_gdbarch_data *linux_gdbarch_data;
> -  struct type *void_ptr_type;
>    struct type *uid_type, *pid_type;
>    struct type *sigval_type, *clock_type;
> -  struct type *siginfo_type, *sifields_type;
> +  struct type *siginfo_type, *sifields_type, *sigfault_union_type;
>    struct type *type;
>  
>    linux_gdbarch_data = get_linux_gdbarch_data (gdbarch);
> @@ -285,11 +284,22 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>    type_allocator alloc (gdbarch);
>  
>    const struct builtin_type *builtin_types = builtin_type (gdbarch);
> +  struct type *short_type = builtin_types->builtin_short;
>    struct type *int_type = builtin_types->builtin_int;
>    struct type *uint_type = builtin_types->builtin_unsigned_int;
>    struct type *long_type = builtin_types->builtin_long;
> -
> -  void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
> +  struct type *unsigned_long_type = builtin_types->builtin_unsigned_long;
> +  struct type *uint32_type = builtin_types->builtin_uint32;
> +  struct type *void_ptr_type
> +    = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
> +
> +  /* Compute padding length, i.e. __ADDR_BND_PKEY_PAD.  */
> +  unsigned alignof_void_ptr = type_align (void_ptr_type);
> +  unsigned padding_size = (alignof_void_ptr < short_type->length ()
> +			   ? short_type->length ()
> +			   : alignof_void_ptr);
> +  struct type *addr_bnd_pkey_padding_type
> +    = init_vector_type (builtin_types->builtin_uint8, padding_size);
>  
>    /* sival_t */
>    sigval_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
> @@ -364,9 +374,41 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>    append_composite_type_field (type, "si_stime", clock_type);
>    append_composite_type_field (sifields_type, "_sigchld", type);
>  
> -  /* _sigfault */
> +  /* Begin _sigfault's anonymous union.  */
> +  sigfault_union_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
> +  /* used on alpha and sparc */
> +  append_composite_type_field (sigfault_union_type, "si_trapno", int_type);
> +  /* used when si_code is BUS_MCEERR_AR or BUS_MCEERR_AO.  */
> +  append_composite_type_field (sigfault_union_type, "si_addr_lsb", short_type);
> +
> +  /* used when si_code=SEGV_BNDERR */
> +  type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  append_composite_type_field (type, "_dummy_bnd", addr_bnd_pkey_padding_type);
> +  append_composite_type_field (type, "si_lower", void_ptr_type);
> +  append_composite_type_field (type, "si_upper", void_ptr_type);
> +  append_composite_type_field (sigfault_union_type, "_addr_bnd", type);
> +
> +  /* used when si_code=SEGV_PKUERR */
> +  type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  append_composite_type_field (type, "_dummy_pkey", addr_bnd_pkey_padding_type);
> +  append_composite_type_field (type, "si_pkey", uint32_type);
> +  append_composite_type_field (sigfault_union_type, "_addr_pkey", type);
> +
> +  /* used when si_code=TRAP_PERF */
> +  type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  append_composite_type_field (type, "si_perf_data", unsigned_long_type);
> +  append_composite_type_field (type, "si_perf_type", uint32_type);
> +  append_composite_type_field (type, "si_perf_flags", uint32_type);
> +  append_composite_type_field (sigfault_union_type, "_perf", type);
> +
> +  /* End _sigfault's anonymous union.  */
> +
> +  /* _sigfault is set by SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT */
>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>    append_composite_type_field (type, "si_addr", void_ptr_type);
> +  /* Since there is no possibility to declare an anonymous union,
> +     using '_anon_union' instead.  */
> +  append_composite_type_field (type, "_anon_union", sigfault_union_type);

Can you expand on why it's not possible to have an anonymous union?  It
is certainly possible to have anonymous unions described in DWARF, which
are then translated to struct types.

I think that the ideal user experience would be for users to be able to
access fields the same way that they do in the code, that is
`si.si_pkey`.  All the _sigfault/_addr_pkey/etc parts are implementation
details that could change.

On top of your patch, if I just delete all the internal field names, it
seems to work just fine, see patch below.  In the end it models
something like this in C:

struct siginto_t
{
  union
  {
    // kill
    struct
    {
      int si_pid;
      int si_uid;
    };

    // timer
    struct
    {
      int si_tid;
      int si_overrun;
      int si_sys_private;
    };

    ...
  };
};


diff --git i/gdb/linux-tdep.c w/gdb/linux-tdep.c
index 4660772752d6..f21fa1e18578 100644
--- i/gdb/linux-tdep.c
+++ w/gdb/linux-tdep.c
@@ -349,21 +349,21 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
   append_composite_type_field (type, "si_pid", pid_type);
   append_composite_type_field (type, "si_uid", uid_type);
-  append_composite_type_field (sifields_type, "_kill", type);
+  append_composite_type_field (sifields_type, "", type);

   /* _timer */
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
   append_composite_type_field (type, "si_tid", int_type);
   append_composite_type_field (type, "si_overrun", int_type);
   append_composite_type_field (type, "si_sigval", sigval_type);
-  append_composite_type_field (sifields_type, "_timer", type);
+  append_composite_type_field (sifields_type, "", type);

   /* _rt */
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
   append_composite_type_field (type, "si_pid", pid_type);
   append_composite_type_field (type, "si_uid", uid_type);
   append_composite_type_field (type, "si_sigval", sigval_type);
-  append_composite_type_field (sifields_type, "_rt", type);
+  append_composite_type_field (sifields_type, "", type);

   /* _sigchld */
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
@@ -372,7 +372,7 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
   append_composite_type_field (type, "si_status", int_type);
   append_composite_type_field (type, "si_utime", clock_type);
   append_composite_type_field (type, "si_stime", clock_type);
-  append_composite_type_field (sifields_type, "_sigchld", type);
+  append_composite_type_field (sifields_type, "", type);

   /* Begin _sigfault's anonymous union.  */
   sigfault_union_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
@@ -386,20 +386,20 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
   append_composite_type_field (type, "_dummy_bnd", addr_bnd_pkey_padding_type);
   append_composite_type_field (type, "si_lower", void_ptr_type);
   append_composite_type_field (type, "si_upper", void_ptr_type);
-  append_composite_type_field (sigfault_union_type, "_addr_bnd", type);
+  append_composite_type_field (sigfault_union_type, "", type);

   /* used when si_code=SEGV_PKUERR */
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
   append_composite_type_field (type, "_dummy_pkey", addr_bnd_pkey_padding_type);
   append_composite_type_field (type, "si_pkey", uint32_type);
-  append_composite_type_field (sigfault_union_type, "_addr_pkey", type);
+  append_composite_type_field (sigfault_union_type, "", type);

   /* used when si_code=TRAP_PERF */
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
   append_composite_type_field (type, "si_perf_data", unsigned_long_type);
   append_composite_type_field (type, "si_perf_type", uint32_type);
   append_composite_type_field (type, "si_perf_flags", uint32_type);
-  append_composite_type_field (sigfault_union_type, "_perf", type);
+  append_composite_type_field (sigfault_union_type, "", type);

   /* End _sigfault's anonymous union.  */

@@ -408,21 +408,21 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
   append_composite_type_field (type, "si_addr", void_ptr_type);
   /* Since there is no possibility to declare an anonymous union,
      using '_anon_union' instead.  */
-  append_composite_type_field (type, "_anon_union", sigfault_union_type);
-  append_composite_type_field (sifields_type, "_sigfault", type);
+  append_composite_type_field (type, "", sigfault_union_type);
+  append_composite_type_field (sifields_type, "", type);

   /* _sigpoll */
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
   append_composite_type_field (type, "si_band", long_type);
   append_composite_type_field (type, "si_fd", int_type);
-  append_composite_type_field (sifields_type, "_sigpoll", type);
+  append_composite_type_field (sifields_type, "", type);

   /* _sigsys */
   type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
   append_composite_type_field (type, "_call_addr", void_ptr_type);
   append_composite_type_field (type, "_syscall", int_type);
   append_composite_type_field (type, "_arch", uint_type);
-  append_composite_type_field (sifields_type, "_sigsys", type);
+  append_composite_type_field (sifields_type, "", type);

   /* struct siginfo */
   siginfo_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
@@ -431,7 +431,7 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
   append_composite_type_field (siginfo_type, "si_errno", int_type);
   append_composite_type_field (siginfo_type, "si_code", int_type);
   append_composite_type_field_aligned (siginfo_type,
-				       "_sifields", sifields_type,
+				       "", sifields_type,
 				       long_type->length ());

   linux_gdbarch_data->siginfo_type = siginfo_type;
diff --git i/gdb/testsuite/gdb.base/siginfo-obj.exp w/gdb/testsuite/gdb.base/siginfo-obj.exp
index 5e36b3340680..272d74ac2805 100644
--- i/gdb/testsuite/gdb.base/siginfo-obj.exp
+++ w/gdb/testsuite/gdb.base/siginfo-obj.exp
@@ -111,11 +111,11 @@ gdb_test "continue" ".*Program received signal SIGSEGV.*" \
 	 "continue to signal, 2nd"

 set test "set si_addr"
-gdb_test "p \$_siginfo._sifields._sigfault.si_addr = 0x666" " = \\(void \\*\\) 0x666"
+gdb_test "p \$_siginfo.si_addr = 0x666" " = \\(void \\*\\) 0x666"
 gdb_test "p \$_siginfo.si_errno = 666" " = 666"
 gdb_test "p \$_siginfo.si_code = 999" " = 999"
 gdb_test "p \$_siginfo.si_signo = 11" " = 11"
-gdb_test "p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey = 123" " = 123"
+gdb_test "p \$_siginfo.si_pkey = 123" " = 123"

 with_test_prefix "validate modified siginfo fields" {
     gdb_test "break $bp_location"
@@ -140,10 +140,10 @@ if {$gcore_created} {
 	"p \$_siginfo.si_errno from core file"
     gdb_test "p \$_siginfo.si_code" " = $ssi_code" \
 	"p \$_siginfo.si_code from core file"
-    gdb_test "p \$_siginfo._sifields._sigfault.si_addr" \
+    gdb_test "p \$_siginfo.si_addr" \
 	" = \\(void \\*\\) $ssi_addr" \
-	"p \$_siginfo._sifields._sigfault.si_addr from core file"
-    gdb_test "p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey" \
+	"p \$_siginfo.si_addr from core file"
+    gdb_test "p \$_siginfo.si_pkey" \
 	" = $ssi_pkey" \
-	"p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey from core file"
+	"p \$_siginfo.si_pkey from core file"
 }

Simon