* [PATCH 0/3] Cleanup and consolidate sigprocmask() for Linux
@ 2017-10-16 4:34 Yury Norov
2017-10-16 4:34 ` [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c Yury Norov
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Yury Norov @ 2017-10-16 4:34 UTC (permalink / raw)
To: libc-alpha; +Cc: Yury Norov
First patch of the series cleans up #ifdefs mess in generic
sigprocmask() implementation at sysdeps/unix/sysv/linux/sigprocmask.c.
Second patch consolidates all Linux implemantations of sigprocmask()
except alpha, and third RFC patch does it for alpha separately.
Yury Norov (3):
Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c
Consolidate Linux sigprocmask() implementation
Consolidate Linux sigprocmask() implementationa for alpha
sysdeps/unix/sysv/linux/alpha/sigprocmask.c | 41 ++--------------------
sysdeps/unix/sysv/linux/ia64/sigprocmask.c | 22 ++----------
sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c | 22 ++----------
sysdeps/unix/sysv/linux/sigprocmask.c | 18 +++++-----
.../unix/sysv/linux/sparc/sparc64/sigprocmask.c | 18 ++--------
sysdeps/unix/sysv/linux/x86_64/sigprocmask.c | 22 ++----------
6 files changed, 19 insertions(+), 124 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c
2017-10-16 4:34 [PATCH 0/3] Cleanup and consolidate sigprocmask() for Linux Yury Norov
@ 2017-10-16 4:34 ` Yury Norov
2017-10-16 9:04 ` Andreas Schwab
2017-10-16 4:35 ` [PATCH 2/3] Consolidate Linux sigprocmask() implementation Yury Norov
2017-10-16 4:35 ` [RFC PATCH 3/3] Consolidate Linux sigprocmask() implementationa for alpha Yury Norov
2 siblings, 1 reply; 11+ messages in thread
From: Yury Norov @ 2017-10-16 4:34 UTC (permalink / raw)
To: libc-alpha; +Cc: Yury Norov
The sysdeps/unix/sysv/linux/sigprocmask.c includes nptl-signals.h
via nptl/pthreadP.h, and so SIGCANCEL and SIGSETXID become defined
unconditionally. But later in the code, there are some checks weither
symbols defined, which is useless. This patch removes useless checks.
* sysdeps/unix/sysv/linux/sigprocmask.c: Remove useless #ifdefs
---
sysdeps/unix/sysv/linux/sigprocmask.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
index d0b8e049b2..e776563336 100644
--- a/sysdeps/unix/sysv/linux/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/sigprocmask.c
@@ -30,26 +30,19 @@
int
__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
{
-#ifdef SIGCANCEL
sigset_t local_newmask;
/* The only thing we have to make sure here is that SIGCANCEL and
SIGSETXID are not blocked. */
if (set != NULL
&& (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
-# ifdef SIGSETXID
- || __builtin_expect (__sigismember (set, SIGSETXID), 0)
-# endif
- ))
+ || __builtin_expect (__sigismember (set, SIGSETXID), 0)))
{
local_newmask = *set;
__sigdelset (&local_newmask, SIGCANCEL);
-# ifdef SIGSETXID
__sigdelset (&local_newmask, SIGSETXID);
-# endif
set = &local_newmask;
}
-#endif
return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
}
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 3/3] Consolidate Linux sigprocmask() implementationa for alpha
2017-10-16 4:34 [PATCH 0/3] Cleanup and consolidate sigprocmask() for Linux Yury Norov
2017-10-16 4:34 ` [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c Yury Norov
2017-10-16 4:35 ` [PATCH 2/3] Consolidate Linux sigprocmask() implementation Yury Norov
@ 2017-10-16 4:35 ` Yury Norov
2 siblings, 0 replies; 11+ messages in thread
From: Yury Norov @ 2017-10-16 4:35 UTC (permalink / raw)
To: libc-alpha; +Cc: Yury Norov
Currently alpha port uses osf_sigprocmask to implement sigprocmask().
Minimal supported kernel version is 3.2, and it exposes rt_sigprocmask
system call for alpha. This RFC patch switches alpha to rt_sigprocmask,
similarly to other ports. If alpha community OK, please pull it.
* sysdeps/unix/sysv/linux/alpha/sigprocmask.c: Consolidate
sigprocmask() implementation.
---
sysdeps/unix/sysv/linux/alpha/sigprocmask.c | 41 ++---------------------------
1 file changed, 2 insertions(+), 39 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c b/sysdeps/unix/sysv/linux/alpha/sigprocmask.c
index ebec70cff0..c8e515860f 100644
--- a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/alpha/sigprocmask.c
@@ -16,43 +16,6 @@
License along with the GNU C Library. If not, see
<http://www.gnu.org/licenses/>. */
-#include <errno.h>
-#include <sysdep.h>
-#include <signal.h>
+#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0
-/* When there is kernel support for more than 64 signals, we'll have to
- switch to a new system call convention here. */
-
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
- unsigned long int setval;
- long result;
-
- if (set)
- setval = set->__val[0];
- else
- {
- setval = 0;
- how = SIG_BLOCK; /* ensure blocked mask doesn't get changed */
- }
-
- result = INLINE_SYSCALL (osf_sigprocmask, 2, how, setval);
- if (result == -1)
- /* If there are ever more than 63 signals, we need to recode this
- in assembler since we wouldn't be able to distinguish a mask of
- all 1s from -1, but for now, we're doing just fine... */
- return result;
-
- if (oset)
- {
- oset->__val[0] = result;
- result = _SIGSET_NWORDS;
- while (--result > 0)
- oset->__val[result] = 0;
- }
- return 0;
-}
-
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask);
+#include <sysdeps/unix/sysv/linux/sigprocmask.c>
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] Consolidate Linux sigprocmask() implementation
2017-10-16 4:34 [PATCH 0/3] Cleanup and consolidate sigprocmask() for Linux Yury Norov
2017-10-16 4:34 ` [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c Yury Norov
@ 2017-10-16 4:35 ` Yury Norov
2017-10-31 21:19 ` Adhemerval Zanella
2017-10-16 4:35 ` [RFC PATCH 3/3] Consolidate Linux sigprocmask() implementationa for alpha Yury Norov
2 siblings, 1 reply; 11+ messages in thread
From: Yury Norov @ 2017-10-16 4:35 UTC (permalink / raw)
To: libc-alpha; +Cc: Yury Norov
ia64, s390-64, sparc64, x86_64 and alpha ports has their own
implementations of sigprocmask(). They all but alpha do exactly
what generic sigprocmask() except the check and clear SIGCANCEL
and SIGSETXID flags.
In this patch, the NEED_CLEAR_SIGCANCEL_SIGSETXID option is
introduced and disabled for that ports which allows to swith
them to generic implementation.
* sysdeps/unix/sysv/linux/sigprocmask.c: Introduce
NEED_CLEAR_SIGCANCEL_SIGSETXID option for consolidation
of Linux sigprocmask() implementation;
* sysdeps/unix/sysv/linux/ia64/sigprocmask.c: Consolidate
sigprocmask() implementation;
* sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c: Likewise;
* sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c: Likewise;
* sysdeps/unix/sysv/linux/x86_64/sigprocmask.c: Likewise.
---
sysdeps/unix/sysv/linux/ia64/sigprocmask.c | 22 ++--------------------
sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c | 22 ++--------------------
sysdeps/unix/sysv/linux/sigprocmask.c | 11 +++++++++--
.../unix/sysv/linux/sparc/sparc64/sigprocmask.c | 18 ++----------------
sysdeps/unix/sysv/linux/x86_64/sigprocmask.c | 22 ++--------------------
5 files changed, 17 insertions(+), 78 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c
index 920c5fd052..9e95e52d58 100644
--- a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c
@@ -17,24 +17,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-/* Linux/ia64 only has rt signals, thus we do not even want to try falling
- back to the old style signals as the default Linux handler does. */
+#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
-
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
+#include <sysdeps/unix/sysv/linux/sigprocmask.c>
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
index a8010e7f14..227b512910 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
@@ -15,24 +15,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-/* 64 bit Linux for S/390 only has rt signals, thus we do not even want to try
- falling back to the old style signals as the default Linux handler does. */
+#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
-
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
+#include <sysdeps/unix/sysv/linux/sigprocmask.c>
diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
index e776563336..94d4d0945f 100644
--- a/sysdeps/unix/sysv/linux/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/sigprocmask.c
@@ -17,19 +17,25 @@
#include <errno.h>
#include <signal.h>
-#include <string.h> /* Needed for string function builtin redirection. */
#include <unistd.h>
#include <sysdep.h>
#include <sys/syscall.h>
-#include <nptl/pthreadP.h> /* SIGCANCEL, SIGSETXID */
+#ifndef NEED_CLEAR_SIGCANCEL_SIGSETXID
+#define NEED_CLEAR_SIGCANCEL_SIGSETXID 1
+#endif
+#if NEED_CLEAR_SIGCANCEL_SIGSETXID
+#include <nptl/pthreadP.h>
+#include <string.h>
+#endif
/* Get and/or change the set of blocked signals. */
int
__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
{
+#if NEED_CLEAR_SIGCANCEL_SIGSETXID
sigset_t local_newmask;
/* The only thing we have to make sure here is that SIGCANCEL and
@@ -43,6 +49,7 @@ __sigprocmask (int how, const sigset_t *set, sigset_t *oset)
__sigdelset (&local_newmask, SIGSETXID);
set = &local_newmask;
}
+#endif
return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
}
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
index ef7d7feb49..0db24d6fce 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
@@ -15,20 +15,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
+#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
+#include <sysdeps/unix/sysv/linux/sigprocmask.c>
diff --git a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
index 1610ddf47f..d3179ebea5 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
@@ -16,24 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-/* Linux/x86_64 only has rt signals, thus we do not even want to try falling
- back to the old style signals as the default Linux handler does. */
+#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
-
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
+#include <sysdeps/unix/sysv/linux/sigprocmask.c>
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c
2017-10-16 4:34 ` [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c Yury Norov
@ 2017-10-16 9:04 ` Andreas Schwab
2017-10-20 15:38 ` Yury Norov
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2017-10-16 9:04 UTC (permalink / raw)
To: Yury Norov; +Cc: libc-alpha
On Okt 16 2017, Yury Norov <ynorov@caviumnetworks.com> wrote:
> * sysdeps/unix/sysv/linux/sigprocmask.c: Remove useless #ifdefs
Ok.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c
2017-10-16 9:04 ` Andreas Schwab
@ 2017-10-20 15:38 ` Yury Norov
2017-10-23 8:07 ` Andreas Schwab
0 siblings, 1 reply; 11+ messages in thread
From: Yury Norov @ 2017-10-20 15:38 UTC (permalink / raw)
To: Andreas Schwab; +Cc: libc-alpha
On Mon, Oct 16, 2017 at 11:04:49AM +0200, Andreas Schwab wrote:
> On Okt 16 2017, Yury Norov <ynorov@caviumnetworks.com> wrote:
>
> > * sysdeps/unix/sysv/linux/sigprocmask.c: Remove useless #ifdefs
>
> Ok.
Hi Andreas, all,
Thank you for your review. I looked at the code again and found
another syscalls that check SIGCANCEl and SIGSETXID. So below is
more generic version. Also notice that all syscalls below do the
same thing - clear nptl-internal bits in sigset. I think it is
worth to create a helper for it, and I'll send the patch little
later.
Yury
From ed0d0bc461662c2d91b2c7fbb46415a0e0051cbc Mon Sep 17 00:00:00 2001
From: Yury Norov <ynorov@caviumnetworks.com>
Date: Mon, 16 Oct 2017 02:39:20 +0300
Subject: [PATCH v2 1/3] Remove useless #ifdefs from Linux sig*.c syscalls
sigprocmask.c, sigtimedwait.c, sigwait.c and sigwaitinfo.c files from
sysdeps/unix/sysv/linux include nptl-signals.h via nptl/pthreadP.h,
and so SIGCANCEL and SIGSETXID become defined unconditionally. But
later in the code, there are some checks weither symbols defined,
which is useless. This patch removes useless checks.
* sysdeps/unix/sysv/linux/sigprocmask.c: Remove useless #ifdefs;
* sysdeps/unix/sysv/linux/sigtimedwait.c: Likewise;
* sysdeps/unix/sysv/linux/sigwait.c: Likewise;
* sysdeps/unix/sysv/linux/sigwaitinfo.c: Likewise.
Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
sysdeps/unix/sysv/linux/sigprocmask.c | 9 +--------
sysdeps/unix/sysv/linux/sigtimedwait.c | 9 +--------
sysdeps/unix/sysv/linux/sigwait.c | 9 +--------
sysdeps/unix/sysv/linux/sigwaitinfo.c | 9 +--------
4 files changed, 4 insertions(+), 32 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
index d0b8e049b2..e776563336 100644
--- a/sysdeps/unix/sysv/linux/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/sigprocmask.c
@@ -30,26 +30,19 @@
int
__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
{
-#ifdef SIGCANCEL
sigset_t local_newmask;
/* The only thing we have to make sure here is that SIGCANCEL and
SIGSETXID are not blocked. */
if (set != NULL
&& (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
-# ifdef SIGSETXID
- || __builtin_expect (__sigismember (set, SIGSETXID), 0)
-# endif
- ))
+ || __builtin_expect (__sigismember (set, SIGSETXID), 0)))
{
local_newmask = *set;
__sigdelset (&local_newmask, SIGCANCEL);
-# ifdef SIGSETXID
__sigdelset (&local_newmask, SIGSETXID);
-# endif
set = &local_newmask;
}
-#endif
return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
}
diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c
index ab1a84ef1c..42afbce22c 100644
--- a/sysdeps/unix/sysv/linux/sigtimedwait.c
+++ b/sysdeps/unix/sysv/linux/sigtimedwait.c
@@ -29,25 +29,18 @@ int
__sigtimedwait (const sigset_t *set, siginfo_t *info,
const struct timespec *timeout)
{
-#ifdef SIGCANCEL
sigset_t tmpset;
if (set != NULL
&& (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
-# ifdef SIGSETXID
- || __builtin_expect (__sigismember (set, SIGSETXID), 0)
-# endif
- ))
+ || __builtin_expect (__sigismember (set, SIGSETXID), 0)))
{
/* Create a temporary mask without the bit for SIGCANCEL set. */
// We are not copying more than we have to.
memcpy (&tmpset, set, _NSIG / 8);
__sigdelset (&tmpset, SIGCANCEL);
-# ifdef SIGSETXID
__sigdelset (&tmpset, SIGSETXID);
-# endif
set = &tmpset;
}
-#endif
/* XXX The size argument hopefully will have to be changed to the
real size of the user-level sigset_t. */
diff --git a/sysdeps/unix/sysv/linux/sigwait.c b/sysdeps/unix/sysv/linux/sigwait.c
index 48bcd2fda7..395bd9feb6 100644
--- a/sysdeps/unix/sysv/linux/sigwait.c
+++ b/sysdeps/unix/sysv/linux/sigwait.c
@@ -33,25 +33,18 @@ do_sigwait (const sigset_t *set, int *sig)
{
int ret;
-#ifdef SIGCANCEL
sigset_t tmpset;
if (set != NULL
&& (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
-# ifdef SIGSETXID
- || __builtin_expect (__sigismember (set, SIGSETXID), 0)
-# endif
- ))
+ || __builtin_expect (__sigismember (set, SIGSETXID), 0)))
{
/* Create a temporary mask without the bit for SIGCANCEL set. */
// We are not copying more than we have to.
memcpy (&tmpset, set, _NSIG / 8);
__sigdelset (&tmpset, SIGCANCEL);
-# ifdef SIGSETXID
__sigdelset (&tmpset, SIGSETXID);
-# endif
set = &tmpset;
}
-#endif
/* XXX The size argument hopefully will have to be changed to the
real size of the user-level sigset_t. */
diff --git a/sysdeps/unix/sysv/linux/sigwaitinfo.c b/sysdeps/unix/sysv/linux/sigwaitinfo.c
index 5a044f08e3..0062d3ea86 100644
--- a/sysdeps/unix/sysv/linux/sigwaitinfo.c
+++ b/sysdeps/unix/sysv/linux/sigwaitinfo.c
@@ -31,25 +31,18 @@
int
__sigwaitinfo (const sigset_t *set, siginfo_t *info)
{
-#ifdef SIGCANCEL
sigset_t tmpset;
if (set != NULL
&& (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
-# ifdef SIGSETXID
- || __builtin_expect (__sigismember (set, SIGSETXID), 0)
-# endif
- ))
+ || __builtin_expect (__sigismember (set, SIGSETXID), 0)))
{
/* Create a temporary mask without the bit for SIGCANCEL set. */
// We are not copying more than we have to.
memcpy (&tmpset, set, _NSIG / 8);
__sigdelset (&tmpset, SIGCANCEL);
-# ifdef SIGSETXID
__sigdelset (&tmpset, SIGSETXID);
-# endif
set = &tmpset;
}
-#endif
/* XXX The size argument hopefully will have to be changed to the
real size of the user-level sigset_t. */
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c
2017-10-20 15:38 ` Yury Norov
@ 2017-10-23 8:07 ` Andreas Schwab
2017-10-31 19:54 ` Adhemerval Zanella
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2017-10-23 8:07 UTC (permalink / raw)
To: Yury Norov; +Cc: libc-alpha
On Okt 20 2017, Yury Norov <ynorov@caviumnetworks.com> wrote:
> * sysdeps/unix/sysv/linux/sigprocmask.c: Remove useless #ifdefs;
> * sysdeps/unix/sysv/linux/sigtimedwait.c: Likewise;
> * sysdeps/unix/sysv/linux/sigwait.c: Likewise;
> * sysdeps/unix/sysv/linux/sigwaitinfo.c: Likewise.
Ok.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c
2017-10-23 8:07 ` Andreas Schwab
@ 2017-10-31 19:54 ` Adhemerval Zanella
0 siblings, 0 replies; 11+ messages in thread
From: Adhemerval Zanella @ 2017-10-31 19:54 UTC (permalink / raw)
To: libc-alpha
On 23/10/2017 06:07, Andreas Schwab wrote:
> On Okt 20 2017, Yury Norov <ynorov@caviumnetworks.com> wrote:
>
>> * sysdeps/unix/sysv/linux/sigprocmask.c: Remove useless #ifdefs;
>> * sysdeps/unix/sysv/linux/sigtimedwait.c: Likewise;
>> * sysdeps/unix/sysv/linux/sigwait.c: Likewise;
>> * sysdeps/unix/sysv/linux/sigwaitinfo.c: Likewise.
>
> Ok.
>
> Andreas.
>
Pushed upstream as 87bbc4c.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Consolidate Linux sigprocmask() implementation
2017-10-16 4:35 ` [PATCH 2/3] Consolidate Linux sigprocmask() implementation Yury Norov
@ 2017-10-31 21:19 ` Adhemerval Zanella
2017-10-31 22:58 ` Yury Norov
0 siblings, 1 reply; 11+ messages in thread
From: Adhemerval Zanella @ 2017-10-31 21:19 UTC (permalink / raw)
To: libc-alpha
On 16/10/2017 02:34, Yury Norov wrote:
> ia64, s390-64, sparc64, x86_64 and alpha ports has their own
> implementations of sigprocmask(). They all but alpha do exactly
> what generic sigprocmask() except the check and clear SIGCANCEL
> and SIGSETXID flags.
>
> In this patch, the NEED_CLEAR_SIGCANCEL_SIGSETXID option is
> introduced and disabled for that ports which allows to swith
> them to generic implementation.
Although the manual do not state the Linux implementation detail I think
all supported Linux architecture should have the same semantic regarding
SIGCANCEL and SIGSETXID. GLIBC on Linux requires both signal to proper
implement both pthread cancellation and set*id function and having
different semanticsis troublesome (a conformant program on a architecture
that does not filter out the signals might inadvertently disable pthread
asynchronous cancellation, set*id synchronization or posix timers).
Also, sigfillset removes SIGCANCEL and SIGSETXID as expected, but
sigaddset and sigdelset does not handle none of internal signals. I also
think we should ignore internal nptl signals on sigaddset and sigdelset.
And for this specific case I don't see adding compat symbols to keep
the old semantic for the related architectures the best approach. There
is a canonical way to actually disable pthread cancellation and masking
SIGSETXID would make set*id non POSIX conformant.
What about the following?
---
This patch consolidates the sigprocmask Linux syscall implementation on
sysdeps/unix/sysv/linux/sigprocmask.c. The changes are:
1. For ia64, s390-64, sparc64, and x86_64 the default semantic for
filter out SIGCANCEL and SIGSETXID is used. Also the Linux pthread
semantic is documented in the signal chapter.
2. A new internal function to check for NPTL internal signals within a
signal set is added (__nptl_has_internal_signal). It is used to
simplify the default sigprocmask.c implementation.
Checked on x86_64-linux-gnu.
* manual/signal.texi: Add a note about internal pthread signals
on Linux.
* sysdeps/unix/sysv/linux/alpha/sigprocmask.c: Remove file.
* sysdeps/unix/sysv/linux/ia64/sigprocmask.c: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c: Likewise.
* sysdeps/unix/sysv/linux/x86_64/sigprocmask.c: Likewise.
* sysdeps/unix/sysv/linux/nptl-signals.h
(__nptl_has_internal_signal): New function.
* sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask):
Use __nptl_has_internal_signal and __nptl_clear_internal_signals
function.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
diff --git a/manual/signal.texi b/manual/signal.texi
index 9577ff0..46696af 100644
--- a/manual/signal.texi
+++ b/manual/signal.texi
@@ -2461,6 +2461,11 @@ well. (In addition, it's not wise to put into your program an
assumption that the system has no signals aside from the ones you know
about.)
+On Linux pthreads internally uses some signals to implement asynchronous
+cancellation, effective ID synchronization for POSIX conformance, and
+posix timer management. These signals are filtered out on signal set
+function manipulation.
+
@deftypefun int sigemptyset (sigset_t *@var{set})
@standards{POSIX.1, signal.h}
@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
diff --git a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c b/sysdeps/unix/sysv/linux/alpha/sigprocmask.c
deleted file mode 100644
index ebec70c..0000000
--- a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c
+++ /dev/null
@@ -1,58 +0,0 @@
-/* Copyright (C) 1993-2017 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
- Contributed by David Mosberger (davidm@azstarnet.com).
-
- 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
- <http://www.gnu.org/licenses/>. */
-
-#include <errno.h>
-#include <sysdep.h>
-#include <signal.h>
-
-/* When there is kernel support for more than 64 signals, we'll have to
- switch to a new system call convention here. */
-
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
- unsigned long int setval;
- long result;
-
- if (set)
- setval = set->__val[0];
- else
- {
- setval = 0;
- how = SIG_BLOCK; /* ensure blocked mask doesn't get changed */
- }
-
- result = INLINE_SYSCALL (osf_sigprocmask, 2, how, setval);
- if (result == -1)
- /* If there are ever more than 63 signals, we need to recode this
- in assembler since we wouldn't be able to distinguish a mask of
- all 1s from -1, but for now, we're doing just fine... */
- return result;
-
- if (oset)
- {
- oset->__val[0] = result;
- result = _SIGSET_NWORDS;
- while (--result > 0)
- oset->__val[result] = 0;
- }
- return 0;
-}
-
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask);
diff --git a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c
deleted file mode 100644
index 920c5fd..0000000
--- a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c
+++ /dev/null
@@ -1,40 +0,0 @@
-/* Copyright (C) 1997-2017 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
- Linux/IA64 specific sigprocmask
- Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999.
-
- 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
- <http://www.gnu.org/licenses/>. */
-
-/* Linux/ia64 only has rt signals, thus we do not even want to try falling
- back to the old style signals as the default Linux handler does. */
-
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
-
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h
index f30c597..6afd3fe 100644
--- a/sysdeps/unix/sysv/linux/nptl-signals.h
+++ b/sysdeps/unix/sysv/linux/nptl-signals.h
@@ -33,6 +33,12 @@
#define SIGSETXID (__SIGRTMIN + 1)
+static inline bool
+__nptl_has_internal_signal (const sigset_t *set)
+{
+ return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID);
+}
+
/* Return is sig is used internally. */
static inline int
__nptl_is_internal_signal (int sig)
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
deleted file mode 100644
index a8010e7..0000000
--- a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/* Copyright (C) 2001-2017 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
- <http://www.gnu.org/licenses/>. */
-
-/* 64 bit Linux for S/390 only has rt signals, thus we do not even want to try
- falling back to the old style signals as the default Linux handler does. */
-
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
-
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
index e776563..d14fc5c 100644
--- a/sysdeps/unix/sysv/linux/sigprocmask.c
+++ b/sysdeps/unix/sysv/linux/sigprocmask.c
@@ -1,4 +1,5 @@
-/* Copyright (C) 1997-2017 Free Software Foundation, Inc.
+/* Get and/or change the set of blocked signals. Linux version.
+ Copyright (C) 1997-2017 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
@@ -17,34 +18,22 @@
#include <errno.h>
#include <signal.h>
-#include <string.h> /* Needed for string function builtin redirection. */
-#include <unistd.h>
+#include <nptl-signals.h>
-#include <sysdep.h>
-#include <sys/syscall.h>
-#include <nptl/pthreadP.h> /* SIGCANCEL, SIGSETXID */
-
-
-/* Get and/or change the set of blocked signals. */
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
- && (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
- || __builtin_expect (__sigismember (set, SIGSETXID), 0)))
+ if (set != NULL && __glibc_unlikely (__nptl_has_internal_signal (set)))
{
local_newmask = *set;
- __sigdelset (&local_newmask, SIGCANCEL);
- __sigdelset (&local_newmask, SIGSETXID);
+ __nptl_clear_internal_signals (&local_newmask);
set = &local_newmask;
}
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
+ return INLINE_SYSCALL_CALL (rt_sigprocmask, how, set, oset, _NSIG / 8);
}
libc_hidden_def (__sigprocmask)
weak_alias (__sigprocmask, sigprocmask)
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
deleted file mode 100644
index ef7d7fe..0000000
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
+++ /dev/null
@@ -1,34 +0,0 @@
-/* Copyright (C) 1997-2017 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
- <http://www.gnu.org/licenses/>. */
-
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
diff --git a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
deleted file mode 100644
index 1610ddf..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
+++ /dev/null
@@ -1,39 +0,0 @@
-/* Copyright (C) 1997-2017 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
- Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999.
-
- 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
- <http://www.gnu.org/licenses/>. */
-
-/* Linux/x86_64 only has rt signals, thus we do not even want to try falling
- back to the old style signals as the default Linux handler does. */
-
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-/* Get and/or change the set of blocked signals. */
-int
-__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
-{
-
- /* XXX The size argument hopefully will have to be changed to the
- real size of the user-level sigset_t. */
- return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
-}
-libc_hidden_def (__sigprocmask)
-weak_alias (__sigprocmask, sigprocmask)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Consolidate Linux sigprocmask() implementation
2017-10-31 21:19 ` Adhemerval Zanella
@ 2017-10-31 22:58 ` Yury Norov
2017-11-03 11:19 ` Adhemerval Zanella
0 siblings, 1 reply; 11+ messages in thread
From: Yury Norov @ 2017-10-31 22:58 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Hi Adhemerval!
On Tue, Oct 31, 2017 at 07:19:01PM -0200, Adhemerval Zanella wrote:
>
>
> On 16/10/2017 02:34, Yury Norov wrote:
> > ia64, s390-64, sparc64, x86_64 and alpha ports has their own
> > implementations of sigprocmask(). They all but alpha do exactly
> > what generic sigprocmask() except the check and clear SIGCANCEL
> > and SIGSETXID flags.
> >
> > In this patch, the NEED_CLEAR_SIGCANCEL_SIGSETXID option is
> > introduced and disabled for that ports which allows to swith
> > them to generic implementation.
>
> Although the manual do not state the Linux implementation detail I think
> all supported Linux architecture should have the same semantic regarding
> SIGCANCEL and SIGSETXID. GLIBC on Linux requires both signal to proper
> implement both pthread cancellation and set*id function and having
> different semanticsis troublesome (a conformant program on a architecture
> that does not filter out the signals might inadvertently disable pthread
> asynchronous cancellation, set*id synchronization or posix timers).
>
> Also, sigfillset removes SIGCANCEL and SIGSETXID as expected, but
> sigaddset and sigdelset does not handle none of internal signals. I also
> think we should ignore internal nptl signals on sigaddset and sigdelset.
>
> And for this specific case I don't see adding compat symbols to keep
> the old semantic for the related architectures the best approach. There
> is a canonical way to actually disable pthread cancellation and masking
> SIGSETXID would make set*id non POSIX conformant.
>
> What about the following?
I suspected that sigprocmask is buggy and should be fixed as you
suggested here. Now after your explanation I'm convinced with it. But
your patch changes user interface to glibc which may break existing
software.
The most conservative way to proceed with it is to leave the existing
behavior for affected platforms as is. For software compiled against
glibc-2.27 or newer we can use versioning to wire sigprocmask to
__new_sigprocmask, which would emit warning for x86 and others, and
clear internal signals if they appear.
But I'm not familiar with nptl, and if you think that silent API
change will not hurt users, I'm OK with your patch as is. In this
case I would only ask you to add notes about this changes to NEWS,
and especially about alpha as it is switched to new syscall.
Yury
> ---
>
> This patch consolidates the sigprocmask Linux syscall implementation on
> sysdeps/unix/sysv/linux/sigprocmask.c. The changes are:
>
> 1. For ia64, s390-64, sparc64, and x86_64 the default semantic for
> filter out SIGCANCEL and SIGSETXID is used. Also the Linux pthread
> semantic is documented in the signal chapter.
>
> 2. A new internal function to check for NPTL internal signals within a
> signal set is added (__nptl_has_internal_signal). It is used to
> simplify the default sigprocmask.c implementation.
>
> Checked on x86_64-linux-gnu.
>
> * manual/signal.texi: Add a note about internal pthread signals
> on Linux.
> * sysdeps/unix/sysv/linux/alpha/sigprocmask.c: Remove file.
> * sysdeps/unix/sysv/linux/ia64/sigprocmask.c: Likewise.
> * sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c: Likewise.
> * sysdeps/unix/sysv/linux/x86_64/sigprocmask.c: Likewise.
> * sysdeps/unix/sysv/linux/nptl-signals.h
> (__nptl_has_internal_signal): New function.
> * sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask):
> Use __nptl_has_internal_signal and __nptl_clear_internal_signals
> function.
>
> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> ---
>
> diff --git a/manual/signal.texi b/manual/signal.texi
> index 9577ff0..46696af 100644
> --- a/manual/signal.texi
> +++ b/manual/signal.texi
> @@ -2461,6 +2461,11 @@ well. (In addition, it's not wise to put into your program an
> assumption that the system has no signals aside from the ones you know
> about.)
>
> +On Linux pthreads internally uses some signals to implement asynchronous
> +cancellation, effective ID synchronization for POSIX conformance, and
> +posix timer management. These signals are filtered out on signal set
> +function manipulation.
> +
> @deftypefun int sigemptyset (sigset_t *@var{set})
> @standards{POSIX.1, signal.h}
> @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> diff --git a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c b/sysdeps/unix/sysv/linux/alpha/sigprocmask.c
> deleted file mode 100644
> index ebec70c..0000000
> --- a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -/* Copyright (C) 1993-2017 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> - Contributed by David Mosberger (davidm@azstarnet.com).
> -
> - 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
> - <http://www.gnu.org/licenses/>. */
> -
> -#include <errno.h>
> -#include <sysdep.h>
> -#include <signal.h>
> -
> -/* When there is kernel support for more than 64 signals, we'll have to
> - switch to a new system call convention here. */
> -
> -int
> -__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
> -{
> - unsigned long int setval;
> - long result;
> -
> - if (set)
> - setval = set->__val[0];
> - else
> - {
> - setval = 0;
> - how = SIG_BLOCK; /* ensure blocked mask doesn't get changed */
> - }
> -
> - result = INLINE_SYSCALL (osf_sigprocmask, 2, how, setval);
> - if (result == -1)
> - /* If there are ever more than 63 signals, we need to recode this
> - in assembler since we wouldn't be able to distinguish a mask of
> - all 1s from -1, but for now, we're doing just fine... */
> - return result;
> -
> - if (oset)
> - {
> - oset->__val[0] = result;
> - result = _SIGSET_NWORDS;
> - while (--result > 0)
> - oset->__val[result] = 0;
> - }
> - return 0;
> -}
> -
> -libc_hidden_def (__sigprocmask)
> -weak_alias (__sigprocmask, sigprocmask);
> diff --git a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c
> deleted file mode 100644
> index 920c5fd..0000000
> --- a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -/* Copyright (C) 1997-2017 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> - Linux/IA64 specific sigprocmask
> - Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999.
> -
> - 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
> - <http://www.gnu.org/licenses/>. */
> -
> -/* Linux/ia64 only has rt signals, thus we do not even want to try falling
> - back to the old style signals as the default Linux handler does. */
> -
> -#include <errno.h>
> -#include <signal.h>
> -#include <unistd.h>
> -
> -#include <sysdep.h>
> -#include <sys/syscall.h>
> -
> -/* Get and/or change the set of blocked signals. */
> -int
> -__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
> -{
> -
> - /* XXX The size argument hopefully will have to be changed to the
> - real size of the user-level sigset_t. */
> - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
> -}
> -libc_hidden_def (__sigprocmask)
> -weak_alias (__sigprocmask, sigprocmask)
> diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h
> index f30c597..6afd3fe 100644
> --- a/sysdeps/unix/sysv/linux/nptl-signals.h
> +++ b/sysdeps/unix/sysv/linux/nptl-signals.h
> @@ -33,6 +33,12 @@
> #define SIGSETXID (__SIGRTMIN + 1)
>
>
> +static inline bool
> +__nptl_has_internal_signal (const sigset_t *set)
> +{
> + return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID);
> +}
> +
> /* Return is sig is used internally. */
> static inline int
> __nptl_is_internal_signal (int sig)
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
> deleted file mode 100644
> index a8010e7..0000000
> --- a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -/* Copyright (C) 2001-2017 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
> - <http://www.gnu.org/licenses/>. */
> -
> -/* 64 bit Linux for S/390 only has rt signals, thus we do not even want to try
> - falling back to the old style signals as the default Linux handler does. */
> -
> -#include <errno.h>
> -#include <signal.h>
> -#include <unistd.h>
> -
> -#include <sysdep.h>
> -#include <sys/syscall.h>
> -
> -/* Get and/or change the set of blocked signals. */
> -int
> -__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
> -{
> -
> - /* XXX The size argument hopefully will have to be changed to the
> - real size of the user-level sigset_t. */
> - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
> -}
> -libc_hidden_def (__sigprocmask)
> -weak_alias (__sigprocmask, sigprocmask)
> diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c
> index e776563..d14fc5c 100644
> --- a/sysdeps/unix/sysv/linux/sigprocmask.c
> +++ b/sysdeps/unix/sysv/linux/sigprocmask.c
> @@ -1,4 +1,5 @@
> -/* Copyright (C) 1997-2017 Free Software Foundation, Inc.
> +/* Get and/or change the set of blocked signals. Linux version.
> + Copyright (C) 1997-2017 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
> @@ -17,34 +18,22 @@
>
> #include <errno.h>
> #include <signal.h>
> -#include <string.h> /* Needed for string function builtin redirection. */
> -#include <unistd.h>
> +#include <nptl-signals.h>
>
> -#include <sysdep.h>
> -#include <sys/syscall.h>
>
> -#include <nptl/pthreadP.h> /* SIGCANCEL, SIGSETXID */
> -
> -
> -/* Get and/or change the set of blocked signals. */
> 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
> - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
> - || __builtin_expect (__sigismember (set, SIGSETXID), 0)))
> + if (set != NULL && __glibc_unlikely (__nptl_has_internal_signal (set)))
> {
> local_newmask = *set;
> - __sigdelset (&local_newmask, SIGCANCEL);
> - __sigdelset (&local_newmask, SIGSETXID);
> + __nptl_clear_internal_signals (&local_newmask);
> set = &local_newmask;
> }
>
> - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
> + return INLINE_SYSCALL_CALL (rt_sigprocmask, how, set, oset, _NSIG / 8);
> }
> libc_hidden_def (__sigprocmask)
> weak_alias (__sigprocmask, sigprocmask)
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
> deleted file mode 100644
> index ef7d7fe..0000000
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
> +++ /dev/null
> @@ -1,34 +0,0 @@
> -/* Copyright (C) 1997-2017 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
> - <http://www.gnu.org/licenses/>. */
> -
> -#include <errno.h>
> -#include <signal.h>
> -#include <unistd.h>
> -
> -#include <sysdep.h>
> -#include <sys/syscall.h>
> -
> -/* Get and/or change the set of blocked signals. */
> -int
> -__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
> -{
> - /* XXX The size argument hopefully will have to be changed to the
> - real size of the user-level sigset_t. */
> - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
> -}
> -libc_hidden_def (__sigprocmask)
> -weak_alias (__sigprocmask, sigprocmask)
> diff --git a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
> deleted file mode 100644
> index 1610ddf..0000000
> --- a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -/* Copyright (C) 1997-2017 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> - Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999.
> -
> - 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
> - <http://www.gnu.org/licenses/>. */
> -
> -/* Linux/x86_64 only has rt signals, thus we do not even want to try falling
> - back to the old style signals as the default Linux handler does. */
> -
> -#include <errno.h>
> -#include <signal.h>
> -#include <unistd.h>
> -
> -#include <sysdep.h>
> -#include <sys/syscall.h>
> -
> -/* Get and/or change the set of blocked signals. */
> -int
> -__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
> -{
> -
> - /* XXX The size argument hopefully will have to be changed to the
> - real size of the user-level sigset_t. */
> - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8);
> -}
> -libc_hidden_def (__sigprocmask)
> -weak_alias (__sigprocmask, sigprocmask)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Consolidate Linux sigprocmask() implementation
2017-10-31 22:58 ` Yury Norov
@ 2017-11-03 11:19 ` Adhemerval Zanella
0 siblings, 0 replies; 11+ messages in thread
From: Adhemerval Zanella @ 2017-11-03 11:19 UTC (permalink / raw)
To: Yury Norov; +Cc: libc-alpha
On 31/10/2017 20:58, Yury Norov wrote:
> Hi Adhemerval!
>
> On Tue, Oct 31, 2017 at 07:19:01PM -0200, Adhemerval Zanella wrote:
>>
>>
>> On 16/10/2017 02:34, Yury Norov wrote:
>>> ia64, s390-64, sparc64, x86_64 and alpha ports has their own
>>> implementations of sigprocmask(). They all but alpha do exactly
>>> what generic sigprocmask() except the check and clear SIGCANCEL
>>> and SIGSETXID flags.
>>>
>>> In this patch, the NEED_CLEAR_SIGCANCEL_SIGSETXID option is
>>> introduced and disabled for that ports which allows to swith
>>> them to generic implementation.
>>
>> Although the manual do not state the Linux implementation detail I think
>> all supported Linux architecture should have the same semantic regarding
>> SIGCANCEL and SIGSETXID. GLIBC on Linux requires both signal to proper
>> implement both pthread cancellation and set*id function and having
>> different semanticsis troublesome (a conformant program on a architecture
>> that does not filter out the signals might inadvertently disable pthread
>> asynchronous cancellation, set*id synchronization or posix timers).
>>
>> Also, sigfillset removes SIGCANCEL and SIGSETXID as expected, but
>> sigaddset and sigdelset does not handle none of internal signals. I also
>> think we should ignore internal nptl signals on sigaddset and sigdelset.
>>
>> And for this specific case I don't see adding compat symbols to keep
>> the old semantic for the related architectures the best approach. There
>> is a canonical way to actually disable pthread cancellation and masking
>> SIGSETXID would make set*id non POSIX conformant.
>>
>> What about the following?
>
> I suspected that sigprocmask is buggy and should be fixed as you
> suggested here. Now after your explanation I'm convinced with it. But
> your patch changes user interface to glibc which may break existing
> software.
>
> The most conservative way to proceed with it is to leave the existing
> behavior for affected platforms as is. For software compiled against
> glibc-2.27 or newer we can use versioning to wire sigprocmask to
> __new_sigprocmask, which would emit warning for x86 and others, and
> clear internal signals if they appear.
>
> But I'm not familiar with nptl, and if you think that silent API
> change will not hurt users, I'm OK with your patch as is. In this
> case I would only ask you to add notes about this changes to NEWS,
> and especially about alpha as it is switched to new syscall.
>
> Yury
I think the main problem of providing a compat symbol is besides
interfering with both pthread cancellation and posix timers (a explicit
conformance break), not filtering out NPTL internal signals for set*id
programs might be a security issue where the user/group id is not
synchronized over the threads as expected by a POSIX standard.
I have opened BZ#22391 [1] to track this issue. I am also preparing
a patch set to fix this over the signal implementations.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=22391
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-11-03 11:19 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 4:34 [PATCH 0/3] Cleanup and consolidate sigprocmask() for Linux Yury Norov
2017-10-16 4:34 ` [PATCH 1/3] Remove useless #ifdefs from sysdeps/unix/sysv/linux/sigprocmask.c Yury Norov
2017-10-16 9:04 ` Andreas Schwab
2017-10-20 15:38 ` Yury Norov
2017-10-23 8:07 ` Andreas Schwab
2017-10-31 19:54 ` Adhemerval Zanella
2017-10-16 4:35 ` [PATCH 2/3] Consolidate Linux sigprocmask() implementation Yury Norov
2017-10-31 21:19 ` Adhemerval Zanella
2017-10-31 22:58 ` Yury Norov
2017-11-03 11:19 ` Adhemerval Zanella
2017-10-16 4:35 ` [RFC PATCH 3/3] Consolidate Linux sigprocmask() implementationa for alpha Yury Norov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).