Re: [PATCH] Linux: Support non-variadic calls to prctl (bug 29770)

["H.J. Lu" <hjl.tools@gmail.com>]

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
>


-- 
H.J.