[PATCH v2] Refactor internal-signals.h

[PATCH v2] Refactor internal-signals.h

[Adhemerval Zanella]

The main drive is to optimize the internal usage and required size
when sigset_t is embedded in other data structures.  On Linux, the
current supported signal set requires up to 8 bytes (16 on mips),
was lower than the user defined sigset_t (128 bytes).

A new internal type internal_sigset_t is added, along with the
functions to operate on it similar to the ones for sigset_t.  The
internal-signals.h is also refactored to remove unused functions

Besides small stack usage on some functions (posix_spawn, abort)
it lower the struct pthread by about 120 bytes (112 on mips).

Checked on x86_64-linux-gnu.
---
v2:
- Fixed typos.
- Removed unused signal_block_sigtimer function.
- Removed unused signal_unblock_sigtimer argument.
---
 nptl/descr.h                                  |   3 +-
 nptl/pthread_attr_setsigmask.c                |   2 +-
 nptl/pthread_create.c                         |  16 +--
 nptl/pthread_kill.c                           |  10 +-
 nptl/pthread_sigmask.c                        |   2 +-
 rt/tst-timer-sigmask.c                        |   2 +-
 signal/sigaction.c                            |   2 +-
 signal/sigaddset.c                            |   2 +-
 signal/sigdelset.c                            |   2 +-
 signal/sigfillset.c                           |   2 +-
 stdlib/abort.c                                |  10 +-
 sysdeps/generic/internal-signals.h            |  31 +-----
 sysdeps/posix/signal.c                        |   2 +-
 sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c |   2 +-
 sysdeps/unix/sysv/linux/internal-signals.h    |  61 +++++-----
 sysdeps/unix/sysv/linux/internal-sigset.h     | 105 ++++++++++++++++++
 sysdeps/unix/sysv/linux/spawni.c              |  14 ++-
 sysdeps/unix/sysv/linux/timer_routines.c      |   2 +-
 18 files changed, 178 insertions(+), 92 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/internal-sigset.h

diff --git a/nptl/descr.h b/nptl/descr.h
index b5852632e3..5cacb286f3 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -35,6 +35,7 @@
 #include <kernel-features.h>
 #include <tls-internal-struct.h>
 #include <sys/rseq.h>
+#include <internal-sigset.h>
 
 #ifndef TCB_ALIGNMENT
 # define TCB_ALIGNMENT 32
@@ -387,7 +388,7 @@ struct pthread
   /* Signal mask for the new thread.  Used during thread startup to
      restore the signal mask.  (Threads are launched with all signals
      masked.)  */
-  sigset_t sigmask;
+  internal_sigset_t sigmask;
 
   /* Indicates whether is a C11 thread created by thrd_creat.  */
   bool c11;
diff --git a/nptl/pthread_attr_setsigmask.c b/nptl/pthread_attr_setsigmask.c
index a6e650f4cc..908a0c13ef 100644
--- a/nptl/pthread_attr_setsigmask.c
+++ b/nptl/pthread_attr_setsigmask.c
@@ -28,7 +28,7 @@ pthread_attr_setsigmask_np (pthread_attr_t *attr, const sigset_t *sigmask)
 
   /* Filter out internal signals.  */
   struct pthread_attr *iattr = (struct pthread_attr *) attr;
-  __clear_internal_signals (&iattr->extension->sigmask);
+  clear_internal_signals (&iattr->extension->sigmask);
 
   return 0;
 }
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index a06d611e32..308db65cd4 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -423,7 +423,7 @@ start_thread (void *arg)
       /* Store the new cleanup handler info.  */
       THREAD_SETMEM (pd, cleanup_jmp_buf, &unwind_buf);
 
-      __libc_signal_restore_set (&pd->sigmask);
+      internal_signal_restore_set (&pd->sigmask);
 
       LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg);
 
@@ -501,8 +501,8 @@ start_thread (void *arg)
      signal to be delivered.  (SIGSETXID cannot run application code,
      nor does it use pthread_kill.)  Reuse the pd->sigmask space for
      computing the signal mask, to save stack space.  */
-  __sigfillset (&pd->sigmask);
-  __sigdelset (&pd->sigmask, SIGSETXID);
+  internal_sigfillset (&pd->sigmask);
+  internal_sigdelset (&pd->sigmask, SIGSETXID);
   INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &pd->sigmask, NULL,
 			 __NSIG_BYTES);
 
@@ -769,14 +769,14 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
   /* Block all signals, so that the new thread starts out with
      signals disabled.  This avoids race conditions in the thread
      startup.  */
-  sigset_t original_sigmask;
-  __libc_signal_block_all (&original_sigmask);
+  internal_sigset_t original_sigmask;
+  internal_signal_block_all (&original_sigmask);
 
   if (iattr->extension != NULL && iattr->extension->sigmask_set)
     /* Use the signal mask in the attribute.  The internal signals
        have already been filtered by the public
        pthread_attr_setsigmask_np interface.  */
-    pd->sigmask = iattr->extension->sigmask;
+    internal_sigset_from_sigset (&pd->sigmask, &iattr->extension->sigmask);
   else
     {
       /* Conceptually, the new thread needs to inherit the signal mask
@@ -786,7 +786,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
       pd->sigmask = original_sigmask;
       /* Reset the cancellation signal mask in case this thread is
 	 running cancellation.  */
-      __sigdelset (&pd->sigmask, SIGCANCEL);
+      internal_sigdelset (&pd->sigmask, SIGCANCEL);
     }
 
   /* Start the thread.  */
@@ -833,7 +833,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
 
   /* Return to the previous signal mask, after creating the new
      thread.  */
-  __libc_signal_restore_set (&original_sigmask);
+  internal_signal_restore_set (&original_sigmask);
 
   if (__glibc_unlikely (retval != 0))
     {
diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c
index 8015e10b70..178d38d3bd 100644
--- a/nptl/pthread_kill.c
+++ b/nptl/pthread_kill.c
@@ -45,8 +45,8 @@ __pthread_kill_implementation (pthread_t threadid, int signo, int no_tid)
     }
 
   /* Block all signals, as required by pd->exit_lock.  */
-  sigset_t old_mask;
-  __libc_signal_block_all (&old_mask);
+  internal_sigset_t old_mask;
+  internal_signal_block_all (&old_mask);
   __libc_lock_lock (pd->exit_lock);
 
   int ret;
@@ -64,7 +64,7 @@ __pthread_kill_implementation (pthread_t threadid, int signo, int no_tid)
     }
 
   __libc_lock_unlock (pd->exit_lock);
-  __libc_signal_restore_set (&old_mask);
+  internal_signal_restore_set (&old_mask);
 
   return ret;
 }
@@ -83,7 +83,7 @@ __pthread_kill (pthread_t threadid, int signo)
 {
   /* Disallow sending the signal we use for cancellation, timers,
      for the setxid implementation.  */
-  if (__is_internal_signal (signo))
+  if (is_internal_signal (signo))
     return EINVAL;
 
   return __pthread_kill_internal (threadid, signo);
@@ -102,7 +102,7 @@ int
 attribute_compat_text_section
 __pthread_kill_esrch (pthread_t threadid, int signo)
 {
-  if (__is_internal_signal (signo))
+  if (is_internal_signal (signo))
     return EINVAL;
 
   return __pthread_kill_implementation (threadid, signo, ESRCH);
diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c
index 20f811cc6b..977740b97e 100644
--- a/nptl/pthread_sigmask.c
+++ b/nptl/pthread_sigmask.c
@@ -32,7 +32,7 @@ __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
          || __glibc_unlikely (__sigismember (newmask, SIGSETXID))))
     {
       local_newmask = *newmask;
-      __clear_internal_signals (&local_newmask);
+      clear_internal_signals (&local_newmask);
       newmask = &local_newmask;
     }
 
diff --git a/rt/tst-timer-sigmask.c b/rt/tst-timer-sigmask.c
index 11acb670ab..4d088a691d 100644
--- a/rt/tst-timer-sigmask.c
+++ b/rt/tst-timer-sigmask.c
@@ -43,7 +43,7 @@ thread_handler (union sigval sv)
       if (sigismember (&ss, sig))
 	{
 	  TEST_VERIFY (sig != SIGKILL && sig != SIGSTOP);
-	  TEST_VERIFY (!__is_internal_signal (sig));
+	  TEST_VERIFY (!is_internal_signal (sig));
 	}
       if (test_verbose && sigismember (&ss, sig))
 	printf ("%d, ", sig);
diff --git a/signal/sigaction.c b/signal/sigaction.c
index 8e94b9a10b..d56b3196ca 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 || __is_internal_signal (sig))
+  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 c6ef0a953a..f0fed84dd7 100644
--- a/signal/sigaddset.c
+++ b/signal/sigaddset.c
@@ -25,7 +25,7 @@ int
 sigaddset (sigset_t *set, int signo)
 {
   if (set == NULL || signo <= 0 || signo >= NSIG
-      || __is_internal_signal (signo))
+      || is_internal_signal (signo))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/signal/sigdelset.c b/signal/sigdelset.c
index 81bb9a908a..2c973c9b84 100644
--- a/signal/sigdelset.c
+++ b/signal/sigdelset.c
@@ -25,7 +25,7 @@ int
 sigdelset (sigset_t *set, int signo)
 {
   if (set == NULL || signo <= 0 || signo >= NSIG
-      || __is_internal_signal (signo))
+      || is_internal_signal (signo))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/signal/sigfillset.c b/signal/sigfillset.c
index b52ef06aa0..1d11ae4fec 100644
--- a/signal/sigfillset.c
+++ b/signal/sigfillset.c
@@ -31,7 +31,7 @@ sigfillset (sigset_t *set)
     }
 
   __sigfillset (set);
-  __clear_internal_signals (set);
+  clear_internal_signals (set);
   return 0;
 }
 libc_hidden_def (sigfillset)
diff --git a/stdlib/abort.c b/stdlib/abort.c
index bcf72f9356..9c1065af0e 100644
--- a/stdlib/abort.c
+++ b/stdlib/abort.c
@@ -21,7 +21,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <sigsetops.h>
+#include <internal-signals.h>
 
 /* Try to get a machine dependent instruction which will make the
    program crash.  This is used in case everything else fails.  */
@@ -48,7 +48,6 @@ void
 abort (void)
 {
   struct sigaction act;
-  sigset_t sigs;
 
   /* First acquire the lock.  */
   __libc_lock_lock_recursive (lock);
@@ -59,9 +58,10 @@ abort (void)
   if (stage == 0)
     {
       ++stage;
-      __sigemptyset (&sigs);
-      __sigaddset (&sigs, SIGABRT);
-      __sigprocmask (SIG_UNBLOCK, &sigs, 0);
+      internal_sigset_t sigs;
+      internal_sigemptyset (&sigs);
+      internal_sigaddset (&sigs, SIGABRT);
+      internal_sigprocmask (SIG_UNBLOCK, &sigs, NULL);
     }
 
   /* Send signal which possibly calls a user handler.  */
diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h
index 6121c117ab..0c8f67f1a2 100644
--- a/sysdeps/generic/internal-signals.h
+++ b/sysdeps/generic/internal-signals.h
@@ -29,39 +29,20 @@
 #define RESERVED_SIGRT  0
 
 static inline bool
-__is_internal_signal (int sig)
+is_internal_signal (int sig)
 {
   return false;
 }
 
 static inline void
-__clear_internal_signals (sigset_t *set)
+clear_internal_signals (sigset_t *set)
 {
 }
 
-static inline void
-__libc_signal_block_all (sigset_t *set)
-{
-  sigset_t allset;
-  __sigfillset (&allset);
-  __sigprocmask (SIG_BLOCK, &allset, set);
-}
-
-static inline void
-__libc_signal_block_app (sigset_t *set)
-{
-  sigset_t allset;
-  __sigfillset (&allset);
-  __clear_internal_signals (&allset);
-  __sigprocmask (SIG_BLOCK, &allset, set);
-}
-
-/* Restore current process signal mask.  */
-static inline void
-__libc_signal_restore_set (const sigset_t *set)
-{
-  __sigprocmask (SIG_SETMASK, set, NULL);
-}
+typedef sigset_t internal_sigset_t;
 
+#define internal_sigemptyset(__s)            sigemptyset (__s)
+#define internal_sigaddset(__s, __i)	     sigaddset (__s, __i)
+#define internal_sigprocmask(__h, __s, __o)  sigprocmask (__h, __s, __o)
 
 #endif /* __INTERNAL_SIGNALS_H  */
diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c
index 8f17b4278d..88c3757126 100644
--- a/sysdeps/posix/signal.c
+++ b/sysdeps/posix/signal.c
@@ -32,7 +32,7 @@ __bsd_signal (int sig, __sighandler_t handler)
 
   /* Check signal extents to protect __sigismember.  */
   if (handler == SIG_ERR || sig < 1 || sig >= NSIG
-      || __is_internal_signal (sig))
+      || is_internal_signal (sig))
     {
       __set_errno (EINVAL);
       return SIG_ERR;
diff --git a/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c b/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c
index a59ad78036..a53ae79965 100644
--- a/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c
+++ b/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c
@@ -34,7 +34,7 @@ __libc_unwind_longjmp (sigjmp_buf env, int val)
 
   if (env[0].__mask_was_saved)
     /* Restore the saved signal mask.  */
-    __libc_signal_restore_set (&env[0].__saved_mask);
+    __sigprocmask (SIG_SETMASK, &env[0].__saved_mask, NULL);
 
   /* Call the machine-dependent function to restore machine state.  */
   __sigstack_longjmp (env[0].__jmpbuf, val ?: 1);
diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
index f9efb6a159..f30cbea07d 100644
--- a/sysdeps/unix/sysv/linux/internal-signals.h
+++ b/sysdeps/unix/sysv/linux/internal-signals.h
@@ -19,10 +19,11 @@
 #ifndef __INTERNAL_SIGNALS_H
 # define __INTERNAL_SIGNALS_H
 
+#include <internal-sigset.h>
+#include <limits.h>
 #include <signal.h>
 #include <sigsetops.h>
 #include <stdbool.h>
-#include <limits.h>
 #include <stddef.h>
 #include <sysdep.h>
 
@@ -47,67 +48,63 @@
 
 /* Return is sig is used internally.  */
 static inline bool
-__is_internal_signal (int sig)
+is_internal_signal (int sig)
 {
   return (sig == SIGCANCEL) || (sig == SIGSETXID);
 }
 
 /* Remove internal glibc signal from the mask.  */
 static inline void
-__clear_internal_signals (sigset_t *set)
+clear_internal_signals (sigset_t *set)
 {
   __sigdelset (set, SIGCANCEL);
   __sigdelset (set, SIGSETXID);
 }
 
-static const sigset_t sigall_set = {
-   .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 }
-};
-
-static const sigset_t sigtimer_set = {
-  .__val = { [0]                      = __sigmask (SIGTIMER),
-             [1 ... _SIGSET_NWORDS-1] = 0 }
+static const internal_sigset_t sigall_set = {
+   .__val = {[0 ...  __NSIG_WORDS-1 ] =  -1 }
 };
 
-/* Block all signals, including internal glibc ones.  */
-static inline void
-__libc_signal_block_all (sigset_t *set)
+/* Obtain and change blocked signals, including internal glibc ones.  */
+static inline int
+internal_sigprocmask (int how, const internal_sigset_t *set,
+		      internal_sigset_t *oldset)
 {
-  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &sigall_set, set,
-			 __NSIG_BYTES);
+  return INTERNAL_SYSCALL_CALL (rt_sigprocmask, how, set, oldset,
+				__NSIG_BYTES);
 }
 
-/* Block all application signals (excluding internal glibc ones).  */
+/* Block all signals, including internal glibc ones.  */
 static inline void
-__libc_signal_block_app (sigset_t *set)
+internal_signal_block_all (internal_sigset_t *oset)
 {
-  sigset_t allset = sigall_set;
-  __clear_internal_signals (&allset);
-  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &allset, set,
+  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &sigall_set, oset,
 			 __NSIG_BYTES);
 }
 
-/* Block only SIGTIMER and return the previous set on SET.  */
+/* Restore current process signal mask.  */
 static inline void
-__libc_signal_block_sigtimer (sigset_t *set)
+internal_signal_restore_set (const internal_sigset_t *set)
 {
-  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &sigtimer_set, set,
+  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_SETMASK, set, NULL,
 			 __NSIG_BYTES);
 }
 
+
+/* It is used on timer_create code directly on sigwaitinfo call, so it can not
+   use the internal_sigset_t definitions.  */
+static const sigset_t sigtimer_set = {
+  .__val = { [0]                      = __sigmask (SIGTIMER),
+             [1 ... _SIGSET_NWORDS-1] = 0
+  }
+};
+
 /* Unblock only SIGTIMER and return the previous set on SET.  */
 static inline void
-__libc_signal_unblock_sigtimer (sigset_t *set)
+signal_unblock_sigtimer (void)
 {
-  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_UNBLOCK, &sigtimer_set, set,
+  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_UNBLOCK, &sigtimer_set, NULL,
 			 __NSIG_BYTES);
 }
 
-/* Restore current process signal mask.  */
-static inline void
-__libc_signal_restore_set (const sigset_t *set)
-{
-  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_SETMASK, set, NULL,
-			 __NSIG_BYTES);
-}
 #endif
diff --git a/sysdeps/unix/sysv/linux/internal-sigset.h b/sysdeps/unix/sysv/linux/internal-sigset.h
new file mode 100644
index 0000000000..46939ddf15
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/internal-sigset.h
@@ -0,0 +1,105 @@
+/* Internal sigset_t definition.
+   Copyright (C) 2022 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _INTERNAL_SIGSET_H
+#define _INTERNAL_SIGSET_H
+
+#include <sigsetops.h>
+
+typedef struct
+{
+  unsigned long int __val[__NSIG_WORDS];
+} internal_sigset_t;
+
+static inline void
+internal_sigset_from_sigset (internal_sigset_t *iset, const sigset_t *set)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+   iset->__val[cnt] = set->__val[cnt];
+}
+
+static inline void
+internal_sigemptyset (internal_sigset_t *set)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+   set->__val[cnt] = 0;
+}
+
+static inline void
+internal_sigfillset (internal_sigset_t *set)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+   set->__val[cnt] = ~0UL;
+}
+
+static inline int
+internal_sigisemptyset (const internal_sigset_t *set)
+{
+  int cnt = __NSIG_WORDS;
+  int ret = set->__val[--cnt];
+  while (ret == 0 && --cnt >= 0)
+    ret = set->__val[cnt];
+  return ret == 0;
+}
+
+static inline void
+internal_sigandset (internal_sigset_t *dest, const internal_sigset_t *left,
+		    const internal_sigset_t *right)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+    dest->__val[cnt] = left->__val[cnt] & right->__val[cnt];
+}
+
+static inline void
+internal_sigorset (internal_sigset_t *dest, const internal_sigset_t *left,
+		   const internal_sigset_t *right)
+{
+  int cnt = __NSIG_WORDS;
+  while (--cnt >= 0)
+    dest->__val[cnt] = left->__val[cnt] | right->__val[cnt];
+}
+
+static inline int
+internal_sigismember (const internal_sigset_t *set, int sig)
+{
+  unsigned long int mask = __sigmask (sig);
+  unsigned long int word = __sigword (sig);
+  return set->__val[word] & mask ? 1 : 0;
+}
+
+static inline void
+internal_sigaddset (internal_sigset_t *set, int sig)
+{
+  unsigned long int mask = __sigmask (sig);
+  unsigned long int word = __sigword (sig);
+  set->__val[word] |= mask;
+}
+
+static inline void
+internal_sigdelset (internal_sigset_t *set, int sig)
+{
+  unsigned long int mask = __sigmask (sig);
+  unsigned long int word = __sigword (sig);
+  set->__val[word] &= ~mask;
+}
+
+#endif /* _INTERNAL_SIGSET_H */
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index d6f5ca89cd..ee843a2247 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -57,7 +57,7 @@
 
 struct posix_spawn_args
 {
-  sigset_t oldmask;
+  internal_sigset_t oldmask;
   const char *file;
   int (*exec) (const char *, char *const *, char *const *);
   const posix_spawn_file_actions_t *fa;
@@ -124,7 +124,7 @@ __spawni_child (void *arguments)
 	}
       else if (__sigismember (&hset, sig))
 	{
-	  if (__is_internal_signal (sig))
+	  if (is_internal_signal (sig))
 	    sa.sa_handler = SIG_IGN;
 	  else
 	    {
@@ -284,8 +284,10 @@ __spawni_child (void *arguments)
 
   /* Set the initial signal mask of the child if POSIX_SPAWN_SETSIGMASK
      is set, otherwise restore the previous one.  */
-  __sigprocmask (SIG_SETMASK, (attr->__flags & POSIX_SPAWN_SETSIGMASK)
-		 ? &attr->__ss : &args->oldmask, 0);
+  if (attr->__flags & POSIX_SPAWN_SETSIGMASK)
+    __sigprocmask (SIG_SETMASK, &attr->__ss, NULL);
+  else
+    internal_sigprocmask (SIG_SETMASK, &args->oldmask, NULL);
 
   args->exec (args->file, args->argv, args->envp);
 
@@ -368,7 +370,7 @@ __spawnix (pid_t * pid, const char *file,
   args.envp = envp;
   args.xflags = xflags;
 
-  __libc_signal_block_all (&args.oldmask);
+  internal_signal_block_all (&args.oldmask);
 
   /* The clone flags used will create a new child that will run in the same
      memory space (CLONE_VM) and the execution of calling thread will be
@@ -416,7 +418,7 @@ __spawnix (pid_t * pid, const char *file,
   if ((ec == 0) && (pid != NULL))
     *pid = new_pid;
 
-  __libc_signal_restore_set (&args.oldmask);
+  internal_signal_restore_set (&args.oldmask);
 
   __pthread_setcancelstate (state, NULL);
 
diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c
index 8f0a2f635d..7433f7f559 100644
--- a/sysdeps/unix/sysv/linux/timer_routines.c
+++ b/sysdeps/unix/sysv/linux/timer_routines.c
@@ -41,7 +41,7 @@ struct thread_start_data
 static void *
 timer_sigev_thread (void *arg)
 {
-  __libc_signal_unblock_sigtimer (NULL);
+  signal_unblock_sigtimer ();
 
   struct thread_start_data *td = (struct thread_start_data *) arg;
   void (*thrfunc) (sigval_t) = td->thrfunc;
-- 
2.34.1

Re: [PATCH v2] Refactor internal-signals.h

[Arjun Shankar]

Hi Adhemerval,

Thank you! This LGTM with one change:

In signal_unblock_sigtimer, the reference to SET needs to be removed:
> /* Unblock only SIGTIMER and return the previous set on SET.  */

Reviewed-by: Arjun Shankar <arjun@redhat.com>

On Wed, Jun 29, 2022 at 10:02 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> The main drive is to optimize the internal usage and required size
> when sigset_t is embedded in other data structures.  On Linux, the
> current supported signal set requires up to 8 bytes (16 on mips),
> was lower than the user defined sigset_t (128 bytes).
>
> A new internal type internal_sigset_t is added, along with the
> functions to operate on it similar to the ones for sigset_t.  The
> internal-signals.h is also refactored to remove unused functions
>
> Besides small stack usage on some functions (posix_spawn, abort)
> it lower the struct pthread by about 120 bytes (112 on mips).
>
> Checked on x86_64-linux-gnu.
> ---
> v2:
> - Fixed typos.
> - Removed unused signal_block_sigtimer function.
> - Removed unused signal_unblock_sigtimer argument.
> ---
>  nptl/descr.h                                  |   3 +-
>  nptl/pthread_attr_setsigmask.c                |   2 +-
>  nptl/pthread_create.c                         |  16 +--
>  nptl/pthread_kill.c                           |  10 +-
>  nptl/pthread_sigmask.c                        |   2 +-
>  rt/tst-timer-sigmask.c                        |   2 +-
>  signal/sigaction.c                            |   2 +-
>  signal/sigaddset.c                            |   2 +-
>  signal/sigdelset.c                            |   2 +-
>  signal/sigfillset.c                           |   2 +-
>  stdlib/abort.c                                |  10 +-
>  sysdeps/generic/internal-signals.h            |  31 +-----
>  sysdeps/posix/signal.c                        |   2 +-
>  sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c |   2 +-
>  sysdeps/unix/sysv/linux/internal-signals.h    |  61 +++++-----
>  sysdeps/unix/sysv/linux/internal-sigset.h     | 105 ++++++++++++++++++
>  sysdeps/unix/sysv/linux/spawni.c              |  14 ++-
>  sysdeps/unix/sysv/linux/timer_routines.c      |   2 +-
>  18 files changed, 178 insertions(+), 92 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/internal-sigset.h
>
> diff --git a/nptl/descr.h b/nptl/descr.h
> index b5852632e3..5cacb286f3 100644
> --- a/nptl/descr.h
> +++ b/nptl/descr.h
> @@ -35,6 +35,7 @@
>  #include <kernel-features.h>
>  #include <tls-internal-struct.h>
>  #include <sys/rseq.h>
> +#include <internal-sigset.h>
>
>  #ifndef TCB_ALIGNMENT
>  # define TCB_ALIGNMENT 32
> @@ -387,7 +388,7 @@ struct pthread
>    /* Signal mask for the new thread.  Used during thread startup to
>       restore the signal mask.  (Threads are launched with all signals
>       masked.)  */
> -  sigset_t sigmask;
> +  internal_sigset_t sigmask;
>
>    /* Indicates whether is a C11 thread created by thrd_creat.  */
>    bool c11;
> diff --git a/nptl/pthread_attr_setsigmask.c b/nptl/pthread_attr_setsigmask.c
> index a6e650f4cc..908a0c13ef 100644
> --- a/nptl/pthread_attr_setsigmask.c
> +++ b/nptl/pthread_attr_setsigmask.c
> @@ -28,7 +28,7 @@ pthread_attr_setsigmask_np (pthread_attr_t *attr, const sigset_t *sigmask)
>
>    /* Filter out internal signals.  */
>    struct pthread_attr *iattr = (struct pthread_attr *) attr;
> -  __clear_internal_signals (&iattr->extension->sigmask);
> +  clear_internal_signals (&iattr->extension->sigmask);
>
>    return 0;
>  }
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index a06d611e32..308db65cd4 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -423,7 +423,7 @@ start_thread (void *arg)
>        /* Store the new cleanup handler info.  */
>        THREAD_SETMEM (pd, cleanup_jmp_buf, &unwind_buf);
>
> -      __libc_signal_restore_set (&pd->sigmask);
> +      internal_signal_restore_set (&pd->sigmask);
>
>        LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg);
>
> @@ -501,8 +501,8 @@ start_thread (void *arg)
>       signal to be delivered.  (SIGSETXID cannot run application code,
>       nor does it use pthread_kill.)  Reuse the pd->sigmask space for
>       computing the signal mask, to save stack space.  */
> -  __sigfillset (&pd->sigmask);
> -  __sigdelset (&pd->sigmask, SIGSETXID);
> +  internal_sigfillset (&pd->sigmask);
> +  internal_sigdelset (&pd->sigmask, SIGSETXID);
>    INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &pd->sigmask, NULL,
>                          __NSIG_BYTES);
>
> @@ -769,14 +769,14 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>    /* Block all signals, so that the new thread starts out with
>       signals disabled.  This avoids race conditions in the thread
>       startup.  */
> -  sigset_t original_sigmask;
> -  __libc_signal_block_all (&original_sigmask);
> +  internal_sigset_t original_sigmask;
> +  internal_signal_block_all (&original_sigmask);
>
>    if (iattr->extension != NULL && iattr->extension->sigmask_set)
>      /* Use the signal mask in the attribute.  The internal signals
>         have already been filtered by the public
>         pthread_attr_setsigmask_np interface.  */
> -    pd->sigmask = iattr->extension->sigmask;
> +    internal_sigset_from_sigset (&pd->sigmask, &iattr->extension->sigmask);
>    else
>      {
>        /* Conceptually, the new thread needs to inherit the signal mask
> @@ -786,7 +786,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>        pd->sigmask = original_sigmask;
>        /* Reset the cancellation signal mask in case this thread is
>          running cancellation.  */
> -      __sigdelset (&pd->sigmask, SIGCANCEL);
> +      internal_sigdelset (&pd->sigmask, SIGCANCEL);
>      }
>
>    /* Start the thread.  */
> @@ -833,7 +833,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>
>    /* Return to the previous signal mask, after creating the new
>       thread.  */
> -  __libc_signal_restore_set (&original_sigmask);
> +  internal_signal_restore_set (&original_sigmask);
>
>    if (__glibc_unlikely (retval != 0))
>      {
> diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c
> index 8015e10b70..178d38d3bd 100644
> --- a/nptl/pthread_kill.c
> +++ b/nptl/pthread_kill.c
> @@ -45,8 +45,8 @@ __pthread_kill_implementation (pthread_t threadid, int signo, int no_tid)
>      }
>
>    /* Block all signals, as required by pd->exit_lock.  */
> -  sigset_t old_mask;
> -  __libc_signal_block_all (&old_mask);
> +  internal_sigset_t old_mask;
> +  internal_signal_block_all (&old_mask);
>    __libc_lock_lock (pd->exit_lock);
>
>    int ret;
> @@ -64,7 +64,7 @@ __pthread_kill_implementation (pthread_t threadid, int signo, int no_tid)
>      }
>
>    __libc_lock_unlock (pd->exit_lock);
> -  __libc_signal_restore_set (&old_mask);
> +  internal_signal_restore_set (&old_mask);
>
>    return ret;
>  }
> @@ -83,7 +83,7 @@ __pthread_kill (pthread_t threadid, int signo)
>  {
>    /* Disallow sending the signal we use for cancellation, timers,
>       for the setxid implementation.  */
> -  if (__is_internal_signal (signo))
> +  if (is_internal_signal (signo))
>      return EINVAL;
>
>    return __pthread_kill_internal (threadid, signo);
> @@ -102,7 +102,7 @@ int
>  attribute_compat_text_section
>  __pthread_kill_esrch (pthread_t threadid, int signo)
>  {
> -  if (__is_internal_signal (signo))
> +  if (is_internal_signal (signo))
>      return EINVAL;
>
>    return __pthread_kill_implementation (threadid, signo, ESRCH);
> diff --git a/nptl/pthread_sigmask.c b/nptl/pthread_sigmask.c
> index 20f811cc6b..977740b97e 100644
> --- a/nptl/pthread_sigmask.c
> +++ b/nptl/pthread_sigmask.c
> @@ -32,7 +32,7 @@ __pthread_sigmask (int how, const sigset_t *newmask, sigset_t *oldmask)
>           || __glibc_unlikely (__sigismember (newmask, SIGSETXID))))
>      {
>        local_newmask = *newmask;
> -      __clear_internal_signals (&local_newmask);
> +      clear_internal_signals (&local_newmask);
>        newmask = &local_newmask;
>      }
>
> diff --git a/rt/tst-timer-sigmask.c b/rt/tst-timer-sigmask.c
> index 11acb670ab..4d088a691d 100644
> --- a/rt/tst-timer-sigmask.c
> +++ b/rt/tst-timer-sigmask.c
> @@ -43,7 +43,7 @@ thread_handler (union sigval sv)
>        if (sigismember (&ss, sig))
>         {
>           TEST_VERIFY (sig != SIGKILL && sig != SIGSTOP);
> -         TEST_VERIFY (!__is_internal_signal (sig));
> +         TEST_VERIFY (!is_internal_signal (sig));
>         }
>        if (test_verbose && sigismember (&ss, sig))
>         printf ("%d, ", sig);
> diff --git a/signal/sigaction.c b/signal/sigaction.c
> index 8e94b9a10b..d56b3196ca 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 || __is_internal_signal (sig))
> +  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 c6ef0a953a..f0fed84dd7 100644
> --- a/signal/sigaddset.c
> +++ b/signal/sigaddset.c
> @@ -25,7 +25,7 @@ int
>  sigaddset (sigset_t *set, int signo)
>  {
>    if (set == NULL || signo <= 0 || signo >= NSIG
> -      || __is_internal_signal (signo))
> +      || is_internal_signal (signo))
>      {
>        __set_errno (EINVAL);
>        return -1;
> diff --git a/signal/sigdelset.c b/signal/sigdelset.c
> index 81bb9a908a..2c973c9b84 100644
> --- a/signal/sigdelset.c
> +++ b/signal/sigdelset.c
> @@ -25,7 +25,7 @@ int
>  sigdelset (sigset_t *set, int signo)
>  {
>    if (set == NULL || signo <= 0 || signo >= NSIG
> -      || __is_internal_signal (signo))
> +      || is_internal_signal (signo))
>      {
>        __set_errno (EINVAL);
>        return -1;
> diff --git a/signal/sigfillset.c b/signal/sigfillset.c
> index b52ef06aa0..1d11ae4fec 100644
> --- a/signal/sigfillset.c
> +++ b/signal/sigfillset.c
> @@ -31,7 +31,7 @@ sigfillset (sigset_t *set)
>      }
>
>    __sigfillset (set);
> -  __clear_internal_signals (set);
> +  clear_internal_signals (set);
>    return 0;
>  }
>  libc_hidden_def (sigfillset)
> diff --git a/stdlib/abort.c b/stdlib/abort.c
> index bcf72f9356..9c1065af0e 100644
> --- a/stdlib/abort.c
> +++ b/stdlib/abort.c
> @@ -21,7 +21,7 @@
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> -#include <sigsetops.h>
> +#include <internal-signals.h>
>
>  /* Try to get a machine dependent instruction which will make the
>     program crash.  This is used in case everything else fails.  */
> @@ -48,7 +48,6 @@ void
>  abort (void)
>  {
>    struct sigaction act;
> -  sigset_t sigs;
>
>    /* First acquire the lock.  */
>    __libc_lock_lock_recursive (lock);
> @@ -59,9 +58,10 @@ abort (void)
>    if (stage == 0)
>      {
>        ++stage;
> -      __sigemptyset (&sigs);
> -      __sigaddset (&sigs, SIGABRT);
> -      __sigprocmask (SIG_UNBLOCK, &sigs, 0);
> +      internal_sigset_t sigs;
> +      internal_sigemptyset (&sigs);
> +      internal_sigaddset (&sigs, SIGABRT);
> +      internal_sigprocmask (SIG_UNBLOCK, &sigs, NULL);
>      }
>
>    /* Send signal which possibly calls a user handler.  */
> diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h
> index 6121c117ab..0c8f67f1a2 100644
> --- a/sysdeps/generic/internal-signals.h
> +++ b/sysdeps/generic/internal-signals.h
> @@ -29,39 +29,20 @@
>  #define RESERVED_SIGRT  0
>
>  static inline bool
> -__is_internal_signal (int sig)
> +is_internal_signal (int sig)
>  {
>    return false;
>  }
>
>  static inline void
> -__clear_internal_signals (sigset_t *set)
> +clear_internal_signals (sigset_t *set)
>  {
>  }
>
> -static inline void
> -__libc_signal_block_all (sigset_t *set)
> -{
> -  sigset_t allset;
> -  __sigfillset (&allset);
> -  __sigprocmask (SIG_BLOCK, &allset, set);
> -}
> -
> -static inline void
> -__libc_signal_block_app (sigset_t *set)
> -{
> -  sigset_t allset;
> -  __sigfillset (&allset);
> -  __clear_internal_signals (&allset);
> -  __sigprocmask (SIG_BLOCK, &allset, set);
> -}
> -
> -/* Restore current process signal mask.  */
> -static inline void
> -__libc_signal_restore_set (const sigset_t *set)
> -{
> -  __sigprocmask (SIG_SETMASK, set, NULL);
> -}
> +typedef sigset_t internal_sigset_t;
>
> +#define internal_sigemptyset(__s)            sigemptyset (__s)
> +#define internal_sigaddset(__s, __i)        sigaddset (__s, __i)
> +#define internal_sigprocmask(__h, __s, __o)  sigprocmask (__h, __s, __o)
>
>  #endif /* __INTERNAL_SIGNALS_H  */
> diff --git a/sysdeps/posix/signal.c b/sysdeps/posix/signal.c
> index 8f17b4278d..88c3757126 100644
> --- a/sysdeps/posix/signal.c
> +++ b/sysdeps/posix/signal.c
> @@ -32,7 +32,7 @@ __bsd_signal (int sig, __sighandler_t handler)
>
>    /* Check signal extents to protect __sigismember.  */
>    if (handler == SIG_ERR || sig < 1 || sig >= NSIG
> -      || __is_internal_signal (sig))
> +      || is_internal_signal (sig))
>      {
>        __set_errno (EINVAL);
>        return SIG_ERR;
> diff --git a/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c b/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c
> index a59ad78036..a53ae79965 100644
> --- a/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c
> +++ b/sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c
> @@ -34,7 +34,7 @@ __libc_unwind_longjmp (sigjmp_buf env, int val)
>
>    if (env[0].__mask_was_saved)
>      /* Restore the saved signal mask.  */
> -    __libc_signal_restore_set (&env[0].__saved_mask);
> +    __sigprocmask (SIG_SETMASK, &env[0].__saved_mask, NULL);
>
>    /* Call the machine-dependent function to restore machine state.  */
>    __sigstack_longjmp (env[0].__jmpbuf, val ?: 1);
> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
> index f9efb6a159..f30cbea07d 100644
> --- a/sysdeps/unix/sysv/linux/internal-signals.h
> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
> @@ -19,10 +19,11 @@
>  #ifndef __INTERNAL_SIGNALS_H
>  # define __INTERNAL_SIGNALS_H
>
> +#include <internal-sigset.h>
> +#include <limits.h>
>  #include <signal.h>
>  #include <sigsetops.h>
>  #include <stdbool.h>
> -#include <limits.h>
>  #include <stddef.h>
>  #include <sysdep.h>
>
> @@ -47,67 +48,63 @@
>
>  /* Return is sig is used internally.  */
>  static inline bool
> -__is_internal_signal (int sig)
> +is_internal_signal (int sig)
>  {
>    return (sig == SIGCANCEL) || (sig == SIGSETXID);
>  }
>
>  /* Remove internal glibc signal from the mask.  */
>  static inline void
> -__clear_internal_signals (sigset_t *set)
> +clear_internal_signals (sigset_t *set)
>  {
>    __sigdelset (set, SIGCANCEL);
>    __sigdelset (set, SIGSETXID);
>  }
>
> -static const sigset_t sigall_set = {
> -   .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 }
> -};
> -
> -static const sigset_t sigtimer_set = {
> -  .__val = { [0]                      = __sigmask (SIGTIMER),
> -             [1 ... _SIGSET_NWORDS-1] = 0 }
> +static const internal_sigset_t sigall_set = {
> +   .__val = {[0 ...  __NSIG_WORDS-1 ] =  -1 }
>  };
>
> -/* Block all signals, including internal glibc ones.  */
> -static inline void
> -__libc_signal_block_all (sigset_t *set)
> +/* Obtain and change blocked signals, including internal glibc ones.  */
> +static inline int
> +internal_sigprocmask (int how, const internal_sigset_t *set,
> +                     internal_sigset_t *oldset)
>  {
> -  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &sigall_set, set,
> -                        __NSIG_BYTES);
> +  return INTERNAL_SYSCALL_CALL (rt_sigprocmask, how, set, oldset,
> +                               __NSIG_BYTES);
>  }
>
> -/* Block all application signals (excluding internal glibc ones).  */
> +/* Block all signals, including internal glibc ones.  */
>  static inline void
> -__libc_signal_block_app (sigset_t *set)
> +internal_signal_block_all (internal_sigset_t *oset)
>  {
> -  sigset_t allset = sigall_set;
> -  __clear_internal_signals (&allset);
> -  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &allset, set,
> +  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &sigall_set, oset,
>                          __NSIG_BYTES);
>  }
>
> -/* Block only SIGTIMER and return the previous set on SET.  */
> +/* Restore current process signal mask.  */
>  static inline void
> -__libc_signal_block_sigtimer (sigset_t *set)
> +internal_signal_restore_set (const internal_sigset_t *set)
>  {
> -  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &sigtimer_set, set,
> +  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_SETMASK, set, NULL,
>                          __NSIG_BYTES);
>  }
>
> +
> +/* It is used on timer_create code directly on sigwaitinfo call, so it can not
> +   use the internal_sigset_t definitions.  */
> +static const sigset_t sigtimer_set = {
> +  .__val = { [0]                      = __sigmask (SIGTIMER),
> +             [1 ... _SIGSET_NWORDS-1] = 0
> +  }
> +};
> +
>  /* Unblock only SIGTIMER and return the previous set on SET.  */
>  static inline void
> -__libc_signal_unblock_sigtimer (sigset_t *set)
> +signal_unblock_sigtimer (void)
>  {
> -  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_UNBLOCK, &sigtimer_set, set,
> +  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_UNBLOCK, &sigtimer_set, NULL,
>                          __NSIG_BYTES);
>  }
>
> -/* Restore current process signal mask.  */
> -static inline void
> -__libc_signal_restore_set (const sigset_t *set)
> -{
> -  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_SETMASK, set, NULL,
> -                        __NSIG_BYTES);
> -}
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/internal-sigset.h b/sysdeps/unix/sysv/linux/internal-sigset.h
> new file mode 100644
> index 0000000000..46939ddf15
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/internal-sigset.h
> @@ -0,0 +1,105 @@
> +/* Internal sigset_t definition.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _INTERNAL_SIGSET_H
> +#define _INTERNAL_SIGSET_H
> +
> +#include <sigsetops.h>
> +
> +typedef struct
> +{
> +  unsigned long int __val[__NSIG_WORDS];
> +} internal_sigset_t;
> +
> +static inline void
> +internal_sigset_from_sigset (internal_sigset_t *iset, const sigset_t *set)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +   iset->__val[cnt] = set->__val[cnt];
> +}
> +
> +static inline void
> +internal_sigemptyset (internal_sigset_t *set)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +   set->__val[cnt] = 0;
> +}
> +
> +static inline void
> +internal_sigfillset (internal_sigset_t *set)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +   set->__val[cnt] = ~0UL;
> +}
> +
> +static inline int
> +internal_sigisemptyset (const internal_sigset_t *set)
> +{
> +  int cnt = __NSIG_WORDS;
> +  int ret = set->__val[--cnt];
> +  while (ret == 0 && --cnt >= 0)
> +    ret = set->__val[cnt];
> +  return ret == 0;
> +}
> +
> +static inline void
> +internal_sigandset (internal_sigset_t *dest, const internal_sigset_t *left,
> +                   const internal_sigset_t *right)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +    dest->__val[cnt] = left->__val[cnt] & right->__val[cnt];
> +}
> +
> +static inline void
> +internal_sigorset (internal_sigset_t *dest, const internal_sigset_t *left,
> +                  const internal_sigset_t *right)
> +{
> +  int cnt = __NSIG_WORDS;
> +  while (--cnt >= 0)
> +    dest->__val[cnt] = left->__val[cnt] | right->__val[cnt];
> +}
> +
> +static inline int
> +internal_sigismember (const internal_sigset_t *set, int sig)
> +{
> +  unsigned long int mask = __sigmask (sig);
> +  unsigned long int word = __sigword (sig);
> +  return set->__val[word] & mask ? 1 : 0;
> +}
> +
> +static inline void
> +internal_sigaddset (internal_sigset_t *set, int sig)
> +{
> +  unsigned long int mask = __sigmask (sig);
> +  unsigned long int word = __sigword (sig);
> +  set->__val[word] |= mask;
> +}
> +
> +static inline void
> +internal_sigdelset (internal_sigset_t *set, int sig)
> +{
> +  unsigned long int mask = __sigmask (sig);
> +  unsigned long int word = __sigword (sig);
> +  set->__val[word] &= ~mask;
> +}
> +
> +#endif /* _INTERNAL_SIGSET_H */
> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
> index d6f5ca89cd..ee843a2247 100644
> --- a/sysdeps/unix/sysv/linux/spawni.c
> +++ b/sysdeps/unix/sysv/linux/spawni.c
> @@ -57,7 +57,7 @@
>
>  struct posix_spawn_args
>  {
> -  sigset_t oldmask;
> +  internal_sigset_t oldmask;
>    const char *file;
>    int (*exec) (const char *, char *const *, char *const *);
>    const posix_spawn_file_actions_t *fa;
> @@ -124,7 +124,7 @@ __spawni_child (void *arguments)
>         }
>        else if (__sigismember (&hset, sig))
>         {
> -         if (__is_internal_signal (sig))
> +         if (is_internal_signal (sig))
>             sa.sa_handler = SIG_IGN;
>           else
>             {
> @@ -284,8 +284,10 @@ __spawni_child (void *arguments)
>
>    /* Set the initial signal mask of the child if POSIX_SPAWN_SETSIGMASK
>       is set, otherwise restore the previous one.  */
> -  __sigprocmask (SIG_SETMASK, (attr->__flags & POSIX_SPAWN_SETSIGMASK)
> -                ? &attr->__ss : &args->oldmask, 0);
> +  if (attr->__flags & POSIX_SPAWN_SETSIGMASK)
> +    __sigprocmask (SIG_SETMASK, &attr->__ss, NULL);
> +  else
> +    internal_sigprocmask (SIG_SETMASK, &args->oldmask, NULL);
>
>    args->exec (args->file, args->argv, args->envp);
>
> @@ -368,7 +370,7 @@ __spawnix (pid_t * pid, const char *file,
>    args.envp = envp;
>    args.xflags = xflags;
>
> -  __libc_signal_block_all (&args.oldmask);
> +  internal_signal_block_all (&args.oldmask);
>
>    /* The clone flags used will create a new child that will run in the same
>       memory space (CLONE_VM) and the execution of calling thread will be
> @@ -416,7 +418,7 @@ __spawnix (pid_t * pid, const char *file,
>    if ((ec == 0) && (pid != NULL))
>      *pid = new_pid;
>
> -  __libc_signal_restore_set (&args.oldmask);
> +  internal_signal_restore_set (&args.oldmask);
>
>    __pthread_setcancelstate (state, NULL);
>
> diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c
> index 8f0a2f635d..7433f7f559 100644
> --- a/sysdeps/unix/sysv/linux/timer_routines.c
> +++ b/sysdeps/unix/sysv/linux/timer_routines.c
> @@ -41,7 +41,7 @@ struct thread_start_data
>  static void *
>  timer_sigev_thread (void *arg)
>  {
> -  __libc_signal_unblock_sigtimer (NULL);
> +  signal_unblock_sigtimer ();
>
>    struct thread_start_data *td = (struct thread_start_data *) arg;
>    void (*thrfunc) (sigval_t) = td->thrfunc;
> --
> 2.34.1
>

Re: [PATCH v2] Refactor internal-signals.h

[Joseph Myers]

I'm seeing linknamespace failures for Hurd I suspect are caused by this 
change.  E.g.:

[initial] __assert_fail -> [libc.a(assert.o)] __abort_msg -> [libc.a(abort.o)] sigaddset
[initial] __assert_fail -> [libc.a(assert.o)] __abort_msg -> [libc.a(abort.o)] sigemptyset
[initial] __assert_fail -> [libc.a(assert.o)] __abort_msg -> [libc.a(abort.o)] sigprocmask

https://sourceware.org/pipermail/libc-testresults/2022q3/009899.html

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2] Refactor internal-signals.h

[Adhemerval Zanella]

> On 1 Jul 2022, at 11:55, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> I'm seeing linknamespace failures for Hurd I suspect are caused by this 
> change.  E.g.:
> 
> [initial] __assert_fail -> [libc.a(assert.o)] __abort_msg -> [libc.a(abort.o)] sigaddset
> [initial] __assert_fail -> [libc.a(assert.o)] __abort_msg -> [libc.a(abort.o)] sigemptyset
> [initial] __assert_fail -> [libc.a(assert.o)] __abort_msg -> [libc.a(abort.o)] sigprocmask
> 
> https://sourceware.org/pipermail/libc-testresults/2022q3/009899.html

I will take a look.

> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com