diff mbox series

[4/4] signal: Only handle on NSIG signals on signal functions (BZ #25657)

Message ID 20200313194827.4467-4-adhemerval.zanella@linaro.org
State New
Headers show
Series [v2,1/4] nptl: Move pthread_sigmask implementation to libc | expand

Commit Message

develop--- via Libc-alpha March 13, 2020, 7:48 p.m. UTC 
The upper bits of the sigset_t s not fully initialized in the signal
mask calls that return information from kernel (sigprocmask,
sigpending, and pthread_sigmask), since the exported sigset_t size
(1024 bits) is larger than Linux support one (64 or 128 bits).
It might make sigisemptyset/sigorset/sigandset fail if the mask
is filled prior the call.

This patch changes the internal signal function to handle up to
supported Linux signal number (_NSIG), the remaining bits are
untouched.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/Makefile                        |   2 +-
 nptl/pthread_sigmask.c               |   7 +-
 nptl/tst-signal8.c                   |  62 ++++++++++++
 signal/Makefile                      |   1 +
 signal/sigsetops.c                   |  12 +--
 signal/tst-sigisemptyset.c           |  95 ++++++++++++++++++
 sysdeps/unix/sysv/linux/sigpending.c |   6 +-
 sysdeps/unix/sysv/linux/sigsetops.h  | 143 +++++++++++++--------------
 8 files changed, 236 insertions(+), 92 deletions(-)
 create mode 100644 nptl/tst-signal8.c
 create mode 100644 signal/tst-sigisemptyset.c

Comments

Adhemerval Zanella Netto April 17, 2020, 1:25 p.m. UTC | #1 
Ping.

On 13/03/2020 16:48, Adhemerval Zanella wrote:
> The upper bits of the sigset_t s not fully initialized in the signal
> mask calls that return information from kernel (sigprocmask,
> sigpending, and pthread_sigmask), since the exported sigset_t size
> (1024 bits) is larger than Linux support one (64 or 128 bits).
> It might make sigisemptyset/sigorset/sigandset fail if the mask
> is filled prior the call.
> 
> This patch changes the internal signal function to handle up to
> supported Linux signal number (_NSIG), the remaining bits are
> untouched.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/Makefile                        |   2 +-
>  nptl/pthread_sigmask.c               |   7 +-
>  nptl/tst-signal8.c                   |  62 ++++++++++++
>  signal/Makefile                      |   1 +
>  signal/sigsetops.c                   |  12 +--
>  signal/tst-sigisemptyset.c           |  95 ++++++++++++++++++
>  sysdeps/unix/sysv/linux/sigpending.c |   6 +-
>  sysdeps/unix/sysv/linux/sigsetops.h  | 143 +++++++++++++--------------
>  8 files changed, 236 insertions(+), 92 deletions(-)
>  create mode 100644 nptl/tst-signal8.c
>  create mode 100644 signal/tst-sigisemptyset.c
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 4816fa254e..33fa03807e 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -293,7 +293,7 @@ tests = tst-attr2 tst-attr3 tst-default-attr \
>  	tst-cleanup0 tst-cleanup1 tst-cleanup2 tst-cleanup3 tst-cleanup4 \
>  	tst-flock1 tst-flock2 \
>  	tst-signal1 tst-signal2 tst-signal3 tst-signal4 tst-signal5 \
> -	tst-signal6 \
> +	tst-signal6 tst-signal8 \
>  	tst-exec1 tst-exec2 tst-exec3 tst-exec4 tst-exec5 \
>  	tst-exit1 tst-exit2 tst-exit3 \
>  	tst-stdio1 tst-stdio2 \
> diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c
> index c6c6e83c08..d266d296c5 100644
> --- a/nptl/pthread_sigmask.c
> +++ b/nptl/pthread_sigmask.c
> @@ -29,12 +29,11 @@ __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
>    /* 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)))
> +      && (__glibc_unlikely (__sigismember (newmask, SIGCANCEL))
> +         || __glibc_unlikely (__sigismember (newmask, SIGSETXID))))
>      {
>        local_newmask = *newmask;
> -      __sigdelset (&local_newmask, SIGCANCEL);
> -      __sigdelset (&local_newmask, SIGSETXID);
> +      __clear_internal_signals (&local_newmask);
>        newmask = &local_newmask;
>      }
>  
> diff --git a/nptl/tst-signal8.c b/nptl/tst-signal8.c
> new file mode 100644
> index 0000000000..9da7e5ef07
> --- /dev/null
> +++ b/nptl/tst-signal8.c
> @@ -0,0 +1,62 @@
> +/* Tests for sigisemptyset and pthread_sigmask.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   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 <signal.h>
> +
> +#include <support/check.h>
> +#include <support/xthread.h>
> +
> +static void *
> +tf (void *arg)
> +{
> +  {
> +    sigset_t set;
> +    sigemptyset (&set);
> +    TEST_COMPARE (pthread_sigmask (SIG_BLOCK, 0, &set), 0);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  {
> +    sigset_t set;
> +    sigfillset (&set);
> +    TEST_COMPARE (pthread_sigmask (SIG_BLOCK, 0, &set), 0);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  return NULL;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  /* Ensure current SIG_BLOCK mask empty.  */
> +  {
> +    sigset_t set;
> +    sigemptyset (&set);
> +    TEST_COMPARE (sigprocmask (SIG_BLOCK, &set, 0), 0);
> +  }
> +
> +  {
> +    pthread_t thr = xpthread_create (NULL, tf, NULL);
> +    xpthread_join (thr);
> +  }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/signal/Makefile b/signal/Makefile
> index 37de438bba..f3c19e2992 100644
> --- a/signal/Makefile
> +++ b/signal/Makefile
> @@ -49,6 +49,7 @@ tests		:= tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 \
>  		   tst-sigwait-eintr tst-sigaction \
>  		   tst-minsigstksz-1 tst-minsigstksz-2 tst-minsigstksz-3 \
>  		   tst-minsigstksz-3a tst-minsigstksz-4 \
> +		   tst-sigisemptyset
>  
>  include ../Rules
>  
> diff --git a/signal/sigsetops.c b/signal/sigsetops.c
> index eb89e6780e..1165377876 100644
> --- a/signal/sigsetops.c
> +++ b/signal/sigsetops.c
> @@ -26,28 +26,28 @@
>  
>  int
>  attribute_compat_text_section
> -(__sigismember) (const __sigset_t *set, int sig)
> +__sigismember_compat (const __sigset_t *set, int sig)
>  {
>    return __sigismember (set, sig);
>  }
> -compat_symbol (libc, __sigismember, __sigismember, GLIBC_2_0);
> +compat_symbol (libc, __sigismember_compat, __sigismember, GLIBC_2_0);
>  
>  int
>  attribute_compat_text_section
> -(__sigaddset) (__sigset_t *set, int sig)
> +__sigaddset_compat (__sigset_t *set, int sig)
>  {
>    __sigaddset (set, sig);
>    return 0;
>  }
> -compat_symbol (libc, __sigaddset, __sigaddset, GLIBC_2_0);
> +compat_symbol (libc, __sigaddset_compat, __sigaddset, GLIBC_2_0);
>  
>  int
>  attribute_compat_text_section
> -(__sigdelset) (__sigset_t *set, int sig)
> +__sigdelset_compat (__sigset_t *set, int sig)
>  {
>    __sigdelset (set, sig);
>    return 0;
>  }
> -compat_symbol (libc, __sigdelset, __sigdelset, GLIBC_2_0);
> +compat_symbol (libc, __sigdelset_compat, __sigdelset, GLIBC_2_0);
>  
>  #endif
> diff --git a/signal/tst-sigisemptyset.c b/signal/tst-sigisemptyset.c
> new file mode 100644
> index 0000000000..9ed95496f2
> --- /dev/null
> +++ b/signal/tst-sigisemptyset.c
> @@ -0,0 +1,95 @@
> +/* Tests for sigisemptyset/sigorset/sigandset.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   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 <signal.h>
> +
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> +  {
> +    sigset_t set;
> +    sigemptyset (&set);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  {
> +    sigset_t set;
> +    sigfillset (&set);
> +    TEST_COMPARE (sigisemptyset (&set), 0);
> +  }
> +
> +  {
> +    sigset_t setfill, setempty, set;
> +    sigfillset (&setfill);
> +    sigemptyset (&setempty);
> +
> +    sigorset (&set, &setfill, &setempty);
> +    TEST_COMPARE (sigisemptyset (&set), 0);
> +
> +    sigandset (&set, &setfill, &setempty);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  /* Ensure current SIG_BLOCK mask empty.  */
> +  {
> +    sigset_t set;
> +    sigemptyset (&set);
> +    TEST_COMPARE (sigprocmask (SIG_BLOCK, &set, 0), 0);
> +  }
> +
> +  {
> +    sigset_t set;
> +    sigemptyset (&set);
> +    TEST_COMPARE (sigprocmask (SIG_BLOCK, 0, &set), 0);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  {
> +    sigset_t set;
> +    sigfillset (&set);
> +    TEST_COMPARE (sigprocmask (SIG_BLOCK, 0, &set), 0);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  /* Block all signals.  */
> +  {
> +    sigset_t set;
> +    sigfillset (&set);
> +    TEST_COMPARE (sigprocmask (SIG_BLOCK, &set, 0), 0);
> +  }
> +
> +  {
> +    sigset_t set;
> +    sigemptyset (&set);
> +    TEST_COMPARE (sigpending (&set), 0);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  {
> +    sigset_t set;
> +    sigfillset (&set);
> +    TEST_COMPARE (sigpending (&set), 0);
> +    TEST_COMPARE (sigisemptyset (&set), 1);
> +  }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/unix/sysv/linux/sigpending.c b/sysdeps/unix/sysv/linux/sigpending.c
> index f6bedb5182..458a3cf99e 100644
> --- a/sysdeps/unix/sysv/linux/sigpending.c
> +++ b/sysdeps/unix/sysv/linux/sigpending.c
> @@ -15,13 +15,9 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <errno.h>
>  #include <signal.h>
> -#include <unistd.h>
> -
>  #include <sysdep.h>
> -#include <sys/syscall.h>
> -
> +#include <sigsetops.h>
>  
>  /* Change the set of blocked signals to SET,
>     wait until a signal arrives, and restore the set of blocked signals.  */
> diff --git a/sysdeps/unix/sysv/linux/sigsetops.h b/sysdeps/unix/sysv/linux/sigsetops.h
> index cb97c98b10..f6ca34c842 100644
> --- a/sysdeps/unix/sysv/linux/sigsetops.h
> +++ b/sysdeps/unix/sysv/linux/sigsetops.h
> @@ -28,81 +28,72 @@
>  /* Return the word index for SIG.  */
>  # define __sigword(sig) (((sig) - 1) / (8 * sizeof (unsigned long int)))
>  
> -# define __sigemptyset(set)					\
> -  (__extension__ ({						\
> -    int __cnt = _SIGSET_NWORDS;					\
> -    sigset_t *__set = (set);					\
> -    while (--__cnt >= 0)					\
> -      __set->__val[__cnt] = 0;					\
> -    (void)0;							\
> -  }))
> -
> -# define __sigfillset(set)					\
> -  (__extension__ ({						\
> -    int __cnt = _SIGSET_NWORDS;					\
> -    sigset_t *__set = (set);					\
> -    while (--__cnt >= 0)					\
> -      __set->__val[__cnt] = ~0UL;				\
> -    (void)0;							\
> -  }))
> -
> -# define __sigisemptyset(set)					\
> -  (__extension__ ({						\
> -    int __cnt = _SIGSET_NWORDS;					\
> -    const sigset_t *__set = (set);				\
> -    int __ret = __set->__val[--__cnt];				\
> -    while (!__ret && --__cnt >= 0)				\
> -      __ret = __set->__val[__cnt];				\
> -    __ret == 0;							\
> -  }))
> -
> -# define __sigandset(dest, left, right)				\
> -  (__extension__ ({						\
> -    int __cnt = _SIGSET_NWORDS;					\
> -    sigset_t *__dest = (dest);					\
> -    const sigset_t *__left = (left);				\
> -    const sigset_t *__right = (right);				\
> -    while (--__cnt >= 0)					\
> -      __dest->__val[__cnt] = (__left->__val[__cnt]		\
> -			      & __right->__val[__cnt]);		\
> -    (void)0;							\
> -  }))
> -
> -# define __sigorset(dest, left, right)				\
> -  (__extension__ ({						\
> -    int __cnt = _SIGSET_NWORDS;					\
> -    sigset_t *__dest = (dest);					\
> -    const sigset_t *__left = (left);				\
> -    const sigset_t *__right = (right);				\
> -    while (--__cnt >= 0)					\
> -      __dest->__val[__cnt] = (__left->__val[__cnt]		\
> -			      | __right->__val[__cnt]);		\
> -    (void)0;							\
> -  }))
> -
> -/* These macros needn't check for a bogus signal number;
> -   error checking is done in the non-__ versions.  */
> -# define __sigismember(set, sig)				\
> -  (__extension__ ({						\
> -    unsigned long int __mask = __sigmask (sig);			\
> -    unsigned long int __word = __sigword (sig);			\
> -    (set)->__val[__word] & __mask ? 1 : 0;			\
> -  }))
> -
> -# define __sigaddset(set, sig)					\
> -  (__extension__ ({						\
> -    unsigned long int __mask = __sigmask (sig);			\
> -    unsigned long int __word = __sigword (sig);			\
> -    (set)->__val[__word] |= __mask;				\
> -    (void)0;							\
> -  }))
> -
> -# define __sigdelset(set, sig)					\
> -  (__extension__ ({						\
> -    unsigned long int __mask = __sigmask (sig);			\
> -    unsigned long int __word = __sigword (sig);			\
> -    (set)->__val[__word] &= ~__mask;				\
> -    (void)0;							\
> - }))
> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
> +
> +static inline void
> +__sigemptyset (sigset_t *set)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +   set->__val[cnt] = 0;
> +}
> +
> +static inline void
> +__sigfillset (sigset_t *set)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +   set->__val[cnt] = ~0UL;
> +}
> +
> +static inline int
> +__sigisemptyset (const sigset_t *set)
> +{
> +  int cnt = __NSIG_WORDS;
> +  int ret = set->__val[--cnt];
> +  while (ret == 0 && --cnt >= 0)
> +    ret = set->__val[cnt];
> +  return ret == 0;
> +}
> +
> +static inline void
> +__sigandset (sigset_t *dest, const sigset_t *left, const sigset_t *right)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +    dest->__val[cnt] = left->__val[cnt] & right->__val[cnt];
> +}
> +
> +static inline void
> +__sigorset (sigset_t *dest, const sigset_t *left, const sigset_t *right)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +    dest->__val[cnt] = left->__val[cnt] | right->__val[cnt];
> +}
> +
> +static inline int
> +__sigismember (const sigset_t *set, int sig)
> +{
> +  unsigned long int mask = __sigmask (sig);
> +  unsigned long int word = __sigword (sig);
> +  return set->__val[word] & mask ? 1 : 0;
> +}
> +
> +static inline void
> +__sigaddset (sigset_t *set, int sig)
> +{
> +  unsigned long int mask = __sigmask (sig);
> +  unsigned long int word = __sigword (sig);
> +  set->__val[word] |= mask;
> +}
> +
> +static inline void
> +__sigdelset (sigset_t *set, int sig)
> +{
> +  unsigned long int mask = __sigmask (sig);
> +  unsigned long int word = __sigword (sig);
> +  set->__val[word] &= ~mask;
> +}
>  
>  #endif /* bits/sigsetops.h */
>
Florian Weimer April 21, 2020, 12:53 p.m. UTC | #2 
* Adhemerval Zanella via Libc-alpha:

> The upper bits of the sigset_t s not fully initialized in the signal
> mask calls that return information from kernel (sigprocmask,
> sigpending, and pthread_sigmask), since the exported sigset_t size
> (1024 bits) is larger than Linux support one (64 or 128 bits).
> It might make sigisemptyset/sigorset/sigandset fail if the mask
> is filled prior the call.
>
> This patch changes the internal signal function to handle up to
> supported Linux signal number (_NSIG), the remaining bits are
> untouched.

Sorry, I have trouble what the intended outcome of this change is.

What is the expected behavior here?  We have an overly large sigset_t,
but only signal numbers from 0 to NSIG - 1 (inclusive) are valid in
all function calls?
Adhemerval Zanella Netto April 21, 2020, 1:11 p.m. UTC | #3 
On 21/04/2020 09:53, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> The upper bits of the sigset_t s not fully initialized in the signal
>> mask calls that return information from kernel (sigprocmask,
>> sigpending, and pthread_sigmask), since the exported sigset_t size
>> (1024 bits) is larger than Linux support one (64 or 128 bits).
>> It might make sigisemptyset/sigorset/sigandset fail if the mask
>> is filled prior the call.
>>
>> This patch changes the internal signal function to handle up to
>> supported Linux signal number (_NSIG), the remaining bits are
>> untouched.
> 
> Sorry, I have trouble what the intended outcome of this change is.
> 
> What is the expected behavior here?  We have an overly large sigset_t,
> but only signal numbers from 0 to NSIG - 1 (inclusive) are valid in
> all function calls?
> 

The bug report link to a stackoverflow example, which simplified is:

  #define _GNU_SOURCE
  #include <stdio.h>
  #include <signal.h>
  int
  main (int argc, char *argv[])
  { 
    sigset_t set;
    sigfillset (&set);
    sigprocmask (SIG_BLOCK, NULL, &set);
    printf ("%d\n", sigisemptyset (&set));
   return 0;
  }

The main issue is GNU extensions sigisemptyset/sigorset/sigandset
operates on full internal signal bits where kernel only updates
up to _NSIG signals.
Florian Weimer April 21, 2020, 1:14 p.m. UTC | #4 
* Adhemerval Zanella:

> On 21/04/2020 09:53, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>> The upper bits of the sigset_t s not fully initialized in the signal
>>> mask calls that return information from kernel (sigprocmask,
>>> sigpending, and pthread_sigmask), since the exported sigset_t size
>>> (1024 bits) is larger than Linux support one (64 or 128 bits).
>>> It might make sigisemptyset/sigorset/sigandset fail if the mask
>>> is filled prior the call.
>>>
>>> This patch changes the internal signal function to handle up to
>>> supported Linux signal number (_NSIG), the remaining bits are
>>> untouched.
>> 
>> Sorry, I have trouble what the intended outcome of this change is.
>> 
>> What is the expected behavior here?  We have an overly large sigset_t,
>> but only signal numbers from 0 to NSIG - 1 (inclusive) are valid in
>> all function calls?
>> 
>
> The bug report link to a stackoverflow example, which simplified is:
>
>   #define _GNU_SOURCE
>   #include <stdio.h>
>   #include <signal.h>
>   int
>   main (int argc, char *argv[])
>   { 
>     sigset_t set;
>     sigfillset (&set);
>     sigprocmask (SIG_BLOCK, NULL, &set);
>     printf ("%d\n", sigisemptyset (&set));
>    return 0;
>   }
>
> The main issue is GNU extensions sigisemptyset/sigorset/sigandset
> operates on full internal signal bits where kernel only updates
> up to _NSIG signals.

I get that, but what do you want to do to fix it?

It's not entirely clear from the changes in the patch.
Adhemerval Zanella Netto April 21, 2020, 1:29 p.m. UTC | #5 
On 21/04/2020 10:14, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 21/04/2020 09:53, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> The upper bits of the sigset_t s not fully initialized in the signal
>>>> mask calls that return information from kernel (sigprocmask,
>>>> sigpending, and pthread_sigmask), since the exported sigset_t size
>>>> (1024 bits) is larger than Linux support one (64 or 128 bits).
>>>> It might make sigisemptyset/sigorset/sigandset fail if the mask
>>>> is filled prior the call.
>>>>
>>>> This patch changes the internal signal function to handle up to
>>>> supported Linux signal number (_NSIG), the remaining bits are
>>>> untouched.
>>>
>>> Sorry, I have trouble what the intended outcome of this change is.
>>>
>>> What is the expected behavior here?  We have an overly large sigset_t,
>>> but only signal numbers from 0 to NSIG - 1 (inclusive) are valid in
>>> all function calls?
>>>
>>
>> The bug report link to a stackoverflow example, which simplified is:
>>
>>   #define _GNU_SOURCE
>>   #include <stdio.h>
>>   #include <signal.h>
>>   int
>>   main (int argc, char *argv[])
>>   { 
>>     sigset_t set;
>>     sigfillset (&set);
>>     sigprocmask (SIG_BLOCK, NULL, &set);
>>     printf ("%d\n", sigisemptyset (&set));
>>    return 0;
>>   }
>>
>> The main issue is GNU extensions sigisemptyset/sigorset/sigandset
>> operates on full internal signal bits where kernel only updates
>> up to _NSIG signals.
> 
> I get that, but what do you want to do to fix it?
> 
> It's not entirely clear from the changes in the patch.
> 

Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
when manipulating sigset_t masks.
Florian Weimer April 21, 2020, 1:47 p.m. UTC | #6 
* Adhemerval Zanella:

> On 21/04/2020 10:14, Florian Weimer wrote:
> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
> when manipulating sigset_t masks.

Hmm.  __NSIG_WORDS is defined like this:

> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))

This gives us two words for _NSIG == 65.  Is this correct?
Adhemerval Zanella Netto April 21, 2020, 2:02 p.m. UTC | #7 
On 21/04/2020 10:47, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 21/04/2020 10:14, Florian Weimer wrote:
>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>> when manipulating sigset_t masks.
> 
> Hmm.  __NSIG_WORDS is defined like this:
> 
>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
> 
> This gives us two words for _NSIG == 65.  Is this correct?
> 

Two words (32-bits) for all 64-bit architectures, except mips64 that
defined _NSIG as 128 as requires 4 words. And 4 words for all 32-bit 
architectures and 8 for mips32.
Florian Weimer April 21, 2020, 2:07 p.m. UTC | #8 
* Adhemerval Zanella:

> On 21/04/2020 10:47, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 21/04/2020 10:14, Florian Weimer wrote:
>>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>>> when manipulating sigset_t masks.
>> 
>> Hmm.  __NSIG_WORDS is defined like this:
>> 
>>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
>> 
>> This gives us two words for _NSIG == 65.  Is this correct?
>
> Two words (32-bits) for all 64-bit architectures,

sizeof (unsigned long int) is 8 on 64-bit.  65 / (8 * 8) is 1.

I assume that for _NSIG == 65, the signal numbers from 0 to 64 are all
valid and can be manipulated using sigaddset etc.  This suggests to me
that we need three 32-bit longs and two 64-bit longs.

Sorry, maybe I'm confused, I'm not feeling too well this week.
Adhemerval Zanella Netto April 21, 2020, 2:10 p.m. UTC | #9 
On 21/04/2020 11:07, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 21/04/2020 10:47, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> On 21/04/2020 10:14, Florian Weimer wrote:
>>>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>>>> when manipulating sigset_t masks.
>>>
>>> Hmm.  __NSIG_WORDS is defined like this:
>>>
>>>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
>>>
>>> This gives us two words for _NSIG == 65.  Is this correct?
>>
>> Two words (32-bits) for all 64-bit architectures,
> 
> sizeof (unsigned long int) is 8 on 64-bit.  65 / (8 * 8) is 1.
> 
> I assume that for _NSIG == 65, the signal numbers from 0 to 64 are all
> valid and can be manipulated using sigaddset etc.  This suggests to me
> that we need three 32-bit longs and two 64-bit longs.

Indeed my mistake here, I was reading the value as the required words
the maximum accessed index.

> 
> Sorry, maybe I'm confused, I'm not feeling too well this week.
> 

I hope you get better soon.
Florian Weimer April 21, 2020, 2:35 p.m. UTC | #10 
* Adhemerval Zanella:

> On 21/04/2020 11:07, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 21/04/2020 10:47, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> On 21/04/2020 10:14, Florian Weimer wrote:
>>>>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>>>>> when manipulating sigset_t masks.
>>>>
>>>> Hmm.  __NSIG_WORDS is defined like this:
>>>>
>>>>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
>>>>
>>>> This gives us two words for _NSIG == 65.  Is this correct?
>>>
>>> Two words (32-bits) for all 64-bit architectures,
>> 
>> sizeof (unsigned long int) is 8 on 64-bit.  65 / (8 * 8) is 1.
>> 
>> I assume that for _NSIG == 65, the signal numbers from 0 to 64 are all
>> valid and can be manipulated using sigaddset etc.  This suggests to me
>> that we need three 32-bit longs and two 64-bit longs.
>
> Indeed my mistake here, I was reading the value as the required words
> the maximum accessed index.

Ahh.  Please put also a comment into sigsetops.h how this is supposed
to work, that is, which signal numbers are expected to be valid.
Adhemerval Zanella Netto April 21, 2020, 2:36 p.m. UTC | #11 
On 21/04/2020 11:35, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 21/04/2020 11:07, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> On 21/04/2020 10:47, Florian Weimer wrote:
>>>>> * Adhemerval Zanella:
>>>>>
>>>>>> On 21/04/2020 10:14, Florian Weimer wrote:
>>>>>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>>>>>> when manipulating sigset_t masks.
>>>>>
>>>>> Hmm.  __NSIG_WORDS is defined like this:
>>>>>
>>>>>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
>>>>>
>>>>> This gives us two words for _NSIG == 65.  Is this correct?
>>>>
>>>> Two words (32-bits) for all 64-bit architectures,
>>>
>>> sizeof (unsigned long int) is 8 on 64-bit.  65 / (8 * 8) is 1.
>>>
>>> I assume that for _NSIG == 65, the signal numbers from 0 to 64 are all
>>> valid and can be manipulated using sigaddset etc.  This suggests to me
>>> that we need three 32-bit longs and two 64-bit longs.
>>
>> Indeed my mistake here, I was reading the value as the required words
>> the maximum accessed index.
> 
> Ahh.  Please put also a comment into sigsetops.h how this is supposed
> to work, that is, which signal numbers are expected to be valid.
> 

Alright. I will do it.
Andreas Schwab April 21, 2020, 3:05 p.m. UTC | #12 
On Apr 21 2020, Adhemerval Zanella via Libc-alpha wrote:

> On 21/04/2020 11:07, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 21/04/2020 10:47, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> On 21/04/2020 10:14, Florian Weimer wrote:
>>>>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>>>>> when manipulating sigset_t masks.
>>>>
>>>> Hmm.  __NSIG_WORDS is defined like this:
>>>>
>>>>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
>>>>
>>>> This gives us two words for _NSIG == 65.  Is this correct?
>>>
>>> Two words (32-bits) for all 64-bit architectures,
>> 
>> sizeof (unsigned long int) is 8 on 64-bit.  65 / (8 * 8) is 1.
>> 
>> I assume that for _NSIG == 65, the signal numbers from 0 to 64 are all
>> valid and can be manipulated using sigaddset etc.  This suggests to me
>> that we need three 32-bit longs and two 64-bit longs.
>
> Indeed my mistake here, I was reading the value as the required words
> the maximum accessed index.

Note that signal numbers are 1-based, and bit 0 of a signal mask is for
signal 1.

Andreas.
Florian Weimer April 21, 2020, 5:28 p.m. UTC | #13 
* Andreas Schwab:

> On Apr 21 2020, Adhemerval Zanella via Libc-alpha wrote:
>
>> On 21/04/2020 11:07, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>> 
>>>> On 21/04/2020 10:47, Florian Weimer wrote:
>>>>> * Adhemerval Zanella:
>>>>>
>>>>>> On 21/04/2020 10:14, Florian Weimer wrote:
>>>>>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>>>>>> when manipulating sigset_t masks.
>>>>>
>>>>> Hmm.  __NSIG_WORDS is defined like this:
>>>>>
>>>>>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
>>>>>
>>>>> This gives us two words for _NSIG == 65.  Is this correct?
>>>>
>>>> Two words (32-bits) for all 64-bit architectures,
>>> 
>>> sizeof (unsigned long int) is 8 on 64-bit.  65 / (8 * 8) is 1.
>>> 
>>> I assume that for _NSIG == 65, the signal numbers from 0 to 64 are all
>>> valid and can be manipulated using sigaddset etc.  This suggests to me
>>> that we need three 32-bit longs and two 64-bit longs.
>>
>> Indeed my mistake here, I was reading the value as the required words
>> the maximum accessed index.
>
> Note that signal numbers are 1-based, and bit 0 of a signal mask is for
> signal 1.

Right, all the more reason to spell this out in a top-level comment in
the header file.
Adhemerval Zanella Netto April 21, 2020, 5:49 p.m. UTC | #14 
On 21/04/2020 14:28, Florian Weimer wrote:
> * Andreas Schwab:
> 
>> On Apr 21 2020, Adhemerval Zanella via Libc-alpha wrote:
>>
>>> On 21/04/2020 11:07, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> On 21/04/2020 10:47, Florian Weimer wrote:
>>>>>> * Adhemerval Zanella:
>>>>>>
>>>>>>> On 21/04/2020 10:14, Florian Weimer wrote:
>>>>>>> Only handle __NSIG_WORDS on linux sigsetopts.h instead of _SIGSET_NWORDS
>>>>>>> when manipulating sigset_t masks.
>>>>>>
>>>>>> Hmm.  __NSIG_WORDS is defined like this:
>>>>>>
>>>>>>> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
>>>>>>
>>>>>> This gives us two words for _NSIG == 65.  Is this correct?
>>>>>
>>>>> Two words (32-bits) for all 64-bit architectures,
>>>>
>>>> sizeof (unsigned long int) is 8 on 64-bit.  65 / (8 * 8) is 1.
>>>>
>>>> I assume that for _NSIG == 65, the signal numbers from 0 to 64 are all
>>>> valid and can be manipulated using sigaddset etc.  This suggests to me
>>>> that we need three 32-bit longs and two 64-bit longs.
>>>
>>> Indeed my mistake here, I was reading the value as the required words
>>> the maximum accessed index.
>>
>> Note that signal numbers are 1-based, and bit 0 of a signal mask is for
>> signal 1.
> 
> Right, all the more reason to spell this out in a top-level comment in
> the header file.
> 

I have added the following comment on linux sigsetopts.h:

  /* Linux sig* functions only handle up to __NSIG_WORDS words instead of
     full _SIGSET_NWORDS sigset size.  The signal numbers are 1-based, and
     bit 0 of a signal mask is for signal 1.  */
Andreas Schwab April 21, 2020, 6:40 p.m. UTC | #15 
On Apr 21 2020, Adhemerval Zanella wrote:

> I have added the following comment on linux sigsetopts.h:
>
>   /* Linux sig* functions only handle up to __NSIG_WORDS words instead of
>      full _SIGSET_NWORDS sigset size.  The signal numbers are 1-based, and
>      bit 0 of a signal mask is for signal 1.  */

You forgot to fix __NSIG_WORDS.

Andreas.
Adhemerval Zanella Netto April 21, 2020, 7 p.m. UTC | #16 
On 21/04/2020 15:40, Andreas Schwab wrote:
> On Apr 21 2020, Adhemerval Zanella wrote:
> 
>> I have added the following comment on linux sigsetopts.h:
>>
>>   /* Linux sig* functions only handle up to __NSIG_WORDS words instead of
>>      full _SIGSET_NWORDS sigset size.  The signal numbers are 1-based, and
>>      bit 0 of a signal mask is for signal 1.  */
> 
> You forgot to fix __NSIG_WORDS.

Hum what exactly? I missing to see what might be missing here. Is the
related to comment or the __NSIG_WORDS value itself?
Andreas Schwab April 21, 2020, 9:14 p.m. UTC | #17 
On Apr 21 2020, Adhemerval Zanella wrote:

> On 21/04/2020 15:40, Andreas Schwab wrote:
>> On Apr 21 2020, Adhemerval Zanella wrote:
>> 
>>> I have added the following comment on linux sigsetopts.h:
>>>
>>>   /* Linux sig* functions only handle up to __NSIG_WORDS words instead of
>>>      full _SIGSET_NWORDS sigset size.  The signal numbers are 1-based, and
>>>      bit 0 of a signal mask is for signal 1.  */
>> 
>> You forgot to fix __NSIG_WORDS.
>
> Hum what exactly? I missing to see what might be missing here. Is the
> related to comment or the __NSIG_WORDS value itself?

The problem really is that _NSIG is bogus, it is actually the number of
signals plus one.  That doesn't change the value of __NSIG_WORDS, but if
there would actually be 65 signals then __NSIG_WORDS would be too small.

Note also that mips has the wrong value for __SIGRTMAX (it should be
128).

Andreas.
Florian Weimer April 22, 2020, 8:32 a.m. UTC | #18 
* Andreas Schwab:

> On Apr 21 2020, Adhemerval Zanella wrote:
>
>> On 21/04/2020 15:40, Andreas Schwab wrote:
>>> On Apr 21 2020, Adhemerval Zanella wrote:
>>> 
>>>> I have added the following comment on linux sigsetopts.h:
>>>>
>>>>   /* Linux sig* functions only handle up to __NSIG_WORDS words instead of
>>>>      full _SIGSET_NWORDS sigset size.  The signal numbers are 1-based, and
>>>>      bit 0 of a signal mask is for signal 1.  */
>>> 
>>> You forgot to fix __NSIG_WORDS.
>>
>> Hum what exactly? I missing to see what might be missing here. Is the
>> related to comment or the __NSIG_WORDS value itself?
>
> The problem really is that _NSIG is bogus, it is actually the number of
> signals plus one.  That doesn't change the value of __NSIG_WORDS, but if
> there would actually be 65 signals then __NSIG_WORDS would be too small.
>
> Note also that mips has the wrong value for __SIGRTMAX (it should be
> 128).

Adhemerval, if you address this, please also fix these whitespace
issue:

+# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))

(Space before define (it's not nested) and after “unsigned long int”.)
Adhemerval Zanella Netto April 22, 2020, 11:34 a.m. UTC | #19 
> Il giorno 22 apr 2020, alle ore 05:32, Florian Weimer <fw@deneb.enyo.de> ha scritto:
> 
> * Andreas Schwab:
> 
>>> On Apr 21 2020, Adhemerval Zanella wrote:
>>> 
>>> On 21/04/2020 15:40, Andreas Schwab wrote:
>>>> On Apr 21 2020, Adhemerval Zanella wrote:
>>>> 
>>>>> I have added the following comment on linux sigsetopts.h:
>>>>> 
>>>>>  /* Linux sig* functions only handle up to __NSIG_WORDS words instead of
>>>>>     full _SIGSET_NWORDS sigset size.  The signal numbers are 1-based, and
>>>>>     bit 0 of a signal mask is for signal 1.  */
>>>> 
>>>> You forgot to fix __NSIG_WORDS.
>>> 
>>> Hum what exactly? I missing to see what might be missing here. Is the
>>> related to comment or the __NSIG_WORDS value itself?
>> 
>> The problem really is that _NSIG is bogus, it is actually the number of
>> signals plus one.  That doesn't change the value of __NSIG_WORDS, but if
>> there would actually be 65 signals then __NSIG_WORDS would be too small.
>> 
>> Note also that mips has the wrong value for __SIGRTMAX (it should be
>> 128).
> 
> Adhemerval, if you address this, please also fix these whitespace
> issue:
> 
> +# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
> 
> (Space before define (it's not nested) and after “unsigned long int”.)

I will check this out.
diff mbox series

Patch

diff --git a/nptl/Makefile b/nptl/Makefile
index 4816fa254e..33fa03807e 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -293,7 +293,7 @@  tests = tst-attr2 tst-attr3 tst-default-attr \
 	tst-cleanup0 tst-cleanup1 tst-cleanup2 tst-cleanup3 tst-cleanup4 \
 	tst-flock1 tst-flock2 \
 	tst-signal1 tst-signal2 tst-signal3 tst-signal4 tst-signal5 \
-	tst-signal6 \
+	tst-signal6 tst-signal8 \
 	tst-exec1 tst-exec2 tst-exec3 tst-exec4 tst-exec5 \
 	tst-exit1 tst-exit2 tst-exit3 \
 	tst-stdio1 tst-stdio2 \
diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c
index c6c6e83c08..d266d296c5 100644
--- a/nptl/pthread_sigmask.c
+++ b/nptl/pthread_sigmask.c
@@ -29,12 +29,11 @@  __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
   /* 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)))
+      && (__glibc_unlikely (__sigismember (newmask, SIGCANCEL))
+         || __glibc_unlikely (__sigismember (newmask, SIGSETXID))))
     {
       local_newmask = *newmask;
-      __sigdelset (&local_newmask, SIGCANCEL);
-      __sigdelset (&local_newmask, SIGSETXID);
+      __clear_internal_signals (&local_newmask);
       newmask = &local_newmask;
     }
 
diff --git a/nptl/tst-signal8.c b/nptl/tst-signal8.c
new file mode 100644
index 0000000000..9da7e5ef07
--- /dev/null
+++ b/nptl/tst-signal8.c
@@ -0,0 +1,62 @@ 
+/* Tests for sigisemptyset and pthread_sigmask.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   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 <signal.h>
+
+#include <support/check.h>
+#include <support/xthread.h>
+
+static void *
+tf (void *arg)
+{
+  {
+    sigset_t set;
+    sigemptyset (&set);
+    TEST_COMPARE (pthread_sigmask (SIG_BLOCK, 0, &set), 0);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  {
+    sigset_t set;
+    sigfillset (&set);
+    TEST_COMPARE (pthread_sigmask (SIG_BLOCK, 0, &set), 0);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  /* Ensure current SIG_BLOCK mask empty.  */
+  {
+    sigset_t set;
+    sigemptyset (&set);
+    TEST_COMPARE (sigprocmask (SIG_BLOCK, &set, 0), 0);
+  }
+
+  {
+    pthread_t thr = xpthread_create (NULL, tf, NULL);
+    xpthread_join (thr);
+  }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/signal/Makefile b/signal/Makefile
index 37de438bba..f3c19e2992 100644
--- a/signal/Makefile
+++ b/signal/Makefile
@@ -49,6 +49,7 @@  tests		:= tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 \
 		   tst-sigwait-eintr tst-sigaction \
 		   tst-minsigstksz-1 tst-minsigstksz-2 tst-minsigstksz-3 \
 		   tst-minsigstksz-3a tst-minsigstksz-4 \
+		   tst-sigisemptyset
 
 include ../Rules
 
diff --git a/signal/sigsetops.c b/signal/sigsetops.c
index eb89e6780e..1165377876 100644
--- a/signal/sigsetops.c
+++ b/signal/sigsetops.c
@@ -26,28 +26,28 @@ 
 
 int
 attribute_compat_text_section
-(__sigismember) (const __sigset_t *set, int sig)
+__sigismember_compat (const __sigset_t *set, int sig)
 {
   return __sigismember (set, sig);
 }
-compat_symbol (libc, __sigismember, __sigismember, GLIBC_2_0);
+compat_symbol (libc, __sigismember_compat, __sigismember, GLIBC_2_0);
 
 int
 attribute_compat_text_section
-(__sigaddset) (__sigset_t *set, int sig)
+__sigaddset_compat (__sigset_t *set, int sig)
 {
   __sigaddset (set, sig);
   return 0;
 }
-compat_symbol (libc, __sigaddset, __sigaddset, GLIBC_2_0);
+compat_symbol (libc, __sigaddset_compat, __sigaddset, GLIBC_2_0);
 
 int
 attribute_compat_text_section
-(__sigdelset) (__sigset_t *set, int sig)
+__sigdelset_compat (__sigset_t *set, int sig)
 {
   __sigdelset (set, sig);
   return 0;
 }
-compat_symbol (libc, __sigdelset, __sigdelset, GLIBC_2_0);
+compat_symbol (libc, __sigdelset_compat, __sigdelset, GLIBC_2_0);
 
 #endif
diff --git a/signal/tst-sigisemptyset.c b/signal/tst-sigisemptyset.c
new file mode 100644
index 0000000000..9ed95496f2
--- /dev/null
+++ b/signal/tst-sigisemptyset.c
@@ -0,0 +1,95 @@ 
+/* Tests for sigisemptyset/sigorset/sigandset.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   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 <signal.h>
+
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+  {
+    sigset_t set;
+    sigemptyset (&set);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  {
+    sigset_t set;
+    sigfillset (&set);
+    TEST_COMPARE (sigisemptyset (&set), 0);
+  }
+
+  {
+    sigset_t setfill, setempty, set;
+    sigfillset (&setfill);
+    sigemptyset (&setempty);
+
+    sigorset (&set, &setfill, &setempty);
+    TEST_COMPARE (sigisemptyset (&set), 0);
+
+    sigandset (&set, &setfill, &setempty);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  /* Ensure current SIG_BLOCK mask empty.  */
+  {
+    sigset_t set;
+    sigemptyset (&set);
+    TEST_COMPARE (sigprocmask (SIG_BLOCK, &set, 0), 0);
+  }
+
+  {
+    sigset_t set;
+    sigemptyset (&set);
+    TEST_COMPARE (sigprocmask (SIG_BLOCK, 0, &set), 0);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  {
+    sigset_t set;
+    sigfillset (&set);
+    TEST_COMPARE (sigprocmask (SIG_BLOCK, 0, &set), 0);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  /* Block all signals.  */
+  {
+    sigset_t set;
+    sigfillset (&set);
+    TEST_COMPARE (sigprocmask (SIG_BLOCK, &set, 0), 0);
+  }
+
+  {
+    sigset_t set;
+    sigemptyset (&set);
+    TEST_COMPARE (sigpending (&set), 0);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  {
+    sigset_t set;
+    sigfillset (&set);
+    TEST_COMPARE (sigpending (&set), 0);
+    TEST_COMPARE (sigisemptyset (&set), 1);
+  }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/sigpending.c b/sysdeps/unix/sysv/linux/sigpending.c
index f6bedb5182..458a3cf99e 100644
--- a/sysdeps/unix/sysv/linux/sigpending.c
+++ b/sysdeps/unix/sysv/linux/sigpending.c
@@ -15,13 +15,9 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
 #include <signal.h>
-#include <unistd.h>
-
 #include <sysdep.h>
-#include <sys/syscall.h>
-
+#include <sigsetops.h>
 
 /* Change the set of blocked signals to SET,
    wait until a signal arrives, and restore the set of blocked signals.  */
diff --git a/sysdeps/unix/sysv/linux/sigsetops.h b/sysdeps/unix/sysv/linux/sigsetops.h
index cb97c98b10..f6ca34c842 100644
--- a/sysdeps/unix/sysv/linux/sigsetops.h
+++ b/sysdeps/unix/sysv/linux/sigsetops.h
@@ -28,81 +28,72 @@ 
 /* Return the word index for SIG.  */
 # define __sigword(sig) (((sig) - 1) / (8 * sizeof (unsigned long int)))
 
-# define __sigemptyset(set)					\
-  (__extension__ ({						\
-    int __cnt = _SIGSET_NWORDS;					\
-    sigset_t *__set = (set);					\
-    while (--__cnt >= 0)					\
-      __set->__val[__cnt] = 0;					\
-    (void)0;							\
-  }))
-
-# define __sigfillset(set)					\
-  (__extension__ ({						\
-    int __cnt = _SIGSET_NWORDS;					\
-    sigset_t *__set = (set);					\
-    while (--__cnt >= 0)					\
-      __set->__val[__cnt] = ~0UL;				\
-    (void)0;							\
-  }))
-
-# define __sigisemptyset(set)					\
-  (__extension__ ({						\
-    int __cnt = _SIGSET_NWORDS;					\
-    const sigset_t *__set = (set);				\
-    int __ret = __set->__val[--__cnt];				\
-    while (!__ret && --__cnt >= 0)				\
-      __ret = __set->__val[__cnt];				\
-    __ret == 0;							\
-  }))
-
-# define __sigandset(dest, left, right)				\
-  (__extension__ ({						\
-    int __cnt = _SIGSET_NWORDS;					\
-    sigset_t *__dest = (dest);					\
-    const sigset_t *__left = (left);				\
-    const sigset_t *__right = (right);				\
-    while (--__cnt >= 0)					\
-      __dest->__val[__cnt] = (__left->__val[__cnt]		\
-			      & __right->__val[__cnt]);		\
-    (void)0;							\
-  }))
-
-# define __sigorset(dest, left, right)				\
-  (__extension__ ({						\
-    int __cnt = _SIGSET_NWORDS;					\
-    sigset_t *__dest = (dest);					\
-    const sigset_t *__left = (left);				\
-    const sigset_t *__right = (right);				\
-    while (--__cnt >= 0)					\
-      __dest->__val[__cnt] = (__left->__val[__cnt]		\
-			      | __right->__val[__cnt]);		\
-    (void)0;							\
-  }))
-
-/* These macros needn't check for a bogus signal number;
-   error checking is done in the non-__ versions.  */
-# define __sigismember(set, sig)				\
-  (__extension__ ({						\
-    unsigned long int __mask = __sigmask (sig);			\
-    unsigned long int __word = __sigword (sig);			\
-    (set)->__val[__word] & __mask ? 1 : 0;			\
-  }))
-
-# define __sigaddset(set, sig)					\
-  (__extension__ ({						\
-    unsigned long int __mask = __sigmask (sig);			\
-    unsigned long int __word = __sigword (sig);			\
-    (set)->__val[__word] |= __mask;				\
-    (void)0;							\
-  }))
-
-# define __sigdelset(set, sig)					\
-  (__extension__ ({						\
-    unsigned long int __mask = __sigmask (sig);			\
-    unsigned long int __word = __sigword (sig);			\
-    (set)->__val[__word] &= ~__mask;				\
-    (void)0;							\
- }))
+# define __NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int )))
+
+static inline void
+__sigemptyset (sigset_t *set)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+   set->__val[cnt] = 0;
+}
+
+static inline void
+__sigfillset (sigset_t *set)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+   set->__val[cnt] = ~0UL;
+}
+
+static inline int
+__sigisemptyset (const sigset_t *set)
+{
+  int cnt = __NSIG_WORDS;
+  int ret = set->__val[--cnt];
+  while (ret == 0 && --cnt >= 0)
+    ret = set->__val[cnt];
+  return ret == 0;
+}
+
+static inline void
+__sigandset (sigset_t *dest, const sigset_t *left, const sigset_t *right)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+    dest->__val[cnt] = left->__val[cnt] & right->__val[cnt];
+}
+
+static inline void
+__sigorset (sigset_t *dest, const sigset_t *left, const sigset_t *right)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+    dest->__val[cnt] = left->__val[cnt] | right->__val[cnt];
+}
+
+static inline int
+__sigismember (const sigset_t *set, int sig)
+{
+  unsigned long int mask = __sigmask (sig);
+  unsigned long int word = __sigword (sig);
+  return set->__val[word] & mask ? 1 : 0;
+}
+
+static inline void
+__sigaddset (sigset_t *set, int sig)
+{
+  unsigned long int mask = __sigmask (sig);
+  unsigned long int word = __sigword (sig);
+  set->__val[word] |= mask;
+}
+
+static inline void
+__sigdelset (sigset_t *set, int sig)
+{
+  unsigned long int mask = __sigmask (sig);
+  unsigned long int word = __sigword (sig);
+  set->__val[word] &= ~mask;
+}
 
 #endif /* bits/sigsetops.h */