[PATCH 5/6] nptl: Implement raise with pthread_kill
Adhemerval Zanella
adhemerval.zanella@linaro.org
Fri Dec 4 18:32:51 GMT 2020
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)
>
More information about the Libc-alpha
mailing list