Message ID | 20230108135904.851762-1-ammar.faizi@intel.com |
---|---|
Headers | show |
Series | nolibc signal handling support | expand |
On Sun, Jan 08, 2023 at 07:49:30PM +0100, Willy Tarreau wrote: > On Mon, Jan 09, 2023 at 01:31:17AM +0700, Ammar Faizi wrote: > > I'll be pondering this code this week (to follow what actually the > > rt_sigaction wants on i386 and arm): > > > > https://github.com/torvalds/linux/blob/v6.2-rc3/kernel/signal.c#L4404-L4434 > > Seems like it could simply be a matter of sigsetsize, which is the > first one returning -EINVAL. > > > Hopefully, I can get it sorted before the weekend. > > OK! I couldn't dedicate much time to this, but I looked into it, and here's my report on the progress. I didn't manage to find a proper solution to this. But yes, you're right. It's a matter of 'sizeof(sigset_t)'. So here is my observation. Currently, nolibc's sys.h includes this: #include <asm/signal.h> The definition of 'sigset_t' in that header is: typedef unsigned long sigset_t; On i386, 'sizeof(unsigned long)' is 4, but on x86-64 it's 8. That is not the 'sigset_t' that the kernel wants. The kernel wants the 'sigset_t' that is in <asm-generic/signal.h>: #define _NSIG 64 #define _NSIG_BPW __BITS_PER_LONG // this 64 on x86-64, but 32 on i386. #define _NSIG_WORDS (_NSIG / _NSIG_BPW) typedef struct { unsigned long sig[_NSIG_WORDS]; } sigset_t; The above struct is always 8 bytes in size. In other words: _NSIG_WORDS == 2 on i386 _NSIG_WORDS == 1 on x86-64 sizeof(unsigned long) == 4 on i386 sizeof(unsigned long) == 8 on x86-64 Therefore, sizeof(unsigned long [_NSIG_WORDS]) is always 8 on both architectures. That's the correct size. I tried to #include <asm-generic/signal.h> but it conflicts with the other 'sigset_t' definition. So I can't do that. Why are there two different definitions of 'sigset_t'? I don't know. I probably should read the story behind this syscall to get it implemented right. Let me ponder this again on Monday. But at least I tell what I have found so people can give some comments on it...
On Sun, Jan 15, 2023 at 11:01 PM Ammar Faizi wrote: > That is not the 'sigset_t' that the kernel wants. The kernel wants the > 'sigset_t' that is in <asm-generic/signal.h>: > > #define _NSIG 64 > #define _NSIG_BPW __BITS_PER_LONG // this 64 on x86-64, but 32 on i386. > #define _NSIG_WORDS (_NSIG / _NSIG_BPW) > > typedef struct { > unsigned long sig[_NSIG_WORDS]; > } sigset_t; > > The above struct is always 8 bytes in size. In other words: > > _NSIG_WORDS == 2 on i386 > _NSIG_WORDS == 1 on x86-64 > sizeof(unsigned long) == 4 on i386 > sizeof(unsigned long) == 8 on x86-64 > > Therefore, sizeof(unsigned long [_NSIG_WORDS]) is always 8 on both > architectures. That's the correct size. > > I tried to #include <asm-generic/signal.h> but it conflicts with the > other 'sigset_t' definition. So I can't do that. Read the glibc sigaction implementation, it has different struct sigaction definitions for user and kernel too. https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/libc_sigaction.c;h=3cbf241a5fa28296c910fa40a41b09d2b6113b05;hb=7e31d166510ac4adbf53d5e8144c709a37dd8c7a Since nolibc compiles everything in a single translation unit, you can't have a different sigset_t definition. You need to copy the definition to nolibc header and change the type name to something else for internal use only. > Why are there two different definitions of 'sigset_t'? I don't know. > > I probably should read the story behind this syscall to get it > implemented right. Let me ponder this again on Monday. But at least I > tell what I have found so people can give some comments on it... `man 2 rt_sigaction` tells the story. Here is the bedtime story, have a nice dream :-) The original Linux system call was named sigaction(). However, with the addition of real-time signals in Linux 2.2, the fixed-size, 32-bit sigset_t type supported by that system call was no longer fit for purpose. Consequently, a new system call, rt_sigaction(), was added to support an enlarged sigset_t type. The new system call takes a fourth argument, size_t sigsetsize, which specifies the size in bytes of the signal sets in act.sa_mask and oldact.sa_mask. This argument is currently required to have the value sizeof(sigset_t) (or the error EINVAL results). The glibc sigaction() wrapper function hides these details from us, transparā ently calling rt_sigaction() when the kernel provides it.