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

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

Hello Matthieu,

Sorry for the delay in responding.

Matthieu Longo <matthieu.longo@arm.com> writes:

> On 19/06/2026 11:45, Matthieu Longo wrote:
>> On 19/06/2026 04:58, Thiago Jung Bauermann wrote:
>>>
>>> Matthieu Longo <matthieu.longo@arm.com> writes:
>>>
>>>> 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://github.com/torvalds/linux/blob/2b414a95b8f7307d42173ba9e580d6
>>>>       d3e2bcbfce/include/uapi/asm-generic/siginfo.h#L69-L100
>>>
>>> Nit: https://github.com/torvalds/linux/ is a mirror. It's better to use
>>> the official address at
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>>
>> Fixed.

Thank you for the fixes!

>>>> +  /* used when si_code=SEGV_BNDERR */
>>>> +  type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>>> +  append_composite_type_field (type, "_dummy_bnd", short_type);
>>>
>>> The type of _dummy_bnd isn't that important in practice because the
>>> field is just for padding, but its size is wrong AFAICS:
>>>
>>> $ cat siginfo_t.c
>>> #include <stdio.h>
>>> #include <asm/siginfo.h>
>>>
>>> int main (int argc, char *argv[]) {
>>>      siginfo_t siginfo;
>>>
>>>      printf ("sizeof dummy_bnd = %lu\n",
>>>              sizeof(siginfo._sifields._sigfault._addr_bnd._dummy_bnd));
>>>
>>>      return 0;
>>> }
>>> $ make siginfo_t
>>> cc     siginfo_t.c   -o siginfo_t
>>> $ ./siginfo_t
>>> sizeof dummy_bnd = 8
>>>
>> Sorry, I misread the macro __ADDR_BND_PKEY_PAD.
>> 
>>>> +  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", short_type);
>>>
>>> This one also has the wrong size, because <asm/siginfo.h> uses the same
>>> __ADDR_BND_PKEY_PAD constant for the size of _dummy_bnd and of
>>> _dummy_pkey.
>>>
>> Fixed as follows:
>> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
>> index 5baad41a083..19289cf4b3f 100644
>> --- a/gdb/linux-tdep.c
>> +++ b/gdb/linux-tdep.c
>> @@ -293,6 +293,13 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>     struct type *uint32_type = builtin_types->builtin_uint32;
>>     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 < sizeof(short)
>> +                          ? sizeof(short)
>> +                          : alignof_void_ptr);

Nit: the two lines above are indented with spaces rather than tabs.

The binutils-gdb repo has an .editorconfig file, so one way to make it
easier to follow the project coding style is to make your editor follow
it. Many editors have built-in support for the file, others have plugins
available:

https://editorconfig.org/

> By the way, I forgot to ask.
> Would you prefer me to publish a v2 instead of the diff above ?

Yes, please. Also note that I can't approve this patch. With the above
whitespace issue fixed, you can add to it:

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

-- 
Thiago
(he/him)