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

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

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 */
>