linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] signal handling support for nolibc
@ 2025-07-31 20:12 Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 1/4] selftests/nolibc: fix EXPECT_NZ macro Benjamin Berg
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Benjamin Berg @ 2025-07-31 20:12 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, linux-kselftest; +Cc: Benjamin Berg

From: Benjamin Berg <benjamin.berg@intel.com>

Hi,

This patchset adds signal handling to nolibc. Initially, I would like to
use this for tests. But in the long run, the goal is to use nolibc for
the UML kernel itself. In both cases, signal handling will be needed.

With v3 everything is now included in nolibc instead of trying to use
the messy kernel headers.

Benjamin

Benjamin Berg (4):
  selftests/nolibc: fix EXPECT_NZ macro
  selftests/nolibc: remove outdated comment about construct order
  tools/nolibc: add more generic bitmask macros for FD_*
  tools/nolibc: add signal support

 tools/include/nolibc/Makefile                |   1 +
 tools/include/nolibc/arch-s390.h             |   4 +-
 tools/include/nolibc/asm-signal.h            | 237 +++++++++++++++++++
 tools/include/nolibc/signal.h                | 179 ++++++++++++++
 tools/include/nolibc/sys.h                   |   2 +-
 tools/include/nolibc/sys/wait.h              |   1 +
 tools/include/nolibc/time.h                  |   2 +-
 tools/include/nolibc/types.h                 |  81 ++++---
 tools/testing/selftests/nolibc/nolibc-test.c | 139 ++++++++++-
 9 files changed, 608 insertions(+), 38 deletions(-)
 create mode 100644 tools/include/nolibc/asm-signal.h

-- 
2.50.1


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

* [PATCH v3 1/4] selftests/nolibc: fix EXPECT_NZ macro
  2025-07-31 20:12 [PATCH v3 0/4] signal handling support for nolibc Benjamin Berg
@ 2025-07-31 20:12 ` Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 2/4] selftests/nolibc: remove outdated comment about construct order Benjamin Berg
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Berg @ 2025-07-31 20:12 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, linux-kselftest; +Cc: Benjamin Berg

From: Benjamin Berg <benjamin.berg@intel.com>

The expect non-zero macro was incorrect and never used. Fix its
definition.

Fixes: 362aecb2d8cfa ("selftests/nolibc: add basic infrastructure to ease creation of nolibc tests")
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index a297ee0d6d07..97efc98b6a3d 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -196,8 +196,8 @@ int expect_zr(int expr, int llen)
 }
 
 
-#define EXPECT_NZ(cond, expr, val)			\
-	do { if (!(cond)) result(llen, SKIPPED); else ret += expect_nz(expr, llen; } while (0)
+#define EXPECT_NZ(cond, expr)				\
+	do { if (!(cond)) result(llen, SKIPPED); else ret += expect_nz(expr, llen); } while (0)
 
 static __attribute__((unused))
 int expect_nz(int expr, int llen)
-- 
2.50.1


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

* [PATCH v3 2/4] selftests/nolibc: remove outdated comment about construct order
  2025-07-31 20:12 [PATCH v3 0/4] signal handling support for nolibc Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 1/4] selftests/nolibc: fix EXPECT_NZ macro Benjamin Berg
@ 2025-07-31 20:12 ` Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 3/4] tools/nolibc: add more generic bitmask macros for FD_* Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 4/4] tools/nolibc: add signal support Benjamin Berg
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Berg @ 2025-07-31 20:12 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, linux-kselftest; +Cc: Benjamin Berg

From: Benjamin Berg <benjamin.berg@intel.com>

The constructor order is not (and should not) be tested. Remove the
comment.

Fixes: a782d45c867c ("selftests/nolibc: stop testing constructor order")
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 97efc98b6a3d..180f0436127a 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -686,7 +686,6 @@ int expect_strtox(int llen, void *func, const char *input, int base, intmax_t ex
 #define CASE_TEST(name) \
 	case __LINE__: llen += printf("%d %s", test, #name);
 
-/* constructors validate that they are executed in definition order */
 __attribute__((constructor))
 static void constructor1(void)
 {
-- 
2.50.1


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

* [PATCH v3 3/4] tools/nolibc: add more generic bitmask macros for FD_*
  2025-07-31 20:12 [PATCH v3 0/4] signal handling support for nolibc Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 1/4] selftests/nolibc: fix EXPECT_NZ macro Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 2/4] selftests/nolibc: remove outdated comment about construct order Benjamin Berg
@ 2025-07-31 20:12 ` Benjamin Berg
  2025-07-31 20:12 ` [PATCH v3 4/4] tools/nolibc: add signal support Benjamin Berg
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Berg @ 2025-07-31 20:12 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, linux-kselftest; +Cc: Benjamin Berg

From: Benjamin Berg <benjamin.berg@intel.com>

The FD_* macros are assuming a specific type for the bitmask. Add new
macros that introspect the type of the passed variable in order to know
the size of the bitmask. This way the same macros can be used for other
purposes.

Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>

---

v2:
- Rename the macros for consistency and to mark them internal
- Fix shift to use the correct element type
---
 tools/include/nolibc/types.h | 72 ++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 32 deletions(-)

diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 16c6e9ec9451..f7f2ddf41e89 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -115,48 +115,56 @@
 #define EXIT_SUCCESS 0
 #define EXIT_FAILURE 1
 
-#define FD_SETIDXMASK (8 * sizeof(unsigned long))
-#define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
-
-/* for select() */
-typedef struct {
-	unsigned long fds[(FD_SETSIZE + FD_SETBITMASK) / FD_SETIDXMASK];
-} fd_set;
-
-#define FD_CLR(fd, set) do {						\
-		fd_set *__set = (set);					\
-		int __fd = (fd);					\
-		if (__fd >= 0)						\
-			__set->fds[__fd / FD_SETIDXMASK] &=		\
-				~(1U << (__fd & FD_SETBITMASK));	\
+#define __NOLIBC_BITMASK_CLEAR(num, set) do {				     \
+		__typeof__(set) *__set = &(set);			     \
+		int __num = (num);					     \
+		__typeof__(**__set) __bit = 1;				     \
+		if (__num >= 0 && __num < 8 * (ssize_t)sizeof(*__set))	     \
+			(*__set)[__num / (8 * sizeof(**__set))] &=	     \
+				~(__bit << (__num % (8 * sizeof(**__set)))); \
 	} while (0)
 
-#define FD_SET(fd, set) do {						\
-		fd_set *__set = (set);					\
-		int __fd = (fd);					\
-		if (__fd >= 0)						\
-			__set->fds[__fd / FD_SETIDXMASK] |=		\
-				1 << (__fd & FD_SETBITMASK);		\
+#define __NOLIBC_BITMASK_SET(num, set) do {				  \
+		__typeof__(set) *__set = &(set);			  \
+		int __num = (num);					  \
+		__typeof__(**__set) __bit = 1;				  \
+		if (__num >= 0 && __num < 8 * (ssize_t)sizeof(*__set))	  \
+			(*__set)[__num / (8 * sizeof(**__set))] |=	  \
+				__bit << (__num % (8 * sizeof(**__set))); \
 	} while (0)
 
-#define FD_ISSET(fd, set) ({						\
-			fd_set *__set = (set);				\
-			int __fd = (fd);				\
-		int __r = 0;						\
-		if (__fd >= 0)						\
-			__r = !!(__set->fds[__fd / FD_SETIDXMASK] &	\
-1U << (__fd & FD_SETBITMASK));						\
-		__r;							\
+#define __NOLIBC_BITMASK_TEST(num, set) ({				  \
+		__typeof__(set) *__set = &(set);			  \
+		int __num = (num);					  \
+		__typeof__(**__set) __r = 0;				  \
+		__typeof__(**__set) __bit = 1;				  \
+		if (__num >= 0 && __num < 8 * (ssize_t)sizeof(*__set))	  \
+			__r = (*__set)[__num / (8 * sizeof(**__set))] &	  \
+			      (__bit << (__num % (8 * sizeof(**__set)))); \
+		!!__r;							  \
 	})
 
-#define FD_ZERO(set) do {						\
-		fd_set *__set = (set);					\
+#define __NOLIBC_BITMASK_ZERO(set) do {					\
+		__typeof__(set) *__set = &(set);			\
 		int __idx;						\
-		int __size = (FD_SETSIZE+FD_SETBITMASK) / FD_SETIDXMASK;\
+		int __size = sizeof(*__set) / sizeof(**__set);		\
 		for (__idx = 0; __idx < __size; __idx++)		\
-			__set->fds[__idx] = 0;				\
+			(*__set)[__idx] = 0;				\
 	} while (0)
 
+#define FD_SETIDXMASK (8 * sizeof(unsigned long))
+#define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
+
+/* for select() */
+typedef struct {
+	unsigned long fds[(FD_SETSIZE + FD_SETBITMASK) / FD_SETIDXMASK];
+} fd_set;
+
+#define FD_CLR(fd, set) __NOLIBC_BITMASK_CLEAR(fd, (set)->fds)
+#define FD_SET(fd, set) __NOLIBC_BITMASK_SET(fd, (set)->fds)
+#define FD_ISSET(fd, set) __NOLIBC_BITMASK_TEST(fd, (set)->fds)
+#define FD_ZERO(set) __NOLIBC_BITMASK_ZERO((set)->fds)
+
 /* for getdents64() */
 struct linux_dirent64 {
 	uint64_t       d_ino;
-- 
2.50.1


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

* [PATCH v3 4/4] tools/nolibc: add signal support
  2025-07-31 20:12 [PATCH v3 0/4] signal handling support for nolibc Benjamin Berg
                   ` (2 preceding siblings ...)
  2025-07-31 20:12 ` [PATCH v3 3/4] tools/nolibc: add more generic bitmask macros for FD_* Benjamin Berg
@ 2025-07-31 20:12 ` Benjamin Berg
  2025-08-01 15:09   ` Thomas Weißschuh
  3 siblings, 1 reply; 9+ messages in thread
From: Benjamin Berg @ 2025-07-31 20:12 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, linux-kselftest; +Cc: Benjamin Berg

From: Benjamin Berg <benjamin.berg@intel.com>

Add support for sigaction() using the rt_sigaction syscall and implement
the normal sa_mask helpers.

For the uapi definitions, everything is copied into nolibc. This avoids
issues with kernel architecture headers that are not usable with the
rt_sigaction syscall.

Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>

---

v3:
- put everything into signal.h and the new asm-signal.h
- split out sigset_t tests
- actually mark signal_check static
- remove unused string.h include
- fix SIGUSR2 reset
- Use integer for signal_check as the signals are emitted from the
  syscall context.

v2:
- Use newly added macros to check signal emission order
- Add tests for sigset handling
- Restore the default handler after signal test
- make signal_check variable static

v1:
- Update architecture support (adding sh)
- Move sparc sys_rt_sigaction logic into its header
- Add sig_atomic_t
- Use new BITSET_* macros
- Move test into syscall suite
- Various other small changes
---
 tools/include/nolibc/Makefile                |   1 +
 tools/include/nolibc/arch-s390.h             |   4 +-
 tools/include/nolibc/asm-signal.h            | 237 +++++++++++++++++++
 tools/include/nolibc/signal.h                | 179 ++++++++++++++
 tools/include/nolibc/sys.h                   |   2 +-
 tools/include/nolibc/sys/wait.h              |   1 +
 tools/include/nolibc/time.h                  |   2 +-
 tools/include/nolibc/types.h                 |   9 +
 tools/testing/selftests/nolibc/nolibc-test.c | 134 +++++++++++
 9 files changed, 566 insertions(+), 3 deletions(-)
 create mode 100644 tools/include/nolibc/asm-signal.h

diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index 143c2d2c2ba6..55db0677cfc3 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -25,6 +25,7 @@ endif
 
 arch_file := arch-$(ARCH).h
 all_files := \
+		asm-signal.h \
 		compiler.h \
 		crt.h \
 		ctype.h \
diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h
index df4c3cc713ac..f22f9a6d9887 100644
--- a/tools/include/nolibc/arch-s390.h
+++ b/tools/include/nolibc/arch-s390.h
@@ -5,13 +5,15 @@
 
 #ifndef _NOLIBC_ARCH_S390_H
 #define _NOLIBC_ARCH_S390_H
-#include <linux/signal.h>
 #include <linux/unistd.h>
 
 #include "compiler.h"
 #include "crt.h"
 #include "std.h"
 
+/* For SIGCHLD */
+#include "asm-signal.h"
+
 /* Syscalls for s390:
  *   - registers are 64-bit
  *   - syscall number is passed in r1
diff --git a/tools/include/nolibc/asm-signal.h b/tools/include/nolibc/asm-signal.h
new file mode 100644
index 000000000000..7831595745a8
--- /dev/null
+++ b/tools/include/nolibc/asm-signal.h
@@ -0,0 +1,237 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+/*
+ * ASM signal definitions for NOLIBC
+ */
+
+#ifndef _NOLIBC_ASM_SIGNAL_H
+#define _NOLIBC_ASM_SIGNAL_H
+
+/*
+ * This reproduces the kernel headers for the different architectures.
+ */
+
+#include <linux/types.h>
+
+#if defined(__mips__)
+#define _NSIG		128
+#else
+#define _NSIG		64
+#endif
+#define _NSIG_BPW	__BITS_PER_LONG
+#define _NSIG_WORDS	(_NSIG / _NSIG_BPW)
+
+typedef struct {
+	unsigned long sig[_NSIG_WORDS];
+} sigset_t;
+
+#if defined(__mips__)
+#define SIGHUP		 1	/* Hangup (POSIX).  */
+#define SIGINT		 2	/* Interrupt (ANSI).  */
+#define SIGQUIT		 3	/* Quit (POSIX).  */
+#define SIGILL		 4	/* Illegal instruction (ANSI).	*/
+#define SIGTRAP		 5	/* Trace trap (POSIX).	*/
+#define SIGIOT		 6	/* IOT trap (4.2 BSD).	*/
+#define SIGABRT		 SIGIOT /* Abort (ANSI).  */
+#define SIGEMT		 7
+#define SIGFPE		 8	/* Floating-point exception (ANSI).  */
+#define SIGKILL		 9	/* Kill, unblockable (POSIX).  */
+#define SIGBUS		10	/* BUS error (4.2 BSD).	 */
+#define SIGSEGV		11	/* Segmentation violation (ANSI).  */
+#define SIGSYS		12
+#define SIGPIPE		13	/* Broken pipe (POSIX).	 */
+#define SIGALRM		14	/* Alarm clock (POSIX).	 */
+#define SIGTERM		15	/* Termination (ANSI).	*/
+#define SIGUSR1		16	/* User-defined signal 1 (POSIX).  */
+#define SIGUSR2		17	/* User-defined signal 2 (POSIX).  */
+#define SIGCHLD		18	/* Child status has changed (POSIX).  */
+#define SIGCLD		SIGCHLD /* Same as SIGCHLD (System V).	*/
+#define SIGPWR		19	/* Power failure restart (System V).  */
+#define SIGWINCH	20	/* Window size change (4.3 BSD, Sun).  */
+#define SIGURG		21	/* Urgent condition on socket (4.2 BSD).  */
+#define SIGIO		22	/* I/O now possible (4.2 BSD).	*/
+#define SIGPOLL		SIGIO	/* Pollable event occurred (System V).	*/
+#define SIGSTOP		23	/* Stop, unblockable (POSIX).  */
+#define SIGTSTP		24	/* Keyboard stop (POSIX).  */
+#define SIGCONT		25	/* Continue (POSIX).  */
+#define SIGTTIN		26	/* Background read from tty (POSIX).  */
+#define SIGTTOU		27	/* Background write to tty (POSIX).  */
+#define SIGVTALRM	28	/* Virtual alarm clock (4.2 BSD).  */
+#define SIGPROF		29	/* Profiling alarm clock (4.2 BSD).  */
+#define SIGXCPU		30	/* CPU limit exceeded (4.2 BSD).  */
+#define SIGXFSZ		31	/* File size limit exceeded (4.2 BSD).	*/
+
+#elif defined(__sparc__)
+/* On the Sparc the signal handlers get passed a 'sub-signal' code
+ * for certain signal types, which we document here.
+ */
+#define SIGHUP		 1
+#define SIGINT		 2
+#define SIGQUIT		 3
+#define SIGILL		 4
+#define    SUBSIG_STACK       0
+#define    SUBSIG_ILLINST     2
+#define    SUBSIG_PRIVINST    3
+#define    SUBSIG_BADTRAP(t)  (0x80 + (t))
+
+#define SIGTRAP		 5
+#define SIGABRT		 6
+#define SIGIOT		 6
+
+#define SIGEMT           7
+#define    SUBSIG_TAG    10
+
+#define SIGFPE		 8
+#define    SUBSIG_FPDISABLED     0x400
+#define    SUBSIG_FPERROR        0x404
+#define    SUBSIG_FPINTOVFL      0x001
+#define    SUBSIG_FPSTSIG        0x002
+#define    SUBSIG_IDIVZERO       0x014
+#define    SUBSIG_FPINEXACT      0x0c4
+#define    SUBSIG_FPDIVZERO      0x0c8
+#define    SUBSIG_FPUNFLOW       0x0cc
+#define    SUBSIG_FPOPERROR      0x0d0
+#define    SUBSIG_FPOVFLOW       0x0d4
+
+#define SIGKILL		 9
+#define SIGBUS          10
+#define    SUBSIG_BUSTIMEOUT    1
+#define    SUBSIG_ALIGNMENT     2
+#define    SUBSIG_MISCERROR     5
+
+#define SIGSEGV		11
+#define    SUBSIG_NOMAPPING     3
+#define    SUBSIG_PROTECTION    4
+#define    SUBSIG_SEGERROR      5
+
+#define SIGSYS		12
+
+#define SIGPIPE		13
+#define SIGALRM		14
+#define SIGTERM		15
+#define SIGURG          16
+
+/* SunOS values which deviate from the Linux/i386 ones */
+#define SIGSTOP		17
+#define SIGTSTP		18
+#define SIGCONT		19
+#define SIGCHLD		20
+#define SIGTTIN		21
+#define SIGTTOU		22
+#define SIGIO		23
+#define SIGPOLL		SIGIO   /* SysV name for SIGIO */
+#define SIGXCPU		24
+#define SIGXFSZ		25
+#define SIGVTALRM	26
+#define SIGPROF		27
+#define SIGWINCH	28
+#define SIGLOST		29
+#define SIGPWR		SIGLOST
+#define SIGUSR1		30
+#define SIGUSR2		31
+
+#else /* asm-generic signal definitions */
+#define SIGHUP		 1
+#define SIGINT		 2
+#define SIGQUIT		 3
+#define SIGILL		 4
+#define SIGTRAP		 5
+#define SIGABRT		 6
+#define SIGIOT		 6
+#define SIGBUS		 7
+#define SIGFPE		 8
+#define SIGKILL		 9
+#define SIGUSR1		10
+#define SIGSEGV		11
+#define SIGUSR2		12
+#define SIGPIPE		13
+#define SIGALRM		14
+#define SIGTERM		15
+#define SIGSTKFLT	16
+#define SIGCHLD		17
+#define SIGCONT		18
+#define SIGSTOP		19
+#define SIGTSTP		20
+#define SIGTTIN		21
+#define SIGTTOU		22
+#define SIGURG		23
+#define SIGXCPU		24
+#define SIGXFSZ		25
+#define SIGVTALRM	26
+#define SIGPROF		27
+#define SIGWINCH	28
+#define SIGIO		29
+#define SIGPOLL		SIGIO
+/*
+#define SIGLOST		29
+*/
+#define SIGPWR		30
+#define SIGSYS		31
+#define	SIGUNUSED	31
+#endif
+
+/* These should not be considered constants from userland.  */
+#define SIGRTMIN	32
+#ifndef SIGRTMAX
+#define SIGRTMAX	_NSIG
+#endif
+
+#if !defined MINSIGSTKSZ || !defined SIGSTKSZ
+#if defined(__aarch64__)
+#define MINSIGSTKSZ	 5120
+#define SIGSTKSZ	16384
+#elif defined(__loongarch__) || defined(__sparc__)
+#define MINSIGSTKSZ	 4096
+#define SIGSTKSZ	16384
+#elif defined(__powerpc64__)
+#define MINSIGSTKSZ	 8192
+#define SIGSTKSZ	32768
+#else
+#define MINSIGSTKSZ	 2048
+#define SIGSTKSZ	 8192
+#endif
+#endif
+
+#if defined(__mips__)
+#define SA_ONSTACK      0x08000000
+#define SA_RESETHAND    0x80000000
+#define SA_RESTART      0x10000000
+#define SA_SIGINFO      0x00000008
+#define SA_NODEFER      0x40000000
+#define SA_NOCLDWAIT    0x00010000
+#define SA_NOCLDSTOP    0x00000001
+
+#elif defined(__sparc__)
+#define _SV_SSTACK    1u
+#define _SV_INTR      2u
+#define _SV_RESET     4u
+#define _SV_IGNCHILD  8u
+
+#define SA_NOCLDSTOP    _SV_IGNCHILD
+#define SA_STACK        _SV_SSTACK
+#define SA_ONSTACK      _SV_SSTACK
+#define SA_RESTART      _SV_INTR
+#define SA_RESETHAND    _SV_RESET
+#define SA_NODEFER      0x20u
+#define SA_NOCLDWAIT    0x100u
+#define SA_SIGINFO      0x200u
+
+#else
+#define SA_NOCLDSTOP    0x00000001
+#define SA_NOCLDWAIT    0x00000002
+#define SA_SIGINFO      0x00000004
+#define SA_UNSUPPORTED  0x00000400
+#define SA_EXPOSE_TAGBITS       0x00000800
+#define SA_ONSTACK      0x08000000
+#define SA_RESTART      0x10000000
+#define SA_NODEFER      0x40000000
+#define SA_RESETHAND    0x80000000
+#endif
+
+#define SA_NOMASK       SA_NODEFER
+#define SA_ONESHOT      SA_RESETHAND
+
+#if defined(__ARM_EABI__) || defined(__aarch64__) || defined(__m68k__) || defined(__powerpc__) || defined(__s390x__) || defined(__s390__) || defined(__sh__) || defined(__i386__) || defined(__x86_64__)
+#define SA_RESTORER	0x04000000
+#endif
+
+#endif /* _NOLIBC_ASM_SIGNAL_H */
\ No newline at end of file
diff --git a/tools/include/nolibc/signal.h b/tools/include/nolibc/signal.h
index ac13e53ac31d..8a71bba8c1b6 100644
--- a/tools/include/nolibc/signal.h
+++ b/tools/include/nolibc/signal.h
@@ -14,6 +14,46 @@
 #include "arch.h"
 #include "types.h"
 #include "sys.h"
+#include <asm/siginfo.h>
+#include "asm-signal.h"
+
+typedef void __signalfn_t(int);
+typedef __signalfn_t *__sighandler_t;
+
+typedef void __restorefn_t(void);
+typedef __restorefn_t *__sigrestore_t;
+
+#define SIG_DFL ((__sighandler_t)0)     /* default signal handling */
+#define SIG_IGN ((__sighandler_t)1)     /* ignore signal */
+#define SIG_ERR ((__sighandler_t)-1)    /* error return from signal */
+
+#if defined(__mips__)
+struct sigaction {
+        unsigned int    sa_flags;
+        __sighandler_t  sa_handler;
+        sigset_t        sa_mask;
+};
+#else
+struct sigaction {
+	__sighandler_t sa_handler;
+	unsigned long sa_flags;
+#if defined(SA_RESTORER) || defined(__sparc__)
+	__sigrestore_t sa_restorer;
+#endif
+	sigset_t sa_mask;		/* mask last for extensibility */
+};
+#endif
+
+typedef struct sigaltstack {
+	void *ss_sp;
+	int ss_flags;
+	__kernel_size_t ss_size;
+} stack_t;
+
+#ifndef __sig_atomic_t_defined
+#define __sig_atomic_t_defined 1
+typedef int sig_atomic_t;
+#endif
 
 /* This one is not marked static as it's needed by libgcc for divide by zero */
 int raise(int signal);
@@ -23,4 +63,143 @@ int raise(int signal)
 	return sys_kill(sys_getpid(), signal);
 }
 
+/*
+ * sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
+ */
+#if defined(__sparc__)
+/*
+ * Sparc needs a restorer, which needs to be implemented in assembler and
+ * passed as a separate argument.
+ */
+
+void __nolibc_sa_restorer(void);
+void __nolibc_sa_restorer_wrapper(void);
+void __attribute__((weak,noreturn)) __nolibc_entrypoint __no_stack_protector
+__nolibc_sa_restorer_wrapper(void)
+{
+	/* The C function will have a prologue corrupting "sp" */
+	__asm__  volatile (
+		".section .text\n"
+		".align 4\n"
+		".type __nolibc_sa_restorer, @function\n"
+		"__nolibc_sa_restorer:\n"
+		"nop\n"
+		"nop\n"
+		"mov %0, %%g1 \n"
+#ifdef __arch64__
+		"t 0x6d\n"
+#else
+		"t 0x10\n"
+#endif
+		".size __nolibc_sa_restorer, .-__nolibc_sa_restorer\n"
+		:: "n"(__NR_rt_sigreturn)
+	);
+	__nolibc_entrypoint_epilogue();
+}
+
+static __attribute__((unused))
+int sys_rt_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
+{
+	struct sigaction real_act = *act;
+
+	/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
+	real_act.sa_flags |= SA_SIGINFO;
+
+	return my_syscall5(__NR_rt_sigaction, signum, &real_act, oldact,
+			   __nolibc_sa_restorer, sizeof(act->sa_mask));
+}
+
+#else
+#if defined(__x86_64__) || defined(__i386_) || defined(__powerpc__)
+static __no_stack_protector
+void __nolibc_sa_restorer(void)
+{
+	my_syscall0(__NR_rt_sigreturn);
+}
+#endif
+
+static __attribute__((unused))
+int sys_rt_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
+{
+	struct sigaction real_act = *act;
+#if defined(__x86_64__) || defined(__i386_) || defined(__powerpc__)
+	if (!(real_act.sa_flags & SA_RESTORER)) {
+		real_act.sa_flags |= SA_RESTORER;
+		real_act.sa_restorer = __nolibc_sa_restorer;
+	}
+#endif
+#if defined(__i386__)
+	/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
+	real_act.sa_flags |= SA_SIGINFO;
+#endif
+
+	return my_syscall4(__NR_rt_sigaction, signum, &real_act, oldact,
+			   sizeof(act->sa_mask));
+}
+#endif
+
+static __attribute__((unused))
+int sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
+{
+	return __sysret(sys_rt_sigaction(signum, act, oldact));
+}
+
+/*
+ * int sigemptyset(sigset_t *set)
+ */
+static __attribute__((unused))
+int sigemptyset(sigset_t *set)
+{
+	__NOLIBC_BITMASK_ZERO(set->sig);
+	return 0;
+}
+
+/*
+ * int sigfillset(sigset_t *set)
+ */
+static __attribute__((unused))
+int sigfillset(sigset_t *set)
+{
+	__NOLIBC_BITMASK_FILL(set->sig);
+	return 0;
+}
+
+/*
+ * int sigaddset(sigset_t *set, int signum)
+ */
+static __attribute__((unused))
+int sigaddset(sigset_t *set, int signum)
+{
+	if (signum < 1 || signum > _NSIG)
+		return __sysret(-EINVAL);
+
+	__NOLIBC_BITMASK_SET(signum - 1, set->sig);
+	return 0;
+}
+
+/*
+ * int sigdelset(sigset_t *set, int signum)
+ */
+static __attribute__((unused))
+int sigdelset(sigset_t *set, int signum)
+{
+	if (signum < 1 || signum > _NSIG)
+		return __sysret(-EINVAL);
+
+	__NOLIBC_BITMASK_CLEAR(signum - 1, set->sig);
+	return 0;
+}
+
+/*
+ * int sigismember(sigset_t *set, int signum)
+ */
+static __attribute__((unused))
+int sigismember(sigset_t *set, int signum)
+{
+	if (signum < 1 || signum > _NSIG)
+		return __sysret(-EINVAL);
+
+	return __NOLIBC_BITMASK_TEST(signum - 1, set->sig);
+}
+
 #endif /* _NOLIBC_SIGNAL_H */
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 295e71d34aba..a790e816565b 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -14,7 +14,6 @@
 
 /* system includes */
 #include <linux/unistd.h>
-#include <linux/signal.h>  /* for SIGCHLD */
 #include <linux/termios.h>
 #include <linux/mman.h>
 #include <linux/fs.h>
@@ -28,6 +27,7 @@
 #include "errno.h"
 #include "stdarg.h"
 #include "types.h"
+#include "asm-signal.h" /* for SIGCHLD */
 
 
 /* Syscall return helper: takes the syscall value in argument and checks for an
diff --git a/tools/include/nolibc/sys/wait.h b/tools/include/nolibc/sys/wait.h
index 56ddb806da7f..e2aa90cc3cf3 100644
--- a/tools/include/nolibc/sys/wait.h
+++ b/tools/include/nolibc/sys/wait.h
@@ -10,6 +10,7 @@
 #ifndef _NOLIBC_SYS_WAIT_H
 #define _NOLIBC_SYS_WAIT_H
 
+#include <asm/siginfo.h>
 #include "../arch.h"
 #include "../std.h"
 #include "../types.h"
diff --git a/tools/include/nolibc/time.h b/tools/include/nolibc/time.h
index d02bc44d2643..6734a3a702a6 100644
--- a/tools/include/nolibc/time.h
+++ b/tools/include/nolibc/time.h
@@ -15,7 +15,7 @@
 #include "types.h"
 #include "sys.h"
 
-#include <linux/signal.h>
+#include <asm/siginfo.h>
 #include <linux/time.h>
 
 static __inline__
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index f7f2ddf41e89..7e205386b72c 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -152,6 +152,15 @@
 			(*__set)[__idx] = 0;				\
 	} while (0)
 
+#define __NOLIBC_BITMASK_FILL(set) do {					\
+		__typeof__(set) *__set = &(set);			\
+		int __idx;						\
+		int __size = sizeof(*__set) / sizeof(**__set);		\
+		__typeof__(**__set) __zero = 0;				\
+		for (__idx = 0; __idx < __size; __idx++)		\
+			(*__set)[__idx] = ~__zero;			\
+	} while (0)
+
 #define FD_SETIDXMASK (8 * sizeof(unsigned long))
 #define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
 
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 180f0436127a..75b96eaa4c65 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1269,6 +1269,138 @@ int test_namespace(void)
 	return ret;
 }
 
+int test_sigset_t(int test_idx)
+{
+	int llen;
+	int ret = 0;
+
+#ifdef NOLIBC
+	if (is_nolibc) {
+		sigset_t sigset;
+
+		sigfillset(&sigset);
+		llen = printf("    sigset.sig[0] (full): ");
+		EXPECT_EQ(1, sigset.sig[0],
+			~(__typeof__(sigset.sig[0]))0);
+		llen = printf("    sigset.sig[%d] (full): ", (int)_NSIG_WORDS - 1);
+		EXPECT_EQ(1, sigset.sig[_NSIG_WORDS - 1],
+			~(__typeof__(sigset.sig[0]))0);
+
+		sigemptyset(&sigset);
+		llen = printf("    sigset.sig[0] (empty): ");
+		EXPECT_EQ(1, sigset.sig[0], 0);
+		llen = printf("    sigset.sig[%d] (empty): ", (int)_NSIG_WORDS - 1);
+		EXPECT_EQ(1, sigset.sig[_NSIG_WORDS - 1], 0);
+
+		/* SIGUSR2 is always in the first word */
+		sigaddset(&sigset, SIGUSR2);
+		llen = printf("    sigset.sig[0] (SIGUSR2 set): ");
+		EXPECT_EQ(1, sigset.sig[0], 1 << (SIGUSR2 - 1));
+
+		llen = printf("    sigset.sig[0] (test SIGUSR2): ");
+		EXPECT_NZ(1, sigismember(&sigset, SIGUSR2));
+
+		sigdelset(&sigset, SIGUSR2);
+		llen = printf("    sigset.sig[0] (SIGUSR2 unset): ");
+		EXPECT_ZR(1, sigismember(&sigset, SIGUSR2));
+
+		/* _NSIG is the highest valid number and may not be in the first word */
+		sigaddset(&sigset, _NSIG);
+		llen = printf("    sigset.sig[%d] (_NSIG set): ", (int)_NSIG_WORDS - 1);
+		EXPECT_EQ(1, sigset.sig[_NSIG_WORDS - 1],
+			1UL << (_NSIG - (_NSIG_WORDS - 1) * _NSIG_BPW - 1));
+
+		llen = printf("    sigset.sig[%d] (test _NSIG): ", (int)_NSIG_WORDS - 1);
+		EXPECT_NZ(1, sigismember(&sigset, _NSIG));
+
+		sigdelset(&sigset, _NSIG);
+		llen = printf("    sigset.sig[%d] (_NSIG unset): ", (int)_NSIG_WORDS - 1);
+		EXPECT_ZR(1, sigismember(&sigset, _NSIG));
+
+		llen = printf("%d %s", test_idx, "sigset_t");
+		EXPECT_EQ(1, ret, 0);
+	} else
+#endif
+	{
+		llen = printf("%d %s", test_idx, "sigset_t");
+		result(llen, SKIPPED);
+	}
+
+	return ret;
+}
+
+static int signal_check;
+
+static void sighandler(int signum)
+{
+	if (signum == SIGUSR1) {
+		kill(getpid(), SIGUSR2);
+		/* The second step has not run because SIGUSR2 is masked */
+		signal_check = 0x1;
+	} else {
+		signal_check |= 0x2;
+	}
+}
+
+int test_signals(int test_idx)
+{
+	struct sigaction sa = {
+		.sa_flags = 0,
+		.sa_handler = sighandler,
+	};
+	struct sigaction sa_old = {
+		/* Anything other than SIG_DFL */
+		.sa_handler = sighandler,
+	};
+	int llen; /* line length */
+	int ret = 0;
+	int res;
+
+	signal_check = 0;
+
+	/* sa_mask is empty at this point, set SIGUSR2 to verify masking */
+	sigaddset(&sa.sa_mask, SIGUSR2);
+
+	res = sigaction(SIGUSR1, &sa, &sa_old);
+	llen = printf("    register SIGUSR1:");
+	EXPECT_SYSZR(1, res);
+	if (res)
+		goto out;
+
+	llen = printf("    sa_old.sa_handler: SIG_DFL (%p)", SIG_DFL);
+	EXPECT_PTREQ(1, SIG_DFL, sa_old.sa_handler);
+	if (res)
+		goto out;
+
+	res = sigaction(SIGUSR2, &sa, NULL);
+	llen = printf("    register SIGUSR2");
+	EXPECT_SYSZR(1, res);
+	if (res)
+		goto out;
+
+	/* Trigger the first signal. */
+	kill(getpid(), SIGUSR1);
+
+	/* Check the two signal handlers ran in the expected order */
+	llen = printf("    signal emission: ");
+	EXPECT_EQ(1, signal_check, 0x3);
+
+out:
+	sa.sa_handler = SIG_DFL;
+	res = sigaction(SIGUSR1, &sa, NULL);
+	llen = printf("    restore SIGUSR1");
+	EXPECT_SYSZR(1, res);
+
+	res = sigaction(SIGUSR2, &sa, NULL);
+	llen = printf("    restore SIGUSR2");
+	EXPECT_SYSZR(1, res);
+
+	llen = printf("%d %s", test_idx, "sigaction");
+	EXPECT_EQ(1, ret, 0);
+
+	return ret;
+}
+
 /* Run syscall tests between IDs <min> and <max>.
  * Return 0 on success, non-zero on failure.
  */
@@ -1397,6 +1529,8 @@ int run_syscall(int min, int max)
 		CASE_TEST(syscall_noargs);    EXPECT_SYSEQ(1, syscall(__NR_getpid), getpid()); break;
 		CASE_TEST(syscall_args);      EXPECT_SYSER(1, syscall(__NR_statx, 0, NULL, 0, 0, NULL), -1, EFAULT); break;
 		CASE_TEST(namespace);         EXPECT_SYSZR(euid0 && proc, test_namespace()); break;
+		case __LINE__:                ret += test_sigset_t(test); break;
+		case __LINE__:                ret += test_signals(test); break;
 		case __LINE__:
 			return ret; /* must be last */
 		/* note: do not set any defaults so as to permit holes above */
-- 
2.50.1


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

* Re: [PATCH v3 4/4] tools/nolibc: add signal support
  2025-07-31 20:12 ` [PATCH v3 4/4] tools/nolibc: add signal support Benjamin Berg
@ 2025-08-01 15:09   ` Thomas Weißschuh
  2025-08-02 10:26     ` Berg, Benjamin
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Weißschuh @ 2025-08-01 15:09 UTC (permalink / raw)
  To: Benjamin Berg; +Cc: Willy Tarreau, linux-kselftest, Benjamin Berg

Hi Benjamin,

On 2025-07-31 22:12:25+0200, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.berg@intel.com>
> 
> Add support for sigaction() using the rt_sigaction syscall and implement
> the normal sa_mask helpers.
> 
> For the uapi definitions, everything is copied into nolibc. This avoids
> issues with kernel architecture headers that are not usable with the
> rt_sigaction syscall.
> 
> Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> 
> ---
> 
> v3:
> - put everything into signal.h and the new asm-signal.h

Hm, did we decide on that? We don't want the per-architecture include
dance, but static overrides should still be fine I think.
Keeping the architecture ifdeffery inside the respective arch header.
And all the generic stuff in a shared header.

> - split out sigset_t tests
> - actually mark signal_check static
> - remove unused string.h include
> - fix SIGUSR2 reset
> - Use integer for signal_check as the signals are emitted from the
>   syscall context.

I don't understand this point, isn't it a signal handler?

> v2:
> - Use newly added macros to check signal emission order
> - Add tests for sigset handling
> - Restore the default handler after signal test
> - make signal_check variable static
> 
> v1:
> - Update architecture support (adding sh)
> - Move sparc sys_rt_sigaction logic into its header
> - Add sig_atomic_t
> - Use new BITSET_* macros
> - Move test into syscall suite
> - Various other small changes
> ---
>  tools/include/nolibc/Makefile                |   1 +
>  tools/include/nolibc/arch-s390.h             |   4 +-
>  tools/include/nolibc/asm-signal.h            | 237 +++++++++++++++++++
>  tools/include/nolibc/signal.h                | 179 ++++++++++++++
>  tools/include/nolibc/sys.h                   |   2 +-
>  tools/include/nolibc/sys/wait.h              |   1 +
>  tools/include/nolibc/time.h                  |   2 +-
>  tools/include/nolibc/types.h                 |   9 +
>  tools/testing/selftests/nolibc/nolibc-test.c | 134 +++++++++++
>  9 files changed, 566 insertions(+), 3 deletions(-)
>  create mode 100644 tools/include/nolibc/asm-signal.h

(...)

> diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> index 295e71d34aba..a790e816565b 100644
> --- a/tools/include/nolibc/sys.h
> +++ b/tools/include/nolibc/sys.h
> @@ -14,7 +14,6 @@
>  
>  /* system includes */
>  #include <linux/unistd.h>
> -#include <linux/signal.h>  /* for SIGCHLD */
>  #include <linux/termios.h>
>  #include <linux/mman.h>
>  #include <linux/fs.h>
> @@ -28,6 +27,7 @@
>  #include "errno.h"
>  #include "stdarg.h"
>  #include "types.h"
> +#include "asm-signal.h" /* for SIGCHLD */

#include "signal.h"

>  /* Syscall return helper: takes the syscall value in argument and checks for an
> diff --git a/tools/include/nolibc/sys/wait.h b/tools/include/nolibc/sys/wait.h
> index 56ddb806da7f..e2aa90cc3cf3 100644
> --- a/tools/include/nolibc/sys/wait.h
> +++ b/tools/include/nolibc/sys/wait.h
> @@ -10,6 +10,7 @@
>  #ifndef _NOLIBC_SYS_WAIT_H
>  #define _NOLIBC_SYS_WAIT_H
>  
> +#include <asm/siginfo.h>

#include "signal.h"

The asm/ usage should be hidden.

>  #include "../arch.h"
>  #include "../std.h"
>  #include "../types.h"

(...)

> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 180f0436127a..75b96eaa4c65 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -1269,6 +1269,138 @@ int test_namespace(void)
>  	return ret;
>  }
>  
> +int test_sigset_t(int test_idx)
> +{
> +	int llen;
> +	int ret = 0;
> +
> +#ifdef NOLIBC
> +	if (is_nolibc) {

This looks unnecessary. The #ifdef should be sufficient.

> +		sigset_t sigset;
> +

(...)


Looks nice, thanks!

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

* Re: [PATCH v3 4/4] tools/nolibc: add signal support
  2025-08-01 15:09   ` Thomas Weißschuh
@ 2025-08-02 10:26     ` Berg, Benjamin
  2025-08-02 11:26       ` Thomas Weißschuh
  0 siblings, 1 reply; 9+ messages in thread
From: Berg, Benjamin @ 2025-08-02 10:26 UTC (permalink / raw)
  To: linux@weissschuh.net; +Cc: w@1wt.eu, linux-kselftest@vger.kernel.org

Hi Thomas,

On Fri, 2025-08-01 at 17:09 +0200, Thomas Weißschuh wrote:
> On 2025-07-31 22:12:25+0200, Benjamin Berg wrote:
> > From: Benjamin Berg <benjamin.berg@intel.com>
> > 
> > Add support for sigaction() using the rt_sigaction syscall and implement
> > the normal sa_mask helpers.
> > 
> > For the uapi definitions, everything is copied into nolibc. This avoids
> > issues with kernel architecture headers that are not usable with the
> > rt_sigaction syscall.
> > 
> > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> > 
> > ---
> > 
> > v3:
> > - put everything into signal.h and the new asm-signal.h
> 
> Hm, did we decide on that? We don't want the per-architecture include
> dance, but static overrides should still be fine I think.
> Keeping the architecture ifdeffery inside the respective arch header.
> And all the generic stuff in a shared header.

I probably just didn't really understand what you meant :-)

You are right, we can have the common definitions in signal.h and just
skip them if the architecture header did already define them.

I think I'll also drop asm-signal.h again, see below.

> > - split out sigset_t tests
> > - actually mark signal_check static
> > - remove unused string.h include
> > - fix SIGUSR2 reset
> > - Use integer for signal_check as the signals are emitted from the
> >   syscall context.
> 
> I don't understand this point, isn't it a signal handler?

My reasoning is, that the signal emission by the kernel happens from
the kill syscall or function return. Both cases implicitly act as a
memory barrier. So in this specific case we do not actually need an
atomic variable.

> > v2:
> > - Use newly added macros to check signal emission order
> > - Add tests for sigset handling
> > - Restore the default handler after signal test
> > - make signal_check variable static
> > 
> > v1:
> > - Update architecture support (adding sh)
> > - Move sparc sys_rt_sigaction logic into its header
> > - Add sig_atomic_t
> > - Use new BITSET_* macros
> > - Move test into syscall suite
> > - Various other small changes
> > ---
> >  tools/include/nolibc/Makefile                |   1 +
> >  tools/include/nolibc/arch-s390.h             |   4 +-
> >  tools/include/nolibc/asm-signal.h            | 237 +++++++++++++++++++
> >  tools/include/nolibc/signal.h                | 179 ++++++++++++++
> >  tools/include/nolibc/sys.h                   |   2 +-
> >  tools/include/nolibc/sys/wait.h              |   1 +
> >  tools/include/nolibc/time.h                  |   2 +-
> >  tools/include/nolibc/types.h                 |   9 +
> >  tools/testing/selftests/nolibc/nolibc-test.c | 134 +++++++++++
> >  9 files changed, 566 insertions(+), 3 deletions(-)
> >  create mode 100644 tools/include/nolibc/asm-signal.h
> 
> (...)
> 
> > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> > index 295e71d34aba..a790e816565b 100644
> > --- a/tools/include/nolibc/sys.h
> > +++ b/tools/include/nolibc/sys.h
> > @@ -14,7 +14,6 @@
> >  
> >  /* system includes */
> >  #include <linux/unistd.h>
> > -#include <linux/signal.h>  /* for SIGCHLD */
> >  #include <linux/termios.h>
> >  #include <linux/mman.h>
> >  #include <linux/fs.h>
> > @@ -28,6 +27,7 @@
> >  #include "errno.h"
> >  #include "stdarg.h"
> >  #include "types.h"
> > +#include "asm-signal.h" /* for SIGCHLD */
> 
> #include "signal.h"

Right, this and asm-signal.h happened because signal.h uses sys_kill()
for raise(), resulting in a circular dependency.

The simplest solution is probably to avoid the circular include by
implementing raise() as:

int raise(int signal);
__attribute__((weak,unused,section(".text.nolibc_raise")))
int raise(int signal)
{
	return my_syscall2(__NR_kill, my_syscall0(__NR_getpid), signal);
}

> >  /* Syscall return helper: takes the syscall value in argument and checks for an
> > diff --git a/tools/include/nolibc/sys/wait.h b/tools/include/nolibc/sys/wait.h
> > index 56ddb806da7f..e2aa90cc3cf3 100644
> > --- a/tools/include/nolibc/sys/wait.h
> > +++ b/tools/include/nolibc/sys/wait.h
> > @@ -10,6 +10,7 @@
> >  #ifndef _NOLIBC_SYS_WAIT_H
> >  #define _NOLIBC_SYS_WAIT_H
> >  
> > +#include <asm/siginfo.h>
> 
> #include "signal.h"
> 
> The asm/ usage should be hidden.
> 
> >  #include "../arch.h"
> >  #include "../std.h"
> >  #include "../types.h"
> 
> (...)
> 
> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > index 180f0436127a..75b96eaa4c65 100644
> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > @@ -1269,6 +1269,138 @@ int test_namespace(void)
> >  	return ret;
> >  }
> >  
> > +int test_sigset_t(int test_idx)
> > +{
> > +	int llen;
> > +	int ret = 0;
> > +
> > +#ifdef NOLIBC
> > +	if (is_nolibc) {
> 
> This looks unnecessary. The #ifdef should be sufficient.
> 
> > +		sigset_t sigset;
> > +
> 
> (...)
> 
> 
> Looks nice, thanks!

Great, thanks for the reviews!

Benjamin
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

* Re: [PATCH v3 4/4] tools/nolibc: add signal support
  2025-08-02 10:26     ` Berg, Benjamin
@ 2025-08-02 11:26       ` Thomas Weißschuh
  2025-08-02 12:43         ` Berg, Benjamin
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Weißschuh @ 2025-08-02 11:26 UTC (permalink / raw)
  To: Berg, Benjamin; +Cc: w@1wt.eu, linux-kselftest@vger.kernel.org

Note: Please also Cc the patches to LKML in the future.

On 2025-08-02 10:26:34+0000, Berg, Benjamin wrote:
> On Fri, 2025-08-01 at 17:09 +0200, Thomas Weißschuh wrote:
> > On 2025-07-31 22:12:25+0200, Benjamin Berg wrote:
> > > From: Benjamin Berg <benjamin.berg@intel.com>
> > > 
> > > Add support for sigaction() using the rt_sigaction syscall and implement
> > > the normal sa_mask helpers.
> > > 
> > > For the uapi definitions, everything is copied into nolibc. This avoids
> > > issues with kernel architecture headers that are not usable with the
> > > rt_sigaction syscall.
> > > 
> > > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> > > 
> > > ---
> > > 
> > > v3:
> > > - put everything into signal.h and the new asm-signal.h
> > 
> > Hm, did we decide on that? We don't want the per-architecture include
> > dance, but static overrides should still be fine I think.
> > Keeping the architecture ifdeffery inside the respective arch header.
> > And all the generic stuff in a shared header.
> 
> I probably just didn't really understand what you meant :-)
> 
> You are right, we can have the common definitions in signal.h and just
> skip them if the architecture header did already define them.

Sounds good to me.

> I think I'll also drop asm-signal.h again, see below.
> 
> > > - split out sigset_t tests
> > > - actually mark signal_check static
> > > - remove unused string.h include
> > > - fix SIGUSR2 reset
> > > - Use integer for signal_check as the signals are emitted from the
> > >   syscall context.
> > 
> > I don't understand this point, isn't it a signal handler?
> 
> My reasoning is, that the signal emission by the kernel happens from
> the kill syscall or function return. Both cases implicitly act as a
> memory barrier. So in this specific case we do not actually need an
> atomic variable.

Ok. Given that the code is doing a read-modify-write, the guarantees of
sig_atomic_t are probably not enough anyways.
What about volatile?

> > > v2:
> > > - Use newly added macros to check signal emission order
> > > - Add tests for sigset handling
> > > - Restore the default handler after signal test
> > > - make signal_check variable static
> > > 
> > > v1:
> > > - Update architecture support (adding sh)
> > > - Move sparc sys_rt_sigaction logic into its header
> > > - Add sig_atomic_t
> > > - Use new BITSET_* macros
> > > - Move test into syscall suite
> > > - Various other small changes
> > > ---
> > >  tools/include/nolibc/Makefile                |   1 +
> > >  tools/include/nolibc/arch-s390.h             |   4 +-
> > >  tools/include/nolibc/asm-signal.h            | 237 +++++++++++++++++++
> > >  tools/include/nolibc/signal.h                | 179 ++++++++++++++
> > >  tools/include/nolibc/sys.h                   |   2 +-
> > >  tools/include/nolibc/sys/wait.h              |   1 +
> > >  tools/include/nolibc/time.h                  |   2 +-
> > >  tools/include/nolibc/types.h                 |   9 +
> > >  tools/testing/selftests/nolibc/nolibc-test.c | 134 +++++++++++
> > >  9 files changed, 566 insertions(+), 3 deletions(-)
> > >  create mode 100644 tools/include/nolibc/asm-signal.h
> > 
> > (...)
> > 
> > > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> > > index 295e71d34aba..a790e816565b 100644
> > > --- a/tools/include/nolibc/sys.h
> > > +++ b/tools/include/nolibc/sys.h
> > > @@ -14,7 +14,6 @@
> > >  
> > >  /* system includes */
> > >  #include <linux/unistd.h>
> > > -#include <linux/signal.h>  /* for SIGCHLD */
> > >  #include <linux/termios.h>
> > >  #include <linux/mman.h>
> > >  #include <linux/fs.h>
> > > @@ -28,6 +27,7 @@
> > >  #include "errno.h"
> > >  #include "stdarg.h"
> > >  #include "types.h"
> > > +#include "asm-signal.h" /* for SIGCHLD */
> > 
> > #include "signal.h"
> 
> Right, this and asm-signal.h happened because signal.h uses sys_kill()
> for raise(), resulting in a circular dependency.
> 
> The simplest solution is probably to avoid the circular include by
> implementing raise() as:
> 
> int raise(int signal);
> __attribute__((weak,unused,section(".text.nolibc_raise")))
> int raise(int signal)
> {
> 	return my_syscall2(__NR_kill, my_syscall0(__NR_getpid), signal);
> }

Also sounds good. Or add local declarations sys_kill() and sys_getpid().

(...)

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

* Re: [PATCH v3 4/4] tools/nolibc: add signal support
  2025-08-02 11:26       ` Thomas Weißschuh
@ 2025-08-02 12:43         ` Berg, Benjamin
  0 siblings, 0 replies; 9+ messages in thread
From: Berg, Benjamin @ 2025-08-02 12:43 UTC (permalink / raw)
  To: linux@weissschuh.net; +Cc: w@1wt.eu, linux-kselftest@vger.kernel.org

Hi,

On Sat, 2025-08-02 at 13:26 +0200, Thomas Weißschuh wrote:
> Note: Please also Cc the patches to LKML in the future.
> 
> On 2025-08-02 10:26:34+0000, Berg, Benjamin wrote:
> > On Fri, 2025-08-01 at 17:09 +0200, Thomas Weißschuh wrote:
> > > On 2025-07-31 22:12:25+0200, Benjamin Berg wrote:
> > > > From: Benjamin Berg <benjamin.berg@intel.com>
> > > > 
> > > > Add support for sigaction() using the rt_sigaction syscall and implement
> > > > the normal sa_mask helpers.
> > > > 
> > > > For the uapi definitions, everything is copied into nolibc. This avoids
> > > > issues with kernel architecture headers that are not usable with the
> > > > rt_sigaction syscall.
> > > > 
> > > > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> > > > 
> > > > ---
> > > > 
> > > > v3:
> > > > - put everything into signal.h and the new asm-signal.h
> > > 
> > > Hm, did we decide on that? We don't want the per-architecture include
> > > dance, but static overrides should still be fine I think.
> > > Keeping the architecture ifdeffery inside the respective arch header.
> > > And all the generic stuff in a shared header.
> > 
> > I probably just didn't really understand what you meant :-)
> > 
> > You are right, we can have the common definitions in signal.h and just
> > skip them if the architecture header did already define them.
> 
> Sounds good to me.
> 
> > I think I'll also drop asm-signal.h again, see below.
> > 
> > > > - split out sigset_t tests
> > > > - actually mark signal_check static
> > > > - remove unused string.h include
> > > > - fix SIGUSR2 reset
> > > > - Use integer for signal_check as the signals are emitted from the
> > > >   syscall context.
> > > 
> > > I don't understand this point, isn't it a signal handler?
> > 
> > My reasoning is, that the signal emission by the kernel happens from
> > the kill syscall or function return. Both cases implicitly act as a
> > memory barrier. So in this specific case we do not actually need an
> > atomic variable.
> 
> Ok. Given that the code is doing a read-modify-write, the guarantees of
> sig_atomic_t are probably not enough anyways.
> What about volatile?

As I said, I don't think we need any additional guarantees. The
compiler is not permitted/able to keep the variable in a register at
the relevant times.

This is because syscalls act as memory barrier (the kill here). I
believe the same is true for the function return of the signal handler.
So the fact that the signal emission can only happen in these two
specific locations results in sufficient guarantees.

Now, all this would not be true if the kill was done by a separate
thread or process. But, it should be entirely safe here.

I'll add a comment about why no signal safety measures are needed.

Benjamin

> > > > v2:
> > > > - Use newly added macros to check signal emission order
> > > > - Add tests for sigset handling
> > > > - Restore the default handler after signal test
> > > > - make signal_check variable static
> > > > 
> > > > v1:
> > > > - Update architecture support (adding sh)
> > > > - Move sparc sys_rt_sigaction logic into its header
> > > > - Add sig_atomic_t
> > > > - Use new BITSET_* macros
> > > > - Move test into syscall suite
> > > > - Various other small changes
> > > > ---
> > > >  tools/include/nolibc/Makefile                |   1 +
> > > >  tools/include/nolibc/arch-s390.h             |   4 +-
> > > >  tools/include/nolibc/asm-signal.h            | 237 +++++++++++++++++++
> > > >  tools/include/nolibc/signal.h                | 179 ++++++++++++++
> > > >  tools/include/nolibc/sys.h                   |   2 +-
> > > >  tools/include/nolibc/sys/wait.h              |   1 +
> > > >  tools/include/nolibc/time.h                  |   2 +-
> > > >  tools/include/nolibc/types.h                 |   9 +
> > > >  tools/testing/selftests/nolibc/nolibc-test.c | 134 +++++++++++
> > > >  9 files changed, 566 insertions(+), 3 deletions(-)
> > > >  create mode 100644 tools/include/nolibc/asm-signal.h
> > > 
> > > (...)
> > > 
> > > > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> > > > index 295e71d34aba..a790e816565b 100644
> > > > --- a/tools/include/nolibc/sys.h
> > > > +++ b/tools/include/nolibc/sys.h
> > > > @@ -14,7 +14,6 @@
> > > >  
> > > >  /* system includes */
> > > >  #include <linux/unistd.h>
> > > > -#include <linux/signal.h>  /* for SIGCHLD */
> > > >  #include <linux/termios.h>
> > > >  #include <linux/mman.h>
> > > >  #include <linux/fs.h>
> > > > @@ -28,6 +27,7 @@
> > > >  #include "errno.h"
> > > >  #include "stdarg.h"
> > > >  #include "types.h"
> > > > +#include "asm-signal.h" /* for SIGCHLD */
> > > 
> > > #include "signal.h"
> > 
> > Right, this and asm-signal.h happened because signal.h uses sys_kill()
> > for raise(), resulting in a circular dependency.
> > 
> > The simplest solution is probably to avoid the circular include by
> > implementing raise() as:
> > 
> > int raise(int signal);
> > __attribute__((weak,unused,section(".text.nolibc_raise")))
> > int raise(int signal)
> > {
> > 	return my_syscall2(__NR_kill, my_syscall0(__NR_getpid), signal);
> > }
> 
> Also sounds good. Or add local declarations sys_kill() and sys_getpid().
> 
> (...)
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

end of thread, other threads:[~2025-08-02 12:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-31 20:12 [PATCH v3 0/4] signal handling support for nolibc Benjamin Berg
2025-07-31 20:12 ` [PATCH v3 1/4] selftests/nolibc: fix EXPECT_NZ macro Benjamin Berg
2025-07-31 20:12 ` [PATCH v3 2/4] selftests/nolibc: remove outdated comment about construct order Benjamin Berg
2025-07-31 20:12 ` [PATCH v3 3/4] tools/nolibc: add more generic bitmask macros for FD_* Benjamin Berg
2025-07-31 20:12 ` [PATCH v3 4/4] tools/nolibc: add signal support Benjamin Berg
2025-08-01 15:09   ` Thomas Weißschuh
2025-08-02 10:26     ` Berg, Benjamin
2025-08-02 11:26       ` Thomas Weißschuh
2025-08-02 12:43         ` Berg, Benjamin

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 NNTP newsgroup(s).