Re: PATCH: Support x32 siginfo_t conversion

["H.J. Lu" <hjl dot tools at gmail dot 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.