Re: [PATCH V2 4/5] Add bound related fields to the siginfo structure.

[Pedro Alves <palves@redhat.com>]

On 12/17/2015 04:56 PM, Walfred Tedeschi wrote:
> New kernel and glibc patches have introduced fields in the
> segmentation fault fields.  New fields will be conditional and requested
> on demand by the customers.

I think that by "customers" you mean that the patch adds them
to x86 only, right?  "API client" is more usual than "API customer",
which otherwise sounds like you're talking about Intel's
customers.  :-)  I'd suggest writing:

Both Linux and glibc have introduced bound related fields
in the segmentation fault fields of the siginfo_t type.  Add the
new fields to our x86's siginfo_t type too.

> 
> Kernel patch:
> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=ee1b58d36aa1b5a79eaba11f5c3633c88231da83
> 
> Glibc patch:
> http://repo.or.cz/w/glibc.git/commit/d4358b51c26a634eb885955aea06cad26af6f696
> 
> 2015-12-08  Walfred Tedeschi  <walfred.tedeschi@intel.com>
> 
> 	* linux-tdep.c (linux_get_siginfo_type): Add the _addr_bnd
> 	structure to the siginfo conditionally if extra_fields is
> 	LINUX_SIGINFO_FIELD_ADDR_BND.

"conditionally" is redundant with if.  Should be "contains" rather than
"is".  When talking about a variable's value, use uppercase:

 	* linux-tdep.c (linux_get_siginfo_type): Add the _addr_bnd
 	structure to the siginfo type if EXTRA_FIELDS contains
 	LINUX_SIGINFO_FIELD_ADDR_BND.

> 
> ---
>  gdb/linux-tdep.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index e41b6f4..73c70cb 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -251,7 +251,7 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
>  				    linux_siginfo_extra_fields extra_fields)
>  {
>    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;
> @@ -267,6 +267,8 @@ linux_get_siginfo_type_with_fields (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 */
> @@ -344,6 +346,18 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
>    append_composite_type_field (type, "si_addr", void_ptr_type);
>    append_composite_type_field (sifields_type, "_sigfault", type);
>  
> +  if (extra_fields == LINUX_SIGINFO_FIELD_ADDR_BND)

This is a bit flags type so that other fields can be added in the future.
Write:

  if ((extra_fields & LINUX_SIGINFO_FIELD_ADDR_BND) != 0)

> +    {
> +      struct type *sigfault_bnd_fields;
> +
> +      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);

We add "_sigfault" twice?

> +    }
> +
>    /* _sigpoll */
>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>    append_composite_type_field (type, "si_band", long_type);
> 


Thanks,
Pedro Alves