RE: [PATCH v1] Synchronize siginfo type described in GDB with the kernel and glibc ones.

["Tedeschi, Walfred" <walfred.tedeschi@intel.com>]

Joel and all,

First of all, Thanks a lot for your review, on both patches!

I had a discussion on the GDB list about this topic.
Observation from Yao was that we could use this structure on new and old kernels.
https://sourceware.org/ml/gdb/2015-07/msg00029.html

This was the reason why I wanted to keep it simple and avoid doing any king of check.

I understood from the kernel and glibc patches that the kernel is generic (all architectures) and the 
glibc is added for Intel only.
 
Finally I think it is also better to describe a bit what happens with the new fields in the kernel side.

In case we have a bound violation, the kernel does a disassemble of the current instruction,
Reads the values of the operands of the compare bound instructions and set the _low and _upper bound.
At the same time it sets the sig_code to 3. I.e. Those fields are only valid if the sigcode is 3.

1. Make a mechanism to determine which siginfo version to be used depending from the system being debugged.
I am not sure if there is such a mechanism in GDB already or how error prune it would be to be
able to detect pre presence of these fields from the kernel side and glibc (in the case of
the glibc also taking into account the architecture). 

2. Set the field _low and _upper to 0 in case the sigcode is not 3.

Any ideas or comments on that.

Thanks and regards,
-Fred

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Thursday, November 19, 2015 12:02 AM
To: Tedeschi, Walfred
Cc: palves@redhat.com; gdb-patches@sourceware.org
Subject: Re: [PATCH v1] Synchronize siginfo type described in GDB with the kernel and glibc ones.

> New kernel and glibc patches have introduced fields in the 
> segmentation fault fields.
> 
> Kernel patch:
> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=ee1
> b58d36aa1b5a79eaba11f5c3633c88231da83
> 
> Glibc patch:
> http://repo.or.cz/w/glibc.git/commit/d4358b51c26a634eb885955aea06cad26
> af6f696
> 
> 2015-07-21  Walfred Tedeschi  <walfred.tedeschi@intel.com>
> 
> 	* linux-tdep.c (linux_get_siginfo_type): Add the _addr_bnd
> 	structure to the siginfo.

I would really like someone like Pedro to review this patch, as I am really uncertain about the idea of extending our view of the structure without a way of checking that you have kernel and glibc support for it. As far as I can tell, this sets things up so as to ready undefined data, thus leading to undefined behavior.

> 
> ---
>  gdb/linux-tdep.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index 
> 7c24eaa..de773ae 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -248,10 +248,10 @@ static struct type *  linux_get_siginfo_type 
> (struct gdbarch *gdbarch)  {
>    struct linux_gdbarch_data *linux_gdbarch_data;
> -  struct type *int_type, *uint_type, *long_type, *void_ptr_type;
> +  struct type *int_type, *uint_type, *long_type, *void_ptr_type, 
> + *short_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_bnd_fields;
>    struct type *type;
>  
>    linux_gdbarch_data = get_linux_gdbarch_data (gdbarch); @@ -264,6 
> +264,8 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>  				 1, "unsigned int");
>    long_type = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch),
>  				 0, "long");
> +  short_type = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch),
> +				 0, "short");
>    void_ptr_type = lookup_pointer_type (builtin_type 
> (gdbarch)->builtin_void);
>  
>    /* sival_t */
> @@ -339,6 +341,12 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>    /* _sigfault */
>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>    append_composite_type_field (type, "si_addr", void_ptr_type);
> +  append_composite_type_field (type, "_addr_lsb", short_type);
> +
> +  sigfault_bnd_fields = arch_composite_type (gdbarch, NULL, 
> + TYPE_CODE_STRUCT);  append_composite_type_field 
> + (sigfault_bnd_fields, "_lower", void_ptr_type);  
> + append_composite_type_field (sigfault_bnd_fields, "_upper", 
> + void_ptr_type);  append_composite_type_field (type, "_addr_bnd", 
> + sigfault_bnd_fields);
>    append_composite_type_field (sifields_type, "_sigfault", type);
>  
>    /* _sigpoll */
> --
> 2.1.4

--
Joel
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928