[5/6] nptl: Implement raise with pthread_kill

  The internal __pthread_kill_internal symbol has an extra argument
that specified if all signal or just application ones should be
blocked (raise can not be used with SIGCANCEL or SIGTIMER, so there
it no need to block them).

Checked on x86_64-linux-gnu.
---
 nptl/Makefile                      |  1 +
 nptl/Versions                      |  1 +
 nptl/pt-raise.c                    | 11 +-----
 nptl/pthreadP.h                    |  3 ++
 nptl/pthread_kill.c                | 28 +--------------
 nptl/pthread_kill_internal.c       | 56 ++++++++++++++++++++++++++++++
 sysdeps/posix/raise.c              | 10 ++++--
 sysdeps/unix/sysv/linux/pt-raise.c | 20 -----------
 sysdeps/unix/sysv/linux/raise.c    | 52 ---------------------------
 9 files changed, 71 insertions(+), 111 deletions(-)
 create mode 100644 nptl/pthread_kill_internal.c
 delete mode 100644 sysdeps/unix/sysv/linux/pt-raise.c
 delete mode 100644 sysdeps/unix/sysv/linux/raise.c
  

On 04/12/2020 15:09, Adhemerval Zanella wrote:
> The internal __pthread_kill_internal symbol has an extra argument
> that specified if all signal or just application ones should be
> blocked (raise can not be used with SIGCANCEL or SIGTIMER, so there
> it no need to block them).
> 
> Checked on x86_64-linux-gnu.
> ---
>  nptl/Makefile                      |  1 +
>  nptl/Versions                      |  1 +
>  nptl/pt-raise.c                    | 11 +-----
>  nptl/pthreadP.h                    |  3 ++
>  nptl/pthread_kill.c                | 28 +--------------
>  nptl/pthread_kill_internal.c       | 56 ++++++++++++++++++++++++++++++
>  sysdeps/posix/raise.c              | 10 ++++--
>  sysdeps/unix/sysv/linux/pt-raise.c | 20 -----------
>  sysdeps/unix/sysv/linux/raise.c    | 52 ---------------------------
>  9 files changed, 71 insertions(+), 111 deletions(-)
>  create mode 100644 nptl/pthread_kill_internal.c
>  delete mode 100644 sysdeps/unix/sysv/linux/pt-raise.c
>  delete mode 100644 sysdeps/unix/sysv/linux/raise.c
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 3f6e77f63f..424b96656a 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -66,6 +66,7 @@ routines = \
>    pthread_getattr_np \
>    pthread_getschedparam \
>    pthread_kill \
> +  pthread_kill_internal \
>    pthread_self \
>    pthread_setschedparam \
>    pthread_sigmask \
> diff --git a/nptl/Versions b/nptl/Versions
> index 7cfe39a91c..39578ef4e6 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -66,6 +66,7 @@ libc {
>      __pthread_attr_copy;
>      __pthread_getattr_default_np;
>      __pthread_attr_setsigmask_internal;
> +    __pthread_kill_internal;
>    }
>  }
>  
> diff --git a/nptl/pt-raise.c b/nptl/pt-raise.c
> index 069b33a86e..286a9f686a 100644
> --- a/nptl/pt-raise.c
> +++ b/nptl/pt-raise.c
> @@ -17,13 +17,4 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <pthread.h>
> -#include <signal.h>
> -
> -
> -int
> -raise (int sig)
> -{
> -  /* This is what POSIX says must happen.  */
> -  return pthread_kill (pthread_self (), sig);
> -}
> +#include "raise.c"
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index a7510f9f63..2f34ac1ab9 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -526,6 +526,9 @@ extern int __pthread_equal (pthread_t thread1, pthread_t thread2);
>  extern int __pthread_detach (pthread_t th);
>  extern int __pthread_cancel (pthread_t th);
>  extern int __pthread_kill (pthread_t threadid, int signo);
> +extern int __pthread_kill_internal (pthread_t threadif, int signo,
> +				    bool block_all);
> +libc_hidden_proto (__pthread_kill_internal)
>  extern void __pthread_exit (void *value) __attribute__ ((__noreturn__));
>  extern int __pthread_join (pthread_t threadid, void **thread_return);
>  extern int __pthread_setcanceltype (int type, int *oldtype);
> diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c
> index c547c5fe58..d114cbfca6 100644
> --- a/nptl/pthread_kill.c
> +++ b/nptl/pthread_kill.c
> @@ -16,37 +16,11 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <unistd.h>
>  #include <pthreadP.h>
>  
>  int
>  __pthread_kill (pthread_t threadid, int signo)
>  {
> -  /* Disallow sending the signal we use for cancellation, timers,
> -     for the setxid implementation.  */
> -  if (__is_internal_signal (signo))
> -    return EINVAL;
> -
> -  /* Force load of pd->tid into local variable or register.  Otherwise
> -     if a thread exits between ESRCH test and tgkill, we might return
> -     EINVAL, because pd->tid would be cleared by the kernel.  */
> -  struct pthread *pd = (struct pthread *) threadid;
> -  pid_t tid = atomic_forced_read (pd->tid);
> -  if (__glibc_unlikely (tid <= 0))
> -    /* Not a valid thread handle.  */
> -    return ESRCH;
> -
> -  sigset_t set;
> -  __libc_signal_block_all (&set);
> -
> -  /* We have a special syscall to do the work.  */
> -  pid_t pid = __getpid ();
> -
> -  int val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo);
> -  val = (INTERNAL_SYSCALL_ERROR_P (val) ? INTERNAL_SYSCALL_ERRNO (val) : 0);
> -
> -  __libc_signal_restore_set (&set);
> -
> -  return val;
> +  return __pthread_kill_internal (threadid, signo, true);
>  }
>  strong_alias (__pthread_kill, pthread_kill)
> diff --git a/nptl/pthread_kill_internal.c b/nptl/pthread_kill_internal.c
> new file mode 100644
> index 0000000000..46a47650a1
> --- /dev/null
> +++ b/nptl/pthread_kill_internal.c
> @@ -0,0 +1,56 @@
> +/* Common implementation for raise/pthread_kill.  Linux version.
> +   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 <unistd.h>
> +#include <pthreadP.h>
> +
> +int
> +__pthread_kill_internal (pthread_t threadid, int signo, bool block_all)
> +{
> +  /* Disallow sending the signal we use for cancellation, timers,
> +     for the setxid implementation.  */
> +  if (__is_internal_signal (signo))
> +    return EINVAL;
> +
> +  /* Force load of pd->tid into local variable or register.  Otherwise
> +     if a thread exits between ESRCH test and tgkill, we might return
> +     EINVAL, because pd->tid would be cleared by the kernel.  */
> +  struct pthread *pd = (struct pthread *) threadid;
> +  pid_t tid = atomic_forced_read (pd->tid);
> +  if (__glibc_unlikely (tid <= 0))
> +    /* Not a valid thread handle.  */
> +    return ESRCH;
> +
> +  sigset_t set;
> +  if (block_all)
> +    __libc_signal_block_all (&set);
> +  else
> +    __libc_signal_block_app (&set);
> +
> +  /* We have a special syscall to do the work.  */
> +  pid_t pid = __getpid ();

As Florian has pointed out, using the cached tid might not work when
vfork is used.  In this case I think is better to remove the atomic
read and issue __gettid () instead (as Linux raise does currently).

I will send an updated version.

> +
> +  int val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo);
> +  val = (INTERNAL_SYSCALL_ERROR_P (val) ? INTERNAL_SYSCALL_ERRNO (val) : 0);
> +
> +  __libc_signal_restore_set (&set);
> +
> +  return val;
> +}
> +libc_hidden_def (__pthread_kill_internal)
> +
> diff --git a/sysdeps/posix/raise.c b/sysdeps/posix/raise.c
> index 32cb108f0b..8a3ea95b3e 100644
> --- a/sysdeps/posix/raise.c
> +++ b/sysdeps/posix/raise.c
> @@ -16,13 +16,19 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <signal.h>
> -#include <unistd.h>
> +#include <nptl/pthreadP.h>
>  
>  /* Raise the signal SIG.  */
>  int
>  raise (int sig)
>  {
> -  return __kill (__getpid (), sig);
> +  int r = __pthread_kill_internal ((pthread_t) THREAD_SELF, sig, false);
> +  if (r != 0)
> +    {
> +      __set_errno (r);
> +      r = -1;
> +    }
> +  return r;
>  }
>  libc_hidden_def (raise)
>  weak_alias (raise, gsignal)
> diff --git a/sysdeps/unix/sysv/linux/pt-raise.c b/sysdeps/unix/sysv/linux/pt-raise.c
> deleted file mode 100644
> index 0c8246b0cc..0000000000
> --- a/sysdeps/unix/sysv/linux/pt-raise.c
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* ISO C raise function for libpthread.
> -   Copyright (C) 2002-2020 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 <sysdeps/unix/sysv/linux/raise.c>
> diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c
> deleted file mode 100644
> index 3b90ae1d55..0000000000
> --- a/sysdeps/unix/sysv/linux/raise.c
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -/* Copyright (C) 2002-2020 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 <signal.h>
> -#include <sysdep.h>
> -#include <errno.h>
> -#include <sys/types.h>
> -#include <unistd.h>
> -#include <internal-signals.h>
> -
> -int
> -raise (int sig)
> -{
> -  /* rt_sigprocmask may fail if:
> -
> -     1. sigsetsize != sizeof (sigset_t) (EINVAL)
> -     2. a failure in copy from/to user space (EFAULT)
> -     3. an invalid 'how' operation (EINVAL)
> -
> -     The first case is already handle in glibc syscall call by using the arch
> -     defined _NSIG.  Second case is handled by using a stack allocated mask.
> -     The last one should be handled by the block/unblock functions.  */
> -
> -  sigset_t set;
> -  __libc_signal_block_app (&set);
> -
> -  pid_t pid = INTERNAL_SYSCALL_CALL (getpid);
> -  pid_t tid = INTERNAL_SYSCALL_CALL (gettid);
> -
> -  int ret = INLINE_SYSCALL_CALL (tgkill, pid, tid, sig);
> -
> -  __libc_signal_restore_set (&set);
> -
> -  return ret;
> -}
> -libc_hidden_def (raise)
> -weak_alias (raise, gsignal)
>