From: "H.J. Lu" <hjl.tools@gmail.com>
To: Florian Weimer <fweimer@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH] Linux: Support non-variadic calls to prctl (bug 29770)
Date: Thu, 10 Nov 2022 10:43:26 -0800 [thread overview]
Message-ID: <CAMe9rOqVyU9ob+JrKzT74VQ+TRx_c52BemGSs22Uo5Vi7t2Axg@mail.gmail.com> (raw)
In-Reply-To: <878rkiu72f.fsf@oldenburg.str.redhat.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);
> # 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?
--
H.J.
next prev parent reply other threads:[~2022-11-10 18:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-10 16:07 Florian Weimer
2022-11-10 18:43 ` H.J. Lu [this message]
2022-11-10 19:18 ` Florian Weimer
2022-11-10 19:55 ` H.J. Lu
2022-11-10 20:08 ` Florian Weimer
2022-11-11 13:27 ` Szabolcs Nagy
2023-09-04 14:46 ` Florian Weimer
2023-09-04 16:37 ` Szabolcs Nagy
2022-11-10 20:34 ` H.J. Lu
2022-11-10 20:41 ` Florian Weimer
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=CAMe9rOqVyU9ob+JrKzT74VQ+TRx_c52BemGSs22Uo5Vi7t2Axg@mail.gmail.com \
--to=hjl.tools@gmail.com \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).