This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Support x32 siginfo_t conversion


> Date: Fri, 11 May 2012 13:55:29 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> On Fri, May 11, 2012 at 1:11 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Date: Fri, 11 May 2012 12:21:33 -0700
> >> From: "H.J. Lu" <hongjiu.lu@intel.com>
> >>
> >> Hi,
> >>
> >> This patch implements x32 siginfo_t conversion. ?Tested on Linux/x86-64.
> >> OK to install?
> >>
> >> Thanks.
> >>
> >>
> >> H.J.
> >> --
> >> ? ? ? * amd64-linux-nat.c (compat_x32_clock_t): New.
> >> ? ? ? (compat_x32_siginfo_t): Likewise.
> >> ? ? ? (compat_x32_siginfo_from_siginfo): Likewise.
> >> ? ? ? (siginfo_from_compat_x32_siginfo): Likewise.
> >> ? ? ? (amd64_linux_siginfo_fixup): Call compat_x32_siginfo_from_siginfo
> >> ? ? ? and siginfo_from_compat_x32_siginfo for x32.
> >>
> >> diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
> >> index 3be8404..97c9a49 100644
> >> --- a/gdb/amd64-linux-nat.c
> >> +++ b/gdb/amd64-linux-nat.c
> >> @@ -591,6 +591,67 @@ typedef struct compat_siginfo
> >> ? ?} _sifields;
> >> ?} compat_siginfo_t;
> >>
> >> +/* For x32, clock_t in _sigchld is 64bit aligned at 4 bytes. ?*/
> >> +typedef long __attribute__ ((__aligned__ (4))) compat_x32_clock_t;
> >
> > Sorry, but that isn't acceptable.
> >
> > Is your X32 ABI really that broken?
> >
> >> +typedef struct compat_x32_siginfo
> >> +{
> >> + ?int si_signo;
> >> + ?int si_errno;
> >> + ?int si_code;
> >> +
> >> + ?union
> >> + ?{
> >> + ? ?int _pad[((128 / sizeof (int)) - 3)];
> >> +
> >> + ? ?/* kill() */
> >> + ? ?struct
> >> + ? ?{
> >> + ? ? ?unsigned int _pid;
> >> + ? ? ?unsigned int _uid;
> >> + ? ?} _kill;
> >> +
> >> + ? ?/* POSIX.1b timers */
> >> + ? ?struct
> >> + ? ?{
> >> + ? ? ?compat_timer_t _tid;
> >> + ? ? ?int _overrun;
> >> + ? ? ?compat_sigval_t _sigval;
> >> + ? ?} _timer;
> >> +
> >> + ? ?/* POSIX.1b signals */
> >> + ? ?struct
> >> + ? ?{
> >> + ? ? ?unsigned int _pid;
> >> + ? ? ?unsigned int _uid;
> >> + ? ? ?compat_sigval_t _sigval;
> >> + ? ?} _rt;
> >> +
> >> + ? ?/* SIGCHLD */
> >> + ? ?struct
> >> + ? ?{
> >> + ? ? ?unsigned int _pid;
> >> + ? ? ?unsigned int _uid;
> >> + ? ? ?int _status;
> >> + ? ? ?compat_x32_clock_t _utime;
> >> + ? ? ?compat_x32_clock_t _stime;
> >> + ? ?} _sigchld;
> >> +
> >> + ? ?/* SIGILL, SIGFPE, SIGSEGV, SIGBUS */
> >> + ? ?struct
> >> + ? ?{
> >> + ? ? ?unsigned int _addr;
> >> + ? ?} _sigfault;
> >> +
> >> + ? ?/* SIGPOLL */
> >> + ? ?struct
> >> + ? ?{
> >> + ? ? ?int _band;
> >> + ? ? ?int _fd;
> >> + ? ?} _sigpoll;
> >> + ?} _sifields;
> >> +} compat_x32_siginfo_t __attribute__ ((__aligned__ (8)));
> >
> > Same here. ?I don't think you need alignment here, even with the broken ABI.
> >
> > If it really is too late to fix the X32 ABI, you'll have to write this
> > portably by splitting _utime and _stime into two 32-bit variables and
> > write code that correctly sets the upper and lower 32-bits.
> >
> >
> 
> X32 ABI choice is done on purpose.  X32 siginfo_t has
> 
> typedef long __attribute__ ((__aligned__ (4))) compat_x32_clock_t;
> 
> typedef struct compat_x32_siginfo
> {
>   int si_signo;
>   int si_errno;
>   int si_code;
> 
>   union
>   {
> ...
>     /* SIGCHLD */
>     struct
>     {
>       unsigned int _pid;
>       unsigned int _uid;
>       int _status;
>       compat_x32_clock_t _utime;
>       compat_x32_clock_t _stime;
>     } _sigchld;
> ...
> } compat_x32_siginfo_t __attribute__ ((__aligned__ (8)));
> 
> struct info is aligned at 8 bytes and type of _utime/_stime is aligned
> at 4 bytes.  However,  _utime offset is 3 * 4 + 3 * 4 == 24 bytes. So
> in reality, the addresses of _utime/_stime are 8 bytes aligned.  There
> are no needs to split _utime and _stime into two 32-bit variables
> since their addresses are 64bits aligned.

But there is no way you can easily express that syntax with standard C
syntax[1].  That's why you had to resort to using GCC's __attribute__
syntax.  For GDB you'll have to figure out a way to do this without
using __attribute__ ((__aligned__ (...))).

My recommendation would be to define a compat_x32 structure just for
SIGCHLD (without using a union) and use the normal 32-bit comap
structure for all the other conversions.

Cheers,

Mark


[1] Well there is in C11, but you can't rely on that being properly
    implemented for at least another 5 years or so.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]