* RFC: Remove --disable-hidden-plt @ 2021-04-03 17:41 H.J. Lu 2021-04-06 17:12 ` Adhemerval Zanella 0 siblings, 1 reply; 7+ messages in thread From: H.J. Lu @ 2021-04-03 17:41 UTC (permalink / raw) To: GNU C Library --disable-hidden-plt was added by commit 749a9a4fbfd261e2d9811d9bc4507478c2f7cc58 Author: Roland McGrath <roland@gnu.org> Date: Tue Oct 1 08:45:44 2002 +0000 2002-10-01 Roland McGrath <roland@frob.com> * config.h.in (NO_HIDDEN): New #undef. * include/libc-symbols.h [! NO_HIDDEN]: Add this condition to nonempty definitions of hidden_proto et al. * configure.in: Grok --disable-hidden-plt to define NO_HIDDEN. * configure: Regenerated. * sysdeps/mach/hurd/configure.in: Always define NO_HIDDEN. * sysdeps/mach/hurd/configure: Regenerated. It doesn't work on x86-64: https://sourceware.org/bugzilla/show_bug.cgi?id=27692 Should it be removed? -- H.J. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: RFC: Remove --disable-hidden-plt 2021-04-03 17:41 RFC: Remove --disable-hidden-plt H.J. Lu @ 2021-04-06 17:12 ` Adhemerval Zanella 2021-04-06 17:18 ` Samuel Thibault 0 siblings, 1 reply; 7+ messages in thread From: Adhemerval Zanella @ 2021-04-06 17:12 UTC (permalink / raw) To: libc-alpha, Szabolcs Nagy, rrh.henry On 03/04/2021 14:41, H.J. Lu via Libc-alpha wrote: > --disable-hidden-plt was added by > > commit 749a9a4fbfd261e2d9811d9bc4507478c2f7cc58 > Author: Roland McGrath <roland@gnu.org> > Date: Tue Oct 1 08:45:44 2002 +0000 > > 2002-10-01 Roland McGrath <roland@frob.com> > > * config.h.in (NO_HIDDEN): New #undef. > * include/libc-symbols.h [! NO_HIDDEN]: Add this condition to > nonempty definitions of hidden_proto et al. > * configure.in: Grok --disable-hidden-plt to define NO_HIDDEN. > * configure: Regenerated. > * sysdeps/mach/hurd/configure.in: Always define NO_HIDDEN. > * sysdeps/mach/hurd/configure: Regenerated. > > It doesn't work on x86-64: > > https://sourceware.org/bugzilla/show_bug.cgi?id=27692 > > Should it be removed? > My take is to remove it. Besides the reasons noted by Szabolcs [1], some symbols interposition are not really possible on some scenarios (for instance intra IFUNC call through hidden symbol on i686 and ppc32), it requires more development effort (as any configure switch and we will probably need a configure check to see if *all* supposed intra calls are actually being called through PLT and fix bugs with missing support), there is no clear advantage of providing such functionality, and it broken on every architecture for some time (at least since 2.27). However it seems it will still need to support it internally for Hurd and it does catch a potential issue where some implementation uses a non intended interface (such as __hidden_ver1). I tried to understand why Roland's has added it in the first place, but there is no discussion about it on libc-alpha in Sep/Oct 2002. [1] https://sourceware.org/bugzilla/show_bug.cgi?id=27692#c7 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: RFC: Remove --disable-hidden-plt 2021-04-06 17:12 ` Adhemerval Zanella @ 2021-04-06 17:18 ` Samuel Thibault 2021-04-06 17:39 ` Adhemerval Zanella 0 siblings, 1 reply; 7+ messages in thread From: Samuel Thibault @ 2021-04-06 17:18 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha, Szabolcs Nagy, rrh.henry Adhemerval Zanella via Libc-alpha, le mar. 06 avril 2021 14:12:41 -0300, a ecrit: > > > On 03/04/2021 14:41, H.J. Lu via Libc-alpha wrote: > > --disable-hidden-plt was added by > > > > commit 749a9a4fbfd261e2d9811d9bc4507478c2f7cc58 > > Author: Roland McGrath <roland@gnu.org> > > Date: Tue Oct 1 08:45:44 2002 +0000 > > > > 2002-10-01 Roland McGrath <roland@frob.com> > > > > * config.h.in (NO_HIDDEN): New #undef. > > * include/libc-symbols.h [! NO_HIDDEN]: Add this condition to > > nonempty definitions of hidden_proto et al. > > * configure.in: Grok --disable-hidden-plt to define NO_HIDDEN. > > * configure: Regenerated. > > * sysdeps/mach/hurd/configure.in: Always define NO_HIDDEN. > > * sysdeps/mach/hurd/configure: Regenerated. > > > > It doesn't work on x86-64: > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=27692 > > > > Should it be removed? > > > > My take is to remove it. Besides the reasons noted by Szabolcs [1], > some symbols interposition are not really possible on some scenarios > (for instance intra IFUNC call through hidden symbol on i686 and ppc32), > it requires more development effort (as any configure switch and we > will probably need a configure check to see if *all* supposed intra > calls are actually being called through PLT and fix bugs with missing > support), there is no clear advantage of providing such functionality, > and it broken on every architecture for some time (at least since > 2.27). > > However it seems it will still need to support it internally for > Hurd Yes, some symbols in ld.so (most of sysdeps/mach/hurd/dl-sysdep.c) *need* to be interposed by the loaded libc.so, because the implementation in ld.so is to be used only at bootstrap of the program, and then overriden by the real full-fledged implementation. Samuel > and it does catch a potential issue where some implementation > uses a non intended interface (such as __hidden_ver1). > > I tried to understand why Roland's has added it in the first place, > but there is no discussion about it on libc-alpha in Sep/Oct 2002. > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=27692#c7 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: RFC: Remove --disable-hidden-plt 2021-04-06 17:18 ` Samuel Thibault @ 2021-04-06 17:39 ` Adhemerval Zanella 2021-04-06 18:03 ` Samuel Thibault 0 siblings, 1 reply; 7+ messages in thread From: Adhemerval Zanella @ 2021-04-06 17:39 UTC (permalink / raw) To: Samuel Thibault; +Cc: libc-alpha, Szabolcs Nagy, rrh.henry On 06/04/2021 14:18, Samuel Thibault wrote: > Adhemerval Zanella via Libc-alpha, le mar. 06 avril 2021 14:12:41 -0300, a ecrit: >> >> >> On 03/04/2021 14:41, H.J. Lu via Libc-alpha wrote: >>> --disable-hidden-plt was added by >>> >>> commit 749a9a4fbfd261e2d9811d9bc4507478c2f7cc58 >>> Author: Roland McGrath <roland@gnu.org> >>> Date: Tue Oct 1 08:45:44 2002 +0000 >>> >>> 2002-10-01 Roland McGrath <roland@frob.com> >>> >>> * config.h.in (NO_HIDDEN): New #undef. >>> * include/libc-symbols.h [! NO_HIDDEN]: Add this condition to >>> nonempty definitions of hidden_proto et al. >>> * configure.in: Grok --disable-hidden-plt to define NO_HIDDEN. >>> * configure: Regenerated. >>> * sysdeps/mach/hurd/configure.in: Always define NO_HIDDEN. >>> * sysdeps/mach/hurd/configure: Regenerated. >>> >>> It doesn't work on x86-64: >>> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=27692 >>> >>> Should it be removed? >>> >> >> My take is to remove it. Besides the reasons noted by Szabolcs [1], >> some symbols interposition are not really possible on some scenarios >> (for instance intra IFUNC call through hidden symbol on i686 and ppc32), >> it requires more development effort (as any configure switch and we >> will probably need a configure check to see if *all* supposed intra >> calls are actually being called through PLT and fix bugs with missing >> support), there is no clear advantage of providing such functionality, >> and it broken on every architecture for some time (at least since >> 2.27). >> >> However it seems it will still need to support it internally for >> Hurd > > Yes, some symbols in ld.so (most of sysdeps/mach/hurd/dl-sysdep.c) > *need* to be interposed by the loaded libc.so, because the > implementation in ld.so is to be used only at bootstrap of the program, > and then overriden by the real full-fledged implementation. Are these the only ones required to Hurd? I wonder if would be simple to instead of have the bit NO_HIDDEN hammer, to create a different hidden_def/hidden_proto to be used on the required symbols (where default version would map to use non-PLT call and Hurd could just reimplement it). ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: RFC: Remove --disable-hidden-plt 2021-04-06 17:39 ` Adhemerval Zanella @ 2021-04-06 18:03 ` Samuel Thibault 2021-04-06 18:15 ` Adhemerval Zanella 0 siblings, 1 reply; 7+ messages in thread From: Samuel Thibault @ 2021-04-06 18:03 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha, Szabolcs Nagy, rrh.henry Adhemerval Zanella, le mar. 06 avril 2021 14:39:27 -0300, a ecrit: > On 06/04/2021 14:18, Samuel Thibault wrote: > > Adhemerval Zanella via Libc-alpha, le mar. 06 avril 2021 14:12:41 -0300, a ecrit: > >> On 03/04/2021 14:41, H.J. Lu via Libc-alpha wrote: > >> However it seems it will still need to support it internally for > >> Hurd > > > > Yes, some symbols in ld.so (most of sysdeps/mach/hurd/dl-sysdep.c) > > *need* to be interposed by the loaded libc.so, because the > > implementation in ld.so is to be used only at bootstrap of the program, > > and then overriden by the real full-fledged implementation. > > Are these the only ones required to Hurd? Yes. Previously we were using the NO_HIDDEN hammer, but: > I wonder if would be simple to instead of have the bit NO_HIDDEN > hammer, to create a different hidden_def/hidden_proto to be used on > the required symbols (where default version would map to use non-PLT > call and Hurd could just reimplement it). That was already roughly done in 3d1870fa3301c5cd00d5fdab0014c4e22b71fef2 where we made all rtld_hidden_proto respect NO_RTLD_HIDDEN. I didn't specifically mark the exactly required functions, but the localplt check exceptions in sysdeps/mach/hurd/i386/localplt.data is already exactly the list of needed PLTs, so I don't think it's worth the extra maintenance complexity of special-casing in the headers for everyone in addition to the rtld implementation in dl-sysdep.c. Samuel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: RFC: Remove --disable-hidden-plt 2021-04-06 18:03 ` Samuel Thibault @ 2021-04-06 18:15 ` Adhemerval Zanella 2021-04-06 18:43 ` Samuel Thibault 0 siblings, 1 reply; 7+ messages in thread From: Adhemerval Zanella @ 2021-04-06 18:15 UTC (permalink / raw) To: Samuel Thibault; +Cc: libc-alpha, Szabolcs Nagy, rrh.henry On 06/04/2021 15:03, Samuel Thibault wrote: > Adhemerval Zanella, le mar. 06 avril 2021 14:39:27 -0300, a ecrit: >> On 06/04/2021 14:18, Samuel Thibault wrote: >>> Adhemerval Zanella via Libc-alpha, le mar. 06 avril 2021 14:12:41 -0300, a ecrit: >>>> On 03/04/2021 14:41, H.J. Lu via Libc-alpha wrote: >>>> However it seems it will still need to support it internally for >>>> Hurd >>> >>> Yes, some symbols in ld.so (most of sysdeps/mach/hurd/dl-sysdep.c) >>> *need* to be interposed by the loaded libc.so, because the >>> implementation in ld.so is to be used only at bootstrap of the program, >>> and then overriden by the real full-fledged implementation. >> >> Are these the only ones required to Hurd? > > Yes. Previously we were using the NO_HIDDEN hammer, but: > >> I wonder if would be simple to instead of have the bit NO_HIDDEN >> hammer, to create a different hidden_def/hidden_proto to be used on >> the required symbols (where default version would map to use non-PLT >> call and Hurd could just reimplement it). > > That was already roughly done in > 3d1870fa3301c5cd00d5fdab0014c4e22b71fef2 > where we made all rtld_hidden_proto respect NO_RTLD_HIDDEN. > > I didn't specifically mark the exactly required functions, but the > localplt check exceptions in sysdeps/mach/hurd/i386/localplt.data is > already exactly the list of needed PLTs, so I don't think it's worth > the extra maintenance complexity of special-casing in the headers for > everyone in addition to the rtld implementation in dl-sysdep.c. Currently NO_RTLD_HIDDEN spills on some generic code: config.h.in:#undef NO_RTLD_HIDDEN elf/dl-minimal.c:# ifndef NO_RTLD_HIDDEN elf/dl-minimal.c:# ifndef NO_RTLD_HIDDEN include/assert.h:# if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN) include/sys/stat.h:# if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN) include/unistd.h:# ifndef NO_RTLD_HIDDEN With the removal of NO_HIDDEN configure support, I would like to move NO_RTLD_HIDDEN to be Hurd specific if possible. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: RFC: Remove --disable-hidden-plt 2021-04-06 18:15 ` Adhemerval Zanella @ 2021-04-06 18:43 ` Samuel Thibault 0 siblings, 0 replies; 7+ messages in thread From: Samuel Thibault @ 2021-04-06 18:43 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha, Szabolcs Nagy, rrh.henry Adhemerval Zanella, le mar. 06 avril 2021 15:15:01 -0300, a ecrit: > Currently NO_RTLD_HIDDEN spills on some generic code: > > config.h.in:#undef NO_RTLD_HIDDEN > elf/dl-minimal.c:# ifndef NO_RTLD_HIDDEN > elf/dl-minimal.c:# ifndef NO_RTLD_HIDDEN > include/assert.h:# if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN) > include/sys/stat.h:# if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN) > include/unistd.h:# ifndef NO_RTLD_HIDDEN Ah, so actually I eventually did the work of marking the specific functions. That's the extra maintenance complexity I was talking about. > With the removal of NO_HIDDEN configure support, I would like to > move NO_RTLD_HIDDEN to be Hurd specific But we don't want to duplicate code either. > if possible. It's always possible to duplicate the code but that'd be forking maintenance. We could split the few NO_RTLD_HIDDEN lines in a separate header that sysdeps/mach/hurd/ could override with an empty file? It does not seem to me really making the code more readable than the ifndef... Samuel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-06 18:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-03 17:41 RFC: Remove --disable-hidden-plt H.J. Lu 2021-04-06 17:12 ` Adhemerval Zanella 2021-04-06 17:18 ` Samuel Thibault 2021-04-06 17:39 ` Adhemerval Zanella 2021-04-06 18:03 ` Samuel Thibault 2021-04-06 18:15 ` Adhemerval Zanella 2021-04-06 18:43 ` Samuel Thibault
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).