| Message ID | 87zhhyv4nj.fsf@oldenburg2.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | nptl: SIGCANCEL, SIGTIMER, SIGSETXID are always defined | expand |
On 10/18/19 4:28 AM, Florian Weimer wrote: > All nptl targets have these signal definitions nowadays. This > changes also replaces the nptl-generic version of pthread_sigmask > with the Linux version. > > Tested on x86_64-linux-gnu and i686-linux-gnu. Built with > build-many-glibcs.py. This looks good to me. I agree all three defines should always be defined by all targets. Folding the Linux version is OK, since it's largely generic based on the various standard signal handling APIs. OK for master. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > ----- > nptl/allocatestack.c | 3 -- > nptl/nptl-init.c | 16 --------- > nptl/pthread_cancel.c | 7 ---- > nptl/pthread_create.c | 2 -- > nptl/pthread_setcanceltype.c | 5 --- > nptl/pthread_sigmask.c | 40 ++++++++++++++++------- > nptl/tst-cancel25.c | 2 -- > nptl/tst-signal7.c | 4 --- > sysdeps/nptl/allocrtsig.c | 29 +++-------------- > sysdeps/unix/sysv/linux/pthread_sigmask.c | 54 ------------------------------- > 10 files changed, 34 insertions(+), 128 deletions(-) > > diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c > index 64a9ae6066..086efb75ec 100644 > --- a/nptl/allocatestack.c > +++ b/nptl/allocatestack.c > @@ -963,7 +963,6 @@ __reclaim_stacks (void) > } > > > -#ifdef SIGSETXID > static void > setxid_mark_thread (struct xid_command *cmdp, struct pthread *t) > { > @@ -1174,8 +1173,6 @@ __nptl_setxid (struct xid_command *cmdp) > lll_unlock (stack_cache_lock, LLL_PRIVATE); > return result; > } > -#endif /* SIGSETXID. */ > - > OK. > static inline void __attribute__((always_inline)) > init_one_static_tls (struct pthread *curp, struct link_map *map) > diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c > index ea91b9e138..acc0f3672b 100644 > --- a/nptl/nptl-init.c > +++ b/nptl/nptl-init.c > @@ -114,9 +114,7 @@ static const struct pthread_functions pthread_functions = > .ptr_nthreads = &__nptl_nthreads, > .ptr___pthread_unwind = &__pthread_unwind, > .ptr__nptl_deallocate_tsd = __nptl_deallocate_tsd, > -# ifdef SIGSETXID > .ptr__nptl_setxid = __nptl_setxid, > -# endif OK. > .ptr_set_robust = __nptl_set_robust > }; > # define ptr_pthread_functions &pthread_functions > @@ -139,7 +137,6 @@ __nptl_set_robust (struct pthread *self) > } > > > -#ifdef SIGCANCEL > /* For asynchronous cancellation we use a signal. This is the handler. */ > static void > sigcancel_handler (int sig, siginfo_t *si, void *ctx) > @@ -185,10 +182,8 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) > oldval = curval; > } > } > -#endif > OK. > > -#ifdef SIGSETXID > struct xid_command *__xidcmd attribute_hidden; > > /* We use the SIGSETXID signal in the setuid, setgid, etc. implementations to > @@ -234,7 +229,6 @@ sighandler_setxid (int sig, siginfo_t *si, void *ctx) > if (atomic_decrement_val (&__xidcmd->cntr) == 0) > futex_wake ((unsigned int *) &__xidcmd->cntr, 1, FUTEX_PRIVATE); > } > -#endif OK. > > > /* When using __thread for this, we do it in libc so as not > @@ -285,41 +279,31 @@ __pthread_initialize_minimal_internal (void) > had to set __nptl_initial_report_events. Propagate its setting. */ > THREAD_SETMEM (pd, report_events, __nptl_initial_report_events); > > -#if defined SIGCANCEL || defined SIGSETXID > struct sigaction sa; > __sigemptyset (&sa.sa_mask); > > -# ifdef SIGCANCEL > /* Install the cancellation signal handler. If for some reason we > cannot install the handler we do not abort. Maybe we should, but > it is only asynchronous cancellation which is affected. */ > sa.sa_sigaction = sigcancel_handler; > sa.sa_flags = SA_SIGINFO; > (void) __libc_sigaction (SIGCANCEL, &sa, NULL); > -# endif > > -# ifdef SIGSETXID > /* Install the handle to change the threads' uid/gid. */ > sa.sa_sigaction = sighandler_setxid; > sa.sa_flags = SA_SIGINFO | SA_RESTART; > (void) __libc_sigaction (SIGSETXID, &sa, NULL); > -# endif > > /* The parent process might have left the signals blocked. Just in > case, unblock it. We reuse the signal mask in the sigaction > structure. It is already cleared. */ > -# ifdef SIGCANCEL > __sigaddset (&sa.sa_mask, SIGCANCEL); > -# endif > -# ifdef SIGSETXID > __sigaddset (&sa.sa_mask, SIGSETXID); > -# endif OK. > { > INTERNAL_SYSCALL_DECL (err); > (void) INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_UNBLOCK, &sa.sa_mask, > NULL, _NSIG / 8); > } > -#endif > > /* Get the size of the static and alignment requirements for the TLS > block. */ > diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c > index 64ac12e60a..f6be8c1dd6 100644 > --- a/nptl/pthread_cancel.c > +++ b/nptl/pthread_cancel.c > @@ -63,7 +63,6 @@ __pthread_cancel (pthread_t th) > oldval)) > goto again; > > -#ifdef SIGCANCEL > /* The cancellation handler will take care of marking the > thread as canceled. */ > pid_t pid = __getpid (); > @@ -73,12 +72,6 @@ __pthread_cancel (pthread_t th) > SIGCANCEL); > if (INTERNAL_SYSCALL_ERROR_P (val, err)) > result = INTERNAL_SYSCALL_ERRNO (val, err); > -#else > - /* It should be impossible to get here at all, since > - pthread_setcanceltype should never have allowed > - PTHREAD_CANCEL_ASYNCHRONOUS to be set. */ > - abort (); > -#endif OK. Remove else entirely which is good. > > break; > } > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index 130937c3c4..5682c9c2c0 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -402,7 +402,6 @@ START_THREAD_DEFN > } > #endif > > -#ifdef SIGCANCEL > /* If the parent was running cancellation handlers while creating > the thread the new thread inherited the signal mask. Reset the > cancellation signal mask. */ > @@ -415,7 +414,6 @@ START_THREAD_DEFN > (void) INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_UNBLOCK, &mask, > NULL, _NSIG / 8); > } > -#endif OK. > > /* This is where the try/finally block should be created. For > compilers without that support we do use setjmp. */ > diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c > index d771c31e46..99bc13e993 100644 > --- a/nptl/pthread_setcanceltype.c > +++ b/nptl/pthread_setcanceltype.c > @@ -27,11 +27,6 @@ __pthread_setcanceltype (int type, int *oldtype) > if (type < PTHREAD_CANCEL_DEFERRED || type > PTHREAD_CANCEL_ASYNCHRONOUS) > return EINVAL; > > -#ifndef SIGCANCEL > - if (type == PTHREAD_CANCEL_ASYNCHRONOUS) > - return ENOTSUP; > -#endif OK. > - > volatile struct pthread *self = THREAD_SELF; > > int oldval = THREAD_GETMEM (self, cancelhandling); > diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c > index c8df527995..4aa774d01d 100644 > --- a/nptl/pthread_sigmask.c > +++ b/nptl/pthread_sigmask.c > @@ -1,6 +1,6 @@ > -/* Examine and change blocked signals for a thread. Generic POSIX version. > - Copyright (C) 2014-2019 Free Software Foundation, Inc. > +/* Copyright (C) 2002-2019 Free Software Foundation, Inc. > This file is part of the GNU C Library. > + Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. > > The GNU C Library is free software; you can redistribute it and/or > modify it under the terms of the GNU Lesser General Public > @@ -19,18 +19,36 @@ > #include <errno.h> > #include <signal.h> > #include <pthreadP.h> > +#include <sysdep.h> > > -#if defined SIGCANCEL || defined SIGTIMER || defined SIGSETXID > -# error "This implementation assumes no internal-only signal numbers." > -#endif OK. > > int > pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask) > { > - /* Here we assume that sigprocmask actually does everything right. > - The only difference is the return value protocol. */ > - int result = sigprocmask (how, newmask, oldmask); > - if (result < 0) > - result = errno; > - return result; > + sigset_t local_newmask; > + > + /* The only thing we have to make sure here is that SIGCANCEL and > + SIGSETXID is not blocked. */ > + if (newmask != NULL > + && (__builtin_expect (__sigismember (newmask, SIGCANCEL), 0) > + || __builtin_expect (__sigismember (newmask, SIGSETXID), 0))) > + { > + local_newmask = *newmask; > + __sigdelset (&local_newmask, SIGCANCEL); > + __sigdelset (&local_newmask, SIGSETXID); > + newmask = &local_newmask; > + } > + > +#ifdef INTERNAL_SYSCALL > + /* We know that realtime signals are available if NPTL is used. */ > + INTERNAL_SYSCALL_DECL (err); > + int result = INTERNAL_SYSCALL (rt_sigprocmask, err, 4, how, newmask, > + oldmask, _NSIG / 8); > + > + return (INTERNAL_SYSCALL_ERROR_P (result, err) > + ? INTERNAL_SYSCALL_ERRNO (result, err) > + : 0); > +#else > + return sigprocmask (how, newmask, oldmask) == -1 ? errno : 0; > +#endif OK, this is the generic and linux versions folded together. > } > diff --git a/nptl/tst-cancel25.c b/nptl/tst-cancel25.c > index 24ddd3c01c..c6e1c23c02 100644 > --- a/nptl/tst-cancel25.c > +++ b/nptl/tst-cancel25.c > @@ -12,7 +12,6 @@ static pthread_t th2; > static void * > tf2 (void *arg) > { > -#ifdef SIGCANCEL > sigset_t mask; > if (pthread_sigmask (SIG_SETMASK, NULL, &mask) != 0) > { > @@ -24,7 +23,6 @@ tf2 (void *arg) > puts ("SIGCANCEL blocked in new thread"); > exit (1); > } > -#endif OK. > > /* Sync with the main thread so that we do not test anything else. */ > int e = pthread_barrier_wait (&b); > diff --git a/nptl/tst-signal7.c b/nptl/tst-signal7.c > index de67ab493c..f42ff6887a 100644 > --- a/nptl/tst-signal7.c > +++ b/nptl/tst-signal7.c > @@ -27,7 +27,6 @@ do_test (void) > { > int result = 0; > > -#ifdef SIGCANCEL > errno = 0; > if (sigaction (SIGCANCEL, NULL, NULL) == 0) > { > @@ -39,9 +38,7 @@ do_test (void) > puts ("sigaction(SIGCANCEL) did not set errno to EINVAL"); > result = 1; > } > -#endif > > -#ifdef SIGSETXID > errno = 0; > if (sigaction (SIGSETXID, NULL, NULL) == 0) > { > @@ -53,7 +50,6 @@ do_test (void) > puts ("sigaction(SIGSETXID) did not set errno to EINVAL"); > result = 1; > } > -#endif OK. > > return result; > } > diff --git a/sysdeps/nptl/allocrtsig.c b/sysdeps/nptl/allocrtsig.c > index e9ea038655..3f62bf40e7 100644 > --- a/sysdeps/nptl/allocrtsig.c > +++ b/sysdeps/nptl/allocrtsig.c > @@ -19,32 +19,13 @@ > #include <signal.h> > #include <nptl/pthreadP.h> > > -/* Up to three special signals might be used privately by libpthread. > - Figure out how many unique ones are actually used. */ > - > -#ifdef SIGCANCEL > -# define SIGCANCEL_CONSUMES 1 > -#else > -# define SIGCANCEL_CONSUMES 0 > -#endif > - > -#if defined SIGTIMER && (!defined SIGCANCEL || SIGTIMER != SIGCANCEL) > -# define SIGTIMER_CONSUMES 1 > -#else > -# define SIGTIMER_CONSUMES 0 > -#endif > - > -#if (defined SIGSETXID \ > - && (!defined SIGCANCEL || SIGSETXID != SIGCANCEL) \ > - && (!defined SIGTIMER || SIGSETXID != SIGTIMER)) > -# define SIGSETXID_CONSUMES 1 > -#else > -# define SIGSETXID_CONSUMES 0 > +#if SIGTIMER != SIGCANCEL > +# error "SIGTIMER and SIGCANCEL must be the same" > #endif OK. Remove the *_CONSUMES macros. > > /* This tells the generic code (included below) how many signal > - numbers need to be reserved for libpthread's private uses. */ > -#define RESERVED_SIGRT \ > - (SIGCANCEL_CONSUMES + SIGTIMER_CONSUMES + SIGSETXID_CONSUMES) > + numbers need to be reserved for libpthread's private uses > + (SIGCANCEL and SIGSETXID). */ > +#define RESERVED_SIGRT 2 OK. > > #include <signal/allocrtsig.c> > diff --git a/sysdeps/unix/sysv/linux/pthread_sigmask.c b/sysdeps/unix/sysv/linux/pthread_sigmask.c > deleted file mode 100644 > index 4aa774d01d..0000000000 > --- a/sysdeps/unix/sysv/linux/pthread_sigmask.c > +++ /dev/null > @@ -1,54 +0,0 @@ > -/* Copyright (C) 2002-2019 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <https://www.gnu.org/licenses/>. */ > - > -#include <errno.h> > -#include <signal.h> > -#include <pthreadP.h> > -#include <sysdep.h> > - > - > -int > -pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask) > -{ > - sigset_t local_newmask; > - > - /* The only thing we have to make sure here is that SIGCANCEL and > - SIGSETXID is not blocked. */ > - if (newmask != NULL > - && (__builtin_expect (__sigismember (newmask, SIGCANCEL), 0) > - || __builtin_expect (__sigismember (newmask, SIGSETXID), 0))) > - { > - local_newmask = *newmask; > - __sigdelset (&local_newmask, SIGCANCEL); > - __sigdelset (&local_newmask, SIGSETXID); > - newmask = &local_newmask; > - } > - > -#ifdef INTERNAL_SYSCALL > - /* We know that realtime signals are available if NPTL is used. */ > - INTERNAL_SYSCALL_DECL (err); > - int result = INTERNAL_SYSCALL (rt_sigprocmask, err, 4, how, newmask, > - oldmask, _NSIG / 8); > - > - return (INTERNAL_SYSCALL_ERROR_P (result, err) > - ? INTERNAL_SYSCALL_ERRNO (result, err) > - : 0); > -#else > - return sigprocmask (how, newmask, oldmask) == -1 ? errno : 0; > -#endif > -} OK. >
On 18/10/2019 05:28, Florian Weimer wrote: > All nptl targets have these signal definitions nowadays. This > changes also replaces the nptl-generic version of pthread_sigmask > with the Linux version. > > Tested on x86_64-linux-gnu and i686-linux-gnu. Built with > build-many-glibcs.py. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > ----- > nptl/allocatestack.c | 3 -- > nptl/nptl-init.c | 16 --------- > nptl/pthread_cancel.c | 7 ---- > nptl/pthread_create.c | 2 -- > nptl/pthread_setcanceltype.c | 5 --- > nptl/pthread_sigmask.c | 40 ++++++++++++++++------- > nptl/tst-cancel25.c | 2 -- > nptl/tst-signal7.c | 4 --- > sysdeps/nptl/allocrtsig.c | 29 +++-------------- > sysdeps/unix/sysv/linux/pthread_sigmask.c | 54 ------------------------------- > 10 files changed, 34 insertions(+), 128 deletions(-) > > diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c > index 64a9ae6066..086efb75ec 100644 > --- a/nptl/allocatestack.c > +++ b/nptl/allocatestack.c > @@ -963,7 +963,6 @@ __reclaim_stacks (void) > } > > > -#ifdef SIGSETXID > static void > setxid_mark_thread (struct xid_command *cmdp, struct pthread *t) > { > @@ -1174,8 +1173,6 @@ __nptl_setxid (struct xid_command *cmdp) > lll_unlock (stack_cache_lock, LLL_PRIVATE); > return result; > } > -#endif /* SIGSETXID. */ > - > > static inline void __attribute__((always_inline)) > init_one_static_tls (struct pthread *curp, struct link_map *map) Ok. > diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c > index ea91b9e138..acc0f3672b 100644 > --- a/nptl/nptl-init.c > +++ b/nptl/nptl-init.c > @@ -114,9 +114,7 @@ static const struct pthread_functions pthread_functions = > .ptr_nthreads = &__nptl_nthreads, > .ptr___pthread_unwind = &__pthread_unwind, > .ptr__nptl_deallocate_tsd = __nptl_deallocate_tsd, > -# ifdef SIGSETXID > .ptr__nptl_setxid = __nptl_setxid, > -# endif > .ptr_set_robust = __nptl_set_robust > }; > # define ptr_pthread_functions &pthread_functions Ok. > @@ -139,7 +137,6 @@ __nptl_set_robust (struct pthread *self) > } > > > -#ifdef SIGCANCEL > /* For asynchronous cancellation we use a signal. This is the handler. */ > static void > sigcancel_handler (int sig, siginfo_t *si, void *ctx) > @@ -185,10 +182,8 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) > oldval = curval; > } > } > -#endif > > > -#ifdef SIGSETXID > struct xid_command *__xidcmd attribute_hidden; > > /* We use the SIGSETXID signal in the setuid, setgid, etc. implementations to > @@ -234,7 +229,6 @@ sighandler_setxid (int sig, siginfo_t *si, void *ctx) > if (atomic_decrement_val (&__xidcmd->cntr) == 0) > futex_wake ((unsigned int *) &__xidcmd->cntr, 1, FUTEX_PRIVATE); > } > -#endif > > > /* When using __thread for this, we do it in libc so as not Ok. > @@ -285,41 +279,31 @@ __pthread_initialize_minimal_internal (void) > had to set __nptl_initial_report_events. Propagate its setting. */ > THREAD_SETMEM (pd, report_events, __nptl_initial_report_events); > > -#if defined SIGCANCEL || defined SIGSETXID > struct sigaction sa; > __sigemptyset (&sa.sa_mask); > > -# ifdef SIGCANCEL > /* Install the cancellation signal handler. If for some reason we > cannot install the handler we do not abort. Maybe we should, but > it is only asynchronous cancellation which is affected. */ > sa.sa_sigaction = sigcancel_handler; > sa.sa_flags = SA_SIGINFO; > (void) __libc_sigaction (SIGCANCEL, &sa, NULL); > -# endif > > -# ifdef SIGSETXID > /* Install the handle to change the threads' uid/gid. */ > sa.sa_sigaction = sighandler_setxid; > sa.sa_flags = SA_SIGINFO | SA_RESTART; > (void) __libc_sigaction (SIGSETXID, &sa, NULL); > -# endif > > /* The parent process might have left the signals blocked. Just in > case, unblock it. We reuse the signal mask in the sigaction > structure. It is already cleared. */ > -# ifdef SIGCANCEL > __sigaddset (&sa.sa_mask, SIGCANCEL); > -# endif > -# ifdef SIGSETXID > __sigaddset (&sa.sa_mask, SIGSETXID); > -# endif > { > INTERNAL_SYSCALL_DECL (err); > (void) INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_UNBLOCK, &sa.sa_mask, > NULL, _NSIG / 8); > } > -#endif > > /* Get the size of the static and alignment requirements for the TLS > block. */ Ok. > diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c > index 64ac12e60a..f6be8c1dd6 100644 > --- a/nptl/pthread_cancel.c > +++ b/nptl/pthread_cancel.c > @@ -63,7 +63,6 @@ __pthread_cancel (pthread_t th) > oldval)) > goto again; > > -#ifdef SIGCANCEL > /* The cancellation handler will take care of marking the > thread as canceled. */ > pid_t pid = __getpid (); > @@ -73,12 +72,6 @@ __pthread_cancel (pthread_t th) > SIGCANCEL); > if (INTERNAL_SYSCALL_ERROR_P (val, err)) > result = INTERNAL_SYSCALL_ERRNO (val, err); > -#else > - /* It should be impossible to get here at all, since > - pthread_setcanceltype should never have allowed > - PTHREAD_CANCEL_ASYNCHRONOUS to be set. */ > - abort (); > -#endif > > break; > } Ok. > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index 130937c3c4..5682c9c2c0 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -402,7 +402,6 @@ START_THREAD_DEFN > } > #endif > > -#ifdef SIGCANCEL > /* If the parent was running cancellation handlers while creating > the thread the new thread inherited the signal mask. Reset the > cancellation signal mask. */ > @@ -415,7 +414,6 @@ START_THREAD_DEFN > (void) INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_UNBLOCK, &mask, > NULL, _NSIG / 8); > } > -#endif > > /* This is where the try/finally block should be created. For > compilers without that support we do use setjmp. */ OK. > diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c > index d771c31e46..99bc13e993 100644 > --- a/nptl/pthread_setcanceltype.c > +++ b/nptl/pthread_setcanceltype.c > @@ -27,11 +27,6 @@ __pthread_setcanceltype (int type, int *oldtype) > if (type < PTHREAD_CANCEL_DEFERRED || type > PTHREAD_CANCEL_ASYNCHRONOUS) > return EINVAL; > > -#ifndef SIGCANCEL > - if (type == PTHREAD_CANCEL_ASYNCHRONOUS) > - return ENOTSUP; > -#endif > - > volatile struct pthread *self = THREAD_SELF; > > int oldval = THREAD_GETMEM (self, cancelhandling); OK. > diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c > index c8df527995..4aa774d01d 100644 > --- a/nptl/pthread_sigmask.c > +++ b/nptl/pthread_sigmask.c > @@ -1,6 +1,6 @@ > -/* Examine and change blocked signals for a thread. Generic POSIX version. > - Copyright (C) 2014-2019 Free Software Foundation, Inc. > +/* Copyright (C) 2002-2019 Free Software Foundation, Inc. > This file is part of the GNU C Library. > + Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. > > The GNU C Library is free software; you can redistribute it and/or > modify it under the terms of the GNU Lesser General Public I would say to keep the only comment, maybe adjusting the following line. > @@ -19,18 +19,36 @@ > #include <errno.h> > #include <signal.h> > #include <pthreadP.h> > +#include <sysdep.h> > > -#if defined SIGCANCEL || defined SIGTIMER || defined SIGSETXID > -# error "This implementation assumes no internal-only signal numbers." > -#endif > > int > pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask) > { > - /* Here we assume that sigprocmask actually does everything right. > - The only difference is the return value protocol. */ > - int result = sigprocmask (how, newmask, oldmask); > - if (result < 0) > - result = errno; > - return result; > + sigset_t local_newmask; > + > + /* The only thing we have to make sure here is that SIGCANCEL and > + SIGSETXID is not blocked. */ > + if (newmask != NULL > + && (__builtin_expect (__sigismember (newmask, SIGCANCEL), 0) > + || __builtin_expect (__sigismember (newmask, SIGSETXID), 0))) > + { Since you are touching it replace both with __glibc_likely. > + local_newmask = *newmask; > + __sigdelset (&local_newmask, SIGCANCEL); > + __sigdelset (&local_newmask, SIGSETXID); > + newmask = &local_newmask; > + } > + > +#ifdef INTERNAL_SYSCALL > + /* We know that realtime signals are available if NPTL is used. */ > + INTERNAL_SYSCALL_DECL (err); > + int result = INTERNAL_SYSCALL (rt_sigprocmask, err, 4, how, newmask, > + oldmask, _NSIG / 8); > + > + return (INTERNAL_SYSCALL_ERROR_P (result, err) > + ? INTERNAL_SYSCALL_ERRNO (result, err) > + : 0); > +#else > + return sigprocmask (how, newmask, oldmask) == -1 ? errno : 0; > +#endif > } Also, I think there is no need to keep checking for INTERNAL_SYSCALL. Just use INTERNAL_SYSCALL_CALL. > diff --git a/nptl/tst-cancel25.c b/nptl/tst-cancel25.c > index 24ddd3c01c..c6e1c23c02 100644 > --- a/nptl/tst-cancel25.c > +++ b/nptl/tst-cancel25.c > @@ -12,7 +12,6 @@ static pthread_t th2; > static void * > tf2 (void *arg) > { > -#ifdef SIGCANCEL > sigset_t mask; > if (pthread_sigmask (SIG_SETMASK, NULL, &mask) != 0) > { > @@ -24,7 +23,6 @@ tf2 (void *arg) > puts ("SIGCANCEL blocked in new thread"); > exit (1); > } > -#endif > > /* Sync with the main thread so that we do not test anything else. */ > int e = pthread_barrier_wait (&b); Ok. > diff --git a/nptl/tst-signal7.c b/nptl/tst-signal7.c > index de67ab493c..f42ff6887a 100644 > --- a/nptl/tst-signal7.c > +++ b/nptl/tst-signal7.c > @@ -27,7 +27,6 @@ do_test (void) > { > int result = 0; > > -#ifdef SIGCANCEL > errno = 0; > if (sigaction (SIGCANCEL, NULL, NULL) == 0) > { > @@ -39,9 +38,7 @@ do_test (void) > puts ("sigaction(SIGCANCEL) did not set errno to EINVAL"); > result = 1; > } > -#endif > > -#ifdef SIGSETXID > errno = 0; > if (sigaction (SIGSETXID, NULL, NULL) == 0) > { > @@ -53,7 +50,6 @@ do_test (void) > puts ("sigaction(SIGSETXID) did not set errno to EINVAL"); > result = 1; > } > -#endif > > return result; > } Ok. > diff --git a/sysdeps/nptl/allocrtsig.c b/sysdeps/nptl/allocrtsig.c > index e9ea038655..3f62bf40e7 100644 > --- a/sysdeps/nptl/allocrtsig.c > +++ b/sysdeps/nptl/allocrtsig.c > @@ -19,32 +19,13 @@ > #include <signal.h> > #include <nptl/pthreadP.h> > > -/* Up to three special signals might be used privately by libpthread. > - Figure out how many unique ones are actually used. */ > - > -#ifdef SIGCANCEL > -# define SIGCANCEL_CONSUMES 1 > -#else > -# define SIGCANCEL_CONSUMES 0 > -#endif > - > -#if defined SIGTIMER && (!defined SIGCANCEL || SIGTIMER != SIGCANCEL) > -# define SIGTIMER_CONSUMES 1 > -#else > -# define SIGTIMER_CONSUMES 0 > -#endif > - > -#if (defined SIGSETXID \ > - && (!defined SIGCANCEL || SIGSETXID != SIGCANCEL) \ > - && (!defined SIGTIMER || SIGSETXID != SIGTIMER)) > -# define SIGSETXID_CONSUMES 1 > -#else > -# define SIGSETXID_CONSUMES 0 > +#if SIGTIMER != SIGCANCEL > +# error "SIGTIMER and SIGCANCEL must be the same" > #endif Not sure if it is really required them to be the same. > > /* This tells the generic code (included below) how many signal > - numbers need to be reserved for libpthread's private uses. */ > -#define RESERVED_SIGRT \ > - (SIGCANCEL_CONSUMES + SIGTIMER_CONSUMES + SIGSETXID_CONSUMES) > + numbers need to be reserved for libpthread's private uses > + (SIGCANCEL and SIGSETXID). */ > +#define RESERVED_SIGRT 2 > > #include <signal/allocrtsig.c> Ok. > diff --git a/sysdeps/unix/sysv/linux/pthread_sigmask.c b/sysdeps/unix/sysv/linux/pthread_sigmask.c > deleted file mode 100644 > index 4aa774d01d..0000000000 > --- a/sysdeps/unix/sysv/linux/pthread_sigmask.c > +++ /dev/null > @@ -1,54 +0,0 @@ > -/* Copyright (C) 2002-2019 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <https://www.gnu.org/licenses/>. */ > - > -#include <errno.h> > -#include <signal.h> > -#include <pthreadP.h> > -#include <sysdep.h> > - > - > -int > -pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask) > -{ > - sigset_t local_newmask; > - > - /* The only thing we have to make sure here is that SIGCANCEL and > - SIGSETXID is not blocked. */ > - if (newmask != NULL > - && (__builtin_expect (__sigismember (newmask, SIGCANCEL), 0) > - || __builtin_expect (__sigismember (newmask, SIGSETXID), 0))) > - { > - local_newmask = *newmask; > - __sigdelset (&local_newmask, SIGCANCEL); > - __sigdelset (&local_newmask, SIGSETXID); > - newmask = &local_newmask; > - } > - > -#ifdef INTERNAL_SYSCALL > - /* We know that realtime signals are available if NPTL is used. */ > - INTERNAL_SYSCALL_DECL (err); > - int result = INTERNAL_SYSCALL (rt_sigprocmask, err, 4, how, newmask, > - oldmask, _NSIG / 8); > - > - return (INTERNAL_SYSCALL_ERROR_P (result, err) > - ? INTERNAL_SYSCALL_ERRNO (result, err) > - : 0); > -#else > - return sigprocmask (how, newmask, oldmask) == -1 ? errno : 0; > -#endif > -} > Ok.
* Adhemerval Zanella: > On 18/10/2019 05:28, Florian Weimer wrote: >> All nptl targets have these signal definitions nowadays. This >> changes also replaces the nptl-generic version of pthread_sigmask >> with the Linux version. >> >> Tested on x86_64-linux-gnu and i686-linux-gnu. Built with >> build-many-glibcs.py. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Sorry, I didn't see your review in my inbox, only Carlos'. >> diff --git a/sysdeps/nptl/allocrtsig.c b/sysdeps/nptl/allocrtsig.c >> index e9ea038655..3f62bf40e7 100644 >> --- a/sysdeps/nptl/allocrtsig.c >> +++ b/sysdeps/nptl/allocrtsig.c >> @@ -19,32 +19,13 @@ >> #include <signal.h> >> #include <nptl/pthreadP.h> >> >> -/* Up to three special signals might be used privately by libpthread. >> - Figure out how many unique ones are actually used. */ >> - >> -#ifdef SIGCANCEL >> -# define SIGCANCEL_CONSUMES 1 >> -#else >> -# define SIGCANCEL_CONSUMES 0 >> -#endif >> - >> -#if defined SIGTIMER && (!defined SIGCANCEL || SIGTIMER != SIGCANCEL) >> -# define SIGTIMER_CONSUMES 1 >> -#else >> -# define SIGTIMER_CONSUMES 0 >> -#endif >> - >> -#if (defined SIGSETXID \ >> - && (!defined SIGCANCEL || SIGSETXID != SIGCANCEL) \ >> - && (!defined SIGTIMER || SIGSETXID != SIGTIMER)) >> -# define SIGSETXID_CONSUMES 1 >> -#else >> -# define SIGSETXID_CONSUMES 0 >> +#if SIGTIMER != SIGCANCEL >> +# error "SIGTIMER and SIGCANCEL must be the same" >> #endif > > Not sure if it is really required them to be the same. It is required for the constant 2 here: >> /* This tells the generic code (included below) how many signal >> + numbers need to be reserved for libpthread's private uses >> + (SIGCANCEL and SIGSETXID). */ >> +#define RESERVED_SIGRT 2 >> >> #include <signal/allocrtsig.c> Thanks, Florian
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 64a9ae6066..086efb75ec 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -963,7 +963,6 @@ __reclaim_stacks (void) } -#ifdef SIGSETXID static void setxid_mark_thread (struct xid_command *cmdp, struct pthread *t) { @@ -1174,8 +1173,6 @@ __nptl_setxid (struct xid_command *cmdp) lll_unlock (stack_cache_lock, LLL_PRIVATE); return result; } -#endif /* SIGSETXID. */ - static inline void __attribute__((always_inline)) init_one_static_tls (struct pthread *curp, struct link_map *map) diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index ea91b9e138..acc0f3672b 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -114,9 +114,7 @@ static const struct pthread_functions pthread_functions = .ptr_nthreads = &__nptl_nthreads, .ptr___pthread_unwind = &__pthread_unwind, .ptr__nptl_deallocate_tsd = __nptl_deallocate_tsd, -# ifdef SIGSETXID .ptr__nptl_setxid = __nptl_setxid, -# endif .ptr_set_robust = __nptl_set_robust }; # define ptr_pthread_functions &pthread_functions @@ -139,7 +137,6 @@ __nptl_set_robust (struct pthread *self) } -#ifdef SIGCANCEL /* For asynchronous cancellation we use a signal. This is the handler. */ static void sigcancel_handler (int sig, siginfo_t *si, void *ctx) @@ -185,10 +182,8 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) oldval = curval; } } -#endif -#ifdef SIGSETXID struct xid_command *__xidcmd attribute_hidden; /* We use the SIGSETXID signal in the setuid, setgid, etc. implementations to @@ -234,7 +229,6 @@ sighandler_setxid (int sig, siginfo_t *si, void *ctx) if (atomic_decrement_val (&__xidcmd->cntr) == 0) futex_wake ((unsigned int *) &__xidcmd->cntr, 1, FUTEX_PRIVATE); } -#endif /* When using __thread for this, we do it in libc so as not @@ -285,41 +279,31 @@ __pthread_initialize_minimal_internal (void) had to set __nptl_initial_report_events. Propagate its setting. */ THREAD_SETMEM (pd, report_events, __nptl_initial_report_events); -#if defined SIGCANCEL || defined SIGSETXID struct sigaction sa; __sigemptyset (&sa.sa_mask); -# ifdef SIGCANCEL /* Install the cancellation signal handler. If for some reason we cannot install the handler we do not abort. Maybe we should, but it is only asynchronous cancellation which is affected. */ sa.sa_sigaction = sigcancel_handler; sa.sa_flags = SA_SIGINFO; (void) __libc_sigaction (SIGCANCEL, &sa, NULL); -# endif -# ifdef SIGSETXID /* Install the handle to change the threads' uid/gid. */ sa.sa_sigaction = sighandler_setxid; sa.sa_flags = SA_SIGINFO | SA_RESTART; (void) __libc_sigaction (SIGSETXID, &sa, NULL); -# endif /* The parent process might have left the signals blocked. Just in case, unblock it. We reuse the signal mask in the sigaction structure. It is already cleared. */ -# ifdef SIGCANCEL __sigaddset (&sa.sa_mask, SIGCANCEL); -# endif -# ifdef SIGSETXID __sigaddset (&sa.sa_mask, SIGSETXID); -# endif { INTERNAL_SYSCALL_DECL (err); (void) INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_UNBLOCK, &sa.sa_mask, NULL, _NSIG / 8); } -#endif /* Get the size of the static and alignment requirements for the TLS block. */ diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 64ac12e60a..f6be8c1dd6 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -63,7 +63,6 @@ __pthread_cancel (pthread_t th) oldval)) goto again; -#ifdef SIGCANCEL /* The cancellation handler will take care of marking the thread as canceled. */ pid_t pid = __getpid (); @@ -73,12 +72,6 @@ __pthread_cancel (pthread_t th) SIGCANCEL); if (INTERNAL_SYSCALL_ERROR_P (val, err)) result = INTERNAL_SYSCALL_ERRNO (val, err); -#else - /* It should be impossible to get here at all, since - pthread_setcanceltype should never have allowed - PTHREAD_CANCEL_ASYNCHRONOUS to be set. */ - abort (); -#endif break; } diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 130937c3c4..5682c9c2c0 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -402,7 +402,6 @@ START_THREAD_DEFN } #endif -#ifdef SIGCANCEL /* If the parent was running cancellation handlers while creating the thread the new thread inherited the signal mask. Reset the cancellation signal mask. */ @@ -415,7 +414,6 @@ START_THREAD_DEFN (void) INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_UNBLOCK, &mask, NULL, _NSIG / 8); } -#endif /* This is where the try/finally block should be created. For compilers without that support we do use setjmp. */ diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c index d771c31e46..99bc13e993 100644 --- a/nptl/pthread_setcanceltype.c +++ b/nptl/pthread_setcanceltype.c @@ -27,11 +27,6 @@ __pthread_setcanceltype (int type, int *oldtype) if (type < PTHREAD_CANCEL_DEFERRED || type > PTHREAD_CANCEL_ASYNCHRONOUS) return EINVAL; -#ifndef SIGCANCEL - if (type == PTHREAD_CANCEL_ASYNCHRONOUS) - return ENOTSUP; -#endif - volatile struct pthread *self = THREAD_SELF; int oldval = THREAD_GETMEM (self, cancelhandling); diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c index c8df527995..4aa774d01d 100644 --- a/nptl/pthread_sigmask.c +++ b/nptl/pthread_sigmask.c @@ -1,6 +1,6 @@ -/* Examine and change blocked signals for a thread. Generic POSIX version. - Copyright (C) 2014-2019 Free Software Foundation, Inc. +/* Copyright (C) 2002-2019 Free Software Foundation, Inc. This file is part of the GNU C Library. + Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. The GNU C Library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public @@ -19,18 +19,36 @@ #include <errno.h> #include <signal.h> #include <pthreadP.h> +#include <sysdep.h> -#if defined SIGCANCEL || defined SIGTIMER || defined SIGSETXID -# error "This implementation assumes no internal-only signal numbers." -#endif int pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask) { - /* Here we assume that sigprocmask actually does everything right. - The only difference is the return value protocol. */ - int result = sigprocmask (how, newmask, oldmask); - if (result < 0) - result = errno; - return result; + sigset_t local_newmask; + + /* The only thing we have to make sure here is that SIGCANCEL and + SIGSETXID is not blocked. */ + if (newmask != NULL + && (__builtin_expect (__sigismember (newmask, SIGCANCEL), 0) + || __builtin_expect (__sigismember (newmask, SIGSETXID), 0))) + { + local_newmask = *newmask; + __sigdelset (&local_newmask, SIGCANCEL); + __sigdelset (&local_newmask, SIGSETXID); + newmask = &local_newmask; + } + +#ifdef INTERNAL_SYSCALL + /* We know that realtime signals are available if NPTL is used. */ + INTERNAL_SYSCALL_DECL (err); + int result = INTERNAL_SYSCALL (rt_sigprocmask, err, 4, how, newmask, + oldmask, _NSIG / 8); + + return (INTERNAL_SYSCALL_ERROR_P (result, err) + ? INTERNAL_SYSCALL_ERRNO (result, err) + : 0); +#else + return sigprocmask (how, newmask, oldmask) == -1 ? errno : 0; +#endif } diff --git a/nptl/tst-cancel25.c b/nptl/tst-cancel25.c index 24ddd3c01c..c6e1c23c02 100644 --- a/nptl/tst-cancel25.c +++ b/nptl/tst-cancel25.c @@ -12,7 +12,6 @@ static pthread_t th2; static void * tf2 (void *arg) { -#ifdef SIGCANCEL sigset_t mask; if (pthread_sigmask (SIG_SETMASK, NULL, &mask) != 0) { @@ -24,7 +23,6 @@ tf2 (void *arg) puts ("SIGCANCEL blocked in new thread"); exit (1); } -#endif /* Sync with the main thread so that we do not test anything else. */ int e = pthread_barrier_wait (&b); diff --git a/nptl/tst-signal7.c b/nptl/tst-signal7.c index de67ab493c..f42ff6887a 100644 --- a/nptl/tst-signal7.c +++ b/nptl/tst-signal7.c @@ -27,7 +27,6 @@ do_test (void) { int result = 0; -#ifdef SIGCANCEL errno = 0; if (sigaction (SIGCANCEL, NULL, NULL) == 0) { @@ -39,9 +38,7 @@ do_test (void) puts ("sigaction(SIGCANCEL) did not set errno to EINVAL"); result = 1; } -#endif -#ifdef SIGSETXID errno = 0; if (sigaction (SIGSETXID, NULL, NULL) == 0) { @@ -53,7 +50,6 @@ do_test (void) puts ("sigaction(SIGSETXID) did not set errno to EINVAL"); result = 1; } -#endif return result; } diff --git a/sysdeps/nptl/allocrtsig.c b/sysdeps/nptl/allocrtsig.c index e9ea038655..3f62bf40e7 100644 --- a/sysdeps/nptl/allocrtsig.c +++ b/sysdeps/nptl/allocrtsig.c @@ -19,32 +19,13 @@ #include <signal.h> #include <nptl/pthreadP.h> -/* Up to three special signals might be used privately by libpthread. - Figure out how many unique ones are actually used. */ - -#ifdef SIGCANCEL -# define SIGCANCEL_CONSUMES 1 -#else -# define SIGCANCEL_CONSUMES 0 -#endif - -#if defined SIGTIMER && (!defined SIGCANCEL || SIGTIMER != SIGCANCEL) -# define SIGTIMER_CONSUMES 1 -#else -# define SIGTIMER_CONSUMES 0 -#endif - -#if (defined SIGSETXID \ - && (!defined SIGCANCEL || SIGSETXID != SIGCANCEL) \ - && (!defined SIGTIMER || SIGSETXID != SIGTIMER)) -# define SIGSETXID_CONSUMES 1 -#else -# define SIGSETXID_CONSUMES 0 +#if SIGTIMER != SIGCANCEL +# error "SIGTIMER and SIGCANCEL must be the same" #endif /* This tells the generic code (included below) how many signal - numbers need to be reserved for libpthread's private uses. */ -#define RESERVED_SIGRT \ - (SIGCANCEL_CONSUMES + SIGTIMER_CONSUMES + SIGSETXID_CONSUMES) + numbers need to be reserved for libpthread's private uses + (SIGCANCEL and SIGSETXID). */ +#define RESERVED_SIGRT 2 #include <signal/allocrtsig.c> diff --git a/sysdeps/unix/sysv/linux/pthread_sigmask.c b/sysdeps/unix/sysv/linux/pthread_sigmask.c deleted file mode 100644 index 4aa774d01d..0000000000 --- a/sysdeps/unix/sysv/linux/pthread_sigmask.c +++ /dev/null @@ -1,54 +0,0 @@ -/* Copyright (C) 2002-2019 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <https://www.gnu.org/licenses/>. */ - -#include <errno.h> -#include <signal.h> -#include <pthreadP.h> -#include <sysdep.h> - - -int -pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask) -{ - sigset_t local_newmask; - - /* The only thing we have to make sure here is that SIGCANCEL and - SIGSETXID is not blocked. */ - if (newmask != NULL - && (__builtin_expect (__sigismember (newmask, SIGCANCEL), 0) - || __builtin_expect (__sigismember (newmask, SIGSETXID), 0))) - { - local_newmask = *newmask; - __sigdelset (&local_newmask, SIGCANCEL); - __sigdelset (&local_newmask, SIGSETXID); - newmask = &local_newmask; - } - -#ifdef INTERNAL_SYSCALL - /* We know that realtime signals are available if NPTL is used. */ - INTERNAL_SYSCALL_DECL (err); - int result = INTERNAL_SYSCALL (rt_sigprocmask, err, 4, how, newmask, - oldmask, _NSIG / 8); - - return (INTERNAL_SYSCALL_ERROR_P (result, err) - ? INTERNAL_SYSCALL_ERRNO (result, err) - : 0); -#else - return sigprocmask (how, newmask, oldmask) == -1 ? errno : 0; -#endif -}