RE: [PATCH V2 5/5] Adapt siginfo fixup for the new bnd fields.

Tedeschi, Walfred
2015-12-18T14:53:24Z

Pedro,

Yes, This way we don't need to rely on the systems where gdb/gdbserver was compiled.

I understood as an intermediate step in that the siginfo buffer from the system is memcopied to the internal structure and then to compat and vice versa.

I.e.:
buffer -> nat-> compat (x32).
Or
Compat (x32) -> nat -> buffer.

Thanks for your reviews!
Regards,
-Fred

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com]
Sent: Friday, December 18, 2015 3:43 PM
To: Tedeschi, Walfred; brobecker@adacore.com
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH V2 5/5] Adapt siginfo fixup for the new bnd fields.

On 12/17/2015 04:56 PM, Walfred Tedeschi wrote:
> New bnds fields will be always present for x86 architecture.
> Fixup for compatibility layer 32bits has to be fixed.
>
> It was added the nat_siginfo to serving as intermediate step between
> kernel provided siginfo and the fix up routine.
>
> When executing compat_siginfo_from_siginfo or
> compat_x32_siginfo_from_siginfo first the buffer read from the kernel
> are converted into the nat_signfo for homogenization, then the fields
> of nat_siginfo are use to set the compat and compat_x32 siginfo structures.
>
> When executing siginfo_from_compat_siginfo or
> siginfo_from_compat_x32_siginfo the process happens in oposite order.
>
> In doing this the fixups become more independent of the system underneath.

Can you expand on the rationale? AFAICS, there's no intermediate step conversion, in the sense of a field-by-field conversion.
It's a straight memcpy to/from the host's siginfo_t.

So is the reason for this new type so that we don't rely on the siginfo_t host's type at all? So that we can access the new fields even if gdb is not compiled on a system with the updated glibc yet?

Thanks,
Pedro Alves

>
> Caveat: No support for MPX on x32.
>
> 2015-12-08 Walfred Tedeschi <walfred.tedeschi@intel.com>
>
> * amd64-linux-siginfo.c (nat_siginfo_t, nat_sigval_t, nat_timeval):
> New types.
> (compat_siginfo): New bnd fields added.
> (compat_x32_siginfo): New field added.
> (cpt_si_addr_lsb): New define.
> (compat_siginfo_from_siginfo): Use nat_siginfo.
> (siginfo_from_compat_siginfo): Use nat_siginfo.
> (compat_x32_siginfo_from_siginfo): Likewise.
> (siginfo_from_compat_x32_siginfo): Likewise.
>
> ---
> gdb/nat/amd64-linux-siginfo.c | 311
> ++++++++++++++++++++++++++++--------------
> 1 file changed, 211 insertions(+), 100 deletions(-)
>
> diff --git a/gdb/nat/amd64-linux-siginfo.c
> b/gdb/nat/amd64-linux-siginfo.c index 22e3552..8f0f486 100644
> --- a/gdb/nat/amd64-linux-siginfo.c
> +++ b/gdb/nat/amd64-linux-siginfo.c
> @@ -30,6 +30,92 @@
> Also, the first step is to make a copy from the original bits to the
> internal structure which is the super set. */
>
> +/* These types below (native_*) define a siginfo type that is layout
> + the most complete siginfo available for the architecture. */
> +
> +typedef int nat_int_t;
> +typedef void* nat_uptr_t;
> +
> +typedef int nat_time_t;
> +typedef int nat_timer_t;
> +
> +/* For native 64-bit, clock_t in _sigchld is 64bit aligned at 4
> +bytes. */ typedef long __attribute__ ((__aligned__ (4)))
> +nat_clock_t;
> +
> +struct nat_timeval
> +{
> + nat_time_t tv_sec;
> + int tv_usec;
> +};
> +
> +typedef union nat_sigval
> +{
> + nat_int_t sival_int;
> + nat_uptr_t sival_ptr;
> +} nat_sigval_t;
> +
> +typedef struct nat_siginfo
> +{
> + int si_signo;
> + int si_errno;
> + int si_code;
> +
> + union
> + {
> + int _pad[((128 / sizeof (int)) - 4)];
> + /* kill() */
> + struct
> + {
> + unsigned int _pid;
> + unsigned int _uid;
> + } _kill;
> +
> + /* POSIX.1b timers */
> + struct
> + {
> + nat_timer_t _tid;
> + int _overrun;
> + nat_sigval_t _sigval;
> + } _timer;
> +
> + /* POSIX.1b signals */
> + struct
> + {
> + unsigned int _pid;
> + unsigned int _uid;
> + nat_sigval_t _sigval;
> + } _rt;
> +
> + /* SIGCHLD */
> + struct
> + {
> + unsigned int _pid;
> + unsigned int _uid;
> + int _status;
> + nat_clock_t _utime;
> + nat_clock_t _stime;
> + } _sigchld;
> +
> + /* SIGILL, SIGFPE, SIGSEGV, SIGBUS */
> + struct
> + {
> + nat_uptr_t _addr;
> + short int _addr_lsb;
> + struct
> + {
> + nat_uptr_t _lower;
> + nat_uptr_t _upper;
> + } si_addr_bnd;
> + } _sigfault;
> +
> + /* SIGPOLL */
> + struct
> + {
> + int _band;
> + int _fd;
> + } _sigpoll;
> + } _sifields;
> +} nat_siginfo_t __attribute__ ((__aligned__ (8)));
>
> /* These types below (compat_*) define a siginfo type that is layout
> compatible with the siginfo type exported by the 32-bit userspace
> @@ -101,6 +187,12 @@ typedef struct compat_siginfo
> struct
> {
> unsigned int _addr;
> + short int _addr_lsb;
> + struct
> + {
> + unsigned int _lower;
> + unsigned int _upper;
> + } si_addr_bnd;
> } _sigfault;
>
> /* SIGPOLL */
> @@ -162,6 +254,7 @@ typedef struct compat_x32_siginfo
> struct
> {
> unsigned int _addr;
> + unsigned int _addr_lsb;
> } _sigfault;
>
> /* SIGPOLL */
> @@ -184,6 +277,7 @@ typedef struct compat_x32_siginfo #define
> cpt_si_stime _sifields._sigchld._stime #define cpt_si_ptr
> _sifields._rt._sigval.sival_ptr #define cpt_si_addr
> _sifields._sigfault._addr
> +#define cpt_si_addr_lsb _sifields._sigfault._addr_lsb
> #define cpt_si_band _sifields._sigpoll._band #define cpt_si_fd
> _sifields._sigpoll._fd
>
> @@ -200,54 +294,58 @@ typedef struct compat_x32_siginfo static void
> compat_siginfo_from_siginfo (compat_siginfo_t *to, siginfo_t *from) {
> + nat_siginfo_t from_nat;
> +
> + gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t)); memcpy
> + (&from_nat, from, sizeof (from_nat));
> memset (to, 0, sizeof (*to));
>
> - to->si_signo = from->si_signo;
> - to->si_errno = from->si_errno;
> - to->si_code = from->si_code;
> + to->si_signo = from_nat.si_signo;
> + to->si_errno = from_nat.si_errno;
> + to->si_code = from_nat.si_code;
>
> if (to->si_code == SI_TIMER)
> {
> - to->cpt_si_timerid = from->si_timerid;
> - to->cpt_si_overrun = from->si_overrun;
> - to->cpt_si_ptr = (intptr_t) from->si_ptr;
> + to->cpt_si_timerid = from_nat.cpt_si_timerid;
> + to->cpt_si_overrun = from_nat.cpt_si_overrun;
> + to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
> }
> else if (to->si_code == SI_USER)
> {
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> }
> else if (to->si_code < 0)
> {
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> - to->cpt_si_ptr = (intptr_t) from->si_ptr;
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> + to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
> }
> else
> {
> switch (to->si_signo)
> {
> case SIGCHLD:
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> - to->cpt_si_status = from->si_status;
> - to->cpt_si_utime = from->si_utime;
> - to->cpt_si_stime = from->si_stime;
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> + to->cpt_si_status = from_nat.cpt_si_status;
> + to->cpt_si_utime = from_nat.cpt_si_utime;
> + to->cpt_si_stime = from_nat.cpt_si_stime;
> break;
> case SIGILL:
> case SIGFPE:
> case SIGSEGV:
> case SIGBUS:
> - to->cpt_si_addr = (intptr_t) from->si_addr;
> + to->cpt_si_addr = (intptr_t) from_nat.cpt_si_addr;
> break;
> case SIGPOLL:
> - to->cpt_si_band = from->si_band;
> - to->cpt_si_fd = from->si_fd;
> + to->cpt_si_band = from_nat.cpt_si_band;
> + to->cpt_si_fd = from_nat.cpt_si_fd;
> break;
> default:
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> - to->cpt_si_ptr = (intptr_t) from->si_ptr;
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> + to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
> break;
> }
> }
> @@ -257,57 +355,62 @@ compat_siginfo_from_siginfo (compat_siginfo_t
> *to, siginfo_t *from) static void siginfo_from_compat_siginfo
> (siginfo_t *to, compat_siginfo_t *from) {
> - memset (to, 0, sizeof (*to));
> + nat_siginfo_t to_nat;
>
> - to->si_signo = from->si_signo;
> - to->si_errno = from->si_errno;
> - to->si_code = from->si_code;
> + gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t)); memset
> + (&to_nat, 0, sizeof (to_nat));
>
> - if (to->si_code == SI_TIMER)
> + to_nat.si_signo = from->si_signo;
> + to_nat.si_errno = from->si_errno;
> + to_nat.si_code = from->si_code;
> +
> + if (to_nat.si_code == SI_TIMER)
> {
> - to->si_timerid = from->cpt_si_timerid;
> - to->si_overrun = from->cpt_si_overrun;
> - to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> + to_nat.cpt_si_timerid = from->cpt_si_timerid;
> + to_nat.cpt_si_overrun = from->cpt_si_overrun;
> + to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> }
> - else if (to->si_code == SI_USER)
> + else if (to_nat.si_code == SI_USER)
> {
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> }
> - if (to->si_code < 0)
> + if (to_nat.si_code < 0)
> {
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> - to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> }
> else
> {
> - switch (to->si_signo)
> + switch (to_nat.si_signo)
> {
> case SIGCHLD:
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> - to->si_status = from->cpt_si_status;
> - to->si_utime = from->cpt_si_utime;
> - to->si_stime = from->cpt_si_stime;
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_status = from->cpt_si_status;
> + to_nat.cpt_si_utime = from->cpt_si_utime;
> + to_nat.cpt_si_stime = from->cpt_si_stime;
> break;
> case SIGILL:
> case SIGFPE:
> case SIGSEGV:
> case SIGBUS:
> - to->si_addr = (void *) (intptr_t) from->cpt_si_addr;
> + to_nat.cpt_si_addr = (void *) (intptr_t) from->cpt_si_addr;
> + to_nat.cpt_si_addr_lsb = (short) from->cpt_si_addr_lsb;
> break;
> case SIGPOLL:
> - to->si_band = from->cpt_si_band;
> - to->si_fd = from->cpt_si_fd;
> + to_nat.cpt_si_band = from->cpt_si_band;
> + to_nat.cpt_si_fd = from->cpt_si_fd;
> break;
> default:
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> - to->si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
> break;
> }
> }
> + memcpy (to, &to_nat, sizeof (to_nat));
> }
>
> /* Converts the system provided siginfo into compatible x32 siginfo.
> */ @@ -315,56 +418,60 @@ static void compat_x32_siginfo_from_siginfo
> (compat_x32_siginfo_t *to,
> siginfo_t *from)
> {
> + nat_siginfo_t from_nat;
> +
> + gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t)); memcpy
> + (&from_nat, from, sizeof (from_nat));
> memset (to, 0, sizeof (*to));
>
> - to->si_signo = from->si_signo;
> - to->si_errno = from->si_errno;
> - to->si_code = from->si_code;
> + to->si_signo = from_nat.si_signo;
> + to->si_errno = from_nat.si_errno;
> + to->si_code = from_nat.si_code;
>
> if (to->si_code == SI_TIMER)
> {
> - to->cpt_si_timerid = from->si_timerid;
> - to->cpt_si_overrun = from->si_overrun;
> - to->cpt_si_ptr = (intptr_t) from->si_ptr;
> + to->cpt_si_timerid = from_nat.cpt_si_timerid;
> + to->cpt_si_overrun = from_nat.cpt_si_overrun;
> + to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
> }
> else if (to->si_code == SI_USER)
> {
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> }
> else if (to->si_code < 0)
> {
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> - to->cpt_si_ptr = (intptr_t) from->si_ptr;
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> + to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
> }
> else
> {
> switch (to->si_signo)
> {
> case SIGCHLD:
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> - to->cpt_si_status = from->si_status;
> - memcpy (&to->cpt_si_utime, &from->si_utime,
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> + to->cpt_si_status = from_nat.cpt_si_status;
> + memcpy (&to->cpt_si_utime, &from_nat.cpt_si_utime,
> sizeof (to->cpt_si_utime));
> - memcpy (&to->cpt_si_stime, &from->si_stime,
> + memcpy (&to->cpt_si_stime, &from_nat.cpt_si_stime,
> sizeof (to->cpt_si_stime));
> break;
> case SIGILL:
> case SIGFPE:
> case SIGSEGV:
> case SIGBUS:
> - to->cpt_si_addr = (intptr_t) from->si_addr;
> + to->cpt_si_addr = (intptr_t) from_nat.cpt_si_addr;
> break;
> case SIGPOLL:
> - to->cpt_si_band = from->si_band;
> - to->cpt_si_fd = from->si_fd;
> + to->cpt_si_band = from_nat.cpt_si_band;
> + to->cpt_si_fd = from_nat.cpt_si_fd;
> break;
> default:
> - to->cpt_si_pid = from->si_pid;
> - to->cpt_si_uid = from->si_uid;
> - to->cpt_si_ptr = (intptr_t) from->si_ptr;
> + to->cpt_si_pid = from_nat.cpt_si_pid;
> + to->cpt_si_uid = from_nat.cpt_si_uid;
> + to->cpt_si_ptr = (intptr_t) from_nat.cpt_si_ptr;
> break;
> }
> }
> @@ -375,59 +482,63 @@ static void
> siginfo_from_compat_x32_siginfo (siginfo_t *to,
> compat_x32_siginfo_t *from)
> {
> - memset (to, 0, sizeof (*to));
> + nat_siginfo_t to_nat;
>
> - to->si_signo = from->si_signo;
> - to->si_errno = from->si_errno;
> - to->si_code = from->si_code;
> + gdb_assert (sizeof (nat_siginfo_t) == sizeof (siginfo_t)); memset
> + (&to_nat, 0, sizeof (to_nat));
>
> - if (to->si_code == SI_TIMER)
> + to_nat.si_signo = from->si_signo;
> + to_nat.si_errno = from->si_errno;
> + to_nat.si_code = from->si_code;
> +
> + if (to_nat.si_code == SI_TIMER)
> {
> - to->si_timerid = from->cpt_si_timerid;
> - to->si_overrun = from->cpt_si_overrun;
> - to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> + to_nat.cpt_si_timerid = from->cpt_si_timerid;
> + to_nat.cpt_si_overrun = from->cpt_si_overrun;
> + to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> }
> - else if (to->si_code == SI_USER)
> + else if (to_nat.si_code == SI_USER)
> {
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> }
> - if (to->si_code < 0)
> + if (to_nat.si_code < 0)
> {
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> - to->si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_ptr = (void *) (intptr_t) from->cpt_si_ptr;
> }
> else
> {
> - switch (to->si_signo)
> + switch (to_nat.si_signo)
> {
> case SIGCHLD:
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> - to->si_status = from->cpt_si_status;
> - memcpy (&to->si_utime, &from->cpt_si_utime,
> - sizeof (to->si_utime));
> - memcpy (&to->si_stime, &from->cpt_si_stime,
> - sizeof (to->si_stime));
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_status = from->cpt_si_status;
> + memcpy (&to_nat.cpt_si_utime, &from->cpt_si_utime,
> + sizeof (to_nat.cpt_si_utime));
> + memcpy (&to_nat.cpt_si_stime, &from->cpt_si_stime,
> + sizeof (to_nat.cpt_si_stime));
> break;
> case SIGILL:
> case SIGFPE:
> case SIGSEGV:
> case SIGBUS:
> - to->si_addr = (void *) (intptr_t) from->cpt_si_addr;
> + to_nat.cpt_si_addr = (void *) (intptr_t) from->cpt_si_addr;
> break;
> case SIGPOLL:
> - to->si_band = from->cpt_si_band;
> - to->si_fd = from->cpt_si_fd;
> + to_nat.cpt_si_band = from->cpt_si_band;
> + to_nat.cpt_si_fd = from->cpt_si_fd;
> break;
> default:
> - to->si_pid = from->cpt_si_pid;
> - to->si_uid = from->cpt_si_uid;
> - to->si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
> + to_nat.cpt_si_pid = from->cpt_si_pid;
> + to_nat.cpt_si_uid = from->cpt_si_uid;
> + to_nat.cpt_si_ptr = (void* ) (intptr_t) from->cpt_si_ptr;
> break;
> }
> }
> + memcpy (to, &to_nat, sizeof (to_nat));
> }
>
> /* Convert a native/host siginfo object, into/from the siginfo in the
>

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