| Message ID | 20160320170528.GO3635@var.home |
|---|---|
| State | New |
| Headers | show |
On Sun, Mar 20, 2016 at 10:05 AM, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote: > Samuel Thibault, on Sun 20 Mar 2016 17:42:14 +0100, wrote: >> AIUI, this is because there are weak definitions in >> sysdeps/mach/hurd/dl-sysdep.c which need to be preempted by the libc >> one, right? Is __access perhaps missing in the list? There is a weak >> definition for __access in dl-sysdep.c. >> >> Also, 6d56699d7e808419ccf244150ecba122156932ba ('Mark internal fcntl >> functions hidden') made __open hidden, but dl-sysdep.c has a weak >> definition, which AIUI needs to be preempted too, so that this commit >> should be split to a dl-fcntl.h that sysdepas/mach/hurd/ can provide, >> right? >> >> (Just asking the question, I'll handle commiting etc.) It is not about weak nor not. By default, there is no difference between weak defined symbol and non-weak defined symbols at run-time. The question should be asked is if __access in dl-sysdep.c can be used in ld.so after bootstrap on Hurd. > Just to explicit things, attached is the fix I'm considering. > > Samuel
H.J. Lu, on Sun 20 Mar 2016 10:14:40 -0700, wrote: > >> Is __access perhaps missing in the list? There is a weak > >> definition for __access in dl-sysdep.c. [...] > It is not about weak nor not. By default, there is no difference > between weak defined symbol and non-weak defined symbols > at run-time. Ah, right. > The question should be asked is if __access in dl-sysdep.c can > be used in ld.so after bootstrap on Hurd. Ok. It seems it's only used in dl_main and in process_envvars only called by dl_main, so it should be fine, right? About __open, it's definitely used for dlopen() :) Thanks, Samuel
On Sun, Mar 20, 2016 at 10:31 AM, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote: > H.J. Lu, on Sun 20 Mar 2016 10:14:40 -0700, wrote: >> >> Is __access perhaps missing in the list? There is a weak >> >> definition for __access in dl-sysdep.c. > [...] >> It is not about weak nor not. By default, there is no difference >> between weak defined symbol and non-weak defined symbols >> at run-time. > > Ah, right. > >> The question should be asked is if __access in dl-sysdep.c can >> be used in ld.so after bootstrap on Hurd. > > Ok. It seems it's only used in dl_main and in process_envvars only > called by dl_main, so it should be fine, right? > > About __open, it's definitely used for dlopen() :) I don't know since they are Hurd specific.
H.J. Lu, on Sun 20 Mar 2016 10:42:11 -0700, wrote: > On Sun, Mar 20, 2016 at 10:40 AM, Samuel Thibault > <samuel.thibault@ens-lyon.org> wrote: > > H.J. Lu, on Sun 20 Mar 2016 10:36:45 -0700, wrote: > >> On Sun, Mar 20, 2016 at 10:31 AM, Samuel Thibault > >> <samuel.thibault@ens-lyon.org> wrote: > >> > H.J. Lu, on Sun 20 Mar 2016 10:14:40 -0700, wrote: > >> >> The question should be asked is if __access in dl-sysdep.c can > >> >> be used in ld.so after bootstrap on Hurd. > >> > > >> > Ok. It seems it's only used in dl_main and in process_envvars only > >> > called by dl_main, so it should be fine, right? > >> > > >> > About __open, it's definitely used for dlopen() :) > >> > >> I don't know since they are Hurd specific. > > > > ? dl_main, process_envvars, __open and dlopen are not Hurd specific. > > Whether a function defined in ld.so can be used after bootstrap is > Hurd specific. Sure, but for __open() for instance, its reference from dl-load.c is not Hurd-specific, so it is clearly used for dlopen() after bootstrap. Samuel
On 03/20/2016 06:05 PM, Samuel Thibault wrote: > hurd: Do not hide rtld symbols which need to be preempted > > * sysdeps/generic/dl-fcntl.h: New file, hides __open and __fcntl. > * sysdeps/mach/hurd/dl-fcntl.h: New file, hides __fcntl only. > * include/fcntl.h [IS_IN (rtld)]: Include <dl-fcntl.h> instead of > hiding __open and __fcntl. > * sysdeps/mach/hurd/dl-unistd.h: Do not hide __access. How did you test this, considering that master is quite far away from building on master? Thanks, Florian
diff --git a/ChangeLog b/ChangeLog index ce46c6b..aa151cf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2016-03-20 Samuel Thibault <samuel.thibault@ens-lyon.org>: + + * sysdeps/generic/dl-fcntl.h: New file, hides __open and __fcntl. + * sysdeps/mach/hurd/dl-fcntl.h: New file, hides __fcntl only. + * include/fcntl.h [IS_IN (rtld)]: Include <dl-fcntl.h> instead of + hiding __open and __fcntl. + * sysdeps/mach/hurd/dl-unistd.h: Do not hide __access. + 2016-03-20 Samuel Thibault <samuel.thibault@ens-lyon.org> * sysdeps/mach/hurd/Makefile ($(common-objpfx)errnos.d): Strip diff --git a/include/fcntl.h b/include/fcntl.h index 4168ee4..3b2c887 100644 --- a/include/fcntl.h +++ b/include/fcntl.h @@ -31,8 +31,7 @@ extern int __openat64_2 (int __fd, const char *__path, int __oflag); #if IS_IN (rtld) -extern __typeof (__open) __open attribute_hidden; -extern __typeof (__fcntl) __fcntl attribute_hidden; +# include <dl-fcntl.h> #endif /* Flag determining whether the *at system calls are available. */ diff --git a/sysdeps/generic/dl-fcntl.h b/sysdeps/generic/dl-fcntl.h new file mode 100644 index 0000000..ee3c49c --- /dev/null +++ b/sysdeps/generic/dl-fcntl.h @@ -0,0 +1,21 @@ +/* Functions with hidden attribute internal to ld.so, which are declared + in include/fcntl.h. Generic version. + Copyright (C) 2016 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 + <http://www.gnu.org/licenses/>. */ + +extern __typeof (__open) __open attribute_hidden; +extern __typeof (__fcntl) __fcntl attribute_hidden; diff --git a/sysdeps/mach/hurd/dl-fcntl.h b/sysdeps/mach/hurd/dl-fcntl.h new file mode 100644 index 0000000..0fef2f1 --- /dev/null +++ b/sysdeps/mach/hurd/dl-fcntl.h @@ -0,0 +1,22 @@ +/* Functions with hidden attribute internal to ld.so, which are declared + in include/fcntl.h. Hurd version. + Copyright (C) 2016 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 + <http://www.gnu.org/licenses/>. */ + +/* __open can't be hidden in ld.so on Hurd since they will be preempted by the + ones in libc.so after bootstrap. */ +extern __typeof (__fcntl) __fcntl attribute_hidden; diff --git a/sysdeps/mach/hurd/dl-unistd.h b/sysdeps/mach/hurd/dl-unistd.h index b3cfb61..88bdd80 100644 --- a/sysdeps/mach/hurd/dl-unistd.h +++ b/sysdeps/mach/hurd/dl-unistd.h @@ -17,10 +17,9 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -/* __close, __getcwd, __getpid, __libc_read and __libc_write can't be +/* __access, __close, __getcwd, __getpid, __libc_read and __libc_write can't be hidden in ld.so on Hurd since they will be preempted by the ones in libc.so after bootstrap. */ -extern __typeof (__access) __access attribute_hidden; extern __typeof (__brk) __brk attribute_hidden; extern __typeof (__lseek) __lseek attribute_hidden; extern __typeof (__profil) __profil attribute_hidden;