Re: PATCH: Support x32 siginfo_t conversion

["H.J. Lu" <hjl.tools@gmail.com>]

On Sat, May 12, 2012 at 2:16 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> 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.
>

This is in amd64-linux-nat.c.  All native Linux/amd64 compilers support
__attribute__ ((__aligned__ (...))).    In any case, I will use a struct with
2 ints.

Thanks.


-- 
H.J.