Message ID | 20200430130333.GA254612@gmail.com |
---|---|
State | New |
Headers | show |
Series | V2 [PATCH] Add a C wrapper for prctl [BZ #25896] | expand |
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.
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) # endif /* !_ISOMAC */ #endif diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index db78eeadd1..0326f92c40 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -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 diff --git a/sysdeps/unix/sysv/linux/prctl.c b/sysdeps/unix/sysv/linux/prctl.c new file mode 100644 index 0000000000..49049fd85f --- /dev/null +++ b/sysdeps/unix/sysv/linux/prctl.c @@ -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) diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list index ced77d49fa..1d8893d1b8 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list +++ b/sysdeps/unix/sysv/linux/syscalls.list @@ -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 diff --git a/sysdeps/unix/sysv/linux/x86/prctl.S b/sysdeps/unix/sysv/linux/x86/prctl.S new file mode 100644 index 0000000000..99bb62ad39 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86/prctl.S @@ -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) diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S b/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S new file mode 100644 index 0000000000..5acf5406a8 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S @@ -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>