[RFC v2] tools/nolibc: add sigaction()

[RFC v2] tools/nolibc: add sigaction()

[Benjamin Berg]

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

In preparation to add tests that use it.

Note that some architectures do not have a usable linux/signal.h include
file. However, in those cases we can use asm-generic/signal.h instead.

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

---

Another attempt at signal handling for nolibc which should actually be
working. Some trickery is needed to get the right definition, but I feel
it is sufficiently clean this way.

Submitting this as RFC mostly because I do not yet have a proper patch
to add a test that uses the feature.

Benjamin
---
 tools/include/nolibc/arch-aarch64.h          |  3 +
 tools/include/nolibc/arch-arm.h              |  7 ++
 tools/include/nolibc/arch-i386.h             | 13 +++
 tools/include/nolibc/arch-loongarch.h        |  3 +
 tools/include/nolibc/arch-m68k.h             | 10 ++
 tools/include/nolibc/arch-mips.h             |  3 +
 tools/include/nolibc/arch-powerpc.h          |  8 ++
 tools/include/nolibc/arch-riscv.h            |  3 +
 tools/include/nolibc/arch-s390.h             |  8 +-
 tools/include/nolibc/arch-sparc.h            | 43 +++++++++
 tools/include/nolibc/arch-x86_64.h           | 10 ++
 tools/include/nolibc/signal.h                | 97 ++++++++++++++++++++
 tools/include/nolibc/sys.h                   |  2 +-
 tools/include/nolibc/time.h                  |  3 +-
 tools/testing/selftests/nolibc/nolibc-test.c | 52 +++++++++++
 15 files changed, 261 insertions(+), 4 deletions(-)

diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index 937a348da42e..736aae6dbd47 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -10,6 +10,9 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Architecture has a usable linux/signal.h */
+#include <linux/signal.h>
+
 /* Syscalls for AARCH64 :
  *   - registers are 64-bit
  *   - stack is 16-byte aligned
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index 1f66e7e5a444..1faf6c2dbeb8 100644
--- a/tools/include/nolibc/arch-arm.h
+++ b/tools/include/nolibc/arch-arm.h
@@ -10,6 +10,13 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Needed to get the correct struct sigaction definition */
+#define SA_RESTORER	0x04000000
+
+/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
+#include <asm-generic/signal.h>
+#include <asm-generic/siginfo.h>
+
 /* Syscalls for ARM in ARM or Thumb modes :
  *   - registers are 32-bit
  *   - stack is 8-byte aligned
diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index 7c9b38e96418..fbec7490a92c 100644
--- a/tools/include/nolibc/arch-i386.h
+++ b/tools/include/nolibc/arch-i386.h
@@ -10,6 +10,19 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Needed to get the correct struct sigaction definition */
+#define SA_RESTORER	0x04000000
+
+/* Restorer must be set on i386 */
+#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
+
+/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
+#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
+
+/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
+#include <asm-generic/signal.h>
+#include <asm-generic/siginfo.h>
+
 /* Syscalls for i386 :
  *   - mostly similar to x86_64
  *   - registers are 32-bit
diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h
index 5511705303ea..68d60d04ef59 100644
--- a/tools/include/nolibc/arch-loongarch.h
+++ b/tools/include/nolibc/arch-loongarch.h
@@ -10,6 +10,9 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Architecture has a usable linux/signal.h */
+#include <linux/signal.h>
+
 /* Syscalls for LoongArch :
  *   - stack is 16-byte aligned
  *   - syscall number is passed in a7
diff --git a/tools/include/nolibc/arch-m68k.h b/tools/include/nolibc/arch-m68k.h
index 6dac1845f298..981b4cc55a69 100644
--- a/tools/include/nolibc/arch-m68k.h
+++ b/tools/include/nolibc/arch-m68k.h
@@ -13,6 +13,16 @@
 #include "compiler.h"
 #include "crt.h"
 
+/*
+ * Needed to get the correct struct sigaction definition. m68k does not use
+ * sa_restorer, but it is included in the structure.
+ */
+#define SA_RESTORER	0x04000000
+
+/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
+#include <asm-generic/signal.h>
+#include <asm-generic/siginfo.h>
+
 #define _NOLIBC_SYSCALL_CLOBBERLIST "memory"
 
 #define my_syscall0(num)                                                      \
diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index 753a8ed2cf69..a8837452e744 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -14,6 +14,9 @@
 #error Unsupported MIPS ABI
 #endif
 
+/* Architecture has a usable linux/signal.h */
+#include <linux/signal.h>
+
 /* Syscalls for MIPS ABI O32 :
  *   - WARNING! there's always a delayed slot!
  *   - WARNING again, the syntax is different, registers take a '$' and numbers
diff --git a/tools/include/nolibc/arch-powerpc.h b/tools/include/nolibc/arch-powerpc.h
index 204564bbcd32..c846a7ddcf3c 100644
--- a/tools/include/nolibc/arch-powerpc.h
+++ b/tools/include/nolibc/arch-powerpc.h
@@ -10,6 +10,14 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Needed to get the correct struct sigaction definition */
+#define SA_RESTORER	0x04000000
+#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
+
+/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
+#include <asm-generic/signal.h>
+#include <asm-generic/siginfo.h>
+
 /* Syscalls for PowerPC :
  *   - stack is 16-byte aligned
  *   - syscall number is passed in r0
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index 885383a86c38..709e6a262d9a 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -10,6 +10,9 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Architecture has a usable linux/signal.h */
+#include <linux/signal.h>
+
 /* Syscalls for RISCV :
  *   - stack is 16-byte aligned
  *   - syscall number is passed in a7
diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h
index df4c3cc713ac..0dccb6d1ad64 100644
--- a/tools/include/nolibc/arch-s390.h
+++ b/tools/include/nolibc/arch-s390.h
@@ -5,13 +5,19 @@
 
 #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"
 
+/* Needed to get the correct struct sigaction definition */
+#define SA_RESTORER	0x04000000
+
+/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
+#include <asm-generic/signal.h>
+#include <asm-generic/siginfo.h>
+
 /* Syscalls for s390:
  *   - registers are 64-bit
  *   - syscall number is passed in r1
diff --git a/tools/include/nolibc/arch-sparc.h b/tools/include/nolibc/arch-sparc.h
index 1435172f3dfe..303291d4b8fb 100644
--- a/tools/include/nolibc/arch-sparc.h
+++ b/tools/include/nolibc/arch-sparc.h
@@ -12,6 +12,19 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
+#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
+
+/* The includes are sane, if one sets __WANT_POSIX1B_SIGNALS__ */
+#define __WANT_POSIX1B_SIGNALS__
+#include <linux/signal.h>
+
+/*
+ * sparc has ODD_RT_SIGACTION, we always pass our restorer as an argument
+ * to rt_sigaction. The restorer is implemented in this file.
+ */
+#define _NOLIBC_RT_SIGACTION_PASSES_RESTORER
+
 /*
  * Syscalls for SPARC:
  *   - registers are native word size
@@ -188,4 +201,34 @@ pid_t sys_fork(void)
 }
 #define sys_fork sys_fork
 
+#define __nolibc_stringify_1(x...)     #x
+#define __nolibc_stringify(x...)       __stringify_1(x)
+
+/* The compiler insists on adding a SAVE call to the start of every function */
+#define __nolibc_sa_restorer __nolibc_sa_restorer
+void __nolibc_sa_restorer (void);
+#ifdef __arch64__
+__asm__(                                                        \
+	".section .text\n"                                      \
+	".align  4 \n"                                          \
+	"__nolibc_sa_restorer:\n"                               \
+	"nop\n"                                                 \
+	"nop\n"                                                 \
+	"mov     " __stringify(__NR_rt_sigreturn) ", %g1 \n"    \
+	"t       0x6d \n");
+#else
+__asm__(                                                        \
+	".section .text\n"                                      \
+	".align  4 \n"                                          \
+	"__nolibc_sa_restorer:\n"                               \
+	"nop\n"                                                 \
+	"nop\n"                                                 \
+	"mov     " __stringify(__NR_rt_sigreturn) ", %g1 \n"    \
+	"t       0x10 \n"                                       \
+	);
+#endif
+
+#undef __nolibc_stringify_1(x...)
+#undef __nolibc_stringify
+
 #endif /* _NOLIBC_ARCH_SPARC_H */
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index 67305e24dbef..9f13a2205876 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -10,6 +10,16 @@
 #include "compiler.h"
 #include "crt.h"
 
+/* Needed to get the correct struct sigaction definition */
+#define SA_RESTORER	0x04000000
+
+/* Restorer must be set on i386 */
+#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
+
+/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
+#include <asm-generic/signal.h>
+#include <asm-generic/siginfo.h>
+
 /* Syscalls for x86_64 :
  *   - registers are 64-bit
  *   - syscall number is passed in rax
diff --git a/tools/include/nolibc/signal.h b/tools/include/nolibc/signal.h
index ac13e53ac31d..829672250ede 100644
--- a/tools/include/nolibc/signal.h
+++ b/tools/include/nolibc/signal.h
@@ -14,6 +14,8 @@
 #include "arch.h"
 #include "types.h"
 #include "sys.h"
+#include "string.h"
+/* signal definitions are included by arch.h */
 
 /* This one is not marked static as it's needed by libgcc for divide by zero */
 int raise(int signal);
@@ -23,4 +25,99 @@ int raise(int signal)
 	return sys_kill(sys_getpid(), signal);
 }
 
+/*
+ * sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
+ */
+#if defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER) && !defined(__nolibc_sa_restorer)
+static __attribute__((noreturn)) __nolibc_entrypoint __no_stack_protector
+void __nolibc_sa_restorer(void)
+{
+	my_syscall0(__NR_rt_sigreturn);
+	__nolibc_entrypoint_epilogue();
+}
+#endif
+
+static __attribute__((unused))
+int sys_rt_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
+{
+	struct sigaction real_act = *act;
+#if defined(SA_RESTORER) && defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER)
+	if (!(real_act.sa_flags & SA_RESTORER)) {
+		real_act.sa_flags |= SA_RESTORER;
+		real_act.sa_restorer = __nolibc_sa_restorer;
+	}
+#endif
+#ifdef _NOLIBC_ARCH_FORCE_SIG_FLAGS
+	real_act.sa_flags |= _NOLIBC_ARCH_FORCE_SIG_FLAGS;
+#endif
+
+#ifndef _NOLIBC_RT_SIGACTION_PASSES_RESTORER
+	return my_syscall4(__NR_rt_sigaction, signum, &real_act, oldact,
+			   sizeof(act->sa_mask));
+#else
+	return my_syscall5(__NR_rt_sigaction, signum, &real_act, oldact,
+			   __nolibc_sa_restorer, 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)
+{
+	memset(set, 0, sizeof(*set));
+	return 0;
+}
+
+/*
+ * int sigfillset(sigset_t *set)
+ */
+static __attribute__((unused))
+int sigfillset(sigset_t *set)
+{
+	memset(set, 0xff, sizeof(*set));
+	return 0;
+}
+
+/*
+ * int sigaddset(sigset_t *set, int signum)
+ */
+static __attribute__((unused))
+int sigaddset(sigset_t *set, int signum)
+{
+	set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] |=
+		1UL << ((signum - 1) % (8 * sizeof(set->sig[0])));
+	return 0;
+}
+
+/*
+ * int sigdelset(sigset_t *set, int signum)
+ */
+static __attribute__((unused))
+int sigdelset(sigset_t *set, int signum)
+{
+	set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] &=
+		~(1UL << ((signum - 1) % (8 * sizeof(set->sig[0]))));
+	return 0;
+}
+
+/*
+ * int sigismember(sigset_t *set, int signum)
+ */
+static __attribute__((unused))
+int sigismember(sigset_t *set, int signum)
+{
+	unsigned long res =
+		set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] &
+			(1UL << ((signum - 1) % (8 * sizeof(set->sig[0]))));
+	return !!res;
+}
+
 #endif /* _NOLIBC_SIGNAL_H */
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 9556c69a6ae1..fb9a1ccf2440 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>
@@ -23,6 +22,7 @@
 #include <linux/auxvec.h>
 #include <linux/fcntl.h> /* for O_* and AT_* */
 #include <linux/stat.h>  /* for statx() */
+/* signal definitions are included by arch.h */
 
 #include "errno.h"
 #include "stdarg.h"
diff --git a/tools/include/nolibc/time.h b/tools/include/nolibc/time.h
index fc387940d51f..70ef31d4117f 100644
--- a/tools/include/nolibc/time.h
+++ b/tools/include/nolibc/time.h
@@ -14,9 +14,8 @@
 #include "arch.h"
 #include "types.h"
 #include "sys.h"
-
-#include <linux/signal.h>
 #include <linux/time.h>
+/* signal definitions are included by arch.h */
 
 static __inline__
 void __nolibc_timespec_user_to_kernel(const struct timespec *ts, struct __kernel_timespec *kts)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index dbe13000fb1a..af66b739ea18 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1750,6 +1750,57 @@ static int run_protection(int min __attribute__((unused)),
 	}
 }
 
+volatile int signal_check;
+
+void test_sighandler(int signum)
+{
+	if (signum == SIGUSR1) {
+		kill(getpid(), SIGUSR2);
+		signal_check = 1;
+	} else {
+		signal_check++;
+	}
+}
+
+int run_signal(int min, int max)
+{
+	struct sigaction sa = {
+		.sa_flags = 0,
+		.sa_handler = test_sighandler,
+	};
+	int llen; /* line length */
+	int ret = 0;
+	int res;
+
+	(void)min;
+	(void)max;
+
+	signal_check = 0;
+
+	sigemptyset(&sa.sa_mask);
+	sigaddset(&sa.sa_mask, SIGUSR2);
+
+	res = sigaction(SIGUSR1, &sa, NULL);
+	llen = printf("register SIGUSR1: %d", res);
+	EXPECT_EQ(1, 0, res);
+	res = sigaction(SIGUSR2, &sa, NULL);
+	llen = printf("register SIGUSR2: %d", res);
+	EXPECT_EQ(1, 0, res);
+
+	/* Trigger the first signal. */
+	kill(getpid(), SIGUSR1);
+
+	/* If signal_check is 1 or higher, then signal emission worked */
+	llen = printf("signal emission: 1 <= signal_check");
+	EXPECT_GE(1, signal_check, 1);
+
+	/* If it is 2, then signal masking worked */
+	llen = printf("signal masking: 2 == signal_check");
+	EXPECT_EQ(1, signal_check, 2);
+
+	return ret;
+}
+
 /* prepare what needs to be prepared for pid 1 (stdio, /dev, /proc, etc) */
 int prepare(void)
 {
@@ -1815,6 +1866,7 @@ static const struct test test_names[] = {
 	{ .name = "stdlib",     .func = run_stdlib     },
 	{ .name = "printf",     .func = run_printf     },
 	{ .name = "protection", .func = run_protection },
+	{ .name = "signal",     .func = run_signal },
 	{ 0 }
 };
 
-- 
2.50.0

Re: [RFC v2] tools/nolibc: add sigaction()

[Thomas Weißschuh]

Hi Benjamin,

Thanks for your patch!

On 2025-07-01 14:29:10+0200, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.berg@intel.com>

Please send the next revisions to the nolibc maintainers from the
MAINTAINERS file. Willy and my @weissschuh.net address.

> In preparation to add tests that use it.

Here "tests" is not clear about its meaning.

> Note that some architectures do not have a usable linux/signal.h include
> file. However, in those cases we can use asm-generic/signal.h instead.

This should explain what "unusable" means.

> Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> 
> ---
> 
> Another attempt at signal handling for nolibc which should actually be
> working. Some trickery is needed to get the right definition, but I feel
> it is sufficiently clean this way.
> 
> Submitting this as RFC mostly because I do not yet have a proper patch
> to add a test that uses the feature.

We do pick up new features in nolibc without them having in-kernel users.
So if you want to get this in already you can drop the RFC state.

> Benjamin
> ---
>  tools/include/nolibc/arch-aarch64.h          |  3 +

In nolibc/for-next, arch-aarch64.h is now called arch-arm64.h.

https://git.kernel.org/pub/scm/linux/kernel/git/nolibc/linux-nolibc.git/log/?h=for-next

>  tools/include/nolibc/arch-arm.h              |  7 ++
>  tools/include/nolibc/arch-i386.h             | 13 +++

... and arch-i386.h was renamed to arch-x86.h

>  tools/include/nolibc/arch-loongarch.h        |  3 +
>  tools/include/nolibc/arch-m68k.h             | 10 ++
>  tools/include/nolibc/arch-mips.h             |  3 +
>  tools/include/nolibc/arch-powerpc.h          |  8 ++
>  tools/include/nolibc/arch-riscv.h            |  3 +
>  tools/include/nolibc/arch-s390.h             |  8 +-
>  tools/include/nolibc/arch-sparc.h            | 43 +++++++++
>  tools/include/nolibc/arch-x86_64.h           | 10 ++

same for arch-x86_64.h, Unlucky timing.

>  tools/include/nolibc/signal.h                | 97 ++++++++++++++++++++
>  tools/include/nolibc/sys.h                   |  2 +-
>  tools/include/nolibc/time.h                  |  3 +-
>  tools/testing/selftests/nolibc/nolibc-test.c | 52 +++++++++++
>  15 files changed, 261 insertions(+), 4 deletions(-)

<snip>

> --- a/tools/include/nolibc/arch-i386.h
> +++ b/tools/include/nolibc/arch-i386.h
> @@ -10,6 +10,19 @@
>  #include "compiler.h"
>  #include "crt.h"
>  
> +/* Needed to get the correct struct sigaction definition */
> +#define SA_RESTORER	0x04000000
> +
> +/* Restorer must be set on i386 */
> +#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
> +
> +/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
> +#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
> +
> +/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
> +#include <asm-generic/signal.h>
> +#include <asm-generic/siginfo.h>

This doesn't work if the user already has <linux/signal.h> included for
some other reason. The symbol names will conflict.

Can we do something like this:

#include <linux/signal.h>

/* lifted from asm-generic/signal.h */
struct __nolibc_sigaction {
	__sighandler_t sa_handler;
	unsigned long sa_flags;
#ifdef SA_RESTORER
	__sigrestore_t sa_restorer;
#endif
	__nolibc_sigset_t sa_mask;
};

int sys_rt_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
{
	struct __nolibc_sigaction nolibc_act, nolibc_oldact;
	int ret;

	/* Convert whatever gunk we got from linux/signal.h to what the
	 * kernel actually expects. If it is the same structure,
	 * hopefully the compiler manages to optimize it away.
	 * See __nolibc_user_timespec_to_kernel() et al.
	 * (Comment not meant to be copied verbatim)
	 */
	__nolibc_sigaction_user_to_kernel(act, &nolibc_act);

#if defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER)
...
#endif

	ret = my_syscall4(__NR_rt_sigaction, signum, &nolibc_act, &nolibc_oldact, sizeof(nolibc_act->sa_mask));

	__nolibc_sigaction_kernel_to_user(&nolibc_oldact, oldact);

	return ret;
}

Or am I missing something?

> +
>  /* Syscalls for i386 :
>   *   - mostly similar to x86_64
>   *   - registers are 32-bit

<snip>

> --- a/tools/include/nolibc/arch-sparc.h
> +++ b/tools/include/nolibc/arch-sparc.h
> @@ -12,6 +12,19 @@
>  #include "compiler.h"
>  #include "crt.h"
>  
> +/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
> +#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
> +
> +/* The includes are sane, if one sets __WANT_POSIX1B_SIGNALS__ */
> +#define __WANT_POSIX1B_SIGNALS__
> +#include <linux/signal.h>

This also assumes the user did not already include <linux/signal.h>
before including nolibc.

> +
> +/*
> + * sparc has ODD_RT_SIGACTION, we always pass our restorer as an argument
> + * to rt_sigaction. The restorer is implemented in this file.
> + */
> +#define _NOLIBC_RT_SIGACTION_PASSES_RESTORER
> +
>  /*
>   * Syscalls for SPARC:
>   *   - registers are native word size
> @@ -188,4 +201,34 @@ pid_t sys_fork(void)
>  }
>  #define sys_fork sys_fork
>  
> +#define __nolibc_stringify_1(x...)     #x
> +#define __nolibc_stringify(x...)       __stringify_1(x)

If we need this, IMO it belongs into compiler.h.

> +
> +/* The compiler insists on adding a SAVE call to the start of every function */
> +#define __nolibc_sa_restorer __nolibc_sa_restorer
> +void __nolibc_sa_restorer (void);
> +#ifdef __arch64__
> +__asm__(                                                        \

We are avoiding bare toplevel asm calls.
You could use the same trick as my SuperH _start() function and use
asm() inside a function.

https://lore.kernel.org/lkml/20250623-nolibc-sh-v2-3-0f5b4b303025@weissschuh.net/

> +	".section .text\n"                                      \
> +	".align  4 \n"                                          \
> +	"__nolibc_sa_restorer:\n"                               \
> +	"nop\n"                                                 \
> +	"nop\n"                                                 \
> +	"mov     " __stringify(__NR_rt_sigreturn) ", %g1 \n"    \
> +	"t       0x6d \n");
> +#else
> +__asm__(                                                        \
> +	".section .text\n"                                      \
> +	".align  4 \n"                                          \
> +	"__nolibc_sa_restorer:\n"                               \
> +	"nop\n"                                                 \
> +	"nop\n"                                                 \
> +	"mov     " __stringify(__NR_rt_sigreturn) ", %g1 \n"    \
> +	"t       0x10 \n"                                       \

Only one line differs. I'd prefer the #ifdef around that.

> +	);
> +#endif
> +
> +#undef __nolibc_stringify_1(x...)
> +#undef __nolibc_stringify

And there is no need to undef it again.

> +
>  #endif /* _NOLIBC_ARCH_SPARC_H */
> diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
> index 67305e24dbef..9f13a2205876 100644

<snip>

> --- a/tools/include/nolibc/signal.h
> +++ b/tools/include/nolibc/signal.h
> @@ -14,6 +14,8 @@
>  #include "arch.h"
>  #include "types.h"
>  #include "sys.h"
> +#include "string.h"
> +/* signal definitions are included by arch.h */
>  
>  /* This one is not marked static as it's needed by libgcc for divide by zero */
>  int raise(int signal);
> @@ -23,4 +25,99 @@ int raise(int signal)
>  	return sys_kill(sys_getpid(), signal);
>  }
>  
> +/*
> + * sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
> + */
> +#if defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER) && !defined(__nolibc_sa_restorer)
> +static __attribute__((noreturn)) __nolibc_entrypoint __no_stack_protector

__attribute__((noreturn)) is not always available and should be behind
__nolibc_has_attribute(). 
I'm a bit unhappy about reusing the entrypoint machinery, but I guess
it's necessary.

> +void __nolibc_sa_restorer(void)
> +{
> +	my_syscall0(__NR_rt_sigreturn);
> +	__nolibc_entrypoint_epilogue();
> +}
> +#endif
> +
> +static __attribute__((unused))
> +int sys_rt_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
> +{
> +	struct sigaction real_act = *act;
> +#if defined(SA_RESTORER) && defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER)

If _NOLIBC_ARCH_NEEDS_SA_RESTORER is set then SA_RESTORER should also be
present. SA_RESTORER doesn't need to be checked also.
Are there cases where SA_RESTORER is defined but not needed?

> +	if (!(real_act.sa_flags & SA_RESTORER)) {
> +		real_act.sa_flags |= SA_RESTORER;
> +		real_act.sa_restorer = __nolibc_sa_restorer;
> +	}
> +#endif
> +#ifdef _NOLIBC_ARCH_FORCE_SIG_FLAGS
> +	real_act.sa_flags |= _NOLIBC_ARCH_FORCE_SIG_FLAGS;
> +#endif
> +
> +#ifndef _NOLIBC_RT_SIGACTION_PASSES_RESTORER
> +	return my_syscall4(__NR_rt_sigaction, signum, &real_act, oldact,
> +			   sizeof(act->sa_mask));
> +#else
> +	return my_syscall5(__NR_rt_sigaction, signum, &real_act, oldact,
> +			   __nolibc_sa_restorer, sizeof(act->sa_mask));

This calling convention is specific to SPARC, so I prefer it to be in
arch-sparc.h. Alpha also uses 5 arguments for the syscall, but of course
in a different order... (I do have nearly done patches for alpha
support).

Also if a user specified their custom sa_restorer in 'act', that one
should be used, no?

> +#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)
> +{
> +	memset(set, 0, sizeof(*set));
> +	return 0;
> +}
> +
> +/*
> + * int sigfillset(sigset_t *set)
> + */
> +static __attribute__((unused))
> +int sigfillset(sigset_t *set)
> +{
> +	memset(set, 0xff, sizeof(*set));
> +	return 0;
> +}
> +
> +/*
> + * int sigaddset(sigset_t *set, int signum)
> + */
> +static __attribute__((unused))
> +int sigaddset(sigset_t *set, int signum)
> +{
> +	set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] |=
> +		1UL << ((signum - 1) % (8 * sizeof(set->sig[0])));
> +	return 0;

This is documented to return EINVAL for an invalid signum.

> +}
> +
> +/*
> + * int sigdelset(sigset_t *set, int signum)
> + */
> +static __attribute__((unused))
> +int sigdelset(sigset_t *set, int signum)
> +{
> +	set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] &=
> +		~(1UL << ((signum - 1) % (8 * sizeof(set->sig[0]))));
> +	return 0;
> +}
> +
> +/*
> + * int sigismember(sigset_t *set, int signum)
> + */
> +static __attribute__((unused))
> +int sigismember(sigset_t *set, int signum)
> +{
> +	unsigned long res =
> +		set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] &
> +			(1UL << ((signum - 1) % (8 * sizeof(set->sig[0]))));
> +	return !!res;
> +}

These are similar to FD_CLR()/FD_SET() etc, no?
Moving both sets of functions to common inline helpers would be nice.

> +
>  #endif /* _NOLIBC_SIGNAL_H */

<snip>

> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index dbe13000fb1a..af66b739ea18 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -1750,6 +1750,57 @@ static int run_protection(int min __attribute__((unused)),
>  	}
>  }
>  
> +volatile int signal_check;

Technically this should be sig_atomic_t.
Which can be a typedef to int.

> +void test_sighandler(int signum)
> +{
> +	if (signum == SIGUSR1) {
> +		kill(getpid(), SIGUSR2);
> +		signal_check = 1;
> +	} else {
> +		signal_check++;
> +	}
> +}
> +
> +int run_signal(int min, int max)
> +{
> +	struct sigaction sa = {
> +		.sa_flags = 0,
> +		.sa_handler = test_sighandler,
> +	};
> +	int llen; /* line length */
> +	int ret = 0;
> +	int res;
> +
> +	(void)min;
> +	(void)max;
> +
> +	signal_check = 0;
> +
> +	sigemptyset(&sa.sa_mask);
> +	sigaddset(&sa.sa_mask, SIGUSR2);

It would be good to do some assertions after sigemptyset() and
sigaddset() to validate the bitfiddling.

> +
> +	res = sigaction(SIGUSR1, &sa, NULL);
> +	llen = printf("register SIGUSR1: %d", res);
> +	EXPECT_EQ(1, 0, res);
> +	res = sigaction(SIGUSR2, &sa, NULL);
> +	llen = printf("register SIGUSR2: %d", res);
> +	EXPECT_EQ(1, 0, res);

Here it would be nice to validate the old action.

> +
> +	/* Trigger the first signal. */
> +	kill(getpid(), SIGUSR1);
> +
> +	/* If signal_check is 1 or higher, then signal emission worked */
> +	llen = printf("signal emission: 1 <= signal_check");
> +	EXPECT_GE(1, signal_check, 1);
> +
> +	/* If it is 2, then signal masking worked */
> +	llen = printf("signal masking: 2 == signal_check");
> +	EXPECT_EQ(1, signal_check, 2);
> +
> +	return ret;
> +}
> +
>  /* prepare what needs to be prepared for pid 1 (stdio, /dev, /proc, etc) */
>  int prepare(void)
>  {
> @@ -1815,6 +1866,7 @@ static const struct test test_names[] = {
>  	{ .name = "stdlib",     .func = run_stdlib     },
>  	{ .name = "printf",     .func = run_printf     },
>  	{ .name = "protection", .func = run_protection },
> +	{ .name = "signal",     .func = run_signal },

These 'struct test's are really more test suites.
This testcase can be part of the syscall suite.

>  	{ 0 }
>  };
>  
> -- 
> 2.50.0
> 

Re: [RFC v2] tools/nolibc: add sigaction()

[Benjamin Berg]

Hi,

On Wed, 2025-07-02 at 00:04 +0200, Thomas Weißschuh wrote:
> [SNIP]
> > --- a/tools/include/nolibc/arch-i386.h
> > +++ b/tools/include/nolibc/arch-i386.h
> > @@ -10,6 +10,19 @@
> >  #include "compiler.h"
> >  #include "crt.h"
> >  
> > +/* Needed to get the correct struct sigaction definition */
> > +#define SA_RESTORER	0x04000000
> > +
> > +/* Restorer must be set on i386 */
> > +#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
> > +
> > +/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
> > +#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
> > +
> > +/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
> > +#include <asm-generic/signal.h>
> > +#include <asm-generic/siginfo.h>
> 
> This doesn't work if the user already has <linux/signal.h> included for
> some other reason. The symbol names will conflict.

I was thinking this is fine. Such a conflict already exists between the
normal glibc <signal.h> and <linux/signal.h>. So there would only be a
problem if the user is explicitly not including <signal.h> to then use
<linux/signal.h>. I doubt that makes sense.

Benjamin

> 
> Can we do something like this:
> 
> #include <linux/signal.h>
> 
> /* lifted from asm-generic/signal.h */
> struct __nolibc_sigaction {
> 	__sighandler_t sa_handler;
> 	unsigned long sa_flags;
> #ifdef SA_RESTORER
> 	__sigrestore_t sa_restorer;
> #endif
> 	__nolibc_sigset_t sa_mask;
> };
> 
> int sys_rt_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
> {
> 	struct __nolibc_sigaction nolibc_act, nolibc_oldact;
> 	int ret;
> 
> 	/* Convert whatever gunk we got from linux/signal.h to what the
> 	 * kernel actually expects. If it is the same structure,
> 	 * hopefully the compiler manages to optimize it away.
> 	 * See __nolibc_user_timespec_to_kernel() et al.
> 	 * (Comment not meant to be copied verbatim)
> 	 */
> 	__nolibc_sigaction_user_to_kernel(act, &nolibc_act);
> 
> #if defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER)
> ...
> #endif
> 
> 	ret = my_syscall4(__NR_rt_sigaction, signum, &nolibc_act, &nolibc_oldact, sizeof(nolibc_act->sa_mask));
> 
> 	__nolibc_sigaction_kernel_to_user(&nolibc_oldact, oldact);
> 
> 	return ret;
> }
> 
> Or am I missing something?
> 
> > +
> >  /* Syscalls for i386 :
> >   *   - mostly similar to x86_64
> >   *   - registers are 32-bit
> 
> <snip>
> 
> > --- a/tools/include/nolibc/arch-sparc.h
> > +++ b/tools/include/nolibc/arch-sparc.h
> > @@ -12,6 +12,19 @@
> >  #include "compiler.h"
> >  #include "crt.h"
> >  
> > +/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
> > +#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
> > +
> > +/* The includes are sane, if one sets __WANT_POSIX1B_SIGNALS__ */
> > +#define __WANT_POSIX1B_SIGNALS__
> > +#include <linux/signal.h>
> 
> This also assumes the user did not already include <linux/signal.h>
> before including nolibc.
> 
> > +
> > +/*
> > + * sparc has ODD_RT_SIGACTION, we always pass our restorer as an argument
> > + * to rt_sigaction. The restorer is implemented in this file.
> > + */
> > +#define _NOLIBC_RT_SIGACTION_PASSES_RESTORER
> > +
> >  /*
> >   * Syscalls for SPARC:
> >   *   - registers are native word size
> > @@ -188,4 +201,34 @@ pid_t sys_fork(void)
> >  }
> >  #define sys_fork sys_fork
> >  
> > +#define __nolibc_stringify_1(x...)     #x
> > +#define __nolibc_stringify(x...)       __stringify_1(x)
> 
> If we need this, IMO it belongs into compiler.h.
> 
> > +
> > +/* The compiler insists on adding a SAVE call to the start of every function */
> > +#define __nolibc_sa_restorer __nolibc_sa_restorer
> > +void __nolibc_sa_restorer (void);
> > +#ifdef __arch64__
> > +__asm__(                                                        \
> 
> We are avoiding bare toplevel asm calls.
> You could use the same trick as my SuperH _start() function and use
> asm() inside a function.
> 
> https://lore.kernel.org/lkml/20250623-nolibc-sh-v2-3-0f5b4b303025@weissschuh.net/
> 
> > +	".section .text\n"                                      \
> > +	".align  4 \n"                                          \
> > +	"__nolibc_sa_restorer:\n"                               \
> > +	"nop\n"                                                 \
> > +	"nop\n"                                                 \
> > +	"mov     " __stringify(__NR_rt_sigreturn) ", %g1 \n"    \
> > +	"t       0x6d \n");
> > +#else
> > +__asm__(                                                        \
> > +	".section .text\n"                                      \
> > +	".align  4 \n"                                          \
> > +	"__nolibc_sa_restorer:\n"                               \
> > +	"nop\n"                                                 \
> > +	"nop\n"                                                 \
> > +	"mov     " __stringify(__NR_rt_sigreturn) ", %g1 \n"    \
> > +	"t       0x10 \n"                                       \
> 
> Only one line differs. I'd prefer the #ifdef around that.
> 
> > +	);
> > +#endif
> > +
> > +#undef __nolibc_stringify_1(x...)
> > +#undef __nolibc_stringify
> 
> And there is no need to undef it again.
> 
> > +
> >  #endif /* _NOLIBC_ARCH_SPARC_H */
> > diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
> > index 67305e24dbef..9f13a2205876 100644
> 
> <snip>
> 
> > --- a/tools/include/nolibc/signal.h
> > +++ b/tools/include/nolibc/signal.h
> > @@ -14,6 +14,8 @@
> >  #include "arch.h"
> >  #include "types.h"
> >  #include "sys.h"
> > +#include "string.h"
> > +/* signal definitions are included by arch.h */
> >  
> >  /* This one is not marked static as it's needed by libgcc for divide by zero */
> >  int raise(int signal);
> > @@ -23,4 +25,99 @@ int raise(int signal)
> >  	return sys_kill(sys_getpid(), signal);
> >  }
> >  
> > +/*
> > + * sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
> > + */
> > +#if defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER) && !defined(__nolibc_sa_restorer)
> > +static __attribute__((noreturn)) __nolibc_entrypoint __no_stack_protector
> 
> __attribute__((noreturn)) is not always available and should be behind
> __nolibc_has_attribute(). 
> I'm a bit unhappy about reusing the entrypoint machinery, but I guess
> it's necessary.
> 
> > +void __nolibc_sa_restorer(void)
> > +{
> > +	my_syscall0(__NR_rt_sigreturn);
> > +	__nolibc_entrypoint_epilogue();
> > +}
> > +#endif
> > +
> > +static __attribute__((unused))
> > +int sys_rt_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact)
> > +{
> > +	struct sigaction real_act = *act;
> > +#if defined(SA_RESTORER) && defined(_NOLIBC_ARCH_NEEDS_SA_RESTORER)
> 
> If _NOLIBC_ARCH_NEEDS_SA_RESTORER is set then SA_RESTORER should also be
> present. SA_RESTORER doesn't need to be checked also.
> Are there cases where SA_RESTORER is defined but not needed?
> 
> > +	if (!(real_act.sa_flags & SA_RESTORER)) {
> > +		real_act.sa_flags |= SA_RESTORER;
> > +		real_act.sa_restorer = __nolibc_sa_restorer;
> > +	}
> > +#endif
> > +#ifdef _NOLIBC_ARCH_FORCE_SIG_FLAGS
> > +	real_act.sa_flags |= _NOLIBC_ARCH_FORCE_SIG_FLAGS;
> > +#endif
> > +
> > +#ifndef _NOLIBC_RT_SIGACTION_PASSES_RESTORER
> > +	return my_syscall4(__NR_rt_sigaction, signum, &real_act, oldact,
> > +			   sizeof(act->sa_mask));
> > +#else
> > +	return my_syscall5(__NR_rt_sigaction, signum, &real_act, oldact,
> > +			   __nolibc_sa_restorer, sizeof(act->sa_mask));
> 
> This calling convention is specific to SPARC, so I prefer it to be in
> arch-sparc.h. Alpha also uses 5 arguments for the syscall, but of course
> in a different order... (I do have nearly done patches for alpha
> support).
> 
> Also if a user specified their custom sa_restorer in 'act', that one
> should be used, no?
> 
> > +#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)
> > +{
> > +	memset(set, 0, sizeof(*set));
> > +	return 0;
> > +}
> > +
> > +/*
> > + * int sigfillset(sigset_t *set)
> > + */
> > +static __attribute__((unused))
> > +int sigfillset(sigset_t *set)
> > +{
> > +	memset(set, 0xff, sizeof(*set));
> > +	return 0;
> > +}
> > +
> > +/*
> > + * int sigaddset(sigset_t *set, int signum)
> > + */
> > +static __attribute__((unused))
> > +int sigaddset(sigset_t *set, int signum)
> > +{
> > +	set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] |=
> > +		1UL << ((signum - 1) % (8 * sizeof(set->sig[0])));
> > +	return 0;
> 
> This is documented to return EINVAL for an invalid signum.
> 
> > +}
> > +
> > +/*
> > + * int sigdelset(sigset_t *set, int signum)
> > + */
> > +static __attribute__((unused))
> > +int sigdelset(sigset_t *set, int signum)
> > +{
> > +	set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] &=
> > +		~(1UL << ((signum - 1) % (8 * sizeof(set->sig[0]))));
> > +	return 0;
> > +}
> > +
> > +/*
> > + * int sigismember(sigset_t *set, int signum)
> > + */
> > +static __attribute__((unused))
> > +int sigismember(sigset_t *set, int signum)
> > +{
> > +	unsigned long res =
> > +		set->sig[(signum - 1) / (8 * sizeof(set->sig[0]))] &
> > +			(1UL << ((signum - 1) % (8 * sizeof(set->sig[0]))));
> > +	return !!res;
> > +}
> 
> These are similar to FD_CLR()/FD_SET() etc, no?
> Moving both sets of functions to common inline helpers would be nice.
> 
> > +
> >  #endif /* _NOLIBC_SIGNAL_H */
> 
> <snip>
> 
> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > index dbe13000fb1a..af66b739ea18 100644
> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > @@ -1750,6 +1750,57 @@ static int run_protection(int min __attribute__((unused)),
> >  	}
> >  }
> >  
> > +volatile int signal_check;
> 
> Technically this should be sig_atomic_t.
> Which can be a typedef to int.
> 
> > +void test_sighandler(int signum)
> > +{
> > +	if (signum == SIGUSR1) {
> > +		kill(getpid(), SIGUSR2);
> > +		signal_check = 1;
> > +	} else {
> > +		signal_check++;
> > +	}
> > +}
> > +
> > +int run_signal(int min, int max)
> > +{
> > +	struct sigaction sa = {
> > +		.sa_flags = 0,
> > +		.sa_handler = test_sighandler,
> > +	};
> > +	int llen; /* line length */
> > +	int ret = 0;
> > +	int res;
> > +
> > +	(void)min;
> > +	(void)max;
> > +
> > +	signal_check = 0;
> > +
> > +	sigemptyset(&sa.sa_mask);
> > +	sigaddset(&sa.sa_mask, SIGUSR2);
> 
> It would be good to do some assertions after sigemptyset() and
> sigaddset() to validate the bitfiddling.
> 
> > +
> > +	res = sigaction(SIGUSR1, &sa, NULL);
> > +	llen = printf("register SIGUSR1: %d", res);
> > +	EXPECT_EQ(1, 0, res);
> > +	res = sigaction(SIGUSR2, &sa, NULL);
> > +	llen = printf("register SIGUSR2: %d", res);
> > +	EXPECT_EQ(1, 0, res);
> 
> Here it would be nice to validate the old action.
> 
> > +
> > +	/* Trigger the first signal. */
> > +	kill(getpid(), SIGUSR1);
> > +
> > +	/* If signal_check is 1 or higher, then signal emission worked */
> > +	llen = printf("signal emission: 1 <= signal_check");
> > +	EXPECT_GE(1, signal_check, 1);
> > +
> > +	/* If it is 2, then signal masking worked */
> > +	llen = printf("signal masking: 2 == signal_check");
> > +	EXPECT_EQ(1, signal_check, 2);
> > +
> > +	return ret;
> > +}
> > +
> >  /* prepare what needs to be prepared for pid 1 (stdio, /dev, /proc, etc) */
> >  int prepare(void)
> >  {
> > @@ -1815,6 +1866,7 @@ static const struct test test_names[] = {
> >  	{ .name = "stdlib",     .func = run_stdlib     },
> >  	{ .name = "printf",     .func = run_printf     },
> >  	{ .name = "protection", .func = run_protection },
> > +	{ .name = "signal",     .func = run_signal },
> 
> These 'struct test's are really more test suites.
> This testcase can be part of the syscall suite.
> 
> >  	{ 0 }
> >  };
> >  
> > -- 
> > 2.50.0
> > 
> 

Re: [RFC v2] tools/nolibc: add sigaction()

[Thomas Weißschuh]

On 2025-07-02 17:33:24+0200, Benjamin Berg wrote:
> On Wed, 2025-07-02 at 00:04 +0200, Thomas Weißschuh wrote:
> > [SNIP]
> > > --- a/tools/include/nolibc/arch-i386.h
> > > +++ b/tools/include/nolibc/arch-i386.h
> > > @@ -10,6 +10,19 @@
> > >  #include "compiler.h"
> > >  #include "crt.h"
> > >  
> > > +/* Needed to get the correct struct sigaction definition */
> > > +#define SA_RESTORER	0x04000000
> > > +
> > > +/* Restorer must be set on i386 */
> > > +#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
> > > +
> > > +/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
> > > +#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
> > > +
> > > +/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
> > > +#include <asm-generic/signal.h>
> > > +#include <asm-generic/siginfo.h>
> > 
> > This doesn't work if the user already has <linux/signal.h> included for
> > some other reason. The symbol names will conflict.
> 
> I was thinking this is fine. Such a conflict already exists between the
> normal glibc <signal.h> and <linux/signal.h>.

It would be enough to keep compatibility with glibc.
But personally I'd like to make it work generally.

> So there would only be a
> problem if the user is explicitly not including <signal.h> to then use
> <linux/signal.h>. I doubt that makes sense.

Technically nolibc is always included as a whole, so any application
using it would be prevented from using linux/signal.h.

Maybe Willy has some strong opinions.
Otherwise I'm also fine if we keep this part as is for now.
Then if everything else is addressed I'll try to actually implement my
proposal on top.

Does this sound reasonable?


Thomas

Re: [RFC v2] tools/nolibc: add sigaction()

[Willy Tarreau]

On Fri, Jul 04, 2025 at 12:50:32PM +0200, Thomas Weißschuh wrote:
> On 2025-07-02 17:33:24+0200, Benjamin Berg wrote:
> > On Wed, 2025-07-02 at 00:04 +0200, Thomas Weißschuh wrote:
> > > [SNIP]
> > > > --- a/tools/include/nolibc/arch-i386.h
> > > > +++ b/tools/include/nolibc/arch-i386.h
> > > > @@ -10,6 +10,19 @@
> > > >  #include "compiler.h"
> > > >  #include "crt.h"
> > > >  
> > > > +/* Needed to get the correct struct sigaction definition */
> > > > +#define SA_RESTORER	0x04000000
> > > > +
> > > > +/* Restorer must be set on i386 */
> > > > +#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
> > > > +
> > > > +/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
> > > > +#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
> > > > +
> > > > +/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
> > > > +#include <asm-generic/signal.h>
> > > > +#include <asm-generic/siginfo.h>
> > > 
> > > This doesn't work if the user already has <linux/signal.h> included for
> > > some other reason. The symbol names will conflict.
> > 
> > I was thinking this is fine. Such a conflict already exists between the
> > normal glibc <signal.h> and <linux/signal.h>.
> 
> It would be enough to keep compatibility with glibc.
> But personally I'd like to make it work generally.
> 
> > So there would only be a
> > problem if the user is explicitly not including <signal.h> to then use
> > <linux/signal.h>. I doubt that makes sense.
> 
> Technically nolibc is always included as a whole, so any application
> using it would be prevented from using linux/signal.h.
> 
> Maybe Willy has some strong opinions.
> Otherwise I'm also fine if we keep this part as is for now.
> Then if everything else is addressed I'll try to actually implement my
> proposal on top.
> 
> Does this sound reasonable?

I think that userland application writers are used to seeing random
failures here and there when they start using linux/ without being invited
to do so. Of course it's never fun to deal with build failures, but the
effort spent overengineering solutions is often not worth being spent
when you figure that sometimes the next application you try will blatantly
fail again. I'd suggest that we stay on the pragmatic side of things: if
we find a solution that works fine for the common case, that may break for
rare cases that already break with glibc, I think we're already fine. That
doesn't mean we don't need to think about it, but it's not something urgent
nor that warrants taking more risks either.

I personally also think that we'll face an increasing amount of build
issue reports due to our attempt at being more compatible. Indeed, in the
past, one had to #ifdef all includes and pass -include nolibc on the gcc
command line. Nowadays by supporting a larger number of functions and
include file names, we encourage developers to try to simply switch their
libc for nolibc. We do know that the coverage remains low, and reports
will come proportionally to the ratio of apparent ease of use to feature
coverage. That's expected and we should try to resist feeling bad about
it. I don't think we'll ever aim at competing with glibc/musl for example,
so a little bit of application adaptation is expected from time to time.

Cheers,
Willy

Re: [RFC v2] tools/nolibc: add sigaction()

[Thomas Weißschuh]

On 2025-07-04 15:33:09+0200, Willy Tarreau wrote:
> On Fri, Jul 04, 2025 at 12:50:32PM +0200, Thomas Weißschuh wrote:
> > On 2025-07-02 17:33:24+0200, Benjamin Berg wrote:
> > > On Wed, 2025-07-02 at 00:04 +0200, Thomas Weißschuh wrote:
> > > > [SNIP]
> > > > > --- a/tools/include/nolibc/arch-i386.h
> > > > > +++ b/tools/include/nolibc/arch-i386.h
> > > > > @@ -10,6 +10,19 @@
> > > > >  #include "compiler.h"
> > > > >  #include "crt.h"
> > > > >  
> > > > > +/* Needed to get the correct struct sigaction definition */
> > > > > +#define SA_RESTORER	0x04000000
> > > > > +
> > > > > +/* Restorer must be set on i386 */
> > > > > +#define _NOLIBC_ARCH_NEEDS_SA_RESTORER
> > > > > +
> > > > > +/* Otherwise we would need to use sigreturn instead of rt_sigreturn */
> > > > > +#define _NOLIBC_ARCH_FORCE_SIG_FLAGS SA_SIGINFO
> > > > > +
> > > > > +/* Avoid linux/signal.h, it has an incorrect _NSIG and sigset_t */
> > > > > +#include <asm-generic/signal.h>
> > > > > +#include <asm-generic/siginfo.h>
> > > > 
> > > > This doesn't work if the user already has <linux/signal.h> included for
> > > > some other reason. The symbol names will conflict.
> > > 
> > > I was thinking this is fine. Such a conflict already exists between the
> > > normal glibc <signal.h> and <linux/signal.h>.
> > 
> > It would be enough to keep compatibility with glibc.
> > But personally I'd like to make it work generally.
> > 
> > > So there would only be a
> > > problem if the user is explicitly not including <signal.h> to then use
> > > <linux/signal.h>. I doubt that makes sense.
> > 
> > Technically nolibc is always included as a whole, so any application
> > using it would be prevented from using linux/signal.h.
> > 
> > Maybe Willy has some strong opinions.
> > Otherwise I'm also fine if we keep this part as is for now.
> > Then if everything else is addressed I'll try to actually implement my
> > proposal on top.
> > 
> > Does this sound reasonable?
> 
> I think that userland application writers are used to seeing random
> failures here and there when they start using linux/ without being invited
> to do so. Of course it's never fun to deal with build failures, but the
> effort spent overengineering solutions is often not worth being spent
> when you figure that sometimes the next application you try will blatantly
> fail again. I'd suggest that we stay on the pragmatic side of things: if
> we find a solution that works fine for the common case, that may break for
> rare cases that already break with glibc, I think we're already fine. That
> doesn't mean we don't need to think about it, but it's not something urgent
> nor that warrants taking more risks either.

I'm all for keeping it simple. However here I think we disagree about
the aproach which would be simpler. As mentioned before let's go with
Benjamin's and I'll try to do my thing on top.
Then we can actually compare two working aproaches (if my idea does work).

<snip>