summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZack Weinberg <zackw@panix.com>2017-08-11 09:43:59 -0400
committerZack Weinberg <zackw@panix.com>2017-08-23 07:42:53 -0400
commit38395be6ca77ffd3a1c09d68db6110333775dfdc (patch)
treef90d61997f4e0dee4b1727082d026a3121262ddf
parent5a706f649de3952271930a8340db4ca8aa50f485 (diff)
downloadglibc-zack/anon-unions.tar.gz
RFC: Use anonymous union for siginfo_tzack/anon-unions
C2011 officially includes an 'anonymous union' feature, and GCC has supported it for many years. It makes sub-fields of a union that's a struct field appear to be fields of the parent struct. If we use this in the definition of siginfo_t, we don't need to define lots of innocuous-looking identifiers like 'si_pid' as macros expanding to chains of field accessors. The catch, however, is that the compiler used to compile *programs that use glibc* - not just glibc itself - must accept the use of this feature (in system headers) even when running in an older conformance mode. This patch only touches siginfo_t, but if people like the idea, we could also do it for several other types: netinet/in.h (in6_addr) sys/stat.h (stat) utmp.h (utmp) signal.h (sigaction, sigevent_t) ucontext.h (ucontext_t) and maybe also - these use names in the user namespace for the fields that would be removed: net/if.h (ifaddr) ifaddrs.h (ifaddrs) netinet/in6.h (ip6_hdr) (really should be bitfields instead) netinet/icmp6.h (many) net/if_ppp.h (ifpppstatsreq, ifpppcstatsreq) net/if_shaper.h (shaperconf) a.out.h (exec) (only some versions) sys/quota.h (dqblk?) (the dq_* macros refer to a field that doesn't exist?!) There are still more hits in sunrpc and nis, but since hopefully that code is going to go away, I don't propose to mess with them. And there may be even more that aren't caught by grepping for '#define IDENT IDENT.IDENT'. As a side note (and this could be split for commit if felt appropriate), the siginfo_t field aliases 'si_int' and 'si_ptr' are not in POSIX. There are a few uses of these within glibc itself, and a handful more in third-party software (not glibc, not uclibc, and not linux) so I have preserved them, but put them under __USE_MISC and added a deprecation warning. This passes the glibc testsuite on x86-64-linux, which probably *doesn't* test the case where someone is compiling a program in an older conformance mode that uses siginfo_t (-std=c99 -D_XOPEN_SOURCE=600, perhaps). What do you think? zw * sysdeps/unix/sysv/linux/bits/types/siginfo_t.h (siginfo_t): Use C2011 anonymous union and anonymous struct-in-union features to define this type. Rename some public fields with their official names. (si_pid, si_uid, si_timerid, si_overrun, si_status, si_utime) (si_stime, si_value, si_addr, si_addr_lsb, si_lower, si_upper) (si_pkey, si_band, si_fd, si_call_addr, si_syscall, si_arch): Do not define as macros. (si_int, si_ptr): Define only when __USE_MISC, with deprecation warnings. * sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h * sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h * sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h (__SI_SIGFAULT_ADDL): Define all fields with their public names when __USE_GNU, or with impl-namespace names otherwise. (si_imm, si_segvflags, si_isr, si_trapno): Do not define as macros. * sysdeps/unix/sysv/linux/timer_routines.c (timer_helper_thread) Use si_value.sival_ptr instead of si_ptr. * sysdeps/unix/sysv/linux/tst-getpid1.c (do_test): Use si_value.sival_int instead of si_int.
-rw-r--r--NEWS16
-rw-r--r--sysdeps/unix/sysv/linux/bits/types/siginfo_t.h83
-rw-r--r--sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h17
-rw-r--r--sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h9
-rw-r--r--sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h9
-rw-r--r--sysdeps/unix/sysv/linux/timer_routines.c2
-rw-r--r--sysdeps/unix/sysv/linux/tst-getpid1.c5
7 files changed, 70 insertions, 71 deletions
diff --git a/NEWS b/NEWS
index 8fe0879bc4..482e355f4f 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,9 @@ Deprecated and removed features, and other changes affecting compatibility:
* On GNU/Linux, the obsolete Linux constant PTRACE_SEIZE_DEVEL is no longer
defined by <sys/ptrace.h>.
+* The nonstandard siginfo_t fields 'si_int' and 'si_ptr' are deprecated, and
+ will be removed in a future release. Programs should instead use
+ 'si_value.sival_int' and 'si_value.sival_ptr', respectively.
* libm no longer supports SVID error handling (calling a user-provided
matherr function on error) or the _LIB_VERSION variable to control error
@@ -27,7 +30,18 @@ Deprecated and removed features, and other changes affecting compatibility:
Changes to build and runtime requirements:
- [Add changes to build and runtime requirements here]
+* Some headers now make unconditional use of the C2011 'anonymous union'
+ feature. In order to compile a program that uses any of these headers,
+ you must use a compiler that supports this feature, even when conforming
+ to an earlier language standard. This has been true of GCC and Clang for
+ many years. The structures and headers affected are:
+
+ - siginfo_t (signal.h, sys/wait.h)
+
+ The advantage of this change is that these headers define many fewer
+ macros with surprising expansions (for instance, the identifier 'si_pid'
+ used to be a macro expanding to '_sifields._kill.si_pid' on GNU/Linux
+ systems).
Security related changes:
diff --git a/sysdeps/unix/sysv/linux/bits/types/siginfo_t.h b/sysdeps/unix/sysv/linux/bits/types/siginfo_t.h
index 33766d1813..2d5fbdaf30 100644
--- a/sysdeps/unix/sysv/linux/bits/types/siginfo_t.h
+++ b/sysdeps/unix/sysv/linux/bits/types/siginfo_t.h
@@ -52,38 +52,33 @@ typedef struct
{
int _pad[__SI_PAD_SIZE];
- /* kill(). */
+ /* Signals sent by kill or sigqueue. si_sigval is only valid for
+ sigqueue. */
struct
{
__pid_t si_pid; /* Sending process ID. */
__uid_t si_uid; /* Real user ID of sending process. */
- } _kill;
+ __sigval_t si_value; /* Signal value. */
+ };
- /* POSIX.1b timers. */
+ /* POSIX.1b timers. 'si_sigval' (above) is also valid. */
struct
{
- int si_tid; /* Timer ID. */
+ int si_timerid; /* Timer ID. */
int si_overrun; /* Overrun count. */
__sigval_t si_sigval; /* Signal value. */
- } _timer;
+ };
- /* POSIX.1b signals. */
+ /* SIGCHLD. The first two fields overlay the si_pid and si_uid
+ fields above. */
struct
{
- __pid_t si_pid; /* Sending process ID. */
- __uid_t si_uid; /* Real user ID of sending process. */
- __sigval_t si_sigval; /* Signal value. */
- } _rt;
-
- /* SIGCHLD. */
- struct
- {
- __pid_t si_pid; /* Which child. */
- __uid_t si_uid; /* Real user ID of sending process. */
+ __pid_t __sigchld_si_pid; /* Which child. */
+ __uid_t __sigchld_si_uid; /* Real user ID of sending process. */
int si_status; /* Exit value or signal. */
__SI_CLOCK_T si_utime;
__SI_CLOCK_T si_stime;
- } _sigchld;
+ };
/* SIGILL, SIGFPE, SIGSEGV, SIGBUS. */
struct
@@ -96,56 +91,42 @@ typedef struct
/* used when si_code=SEGV_BNDERR */
struct
{
- void *_lower;
- void *_upper;
- } _addr_bnd;
+ void *si_lower;
+ void *si_upper;
+ };
/* used when si_code=SEGV_PKUERR */
- __uint32_t _pkey;
- } _bounds;
- } _sigfault;
+ __uint32_t si_pkey;
+ };
+ };
/* SIGPOLL. */
struct
{
long int si_band; /* Band event for SIGPOLL. */
int si_fd;
- } _sigpoll;
+ };
/* SIGSYS. */
#if __SI_HAVE_SIGSYS
struct
{
- void *_call_addr; /* Calling user insn. */
- int _syscall; /* Triggering system call number. */
- unsigned int _arch; /* AUDIT_ARCH_* of syscall. */
- } _sigsys;
+ void *si_call_addr; /* Calling user insn. */
+ int si_syscall; /* Triggering system call number. */
+ unsigned int si_arch; /* AUDIT_ARCH_* of syscall. */
+ };
#endif
- } _sifields;
+ };
} siginfo_t __SI_ALIGNMENT;
-/* X/Open requires some more fields with fixed names. */
-#define si_pid _sifields._kill.si_pid
-#define si_uid _sifields._kill.si_uid
-#define si_timerid _sifields._timer.si_tid
-#define si_overrun _sifields._timer.si_overrun
-#define si_status _sifields._sigchld.si_status
-#define si_utime _sifields._sigchld.si_utime
-#define si_stime _sifields._sigchld.si_stime
-#define si_value _sifields._rt.si_sigval
-#define si_int _sifields._rt.si_sigval.sival_int
-#define si_ptr _sifields._rt.si_sigval.sival_ptr
-#define si_addr _sifields._sigfault.si_addr
-#define si_addr_lsb _sifields._sigfault.si_addr_lsb
-#define si_lower _sifields._sigfault._bounds._addr_bnd._lower
-#define si_upper _sifields._sigfault._bounds._addr_bnd._upper
-#define si_pkey _sifields._sigfault._bounds._pkey
-#define si_band _sifields._sigpoll.si_band
-#define si_fd _sifields._sigpoll.si_fd
-#if __SI_HAVE_SIGSYS
-# define si_call_addr _sifields._sigsys._call_addr
-# define si_syscall _sifields._sigsys._syscall
-# define si_arch _sifields._sigsys._arch
+/* These field aliases are not in POSIX, and are preserved for
+ backward compatibility only. They may be removed in a future
+ release. */
+#ifdef __USE_MISC
+#define si_int si_value.sival_int \
+ __glibc_macro_warning("si_int is deprecated, use si_value.sival_int instead")
+#define si_ptr si_value.sival_ptr \
+ __glibc_macro_warning("si_ptr is deprecated, use si_value.sival_ptr instead")
#endif
#endif
diff --git a/sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h b/sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h
index 8b5647062c..a2144c3771 100644
--- a/sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h
+++ b/sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h
@@ -3,15 +3,16 @@
#define __SI_HAVE_SIGSYS 0
-#define __SI_SIGFAULT_ADDL \
- int _si_imm; \
- unsigned int _si_flags; \
- unsigned long int _si_isr;
-
#ifdef __USE_GNU
-# define si_imm _sifields._sigfault._si_imm
-# define si_segvflags _sifields._sigfault._si_flags
-# define si_isr _sifields._sigfault._si_isr
+# define __SI_SIGFAULT_ADDL \
+ int si_imm; \
+ unsigned int si_segvflags; \
+ unsigned long int si_isr;
+#else
+# define __SI_SIGFAULT_ADDL \
+ int __si_imm; \
+ unsigned int __si_segvflags; \
+ unsigned long int __si_isr;
#endif
#endif
diff --git a/sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h b/sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h
index 9f79715ebe..146fa2c00a 100644
--- a/sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h
+++ b/sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h
@@ -4,9 +4,10 @@
#define __SI_BAND_TYPE int
-#define __SI_SIGFAULT_ADDL \
- int _si_trapno;
-
-#define si_trapno _sifields._sigfault._si_trapno
+#ifdef __USE_GNU
+# define __SI_SIGFAULT_ADDL int si_trapno;
+#else
+# define __SI_SIGFAULT_ADDL int __si_trapno;
+#endif
#endif
diff --git a/sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h b/sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h
index 7d0c24c84b..0421fa3585 100644
--- a/sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h
+++ b/sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h
@@ -2,9 +2,10 @@
#ifndef _BITS_SIGINFO_ARCH_H
#define _BITS_SIGINFO_ARCH_H 1
-#define __SI_SIGFAULT_ADDL \
- int _si_trapno;
-
-#define si_trapno _sifields._sigfault._si_trapno
+#ifdef __USE_GNU
+# define __SI_SIGFAULT_ADDL int si_trapno;
+#else
+# define __SI_SIGFAULT_ADDL int __si_trapno;
+#endif
#endif
diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c
index 1d81304678..696ab450c6 100644
--- a/sysdeps/unix/sysv/linux/timer_routines.c
+++ b/sysdeps/unix/sysv/linux/timer_routines.c
@@ -92,7 +92,7 @@ timer_helper_thread (void *arg)
{
if (si.si_code == SI_TIMER)
{
- struct timer *tk = (struct timer *) si.si_ptr;
+ struct timer *tk = (struct timer *) si.si_value.sival_ptr;
/* Check the timer is still used and will not go away
while we are reading the values here. */
diff --git a/sysdeps/unix/sysv/linux/tst-getpid1.c b/sysdeps/unix/sysv/linux/tst-getpid1.c
index 253ebf2e15..f24faf35e1 100644
--- a/sysdeps/unix/sysv/linux/tst-getpid1.c
+++ b/sysdeps/unix/sysv/linux/tst-getpid1.c
@@ -95,9 +95,10 @@ do_test (void)
return 1;
}
- if (si.si_int != (int) p)
+ if (si.si_value.sival_int != (int) p)
{
- printf ("expected PID %d, got si_int %d\n", (int) p, si.si_int);
+ printf ("expected PID %d, got si_int %d\n",
+ (int) p, si.si_value.sival_int);
kill (p, SIGKILL);
return 1;
}