* [RFC PATCH] Use anonymous union for siginfo_t
@ 2017-08-11 14:03 Zack Weinberg
2017-08-11 16:25 ` Joseph Myers
2017-08-11 16:49 ` Paul Eggert
0 siblings, 2 replies; 4+ messages in thread
From: Zack Weinberg @ 2017-08-11 14:03 UTC (permalink / raw)
To: libc-alpha; +Cc: joseph
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.
---
NEWS | 16 ++++-
sysdeps/unix/sysv/linux/bits/types/siginfo_t.h | 84 +++++++++--------------
sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h | 17 ++---
sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h | 9 +--
sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h | 9 +--
sysdeps/unix/sysv/linux/timer_routines.c | 2 +-
sysdeps/unix/sysv/linux/tst-getpid1.c | 5 +-
7 files changed, 70 insertions(+), 72 deletions(-)
diff --git a/NEWS b/NEWS
index 484c467569..f601ecbb37 100644
--- a/NEWS
+++ b/NEWS
@@ -15,10 +15,24 @@ 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.
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 bed69148f9..dc9265b2a4 100644
--- a/sysdeps/unix/sysv/linux/bits/types/siginfo_t.h
+++ b/sysdeps/unix/sysv/linux/bits/types/siginfo_t.h
@@ -52,38 +52,32 @@ 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 +90,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;
}
--
2.14.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] Use anonymous union for siginfo_t
2017-08-11 14:03 [RFC PATCH] Use anonymous union for siginfo_t Zack Weinberg
@ 2017-08-11 16:25 ` Joseph Myers
2017-08-11 16:49 ` Paul Eggert
1 sibling, 0 replies; 4+ messages in thread
From: Joseph Myers @ 2017-08-11 16:25 UTC (permalink / raw)
To: Zack Weinberg; +Cc: libc-alpha
On Fri, 11 Aug 2017, Zack Weinberg wrote:
> +* Some headers now make unconditional use of the C2011 'anonymous union'
> + feature. In order to compile a program that uses any of these headers,
This is not entirely new. sysdeps/gnu/netinet/tcp.h has used this feature
(inside __extension__) since the _BSD_SOURCE removal, to reconcile two
different APIs for the same structure. There are probably other uses.
(sysdeps/unix/sysv/linux/alpha/bits/stat.h has one conditioned on
__GNUC_PREREQ(3,3), where those conditions could be removed if we agree
this feature is unconditionally required for using glibc headers.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] Use anonymous union for siginfo_t
2017-08-11 14:03 [RFC PATCH] Use anonymous union for siginfo_t Zack Weinberg
2017-08-11 16:25 ` Joseph Myers
@ 2017-08-11 16:49 ` Paul Eggert
2017-08-11 17:57 ` Zack Weinberg
1 sibling, 1 reply; 4+ messages in thread
From: Paul Eggert @ 2017-08-11 16:49 UTC (permalink / raw)
To: Zack Weinberg, libc-alpha; +Cc: joseph
On 08/11/2017 07:02 AM, Zack Weinberg wrote:
> 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.
Perhaps you could survey other non-GCC compilers in use with glibc, as
to whether they support this feature. Not just Clang, but also ARM, Cray
CCE, Fujitsu/Lahey, IBM XL C/C++, Intel, PGI, Oracle Studio, and maybe
PathScale. If recent versions of compilers in practical use on glibc
platforms support the feature, that should be good enough.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] Use anonymous union for siginfo_t
2017-08-11 16:49 ` Paul Eggert
@ 2017-08-11 17:57 ` Zack Weinberg
0 siblings, 0 replies; 4+ messages in thread
From: Zack Weinberg @ 2017-08-11 17:57 UTC (permalink / raw)
To: Paul Eggert; +Cc: GNU C Library
On Fri, Aug 11, 2017 at 12:48 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 08/11/2017 07:02 AM, Zack Weinberg wrote:
>>
>> 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.
>
>
> Perhaps you could survey other non-GCC compilers in use with glibc, as to
> whether they support this feature. Not just Clang, but also ARM, Cray CCE,
> Fujitsu/Lahey, IBM XL C/C++, Intel, PGI, Oracle Studio, and maybe PathScale.
This is a fine idea but I don't know that I can get my hands on all of
these compilers, certainly not this week or next week (family-related
travel). Perhaps people who already have them could give it a try?
This test program should be sufficient...
#define _XOPEN_SOURCE 600
#include <signal.h>
extern void do_something(int);
void
handler(int sig, siginfo_t *info, void *ctx)
{
do_something(info->si_value.sival_int);
}
compiled with the equivalent of -c -std=c99 and the warnings cranked
up as high as they will go. I'll push my branch when I get home.
zw
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-11 17:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-11 14:03 [RFC PATCH] Use anonymous union for siginfo_t Zack Weinberg
2017-08-11 16:25 ` Joseph Myers
2017-08-11 16:49 ` Paul Eggert
2017-08-11 17:57 ` Zack Weinberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).