Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* PATCH: Support x32 siginfo_t conversion
@ 2012-05-11 19:21 H.J. Lu
  2012-05-11 20:11 ` Mark Kettenis
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2012-05-11 19:21 UTC (permalink / raw)
  To: GDB

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;
+
+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)));
+
 #define cpt_si_pid _sifields._kill._pid
 #define cpt_si_uid _sifields._kill._uid
 #define cpt_si_timerid _sifields._timer._tid
@@ -724,6 +785,120 @@ siginfo_from_compat_siginfo (siginfo_t *to, compat_siginfo_t *from)
     }
 }
 
+static void
+compat_x32_siginfo_from_siginfo (compat_x32_siginfo_t *to,
+				 siginfo_t *from)
+{
+  memset (to, 0, sizeof (*to));
+
+  to->si_signo = from->si_signo;
+  to->si_errno = from->si_errno;
+  to->si_code = from->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;
+    }
+  else if (to->si_code == SI_USER)
+    {
+      to->cpt_si_pid = from->si_pid;
+      to->cpt_si_uid = from->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;
+    }
+  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;
+	  break;
+	case SIGILL:
+	case SIGFPE:
+	case SIGSEGV:
+	case SIGBUS:
+	  to->cpt_si_addr = (intptr_t) from->si_addr;
+	  break;
+	case SIGPOLL:
+	  to->cpt_si_band = from->si_band;
+	  to->cpt_si_fd = from->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;
+	  break;
+	}
+    }
+}
+
+static void
+siginfo_from_compat_x32_siginfo (siginfo_t *to,
+				 compat_x32_siginfo_t *from)
+{
+  memset (to, 0, sizeof (*to));
+
+  to->si_signo = from->si_signo;
+  to->si_errno = from->si_errno;
+  to->si_code = from->si_code;
+
+  if (to->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;
+    }
+  else if (to->si_code == SI_USER)
+    {
+      to->si_pid = from->cpt_si_pid;
+      to->si_uid = from->cpt_si_uid;
+    }
+  if (to->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;
+    }
+  else
+    {
+      switch (to->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;
+	  break;
+	case SIGILL:
+	case SIGFPE:
+	case SIGSEGV:
+	case SIGBUS:
+	  to->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;
+	  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;
+	  break;
+	}
+    }
+}
+
 /* Convert a native/host siginfo object, into/from the siginfo in the
    layout of the inferiors' architecture.  Returns true if any
    conversion was done; false otherwise.  If DIRECTION is 1, then copy
@@ -747,6 +922,20 @@ amd64_linux_siginfo_fixup (siginfo_t *native, gdb_byte *inf, int direction)
 
       return 1;
     }
+  /* No fixup for native x32 GDB.  */
+  else if (gdbarch_addr_bit (gdbarch) == 32 && sizeof (void *) == 8)
+    {
+      gdb_assert (sizeof (siginfo_t) == sizeof (compat_x32_siginfo_t));
+
+      if (direction == 0)
+	compat_x32_siginfo_from_siginfo ((struct compat_x32_siginfo *) inf,
+					 native);
+      else
+	siginfo_from_compat_x32_siginfo (native,
+					 (struct compat_x32_siginfo *) inf);
+
+      return 1;
+    }
   else
     return 0;
 }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Support x32 siginfo_t conversion
  2012-05-11 19:21 PATCH: Support x32 siginfo_t conversion H.J. Lu
@ 2012-05-11 20:11 ` Mark Kettenis
  2012-05-11 20:55   ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Kettenis @ 2012-05-11 20:11 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches

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



> +
>  #define cpt_si_pid _sifields._kill._pid
>  #define cpt_si_uid _sifields._kill._uid
>  #define cpt_si_timerid _sifields._timer._tid
> @@ -724,6 +785,120 @@ siginfo_from_compat_siginfo (siginfo_t *to, compat_siginfo_t *from)
>      }
>  }
>  
> +static void
> +compat_x32_siginfo_from_siginfo (compat_x32_siginfo_t *to,
> +				 siginfo_t *from)
> +{
> +  memset (to, 0, sizeof (*to));
> +
> +  to->si_signo = from->si_signo;
> +  to->si_errno = from->si_errno;
> +  to->si_code = from->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;
> +    }
> +  else if (to->si_code == SI_USER)
> +    {
> +      to->cpt_si_pid = from->si_pid;
> +      to->cpt_si_uid = from->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;
> +    }
> +  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;
> +	  break;
> +	case SIGILL:
> +	case SIGFPE:
> +	case SIGSEGV:
> +	case SIGBUS:
> +	  to->cpt_si_addr = (intptr_t) from->si_addr;
> +	  break;
> +	case SIGPOLL:
> +	  to->cpt_si_band = from->si_band;
> +	  to->cpt_si_fd = from->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;
> +	  break;
> +	}
> +    }
> +}
> +
> +static void
> +siginfo_from_compat_x32_siginfo (siginfo_t *to,
> +				 compat_x32_siginfo_t *from)
> +{
> +  memset (to, 0, sizeof (*to));
> +
> +  to->si_signo = from->si_signo;
> +  to->si_errno = from->si_errno;
> +  to->si_code = from->si_code;
> +
> +  if (to->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;
> +    }
> +  else if (to->si_code == SI_USER)
> +    {
> +      to->si_pid = from->cpt_si_pid;
> +      to->si_uid = from->cpt_si_uid;
> +    }
> +  if (to->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;
> +    }
> +  else
> +    {
> +      switch (to->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;
> +	  break;
> +	case SIGILL:
> +	case SIGFPE:
> +	case SIGSEGV:
> +	case SIGBUS:
> +	  to->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;
> +	  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;
> +	  break;
> +	}
> +    }
> +}
> +
>  /* Convert a native/host siginfo object, into/from the siginfo in the
>     layout of the inferiors' architecture.  Returns true if any
>     conversion was done; false otherwise.  If DIRECTION is 1, then copy
> @@ -747,6 +922,20 @@ amd64_linux_siginfo_fixup (siginfo_t *native, gdb_byte *inf, int direction)
>  
>        return 1;
>      }
> +  /* No fixup for native x32 GDB.  */
> +  else if (gdbarch_addr_bit (gdbarch) == 32 && sizeof (void *) == 8)
> +    {
> +      gdb_assert (sizeof (siginfo_t) == sizeof (compat_x32_siginfo_t));
> +
> +      if (direction == 0)
> +	compat_x32_siginfo_from_siginfo ((struct compat_x32_siginfo *) inf,
> +					 native);
> +      else
> +	siginfo_from_compat_x32_siginfo (native,
> +					 (struct compat_x32_siginfo *) inf);
> +
> +      return 1;
> +    }
>    else
>      return 0;
>  }
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Support x32 siginfo_t conversion
  2012-05-11 20:11 ` Mark Kettenis
@ 2012-05-11 20:55   ` H.J. Lu
  2012-05-12 21:16     ` Mark Kettenis
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2012-05-11 20:55 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Support x32 siginfo_t conversion
  2012-05-11 20:55   ` H.J. Lu
@ 2012-05-12 21:16     ` Mark Kettenis
  2012-05-12 23:16       ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Kettenis @ 2012-05-12 21:16 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4432 bytes --]

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Support x32 siginfo_t conversion
  2012-05-12 21:16     ` Mark Kettenis
@ 2012-05-12 23:16       ` H.J. Lu
  2012-05-13  2:25         ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2012-05-12 23:16 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Support x32 siginfo_t conversion
  2012-05-12 23:16       ` H.J. Lu
@ 2012-05-13  2:25         ` H.J. Lu
  0 siblings, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2012-05-13  2:25 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

On Sat, May 12, 2012 at 4:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> 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.

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

Here is the updated patch.  OK to install?

Thanks.


-- 
H.J.
---
diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 5ebba3a..bc28d54 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -591,6 +591,71 @@ typedef struct compat_siginfo
   } _sifields;
 } compat_siginfo_t;

+/* For x32, clock_t in _sigchld is 64bit aligned at 4 bytes.  */
+typedef struct compat_x32_clock
+{
+  int lower;
+  int upper;
+} compat_x32_clock_t;
+
+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;
+
 #define cpt_si_pid _sifields._kill._pid
 #define cpt_si_uid _sifields._kill._uid
 #define cpt_si_timerid _sifields._timer._tid
@@ -724,6 +789,124 @@ siginfo_from_compat_siginfo (siginfo_t *to,
compat_siginfo_t *from)
     }
 }

+static void
+compat_x32_siginfo_from_siginfo (compat_x32_siginfo_t *to,
+				 siginfo_t *from)
+{
+  memset (to, 0, sizeof (*to));
+
+  to->si_signo = from->si_signo;
+  to->si_errno = from->si_errno;
+  to->si_code = from->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;
+    }
+  else if (to->si_code == SI_USER)
+    {
+      to->cpt_si_pid = from->si_pid;
+      to->cpt_si_uid = from->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;
+    }
+  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,
+		  sizeof (to->cpt_si_utime));
+	  memcpy (&to->cpt_si_stime, &from->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;
+	  break;
+	case SIGPOLL:
+	  to->cpt_si_band = from->si_band;
+	  to->cpt_si_fd = from->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;
+	  break;
+	}
+    }
+}
+
+static void
+siginfo_from_compat_x32_siginfo (siginfo_t *to,
+				 compat_x32_siginfo_t *from)
+{
+  memset (to, 0, sizeof (*to));
+
+  to->si_signo = from->si_signo;
+  to->si_errno = from->si_errno;
+  to->si_code = from->si_code;
+
+  if (to->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;
+    }
+  else if (to->si_code == SI_USER)
+    {
+      to->si_pid = from->cpt_si_pid;
+      to->si_uid = from->cpt_si_uid;
+    }
+  if (to->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;
+    }
+  else
+    {
+      switch (to->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));
+	  break;
+	case SIGILL:
+	case SIGFPE:
+	case SIGSEGV:
+	case SIGBUS:
+	  to->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;
+	  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;
+	  break;
+	}
+    }
+}
+
 /* Convert a native/host siginfo object, into/from the siginfo in the
    layout of the inferiors' architecture.  Returns true if any
    conversion was done; false otherwise.  If DIRECTION is 1, then copy
@@ -733,9 +916,10 @@ siginfo_from_compat_siginfo (siginfo_t *to,
compat_siginfo_t *from)
 static int
 amd64_linux_siginfo_fixup (siginfo_t *native, gdb_byte *inf, int direction)
 {
+  struct gdbarch *gdbarch = get_frame_arch (get_current_frame ());
   /* Is the inferior 32-bit?  If so, then do fixup the siginfo
      object.  */
-  if (gdbarch_addr_bit (get_frame_arch (get_current_frame ())) == 32)
+  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 32)
     {
       gdb_assert (sizeof (siginfo_t) == sizeof (compat_siginfo_t));

@@ -746,6 +930,20 @@ amd64_linux_siginfo_fixup (siginfo_t *native,
gdb_byte *inf, int direction)

       return 1;
     }
+  /* No fixup for native x32 GDB.  */
+  else if (gdbarch_addr_bit (gdbarch) == 32 && sizeof (void *) == 8)
+    {
+      gdb_assert (sizeof (siginfo_t) == sizeof (compat_x32_siginfo_t));
+
+      if (direction == 0)
+	compat_x32_siginfo_from_siginfo ((struct compat_x32_siginfo *) inf,
+					 native);
+      else
+	siginfo_from_compat_x32_siginfo (native,
+					 (struct compat_x32_siginfo *) inf);
+
+      return 1;
+    }
   else
     return 0;
 }


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-05-13  2:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-11 19:21 PATCH: Support x32 siginfo_t conversion H.J. Lu
2012-05-11 20:11 ` Mark Kettenis
2012-05-11 20:55   ` H.J. Lu
2012-05-12 21:16     ` Mark Kettenis
2012-05-12 23:16       ` H.J. Lu
2012-05-13  2:25         ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox