| Message ID | 20150821170918.GA29943@intel.com |
|---|---|
| State | New |
| Headers | show |
On 08/21/2015 07:09 PM, H.J. Lu wrote: > This patch adds an internal entry for __sched_getaffinity_new so that > __sched_getaffinity_old calls __sched_getaffinity_new without going > through PLT. > > OK for master? This phenomenon is not restricted to just this file. Why do you need to change this instance in particular? Thanks, Florian
On Wed, Oct 14, 2015 at 5:41 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 08/21/2015 07:09 PM, H.J. Lu wrote: >> This patch adds an internal entry for __sched_getaffinity_new so that >> __sched_getaffinity_old calls __sched_getaffinity_new without going >> through PLT. >> >> OK for master? > > This phenomenon is not restricted to just this file. Why do you need to > change this instance in particular? > I opened BZ #18822 and I will remove unnecessary PLT one by one.
On 10/14/2015 02:52 PM, H.J. Lu wrote: > On Wed, Oct 14, 2015 at 5:41 AM, Florian Weimer <fweimer@redhat.com> wrote: >> On 08/21/2015 07:09 PM, H.J. Lu wrote: >>> This patch adds an internal entry for __sched_getaffinity_new so that >>> __sched_getaffinity_old calls __sched_getaffinity_new without going >>> through PLT. >>> >>> OK for master? >> >> This phenomenon is not restricted to just this file. Why do you need to >> change this instance in particular? >> > > I opened BZ #18822 and I will remove unnecessary PLT one by one. Ah, thanks. Okay for masker. There really should be a better way to deal with this, though (similar to the static namespace issue). But perhaps not with current binutils. Florian
On Wed, 14 Oct 2015, H.J. Lu wrote: > On Wed, Oct 14, 2015 at 5:41 AM, Florian Weimer <fweimer@redhat.com> wrote: > > On 08/21/2015 07:09 PM, H.J. Lu wrote: > >> This patch adds an internal entry for __sched_getaffinity_new so that > >> __sched_getaffinity_old calls __sched_getaffinity_new without going > >> through PLT. > >> > >> OK for master? > > > > This phenomenon is not restricted to just this file. Why do you need to > > change this instance in particular? > > > > I opened BZ #18822 and I will remove unnecessary PLT one by one. Will you also add a testcase for this issue? (You'll need to allow for the relocation / relocations in question depending on the architecture, with some architectures not having any difference in relocations depending on whether functions are marked hidden and so not being able to run such a relocation-based test - it might however be possible to test in a different way not depending on names of relocations. As usual, if you make a change that doesn't include the corresponding changes for all architectures, <https://sourceware.org/glibc/wiki/PortStatus> needs updating to list non-updated architectures and describe what should be done for them.)
On Wed, Oct 14, 2015 at 8:06 AM, Joseph Myers <joseph@codesourcery.com> wrote: > On Wed, 14 Oct 2015, H.J. Lu wrote: > >> On Wed, Oct 14, 2015 at 5:41 AM, Florian Weimer <fweimer@redhat.com> wrote: >> > On 08/21/2015 07:09 PM, H.J. Lu wrote: >> >> This patch adds an internal entry for __sched_getaffinity_new so that >> >> __sched_getaffinity_old calls __sched_getaffinity_new without going >> >> through PLT. >> >> >> >> OK for master? >> > >> > This phenomenon is not restricted to just this file. Why do you need to >> > change this instance in particular? >> > >> >> I opened BZ #18822 and I will remove unnecessary PLT one by one. > > Will you also add a testcase for this issue? (You'll need to allow for > the relocation / relocations in question depending on the architecture, > with some architectures not having any difference in relocations depending > on whether functions are marked hidden and so not being able to run such a > relocation-based test - it might however be possible to test in a > different way not depending on names of relocations. As usual, if you > make a change that doesn't include the corresponding changes for all > architectures, <https://sourceware.org/glibc/wiki/PortStatus> needs > updating to list non-updated architectures and describe what should be > done for them.) I don't have a testcase yet. After removing all unnecessary PLT relocations on i386 and x86-64, we can do something similar to scripts/localplt.awk to check libc_pic.a. Each arch can provide its own list, similar to localplt.data.
On Wed, 14 Oct 2015, H.J. Lu wrote: > I don't have a testcase yet. After removing all unnecessary PLT relocations > on i386 and x86-64, we can do something similar to scripts/localplt.awk > to check libc_pic.a. Each arch can provide its own list, similar to > localplt.data. Note that this applies to all .so files installed by glibc (if a symbol should always be resolved within the same shared library, then this should be visible to the compiler by it being marked hidden at compilation time), not just libc.so. I wonder about a non-architecture-specific approach along the lines of: if an object (in *_pic.a) has a relocation against a symbol (not local to that object file) not exported from its shared library, or (for functions but not for data) exported only at a non-default version, the symbol must have hidden visibility in that object. (The special case for data symbols at non-default versions is because using aliases for data symbols is generally unsafe - it breaks the link between the library's reference to the symbol and the main executable's definition.)
Joseph Myers <joseph@codesourcery.com> writes: > Will you also add a testcase for this issue? (You'll need to allow for > the relocation / relocations in question depending on the architecture, > with some architectures not having any difference in relocations depending > on whether functions are marked hidden and so not being able to run such a > relocation-based test - it might however be possible to test in a > different way not depending on names of relocations. It's highly x86-specific. Most other architectures don't need a special register set up just for doing a PLT call, so it doesn't matter whether it's relaxed by the compiler or the linker. Andreas.
On Wed, 14 Oct 2015, Andreas Schwab wrote: > It's highly x86-specific. Most other architectures don't need a special > register set up just for doing a PLT call, so it doesn't matter whether > it's relaxed by the compiler or the linker. True - the main benefit of an architecture-independent test would be to make it less likely that people check in changes introducing new non-hidden calls on x86 because they tested on non-x86, rather than because it actually affects the generated code on other architectures.
diff --git a/sysdeps/unix/sysv/linux/sched_getaffinity.c b/sysdeps/unix/sysv/linux/sched_getaffinity.c index 9850806..a37573f 100644 --- a/sysdeps/unix/sysv/linux/sched_getaffinity.c +++ b/sysdeps/unix/sysv/linux/sched_getaffinity.c @@ -25,6 +25,11 @@ #ifdef __NR_sched_getaffinity +# if SHLIB_COMPAT (libc, GLIBC_2_3_3, GLIBC_2_3_4) +extern int __sched_getaffinity_new (pid_t, size_t, cpu_set_t *); +libc_hidden_proto (__sched_getaffinity_new) +# endif + int __sched_getaffinity_new (pid_t pid, size_t cpusetsize, cpu_set_t *cpuset) { @@ -44,6 +49,8 @@ versioned_symbol (libc, __sched_getaffinity_new, sched_getaffinity, # if SHLIB_COMPAT (libc, GLIBC_2_3_3, GLIBC_2_3_4) +libc_hidden_def (__sched_getaffinity_new) + int attribute_compat_text_section __sched_getaffinity_old (pid_t pid, cpu_set_t *cpuset)