V2 [PATCH] Add a C wrapper for prctl [BZ #25896]
Commit Message
On Thu, Apr 30, 2020 at 09:26:18AM +0200, Florian Weimer wrote:
> * H. J. Lu via Libc-alpha:
>
> > Add a C wrapper to pass arguments in
> >
> > /* Control process execution. */
> > extern int prctl (int __option, ...) __THROW;
> >
> > to prctl syscall:
> >
> > extern int prctl (int, unsigned long int, unsigned long int,
> > unsigned long int, unsigned long int);
> > ---
> > include/sys/prctl.h | 2 ++
> > sysdeps/unix/sysv/linux/Makefile | 2 +-
> > sysdeps/unix/sysv/linux/prctl.c | 38 +++++++++++++++++++++++++++
> > sysdeps/unix/sysv/linux/syscalls.list | 1 -
> > 4 files changed, 41 insertions(+), 2 deletions(-)
> > create mode 100644 sysdeps/unix/sysv/linux/prctl.c
> >
> > diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> > index 0920ed642b..1a74d83879 100644
> > --- a/include/sys/prctl.h
> > +++ b/include/sys/prctl.h
> > @@ -4,6 +4,8 @@
> > # ifndef _ISOMAC
> >
> > extern int __prctl (int __option, ...);
> > +libc_hidden_proto (__prctl)
> > +libc_hidden_proto (prctl)
>
> I see we have some internal callers of prctl. I will clean this up
> later.
>
>
> Please add a comment that this over-reads arguments for some option
> values, something like:
>
> /* Unconditionally read all potential arguments. This may pass
> garbage values to the kernel, but avoids the need for teaching
> glibc the argument counts of individual options (including ones
> that are added to the kernel in the future). */
>
> Rest looks fine. Generated code is quite a bit worse, but I don't
> think this matter here.
Here is the updated patch. I added the assembly version for x86. Other
arches can do
#include <sysdeps/unix/sysv/linux/x86/prctl.S>
if they are similar.
OK for master?
Thanks.
H.J.
---
Add a C wrapper to pass arguments in
/* Control process execution. */
extern int prctl (int __option, ...) __THROW;
to prctl syscall:
extern int prctl (int, unsigned long int, unsigned long int,
unsigned long int, unsigned long int);
On Linux/x86, since the prctl syscall interface:
extern int prctl (int, unsigned long int, unsigned long int,
unsigned long int, unsigned long int);
and the glibc prctl interface:
extern int prctl (int option, ...);
pass the arguments identically, the assembly verion:
PSEUDO (__prctl, prctl, 5)
ret
PSEUDO_END (__prctl)
is used.
---
include/sys/prctl.h | 2 +
sysdeps/unix/sysv/linux/Makefile | 2 +-
sysdeps/unix/sysv/linux/prctl.c | 43 ++++++++++++++++++++++
sysdeps/unix/sysv/linux/syscalls.list | 1 -
sysdeps/unix/sysv/linux/x86/prctl.S | 38 +++++++++++++++++++
sysdeps/unix/sysv/linux/x86_64/x32/prctl.S | 28 ++++++++++++++
6 files changed, 112 insertions(+), 2 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/prctl.c
create mode 100644 sysdeps/unix/sysv/linux/x86/prctl.S
create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/prctl.S
Comments
On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> index 0920ed642b..1a74d83879 100644
> --- a/include/sys/prctl.h
> +++ b/include/sys/prctl.h
> @@ -4,6 +4,8 @@
> # ifndef _ISOMAC
>
> extern int __prctl (int __option, ...);
> +libc_hidden_proto (__prctl)
> +libc_hidden_proto (prctl)
Why do you need the hidden alias for prctl? All references inside
libc.so should use __prctl.
Andreas.
* Andreas Schwab:
> On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
>
>> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
>> index 0920ed642b..1a74d83879 100644
>> --- a/include/sys/prctl.h
>> +++ b/include/sys/prctl.h
>> @@ -4,6 +4,8 @@
>> # ifndef _ISOMAC
>>
>> extern int __prctl (int __option, ...);
>> +libc_hidden_proto (__prctl)
>> +libc_hidden_proto (prctl)
>
> Why do you need the hidden alias for prctl? All references inside
> libc.so should use __prctl.
Right. I want to clean this up in a separate patch.
On Thu, Apr 30, 2020 at 6:43 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * Andreas Schwab:
>
> > On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
> >
> >> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> >> index 0920ed642b..1a74d83879 100644
> >> --- a/include/sys/prctl.h
> >> +++ b/include/sys/prctl.h
> >> @@ -4,6 +4,8 @@
> >> # ifndef _ISOMAC
> >>
> >> extern int __prctl (int __option, ...);
> >> +libc_hidden_proto (__prctl)
> >> +libc_hidden_proto (prctl)
> >
> > Why do you need the hidden alias for prctl? All references inside
> > libc.so should use __prctl.
>
> Right. I want to clean this up in a separate patch.
I will wait for your cleanup patch and remove the hidden alias for prctl.
On Apr 30 2020, Florian Weimer wrote:
> * Andreas Schwab:
>
>> On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
>>
>>> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
>>> index 0920ed642b..1a74d83879 100644
>>> --- a/include/sys/prctl.h
>>> +++ b/include/sys/prctl.h
>>> @@ -4,6 +4,8 @@
>>> # ifndef _ISOMAC
>>>
>>> extern int __prctl (int __option, ...);
>>> +libc_hidden_proto (__prctl)
>>> +libc_hidden_proto (prctl)
>>
>> Why do you need the hidden alias for prctl? All references inside
>> libc.so should use __prctl.
>
> Right. I want to clean this up in a separate patch.
??? Why adding it in the first place?
Andreas.
On Thu, Apr 30, 2020 at 6:51 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Apr 30 2020, Florian Weimer wrote:
>
> > * Andreas Schwab:
> >
> >> On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
> >>
> >>> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> >>> index 0920ed642b..1a74d83879 100644
> >>> --- a/include/sys/prctl.h
> >>> +++ b/include/sys/prctl.h
> >>> @@ -4,6 +4,8 @@
> >>> # ifndef _ISOMAC
> >>>
> >>> extern int __prctl (int __option, ...);
> >>> +libc_hidden_proto (__prctl)
> >>> +libc_hidden_proto (prctl)
> >>
> >> Why do you need the hidden alias for prctl? All references inside
> >> libc.so should use __prctl.
> >
> > Right. I want to clean this up in a separate patch.
>
> ??? Why adding it in the first place?
>
> Andreas.
>
It was there before my patch:
0000000000000000 T __GI___prctl
0000000000000000 T __GI_prctl
U _GLOBAL_OFFSET_TABLE_
U __libc_errno
0000000000000000 T __prctl
0000000000000000 W prctl
* Andreas Schwab:
> On Apr 30 2020, Florian Weimer wrote:
>
>> * Andreas Schwab:
>>
>>> On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
>>>
>>>> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
>>>> index 0920ed642b..1a74d83879 100644
>>>> --- a/include/sys/prctl.h
>>>> +++ b/include/sys/prctl.h
>>>> @@ -4,6 +4,8 @@
>>>> # ifndef _ISOMAC
>>>>
>>>> extern int __prctl (int __option, ...);
>>>> +libc_hidden_proto (__prctl)
>>>> +libc_hidden_proto (prctl)
>>>
>>> Why do you need the hidden alias for prctl? All references inside
>>> libc.so should use __prctl.
>>
>> Right. I want to clean this up in a separate patch.
>
> ??? Why adding it in the first place?
I thought pthread_getname was linked into libc, but looks like it's in
libpthread, so its use of prctl (not __prctl) should be fine.
H.J., what happens if you drop the hidden alias for prctl?
On Apr 30 2020, H.J. Lu wrote:
> On Thu, Apr 30, 2020 at 6:51 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>>
>> On Apr 30 2020, Florian Weimer wrote:
>>
>> > * Andreas Schwab:
>> >
>> >> On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
>> >>
>> >>> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
>> >>> index 0920ed642b..1a74d83879 100644
>> >>> --- a/include/sys/prctl.h
>> >>> +++ b/include/sys/prctl.h
>> >>> @@ -4,6 +4,8 @@
>> >>> # ifndef _ISOMAC
>> >>>
>> >>> extern int __prctl (int __option, ...);
>> >>> +libc_hidden_proto (__prctl)
>> >>> +libc_hidden_proto (prctl)
>> >>
>> >> Why do you need the hidden alias for prctl? All references inside
>> >> libc.so should use __prctl.
>> >
>> > Right. I want to clean this up in a separate patch.
>>
>> ??? Why adding it in the first place?
>>
>> Andreas.
>>
>
> It was there before my patch:
That doesn't matter, the syscall template adds it blindly to all names.
Andreas.
On 30/04/2020 10:03, H.J. Lu via Libc-alpha wrote:
> Here is the updated patch. I added the assembly version for x86. Other
> arches can do
>
> #include <sysdeps/unix/sysv/linux/x86/prctl.S>
Do we really an assembly optimization for this? I hardly think this
a hotstop symbol.
On Thu, Apr 30, 2020 at 9:06 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 30/04/2020 10:03, H.J. Lu via Libc-alpha wrote:
>
> > Here is the updated patch. I added the assembly version for x86. Other
> > arches can do
> >
> > #include <sysdeps/unix/sysv/linux/x86/prctl.S>
>
> Do we really an assembly optimization for this? I hardly think this
> a hotstop symbol.
I have no strong opinion on this.
* H. J. Lu via Libc-alpha:
> On Thu, Apr 30, 2020 at 9:06 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 30/04/2020 10:03, H.J. Lu via Libc-alpha wrote:
>>
>> > Here is the updated patch. I added the assembly version for x86. Other
>> > arches can do
>> >
>> > #include <sysdeps/unix/sysv/linux/x86/prctl.S>
>>
>> Do we really an assembly optimization for this? I hardly think this
>> a hotstop symbol.
>
> I have no strong opinion on this.
Me neither.
On 30/04/2020 13:51, Florian Weimer wrote:
> * H. J. Lu via Libc-alpha:
>
>> On Thu, Apr 30, 2020 at 9:06 AM Adhemerval Zanella via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>>
>>>
>>> On 30/04/2020 10:03, H.J. Lu via Libc-alpha wrote:
>>>
>>>> Here is the updated patch. I added the assembly version for x86. Other
>>>> arches can do
>>>>
>>>> #include <sysdeps/unix/sysv/linux/x86/prctl.S>
>>>
>>> Do we really an assembly optimization for this? I hardly think this
>>> a hotstop symbol.
>>
>> I have no strong opinion on this.
>
> Me neither.
>
I would prefer to push for an assembly optimization for symbol that
performance is paramount and usually might appear as a hotspot.
My understanding is usually prctl is used scarcely and I think
it would be better to use the C generic.
@@ -4,6 +4,8 @@
# ifndef _ISOMAC
extern int __prctl (int __option, ...);
+libc_hidden_proto (__prctl)
+libc_hidden_proto (prctl)
# endif /* !_ISOMAC */
#endif
@@ -59,7 +59,7 @@ sysdep_routines += adjtimex clone umount umount2 readahead sysctl \
eventfd eventfd_read eventfd_write prlimit \
personality epoll_wait tee vmsplice splice \
open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \
- timerfd_gettime timerfd_settime \
+ timerfd_gettime timerfd_settime prctl \
process_vm_readv process_vm_writev
CFLAGS-gethostid.c = -fexceptions
new file mode 100644
@@ -0,0 +1,43 @@
+/* prctl - Linux specific syscall.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <sysdep.h>
+#include <stdarg.h>
+#include <sys/prctl.h>
+
+/* Unconditionally read all potential arguments. This may pass
+ garbage values to the kernel, but avoids the need for teaching
+ glibc the argument counts of individual options (including ones
+ that are added to the kernel in the future). */
+
+int
+__prctl (int option, ...)
+{
+ 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);
+}
+
+libc_hidden_def (__prctl)
+weak_alias (__prctl, prctl)
+hidden_weak (prctl)
@@ -43,7 +43,6 @@ nfsservctl EXTRA nfsservctl i:ipp __compat_nfsservctl nfsservctl@GLIBC_2.0:GLIBC
pipe - pipe i:f __pipe pipe
pipe2 - pipe2 i:fi __pipe2 pipe2
pivot_root EXTRA pivot_root i:ss pivot_root
-prctl EXTRA prctl i:iiiii __prctl prctl
query_module EXTRA query_module i:sipip __compat_query_module query_module@GLIBC_2.0:GLIBC_2.23
quotactl EXTRA quotactl i:isip quotactl
remap_file_pages - remap_file_pages i:pUiUi __remap_file_pages remap_file_pages
new file mode 100644
@@ -0,0 +1,38 @@
+/* prctl - Linux specific syscall. Linux/x86 version.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <sysdep.h>
+
+/* On Linux/x86, the prctl syscall interface:
+
+ extern int prctl (int, unsigned long int, unsigned long int,
+ unsigned long int, unsigned long int);
+
+ and the glibc prctl interface:
+
+ extern int prctl (int option, ...);
+
+ pass the arguments identically. */
+
+PSEUDO (__prctl, prctl, 5)
+ ret
+PSEUDO_END (__prctl)
+
+hidden_def (__prctl)
+weak_alias (__prctl, prctl)
+hidden_weak (prctl)
new file mode 100644
@@ -0,0 +1,28 @@
+/* prctl - Linux specific syscall. Linux/x32 version.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <sysdep.h>
+
+/* On Linux/x32, we need to zero-extend from 32 bits to 64 bits for the
+ last 4 arguments. */
+
+#undef DOARGS_5
+#define DOARGS_5 \
+ movl %esi, %esi; movl %edx, %edx; movl %ecx, %r10d; movl %r8d, %r8d;
+
+#include <sysdeps/unix/sysv/linux/x86/prctl.S>