public inbox for [email protected]
 help / color / mirror / Atom feed
From: Willy Tarreau <[email protected]>
To: Ammar Faizi <[email protected]>
Cc: Shuah Khan <[email protected]>,
	"Paul E. McKenney" <[email protected]>,
	Gilang Fachrezy <[email protected]>,
	VNLX Kernel Department <[email protected]>,
	Alviro Iskandar Setiawan <[email protected]>,
	Kanna Scarlet <[email protected]>,
	Muhammad Rizki <[email protected]>,
	GNU/Weeb Mailing List <[email protected]>,
	Linux Kernel Mailing List <[email protected]>,
	Linux Kselftest Mailing List <[email protected]>
Subject: Re: [RFC PATCH v1 0/8] nolibc signal handling support
Date: Wed, 28 Dec 2022 14:35:14 +0100	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On Wed, Dec 28, 2022 at 07:01:36PM +0700, Ammar Faizi wrote:
> On 12/28/22 1:49 AM, Willy Tarreau wrote:
> > I'll try to do it but do not want to make you wait too long in case it
> > gets delayed. In the worst case we should only postpone the getauxval()
> > patch and not the other ones.
> 
> I will split it into 2 patchset then.

OK thanks!

I've pushed for you an update which starts to do what I proposed. Errno
and environ are now marked weak for all archs, and _auxv is set for i386,
x86_64, arm64 and arm for now:

   https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git/log/?h=20221227-nolibc-weak-2

You can already use it to implement getauxval(), it will normally work
for these archs.

> > BTW, do you think your arch-specific changes for sigaction() will be
> > easily portable to other architectures ? I feel a bit wary of starting
> > to have different features per architecture given the purpose of the
> > lib, so the more uniform the coverage the better.
> 
> The 'rt_sigaction()' itself doesn't seem to be an arch specific, but
> the way it resumes the execution needs to call 'rt_sigreturn()' which
> is arch specific. I took a look at the kernel source code, most
> architectures read 'struct rt_sigframe' from the stack pointer.
> 
> https://github.com/torvalds/linux/blob/631aa744423173bf921191ba695bbc7c1aabd9e0/arch/x86/kernel/signal_32.c#L145
> https://github.com/torvalds/linux/blob/631aa744423173bf921191ba695bbc7c1aabd9e0/arch/x86/kernel/signal_64.c#L243-L271
> https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/arm64/kernel/signal.c#L668-L699
> https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/arm64/kernel/signal32.c#L259
> https://github.com/torvalds/linux/blob/eb67d239f3aa1711afb0a42eab50459d9f3d672e/arch/riscv/kernel/signal.c#L101
> 
> On the x86-64 arch, the implementation is just like this:
> 
>    __arch_restore_rt:
>        #
>        # ((%rsp - sizeof(long)) must point to 'struct rt_sigframe')
>        #
>        # 'struct rt_sigframe' is automatically constructed by
>        # the kernel when a signal is caught.
>        #
>        movl       $0xf, %eax // __NR_rt_sigreturn == 0xf
>        syscall

I think we could avoid the asm specific stuff is we get rid of the frame
pointer. Please look below:

  __attribute__((weak,unused,noreturn,optimize("omit-frame-pointer"),section(".text.nolibc_rt_sigreturn")))
  void sys_rt_sigreturn()
  {
        my_syscall0(__NR_rt_sigreturn);
        __builtin_unreachable();
  }

It gives me the correct code for x86_64 and i586. I don't know if other
architectures will want to add a prologue. I tried with "naked" but it's
ignored by the compiler since the function is not purely asm. Not very
important but given that we already have everything to perform our calls
it would make sense to stay on this. By the way, for the sake of
consistency with other syscalls, I do think the function (or label if
we can't do otherwise) should be called "sys_rt_sigreturn" as it just
performs a syscall.

> I believe aarch64 and RISCV don't behave differently, but different
> registers.
> 
> Not sure what PowerPC does here, it seems a bit different:
> https://github.com/torvalds/linux/blob/1612c382ffbdf1f673caec76502b1c00e6d35363/arch/powerpc/kernel/signal_64.c#L744

It looks similar to me, it's just that the kernel side differs but I
think it's the same.

> I haven't taken a look at other archs.
> 
> What do you think? Is it affordable for nolibc to implement all of
> these?

Yes I think so. I suspect that we might need to have a few arch-specific
implementations, but we've already had this case a few times and we could
easily use a pair of #define/#ifdef to skip the generic version.

Best regards,
Willy

  reply	other threads:[~2022-12-28 13:35 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-22  3:51 [RFC PATCH v1 0/8] nolibc signal handling support Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 1/8] nolibc/sys: Implement `sigaction(2)` function Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 2/8] nolibc/sys: Implement `signal(2)` function Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 3/8] nolibc/sys: Implement `getpagesize(2)` function Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 4/8] selftests/nolibc: Add `-Wall` and `-Wno-unsed-function` to the CFLAGS Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 5/8] selftests/nolibc: Add `fork(2)` selftest Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 6/8] selftests/nolibc: Add `sigaction(2)` selftest Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 7/8] selftests/nolibc: Add `signal(2)` selftest Ammar Faizi
2022-12-22  3:51 ` [RFC PATCH v1 8/8] selftests/nolibc: Add `getpagesize(2)` selftest Ammar Faizi
2022-12-22  4:34 ` [RFC PATCH v1 0/8] nolibc signal handling support Willy Tarreau
2022-12-22 13:46   ` Ammar Faizi
2022-12-22 14:55     ` Alviro Iskandar Setiawan
2022-12-27  6:26     ` Willy Tarreau
2022-12-27 13:32       ` Ammar Faizi
2022-12-27 13:36         ` Ammar Faizi
2022-12-27 18:58           ` Willy Tarreau
2022-12-28 12:23             ` Ammar Faizi
2022-12-27 18:49         ` Willy Tarreau
2022-12-28 12:01           ` Ammar Faizi
2022-12-28 13:35             ` Willy Tarreau [this message]
2022-12-29 11:41               ` Ammar Faizi
2023-01-03  3:51                 ` Alviro Iskandar Setiawan
2023-01-03  3:54                   ` Willy Tarreau
2023-01-03  3:59                     ` Ammar Faizi
2023-01-08 13:08                       ` [PATCH v1 0/3] nolibc auxiliary vector retrieval support Ammar Faizi
2023-01-08 13:08                         ` [PATCH v1 1/3] nolibc/stdlib: Implement `getauxval(3)` function Ammar Faizi
2023-01-08 13:08                         ` [PATCH v1 2/3] nolibc/sys: Implement `getpagesize(2)` function Ammar Faizi
2023-01-08 13:08                         ` [PATCH v1 3/3] selftests/nolibc: Add `getpagesize(2)` selftest Ammar Faizi
2023-01-08 13:10                       ` [PATCH v2 0/4] nolibc signal handling support Ammar Faizi
2023-01-08 13:10                         ` [PATCH v2 1/4] nolibc/sys: Implement `sigaction(2)` function Ammar Faizi
2023-01-08 13:10                         ` [PATCH v2 2/4] nolibc/sys: Implement `signal(2)` function Ammar Faizi
2023-01-08 13:10                         ` [PATCH v2 3/4] selftests/nolibc: Add `fork(2)` selftest Ammar Faizi
2023-01-08 13:10                         ` [PATCH v2 4/4] selftests/nolibc: Add `sigaction(2)` selftest Ammar Faizi
2023-01-08 13:28                         ` [PATCH v2 0/4] nolibc signal handling support Alviro Iskandar Setiawan
2023-01-08 13:31                           ` Ammar Faizi
2023-01-08 13:39                             ` Ammar Faizi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox