Linux: Support non-variadic calls to prctl (bug 29770)

Message ID 878rkiu72f.fsf@oldenburg.str.redhat.com
State New
Headers
Series Linux: Support non-variadic calls to prctl (bug 29770) |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686
redhat-pt-bot/TryBot-still_applies warning Patch no longer applies to master

Commit Message

Florian Weimer Nov. 10, 2022, 4:07 p.m. UTC
  Commit ff026950e280bc3e9487b41b460fb31bc5b57721 ("Add a C wrapper for
prctl [BZ #25896]") replaced the assembler wrapper with a C function.
However, the C variadic function implementation has a different ABI
on powerpc64le-linux-gnu, and now requires calls with a variadic
prototype.  Therefore, switch to a non-variadic implementation.

The internal __prctl is now non-variadic as well.  Switch uses in
__pthread_getname_np and __pthread_setname_np to direct system calls
to avoid casts and the extra arguments.  Other callers already supply
the extra arguments.  For the MIPS usage in elf_machine_reject_phdr_p,
supply the extra arguments to __prctl.

Visual inspection confirms that the x86-64 x32 version still performs
zero extension on the arguments.

Tested on aarch64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
x86_64-linux-gnu.  Built with build-many-glibcs.py.

---
 include/sys/prctl.h                   |  3 ++-
 nptl/pthread_getname.c                |  2 +-
 nptl/pthread_setname.c                |  2 +-
 sysdeps/mips/dl-machine-reject-phdr.h |  8 ++++----
 sysdeps/unix/sysv/linux/prctl.c       | 18 ++++++++++--------
 5 files changed, 18 insertions(+), 15 deletions(-)


base-commit: 22a46dee24351fd5f4f188ad80554cad79c82524
  

Comments

H.J. Lu Nov. 10, 2022, 6:43 p.m. UTC | #1
On Thu, Nov 10, 2022 at 8:07 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Commit ff026950e280bc3e9487b41b460fb31bc5b57721 ("Add a C wrapper for
> prctl [BZ #25896]") replaced the assembler wrapper with a C function.
> However, the C variadic function implementation has a different ABI
> on powerpc64le-linux-gnu, and now requires calls with a variadic
> prototype.  Therefore, switch to a non-variadic implementation.
>
> The internal __prctl is now non-variadic as well.  Switch uses in
> __pthread_getname_np and __pthread_setname_np to direct system calls
> to avoid casts and the extra arguments.  Other callers already supply
> the extra arguments.  For the MIPS usage in elf_machine_reject_phdr_p,
> supply the extra arguments to __prctl.
>
> Visual inspection confirms that the x86-64 x32 version still performs
> zero extension on the arguments.
>
> Tested on aarch64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
> x86_64-linux-gnu.  Built with build-many-glibcs.py.
>
> ---
>  include/sys/prctl.h                   |  3 ++-
>  nptl/pthread_getname.c                |  2 +-
>  nptl/pthread_setname.c                |  2 +-
>  sysdeps/mips/dl-machine-reject-phdr.h |  8 ++++----
>  sysdeps/unix/sysv/linux/prctl.c       | 18 ++++++++++--------
>  5 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> index d33f3a290e..3b9a949a03 100644
> --- a/include/sys/prctl.h
> +++ b/include/sys/prctl.h
> @@ -3,7 +3,8 @@
>
>  # ifndef _ISOMAC
>
> -extern int __prctl (int __option, ...);
> +extern int __prctl (int __option, unsigned long int, unsigned long int,
> +                    unsigned long int, unsigned long int);
>  libc_hidden_proto (__prctl)
>
>  # endif /* !_ISOMAC */
> diff --git a/nptl/pthread_getname.c b/nptl/pthread_getname.c
> index ebec06e23f..8d485d874b 100644
> --- a/nptl/pthread_getname.c
> +++ b/nptl/pthread_getname.c
> @@ -38,7 +38,7 @@ __pthread_getname_np (pthread_t th, char *buf, size_t len)
>      return ERANGE;
>
>    if (pd == THREAD_SELF)
> -    return __prctl (PR_GET_NAME, buf) ? errno : 0;
> +    return -INTERNAL_SYSCALL_CALL (prctl, PR_GET_NAME, buf);
>
>  #define FMT "/proc/self/task/%u/comm"
>    char fname[sizeof (FMT) + 8];
> diff --git a/nptl/pthread_setname.c b/nptl/pthread_setname.c
> index f24560db47..1da548dbbf 100644
> --- a/nptl/pthread_setname.c
> +++ b/nptl/pthread_setname.c
> @@ -40,7 +40,7 @@ __pthread_setname_np (pthread_t th, const char *name)
>      return ERANGE;
>
>    if (pd == THREAD_SELF)
> -    return __prctl (PR_SET_NAME, name) ? errno : 0;
> +    return -INTERNAL_SYSCALL_CALL (prctl, PR_SET_NAME, name);
>
>  #define FMT "/proc/self/task/%u/comm"
>    char fname[sizeof (FMT) + 8];
> diff --git a/sysdeps/mips/dl-machine-reject-phdr.h b/sysdeps/mips/dl-machine-reject-phdr.h
> index 45b6bcaeac..7efd5fc92b 100644
> --- a/sysdeps/mips/dl-machine-reject-phdr.h
> +++ b/sysdeps/mips/dl-machine-reject-phdr.h
> @@ -169,7 +169,7 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
>    bool cannot_mode_switch = false;
>
>    /* Get the current hardware mode.  */
> -  cur_mode = __prctl (PR_GET_FP_MODE);
> +  cur_mode = __prctl (PR_GET_FP_MODE, 0, 0, 0, 0);
>  # endif
>  #endif
>
> @@ -305,15 +305,15 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
>         /* Set the new mode.  Use fr1_mode if the requirements cannot be met by
>            FR0.  */
>         if (!in_req.fr0)
> -         return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
> -       else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0) != 0)
> +         return __prctl (PR_SET_FP_MODE, fr1_mode, 0, 0, 0) != 0;
> +       else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0, 0, 0, 0) != 0)
>           {
>             /* Setting FR0 can validly fail on an R6 core so retry with the FR1
>                mode as a fall back.  */
>             if (errno != ENOTSUP)
>               return true;
>
> -           return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
> +           return __prctl (PR_SET_FP_MODE, fr1_mode, 0, 0, 0) != 0;
>           }
>        }
>  # endif /* HAVE_PRCTL_FP_MODE */
> diff --git a/sysdeps/unix/sysv/linux/prctl.c b/sysdeps/unix/sysv/linux/prctl.c
> index 25fd968ae2..102d4b0a21 100644
> --- a/sysdeps/unix/sysv/linux/prctl.c
> +++ b/sysdeps/unix/sysv/linux/prctl.c
> @@ -18,7 +18,15 @@
>
>  #include <sysdep.h>
>  #include <stdarg.h>
> +
> +/* Hide the type information from GCC.  The prototype in the installed
> +   header is a variadic function.  The non-variadic implementation
> +   below is both more efficient and more compatible because some ABIs
> +   assume additional work performed by the caller for variadic
> +   functions (notably powerpc64le).  */
> +#define prctl prctl_XXX
>  #include <sys/prctl.h>
> +#undef prctl
>
>  /* Unconditionally read all potential arguments.  This may pass
>     garbage values to the kernel, but avoids the need for teaching
> @@ -26,15 +34,9 @@
>     that are added to the kernel in the future).  */
>
>  int
> -__prctl (int option, ...)
> +__prctl (int option, unsigned long int arg2, unsigned long int arg3,
> +         unsigned long int arg4, unsigned long int arg5)
>  {
> -  va_list arg;
> -  va_start (arg, option);
> -  unsigned long int arg2 = va_arg (arg, unsigned long int);
> -  unsigned long int arg3 = va_arg (arg, unsigned long int);
> -  unsigned long int arg4 = va_arg (arg, unsigned long int);
> -  unsigned long int arg5 = va_arg (arg, unsigned long int);
> -  va_end (arg);
>    return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
>  }
>
>
> base-commit: 22a46dee24351fd5f4f188ad80554cad79c82524
>

prctl is an exported variadic function.  Do we need to keep it?
  
Florian Weimer Nov. 10, 2022, 7:18 p.m. UTC | #2
* H. J. Lu:

> prctl is an exported variadic function.  Do we need to keep it?

Do you mean whether we should keep it as a variadic function or turn it
into a function with a fixed argument list?

Unfortunately the types and number of arguments vary between the
sub-operations.  Pretty much any application calling prctl would have to
be changed.  It's just syscall by another name (and with a fixed number
of course).

Thanks,
Florian
  
H.J. Lu Nov. 10, 2022, 7:55 p.m. UTC | #3
On Thu, Nov 10, 2022 at 11:18 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > prctl is an exported variadic function.  Do we need to keep it?
>
> Do you mean whether we should keep it as a variadic function or turn it
> into a function with a fixed argument list?

Yes.

> Unfortunately the types and number of arguments vary between the
> sub-operations.  Pretty much any application calling prctl would have to
> be changed.  It's just syscall by another name (and with a fixed number
> of course).
>

<sys/prctl.h> has

extern int prctl (int __option, ...) __THROW;

Your change works for

int prctl(int option, unsigned long arg2, unsigned long arg3,
                 unsigned long arg4, unsigned long arg5);

prtcl (...);

But it breaks

#include <sys/prctl.h>

prctl (...);

H.J.
  
Florian Weimer Nov. 10, 2022, 8:08 p.m. UTC | #4
* H. J. Lu:

> On Thu, Nov 10, 2022 at 11:18 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > prctl is an exported variadic function.  Do we need to keep it?
>>
>> Do you mean whether we should keep it as a variadic function or turn it
>> into a function with a fixed argument list?
>
> Yes.
>
>> Unfortunately the types and number of arguments vary between the
>> sub-operations.  Pretty much any application calling prctl would have to
>> be changed.  It's just syscall by another name (and with a fixed number
>> of course).
>>
>
> <sys/prctl.h> has
>
> extern int prctl (int __option, ...) __THROW;
>
> Your change works for
>
> int prctl(int option, unsigned long arg2, unsigned long arg3,
>                  unsigned long arg4, unsigned long arg5);
>
> prtcl (...);
>
> But it breaks
>
> #include <sys/prctl.h>
>
> prctl (...);

I don't think that's true for our current ABIs.  They support
unprototyped calls, and by extension this also means that calls through
a variadic prototype to a prototyped non-variadic function must work as
well.  There are other ABIs which do not work this way, but glibc hasn't
been ported to them (and it's unclear how useful they would be for
GNU/Linux).

The new C implementation is equivalent to the previous assembler
implementation.

Thanks,
Florian
  
H.J. Lu Nov. 10, 2022, 8:34 p.m. UTC | #5
On Thu, Nov 10, 2022 at 8:07 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Commit ff026950e280bc3e9487b41b460fb31bc5b57721 ("Add a C wrapper for
> prctl [BZ #25896]") replaced the assembler wrapper with a C function.
> However, the C variadic function implementation has a different ABI
> on powerpc64le-linux-gnu, and now requires calls with a variadic
> prototype.  Therefore, switch to a non-variadic implementation.
>
> The internal __prctl is now non-variadic as well.  Switch uses in
> __pthread_getname_np and __pthread_setname_np to direct system calls
> to avoid casts and the extra arguments.  Other callers already supply
> the extra arguments.  For the MIPS usage in elf_machine_reject_phdr_p,
> supply the extra arguments to __prctl.
>
> Visual inspection confirms that the x86-64 x32 version still performs
> zero extension on the arguments.
>
> Tested on aarch64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
> x86_64-linux-gnu.  Built with build-many-glibcs.py.
>
> ---
>  include/sys/prctl.h                   |  3 ++-
>  nptl/pthread_getname.c                |  2 +-
>  nptl/pthread_setname.c                |  2 +-
>  sysdeps/mips/dl-machine-reject-phdr.h |  8 ++++----
>  sysdeps/unix/sysv/linux/prctl.c       | 18 ++++++++++--------
>  5 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> index d33f3a290e..3b9a949a03 100644
> --- a/include/sys/prctl.h
> +++ b/include/sys/prctl.h
> @@ -3,7 +3,8 @@
>
>  # ifndef _ISOMAC
>
> -extern int __prctl (int __option, ...);
> +extern int __prctl (int __option, unsigned long int, unsigned long int,
> +                    unsigned long int, unsigned long int);
>  libc_hidden_proto (__prctl)
>
>  # endif /* !_ISOMAC */
> diff --git a/nptl/pthread_getname.c b/nptl/pthread_getname.c
> index ebec06e23f..8d485d874b 100644
> --- a/nptl/pthread_getname.c
> +++ b/nptl/pthread_getname.c
> @@ -38,7 +38,7 @@ __pthread_getname_np (pthread_t th, char *buf, size_t len)
>      return ERANGE;
>
>    if (pd == THREAD_SELF)
> -    return __prctl (PR_GET_NAME, buf) ? errno : 0;
> +    return -INTERNAL_SYSCALL_CALL (prctl, PR_GET_NAME, buf);
>
>  #define FMT "/proc/self/task/%u/comm"
>    char fname[sizeof (FMT) + 8];
> diff --git a/nptl/pthread_setname.c b/nptl/pthread_setname.c
> index f24560db47..1da548dbbf 100644
> --- a/nptl/pthread_setname.c
> +++ b/nptl/pthread_setname.c
> @@ -40,7 +40,7 @@ __pthread_setname_np (pthread_t th, const char *name)
>      return ERANGE;
>
>    if (pd == THREAD_SELF)
> -    return __prctl (PR_SET_NAME, name) ? errno : 0;
> +    return -INTERNAL_SYSCALL_CALL (prctl, PR_SET_NAME, name);
>
>  #define FMT "/proc/self/task/%u/comm"
>    char fname[sizeof (FMT) + 8];
> diff --git a/sysdeps/mips/dl-machine-reject-phdr.h b/sysdeps/mips/dl-machine-reject-phdr.h
> index 45b6bcaeac..7efd5fc92b 100644
> --- a/sysdeps/mips/dl-machine-reject-phdr.h
> +++ b/sysdeps/mips/dl-machine-reject-phdr.h
> @@ -169,7 +169,7 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
>    bool cannot_mode_switch = false;
>
>    /* Get the current hardware mode.  */
> -  cur_mode = __prctl (PR_GET_FP_MODE);
> +  cur_mode = __prctl (PR_GET_FP_MODE, 0, 0, 0, 0);

Can it use INLINE_SYSCALL_CALL here?

>  # endif
>  #endif
>
> @@ -305,15 +305,15 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
>         /* Set the new mode.  Use fr1_mode if the requirements cannot be met by
>            FR0.  */
>         if (!in_req.fr0)
> -         return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
> -       else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0) != 0)
> +         return __prctl (PR_SET_FP_MODE, fr1_mode, 0, 0, 0) != 0;
> +       else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0, 0, 0, 0) != 0)
>           {
>             /* Setting FR0 can validly fail on an R6 core so retry with the FR1
>                mode as a fall back.  */
>             if (errno != ENOTSUP)
>               return true;
>
> -           return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
> +           return __prctl (PR_SET_FP_MODE, fr1_mode, 0, 0, 0) != 0;
>           }
>        }
>  # endif /* HAVE_PRCTL_FP_MODE */
> diff --git a/sysdeps/unix/sysv/linux/prctl.c b/sysdeps/unix/sysv/linux/prctl.c
> index 25fd968ae2..102d4b0a21 100644
> --- a/sysdeps/unix/sysv/linux/prctl.c
> +++ b/sysdeps/unix/sysv/linux/prctl.c
> @@ -18,7 +18,15 @@
>
>  #include <sysdep.h>
>  #include <stdarg.h>
> +
> +/* Hide the type information from GCC.  The prototype in the installed
> +   header is a variadic function.  The non-variadic implementation
> +   below is both more efficient and more compatible because some ABIs
> +   assume additional work performed by the caller for variadic
> +   functions (notably powerpc64le).  */
> +#define prctl prctl_XXX
>  #include <sys/prctl.h>
> +#undef prctl
>
>  /* Unconditionally read all potential arguments.  This may pass
>     garbage values to the kernel, but avoids the need for teaching
> @@ -26,15 +34,9 @@
>     that are added to the kernel in the future).  */
>
>  int
> -__prctl (int option, ...)
> +__prctl (int option, unsigned long int arg2, unsigned long int arg3,
> +         unsigned long int arg4, unsigned long int arg5)
>  {
> -  va_list arg;
> -  va_start (arg, option);
> -  unsigned long int arg2 = va_arg (arg, unsigned long int);
> -  unsigned long int arg3 = va_arg (arg, unsigned long int);
> -  unsigned long int arg4 = va_arg (arg, unsigned long int);
> -  unsigned long int arg5 = va_arg (arg, unsigned long int);
> -  va_end (arg);
>    return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
>  }
>
>
> base-commit: 22a46dee24351fd5f4f188ad80554cad79c82524
>
  
Florian Weimer Nov. 10, 2022, 8:41 p.m. UTC | #6
* H. J. Lu:

>> diff --git a/sysdeps/mips/dl-machine-reject-phdr.h b/sysdeps/mips/dl-machine-reject-phdr.h
>> index 45b6bcaeac..7efd5fc92b 100644
>> --- a/sysdeps/mips/dl-machine-reject-phdr.h
>> +++ b/sysdeps/mips/dl-machine-reject-phdr.h
>> @@ -169,7 +169,7 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
>>    bool cannot_mode_switch = false;
>>
>>    /* Get the current hardware mode.  */
>> -  cur_mode = __prctl (PR_GET_FP_MODE);
>> +  cur_mode = __prctl (PR_GET_FP_MODE, 0, 0, 0, 0);
>
> Can it use INLINE_SYSCALL_CALL here?

Probably, <sysdep.h> should be included by this point.  I can't run this
code for testing, so I tried to make minimal changes.

Thanks,
Florian
  
Szabolcs Nagy Nov. 11, 2022, 1:27 p.m. UTC | #7
The 11/10/2022 21:08, Florian Weimer via Libc-alpha wrote:
> * H. J. Lu:
> 
> > On Thu, Nov 10, 2022 at 11:18 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > prctl is an exported variadic function.  Do we need to keep it?
> >>
> >> Do you mean whether we should keep it as a variadic function or turn it
> >> into a function with a fixed argument list?
> >
> > Yes.
> >
> >> Unfortunately the types and number of arguments vary between the
> >> sub-operations.  Pretty much any application calling prctl would have to
> >> be changed.  It's just syscall by another name (and with a fixed number
> >> of course).
> >>
> >
> > <sys/prctl.h> has
> >
> > extern int prctl (int __option, ...) __THROW;
> >
> > Your change works for
> >
> > int prctl(int option, unsigned long arg2, unsigned long arg3,
> >                  unsigned long arg4, unsigned long arg5);
> >
> > prtcl (...);
> >
> > But it breaks
> >
> > #include <sys/prctl.h>
> >
> > prctl (...);
> 
> I don't think that's true for our current ABIs.  They support
> unprototyped calls, and by extension this also means that calls through
> a variadic prototype to a prototyped non-variadic function must work as
> well.  There are other ABIs which do not work this way, but glibc hasn't
> been ported to them (and it's unclear how useful they would be for
> GNU/Linux).

this is not true in general.

unprototyped call to variadic functions is not valid in c.
so unprototyped call abi does not have to match both the
variadic and non-variadic call abi. (only non-variadic calls
with arg types that promote to themselves need to work.)

e.g. on arm variadic functions always take float args following the
soft float abi, but an unprototyped call with double args uses the
hard float abi on armhf (int vs float regs).

i guess the prctl case works on existing abis now, but i think
future abis might want different variadic call abi (morello is an
example, though that's an experimental target).

prctl is nasty because the arguments are often passed with the
wrong type (e.g. int arguments instead of unsigned long) and
with variadic prototype this can pass wrong value (e.g. non-zero
top bits on 64bit targets) down to the kernel. unfortunately
whatever glibc does internally won't solve this issue: the public
API would need to change.

> 
> The new C implementation is equivalent to the previous assembler
> implementation.
  
Florian Weimer Sept. 4, 2023, 2:46 p.m. UTC | #8
* Szabolcs Nagy:

>> I don't think that's true for our current ABIs.  They support
>> unprototyped calls, and by extension this also means that calls through
>> a variadic prototype to a prototyped non-variadic function must work as
>> well.  There are other ABIs which do not work this way, but glibc hasn't
>> been ported to them (and it's unclear how useful they would be for
>> GNU/Linux).
>
> this is not true in general.
>
> unprototyped call to variadic functions is not valid in c.

It is valid C before C99.

GCC still accepts unprotyped calls with a warning in all C language
modes, which is why I think we need to keep supporting it.

> so unprototyped call abi does not have to match both the
> variadic and non-variadic call abi. (only non-variadic calls
> with arg types that promote to themselves need to work.)

It's true that the ABI does not have to match exactly.  We already have
several ABIs where non-variadic calls to functions implemented as
variadic functions fail.  to my knowledge, we do not have something in
the other direction (and we really can't, for pre-C99 support).

The patch I posted avoids an ABI break on powerpc64le.

> e.g. on arm variadic functions always take float args following the
> soft float abi, but an unprototyped call with double args uses the
> hard float abi on armhf (int vs float regs).

How do you maintain C89 compatibility with such an ABI?  I assume the
difference is only in the non-variadic part.  For the variadic part,
float needs to be promoted to double.

> i guess the prctl case works on existing abis now, but i think
> future abis might want different variadic call abi (morello is an
> example, though that's an experimental target).

The ABIs are already different today on powerpc64le today, even for
prctl.  Variadic functions on powerpc64le assume additional setup by the
caller, which does not happen with a non-variadic call.

The man-pages project used to document prctl as a non-variadic function.

> prctl is nasty because the arguments are often passed with the
> wrong type (e.g. int arguments instead of unsigned long) and
> with variadic prototype this can pass wrong value (e.g. non-zero
> top bits on 64bit targets) down to the kernel. unfortunately
> whatever glibc does internally won't solve this issue: the public
> API would need to change.

The C wrapper I'm fixing tries to solve this for the ILP32 with 64-bit
syscall registers case.  It's just that the original fix exposes another
bug.

Thanks,
Florian
  
Szabolcs Nagy Sept. 4, 2023, 4:37 p.m. UTC | #9
The 09/04/2023 16:46, Florian Weimer wrote:
> * Szabolcs Nagy:
> >> I don't think that's true for our current ABIs.  They support
> >> unprototyped calls, and by extension this also means that calls through
> >> a variadic prototype to a prototyped non-variadic function must work as
> >> well.  There are other ABIs which do not work this way, but glibc hasn't
> >> been ported to them (and it's unclear how useful they would be for
> >> GNU/Linux).
> >
> > this is not true in general.
> >
> > unprototyped call to variadic functions is not valid in c.
> 
> It is valid C before C99.

as far as i can tell iso c never allowed unprototyped call
to variadic functions.

(looking at sections "3.3.2.2 Function calls" and
"3.7.1 Function definitions" in c89.)

> GCC still accepts unprotyped calls with a warning in all C language
> modes, which is why I think we need to keep supporting it.

for me gcc accepts

void f(int, double);
void f();

but errors on

void f(int, ...);
void f();

as it should.

> > so unprototyped call abi does not have to match both the
> > variadic and non-variadic call abi. (only non-variadic calls
> > with arg types that promote to themselves need to work.)
> 
> It's true that the ABI does not have to match exactly.  We already have
> several ABIs where non-variadic calls to functions implemented as
> variadic functions fail.  to my knowledge, we do not have something in
> the other direction (and we really can't, for pre-C99 support).

i gave an example below if we allow floating-point arguments.

> > e.g. on arm variadic functions always take float args following the
> > soft float abi, but an unprototyped call with double args uses the
> > hard float abi on armhf (int vs float regs).
> 
> How do you maintain C89 compatibility with such an ABI?  I assume the
> difference is only in the non-variadic part.  For the variadic part,
> float needs to be promoted to double.

an f(1,1.0) call is abi compat between

double f(int, double);
double f();

declarations (double is passed/returned in fp regs), but

double f(int, ...);

is different (double is passed/returned in a pair of int regs).

this is valid c89 implementation.

> > i guess the prctl case works on existing abis now, but i think
> > future abis might want different variadic call abi (morello is an
> > example, though that's an experimental target).
> 
> The ABIs are already different today on powerpc64le today, even for
> prctl.  Variadic functions on powerpc64le assume additional setup by the
> caller, which does not happen with a non-variadic call.
> 
> The man-pages project used to document prctl as a non-variadic function.

the man-pages project is confused when it is documenting
syscalls, sometimes documenting the linux abi other times
the libc api or not following c rules at all (see e.g.
open or futex man pages).

what matters is how the function was declared in glibc
headers.. glibc could make the declaration non-variadic
which would solve a lot of issues, but i guess a new
symbol should be used for that then.

> > prctl is nasty because the arguments are often passed with the
> > wrong type (e.g. int arguments instead of unsigned long) and
> > with variadic prototype this can pass wrong value (e.g. non-zero
> > top bits on 64bit targets) down to the kernel. unfortunately
> > whatever glibc does internally won't solve this issue: the public
> > API would need to change.
> 
> The C wrapper I'm fixing tries to solve this for the ILP32 with 64-bit
> syscall registers case.  It's just that the original fix exposes another
> bug.

not sure how you can solve this with a wrapper.
unless it is libc header magic.
  

Patch

diff --git a/include/sys/prctl.h b/include/sys/prctl.h
index d33f3a290e..3b9a949a03 100644
--- a/include/sys/prctl.h
+++ b/include/sys/prctl.h
@@ -3,7 +3,8 @@ 
 
 # ifndef _ISOMAC
 
-extern int __prctl (int __option, ...);
+extern int __prctl (int __option, unsigned long int, unsigned long int,
+                    unsigned long int, unsigned long int);
 libc_hidden_proto (__prctl)
 
 # endif /* !_ISOMAC */
diff --git a/nptl/pthread_getname.c b/nptl/pthread_getname.c
index ebec06e23f..8d485d874b 100644
--- a/nptl/pthread_getname.c
+++ b/nptl/pthread_getname.c
@@ -38,7 +38,7 @@  __pthread_getname_np (pthread_t th, char *buf, size_t len)
     return ERANGE;
 
   if (pd == THREAD_SELF)
-    return __prctl (PR_GET_NAME, buf) ? errno : 0;
+    return -INTERNAL_SYSCALL_CALL (prctl, PR_GET_NAME, buf);
 
 #define FMT "/proc/self/task/%u/comm"
   char fname[sizeof (FMT) + 8];
diff --git a/nptl/pthread_setname.c b/nptl/pthread_setname.c
index f24560db47..1da548dbbf 100644
--- a/nptl/pthread_setname.c
+++ b/nptl/pthread_setname.c
@@ -40,7 +40,7 @@  __pthread_setname_np (pthread_t th, const char *name)
     return ERANGE;
 
   if (pd == THREAD_SELF)
-    return __prctl (PR_SET_NAME, name) ? errno : 0;
+    return -INTERNAL_SYSCALL_CALL (prctl, PR_SET_NAME, name);
 
 #define FMT "/proc/self/task/%u/comm"
   char fname[sizeof (FMT) + 8];
diff --git a/sysdeps/mips/dl-machine-reject-phdr.h b/sysdeps/mips/dl-machine-reject-phdr.h
index 45b6bcaeac..7efd5fc92b 100644
--- a/sysdeps/mips/dl-machine-reject-phdr.h
+++ b/sysdeps/mips/dl-machine-reject-phdr.h
@@ -169,7 +169,7 @@  elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
   bool cannot_mode_switch = false;
 
   /* Get the current hardware mode.  */
-  cur_mode = __prctl (PR_GET_FP_MODE);
+  cur_mode = __prctl (PR_GET_FP_MODE, 0, 0, 0, 0);
 # endif
 #endif
 
@@ -305,15 +305,15 @@  elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
 	/* Set the new mode.  Use fr1_mode if the requirements cannot be met by
 	   FR0.  */
 	if (!in_req.fr0)
-	  return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
-	else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0) != 0)
+	  return __prctl (PR_SET_FP_MODE, fr1_mode, 0, 0, 0) != 0;
+	else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0, 0, 0, 0) != 0)
 	  {
 	    /* Setting FR0 can validly fail on an R6 core so retry with the FR1
 	       mode as a fall back.  */
 	    if (errno != ENOTSUP)
 	      return true;
 
-	    return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
+	    return __prctl (PR_SET_FP_MODE, fr1_mode, 0, 0, 0) != 0;
 	  }
       }
 # endif /* HAVE_PRCTL_FP_MODE */
diff --git a/sysdeps/unix/sysv/linux/prctl.c b/sysdeps/unix/sysv/linux/prctl.c
index 25fd968ae2..102d4b0a21 100644
--- a/sysdeps/unix/sysv/linux/prctl.c
+++ b/sysdeps/unix/sysv/linux/prctl.c
@@ -18,7 +18,15 @@ 
 
 #include <sysdep.h>
 #include <stdarg.h>
+
+/* Hide the type information from GCC.  The prototype in the installed
+   header is a variadic function.  The non-variadic implementation
+   below is both more efficient and more compatible because some ABIs
+   assume additional work performed by the caller for variadic
+   functions (notably powerpc64le).  */
+#define prctl prctl_XXX
 #include <sys/prctl.h>
+#undef prctl
 
 /* Unconditionally read all potential arguments.  This may pass
    garbage values to the kernel, but avoids the need for teaching
@@ -26,15 +34,9 @@ 
    that are added to the kernel in the future).  */
 
 int
-__prctl (int option, ...)
+__prctl (int option, unsigned long int arg2, unsigned long int arg3,
+         unsigned long int arg4, unsigned long int arg5)
 {
-  va_list arg;
-  va_start (arg, option);
-  unsigned long int arg2 = va_arg (arg, unsigned long int);
-  unsigned long int arg3 = va_arg (arg, unsigned long int);
-  unsigned long int arg4 = va_arg (arg, unsigned long int);
-  unsigned long int arg5 = va_arg (arg, unsigned long int);
-  va_end (arg);
   return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
 }