public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 3/3] Filter out NPTL internal signals (BZ #22391)
  2017-11-15 17:54 [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
  2017-11-15 17:54 ` [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391) Adhemerval Zanella
@ 2017-11-15 17:54 ` Adhemerval Zanella
  2017-12-12 17:03   ` Adhemerval Zanella
  2017-12-12 17:03 ` [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
  2 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-15 17:54 UTC (permalink / raw)
  To: libc-alpha

This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and
SIGSETXID) from signal functions.  GLIBC on Linux requires both signals to
proper implement pthread cancellation, posix timers, and set*id posix
thread synchronization.

And not filtering out the internal signal is 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.

  - It might also to security issues if SIGSETXID is masked and set*id
    functions are called (some threads might have effective user or group
    id different from the rest).

The changes are basically:

  - Change __is_internal_signal to bool and used on all signal function
    that has a signal number as input.  Also for signal function which accepts
    signals sets (sigset_t) it assumes that canonical function were used to
    add/remove signals which lead to some input simplification.

  - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID.
    It is rewritten to check each signal indidually and to check realtime
    signals using canonical macros.

  - Add generic __clear_internal_signals and __is_internal_signal
    version since both symbols are used on generic implementations.

  - Remove superflous sysdeps/nptl/sigfillset.c.

  - Remove superflous SIGTIMER handling on Linux __is_internal_signal
    since it is the same of SIGCANCEL.

  - Remove dangling define and obvious comment on nptl/sigaction.c.

Checked on x86_64-linux-gnu.

	[BZ #22391]
	* nptl/sigaction.c (__sigaction): Use __is_internal_signal to
	check for internal nptl signals.
	* signal/sigaddset.c (sigaddset): Likewise.
	* signal/sigdelset.c (sigdelset): Likewise.
	* sysdeps/posix/signal.c (__bsd_signal): Likewise.
	* sysdeps/posix/sigset.c (sigset): Call and check sigaddset return
	value.
	* signal/sigfillset.c (sigfillset): User __clear_internal_signals
	to filter out internal nptl signals.
	* signal/tst-sigset.c (do_test): Check ech signal indidually and
	also check realtime signals using standard macros.
	* sysdeps/nptl/nptl-signals.h (__clear_internal_signals,
	__is_internal_signal): New functions.
	* sysdeps/nptl/sigfillset.c: Remove file.
	* sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal):
	Change return to bool.
	(__clear_internal_signals): Remove SIGTIMER clean since it is
	equal to SIGCANEL on Linux.
	* sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume
	signal set was constructed using standard functions.
	* sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Reported-by: Yury Norov <ynorov@caviumnetworks.com>
---
 ChangeLog                                  | 25 ++++++++
 nptl/sigaction.c                           | 14 +----
 signal/sigaction.c                         |  2 +-
 signal/sigaddset.c                         |  5 +-
 signal/sigdelset.c                         |  5 +-
 signal/sigfillset.c                        | 10 +---
 signal/tst-sigset.c                        | 92 ++++++++++++++++++++++--------
 sysdeps/generic/internal-signals.h         | 11 ++++
 sysdeps/nptl/sigfillset.c                  | 20 -------
 sysdeps/posix/signal.c                     |  5 +-
 sysdeps/posix/sigset.c                     | 10 +---
 sysdeps/unix/sysv/linux/internal-signals.h |  4 +-
 sysdeps/unix/sysv/linux/sigtimedwait.c     | 17 +-----
 13 files changed, 124 insertions(+), 96 deletions(-)
 delete mode 100644 sysdeps/nptl/sigfillset.c

diff --git a/nptl/sigaction.c b/nptl/sigaction.c
index 2994fd5..b2ff674 100644
--- a/nptl/sigaction.c
+++ b/nptl/sigaction.c
@@ -16,22 +16,12 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-
-/* This is no complete implementation.  The file is meant to be
-   included in the real implementation to provide the wrapper around
-   __libc_sigaction.  */
-
-#include <nptl/pthreadP.h>
-
-/* We use the libc implementation but we tell it to not allow
-   SIGCANCEL or SIGTIMER to be handled.  */
-#define LIBC_SIGACTION	1
-
+#include <internal-signals.h>
 
 int
 __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
 {
-  if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID))
+  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/signal/sigaction.c b/signal/sigaction.c
index 8a6220c..3025aab 100644
--- a/signal/sigaction.c
+++ b/signal/sigaction.c
@@ -24,7 +24,7 @@
 int
 __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
 {
-  if (sig <= 0 || sig >= NSIG)
+  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/signal/sigaddset.c b/signal/sigaddset.c
index 161be7b..a435f61 100644
--- a/signal/sigaddset.c
+++ b/signal/sigaddset.c
@@ -17,13 +17,14 @@
 
 #include <errno.h>
 #include <signal.h>
-#include <sigsetops.h>
+#include <internal-signals.h>
 
 /* Add SIGNO to SET.  */
 int
 sigaddset (sigset_t *set, int signo)
 {
-  if (set == NULL || signo <= 0 || signo >= NSIG)
+  if (set == NULL || signo <= 0 || signo >= NSIG
+      || __is_internal_signal (signo))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/signal/sigdelset.c b/signal/sigdelset.c
index 2aaa536..01a50ec 100644
--- a/signal/sigdelset.c
+++ b/signal/sigdelset.c
@@ -17,13 +17,14 @@
 
 #include <errno.h>
 #include <signal.h>
-#include <sigsetops.h>
+#include <internal-signals.h>
 
 /* Add SIGNO to SET.  */
 int
 sigdelset (sigset_t *set, int signo)
 {
-  if (set == NULL || signo <= 0 || signo >= NSIG)
+  if (set == NULL || signo <= 0 || signo >= NSIG
+      || __is_internal_signal (signo))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/signal/sigfillset.c b/signal/sigfillset.c
index 0fcc24a..560c66e 100644
--- a/signal/sigfillset.c
+++ b/signal/sigfillset.c
@@ -18,6 +18,7 @@
 #include <errno.h>
 #include <signal.h>
 #include <string.h>
+#include <internal-signals.h>
 
 /* Set all signals in SET.  */
 int
@@ -31,14 +32,7 @@ sigfillset (sigset_t *set)
 
   memset (set, 0xff, sizeof (sigset_t));
 
-  /* If the implementation uses a cancellation signal don't set the bit.  */
-#ifdef SIGCANCEL
-  __sigdelset (set, SIGCANCEL);
-#endif
-  /* Likewise for the signal to implement setxid.  */
-#ifdef SIGSETXID
-  __sigdelset (set, SIGSETXID);
-#endif
+  __clear_internal_signals (set);
 
   return 0;
 }
diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c
index d47adcc..a2b764d 100644
--- a/signal/tst-sigset.c
+++ b/signal/tst-sigset.c
@@ -1,43 +1,85 @@
 /* Test sig*set functions.  */
 
 #include <signal.h>
-#include <stdio.h>
 
-#define TEST_FUNCTION do_test ()
+#include <support/check.h>
+
 static int
 do_test (void)
 {
-  int result = 0;
-  int sig = -1;
+  sigset_t set;
+  TEST_VERIFY (sigemptyset (&set) == 0);
 
-#define TRY(call)							      \
-  if (call)								      \
-    {									      \
-      printf ("%s (sig = %d): %m\n", #call, sig);			      \
-      result = 1;							      \
-    }									      \
-  else
+#define VERIFY(set, sig)			\
+  TEST_VERIFY (sigismember (&set, sig) == 0);	\
+  TEST_VERIFY (sigaddset (&set, sig) == 0);	\
+  TEST_VERIFY (sigismember (&set, sig) != 0);	\
+  TEST_VERIFY (sigdelset (&set, sig) == 0);	\
+  TEST_VERIFY (sigismember (&set, sig) == 0)
 
+  /* ISO C99 signals.  */
+  VERIFY (set, SIGINT);
+  VERIFY (set, SIGILL);
+  VERIFY (set, SIGABRT);
+  VERIFY (set, SIGFPE);
+  VERIFY (set, SIGSEGV);
+  VERIFY (set, SIGTERM);
 
-  sigset_t set;
-  TRY (sigemptyset (&set) != 0);
+  /* Historical signals specified by POSIX. */
+  VERIFY (set, SIGHUP);
+  VERIFY (set, SIGQUIT);
+  VERIFY (set, SIGTRAP);
+  VERIFY (set, SIGKILL);
+  VERIFY (set, SIGBUS);
+  VERIFY (set, SIGSYS);
+  VERIFY (set, SIGPIPE);
+  VERIFY (set, SIGALRM);
+
+  /* New(er) POSIX signals (1003.1-2008, 1003.1-2013).  */
+  VERIFY (set, SIGURG);
+  VERIFY (set, SIGSTOP);
+  VERIFY (set, SIGTSTP);
+  VERIFY (set, SIGCONT);
+  VERIFY (set, SIGCHLD);
+  VERIFY (set, SIGTTIN);
+  VERIFY (set, SIGTTOU);
+  VERIFY (set, SIGPOLL);
+  VERIFY (set, SIGXCPU);
+  VERIFY (set, SIGXFSZ);
+  VERIFY (set, SIGVTALRM);
+  VERIFY (set, SIGPROF);
+  VERIFY (set, SIGUSR1);
+  VERIFY (set, SIGUSR2);
+
+  /* Nonstandard signals found in all modern POSIX systems
+     (including both BSD and Linux).  */
+  VERIFY (set, SIGWINCH);
 
-#ifdef SIGRTMAX
-  int max_sig = SIGRTMAX;
-#else
-  int max_sig = NSIG - 1;
+  /* Arch-specific signals.  */
+#ifdef SIGEMT
+  VERIFY (set, SIGEMT);
+#endif
+#ifdef SIGLOST
+  VERIFY (set, SIGLOST);
+#endif
+#ifdef SIGINFO
+  VERIFY (set, SIGINFO);
+#endif
+#ifdef SIGSTKFLT
+  VERIFY (set, SIGSTKFLT);
+#endif
+#ifdef SIGPWR
+  VERIFY (set, SIGPWR);
 #endif
 
-  for (sig = 1; sig <= max_sig; ++sig)
+  /* Read-time signals (POSIX.1b real-time extensions).  If they are
+     supported SIGRTMAX value is greater than SIGRTMIN.  */
+  for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++)
     {
-      TRY (sigismember (&set, sig) != 0);
-      TRY (sigaddset (&set, sig) != 0);
-      TRY (sigismember (&set, sig) == 0);
-      TRY (sigdelset (&set, sig) != 0);
-      TRY (sigismember (&set, sig) != 0);
+      VERIFY (set, rtsig);
     }
 
-  return result;
+  return 0;
 }
 
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h
index 55bc07d..8a8854c 100644
--- a/sysdeps/generic/internal-signals.h
+++ b/sysdeps/generic/internal-signals.h
@@ -15,3 +15,14 @@
    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/>.  */
+
+static inline void
+__clear_internal_signals (sigset_t *set)
+{
+}
+
+static inline bool
+__is_internal_signal (int sig)
+{
+  return false;
+}
diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c
deleted file mode 100644
index 50e8512..0000000
--- a/sysdeps/nptl/sigfillset.c
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Copyright (C) 2003-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 <nptl/pthreadP.h>
-
-#include <signal/sigfillset.c>
diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c
index 81ba177..87c5d1c 100644
--- a/sysdeps/posix/signal.c
+++ b/sysdeps/posix/signal.c
@@ -18,8 +18,8 @@
 
 #include <errno.h>
 #include <signal.h>
-#include <string.h>	/* For the real memset prototype.  */
 #include <sigsetops.h>
+#include <internal-signals.h>
 
 sigset_t _sigintr attribute_hidden;		/* Set by siginterrupt.  */
 
@@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler)
   struct sigaction act, oact;
 
   /* Check signal extents to protect __sigismember.  */
-  if (handler == SIG_ERR || sig < 1 || sig >= NSIG)
+  if (handler == SIG_ERR || sig < 1 || sig >= NSIG
+      || __is_internal_signal (sig))
     {
       __set_errno (EINVAL);
       return SIG_ERR;
diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c
index a4dfe0a..6234ecf 100644
--- a/sysdeps/posix/sigset.c
+++ b/sysdeps/posix/sigset.c
@@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp)
   sigset_t set;
   sigset_t oset;
 
-  /* Check signal extents to protect __sigismember.  */
-  if (disp == SIG_ERR || sig < 1 || sig >= NSIG)
-    {
-      __set_errno (EINVAL);
-      return SIG_ERR;
-    }
-
   __sigemptyset (&set);
-  __sigaddset (&set, sig);
+  if (sigaddset (&set, sig) < 0)
+    return SIG_ERR;
 
   if (disp == SIG_HOLD)
     {
diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
index bb8234d..bf33bbc 100644
--- a/sysdeps/unix/sysv/linux/internal-signals.h
+++ b/sysdeps/unix/sysv/linux/internal-signals.h
@@ -21,6 +21,8 @@
 
 #include <signal.h>
 #include <sigsetops.h>
+#include <stdbool.h>
+#include <sysdep.h>
 
 /* The signal used for asynchronous cancelation.  */
 #define SIGCANCEL       __SIGRTMIN
@@ -43,7 +45,7 @@ __has_internal_signal (const sigset_t *set)
 }
 
 /* Return is sig is used internally.  */
-static inline int
+static inline bool
 __is_internal_signal (int sig)
 {
   return (sig == SIGCANCEL) || (sig == SIGSETXID);
diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c
index 2d96dd4..b4977e3 100644
--- a/sysdeps/unix/sysv/linux/sigtimedwait.c
+++ b/sysdeps/unix/sysv/linux/sigtimedwait.c
@@ -24,21 +24,8 @@ int
 __sigtimedwait (const sigset_t *set, siginfo_t *info,
 		const struct timespec *timeout)
 {
-  sigset_t tmpset;
-  if (set != NULL
-      && (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
-	  || __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);
-      __sigdelset (&tmpset, SIGSETXID);
-      set = &tmpset;
-    }
-
-    /* XXX The size argument hopefully will have to be changed to the
-       real size of the user-level sigset_t.  */
+  /* XXX The size argument hopefully will have to be changed to the
+     real size of the user-level sigset_t.  */
   int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8);
 
   /* The kernel generates a SI_TKILL code in si_code in case tkill is
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h
@ 2017-11-15 17:54 Adhemerval Zanella
  2017-11-15 17:54 ` [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391) Adhemerval Zanella
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-15 17:54 UTC (permalink / raw)
  To: libc-alpha

Changes from previous version:

  - Cleanup __nptl prefix from the generic names.

  - Remove superflous SIGTERM handling on Linux __is_internal_signal
    and __clear_internal_signals.

---

This patch renames the nptl-signals.h header to internal-signals.h.
On Linux the definitions and functions are not only NPTL related, but
used for other POSIX definitions as well (for instance SIGTIMER for
posix times, SIGSETXID for id functions, and signal block/restore
helpers) and since generic functions will be places and used in generic
implementation it makes more sense to decouple it from NPTL.

Checked on x86_64-linux-gnu.

	* sysdeps/nptl/nptl-signals.h: Move to ...
	* sysdeps/generic/internal-signals.h: ... here.  Adjust internal
	comments.
	* sysdeps/unix/sysv/linux/internal-signals.h: Add include guards.
	(__nptl_is_internal_signal): Rename to __is_internal_signal.
	(__nptl_clear_internal_signals): Rename to __clear_internal_signals.
	* sysdeps/unix/sysv/linux/raise.c: Adjust nptl-signal.h to
	include-signals.h rename.
	* nptl/pthreadP.h: Likewise.
	* sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Call
	__is_internal_signal instead of __nptl_is_internal_signal.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog                                                | 12 ++++++++++++
 nptl/pthreadP.h                                          |  2 +-
 .../{nptl/nptl-signals.h => generic/internal-signals.h}  |  7 +------
 .../sysv/linux/{nptl-signals.h => internal-signals.h}    | 16 ++++++++++------
 sysdeps/unix/sysv/linux/raise.c                          |  2 +-
 sysdeps/unix/sysv/linux/spawni.c                         |  2 +-
 6 files changed, 26 insertions(+), 15 deletions(-)
 rename sysdeps/{nptl/nptl-signals.h => generic/internal-signals.h} (74%)
 rename sysdeps/unix/sysv/linux/{nptl-signals.h => internal-signals.h} (89%)

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 1cc80b6..f3ec832 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -32,7 +32,7 @@
 #include <atomic.h>
 #include <kernel-features.h>
 #include <errno.h>
-#include <nptl-signals.h>
+#include <internal-signals.h>
 
 
 /* Atomic operations on TLS memory.  */
diff --git a/sysdeps/nptl/nptl-signals.h b/sysdeps/generic/internal-signals.h
similarity index 74%
rename from sysdeps/nptl/nptl-signals.h
rename to sysdeps/generic/internal-signals.h
index 298acf2..55bc07d 100644
--- a/sysdeps/nptl/nptl-signals.h
+++ b/sysdeps/generic/internal-signals.h
@@ -1,4 +1,4 @@
-/* Special use of signals in NPTL internals.  Stub version.
+/* Special use of signals internally.  Stub version.
    Copyright (C) 2014-2017 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -15,8 +15,3 @@
    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/>.  */
-
-/* This file can define the macros SIGCANCEL, SIGTIMER, and SIGSETXID to
-   signal numbers reserved by libpthread for those internal purposes.
-
-   Note that some code presumes SIGTIMER is the same as SIGCANCEL.  */
diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
similarity index 89%
rename from sysdeps/unix/sysv/linux/nptl-signals.h
rename to sysdeps/unix/sysv/linux/internal-signals.h
index f30c597..e96a718 100644
--- a/sysdeps/unix/sysv/linux/nptl-signals.h
+++ b/sysdeps/unix/sysv/linux/internal-signals.h
@@ -1,4 +1,4 @@
-/* Special use of signals in NPTL internals.  Linux version.
+/* Special use of signals internally.  Linux version.
    Copyright (C) 2014-2017 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -16,6 +16,9 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef __INTERNAL_SIGNALS_H
+# define __INTERNAL_SIGNALS_H
+
 #include <signal.h>
 #include <sigsetops.h>
 
@@ -35,17 +38,16 @@
 
 /* Return is sig is used internally.  */
 static inline int
-__nptl_is_internal_signal (int sig)
+__is_internal_signal (int sig)
 {
-  return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
+  return (sig == SIGCANCEL) || (sig == SIGSETXID);
 }
 
 /* Remove internal glibc signal from the mask.  */
 static inline void
-__nptl_clear_internal_signals (sigset_t *set)
+__clear_internal_signals (sigset_t *set)
 {
   __sigdelset (set, SIGCANCEL);
-  __sigdelset (set, SIGTIMER);
   __sigdelset (set, SIGSETXID);
 }
 
@@ -66,7 +68,7 @@ static inline int
 __libc_signal_block_app (sigset_t *set)
 {
   sigset_t allset = SIGALL_SET;
-  __nptl_clear_internal_signals (&allset);
+  __clear_internal_signals (&allset);
   INTERNAL_SYSCALL_DECL (err);
   return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,
 			   _NSIG / 8);
@@ -83,3 +85,5 @@ __libc_signal_restore_set (const sigset_t *set)
 
 /* Used to communicate with signal handler.  */
 extern struct xid_command *__xidcmd attribute_hidden;
+
+#endif
diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c
index 70e5a98..1167950 100644
--- a/sysdeps/unix/sysv/linux/raise.c
+++ b/sysdeps/unix/sysv/linux/raise.c
@@ -21,7 +21,7 @@
 #include <errno.h>
 #include <sys/types.h>
 #include <unistd.h>
-#include <nptl-signals.h>
+#include <internal-signals.h>
 
 int
 raise (int sig)
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index fb83c2e..af464b5 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -144,7 +144,7 @@ __spawni_child (void *arguments)
 	}
       else if (sigismember (&hset, sig))
 	{
-	  if (__nptl_is_internal_signal (sig))
+	  if (__is_internal_signal (sig))
 	    sa.sa_handler = SIG_IGN;
 	  else
 	    {
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391)
  2017-11-15 17:54 [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
@ 2017-11-15 17:54 ` Adhemerval Zanella
  2017-11-16  1:10   ` Rical Jasan
  2017-12-12 17:03   ` Adhemerval Zanella
  2017-11-15 17:54 ` [PATCH v3 3/3] Filter out NPTL internal signals " Adhemerval Zanella
  2017-12-12 17:03 ` [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
  2 siblings, 2 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-15 17:54 UTC (permalink / raw)
  To: libc-alpha; +Cc: Zack Weinberg

Changes from previous version:

  - Cleanup __nptl prefix from the generic names.

---

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 (__has_internal_signal).  It is used to
     simplify the default sigprocmask.c implementation.

Checked on x86_64-linux-gnu.

	[BZ #22391]
	* 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/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/internal-signals.h
	(__has_internal_signal): New function.
	* sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask):
	Use __has_internal_signal and __clear_internal_signals
	function.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Signed-off-by: Zack Weinberg <zackw@panix.com>
Reported-by: Yury Norov <ynorov@caviumnetworks.com>
---
 ChangeLog                                          | 17 +++++++
 manual/signal.texi                                 | 37 ++++++++++++++
 sysdeps/unix/sysv/linux/alpha/sigprocmask.c        | 58 ----------------------
 sysdeps/unix/sysv/linux/ia64/sigprocmask.c         | 40 ---------------
 sysdeps/unix/sysv/linux/internal-signals.h         |  6 +++
 sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c | 38 --------------
 sysdeps/unix/sysv/linux/sigprocmask.c              | 23 +++------
 .../unix/sysv/linux/sparc/sparc64/sigprocmask.c    | 34 -------------
 sysdeps/unix/sysv/linux/x86_64/sigprocmask.c       | 39 ---------------
 9 files changed, 66 insertions(+), 226 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/alpha/sigprocmask.c
 delete mode 100644 sysdeps/unix/sysv/linux/ia64/sigprocmask.c
 delete mode 100644 sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
 delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/sigprocmask.c

diff --git a/manual/signal.texi b/manual/signal.texi
index 9577ff0..4f0ef59 100644
--- a/manual/signal.texi
+++ b/manual/signal.texi
@@ -235,6 +235,7 @@ defined.  Since the signal numbers are allocated consecutively,
 * Job Control Signals::         Signals used to support job control.
 * Operation Error Signals::     Used to report operational system errors.
 * Miscellaneous Signals::       Miscellaneous Signals.
+* Internally-Used Signals::     Signals used internally by the C library.
 * Signal Messages::             Printing a message describing a signal.
 @end menu
 
@@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
 The default action is to terminate the process.
 @end deftypevr
 
+@deftypevr Macro int SIGRTMIN
+@deftypevrx Macro int SIGRTMAX
+@standards{POSIX.1, signal.h}
+@cindex real-time signals
+The range of signal numbers @code{SIGRTMIN}, @code{SIGRTMIN+1},
+@dots{}, @code{SIGRTMAX} is also set aside for you to use any way you
+want.  In addition, these signals (and no others) are guaranteed to
+support @dfn{real-time} signal semantics, which unfortunately this
+manual does not yet document.
+
+Unlike all of the other signal number macros, @code{SIGRTMIN} and
+@code{SIGRTMAX} are not compile-time constants, because some operating
+systems make the number of real-time signals tunable on a
+per-installation or even per-process basis.  However, POSIX guarantees
+that there will be at least 8 signal numbers in this range.
+
+The default action for all signals in this range is to terminate the
+process.
+@end deftypevr
+
 @deftypevr Macro int SIGWINCH
 @standards{BSD, signal.h}
 Window size change.  This is generated on some systems (including GNU)
@@ -817,6 +838,22 @@ to print some status information about the system and what the process
 is doing.  Otherwise the default is to do nothing.
 @end deftypevr
 
+@node Internally-Used Signals
+@subsection Internally-Used Signals
+@cindex internally used signals
+
+On some operating systems, @theglibc{} needs to use a few signals from
+the ``true'' real-time range internally, to implement thread
+cancellation, cross-thread effective ID synchronization, POSIX timer
+management, etc.  @Theglibc{} adjusts @code{SIGRTMIN} and
+@code{SIGRTMAX} to exclude these signals, and it also takes steps to
+prevent application code from altering their state: @code{sigprocmask}
+cannot block them and @code{sigaction} cannot redefine their handlers,
+for instance.  Therefore, most programs do not need to know or care
+about these signals.  We mainly document their existence for the sake
+of anyone who has ever wondered why there is a gap between the
+highest-numbered ``normal'' signal and @code{SIGRTMIN} on Linux.
+
 @node Signal Messages
 @subsection Signal Messages
 @cindex signal messages
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/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
index e96a718..bb8234d 100644
--- a/sysdeps/unix/sysv/linux/internal-signals.h
+++ b/sysdeps/unix/sysv/linux/internal-signals.h
@@ -36,6 +36,12 @@
 #define SIGSETXID       (__SIGRTMIN + 1)
 
 
+static inline bool
+__has_internal_signal (const sigset_t *set)
+{
+  return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID);
+}
+
 /* Return is sig is used internally.  */
 static inline int
 __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..e574b5d 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 <internal-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 (__has_internal_signal (set)))
     {
       local_newmask = *set;
-      __sigdelset (&local_newmask, SIGCANCEL);
-      __sigdelset (&local_newmask, SIGSETXID);
+      __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)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391)
  2017-11-15 17:54 ` [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391) Adhemerval Zanella
@ 2017-11-16  1:10   ` Rical Jasan
  2017-11-16 10:36     ` Adhemerval Zanella
  2017-12-12 17:03   ` Adhemerval Zanella
  1 sibling, 1 reply; 15+ messages in thread
From: Rical Jasan @ 2017-11-16  1:10 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Zack Weinberg

Just one final nit.

On 11/15/2017 09:54 AM, Adhemerval Zanella wrote:
> diff --git a/manual/signal.texi b/manual/signal.texi
> index 9577ff0..4f0ef59 100644
> --- a/manual/signal.texi
> +++ b/manual/signal.texi
> @@ -235,6 +235,7 @@ defined.  Since the signal numbers are allocated consecutively,
>  * Job Control Signals::         Signals used to support job control.
>  * Operation Error Signals::     Used to report operational system errors.
>  * Miscellaneous Signals::       Miscellaneous Signals.
> +* Internally-Used Signals::     Signals used internally by the C library.
>  * Signal Messages::             Printing a message describing a signal.
>  @end menu
>  
> @@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
>  The default action is to terminate the process.
>  @end deftypevr
>  
> +@deftypevr Macro int SIGRTMIN
> +@deftypevrx Macro int SIGRTMAX
> +@standards{POSIX.1, signal.h}
> +@cindex real-time signals

This @cindex should go above the @deftypevr to ensure the reader lands
above the heading.  Much of the time the difference probably won't be
noticeable, but it's annoying when you run into that off-chance of it
being on some weird boundary.  The Texinfo manual does mention keeping
index entries above "visible material" [1].

> +The range of signal numbers @code{SIGRTMIN}, @code{SIGRTMIN+1},
> +@dots{}, @code{SIGRTMAX} is also set aside for you to use any way you
> +want.  In addition, these signals (and no others) are guaranteed to
> +support @dfn{real-time} signal semantics, which unfortunately this
> +manual does not yet document.
> +
> +Unlike all of the other signal number macros, @code{SIGRTMIN} and
> +@code{SIGRTMAX} are not compile-time constants, because some operating
> +systems make the number of real-time signals tunable on a
> +per-installation or even per-process basis.  However, POSIX guarantees
> +that there will be at least 8 signal numbers in this range.
> +
> +The default action for all signals in this range is to terminate the
> +process.
> +@end deftypevr
> +
>  @deftypevr Macro int SIGWINCH
>  @standards{BSD, signal.h}
>  Window size change.  This is generated on some systems (including GNU)
> @@ -817,6 +838,22 @@ to print some status information about the system and what the process
>  is doing.  Otherwise the default is to do nothing.
>  @end deftypevr
>  
> +@node Internally-Used Signals
> +@subsection Internally-Used Signals
> +@cindex internally used signals

This @cindex, however, must be underneath, as it won't be associated
with the correct section otherwise.

> +
> +On some operating systems, @theglibc{} needs to use a few signals from
> +the ``true'' real-time range internally, to implement thread
> +cancellation, cross-thread effective ID synchronization, POSIX timer
> +management, etc.  @Theglibc{} adjusts @code{SIGRTMIN} and
> +@code{SIGRTMAX} to exclude these signals, and it also takes steps to
> +prevent application code from altering their state: @code{sigprocmask}
> +cannot block them and @code{sigaction} cannot redefine their handlers,
> +for instance.  Therefore, most programs do not need to know or care
> +about these signals.  We mainly document their existence for the sake
> +of anyone who has ever wondered why there is a gap between the
> +highest-numbered ``normal'' signal and @code{SIGRTMIN} on Linux.
> +
>  @node Signal Messages
>  @subsection Signal Messages
>  @cindex signal messages


Rical

[1]
https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Indexing-Commands.html#Indexing-Commands

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391)
  2017-11-16  1:10   ` Rical Jasan
@ 2017-11-16 10:36     ` Adhemerval Zanella
  2017-11-16 12:11       ` Rical Jasan
  0 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-16 10:36 UTC (permalink / raw)
  To: Rical Jasan; +Cc: libc-alpha, Zack Weinberg



On 15/11/2017 23:16, Rical Jasan wrote:
> Just one final nit.
> 
> On 11/15/2017 09:54 AM, Adhemerval Zanella wrote:
>> diff --git a/manual/signal.texi b/manual/signal.texi
>> index 9577ff0..4f0ef59 100644
>> --- a/manual/signal.texi
>> +++ b/manual/signal.texi
>> @@ -235,6 +235,7 @@ defined.  Since the signal numbers are allocated consecutively,
>>  * Job Control Signals::         Signals used to support job control.
>>  * Operation Error Signals::     Used to report operational system errors.
>>  * Miscellaneous Signals::       Miscellaneous Signals.
>> +* Internally-Used Signals::     Signals used internally by the C library.
>>  * Signal Messages::             Printing a message describing a signal.
>>  @end menu
>>  
>> @@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
>>  The default action is to terminate the process.
>>  @end deftypevr
>>  
>> +@deftypevr Macro int SIGRTMIN
>> +@deftypevrx Macro int SIGRTMAX
>> +@standards{POSIX.1, signal.h}
>> +@cindex real-time signals
> 
> This @cindex should go above the @deftypevr to ensure the reader lands
> above the heading.  Much of the time the difference probably won't be
> noticeable, but it's annoying when you run into that off-chance of it
> being on some weird boundary.  The Texinfo manual does mention keeping
> index entries above "visible material" [1].

Ack, I have changed it locally.

> 
>> +The range of signal numbers @code{SIGRTMIN}, @code{SIGRTMIN+1},
>> +@dots{}, @code{SIGRTMAX} is also set aside for you to use any way you
>> +want.  In addition, these signals (and no others) are guaranteed to
>> +support @dfn{real-time} signal semantics, which unfortunately this
>> +manual does not yet document.
>> +
>> +Unlike all of the other signal number macros, @code{SIGRTMIN} and
>> +@code{SIGRTMAX} are not compile-time constants, because some operating
>> +systems make the number of real-time signals tunable on a
>> +per-installation or even per-process basis.  However, POSIX guarantees
>> +that there will be at least 8 signal numbers in this range.
>> +
>> +The default action for all signals in this range is to terminate the
>> +process.
>> +@end deftypevr
>> +
>>  @deftypevr Macro int SIGWINCH
>>  @standards{BSD, signal.h}
>>  Window size change.  This is generated on some systems (including GNU)
>> @@ -817,6 +838,22 @@ to print some status information about the system and what the process
>>  is doing.  Otherwise the default is to do nothing.
>>  @end deftypevr
>>  
>> +@node Internally-Used Signals
>> +@subsection Internally-Used Signals
>> +@cindex internally used signals
> 
> This @cindex, however, must be underneath, as it won't be associated
> with the correct section otherwise.

Right, so I presume it is on right position, correct?

> 
>> +
>> +On some operating systems, @theglibc{} needs to use a few signals from
>> +the ``true'' real-time range internally, to implement thread
>> +cancellation, cross-thread effective ID synchronization, POSIX timer
>> +management, etc.  @Theglibc{} adjusts @code{SIGRTMIN} and
>> +@code{SIGRTMAX} to exclude these signals, and it also takes steps to
>> +prevent application code from altering their state: @code{sigprocmask}
>> +cannot block them and @code{sigaction} cannot redefine their handlers,
>> +for instance.  Therefore, most programs do not need to know or care
>> +about these signals.  We mainly document their existence for the sake
>> +of anyone who has ever wondered why there is a gap between the
>> +highest-numbered ``normal'' signal and @code{SIGRTMIN} on Linux.
>> +
>>  @node Signal Messages
>>  @subsection Signal Messages
>>  @cindex signal messages
> 
> 
> Rical
> 
> [1]
> https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Indexing-Commands.html#Indexing-Commands
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391)
  2017-11-16 10:36     ` Adhemerval Zanella
@ 2017-11-16 12:11       ` Rical Jasan
  0 siblings, 0 replies; 15+ messages in thread
From: Rical Jasan @ 2017-11-16 12:11 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Zack Weinberg

On 11/16/2017 02:36 AM, Adhemerval Zanella wrote:
> On 15/11/2017 23:16, Rical Jasan wrote:
>> Just one final nit.
>>
>> On 11/15/2017 09:54 AM, Adhemerval Zanella wrote:
>>> diff --git a/manual/signal.texi b/manual/signal.texi
>>> index 9577ff0..4f0ef59 100644
>>> --- a/manual/signal.texi
>>> +++ b/manual/signal.texi
...
>>> @@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
>>>  The default action is to terminate the process.
>>>  @end deftypevr
>>>  
>>> +@deftypevr Macro int SIGRTMIN
>>> +@deftypevrx Macro int SIGRTMAX
>>> +@standards{POSIX.1, signal.h}
>>> +@cindex real-time signals
>>
>> This @cindex should go above the @deftypevr to ensure the reader lands
>> above the heading.  Much of the time the difference probably won't be
>> noticeable, but it's annoying when you run into that off-chance of it
>> being on some weird boundary.  The Texinfo manual does mention keeping
>> index entries above "visible material" [1].
> 
> Ack, I have changed it locally.
...
>>>  
>>> +@node Internally-Used Signals
>>> +@subsection Internally-Used Signals
>>> +@cindex internally used signals
>>
>> This @cindex, however, must be underneath, as it won't be associated
>> with the correct section otherwise.
> 
> Right, so I presume it is on right position, correct?
Correct.  I just felt a need to point out that this was an exception to
the rule above since I was being nit-picky.  :)

Rical

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h
  2017-11-15 17:54 [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
  2017-11-15 17:54 ` [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391) Adhemerval Zanella
  2017-11-15 17:54 ` [PATCH v3 3/3] Filter out NPTL internal signals " Adhemerval Zanella
@ 2017-12-12 17:03 ` Adhemerval Zanella
  2018-01-01 21:56   ` Adhemerval Zanella
  2 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2017-12-12 17:03 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 15/11/2017 15:54, Adhemerval Zanella wrote:
> Changes from previous version:
> 
>   - Cleanup __nptl prefix from the generic names.
> 
>   - Remove superflous SIGTERM handling on Linux __is_internal_signal
>     and __clear_internal_signals.
> 
> ---
> 
> This patch renames the nptl-signals.h header to internal-signals.h.
> On Linux the definitions and functions are not only NPTL related, but
> used for other POSIX definitions as well (for instance SIGTIMER for
> posix times, SIGSETXID for id functions, and signal block/restore
> helpers) and since generic functions will be places and used in generic
> implementation it makes more sense to decouple it from NPTL.
> 
> Checked on x86_64-linux-gnu.
> 
> 	* sysdeps/nptl/nptl-signals.h: Move to ...
> 	* sysdeps/generic/internal-signals.h: ... here.  Adjust internal
> 	comments.
> 	* sysdeps/unix/sysv/linux/internal-signals.h: Add include guards.
> 	(__nptl_is_internal_signal): Rename to __is_internal_signal.
> 	(__nptl_clear_internal_signals): Rename to __clear_internal_signals.
> 	* sysdeps/unix/sysv/linux/raise.c: Adjust nptl-signal.h to
> 	include-signals.h rename.
> 	* nptl/pthreadP.h: Likewise.
> 	* sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Call
> 	__is_internal_signal instead of __nptl_is_internal_signal.
> 
> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
>  ChangeLog                                                | 12 ++++++++++++
>  nptl/pthreadP.h                                          |  2 +-
>  .../{nptl/nptl-signals.h => generic/internal-signals.h}  |  7 +------
>  .../sysv/linux/{nptl-signals.h => internal-signals.h}    | 16 ++++++++++------
>  sysdeps/unix/sysv/linux/raise.c                          |  2 +-
>  sysdeps/unix/sysv/linux/spawni.c                         |  2 +-
>  6 files changed, 26 insertions(+), 15 deletions(-)
>  rename sysdeps/{nptl/nptl-signals.h => generic/internal-signals.h} (74%)
>  rename sysdeps/unix/sysv/linux/{nptl-signals.h => internal-signals.h} (89%)
> 
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 1cc80b6..f3ec832 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -32,7 +32,7 @@
>  #include <atomic.h>
>  #include <kernel-features.h>
>  #include <errno.h>
> -#include <nptl-signals.h>
> +#include <internal-signals.h>
>  
>  
>  /* Atomic operations on TLS memory.  */
> diff --git a/sysdeps/nptl/nptl-signals.h b/sysdeps/generic/internal-signals.h
> similarity index 74%
> rename from sysdeps/nptl/nptl-signals.h
> rename to sysdeps/generic/internal-signals.h
> index 298acf2..55bc07d 100644
> --- a/sysdeps/nptl/nptl-signals.h
> +++ b/sysdeps/generic/internal-signals.h
> @@ -1,4 +1,4 @@
> -/* Special use of signals in NPTL internals.  Stub version.
> +/* Special use of signals internally.  Stub version.
>     Copyright (C) 2014-2017 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> @@ -15,8 +15,3 @@
>     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/>.  */
> -
> -/* This file can define the macros SIGCANCEL, SIGTIMER, and SIGSETXID to
> -   signal numbers reserved by libpthread for those internal purposes.
> -
> -   Note that some code presumes SIGTIMER is the same as SIGCANCEL.  */
> diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
> similarity index 89%
> rename from sysdeps/unix/sysv/linux/nptl-signals.h
> rename to sysdeps/unix/sysv/linux/internal-signals.h
> index f30c597..e96a718 100644
> --- a/sysdeps/unix/sysv/linux/nptl-signals.h
> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
> @@ -1,4 +1,4 @@
> -/* Special use of signals in NPTL internals.  Linux version.
> +/* Special use of signals internally.  Linux version.
>     Copyright (C) 2014-2017 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> +#ifndef __INTERNAL_SIGNALS_H
> +# define __INTERNAL_SIGNALS_H
> +
>  #include <signal.h>
>  #include <sigsetops.h>
>  
> @@ -35,17 +38,16 @@
>  
>  /* Return is sig is used internally.  */
>  static inline int
> -__nptl_is_internal_signal (int sig)
> +__is_internal_signal (int sig)
>  {
> -  return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
> +  return (sig == SIGCANCEL) || (sig == SIGSETXID);
>  }
>  
>  /* Remove internal glibc signal from the mask.  */
>  static inline void
> -__nptl_clear_internal_signals (sigset_t *set)
> +__clear_internal_signals (sigset_t *set)
>  {
>    __sigdelset (set, SIGCANCEL);
> -  __sigdelset (set, SIGTIMER);
>    __sigdelset (set, SIGSETXID);
>  }
>  
> @@ -66,7 +68,7 @@ static inline int
>  __libc_signal_block_app (sigset_t *set)
>  {
>    sigset_t allset = SIGALL_SET;
> -  __nptl_clear_internal_signals (&allset);
> +  __clear_internal_signals (&allset);
>    INTERNAL_SYSCALL_DECL (err);
>    return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,
>  			   _NSIG / 8);
> @@ -83,3 +85,5 @@ __libc_signal_restore_set (const sigset_t *set)
>  
>  /* Used to communicate with signal handler.  */
>  extern struct xid_command *__xidcmd attribute_hidden;
> +
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c
> index 70e5a98..1167950 100644
> --- a/sysdeps/unix/sysv/linux/raise.c
> +++ b/sysdeps/unix/sysv/linux/raise.c
> @@ -21,7 +21,7 @@
>  #include <errno.h>
>  #include <sys/types.h>
>  #include <unistd.h>
> -#include <nptl-signals.h>
> +#include <internal-signals.h>
>  
>  int
>  raise (int sig)
> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
> index fb83c2e..af464b5 100644
> --- a/sysdeps/unix/sysv/linux/spawni.c
> +++ b/sysdeps/unix/sysv/linux/spawni.c
> @@ -144,7 +144,7 @@ __spawni_child (void *arguments)
>  	}
>        else if (sigismember (&hset, sig))
>  	{
> -	  if (__nptl_is_internal_signal (sig))
> +	  if (__is_internal_signal (sig))
>  	    sa.sa_handler = SIG_IGN;
>  	  else
>  	    {
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391)
  2017-11-15 17:54 ` [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391) Adhemerval Zanella
  2017-11-16  1:10   ` Rical Jasan
@ 2017-12-12 17:03   ` Adhemerval Zanella
  2018-01-01 21:56     ` Adhemerval Zanella
  1 sibling, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2017-12-12 17:03 UTC (permalink / raw)
  To: libc-alpha

Ping (with Rical suggested changes).

On 15/11/2017 15:54, Adhemerval Zanella wrote:
> Changes from previous version:
> 
>   - Cleanup __nptl prefix from the generic names.
> 
> ---
> 
> 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 (__has_internal_signal).  It is used to
>      simplify the default sigprocmask.c implementation.
> 
> Checked on x86_64-linux-gnu.
> 
> 	[BZ #22391]
> 	* 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/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/internal-signals.h
> 	(__has_internal_signal): New function.
> 	* sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask):
> 	Use __has_internal_signal and __clear_internal_signals
> 	function.
> 
> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Signed-off-by: Zack Weinberg <zackw@panix.com>
> Reported-by: Yury Norov <ynorov@caviumnetworks.com>
> ---
>  ChangeLog                                          | 17 +++++++
>  manual/signal.texi                                 | 37 ++++++++++++++
>  sysdeps/unix/sysv/linux/alpha/sigprocmask.c        | 58 ----------------------
>  sysdeps/unix/sysv/linux/ia64/sigprocmask.c         | 40 ---------------
>  sysdeps/unix/sysv/linux/internal-signals.h         |  6 +++
>  sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c | 38 --------------
>  sysdeps/unix/sysv/linux/sigprocmask.c              | 23 +++------
>  .../unix/sysv/linux/sparc/sparc64/sigprocmask.c    | 34 -------------
>  sysdeps/unix/sysv/linux/x86_64/sigprocmask.c       | 39 ---------------
>  9 files changed, 66 insertions(+), 226 deletions(-)
>  delete mode 100644 sysdeps/unix/sysv/linux/alpha/sigprocmask.c
>  delete mode 100644 sysdeps/unix/sysv/linux/ia64/sigprocmask.c
>  delete mode 100644 sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
>  delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
>  delete mode 100644 sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
> 
> diff --git a/manual/signal.texi b/manual/signal.texi
> index 9577ff0..4f0ef59 100644
> --- a/manual/signal.texi
> +++ b/manual/signal.texi
> @@ -235,6 +235,7 @@ defined.  Since the signal numbers are allocated consecutively,
>  * Job Control Signals::         Signals used to support job control.
>  * Operation Error Signals::     Used to report operational system errors.
>  * Miscellaneous Signals::       Miscellaneous Signals.
> +* Internally-Used Signals::     Signals used internally by the C library.
>  * Signal Messages::             Printing a message describing a signal.
>  @end menu
>  
> @@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
>  The default action is to terminate the process.
>  @end deftypevr
>  
> +@deftypevr Macro int SIGRTMIN
> +@deftypevrx Macro int SIGRTMAX
> +@standards{POSIX.1, signal.h}
> +@cindex real-time signals
> +The range of signal numbers @code{SIGRTMIN}, @code{SIGRTMIN+1},
> +@dots{}, @code{SIGRTMAX} is also set aside for you to use any way you
> +want.  In addition, these signals (and no others) are guaranteed to
> +support @dfn{real-time} signal semantics, which unfortunately this
> +manual does not yet document.
> +
> +Unlike all of the other signal number macros, @code{SIGRTMIN} and
> +@code{SIGRTMAX} are not compile-time constants, because some operating
> +systems make the number of real-time signals tunable on a
> +per-installation or even per-process basis.  However, POSIX guarantees
> +that there will be at least 8 signal numbers in this range.
> +
> +The default action for all signals in this range is to terminate the
> +process.
> +@end deftypevr
> +
>  @deftypevr Macro int SIGWINCH
>  @standards{BSD, signal.h}
>  Window size change.  This is generated on some systems (including GNU)
> @@ -817,6 +838,22 @@ to print some status information about the system and what the process
>  is doing.  Otherwise the default is to do nothing.
>  @end deftypevr
>  
> +@node Internally-Used Signals
> +@subsection Internally-Used Signals
> +@cindex internally used signals
> +
> +On some operating systems, @theglibc{} needs to use a few signals from
> +the ``true'' real-time range internally, to implement thread
> +cancellation, cross-thread effective ID synchronization, POSIX timer
> +management, etc.  @Theglibc{} adjusts @code{SIGRTMIN} and
> +@code{SIGRTMAX} to exclude these signals, and it also takes steps to
> +prevent application code from altering their state: @code{sigprocmask}
> +cannot block them and @code{sigaction} cannot redefine their handlers,
> +for instance.  Therefore, most programs do not need to know or care
> +about these signals.  We mainly document their existence for the sake
> +of anyone who has ever wondered why there is a gap between the
> +highest-numbered ``normal'' signal and @code{SIGRTMIN} on Linux.
> +
>  @node Signal Messages
>  @subsection Signal Messages
>  @cindex signal messages
> 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/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
> index e96a718..bb8234d 100644
> --- a/sysdeps/unix/sysv/linux/internal-signals.h
> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
> @@ -36,6 +36,12 @@
>  #define SIGSETXID       (__SIGRTMIN + 1)
>  
>  
> +static inline bool
> +__has_internal_signal (const sigset_t *set)
> +{
> +  return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID);
> +}
> +
>  /* Return is sig is used internally.  */
>  static inline int
>  __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..e574b5d 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 <internal-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 (__has_internal_signal (set)))
>      {
>        local_newmask = *set;
> -      __sigdelset (&local_newmask, SIGCANCEL);
> -      __sigdelset (&local_newmask, SIGSETXID);
> +      __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] 15+ messages in thread

* Re: [PATCH v3 3/3] Filter out NPTL internal signals (BZ #22391)
  2017-11-15 17:54 ` [PATCH v3 3/3] Filter out NPTL internal signals " Adhemerval Zanella
@ 2017-12-12 17:03   ` Adhemerval Zanella
  2018-01-01 21:56     ` Adhemerval Zanella
  0 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2017-12-12 17:03 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 15/11/2017 15:54, Adhemerval Zanella wrote:
> This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and
> SIGSETXID) from signal functions.  GLIBC on Linux requires both signals to
> proper implement pthread cancellation, posix timers, and set*id posix
> thread synchronization.
> 
> And not filtering out the internal signal is 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.
> 
>   - It might also to security issues if SIGSETXID is masked and set*id
>     functions are called (some threads might have effective user or group
>     id different from the rest).
> 
> The changes are basically:
> 
>   - Change __is_internal_signal to bool and used on all signal function
>     that has a signal number as input.  Also for signal function which accepts
>     signals sets (sigset_t) it assumes that canonical function were used to
>     add/remove signals which lead to some input simplification.
> 
>   - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID.
>     It is rewritten to check each signal indidually and to check realtime
>     signals using canonical macros.
> 
>   - Add generic __clear_internal_signals and __is_internal_signal
>     version since both symbols are used on generic implementations.
> 
>   - Remove superflous sysdeps/nptl/sigfillset.c.
> 
>   - Remove superflous SIGTIMER handling on Linux __is_internal_signal
>     since it is the same of SIGCANCEL.
> 
>   - Remove dangling define and obvious comment on nptl/sigaction.c.
> 
> Checked on x86_64-linux-gnu.
> 
> 	[BZ #22391]
> 	* nptl/sigaction.c (__sigaction): Use __is_internal_signal to
> 	check for internal nptl signals.
> 	* signal/sigaddset.c (sigaddset): Likewise.
> 	* signal/sigdelset.c (sigdelset): Likewise.
> 	* sysdeps/posix/signal.c (__bsd_signal): Likewise.
> 	* sysdeps/posix/sigset.c (sigset): Call and check sigaddset return
> 	value.
> 	* signal/sigfillset.c (sigfillset): User __clear_internal_signals
> 	to filter out internal nptl signals.
> 	* signal/tst-sigset.c (do_test): Check ech signal indidually and
> 	also check realtime signals using standard macros.
> 	* sysdeps/nptl/nptl-signals.h (__clear_internal_signals,
> 	__is_internal_signal): New functions.
> 	* sysdeps/nptl/sigfillset.c: Remove file.
> 	* sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal):
> 	Change return to bool.
> 	(__clear_internal_signals): Remove SIGTIMER clean since it is
> 	equal to SIGCANEL on Linux.
> 	* sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume
> 	signal set was constructed using standard functions.
> 	* sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise.
> 
> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Reported-by: Yury Norov <ynorov@caviumnetworks.com>
> ---
>  ChangeLog                                  | 25 ++++++++
>  nptl/sigaction.c                           | 14 +----
>  signal/sigaction.c                         |  2 +-
>  signal/sigaddset.c                         |  5 +-
>  signal/sigdelset.c                         |  5 +-
>  signal/sigfillset.c                        | 10 +---
>  signal/tst-sigset.c                        | 92 ++++++++++++++++++++++--------
>  sysdeps/generic/internal-signals.h         | 11 ++++
>  sysdeps/nptl/sigfillset.c                  | 20 -------
>  sysdeps/posix/signal.c                     |  5 +-
>  sysdeps/posix/sigset.c                     | 10 +---
>  sysdeps/unix/sysv/linux/internal-signals.h |  4 +-
>  sysdeps/unix/sysv/linux/sigtimedwait.c     | 17 +-----
>  13 files changed, 124 insertions(+), 96 deletions(-)
>  delete mode 100644 sysdeps/nptl/sigfillset.c
> 
> diff --git a/nptl/sigaction.c b/nptl/sigaction.c
> index 2994fd5..b2ff674 100644
> --- a/nptl/sigaction.c
> +++ b/nptl/sigaction.c
> @@ -16,22 +16,12 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -
> -/* This is no complete implementation.  The file is meant to be
> -   included in the real implementation to provide the wrapper around
> -   __libc_sigaction.  */
> -
> -#include <nptl/pthreadP.h>
> -
> -/* We use the libc implementation but we tell it to not allow
> -   SIGCANCEL or SIGTIMER to be handled.  */
> -#define LIBC_SIGACTION	1
> -
> +#include <internal-signals.h>
>  
>  int
>  __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>  {
> -  if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID))
> +  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
>      {
>        __set_errno (EINVAL);
>        return -1;
> diff --git a/signal/sigaction.c b/signal/sigaction.c
> index 8a6220c..3025aab 100644
> --- a/signal/sigaction.c
> +++ b/signal/sigaction.c
> @@ -24,7 +24,7 @@
>  int
>  __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>  {
> -  if (sig <= 0 || sig >= NSIG)
> +  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
>      {
>        __set_errno (EINVAL);
>        return -1;
> diff --git a/signal/sigaddset.c b/signal/sigaddset.c
> index 161be7b..a435f61 100644
> --- a/signal/sigaddset.c
> +++ b/signal/sigaddset.c
> @@ -17,13 +17,14 @@
>  
>  #include <errno.h>
>  #include <signal.h>
> -#include <sigsetops.h>
> +#include <internal-signals.h>
>  
>  /* Add SIGNO to SET.  */
>  int
>  sigaddset (sigset_t *set, int signo)
>  {
> -  if (set == NULL || signo <= 0 || signo >= NSIG)
> +  if (set == NULL || signo <= 0 || signo >= NSIG
> +      || __is_internal_signal (signo))
>      {
>        __set_errno (EINVAL);
>        return -1;
> diff --git a/signal/sigdelset.c b/signal/sigdelset.c
> index 2aaa536..01a50ec 100644
> --- a/signal/sigdelset.c
> +++ b/signal/sigdelset.c
> @@ -17,13 +17,14 @@
>  
>  #include <errno.h>
>  #include <signal.h>
> -#include <sigsetops.h>
> +#include <internal-signals.h>
>  
>  /* Add SIGNO to SET.  */
>  int
>  sigdelset (sigset_t *set, int signo)
>  {
> -  if (set == NULL || signo <= 0 || signo >= NSIG)
> +  if (set == NULL || signo <= 0 || signo >= NSIG
> +      || __is_internal_signal (signo))
>      {
>        __set_errno (EINVAL);
>        return -1;
> diff --git a/signal/sigfillset.c b/signal/sigfillset.c
> index 0fcc24a..560c66e 100644
> --- a/signal/sigfillset.c
> +++ b/signal/sigfillset.c
> @@ -18,6 +18,7 @@
>  #include <errno.h>
>  #include <signal.h>
>  #include <string.h>
> +#include <internal-signals.h>
>  
>  /* Set all signals in SET.  */
>  int
> @@ -31,14 +32,7 @@ sigfillset (sigset_t *set)
>  
>    memset (set, 0xff, sizeof (sigset_t));
>  
> -  /* If the implementation uses a cancellation signal don't set the bit.  */
> -#ifdef SIGCANCEL
> -  __sigdelset (set, SIGCANCEL);
> -#endif
> -  /* Likewise for the signal to implement setxid.  */
> -#ifdef SIGSETXID
> -  __sigdelset (set, SIGSETXID);
> -#endif
> +  __clear_internal_signals (set);
>  
>    return 0;
>  }
> diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c
> index d47adcc..a2b764d 100644
> --- a/signal/tst-sigset.c
> +++ b/signal/tst-sigset.c
> @@ -1,43 +1,85 @@
>  /* Test sig*set functions.  */
>  
>  #include <signal.h>
> -#include <stdio.h>
>  
> -#define TEST_FUNCTION do_test ()
> +#include <support/check.h>
> +
>  static int
>  do_test (void)
>  {
> -  int result = 0;
> -  int sig = -1;
> +  sigset_t set;
> +  TEST_VERIFY (sigemptyset (&set) == 0);
>  
> -#define TRY(call)							      \
> -  if (call)								      \
> -    {									      \
> -      printf ("%s (sig = %d): %m\n", #call, sig);			      \
> -      result = 1;							      \
> -    }									      \
> -  else
> +#define VERIFY(set, sig)			\
> +  TEST_VERIFY (sigismember (&set, sig) == 0);	\
> +  TEST_VERIFY (sigaddset (&set, sig) == 0);	\
> +  TEST_VERIFY (sigismember (&set, sig) != 0);	\
> +  TEST_VERIFY (sigdelset (&set, sig) == 0);	\
> +  TEST_VERIFY (sigismember (&set, sig) == 0)
>  
> +  /* ISO C99 signals.  */
> +  VERIFY (set, SIGINT);
> +  VERIFY (set, SIGILL);
> +  VERIFY (set, SIGABRT);
> +  VERIFY (set, SIGFPE);
> +  VERIFY (set, SIGSEGV);
> +  VERIFY (set, SIGTERM);
>  
> -  sigset_t set;
> -  TRY (sigemptyset (&set) != 0);
> +  /* Historical signals specified by POSIX. */
> +  VERIFY (set, SIGHUP);
> +  VERIFY (set, SIGQUIT);
> +  VERIFY (set, SIGTRAP);
> +  VERIFY (set, SIGKILL);
> +  VERIFY (set, SIGBUS);
> +  VERIFY (set, SIGSYS);
> +  VERIFY (set, SIGPIPE);
> +  VERIFY (set, SIGALRM);
> +
> +  /* New(er) POSIX signals (1003.1-2008, 1003.1-2013).  */
> +  VERIFY (set, SIGURG);
> +  VERIFY (set, SIGSTOP);
> +  VERIFY (set, SIGTSTP);
> +  VERIFY (set, SIGCONT);
> +  VERIFY (set, SIGCHLD);
> +  VERIFY (set, SIGTTIN);
> +  VERIFY (set, SIGTTOU);
> +  VERIFY (set, SIGPOLL);
> +  VERIFY (set, SIGXCPU);
> +  VERIFY (set, SIGXFSZ);
> +  VERIFY (set, SIGVTALRM);
> +  VERIFY (set, SIGPROF);
> +  VERIFY (set, SIGUSR1);
> +  VERIFY (set, SIGUSR2);
> +
> +  /* Nonstandard signals found in all modern POSIX systems
> +     (including both BSD and Linux).  */
> +  VERIFY (set, SIGWINCH);
>  
> -#ifdef SIGRTMAX
> -  int max_sig = SIGRTMAX;
> -#else
> -  int max_sig = NSIG - 1;
> +  /* Arch-specific signals.  */
> +#ifdef SIGEMT
> +  VERIFY (set, SIGEMT);
> +#endif
> +#ifdef SIGLOST
> +  VERIFY (set, SIGLOST);
> +#endif
> +#ifdef SIGINFO
> +  VERIFY (set, SIGINFO);
> +#endif
> +#ifdef SIGSTKFLT
> +  VERIFY (set, SIGSTKFLT);
> +#endif
> +#ifdef SIGPWR
> +  VERIFY (set, SIGPWR);
>  #endif
>  
> -  for (sig = 1; sig <= max_sig; ++sig)
> +  /* Read-time signals (POSIX.1b real-time extensions).  If they are
> +     supported SIGRTMAX value is greater than SIGRTMIN.  */
> +  for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++)
>      {
> -      TRY (sigismember (&set, sig) != 0);
> -      TRY (sigaddset (&set, sig) != 0);
> -      TRY (sigismember (&set, sig) == 0);
> -      TRY (sigdelset (&set, sig) != 0);
> -      TRY (sigismember (&set, sig) != 0);
> +      VERIFY (set, rtsig);
>      }
>  
> -  return result;
> +  return 0;
>  }
>  
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h
> index 55bc07d..8a8854c 100644
> --- a/sysdeps/generic/internal-signals.h
> +++ b/sysdeps/generic/internal-signals.h
> @@ -15,3 +15,14 @@
>     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/>.  */
> +
> +static inline void
> +__clear_internal_signals (sigset_t *set)
> +{
> +}
> +
> +static inline bool
> +__is_internal_signal (int sig)
> +{
> +  return false;
> +}
> diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c
> deleted file mode 100644
> index 50e8512..0000000
> --- a/sysdeps/nptl/sigfillset.c
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* Copyright (C) 2003-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 <nptl/pthreadP.h>
> -
> -#include <signal/sigfillset.c>
> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c
> index 81ba177..87c5d1c 100644
> --- a/sysdeps/posix/signal.c
> +++ b/sysdeps/posix/signal.c
> @@ -18,8 +18,8 @@
>  
>  #include <errno.h>
>  #include <signal.h>
> -#include <string.h>	/* For the real memset prototype.  */
>  #include <sigsetops.h>
> +#include <internal-signals.h>
>  
>  sigset_t _sigintr attribute_hidden;		/* Set by siginterrupt.  */
>  
> @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler)
>    struct sigaction act, oact;
>  
>    /* Check signal extents to protect __sigismember.  */
> -  if (handler == SIG_ERR || sig < 1 || sig >= NSIG)
> +  if (handler == SIG_ERR || sig < 1 || sig >= NSIG
> +      || __is_internal_signal (sig))
>      {
>        __set_errno (EINVAL);
>        return SIG_ERR;
> diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c
> index a4dfe0a..6234ecf 100644
> --- a/sysdeps/posix/sigset.c
> +++ b/sysdeps/posix/sigset.c
> @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp)
>    sigset_t set;
>    sigset_t oset;
>  
> -  /* Check signal extents to protect __sigismember.  */
> -  if (disp == SIG_ERR || sig < 1 || sig >= NSIG)
> -    {
> -      __set_errno (EINVAL);
> -      return SIG_ERR;
> -    }
> -
>    __sigemptyset (&set);
> -  __sigaddset (&set, sig);
> +  if (sigaddset (&set, sig) < 0)
> +    return SIG_ERR;
>  
>    if (disp == SIG_HOLD)
>      {
> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
> index bb8234d..bf33bbc 100644
> --- a/sysdeps/unix/sysv/linux/internal-signals.h
> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
> @@ -21,6 +21,8 @@
>  
>  #include <signal.h>
>  #include <sigsetops.h>
> +#include <stdbool.h>
> +#include <sysdep.h>
>  
>  /* The signal used for asynchronous cancelation.  */
>  #define SIGCANCEL       __SIGRTMIN
> @@ -43,7 +45,7 @@ __has_internal_signal (const sigset_t *set)
>  }
>  
>  /* Return is sig is used internally.  */
> -static inline int
> +static inline bool
>  __is_internal_signal (int sig)
>  {
>    return (sig == SIGCANCEL) || (sig == SIGSETXID);
> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c
> index 2d96dd4..b4977e3 100644
> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c
> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c
> @@ -24,21 +24,8 @@ int
>  __sigtimedwait (const sigset_t *set, siginfo_t *info,
>  		const struct timespec *timeout)
>  {
> -  sigset_t tmpset;
> -  if (set != NULL
> -      && (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
> -	  || __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);
> -      __sigdelset (&tmpset, SIGSETXID);
> -      set = &tmpset;
> -    }
> -
> -    /* XXX The size argument hopefully will have to be changed to the
> -       real size of the user-level sigset_t.  */
> +  /* XXX The size argument hopefully will have to be changed to the
> +     real size of the user-level sigset_t.  */
>    int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8);
>  
>    /* The kernel generates a SI_TKILL code in si_code in case tkill is
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391)
  2017-12-12 17:03   ` Adhemerval Zanella
@ 2018-01-01 21:56     ` Adhemerval Zanella
  2018-01-17 11:17       ` Adhemerval Zanella
  0 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2018-01-01 21:56 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 12/12/2017 15:03, Adhemerval Zanella wrote:
> Ping (with Rical suggested changes).
> 
> On 15/11/2017 15:54, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>>   - Cleanup __nptl prefix from the generic names.
>>
>> ---
>>
>> 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 (__has_internal_signal).  It is used to
>>      simplify the default sigprocmask.c implementation.
>>
>> Checked on x86_64-linux-gnu.
>>
>> 	[BZ #22391]
>> 	* 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/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/internal-signals.h
>> 	(__has_internal_signal): New function.
>> 	* sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask):
>> 	Use __has_internal_signal and __clear_internal_signals
>> 	function.
>>
>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> Signed-off-by: Zack Weinberg <zackw@panix.com>
>> Reported-by: Yury Norov <ynorov@caviumnetworks.com>
>> ---
>>  ChangeLog                                          | 17 +++++++
>>  manual/signal.texi                                 | 37 ++++++++++++++
>>  sysdeps/unix/sysv/linux/alpha/sigprocmask.c        | 58 ----------------------
>>  sysdeps/unix/sysv/linux/ia64/sigprocmask.c         | 40 ---------------
>>  sysdeps/unix/sysv/linux/internal-signals.h         |  6 +++
>>  sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c | 38 --------------
>>  sysdeps/unix/sysv/linux/sigprocmask.c              | 23 +++------
>>  .../unix/sysv/linux/sparc/sparc64/sigprocmask.c    | 34 -------------
>>  sysdeps/unix/sysv/linux/x86_64/sigprocmask.c       | 39 ---------------
>>  9 files changed, 66 insertions(+), 226 deletions(-)
>>  delete mode 100644 sysdeps/unix/sysv/linux/alpha/sigprocmask.c
>>  delete mode 100644 sysdeps/unix/sysv/linux/ia64/sigprocmask.c
>>  delete mode 100644 sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
>>  delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
>>  delete mode 100644 sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
>>
>> diff --git a/manual/signal.texi b/manual/signal.texi
>> index 9577ff0..4f0ef59 100644
>> --- a/manual/signal.texi
>> +++ b/manual/signal.texi
>> @@ -235,6 +235,7 @@ defined.  Since the signal numbers are allocated consecutively,
>>  * Job Control Signals::         Signals used to support job control.
>>  * Operation Error Signals::     Used to report operational system errors.
>>  * Miscellaneous Signals::       Miscellaneous Signals.
>> +* Internally-Used Signals::     Signals used internally by the C library.
>>  * Signal Messages::             Printing a message describing a signal.
>>  @end menu
>>  
>> @@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
>>  The default action is to terminate the process.
>>  @end deftypevr
>>  
>> +@deftypevr Macro int SIGRTMIN
>> +@deftypevrx Macro int SIGRTMAX
>> +@standards{POSIX.1, signal.h}
>> +@cindex real-time signals
>> +The range of signal numbers @code{SIGRTMIN}, @code{SIGRTMIN+1},
>> +@dots{}, @code{SIGRTMAX} is also set aside for you to use any way you
>> +want.  In addition, these signals (and no others) are guaranteed to
>> +support @dfn{real-time} signal semantics, which unfortunately this
>> +manual does not yet document.
>> +
>> +Unlike all of the other signal number macros, @code{SIGRTMIN} and
>> +@code{SIGRTMAX} are not compile-time constants, because some operating
>> +systems make the number of real-time signals tunable on a
>> +per-installation or even per-process basis.  However, POSIX guarantees
>> +that there will be at least 8 signal numbers in this range.
>> +
>> +The default action for all signals in this range is to terminate the
>> +process.
>> +@end deftypevr
>> +
>>  @deftypevr Macro int SIGWINCH
>>  @standards{BSD, signal.h}
>>  Window size change.  This is generated on some systems (including GNU)
>> @@ -817,6 +838,22 @@ to print some status information about the system and what the process
>>  is doing.  Otherwise the default is to do nothing.
>>  @end deftypevr
>>  
>> +@node Internally-Used Signals
>> +@subsection Internally-Used Signals
>> +@cindex internally used signals
>> +
>> +On some operating systems, @theglibc{} needs to use a few signals from
>> +the ``true'' real-time range internally, to implement thread
>> +cancellation, cross-thread effective ID synchronization, POSIX timer
>> +management, etc.  @Theglibc{} adjusts @code{SIGRTMIN} and
>> +@code{SIGRTMAX} to exclude these signals, and it also takes steps to
>> +prevent application code from altering their state: @code{sigprocmask}
>> +cannot block them and @code{sigaction} cannot redefine their handlers,
>> +for instance.  Therefore, most programs do not need to know or care
>> +about these signals.  We mainly document their existence for the sake
>> +of anyone who has ever wondered why there is a gap between the
>> +highest-numbered ``normal'' signal and @code{SIGRTMIN} on Linux.
>> +
>>  @node Signal Messages
>>  @subsection Signal Messages
>>  @cindex signal messages
>> 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/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
>> index e96a718..bb8234d 100644
>> --- a/sysdeps/unix/sysv/linux/internal-signals.h
>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
>> @@ -36,6 +36,12 @@
>>  #define SIGSETXID       (__SIGRTMIN + 1)
>>  
>>  
>> +static inline bool
>> +__has_internal_signal (const sigset_t *set)
>> +{
>> +  return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID);
>> +}
>> +
>>  /* Return is sig is used internally.  */
>>  static inline int
>>  __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..e574b5d 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 <internal-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 (__has_internal_signal (set)))
>>      {
>>        local_newmask = *set;
>> -      __sigdelset (&local_newmask, SIGCANCEL);
>> -      __sigdelset (&local_newmask, SIGSETXID);
>> +      __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] 15+ messages in thread

* Re: [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h
  2017-12-12 17:03 ` [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
@ 2018-01-01 21:56   ` Adhemerval Zanella
  2018-01-17 11:17     ` Adhemerval Zanella
  0 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2018-01-01 21:56 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 12/12/2017 15:03, Adhemerval Zanella wrote:
> Ping.
> 
> On 15/11/2017 15:54, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>>   - Cleanup __nptl prefix from the generic names.
>>
>>   - Remove superflous SIGTERM handling on Linux __is_internal_signal
>>     and __clear_internal_signals.
>>
>> ---
>>
>> This patch renames the nptl-signals.h header to internal-signals.h.
>> On Linux the definitions and functions are not only NPTL related, but
>> used for other POSIX definitions as well (for instance SIGTIMER for
>> posix times, SIGSETXID for id functions, and signal block/restore
>> helpers) and since generic functions will be places and used in generic
>> implementation it makes more sense to decouple it from NPTL.
>>
>> Checked on x86_64-linux-gnu.
>>
>> 	* sysdeps/nptl/nptl-signals.h: Move to ...
>> 	* sysdeps/generic/internal-signals.h: ... here.  Adjust internal
>> 	comments.
>> 	* sysdeps/unix/sysv/linux/internal-signals.h: Add include guards.
>> 	(__nptl_is_internal_signal): Rename to __is_internal_signal.
>> 	(__nptl_clear_internal_signals): Rename to __clear_internal_signals.
>> 	* sysdeps/unix/sysv/linux/raise.c: Adjust nptl-signal.h to
>> 	include-signals.h rename.
>> 	* nptl/pthreadP.h: Likewise.
>> 	* sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Call
>> 	__is_internal_signal instead of __nptl_is_internal_signal.
>>
>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> ---
>>  ChangeLog                                                | 12 ++++++++++++
>>  nptl/pthreadP.h                                          |  2 +-
>>  .../{nptl/nptl-signals.h => generic/internal-signals.h}  |  7 +------
>>  .../sysv/linux/{nptl-signals.h => internal-signals.h}    | 16 ++++++++++------
>>  sysdeps/unix/sysv/linux/raise.c                          |  2 +-
>>  sysdeps/unix/sysv/linux/spawni.c                         |  2 +-
>>  6 files changed, 26 insertions(+), 15 deletions(-)
>>  rename sysdeps/{nptl/nptl-signals.h => generic/internal-signals.h} (74%)
>>  rename sysdeps/unix/sysv/linux/{nptl-signals.h => internal-signals.h} (89%)
>>
>> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
>> index 1cc80b6..f3ec832 100644
>> --- a/nptl/pthreadP.h
>> +++ b/nptl/pthreadP.h
>> @@ -32,7 +32,7 @@
>>  #include <atomic.h>
>>  #include <kernel-features.h>
>>  #include <errno.h>
>> -#include <nptl-signals.h>
>> +#include <internal-signals.h>
>>  
>>  
>>  /* Atomic operations on TLS memory.  */
>> diff --git a/sysdeps/nptl/nptl-signals.h b/sysdeps/generic/internal-signals.h
>> similarity index 74%
>> rename from sysdeps/nptl/nptl-signals.h
>> rename to sysdeps/generic/internal-signals.h
>> index 298acf2..55bc07d 100644
>> --- a/sysdeps/nptl/nptl-signals.h
>> +++ b/sysdeps/generic/internal-signals.h
>> @@ -1,4 +1,4 @@
>> -/* Special use of signals in NPTL internals.  Stub version.
>> +/* Special use of signals internally.  Stub version.
>>     Copyright (C) 2014-2017 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>  
>> @@ -15,8 +15,3 @@
>>     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/>.  */
>> -
>> -/* This file can define the macros SIGCANCEL, SIGTIMER, and SIGSETXID to
>> -   signal numbers reserved by libpthread for those internal purposes.
>> -
>> -   Note that some code presumes SIGTIMER is the same as SIGCANCEL.  */
>> diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
>> similarity index 89%
>> rename from sysdeps/unix/sysv/linux/nptl-signals.h
>> rename to sysdeps/unix/sysv/linux/internal-signals.h
>> index f30c597..e96a718 100644
>> --- a/sysdeps/unix/sysv/linux/nptl-signals.h
>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
>> @@ -1,4 +1,4 @@
>> -/* Special use of signals in NPTL internals.  Linux version.
>> +/* Special use of signals internally.  Linux version.
>>     Copyright (C) 2014-2017 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>  
>> @@ -16,6 +16,9 @@
>>     License along with the GNU C Library; if not, see
>>     <http://www.gnu.org/licenses/>.  */
>>  
>> +#ifndef __INTERNAL_SIGNALS_H
>> +# define __INTERNAL_SIGNALS_H
>> +
>>  #include <signal.h>
>>  #include <sigsetops.h>
>>  
>> @@ -35,17 +38,16 @@
>>  
>>  /* Return is sig is used internally.  */
>>  static inline int
>> -__nptl_is_internal_signal (int sig)
>> +__is_internal_signal (int sig)
>>  {
>> -  return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
>> +  return (sig == SIGCANCEL) || (sig == SIGSETXID);
>>  }
>>  
>>  /* Remove internal glibc signal from the mask.  */
>>  static inline void
>> -__nptl_clear_internal_signals (sigset_t *set)
>> +__clear_internal_signals (sigset_t *set)
>>  {
>>    __sigdelset (set, SIGCANCEL);
>> -  __sigdelset (set, SIGTIMER);
>>    __sigdelset (set, SIGSETXID);
>>  }
>>  
>> @@ -66,7 +68,7 @@ static inline int
>>  __libc_signal_block_app (sigset_t *set)
>>  {
>>    sigset_t allset = SIGALL_SET;
>> -  __nptl_clear_internal_signals (&allset);
>> +  __clear_internal_signals (&allset);
>>    INTERNAL_SYSCALL_DECL (err);
>>    return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,
>>  			   _NSIG / 8);
>> @@ -83,3 +85,5 @@ __libc_signal_restore_set (const sigset_t *set)
>>  
>>  /* Used to communicate with signal handler.  */
>>  extern struct xid_command *__xidcmd attribute_hidden;
>> +
>> +#endif
>> diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c
>> index 70e5a98..1167950 100644
>> --- a/sysdeps/unix/sysv/linux/raise.c
>> +++ b/sysdeps/unix/sysv/linux/raise.c
>> @@ -21,7 +21,7 @@
>>  #include <errno.h>
>>  #include <sys/types.h>
>>  #include <unistd.h>
>> -#include <nptl-signals.h>
>> +#include <internal-signals.h>
>>  
>>  int
>>  raise (int sig)
>> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
>> index fb83c2e..af464b5 100644
>> --- a/sysdeps/unix/sysv/linux/spawni.c
>> +++ b/sysdeps/unix/sysv/linux/spawni.c
>> @@ -144,7 +144,7 @@ __spawni_child (void *arguments)
>>  	}
>>        else if (sigismember (&hset, sig))
>>  	{
>> -	  if (__nptl_is_internal_signal (sig))
>> +	  if (__is_internal_signal (sig))
>>  	    sa.sa_handler = SIG_IGN;
>>  	  else
>>  	    {
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] Filter out NPTL internal signals (BZ #22391)
  2017-12-12 17:03   ` Adhemerval Zanella
@ 2018-01-01 21:56     ` Adhemerval Zanella
  2018-01-17 11:17       ` Adhemerval Zanella
  0 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2018-01-01 21:56 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 12/12/2017 15:03, Adhemerval Zanella wrote:
> Ping.
> 
> On 15/11/2017 15:54, Adhemerval Zanella wrote:
>> This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and
>> SIGSETXID) from signal functions.  GLIBC on Linux requires both signals to
>> proper implement pthread cancellation, posix timers, and set*id posix
>> thread synchronization.
>>
>> And not filtering out the internal signal is 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.
>>
>>   - It might also to security issues if SIGSETXID is masked and set*id
>>     functions are called (some threads might have effective user or group
>>     id different from the rest).
>>
>> The changes are basically:
>>
>>   - Change __is_internal_signal to bool and used on all signal function
>>     that has a signal number as input.  Also for signal function which accepts
>>     signals sets (sigset_t) it assumes that canonical function were used to
>>     add/remove signals which lead to some input simplification.
>>
>>   - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID.
>>     It is rewritten to check each signal indidually and to check realtime
>>     signals using canonical macros.
>>
>>   - Add generic __clear_internal_signals and __is_internal_signal
>>     version since both symbols are used on generic implementations.
>>
>>   - Remove superflous sysdeps/nptl/sigfillset.c.
>>
>>   - Remove superflous SIGTIMER handling on Linux __is_internal_signal
>>     since it is the same of SIGCANCEL.
>>
>>   - Remove dangling define and obvious comment on nptl/sigaction.c.
>>
>> Checked on x86_64-linux-gnu.
>>
>> 	[BZ #22391]
>> 	* nptl/sigaction.c (__sigaction): Use __is_internal_signal to
>> 	check for internal nptl signals.
>> 	* signal/sigaddset.c (sigaddset): Likewise.
>> 	* signal/sigdelset.c (sigdelset): Likewise.
>> 	* sysdeps/posix/signal.c (__bsd_signal): Likewise.
>> 	* sysdeps/posix/sigset.c (sigset): Call and check sigaddset return
>> 	value.
>> 	* signal/sigfillset.c (sigfillset): User __clear_internal_signals
>> 	to filter out internal nptl signals.
>> 	* signal/tst-sigset.c (do_test): Check ech signal indidually and
>> 	also check realtime signals using standard macros.
>> 	* sysdeps/nptl/nptl-signals.h (__clear_internal_signals,
>> 	__is_internal_signal): New functions.
>> 	* sysdeps/nptl/sigfillset.c: Remove file.
>> 	* sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal):
>> 	Change return to bool.
>> 	(__clear_internal_signals): Remove SIGTIMER clean since it is
>> 	equal to SIGCANEL on Linux.
>> 	* sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume
>> 	signal set was constructed using standard functions.
>> 	* sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise.
>>
>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> Reported-by: Yury Norov <ynorov@caviumnetworks.com>
>> ---
>>  ChangeLog                                  | 25 ++++++++
>>  nptl/sigaction.c                           | 14 +----
>>  signal/sigaction.c                         |  2 +-
>>  signal/sigaddset.c                         |  5 +-
>>  signal/sigdelset.c                         |  5 +-
>>  signal/sigfillset.c                        | 10 +---
>>  signal/tst-sigset.c                        | 92 ++++++++++++++++++++++--------
>>  sysdeps/generic/internal-signals.h         | 11 ++++
>>  sysdeps/nptl/sigfillset.c                  | 20 -------
>>  sysdeps/posix/signal.c                     |  5 +-
>>  sysdeps/posix/sigset.c                     | 10 +---
>>  sysdeps/unix/sysv/linux/internal-signals.h |  4 +-
>>  sysdeps/unix/sysv/linux/sigtimedwait.c     | 17 +-----
>>  13 files changed, 124 insertions(+), 96 deletions(-)
>>  delete mode 100644 sysdeps/nptl/sigfillset.c
>>
>> diff --git a/nptl/sigaction.c b/nptl/sigaction.c
>> index 2994fd5..b2ff674 100644
>> --- a/nptl/sigaction.c
>> +++ b/nptl/sigaction.c
>> @@ -16,22 +16,12 @@
>>     License along with the GNU C Library; if not, see
>>     <http://www.gnu.org/licenses/>.  */
>>  
>> -
>> -/* This is no complete implementation.  The file is meant to be
>> -   included in the real implementation to provide the wrapper around
>> -   __libc_sigaction.  */
>> -
>> -#include <nptl/pthreadP.h>
>> -
>> -/* We use the libc implementation but we tell it to not allow
>> -   SIGCANCEL or SIGTIMER to be handled.  */
>> -#define LIBC_SIGACTION	1
>> -
>> +#include <internal-signals.h>
>>  
>>  int
>>  __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>>  {
>> -  if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID))
>> +  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
>>      {
>>        __set_errno (EINVAL);
>>        return -1;
>> diff --git a/signal/sigaction.c b/signal/sigaction.c
>> index 8a6220c..3025aab 100644
>> --- a/signal/sigaction.c
>> +++ b/signal/sigaction.c
>> @@ -24,7 +24,7 @@
>>  int
>>  __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>>  {
>> -  if (sig <= 0 || sig >= NSIG)
>> +  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
>>      {
>>        __set_errno (EINVAL);
>>        return -1;
>> diff --git a/signal/sigaddset.c b/signal/sigaddset.c
>> index 161be7b..a435f61 100644
>> --- a/signal/sigaddset.c
>> +++ b/signal/sigaddset.c
>> @@ -17,13 +17,14 @@
>>  
>>  #include <errno.h>
>>  #include <signal.h>
>> -#include <sigsetops.h>
>> +#include <internal-signals.h>
>>  
>>  /* Add SIGNO to SET.  */
>>  int
>>  sigaddset (sigset_t *set, int signo)
>>  {
>> -  if (set == NULL || signo <= 0 || signo >= NSIG)
>> +  if (set == NULL || signo <= 0 || signo >= NSIG
>> +      || __is_internal_signal (signo))
>>      {
>>        __set_errno (EINVAL);
>>        return -1;
>> diff --git a/signal/sigdelset.c b/signal/sigdelset.c
>> index 2aaa536..01a50ec 100644
>> --- a/signal/sigdelset.c
>> +++ b/signal/sigdelset.c
>> @@ -17,13 +17,14 @@
>>  
>>  #include <errno.h>
>>  #include <signal.h>
>> -#include <sigsetops.h>
>> +#include <internal-signals.h>
>>  
>>  /* Add SIGNO to SET.  */
>>  int
>>  sigdelset (sigset_t *set, int signo)
>>  {
>> -  if (set == NULL || signo <= 0 || signo >= NSIG)
>> +  if (set == NULL || signo <= 0 || signo >= NSIG
>> +      || __is_internal_signal (signo))
>>      {
>>        __set_errno (EINVAL);
>>        return -1;
>> diff --git a/signal/sigfillset.c b/signal/sigfillset.c
>> index 0fcc24a..560c66e 100644
>> --- a/signal/sigfillset.c
>> +++ b/signal/sigfillset.c
>> @@ -18,6 +18,7 @@
>>  #include <errno.h>
>>  #include <signal.h>
>>  #include <string.h>
>> +#include <internal-signals.h>
>>  
>>  /* Set all signals in SET.  */
>>  int
>> @@ -31,14 +32,7 @@ sigfillset (sigset_t *set)
>>  
>>    memset (set, 0xff, sizeof (sigset_t));
>>  
>> -  /* If the implementation uses a cancellation signal don't set the bit.  */
>> -#ifdef SIGCANCEL
>> -  __sigdelset (set, SIGCANCEL);
>> -#endif
>> -  /* Likewise for the signal to implement setxid.  */
>> -#ifdef SIGSETXID
>> -  __sigdelset (set, SIGSETXID);
>> -#endif
>> +  __clear_internal_signals (set);
>>  
>>    return 0;
>>  }
>> diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c
>> index d47adcc..a2b764d 100644
>> --- a/signal/tst-sigset.c
>> +++ b/signal/tst-sigset.c
>> @@ -1,43 +1,85 @@
>>  /* Test sig*set functions.  */
>>  
>>  #include <signal.h>
>> -#include <stdio.h>
>>  
>> -#define TEST_FUNCTION do_test ()
>> +#include <support/check.h>
>> +
>>  static int
>>  do_test (void)
>>  {
>> -  int result = 0;
>> -  int sig = -1;
>> +  sigset_t set;
>> +  TEST_VERIFY (sigemptyset (&set) == 0);
>>  
>> -#define TRY(call)							      \
>> -  if (call)								      \
>> -    {									      \
>> -      printf ("%s (sig = %d): %m\n", #call, sig);			      \
>> -      result = 1;							      \
>> -    }									      \
>> -  else
>> +#define VERIFY(set, sig)			\
>> +  TEST_VERIFY (sigismember (&set, sig) == 0);	\
>> +  TEST_VERIFY (sigaddset (&set, sig) == 0);	\
>> +  TEST_VERIFY (sigismember (&set, sig) != 0);	\
>> +  TEST_VERIFY (sigdelset (&set, sig) == 0);	\
>> +  TEST_VERIFY (sigismember (&set, sig) == 0)
>>  
>> +  /* ISO C99 signals.  */
>> +  VERIFY (set, SIGINT);
>> +  VERIFY (set, SIGILL);
>> +  VERIFY (set, SIGABRT);
>> +  VERIFY (set, SIGFPE);
>> +  VERIFY (set, SIGSEGV);
>> +  VERIFY (set, SIGTERM);
>>  
>> -  sigset_t set;
>> -  TRY (sigemptyset (&set) != 0);
>> +  /* Historical signals specified by POSIX. */
>> +  VERIFY (set, SIGHUP);
>> +  VERIFY (set, SIGQUIT);
>> +  VERIFY (set, SIGTRAP);
>> +  VERIFY (set, SIGKILL);
>> +  VERIFY (set, SIGBUS);
>> +  VERIFY (set, SIGSYS);
>> +  VERIFY (set, SIGPIPE);
>> +  VERIFY (set, SIGALRM);
>> +
>> +  /* New(er) POSIX signals (1003.1-2008, 1003.1-2013).  */
>> +  VERIFY (set, SIGURG);
>> +  VERIFY (set, SIGSTOP);
>> +  VERIFY (set, SIGTSTP);
>> +  VERIFY (set, SIGCONT);
>> +  VERIFY (set, SIGCHLD);
>> +  VERIFY (set, SIGTTIN);
>> +  VERIFY (set, SIGTTOU);
>> +  VERIFY (set, SIGPOLL);
>> +  VERIFY (set, SIGXCPU);
>> +  VERIFY (set, SIGXFSZ);
>> +  VERIFY (set, SIGVTALRM);
>> +  VERIFY (set, SIGPROF);
>> +  VERIFY (set, SIGUSR1);
>> +  VERIFY (set, SIGUSR2);
>> +
>> +  /* Nonstandard signals found in all modern POSIX systems
>> +     (including both BSD and Linux).  */
>> +  VERIFY (set, SIGWINCH);
>>  
>> -#ifdef SIGRTMAX
>> -  int max_sig = SIGRTMAX;
>> -#else
>> -  int max_sig = NSIG - 1;
>> +  /* Arch-specific signals.  */
>> +#ifdef SIGEMT
>> +  VERIFY (set, SIGEMT);
>> +#endif
>> +#ifdef SIGLOST
>> +  VERIFY (set, SIGLOST);
>> +#endif
>> +#ifdef SIGINFO
>> +  VERIFY (set, SIGINFO);
>> +#endif
>> +#ifdef SIGSTKFLT
>> +  VERIFY (set, SIGSTKFLT);
>> +#endif
>> +#ifdef SIGPWR
>> +  VERIFY (set, SIGPWR);
>>  #endif
>>  
>> -  for (sig = 1; sig <= max_sig; ++sig)
>> +  /* Read-time signals (POSIX.1b real-time extensions).  If they are
>> +     supported SIGRTMAX value is greater than SIGRTMIN.  */
>> +  for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++)
>>      {
>> -      TRY (sigismember (&set, sig) != 0);
>> -      TRY (sigaddset (&set, sig) != 0);
>> -      TRY (sigismember (&set, sig) == 0);
>> -      TRY (sigdelset (&set, sig) != 0);
>> -      TRY (sigismember (&set, sig) != 0);
>> +      VERIFY (set, rtsig);
>>      }
>>  
>> -  return result;
>> +  return 0;
>>  }
>>  
>> -#include "../test-skeleton.c"
>> +#include <support/test-driver.c>
>> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h
>> index 55bc07d..8a8854c 100644
>> --- a/sysdeps/generic/internal-signals.h
>> +++ b/sysdeps/generic/internal-signals.h
>> @@ -15,3 +15,14 @@
>>     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/>.  */
>> +
>> +static inline void
>> +__clear_internal_signals (sigset_t *set)
>> +{
>> +}
>> +
>> +static inline bool
>> +__is_internal_signal (int sig)
>> +{
>> +  return false;
>> +}
>> diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c
>> deleted file mode 100644
>> index 50e8512..0000000
>> --- a/sysdeps/nptl/sigfillset.c
>> +++ /dev/null
>> @@ -1,20 +0,0 @@
>> -/* Copyright (C) 2003-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 <nptl/pthreadP.h>
>> -
>> -#include <signal/sigfillset.c>
>> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c
>> index 81ba177..87c5d1c 100644
>> --- a/sysdeps/posix/signal.c
>> +++ b/sysdeps/posix/signal.c
>> @@ -18,8 +18,8 @@
>>  
>>  #include <errno.h>
>>  #include <signal.h>
>> -#include <string.h>	/* For the real memset prototype.  */
>>  #include <sigsetops.h>
>> +#include <internal-signals.h>
>>  
>>  sigset_t _sigintr attribute_hidden;		/* Set by siginterrupt.  */
>>  
>> @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler)
>>    struct sigaction act, oact;
>>  
>>    /* Check signal extents to protect __sigismember.  */
>> -  if (handler == SIG_ERR || sig < 1 || sig >= NSIG)
>> +  if (handler == SIG_ERR || sig < 1 || sig >= NSIG
>> +      || __is_internal_signal (sig))
>>      {
>>        __set_errno (EINVAL);
>>        return SIG_ERR;
>> diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c
>> index a4dfe0a..6234ecf 100644
>> --- a/sysdeps/posix/sigset.c
>> +++ b/sysdeps/posix/sigset.c
>> @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp)
>>    sigset_t set;
>>    sigset_t oset;
>>  
>> -  /* Check signal extents to protect __sigismember.  */
>> -  if (disp == SIG_ERR || sig < 1 || sig >= NSIG)
>> -    {
>> -      __set_errno (EINVAL);
>> -      return SIG_ERR;
>> -    }
>> -
>>    __sigemptyset (&set);
>> -  __sigaddset (&set, sig);
>> +  if (sigaddset (&set, sig) < 0)
>> +    return SIG_ERR;
>>  
>>    if (disp == SIG_HOLD)
>>      {
>> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
>> index bb8234d..bf33bbc 100644
>> --- a/sysdeps/unix/sysv/linux/internal-signals.h
>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
>> @@ -21,6 +21,8 @@
>>  
>>  #include <signal.h>
>>  #include <sigsetops.h>
>> +#include <stdbool.h>
>> +#include <sysdep.h>
>>  
>>  /* The signal used for asynchronous cancelation.  */
>>  #define SIGCANCEL       __SIGRTMIN
>> @@ -43,7 +45,7 @@ __has_internal_signal (const sigset_t *set)
>>  }
>>  
>>  /* Return is sig is used internally.  */
>> -static inline int
>> +static inline bool
>>  __is_internal_signal (int sig)
>>  {
>>    return (sig == SIGCANCEL) || (sig == SIGSETXID);
>> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c
>> index 2d96dd4..b4977e3 100644
>> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c
>> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c
>> @@ -24,21 +24,8 @@ int
>>  __sigtimedwait (const sigset_t *set, siginfo_t *info,
>>  		const struct timespec *timeout)
>>  {
>> -  sigset_t tmpset;
>> -  if (set != NULL
>> -      && (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
>> -	  || __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);
>> -      __sigdelset (&tmpset, SIGSETXID);
>> -      set = &tmpset;
>> -    }
>> -
>> -    /* XXX The size argument hopefully will have to be changed to the
>> -       real size of the user-level sigset_t.  */
>> +  /* XXX The size argument hopefully will have to be changed to the
>> +     real size of the user-level sigset_t.  */
>>    int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8);
>>  
>>    /* The kernel generates a SI_TKILL code in si_code in case tkill is
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391)
  2018-01-01 21:56     ` Adhemerval Zanella
@ 2018-01-17 11:17       ` Adhemerval Zanella
  0 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2018-01-17 11:17 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 01/01/2018 19:56, Adhemerval Zanella wrote:
> Ping.
> 
> On 12/12/2017 15:03, Adhemerval Zanella wrote:
>> Ping (with Rical suggested changes).
>>
>> On 15/11/2017 15:54, Adhemerval Zanella wrote:
>>> Changes from previous version:
>>>
>>>   - Cleanup __nptl prefix from the generic names.
>>>
>>> ---
>>>
>>> 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 (__has_internal_signal).  It is used to
>>>      simplify the default sigprocmask.c implementation.
>>>
>>> Checked on x86_64-linux-gnu.
>>>
>>> 	[BZ #22391]
>>> 	* 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/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/internal-signals.h
>>> 	(__has_internal_signal): New function.
>>> 	* sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask):
>>> 	Use __has_internal_signal and __clear_internal_signals
>>> 	function.
>>>
>>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>> Signed-off-by: Zack Weinberg <zackw@panix.com>
>>> Reported-by: Yury Norov <ynorov@caviumnetworks.com>
>>> ---
>>>  ChangeLog                                          | 17 +++++++
>>>  manual/signal.texi                                 | 37 ++++++++++++++
>>>  sysdeps/unix/sysv/linux/alpha/sigprocmask.c        | 58 ----------------------
>>>  sysdeps/unix/sysv/linux/ia64/sigprocmask.c         | 40 ---------------
>>>  sysdeps/unix/sysv/linux/internal-signals.h         |  6 +++
>>>  sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c | 38 --------------
>>>  sysdeps/unix/sysv/linux/sigprocmask.c              | 23 +++------
>>>  .../unix/sysv/linux/sparc/sparc64/sigprocmask.c    | 34 -------------
>>>  sysdeps/unix/sysv/linux/x86_64/sigprocmask.c       | 39 ---------------
>>>  9 files changed, 66 insertions(+), 226 deletions(-)
>>>  delete mode 100644 sysdeps/unix/sysv/linux/alpha/sigprocmask.c
>>>  delete mode 100644 sysdeps/unix/sysv/linux/ia64/sigprocmask.c
>>>  delete mode 100644 sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c
>>>  delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c
>>>  delete mode 100644 sysdeps/unix/sysv/linux/x86_64/sigprocmask.c
>>>
>>> diff --git a/manual/signal.texi b/manual/signal.texi
>>> index 9577ff0..4f0ef59 100644
>>> --- a/manual/signal.texi
>>> +++ b/manual/signal.texi
>>> @@ -235,6 +235,7 @@ defined.  Since the signal numbers are allocated consecutively,
>>>  * Job Control Signals::         Signals used to support job control.
>>>  * Operation Error Signals::     Used to report operational system errors.
>>>  * Miscellaneous Signals::       Miscellaneous Signals.
>>> +* Internally-Used Signals::     Signals used internally by the C library.
>>>  * Signal Messages::             Printing a message describing a signal.
>>>  @end menu
>>>  
>>> @@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
>>>  The default action is to terminate the process.
>>>  @end deftypevr
>>>  
>>> +@deftypevr Macro int SIGRTMIN
>>> +@deftypevrx Macro int SIGRTMAX
>>> +@standards{POSIX.1, signal.h}
>>> +@cindex real-time signals
>>> +The range of signal numbers @code{SIGRTMIN}, @code{SIGRTMIN+1},
>>> +@dots{}, @code{SIGRTMAX} is also set aside for you to use any way you
>>> +want.  In addition, these signals (and no others) are guaranteed to
>>> +support @dfn{real-time} signal semantics, which unfortunately this
>>> +manual does not yet document.
>>> +
>>> +Unlike all of the other signal number macros, @code{SIGRTMIN} and
>>> +@code{SIGRTMAX} are not compile-time constants, because some operating
>>> +systems make the number of real-time signals tunable on a
>>> +per-installation or even per-process basis.  However, POSIX guarantees
>>> +that there will be at least 8 signal numbers in this range.
>>> +
>>> +The default action for all signals in this range is to terminate the
>>> +process.
>>> +@end deftypevr
>>> +
>>>  @deftypevr Macro int SIGWINCH
>>>  @standards{BSD, signal.h}
>>>  Window size change.  This is generated on some systems (including GNU)
>>> @@ -817,6 +838,22 @@ to print some status information about the system and what the process
>>>  is doing.  Otherwise the default is to do nothing.
>>>  @end deftypevr
>>>  
>>> +@node Internally-Used Signals
>>> +@subsection Internally-Used Signals
>>> +@cindex internally used signals
>>> +
>>> +On some operating systems, @theglibc{} needs to use a few signals from
>>> +the ``true'' real-time range internally, to implement thread
>>> +cancellation, cross-thread effective ID synchronization, POSIX timer
>>> +management, etc.  @Theglibc{} adjusts @code{SIGRTMIN} and
>>> +@code{SIGRTMAX} to exclude these signals, and it also takes steps to
>>> +prevent application code from altering their state: @code{sigprocmask}
>>> +cannot block them and @code{sigaction} cannot redefine their handlers,
>>> +for instance.  Therefore, most programs do not need to know or care
>>> +about these signals.  We mainly document their existence for the sake
>>> +of anyone who has ever wondered why there is a gap between the
>>> +highest-numbered ``normal'' signal and @code{SIGRTMIN} on Linux.
>>> +
>>>  @node Signal Messages
>>>  @subsection Signal Messages
>>>  @cindex signal messages
>>> 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/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
>>> index e96a718..bb8234d 100644
>>> --- a/sysdeps/unix/sysv/linux/internal-signals.h
>>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
>>> @@ -36,6 +36,12 @@
>>>  #define SIGSETXID       (__SIGRTMIN + 1)
>>>  
>>>  
>>> +static inline bool
>>> +__has_internal_signal (const sigset_t *set)
>>> +{
>>> +  return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID);
>>> +}
>>> +
>>>  /* Return is sig is used internally.  */
>>>  static inline int
>>>  __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..e574b5d 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 <internal-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 (__has_internal_signal (set)))
>>>      {
>>>        local_newmask = *set;
>>> -      __sigdelset (&local_newmask, SIGCANCEL);
>>> -      __sigdelset (&local_newmask, SIGSETXID);
>>> +      __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] 15+ messages in thread

* Re: [PATCH v3 3/3] Filter out NPTL internal signals (BZ #22391)
  2018-01-01 21:56     ` Adhemerval Zanella
@ 2018-01-17 11:17       ` Adhemerval Zanella
  0 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2018-01-17 11:17 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 01/01/2018 19:56, Adhemerval Zanella wrote:
> Ping.
> 
> On 12/12/2017 15:03, Adhemerval Zanella wrote:
>> Ping.
>>
>> On 15/11/2017 15:54, Adhemerval Zanella wrote:
>>> This patch filters out the internal NPTL signals (SIGCANCEL/SIGTIMER and
>>> SIGSETXID) from signal functions.  GLIBC on Linux requires both signals to
>>> proper implement pthread cancellation, posix timers, and set*id posix
>>> thread synchronization.
>>>
>>> And not filtering out the internal signal is 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.
>>>
>>>   - It might also to security issues if SIGSETXID is masked and set*id
>>>     functions are called (some threads might have effective user or group
>>>     id different from the rest).
>>>
>>> The changes are basically:
>>>
>>>   - Change __is_internal_signal to bool and used on all signal function
>>>     that has a signal number as input.  Also for signal function which accepts
>>>     signals sets (sigset_t) it assumes that canonical function were used to
>>>     add/remove signals which lead to some input simplification.
>>>
>>>   - Fix tst-sigset.c to avoid check for SIGCANCEL/SIGTIMER and SIGSETXID.
>>>     It is rewritten to check each signal indidually and to check realtime
>>>     signals using canonical macros.
>>>
>>>   - Add generic __clear_internal_signals and __is_internal_signal
>>>     version since both symbols are used on generic implementations.
>>>
>>>   - Remove superflous sysdeps/nptl/sigfillset.c.
>>>
>>>   - Remove superflous SIGTIMER handling on Linux __is_internal_signal
>>>     since it is the same of SIGCANCEL.
>>>
>>>   - Remove dangling define and obvious comment on nptl/sigaction.c.
>>>
>>> Checked on x86_64-linux-gnu.
>>>
>>> 	[BZ #22391]
>>> 	* nptl/sigaction.c (__sigaction): Use __is_internal_signal to
>>> 	check for internal nptl signals.
>>> 	* signal/sigaddset.c (sigaddset): Likewise.
>>> 	* signal/sigdelset.c (sigdelset): Likewise.
>>> 	* sysdeps/posix/signal.c (__bsd_signal): Likewise.
>>> 	* sysdeps/posix/sigset.c (sigset): Call and check sigaddset return
>>> 	value.
>>> 	* signal/sigfillset.c (sigfillset): User __clear_internal_signals
>>> 	to filter out internal nptl signals.
>>> 	* signal/tst-sigset.c (do_test): Check ech signal indidually and
>>> 	also check realtime signals using standard macros.
>>> 	* sysdeps/nptl/nptl-signals.h (__clear_internal_signals,
>>> 	__is_internal_signal): New functions.
>>> 	* sysdeps/nptl/sigfillset.c: Remove file.
>>> 	* sysdeps/unix/sysv/linux/nptl-signals.h (__is_internal_signal):
>>> 	Change return to bool.
>>> 	(__clear_internal_signals): Remove SIGTIMER clean since it is
>>> 	equal to SIGCANEL on Linux.
>>> 	* sysdeps/unix/sysv/linux/sigtimedwait.c (__sigtimedwait): Assume
>>> 	signal set was constructed using standard functions.
>>> 	* sysdeps/unix/sysv/linux/sigwait.c (do_sigtwait): Likewise.
>>>
>>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>> Reported-by: Yury Norov <ynorov@caviumnetworks.com>
>>> ---
>>>  ChangeLog                                  | 25 ++++++++
>>>  nptl/sigaction.c                           | 14 +----
>>>  signal/sigaction.c                         |  2 +-
>>>  signal/sigaddset.c                         |  5 +-
>>>  signal/sigdelset.c                         |  5 +-
>>>  signal/sigfillset.c                        | 10 +---
>>>  signal/tst-sigset.c                        | 92 ++++++++++++++++++++++--------
>>>  sysdeps/generic/internal-signals.h         | 11 ++++
>>>  sysdeps/nptl/sigfillset.c                  | 20 -------
>>>  sysdeps/posix/signal.c                     |  5 +-
>>>  sysdeps/posix/sigset.c                     | 10 +---
>>>  sysdeps/unix/sysv/linux/internal-signals.h |  4 +-
>>>  sysdeps/unix/sysv/linux/sigtimedwait.c     | 17 +-----
>>>  13 files changed, 124 insertions(+), 96 deletions(-)
>>>  delete mode 100644 sysdeps/nptl/sigfillset.c
>>>
>>> diff --git a/nptl/sigaction.c b/nptl/sigaction.c
>>> index 2994fd5..b2ff674 100644
>>> --- a/nptl/sigaction.c
>>> +++ b/nptl/sigaction.c
>>> @@ -16,22 +16,12 @@
>>>     License along with the GNU C Library; if not, see
>>>     <http://www.gnu.org/licenses/>.  */
>>>  
>>> -
>>> -/* This is no complete implementation.  The file is meant to be
>>> -   included in the real implementation to provide the wrapper around
>>> -   __libc_sigaction.  */
>>> -
>>> -#include <nptl/pthreadP.h>
>>> -
>>> -/* We use the libc implementation but we tell it to not allow
>>> -   SIGCANCEL or SIGTIMER to be handled.  */
>>> -#define LIBC_SIGACTION	1
>>> -
>>> +#include <internal-signals.h>
>>>  
>>>  int
>>>  __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>>>  {
>>> -  if (__glibc_unlikely (sig == SIGCANCEL || sig == SIGSETXID))
>>> +  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
>>>      {
>>>        __set_errno (EINVAL);
>>>        return -1;
>>> diff --git a/signal/sigaction.c b/signal/sigaction.c
>>> index 8a6220c..3025aab 100644
>>> --- a/signal/sigaction.c
>>> +++ b/signal/sigaction.c
>>> @@ -24,7 +24,7 @@
>>>  int
>>>  __sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>>>  {
>>> -  if (sig <= 0 || sig >= NSIG)
>>> +  if (sig <= 0 || sig >= NSIG || __is_internal_signal (sig))
>>>      {
>>>        __set_errno (EINVAL);
>>>        return -1;
>>> diff --git a/signal/sigaddset.c b/signal/sigaddset.c
>>> index 161be7b..a435f61 100644
>>> --- a/signal/sigaddset.c
>>> +++ b/signal/sigaddset.c
>>> @@ -17,13 +17,14 @@
>>>  
>>>  #include <errno.h>
>>>  #include <signal.h>
>>> -#include <sigsetops.h>
>>> +#include <internal-signals.h>
>>>  
>>>  /* Add SIGNO to SET.  */
>>>  int
>>>  sigaddset (sigset_t *set, int signo)
>>>  {
>>> -  if (set == NULL || signo <= 0 || signo >= NSIG)
>>> +  if (set == NULL || signo <= 0 || signo >= NSIG
>>> +      || __is_internal_signal (signo))
>>>      {
>>>        __set_errno (EINVAL);
>>>        return -1;
>>> diff --git a/signal/sigdelset.c b/signal/sigdelset.c
>>> index 2aaa536..01a50ec 100644
>>> --- a/signal/sigdelset.c
>>> +++ b/signal/sigdelset.c
>>> @@ -17,13 +17,14 @@
>>>  
>>>  #include <errno.h>
>>>  #include <signal.h>
>>> -#include <sigsetops.h>
>>> +#include <internal-signals.h>
>>>  
>>>  /* Add SIGNO to SET.  */
>>>  int
>>>  sigdelset (sigset_t *set, int signo)
>>>  {
>>> -  if (set == NULL || signo <= 0 || signo >= NSIG)
>>> +  if (set == NULL || signo <= 0 || signo >= NSIG
>>> +      || __is_internal_signal (signo))
>>>      {
>>>        __set_errno (EINVAL);
>>>        return -1;
>>> diff --git a/signal/sigfillset.c b/signal/sigfillset.c
>>> index 0fcc24a..560c66e 100644
>>> --- a/signal/sigfillset.c
>>> +++ b/signal/sigfillset.c
>>> @@ -18,6 +18,7 @@
>>>  #include <errno.h>
>>>  #include <signal.h>
>>>  #include <string.h>
>>> +#include <internal-signals.h>
>>>  
>>>  /* Set all signals in SET.  */
>>>  int
>>> @@ -31,14 +32,7 @@ sigfillset (sigset_t *set)
>>>  
>>>    memset (set, 0xff, sizeof (sigset_t));
>>>  
>>> -  /* If the implementation uses a cancellation signal don't set the bit.  */
>>> -#ifdef SIGCANCEL
>>> -  __sigdelset (set, SIGCANCEL);
>>> -#endif
>>> -  /* Likewise for the signal to implement setxid.  */
>>> -#ifdef SIGSETXID
>>> -  __sigdelset (set, SIGSETXID);
>>> -#endif
>>> +  __clear_internal_signals (set);
>>>  
>>>    return 0;
>>>  }
>>> diff --git a/signal/tst-sigset.c b/signal/tst-sigset.c
>>> index d47adcc..a2b764d 100644
>>> --- a/signal/tst-sigset.c
>>> +++ b/signal/tst-sigset.c
>>> @@ -1,43 +1,85 @@
>>>  /* Test sig*set functions.  */
>>>  
>>>  #include <signal.h>
>>> -#include <stdio.h>
>>>  
>>> -#define TEST_FUNCTION do_test ()
>>> +#include <support/check.h>
>>> +
>>>  static int
>>>  do_test (void)
>>>  {
>>> -  int result = 0;
>>> -  int sig = -1;
>>> +  sigset_t set;
>>> +  TEST_VERIFY (sigemptyset (&set) == 0);
>>>  
>>> -#define TRY(call)							      \
>>> -  if (call)								      \
>>> -    {									      \
>>> -      printf ("%s (sig = %d): %m\n", #call, sig);			      \
>>> -      result = 1;							      \
>>> -    }									      \
>>> -  else
>>> +#define VERIFY(set, sig)			\
>>> +  TEST_VERIFY (sigismember (&set, sig) == 0);	\
>>> +  TEST_VERIFY (sigaddset (&set, sig) == 0);	\
>>> +  TEST_VERIFY (sigismember (&set, sig) != 0);	\
>>> +  TEST_VERIFY (sigdelset (&set, sig) == 0);	\
>>> +  TEST_VERIFY (sigismember (&set, sig) == 0)
>>>  
>>> +  /* ISO C99 signals.  */
>>> +  VERIFY (set, SIGINT);
>>> +  VERIFY (set, SIGILL);
>>> +  VERIFY (set, SIGABRT);
>>> +  VERIFY (set, SIGFPE);
>>> +  VERIFY (set, SIGSEGV);
>>> +  VERIFY (set, SIGTERM);
>>>  
>>> -  sigset_t set;
>>> -  TRY (sigemptyset (&set) != 0);
>>> +  /* Historical signals specified by POSIX. */
>>> +  VERIFY (set, SIGHUP);
>>> +  VERIFY (set, SIGQUIT);
>>> +  VERIFY (set, SIGTRAP);
>>> +  VERIFY (set, SIGKILL);
>>> +  VERIFY (set, SIGBUS);
>>> +  VERIFY (set, SIGSYS);
>>> +  VERIFY (set, SIGPIPE);
>>> +  VERIFY (set, SIGALRM);
>>> +
>>> +  /* New(er) POSIX signals (1003.1-2008, 1003.1-2013).  */
>>> +  VERIFY (set, SIGURG);
>>> +  VERIFY (set, SIGSTOP);
>>> +  VERIFY (set, SIGTSTP);
>>> +  VERIFY (set, SIGCONT);
>>> +  VERIFY (set, SIGCHLD);
>>> +  VERIFY (set, SIGTTIN);
>>> +  VERIFY (set, SIGTTOU);
>>> +  VERIFY (set, SIGPOLL);
>>> +  VERIFY (set, SIGXCPU);
>>> +  VERIFY (set, SIGXFSZ);
>>> +  VERIFY (set, SIGVTALRM);
>>> +  VERIFY (set, SIGPROF);
>>> +  VERIFY (set, SIGUSR1);
>>> +  VERIFY (set, SIGUSR2);
>>> +
>>> +  /* Nonstandard signals found in all modern POSIX systems
>>> +     (including both BSD and Linux).  */
>>> +  VERIFY (set, SIGWINCH);
>>>  
>>> -#ifdef SIGRTMAX
>>> -  int max_sig = SIGRTMAX;
>>> -#else
>>> -  int max_sig = NSIG - 1;
>>> +  /* Arch-specific signals.  */
>>> +#ifdef SIGEMT
>>> +  VERIFY (set, SIGEMT);
>>> +#endif
>>> +#ifdef SIGLOST
>>> +  VERIFY (set, SIGLOST);
>>> +#endif
>>> +#ifdef SIGINFO
>>> +  VERIFY (set, SIGINFO);
>>> +#endif
>>> +#ifdef SIGSTKFLT
>>> +  VERIFY (set, SIGSTKFLT);
>>> +#endif
>>> +#ifdef SIGPWR
>>> +  VERIFY (set, SIGPWR);
>>>  #endif
>>>  
>>> -  for (sig = 1; sig <= max_sig; ++sig)
>>> +  /* Read-time signals (POSIX.1b real-time extensions).  If they are
>>> +     supported SIGRTMAX value is greater than SIGRTMIN.  */
>>> +  for (int rtsig = SIGRTMIN; rtsig <= SIGRTMAX; rtsig++)
>>>      {
>>> -      TRY (sigismember (&set, sig) != 0);
>>> -      TRY (sigaddset (&set, sig) != 0);
>>> -      TRY (sigismember (&set, sig) == 0);
>>> -      TRY (sigdelset (&set, sig) != 0);
>>> -      TRY (sigismember (&set, sig) != 0);
>>> +      VERIFY (set, rtsig);
>>>      }
>>>  
>>> -  return result;
>>> +  return 0;
>>>  }
>>>  
>>> -#include "../test-skeleton.c"
>>> +#include <support/test-driver.c>
>>> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h
>>> index 55bc07d..8a8854c 100644
>>> --- a/sysdeps/generic/internal-signals.h
>>> +++ b/sysdeps/generic/internal-signals.h
>>> @@ -15,3 +15,14 @@
>>>     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/>.  */
>>> +
>>> +static inline void
>>> +__clear_internal_signals (sigset_t *set)
>>> +{
>>> +}
>>> +
>>> +static inline bool
>>> +__is_internal_signal (int sig)
>>> +{
>>> +  return false;
>>> +}
>>> diff --git a/sysdeps/nptl/sigfillset.c b/sysdeps/nptl/sigfillset.c
>>> deleted file mode 100644
>>> index 50e8512..0000000
>>> --- a/sysdeps/nptl/sigfillset.c
>>> +++ /dev/null
>>> @@ -1,20 +0,0 @@
>>> -/* Copyright (C) 2003-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 <nptl/pthreadP.h>
>>> -
>>> -#include <signal/sigfillset.c>
>>> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c
>>> index 81ba177..87c5d1c 100644
>>> --- a/sysdeps/posix/signal.c
>>> +++ b/sysdeps/posix/signal.c
>>> @@ -18,8 +18,8 @@
>>>  
>>>  #include <errno.h>
>>>  #include <signal.h>
>>> -#include <string.h>	/* For the real memset prototype.  */
>>>  #include <sigsetops.h>
>>> +#include <internal-signals.h>
>>>  
>>>  sigset_t _sigintr attribute_hidden;		/* Set by siginterrupt.  */
>>>  
>>> @@ -31,7 +31,8 @@ __bsd_signal (int sig, __sighandler_t handler)
>>>    struct sigaction act, oact;
>>>  
>>>    /* Check signal extents to protect __sigismember.  */
>>> -  if (handler == SIG_ERR || sig < 1 || sig >= NSIG)
>>> +  if (handler == SIG_ERR || sig < 1 || sig >= NSIG
>>> +      || __is_internal_signal (sig))
>>>      {
>>>        __set_errno (EINVAL);
>>>        return SIG_ERR;
>>> diff --git a/sysdeps/posix/sigset.c b/sysdeps/posix/sigset.c
>>> index a4dfe0a..6234ecf 100644
>>> --- a/sysdeps/posix/sigset.c
>>> +++ b/sysdeps/posix/sigset.c
>>> @@ -31,15 +31,9 @@ sigset (int sig, __sighandler_t disp)
>>>    sigset_t set;
>>>    sigset_t oset;
>>>  
>>> -  /* Check signal extents to protect __sigismember.  */
>>> -  if (disp == SIG_ERR || sig < 1 || sig >= NSIG)
>>> -    {
>>> -      __set_errno (EINVAL);
>>> -      return SIG_ERR;
>>> -    }
>>> -
>>>    __sigemptyset (&set);
>>> -  __sigaddset (&set, sig);
>>> +  if (sigaddset (&set, sig) < 0)
>>> +    return SIG_ERR;
>>>  
>>>    if (disp == SIG_HOLD)
>>>      {
>>> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
>>> index bb8234d..bf33bbc 100644
>>> --- a/sysdeps/unix/sysv/linux/internal-signals.h
>>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
>>> @@ -21,6 +21,8 @@
>>>  
>>>  #include <signal.h>
>>>  #include <sigsetops.h>
>>> +#include <stdbool.h>
>>> +#include <sysdep.h>
>>>  
>>>  /* The signal used for asynchronous cancelation.  */
>>>  #define SIGCANCEL       __SIGRTMIN
>>> @@ -43,7 +45,7 @@ __has_internal_signal (const sigset_t *set)
>>>  }
>>>  
>>>  /* Return is sig is used internally.  */
>>> -static inline int
>>> +static inline bool
>>>  __is_internal_signal (int sig)
>>>  {
>>>    return (sig == SIGCANCEL) || (sig == SIGSETXID);
>>> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c
>>> index 2d96dd4..b4977e3 100644
>>> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c
>>> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c
>>> @@ -24,21 +24,8 @@ int
>>>  __sigtimedwait (const sigset_t *set, siginfo_t *info,
>>>  		const struct timespec *timeout)
>>>  {
>>> -  sigset_t tmpset;
>>> -  if (set != NULL
>>> -      && (__builtin_expect (__sigismember (set, SIGCANCEL), 0)
>>> -	  || __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);
>>> -      __sigdelset (&tmpset, SIGSETXID);
>>> -      set = &tmpset;
>>> -    }
>>> -
>>> -    /* XXX The size argument hopefully will have to be changed to the
>>> -       real size of the user-level sigset_t.  */
>>> +  /* XXX The size argument hopefully will have to be changed to the
>>> +     real size of the user-level sigset_t.  */
>>>    int result = SYSCALL_CANCEL (rt_sigtimedwait, set, info, timeout, _NSIG / 8);
>>>  
>>>    /* The kernel generates a SI_TKILL code in si_code in case tkill is
>>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h
  2018-01-01 21:56   ` Adhemerval Zanella
@ 2018-01-17 11:17     ` Adhemerval Zanella
  0 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2018-01-17 11:17 UTC (permalink / raw)
  To: libc-alpha

Ping.

On 01/01/2018 19:56, Adhemerval Zanella wrote:
> Ping.
> 
> On 12/12/2017 15:03, Adhemerval Zanella wrote:
>> Ping.
>>
>> On 15/11/2017 15:54, Adhemerval Zanella wrote:
>>> Changes from previous version:
>>>
>>>   - Cleanup __nptl prefix from the generic names.
>>>
>>>   - Remove superflous SIGTERM handling on Linux __is_internal_signal
>>>     and __clear_internal_signals.
>>>
>>> ---
>>>
>>> This patch renames the nptl-signals.h header to internal-signals.h.
>>> On Linux the definitions and functions are not only NPTL related, but
>>> used for other POSIX definitions as well (for instance SIGTIMER for
>>> posix times, SIGSETXID for id functions, and signal block/restore
>>> helpers) and since generic functions will be places and used in generic
>>> implementation it makes more sense to decouple it from NPTL.
>>>
>>> Checked on x86_64-linux-gnu.
>>>
>>> 	* sysdeps/nptl/nptl-signals.h: Move to ...
>>> 	* sysdeps/generic/internal-signals.h: ... here.  Adjust internal
>>> 	comments.
>>> 	* sysdeps/unix/sysv/linux/internal-signals.h: Add include guards.
>>> 	(__nptl_is_internal_signal): Rename to __is_internal_signal.
>>> 	(__nptl_clear_internal_signals): Rename to __clear_internal_signals.
>>> 	* sysdeps/unix/sysv/linux/raise.c: Adjust nptl-signal.h to
>>> 	include-signals.h rename.
>>> 	* nptl/pthreadP.h: Likewise.
>>> 	* sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Call
>>> 	__is_internal_signal instead of __nptl_is_internal_signal.
>>>
>>> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>> ---
>>>  ChangeLog                                                | 12 ++++++++++++
>>>  nptl/pthreadP.h                                          |  2 +-
>>>  .../{nptl/nptl-signals.h => generic/internal-signals.h}  |  7 +------
>>>  .../sysv/linux/{nptl-signals.h => internal-signals.h}    | 16 ++++++++++------
>>>  sysdeps/unix/sysv/linux/raise.c                          |  2 +-
>>>  sysdeps/unix/sysv/linux/spawni.c                         |  2 +-
>>>  6 files changed, 26 insertions(+), 15 deletions(-)
>>>  rename sysdeps/{nptl/nptl-signals.h => generic/internal-signals.h} (74%)
>>>  rename sysdeps/unix/sysv/linux/{nptl-signals.h => internal-signals.h} (89%)
>>>
>>> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
>>> index 1cc80b6..f3ec832 100644
>>> --- a/nptl/pthreadP.h
>>> +++ b/nptl/pthreadP.h
>>> @@ -32,7 +32,7 @@
>>>  #include <atomic.h>
>>>  #include <kernel-features.h>
>>>  #include <errno.h>
>>> -#include <nptl-signals.h>
>>> +#include <internal-signals.h>
>>>  
>>>  
>>>  /* Atomic operations on TLS memory.  */
>>> diff --git a/sysdeps/nptl/nptl-signals.h b/sysdeps/generic/internal-signals.h
>>> similarity index 74%
>>> rename from sysdeps/nptl/nptl-signals.h
>>> rename to sysdeps/generic/internal-signals.h
>>> index 298acf2..55bc07d 100644
>>> --- a/sysdeps/nptl/nptl-signals.h
>>> +++ b/sysdeps/generic/internal-signals.h
>>> @@ -1,4 +1,4 @@
>>> -/* Special use of signals in NPTL internals.  Stub version.
>>> +/* Special use of signals internally.  Stub version.
>>>     Copyright (C) 2014-2017 Free Software Foundation, Inc.
>>>     This file is part of the GNU C Library.
>>>  
>>> @@ -15,8 +15,3 @@
>>>     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/>.  */
>>> -
>>> -/* This file can define the macros SIGCANCEL, SIGTIMER, and SIGSETXID to
>>> -   signal numbers reserved by libpthread for those internal purposes.
>>> -
>>> -   Note that some code presumes SIGTIMER is the same as SIGCANCEL.  */
>>> diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
>>> similarity index 89%
>>> rename from sysdeps/unix/sysv/linux/nptl-signals.h
>>> rename to sysdeps/unix/sysv/linux/internal-signals.h
>>> index f30c597..e96a718 100644
>>> --- a/sysdeps/unix/sysv/linux/nptl-signals.h
>>> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
>>> @@ -1,4 +1,4 @@
>>> -/* Special use of signals in NPTL internals.  Linux version.
>>> +/* Special use of signals internally.  Linux version.
>>>     Copyright (C) 2014-2017 Free Software Foundation, Inc.
>>>     This file is part of the GNU C Library.
>>>  
>>> @@ -16,6 +16,9 @@
>>>     License along with the GNU C Library; if not, see
>>>     <http://www.gnu.org/licenses/>.  */
>>>  
>>> +#ifndef __INTERNAL_SIGNALS_H
>>> +# define __INTERNAL_SIGNALS_H
>>> +
>>>  #include <signal.h>
>>>  #include <sigsetops.h>
>>>  
>>> @@ -35,17 +38,16 @@
>>>  
>>>  /* Return is sig is used internally.  */
>>>  static inline int
>>> -__nptl_is_internal_signal (int sig)
>>> +__is_internal_signal (int sig)
>>>  {
>>> -  return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
>>> +  return (sig == SIGCANCEL) || (sig == SIGSETXID);
>>>  }
>>>  
>>>  /* Remove internal glibc signal from the mask.  */
>>>  static inline void
>>> -__nptl_clear_internal_signals (sigset_t *set)
>>> +__clear_internal_signals (sigset_t *set)
>>>  {
>>>    __sigdelset (set, SIGCANCEL);
>>> -  __sigdelset (set, SIGTIMER);
>>>    __sigdelset (set, SIGSETXID);
>>>  }
>>>  
>>> @@ -66,7 +68,7 @@ static inline int
>>>  __libc_signal_block_app (sigset_t *set)
>>>  {
>>>    sigset_t allset = SIGALL_SET;
>>> -  __nptl_clear_internal_signals (&allset);
>>> +  __clear_internal_signals (&allset);
>>>    INTERNAL_SYSCALL_DECL (err);
>>>    return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,
>>>  			   _NSIG / 8);
>>> @@ -83,3 +85,5 @@ __libc_signal_restore_set (const sigset_t *set)
>>>  
>>>  /* Used to communicate with signal handler.  */
>>>  extern struct xid_command *__xidcmd attribute_hidden;
>>> +
>>> +#endif
>>> diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c
>>> index 70e5a98..1167950 100644
>>> --- a/sysdeps/unix/sysv/linux/raise.c
>>> +++ b/sysdeps/unix/sysv/linux/raise.c
>>> @@ -21,7 +21,7 @@
>>>  #include <errno.h>
>>>  #include <sys/types.h>
>>>  #include <unistd.h>
>>> -#include <nptl-signals.h>
>>> +#include <internal-signals.h>
>>>  
>>>  int
>>>  raise (int sig)
>>> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
>>> index fb83c2e..af464b5 100644
>>> --- a/sysdeps/unix/sysv/linux/spawni.c
>>> +++ b/sysdeps/unix/sysv/linux/spawni.c
>>> @@ -144,7 +144,7 @@ __spawni_child (void *arguments)
>>>  	}
>>>        else if (sigismember (&hset, sig))
>>>  	{
>>> -	  if (__nptl_is_internal_signal (sig))
>>> +	  if (__is_internal_signal (sig))
>>>  	    sa.sa_handler = SIG_IGN;
>>>  	  else
>>>  	    {
>>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-01-17 11:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-15 17:54 [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
2017-11-15 17:54 ` [PATCH v3 2/3] Consolidate Linux sigprocmask implementation (BZ #22391) Adhemerval Zanella
2017-11-16  1:10   ` Rical Jasan
2017-11-16 10:36     ` Adhemerval Zanella
2017-11-16 12:11       ` Rical Jasan
2017-12-12 17:03   ` Adhemerval Zanella
2018-01-01 21:56     ` Adhemerval Zanella
2018-01-17 11:17       ` Adhemerval Zanella
2017-11-15 17:54 ` [PATCH v3 3/3] Filter out NPTL internal signals " Adhemerval Zanella
2017-12-12 17:03   ` Adhemerval Zanella
2018-01-01 21:56     ` Adhemerval Zanella
2018-01-17 11:17       ` Adhemerval Zanella
2017-12-12 17:03 ` [PATCH v3 1/3] Rename nptl-signals.h to internal-signals.h Adhemerval Zanella
2018-01-01 21:56   ` Adhemerval Zanella
2018-01-17 11:17     ` Adhemerval Zanella

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).