Re: [PATCH] Possible patch for __syscall_cp

[Rich Felker <dalias@libc.org>]

On Wed, Feb 14, 2018 at 12:09:18PM +0000, Nicholas Wilson wrote:
> On 13 February 2018 14:49, Szabolcs Nagy wrote:
> > i think your patch is ok (__syscall6 should behave the same
> > way as __syscall other than the inlining), but you can fix
> > it for your target only by adding
> >
> > static inline long __syscall(long n, long a, long b, long c, long d, long e, long f)
> > {
> >        return __syscall6(n,a,b,c,d,e,f);
> > }
> >
> > to syscall_arch.h
> 
> Yes, that's the approach I was thinking of when I mentioned we could
> just implement __syscall. In fact, _syscall is declared as varargs,
> so it would have to be:
> 
> static inline long __syscall(long n, ...)
> {
>   va_list va;
>   va_start(va, n);
>   long a = va_arg(va, long);
>   ... etc
>   long f = va_arg(va, long);
>   return __syscall6(n, a, b, ..., f);
> }
> 
> The question really is - would you prefer archs to define __syscall
> that way, or would you rather patch __syscall_cp to allow macro
> expansion? In your opinion, which is cleaner? If you don't want to
> see a shim like that in the archs, would you consider applying the
> __syscall_cp patch for us?

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.

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.

This is all much messier than it should be...

Rich