Re: [PATCH v3 0/5] nolibc signal handling support

[Willy Tarreau]

On Mon, Jan 09, 2023 at 01:31:17AM +0700, Ammar Faizi wrote:
> >   - riscv and mips build are now broken:
> >       sysroot/riscv/include/sys.h:1110:18: error: 'struct sigaction' has no member named 'sa_restorer'
> >        1110 |         if (!act2.sa_restorer) {
> >             |                  ^
> >       sysroot/riscv/include/sys.h:1111:34: error: 'SA_RESTORER' undeclared (first use in this function); did you mean 'SA_RESTART'?
> >        1111 |                 act2.sa_flags |= SA_RESTORER;
> >             |                                  ^~~~~~~~~~~
> >             |                                  SA_RESTART
> 
> Just a speculation:
> This is probably because not all architectures have a SA_RESTORER. I'll
> need to figure out how Linux handles signal on those architectures.

Yes that's the case, there's even some ifdef around it in the generic
code. I have no idea how it works there, at least it seems worth having
a look to make sure we don't miss something easy.

> >   - s390 segfaults:
> >       58 select_fault = -1 EFAULT              [OK]
> >       59 sigactionqemu: uncaught target signal 11 (Segmentation fault) - core dumped
> >       Segmentation fault
> > 
> >     It dies in __restore_rt at 1006ba4 while performing the syscall,
> >     I don't know why, maybe this arch requires an alt stack or whatever :
> > 
> >       0000000001006ba0 <__restore_rt>:
> >        1006ba0:       a7 19 00 ad             lghi    %r1,173
> >        1006ba4:       0a 00                   svc     0
> >        1006ba6:       07 07                   nopr    %r7
> 
> Bah, no clue on this. I'll CC s390 people in the next version and ask
> them to shed some light.

OK.

> 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 also think that the printf() in test_sigaction_sig() are not welcome
> > as they corrupt the output. Maybe one thing you could do to preserve the
> > info would be to prepend a space in front of the message and remove the
> > LF. For example the simple patch below:
> [...]
> > Which is way more readable and still grep-friendly.
> 
> Yeah, that looks much better. Applied to my local git tree with
> attribution.

Don't worry with attribution for such patches from me. I'd rather see
the first patch looking good at once than having an extra one change
the output just for the sake of attribution! It was just a suggestion
anyway and whatever solution you find that improves the output will be
fine.

Thank you!
Willy