[v2,3/4] linux: Use pthread_sigmask on sigprocmask
Commit Message
With pthread_sigmask on libc.so, it allows consolidate both
implementations.
Checked on x86_64-linux-gnu.
---
nptl/pthreadP.h | 1 +
nptl/pthread_sigmask.c | 2 ++
sysdeps/unix/sysv/linux/sigprocmask.c | 20 +++++---------------
3 files changed, 8 insertions(+), 15 deletions(-)
Comments
Ping.
On 13/03/2020 16:48, Adhemerval Zanella wrote:
> With pthread_sigmask on libc.so, it allows consolidate both
> implementations.
>
> Checked on x86_64-linux-gnu.
> ---
> nptl/pthreadP.h | 1 +
> nptl/pthread_sigmask.c | 2 ++
> sysdeps/unix/sysv/linux/sigprocmask.c | 20 +++++---------------
> 3 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index edec8d0501..c6d8fc69be 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -484,6 +484,7 @@ extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t,
> attribute_hidden;
> extern int __pthread_sigmask (int how, const sigset_t *newmask,
> sigset_t *oldmask);
> +libc_hidden_proto (__pthread_sigmask)
>
>
> #if IS_IN (libpthread)
> diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c
> index 0e326d610c..c6c6e83c08 100644
> --- a/nptl/pthread_sigmask.c
> +++ b/nptl/pthread_sigmask.c
> @@ -46,6 +46,8 @@ __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
> ? INTERNAL_SYSCALL_ERRNO (result)
> : 0);
> }
> +libc_hidden_def (__pthread_sigmask)
> +
> versioned_symbol (libc, __pthread_sigmask, pthread_sigmask, GLIBC_2_32);
> #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_32)
> strong_alias (__pthread_sigmask, __pthread_sigmask_2);
> diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
> index eb9e4d5e83..6ed0ab1e6a 100644
> --- a/sysdeps/unix/sysv/linux/sigprocmask.c
> +++ b/sysdeps/unix/sysv/linux/sigprocmask.c
> @@ -22,21 +22,11 @@
> int
> __sigprocmask (int how, const sigset_t *set, sigset_t *oset)
> {
> - sigset_t local_newmask;
> -
> - /* The only thing we have to make sure here is that SIGCANCEL and
> - SIGSETXID are not blocked. */
> - if (set != NULL
> - && __glibc_unlikely (__sigismember (set, SIGCANCEL)
> - || __glibc_unlikely (__sigismember (set, SIGSETXID))))
> - {
> - local_newmask = *set;
> - __sigdelset (&local_newmask, SIGCANCEL);
> - __sigdelset (&local_newmask, SIGSETXID);
> - set = &local_newmask;
> - }
> -
> - return INLINE_SYSCALL_CALL (rt_sigprocmask, how, set, oset, _NSIG / 8);
> + int result = __pthread_sigmask (how, set, oset);
> + if (result == 0)
> + return 0;
> + __set_errno (result);
> + return result;
> }
> libc_hidden_def (__sigprocmask)
> weak_alias (__sigprocmask, sigprocmask)
>
* Adhemerval Zanella via Libc-alpha:
> With pthread_sigmask on libc.so, it allows consolidate both
> implementations.
“in sigprocmask”?
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index edec8d0501..c6d8fc69be 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -484,6 +484,7 @@ extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t,
> attribute_hidden;
> extern int __pthread_sigmask (int how, const sigset_t *newmask,
> sigset_t *oldmask);
> +libc_hidden_proto (__pthread_sigmask)
>
>
> #if IS_IN (libpthread)
> diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c
> index 0e326d610c..c6c6e83c08 100644
> --- a/nptl/pthread_sigmask.c
> +++ b/nptl/pthread_sigmask.c
> @@ -46,6 +46,8 @@ __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
> ? INTERNAL_SYSCALL_ERRNO (result)
> : 0);
> }
> +libc_hidden_def (__pthread_sigmask)
> +
> versioned_symbol (libc, __pthread_sigmask, pthread_sigmask, GLIBC_2_32);
> #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_32)
> strong_alias (__pthread_sigmask, __pthread_sigmask_2);
Ah, perhaps put this into the first commit?
> diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
> index eb9e4d5e83..6ed0ab1e6a 100644
> --- a/sysdeps/unix/sysv/linux/sigprocmask.c
> +++ b/sysdeps/unix/sysv/linux/sigprocmask.c
> + int result = __pthread_sigmask (how, set, oset);
> + if (result == 0)
> + return 0;
> + __set_errno (result);
> + return result;
The final statement needs to return -1. It's curious that this does
not result in a testsuite failure.
On 21/04/2020 09:01, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
>
>> With pthread_sigmask on libc.so, it allows consolidate both
>> implementations.
>
> “in sigprocmask”?
Ack.
>
>> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
>> index edec8d0501..c6d8fc69be 100644
>> --- a/nptl/pthreadP.h
>> +++ b/nptl/pthreadP.h
>> @@ -484,6 +484,7 @@ extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t,
>> attribute_hidden;
>> extern int __pthread_sigmask (int how, const sigset_t *newmask,
>> sigset_t *oldmask);
>> +libc_hidden_proto (__pthread_sigmask)
>>
>>
>> #if IS_IN (libpthread)
>> diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c
>> index 0e326d610c..c6c6e83c08 100644
>> --- a/nptl/pthread_sigmask.c
>> +++ b/nptl/pthread_sigmask.c
>> @@ -46,6 +46,8 @@ __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
>> ? INTERNAL_SYSCALL_ERRNO (result)
>> : 0);
>> }
>> +libc_hidden_def (__pthread_sigmask)
>> +
>> versioned_symbol (libc, __pthread_sigmask, pthread_sigmask, GLIBC_2_32);
>> #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_32)
>> strong_alias (__pthread_sigmask, __pthread_sigmask_2);
>
> Ah, perhaps put this into the first commit?
Alright (although it just does require on this set).
>
>> diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
>> index eb9e4d5e83..6ed0ab1e6a 100644
>> --- a/sysdeps/unix/sysv/linux/sigprocmask.c
>> +++ b/sysdeps/unix/sysv/linux/sigprocmask.c
>
>> + int result = __pthread_sigmask (how, set, oset);
>> + if (result == 0)
>> + return 0;
>> + __set_errno (result);
>> + return result;
>
> The final statement needs to return -1. It's curious that this does
> not result in a testsuite failure.
>
Ack. I think because sigprocmask only fails for an invalid 'how'
(EINVAL) or invalid address (EFAULT) for 'set' or 'oldset', which
are not issued on glibc testsuite.
* Adhemerval Zanella:
>>> diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
>>> index eb9e4d5e83..6ed0ab1e6a 100644
>>> --- a/sysdeps/unix/sysv/linux/sigprocmask.c
>>> +++ b/sysdeps/unix/sysv/linux/sigprocmask.c
>>
>>> + int result = __pthread_sigmask (how, set, oset);
>>> + if (result == 0)
>>> + return 0;
>>> + __set_errno (result);
>>> + return result;
>>
>> The final statement needs to return -1. It's curious that this does
>> not result in a testsuite failure.
>>
>
> Ack. I think because sigprocmask only fails for an invalid 'how'
> (EINVAL) or invalid address (EFAULT) for 'set' or 'oldset', which
> are not issued on glibc testsuite.
Maybe you can add another test to
sysdeps/unix/sysv/linux/test-errno-linux.c, just in case?
On 21/04/2020 09:35, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>>>> diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
>>>> index eb9e4d5e83..6ed0ab1e6a 100644
>>>> --- a/sysdeps/unix/sysv/linux/sigprocmask.c
>>>> +++ b/sysdeps/unix/sysv/linux/sigprocmask.c
>>>
>>>> + int result = __pthread_sigmask (how, set, oset);
>>>> + if (result == 0)
>>>> + return 0;
>>>> + __set_errno (result);
>>>> + return result;
>>>
>>> The final statement needs to return -1. It's curious that this does
>>> not result in a testsuite failure.
>>>
>>
>> Ack. I think because sigprocmask only fails for an invalid 'how'
>> (EINVAL) or invalid address (EFAULT) for 'set' or 'oldset', which
>> are not issued on glibc testsuite.
>
> Maybe you can add another test to
> sysdeps/unix/sysv/linux/test-errno-linux.c, just in case?
>
Ack. I will do it.
@@ -484,6 +484,7 @@ extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t,
attribute_hidden;
extern int __pthread_sigmask (int how, const sigset_t *newmask,
sigset_t *oldmask);
+libc_hidden_proto (__pthread_sigmask)
#if IS_IN (libpthread)
@@ -46,6 +46,8 @@ __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
? INTERNAL_SYSCALL_ERRNO (result)
: 0);
}
+libc_hidden_def (__pthread_sigmask)
+
versioned_symbol (libc, __pthread_sigmask, pthread_sigmask, GLIBC_2_32);
#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_32)
strong_alias (__pthread_sigmask, __pthread_sigmask_2);
@@ -22,21 +22,11 @@
int
__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
{
- sigset_t local_newmask;
-
- /* The only thing we have to make sure here is that SIGCANCEL and
- SIGSETXID are not blocked. */
- if (set != NULL
- && __glibc_unlikely (__sigismember (set, SIGCANCEL)
- || __glibc_unlikely (__sigismember (set, SIGSETXID))))
- {
- local_newmask = *set;
- __sigdelset (&local_newmask, SIGCANCEL);
- __sigdelset (&local_newmask, SIGSETXID);
- set = &local_newmask;
- }
-
- return INLINE_SYSCALL_CALL (rt_sigprocmask, how, set, oset, _NSIG / 8);
+ int result = __pthread_sigmask (how, set, oset);
+ if (result == 0)
+ return 0;
+ __set_errno (result);
+ return result;
}
libc_hidden_def (__sigprocmask)
weak_alias (__sigprocmask, sigprocmask)