mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Nicholas Wilson <nicholas.wilson@realvnc.com>
To: "musl@lists.openwall.com" <musl@lists.openwall.com>
Subject: Re: [PATCH] Possible patch for __syscall_cp
Date: Wed, 21 Feb 2018 10:47:47 +0000	[thread overview]
Message-ID: <DB6PR0502MB3016C1EF73B61F4DB3B9A21FE7CE0@DB6PR0502MB3016.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20180221020357.GA1436@brightrain.aerifal.cx>

On 21 February 2018 02:03, Rich Felker wrote:
> The macro suppression is intentional so that the stub version of
> __syscall_cp (only used in static linking without thread cancellation
> linked) is just a tail call to __syscall rather than a bunch of
> register shuffling. Being that the whole point of the stub version is
> to avoid the size cost of cancellation logic in small static linked
> programs, it doesn't make sense to make a change here.

OK, that makes sense. Thanks!

> musl ports should define an external (but hidden) function __syscall
> replacing (the empty) src/internal/syscall.c. Perhaps that file should
> actually be non-empty and look like the above, which would work
> (albeit suboptimally) for any arch that doesn't make an external call
> for __syscall6.
>
> Unfortunately there are some messy questions about how many arguments
> __syscall is called with and whether it's actually valid for it to be
> defined with a C function using va_arg rather than in asm. These are
> independent of whether we apply your patch or not, I think, and they
> should probably be fixed by ensuring that any place where the external
> __syscall is called actually passes 6 arguments, by adding dummy zero
> arguments to the __syscallN macros in the SYSCALL_NO_INLINE case, and
> in the syscall_arch.h files that use it.

I think that makes sense to me too. I've added the required stub to the Wasm port in this commit:
https://github.com/NWilson/musl/commit/7cd91fc86358076e601783144f242824afd984ec

I guess it makes sense for Wasm to provide this stub itself, since other arches won't be using it, and it's not likely to be useful to many other arches either.

Regarding passing the six args to __syscall, I think that does work already; __syscall_cp is the only place where __syscall is used with macro suppression, and it does use six args. All the other places go via the macros which are able pad the arguments to the correct length.

I'm happy with things staying as they are then in upstream Musl.

Thanks for the help,
Nick


      reply	other threads:[~2018-02-21 10:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-13 14:28 Nicholas Wilson
2018-02-13 14:49 ` Szabolcs Nagy
2018-02-14 12:09   ` Nicholas Wilson
2018-02-21  2:03     ` Rich Felker
2018-02-21 10:47       ` Nicholas Wilson [this message]

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 \
    --in-reply-to=DB6PR0502MB3016C1EF73B61F4DB3B9A21FE7CE0@DB6PR0502MB3016.eurprd05.prod.outlook.com \
    --to=nicholas.wilson@realvnc.com \
    --cc=musl@lists.openwall.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

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).