Re: struct siginfo vs. siginfo_t

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Thu, 15 Mar 2012 11:57:00 -0400, Carlos O'Donell <carlos@systemhalted.org> wrote:
> On Thu, Mar 15, 2012 at 11:05 AM, Thomas Schwinge
> <thomas@codesourcery.com> wrote:
> > On 26 Feb 2012 18:17:52 -0000, drepper@sourceware.org wrote:
> >> http://sources.redhat.com/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=4efeffc1d583597e4f52985b9747269e47b754e2
> >>
> >> commit 4efeffc1d583597e4f52985b9747269e47b754e2
> >> Author: Ulrich Drepper <drepper@gmail.com>
> >> Date: Â Sun Feb 26 13:17:27 2012 -0500
> >>
> >> Â Â Fix up POSIX testing in conformtest
> >
> >> [...]
> >> + Â Â * sysdeps/unix/sysv/linux/bits/siginfo.h: Don't name siginfo_t
> >> + Â Â struct. Â[...]
> >> [...]
> >
> >> diff --git a/sysdeps/unix/sysv/linux/bits/siginfo.h b/sysdeps/unix/sysv/linux/bits/siginfo.h
> >> index ecef39d..0635e2f 100644
> >> --- a/sysdeps/unix/sysv/linux/bits/siginfo.h
> >> +++ b/sysdeps/unix/sysv/linux/bits/siginfo.h
> >> [...]
> >> @@ -47,7 +47,7 @@ typedef union sigval
> >> Â# Âdefine __SI_PAD_SIZE Â Â ((__SI_MAX_SIZE / sizeof (int)) - 3)
> >> Â# endif
> >>
> >> -typedef struct siginfo
> >> +typedef struct
> >> Â Â{
> >> Â Â Âint si_signo; Â Â Â Â Â Â/* Signal number. Â*/
> >> Â Â Âint si_errno; Â Â Â Â Â Â/* If non-zero, an errno value associated with
> >> [...]
> >
> > This change breaks GCC:
> >
> > Â ÂIn file included from /scratch/tschwing/FM_sh-linux-gnu-mk2/src/gcc-mainline/libgcc/unwind-dw2.c:377:0:
> > Â Â./md-unwind-support.h: In function 'sh_fallback_frame_state':
> > Â Â./md-unwind-support.h:182:17: error: field 'info' has incomplete type
> >
> > In my case, this is really libgcc/config/sh/linux-unwind.h:
> >
> > Â Â[...]
> > Â Â Â 181 Â Â Â Â Â Âstruct rt_sigframe {
> > Â Â Â 182 Â Â Â Â Â Â Âstruct siginfo info;
> > Â Â Â 183 Â Â Â Â Â Â Âstruct ucontext uc;
> > Â Â Â 184 Â Â Â Â Â Â} *rt_ = context->cfa;
> > Â Â[...]
> 
> POSIX says you get "siginto_t" *not* "struct siginfo," please fix the code.

The same problem exists in GDB.

I tested the following patch for x86_64-linux-gnu natively, and
sh-linux-gnu cross-testing with gdbserver.  This doesn't cover all
affected configurations, but at least some of them, and the change is
pretty mechanic anyway (apart from the one occucrrence in gdbserver,
every place that used to refert to struct siginfo already must have had
<signal.h> in its include patch, which is the same file that declares
siginfo_t).

OK to commit?

gdb/
	* amd64-linux-nat.c (amd64_linux_siginfo_fixup): Use siginfo_t instead
	of struct siginfo.gdb_siginfo_t.
	* arm-linux-nat.c (arm_linux_stopped_data_address): Likewise.
	* ia64-linux-nat.c (ia64_linux_stopped_data_address): Likewise.
	* linux-nat.c (linux_nat_siginfo_fixup, siginfo_fixup)
	(linux_xfer_siginfo, linux_nat_set_siginfo_fixup)
	(linux_nat_get_siginfo): Likewise.
	* linux-nat.h (lwp_info, linux_nat_set_siginfo_fixup)
	(linux_nat_get_siginfo): Likewise.
	* linux-tdep.c (linux_get_siginfo_type): Likewise.
	* ppc-linux-nat.c (ppc_linux_stopped_data_address): Likewise.
	* procfs.c (gdb_siginfo_t): Likewise.

gdb/gdbserver/
	* linux-arm-low.c (arm_stopped_by_watchpoint): Use siginfo_t instead of
	struct siginfo.gdb_siginfo_t.
	* linux-low.c (siginfo_fixup, linux_xfer_siginfo): Likewise.
	* linux-x86-low.c (x86_siginfo_fixup): Likewise.
	* linux-low.h (linux_target_ops): Likewise.  Include <signal.h> instead
	of declaring struct siginfo as incomplete type.

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index f954483..5ebba3a 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -731,13 +731,13 @@ siginfo_from_compat_siginfo (siginfo_t *to, compat_siginfo_t *from)
    INF.  */
 
 static int
-amd64_linux_siginfo_fixup (struct siginfo *native, gdb_byte *inf, int direction)
+amd64_linux_siginfo_fixup (siginfo_t *native, gdb_byte *inf, int direction)
 {
   /* Is the inferior 32-bit?  If so, then do fixup the siginfo
      object.  */
   if (gdbarch_addr_bit (get_frame_arch (get_current_frame ())) == 32)
     {
-      gdb_assert (sizeof (struct siginfo) == sizeof (compat_siginfo_t));
+      gdb_assert (sizeof (siginfo_t) == sizeof (compat_siginfo_t));
 
       if (direction == 0)
 	compat_siginfo_from_siginfo ((struct compat_siginfo *) inf, native);
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index c5ce21c..f9f6ba5 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -1137,7 +1137,7 @@ arm_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw,
 static int
 arm_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
 {
-  struct siginfo *siginfo_p = linux_nat_get_siginfo (inferior_ptid);
+  siginfo_t *siginfo_p = linux_nat_get_siginfo (inferior_ptid);
   int slot = siginfo_p->si_errno;
 
   /* This must be a hardware breakpoint.  */
diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c
index 8773195..19b827f 100644
--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -637,7 +637,7 @@ static int
 ia64_linux_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 {
   CORE_ADDR psr;
-  struct siginfo *siginfo_p;
+  siginfo_t *siginfo_p;
   struct regcache *regcache = get_current_regcache ();
 
   siginfo_p = linux_nat_get_siginfo (inferior_ptid);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index f80c0c1..d81d55e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -188,7 +188,7 @@ static void (*linux_nat_prepare_to_resume) (struct lwp_info *);
 /* The method to call, if any, when the siginfo object needs to be
    converted between the layout returned by ptrace, and the layout in
    the architecture of the inferior.  */
-static int (*linux_nat_siginfo_fixup) (struct siginfo *,
+static int (*linux_nat_siginfo_fixup) (siginfo_t *,
 				       gdb_byte *,
 				       int);
 
@@ -4232,7 +4232,7 @@ linux_nat_mourn_inferior (struct target_ops *ops)
    layout of the inferiors' architecture.  */
 
 static void
-siginfo_fixup (struct siginfo *siginfo, gdb_byte *inf_siginfo, int direction)
+siginfo_fixup (siginfo_t *siginfo, gdb_byte *inf_siginfo, int direction)
 {
   int done = 0;
 
@@ -4244,9 +4244,9 @@ siginfo_fixup (struct siginfo *siginfo, gdb_byte *inf_siginfo, int direction)
   if (!done)
     {
       if (direction == 1)
-	memcpy (siginfo, inf_siginfo, sizeof (struct siginfo));
+	memcpy (siginfo, inf_siginfo, sizeof (siginfo_t));
       else
-	memcpy (inf_siginfo, siginfo, sizeof (struct siginfo));
+	memcpy (inf_siginfo, siginfo, sizeof (siginfo_t));
     }
 }
 
@@ -4256,8 +4256,8 @@ linux_xfer_siginfo (struct target_ops *ops, enum target_object object,
 		    const gdb_byte *writebuf, ULONGEST offset, LONGEST len)
 {
   int pid;
-  struct siginfo siginfo;
-  gdb_byte inf_siginfo[sizeof (struct siginfo)];
+  siginfo_t siginfo;
+  gdb_byte inf_siginfo[sizeof (siginfo_t)];
 
   gdb_assert (object == TARGET_OBJECT_SIGNAL_INFO);
   gdb_assert (readbuf || writebuf);
@@ -5266,7 +5266,7 @@ linux_nat_set_new_thread (struct target_ops *t,
    inferior.  */
 void
 linux_nat_set_siginfo_fixup (struct target_ops *t,
-			     int (*siginfo_fixup) (struct siginfo *,
+			     int (*siginfo_fixup) (siginfo_t *,
 						   gdb_byte *,
 						   int))
 {
@@ -5285,7 +5285,7 @@ linux_nat_set_prepare_to_resume (struct target_ops *t,
 }
 
 /* Return the saved siginfo associated with PTID.  */
-struct siginfo *
+siginfo_t *
 linux_nat_get_siginfo (ptid_t ptid)
 {
   struct lwp_info *lp = find_lwp_pid (ptid);
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index c9878d9..d87f0cf 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -78,7 +78,7 @@ struct lwp_info
 
   /* Non-zero si_signo if this LWP stopped with a trap.  si_addr may
      be the address of a hardware watchpoint.  */
-  struct siginfo siginfo;
+  siginfo_t siginfo;
 
   /* STOPPED_BY_WATCHPOINT is non-zero if this LWP stopped with a data
      watchpoint trap.  */
@@ -184,7 +184,7 @@ void linux_nat_set_new_thread (struct target_ops *, void (*) (struct lwp_info *)
    that ptrace returns, and the layout in the architecture of the
    inferior.  */
 void linux_nat_set_siginfo_fixup (struct target_ops *,
-				  int (*) (struct siginfo *,
+				  int (*) (siginfo_t *,
 					   gdb_byte *,
 					   int));
 
@@ -198,7 +198,7 @@ void linux_nat_set_prepare_to_resume (struct target_ops *,
 void linux_nat_switch_fork (ptid_t new_ptid);
 
 /* Return the saved siginfo associated with PTID.  */
-struct siginfo *linux_nat_get_siginfo (ptid_t ptid);
+siginfo_t *linux_nat_get_siginfo (ptid_t ptid);
 
 /* Set alternative SIGTRAP-like events recognizer.  */
 void linux_nat_set_status_is_event (struct target_ops *t,
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 1bd157b..9bd11fd 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -2218,7 +2218,7 @@ ppc_linux_thread_exit (struct thread_info *tp, int silent)
 static int
 ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
 {
-  struct siginfo *siginfo_p;
+  siginfo_t *siginfo_p;
 
   siginfo_p = linux_nat_get_siginfo (inferior_ptid);
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 903621d..cb4bc7c 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -266,7 +266,7 @@ typedef struct sigaction gdb_sigaction_t;
 #ifdef HAVE_PR_SIGINFO64_T
 typedef pr_siginfo64_t gdb_siginfo_t;
 #else
-typedef struct siginfo gdb_siginfo_t;
+typedef siginfo_t gdb_siginfo_t;
 #endif
 
 /* On mips-irix, praddset and prdelset are defined in such a way that
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index ff2437d..01208ef 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -631,7 +631,7 @@ static int
 arm_stopped_by_watchpoint (void)
 {
   struct lwp_info *lwp = get_thread_lwp (current_inferior);
-  struct siginfo siginfo;
+  siginfo_t siginfo;
 
   /* We must be able to set hardware watchpoints.  */
   if (arm_linux_get_hw_watchpoint_count () == 0)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d2d4c1d..4734f15 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4779,7 +4779,7 @@ linux_qxfer_osdata (const char *annex,
    layout of the inferiors' architecture.  */
 
 static void
-siginfo_fixup (struct siginfo *siginfo, void *inf_siginfo, int direction)
+siginfo_fixup (siginfo_t *siginfo, void *inf_siginfo, int direction)
 {
   int done = 0;
 
@@ -4791,9 +4791,9 @@ siginfo_fixup (struct siginfo *siginfo, void *inf_siginfo, int direction)
   if (!done)
     {
       if (direction == 1)
-	memcpy (siginfo, inf_siginfo, sizeof (struct siginfo));
+	memcpy (siginfo, inf_siginfo, sizeof (siginfo_t));
       else
-	memcpy (inf_siginfo, siginfo, sizeof (struct siginfo));
+	memcpy (inf_siginfo, siginfo, sizeof (siginfo_t));
     }
 }
 
@@ -4802,8 +4802,8 @@ linux_xfer_siginfo (const char *annex, unsigned char *readbuf,
 		    unsigned const char *writebuf, CORE_ADDR offset, int len)
 {
   int pid;
-  struct siginfo siginfo;
-  char inf_siginfo[sizeof (struct siginfo)];
+  siginfo_t siginfo;
+  char inf_siginfo[sizeof (siginfo_t)];
 
   if (current_inferior == NULL)
     return -1;
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 677d261..3aeae70 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -20,6 +20,7 @@
 #ifdef HAVE_THREAD_DB_H
 #include <thread_db.h>
 #endif
+#include <signal.h>
 
 #include "gdb_proc_service.h"
 
@@ -46,8 +47,6 @@ struct regset_info
 extern struct regset_info target_regsets[];
 #endif
 
-struct siginfo;
-
 struct process_info_private
 {
   /* Arch-specific additions.  */
@@ -109,7 +108,7 @@ struct linux_target_ops
      Returns true if any conversion was done; false otherwise.
      If DIRECTION is 1, then copy from INF to NATIVE.
      If DIRECTION is 0, copy from NATIVE to INF.  */
-  int (*siginfo_fixup) (struct siginfo *native, void *inf, int direction);
+  int (*siginfo_fixup) (siginfo_t *native, void *inf, int direction);
 
   /* Hook to call when a new process is created or attached to.
      If extra per-process architecture-specific data is needed,
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index ed1f8a8..b466b5d 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -918,13 +918,13 @@ siginfo_from_compat_siginfo (siginfo_t *to, compat_siginfo_t *from)
    INF.  */
 
 static int
-x86_siginfo_fixup (struct siginfo *native, void *inf, int direction)
+x86_siginfo_fixup (siginfo_t *native, void *inf, int direction)
 {
 #ifdef __x86_64__
   /* Is the inferior 32-bit?  If so, then fixup the siginfo object.  */
   if (register_size (0) == 4)
     {
-      if (sizeof (struct siginfo) != sizeof (compat_siginfo_t))
+      if (sizeof (siginfo_t) != sizeof (compat_siginfo_t))
 	fatal ("unexpected difference in siginfo");
 
       if (direction == 0)

I did not change refernces in comments that refer to signal frames and
the like.


GrÃÃe,
 Thomas

Attachment: pgp00000.pgp
Description: PGP signature