Re: PATCH: Support x32 siginfo_t conversion

["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.

Thanks.


-- 
H.J.