Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis <luis.machado.foss@gmail.com>
To: Matthieu Longo <matthieu.longo@arm.com>, gdb-patches@sourceware.org
Cc: Thiago Jung Bauermann <thiago.bauermann@linaro.org>,
	Luis Machado <luis.machado@amd.com>, Tom Tromey <tom@tromey.com>,
	Andrew Burgess <aburgess@redhat.com>
Subject: Re: [PATCH v3] gdb: align siginfo_t with the Linux kernel definition
Date: Tue, 21 Jul 2026 22:09:45 +0100	[thread overview]
Message-ID: <cdfa13f3-4043-4314-83d1-b60700fe33f0@gmail.com> (raw)
In-Reply-To: <20260702165231.331169-1-matthieu.longo@arm.com>

On 02/07/2026 17:52, 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.
> 
> [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/linux-tdep.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index a7381677498..d5b0e6e7011 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -275,7 +275,7 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>     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 +285,21 @@ 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;
> +  struct type *unsigned_long_type = builtin_types->builtin_unsigned_long;
> +  struct type *uint32_type = builtin_types->builtin_uint32;
>   
>     void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);

Nit: Blank line here for cleanliness.

> +  /* Compute padding length, i.e. __ADDR_BND_PKEY_PAD.  */
> +  unsigned alignof_void_ptr = type_align (void_ptr_type);
> +  unsigned padding_size = (alignof_void_ptr < sizeof(short)

Logic: Are we mixing host-side sizeof (short) with target-side 
type_align (void_ptr_type) here?

Formatting: Space between sizeof and parens. Multiple cases of this one.

> +			   ? sizeof(short)
> +			   : 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=BUS_MCEERR_AR or used when si_code=BUS_MCEERR_AO */

Nit: Odd phrase. Maybe rewrite as...

"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 '_' instead.  */
> +  append_composite_type_field (type, "_", sigfault_union_type);

Could we name this in a better way? Simply using _ is a bit strange.

>     append_composite_type_field (sifields_type, "_sigfault", type);
>   
>     /* _sigpoll */

Do we need to add some extra tests to validate that gdb can read these 
new fields?

  parent reply	other threads:[~2026-07-21 21:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02 16:52 Matthieu Longo
2026-07-08 15:54 ` Matthieu Longo
2026-07-21  9:17   ` Matthieu Longo
2026-07-21  9:23     ` Luis
2026-07-21 21:09 ` Luis [this message]
2026-07-22 15:00   ` Matthieu Longo
2026-07-22 23:09     ` Luis
2026-07-24  9:18       ` Matthieu Longo
2026-07-24  9:42         ` Matthieu Longo
2026-07-25  7:46         ` Luis
2026-07-26  5:12           ` Maciej W. Rozycki
2026-07-27 12:23           ` Matthieu Longo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cdfa13f3-4043-4314-83d1-b60700fe33f0@gmail.com \
    --to=luis.machado.foss@gmail.com \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado@amd.com \
    --cc=matthieu.longo@arm.com \
    --cc=thiago.bauermann@linaro.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox