[v8,2/3] String: Add hidden defs for __memcmpeq() to enable internal usage
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
No bug.
This commit adds hidden defs for all declarations of __memcmpeq. This
enables usage of __memcmpeq without the PLT for usage internal to
GLIBC.
---
include/string.h | 1 +
string/memcmp.c | 1 +
sysdeps/aarch64/memcmp.S | 1 +
sysdeps/csky/abiv2/memcmp.S | 1 +
sysdeps/i386/i686/memcmp.S | 1 +
sysdeps/i386/i686/multiarch/memcmp.c | 1 +
sysdeps/i386/memcmp.S | 1 +
sysdeps/ia64/memcmp.S | 1 +
sysdeps/powerpc/powerpc32/405/memcmp.S | 1 +
sysdeps/powerpc/powerpc32/power4/memcmp.S | 1 +
sysdeps/powerpc/powerpc32/power7/memcmp.S | 1 +
sysdeps/powerpc/powerpc64/le/power10/memcmp.S | 1 +
sysdeps/powerpc/powerpc64/power4/memcmp.S | 1 +
sysdeps/powerpc/powerpc64/power7/memcmp.S | 1 +
sysdeps/powerpc/powerpc64/power8/memcmp.S | 1 +
sysdeps/s390/memcmp-z900.S | 1 +
sysdeps/s390/memcmp.c | 1 +
sysdeps/sparc/sparc64/memcmp.S | 1 +
sysdeps/x86_64/memcmp.S | 1 +
sysdeps/x86_64/multiarch/memcmp.c | 1 +
20 files changed, 20 insertions(+)
Comments
On Thu, Oct 21, 2021 at 3:39 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> No bug.
>
> This commit adds hidden defs for all declarations of __memcmpeq. This
> enables usage of __memcmpeq without the PLT for usage internal to
> GLIBC.
> ---
> include/string.h | 1 +
> string/memcmp.c | 1 +
> sysdeps/aarch64/memcmp.S | 1 +
> sysdeps/csky/abiv2/memcmp.S | 1 +
> sysdeps/i386/i686/memcmp.S | 1 +
> sysdeps/i386/i686/multiarch/memcmp.c | 1 +
> sysdeps/i386/memcmp.S | 1 +
> sysdeps/ia64/memcmp.S | 1 +
> sysdeps/powerpc/powerpc32/405/memcmp.S | 1 +
> sysdeps/powerpc/powerpc32/power4/memcmp.S | 1 +
> sysdeps/powerpc/powerpc32/power7/memcmp.S | 1 +
> sysdeps/powerpc/powerpc64/le/power10/memcmp.S | 1 +
> sysdeps/powerpc/powerpc64/power4/memcmp.S | 1 +
> sysdeps/powerpc/powerpc64/power7/memcmp.S | 1 +
> sysdeps/powerpc/powerpc64/power8/memcmp.S | 1 +
> sysdeps/s390/memcmp-z900.S | 1 +
> sysdeps/s390/memcmp.c | 1 +
> sysdeps/sparc/sparc64/memcmp.S | 1 +
> sysdeps/x86_64/memcmp.S | 1 +
> sysdeps/x86_64/multiarch/memcmp.c | 1 +
> 20 files changed, 20 insertions(+)
>
> diff --git a/include/string.h b/include/string.h
> index 81dab39891..21f641a413 100644
> --- a/include/string.h
> +++ b/include/string.h
> @@ -112,6 +112,7 @@ extern char *__strsep_g (char **__stringp, const char *__delim);
> libc_hidden_proto (__strsep_g)
> libc_hidden_proto (strnlen)
> libc_hidden_proto (__strnlen)
> +libc_hidden_proto (__memcmpeq)
> libc_hidden_proto (memmem)
> extern __typeof (memmem) __memmem;
> libc_hidden_proto (__memmem)
There are many memcmp calls in ld.so. I think most, if not all, of them
can be changed to __memcmpeq. Can you make another patch to do
that?
On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
> There are many memcmp calls in ld.so. I think most, if not all, of them
> can be changed to __memcmpeq. Can you make another patch to do
> that?
Rather than doing that micro-optimization in the glibc sources, I think
it's better to add the relevant feature to GCC and let glibc get optimized
when built with a new-enough compiler.
On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
>
> > There are many memcmp calls in ld.so. I think most, if not all, of them
> > can be changed to __memcmpeq. Can you make another patch to do
> > that?
>
> Rather than doing that micro-optimization in the glibc sources, I think
> it's better to add the relevant feature to GCC and let glibc get optimized
> when built with a new-enough compiler.
>
Why not? We don't know when __memcmpeq will be supported by GCC
used by glibc developers. If we don't even use it in glibc, why bother?
--
H.J.
On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
> On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
> >
> > > There are many memcmp calls in ld.so. I think most, if not all, of them
> > > can be changed to __memcmpeq. Can you make another patch to do
> > > that?
> >
> > Rather than doing that micro-optimization in the glibc sources, I think
> > it's better to add the relevant feature to GCC and let glibc get optimized
> > when built with a new-enough compiler.
>
> Why not? We don't know when __memcmpeq will be supported by GCC
> used by glibc developers. If we don't even use it in glibc, why bother?
The point of this function is entirely for compilers to generate for calls
from memcmp, not for humans to write direct calls to.
The compiler knows other semantics of memcmp (it might sometimes expand a
memcmp with a small fixed size inline, for example). So changing to a
direct call to __memcmpeq by hand isn't even always an optimization; it
seems better to leave the semantics visible to the compiler. Note that in
other cases we've generally moved *towards* relying on built-in functions
rather than having our own local inline expansions of those or calls to
internal names for those functions (we've done that for calls to various
functions in libm, for example).
On Thu, Oct 21, 2021 at 6:15 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
>
> > On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote:
> > >
> > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
> > >
> > > > There are many memcmp calls in ld.so. I think most, if not all, of them
> > > > can be changed to __memcmpeq. Can you make another patch to do
> > > > that?
> > >
> > > Rather than doing that micro-optimization in the glibc sources, I think
> > > it's better to add the relevant feature to GCC and let glibc get optimized
> > > when built with a new-enough compiler.
> >
> > Why not? We don't know when __memcmpeq will be supported by GCC
> > used by glibc developers. If we don't even use it in glibc, why bother?
>
> The point of this function is entirely for compilers to generate for calls
> from memcmp, not for humans to write direct calls to.
>
> The compiler knows other semantics of memcmp (it might sometimes expand a
> memcmp with a small fixed size inline, for example). So changing to a
> direct call to __memcmpeq by hand isn't even always an optimization; it
> seems better to leave the semantics visible to the compiler. Note that in
> other cases we've generally moved *towards* relying on built-in functions
> rather than having our own local inline expansions of those or calls to
> internal names for those functions (we've done that for calls to various
> functions in libm, for example).
I have gone through all non-test .c files in GLIBC and replaced memcmp
with __memcmpeq where it makes sense. I can post that as a separate
patchset and we can decide whether it's a good idea there.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
On Thu, Oct 21, 2021 at 6:39 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Thu, Oct 21, 2021 at 6:15 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
> >
> > > On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote:
> > > >
> > > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
> > > >
> > > > > There are many memcmp calls in ld.so. I think most, if not all, of them
> > > > > can be changed to __memcmpeq. Can you make another patch to do
> > > > > that?
> > > >
> > > > Rather than doing that micro-optimization in the glibc sources, I think
> > > > it's better to add the relevant feature to GCC and let glibc get optimized
> > > > when built with a new-enough compiler.
> > >
> > > Why not? We don't know when __memcmpeq will be supported by GCC
> > > used by glibc developers. If we don't even use it in glibc, why bother?
> >
> > The point of this function is entirely for compilers to generate for calls
> > from memcmp, not for humans to write direct calls to.
> >
> > The compiler knows other semantics of memcmp (it might sometimes expand a
> > memcmp with a small fixed size inline, for example). So changing to a
> > direct call to __memcmpeq by hand isn't even always an optimization;
Will compilers not implement the same optimization for __memcmpeq?
> it
> > seems better to leave the semantics visible to the compiler. Note that in
> > other cases we've generally moved *towards* relying on built-in functions
> > rather than having our own local inline expansions of those or calls to
> > internal names for those functions (we've done that for calls to various
> > functions in libm, for example).
>
> I have gone through all non-test .c files in GLIBC and replaced memcmp
> with __memcmpeq where it makes sense. I can post that as a separate
> patchset and we can decide whether it's a good idea there.
>
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com
On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote:
> I have gone through all non-test .c files in GLIBC and replaced memcmp
> with __memcmpeq where it makes sense. I can post that as a separate
> patchset and we can decide whether it's a good idea there.
I think it's a bad idea - it obfuscates the source code and it's better
just to call the standard function, leave the code more readable and let
the compiler optimize it as appropriate.
On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote:
> > > The compiler knows other semantics of memcmp (it might sometimes expand a
> > > memcmp with a small fixed size inline, for example). So changing to a
> > > direct call to __memcmpeq by hand isn't even always an optimization;
> Will compilers not implement the same optimization for __memcmpeq?
Why should they? It's intended as an ABI, not an API - that is, an
alternative assembler name the compiler name generate calls to, not a
function for which it should find calls in its input.
On Thu, Oct 21, 2021 at 5:19 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote:
>
> > > > The compiler knows other semantics of memcmp (it might sometimes expand a
> > > > memcmp with a small fixed size inline, for example). So changing to a
> > > > direct call to __memcmpeq by hand isn't even always an optimization;
> > Will compilers not implement the same optimization for __memcmpeq?
>
> Why should they? It's intended as an ABI, not an API - that is, an
> alternative assembler name the compiler name generate calls to, not a
> function for which it should find calls in its input.
>
Compiler builtin optimization doesn't work well for glibc:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67220
On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote:
> On Thu, Oct 21, 2021 at 5:19 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote:
> >
> > > > > The compiler knows other semantics of memcmp (it might sometimes expand a
> > > > > memcmp with a small fixed size inline, for example). So changing to a
> > > > > direct call to __memcmpeq by hand isn't even always an optimization;
> > > Will compilers not implement the same optimization for __memcmpeq?
> >
> > Why should they? It's intended as an ABI, not an API - that is, an
> > alternative assembler name the compiler name generate calls to, not a
> > function for which it should find calls in its input.
> >
>
> Compiler builtin optimization doesn't work well for glibc:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67220
1. So let's fix that in the compiler. This doesn't seem like something so
hard to fix that it should justify obfuscating the source code in glibc by
putting in unnecessary direct calls to an internal name.
2. If we need a single redirection of __memcmpeq to the hidden alias
__GI___memcmpeq, we could certainly put that in
sysdeps/generic/symbol-hacks.h as we do for a few other functions. A
redirection like that in one place isn't problematic the same way putting
in lots of direct calls to an internal name in the source code (for
something better optimized in the compiler) is.
@@ -112,6 +112,7 @@ extern char *__strsep_g (char **__stringp, const char *__delim);
libc_hidden_proto (__strsep_g)
libc_hidden_proto (strnlen)
libc_hidden_proto (__strnlen)
+libc_hidden_proto (__memcmpeq)
libc_hidden_proto (memmem)
extern __typeof (memmem) __memmem;
libc_hidden_proto (__memmem)
@@ -360,4 +360,5 @@ libc_hidden_builtin_def(memcmp)
weak_alias (memcmp, bcmp)
# undef __memcmpeq
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
#endif
@@ -180,3 +180,4 @@ weak_alias (memcmp, bcmp)
#undef __memcmpeq
weak_alias (memcmp, __memcmpeq)
libc_hidden_builtin_def (memcmp)
+libc_hidden_def (__memcmpeq)
@@ -140,4 +140,5 @@ END (memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
libc_hidden_def (memcmp)
+libc_hidden_def (__memcmpeq)
.weak memcmp
@@ -408,3 +408,4 @@ weak_alias (memcmp, bcmp)
#undef __memcmpeq
weak_alias (memcmp, __memcmpeq)
libc_hidden_builtin_def (memcmp)
+libc_hidden_def (__memcmpeq)
@@ -30,4 +30,5 @@ libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
#endif
@@ -73,3 +73,4 @@ weak_alias (memcmp, bcmp)
#undef __memcmpeq
weak_alias (memcmp, __memcmpeq)
libc_hidden_builtin_def (memcmp)
+libc_hidden_def (__memcmpeq)
@@ -162,3 +162,4 @@ END(memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
libc_hidden_builtin_def (memcmp)
+libc_hidden_def (__memcmpeq)
@@ -127,3 +127,4 @@ END (memcmp)
libc_hidden_builtin_def (memcmp)
weak_alias (memcmp,bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
@@ -1374,3 +1374,4 @@ END (memcmp)
libc_hidden_builtin_def (memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
@@ -1374,3 +1374,4 @@ END (memcmp)
libc_hidden_builtin_def (memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
@@ -178,3 +178,4 @@ END (MEMCMP)
libc_hidden_builtin_def (memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
@@ -1375,3 +1375,4 @@ END (MEMCMP)
libc_hidden_builtin_def (memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
@@ -1060,3 +1060,4 @@ END (MEMCMP)
libc_hidden_builtin_def (memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
@@ -1443,3 +1443,4 @@ END (MEMCMP)
libc_hidden_builtin_def (memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
@@ -165,6 +165,7 @@ END(MEMCMP_Z196)
strong_alias (MEMCMP_DEFAULT, memcmp)
weak_alias (memcmp, bcmp)
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
#endif
#if defined SHARED && IS_IN (libc)
@@ -47,4 +47,5 @@ s390_libc_ifunc_expr (__redirect_memcmp, memcmp,
)
weak_alias (memcmp, bcmp);
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
#endif
@@ -140,3 +140,4 @@ weak_alias (memcmp, bcmp)
#undef __memcmpeq
weak_alias (memcmp, __memcmpeq)
libc_hidden_builtin_def (memcmp)
+libc_hidden_def (__memcmpeq)
@@ -361,3 +361,4 @@ weak_alias (memcmp, bcmp)
#undef __memcmpeq
weak_alias (memcmp, __memcmpeq)
libc_hidden_builtin_def (memcmp)
+libc_hidden_def (__memcmpeq)
@@ -31,6 +31,7 @@ libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
weak_alias (memcmp, bcmp)
# undef __memcmpeq
weak_alias (memcmp, __memcmpeq)
+libc_hidden_def (__memcmpeq)
# ifdef SHARED
__hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)