x86-64: Make bcmp an alias of __memcmpeq
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
---
sysdeps/x86_64/memcmp.S | 2 +-
sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
sysdeps/x86_64/multiarch/memcmp.c | 2 --
sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
4 files changed, 7 insertions(+), 5 deletions(-)
Comments
On Sun, Feb 6, 2022 at 1:26 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> ---
> sysdeps/x86_64/memcmp.S | 2 +-
> sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
> sysdeps/x86_64/multiarch/memcmp.c | 2 --
> sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
> 4 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S
> index e02a53ea1e..2768d45f10 100644
> --- a/sysdeps/x86_64/memcmp.S
> +++ b/sysdeps/x86_64/memcmp.S
> @@ -405,8 +405,8 @@ END(memcmp)
>
> #ifdef USE_AS_MEMCMPEQ
> libc_hidden_def (memcmp)
> -#else
> # undef bcmp
> weak_alias (memcmp, bcmp)
> +#else
> libc_hidden_builtin_def (memcmp)
> #endif
> diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> index e10555638d..d825c13ede 100644
> --- a/sysdeps/x86_64/multiarch/memcmp-sse2.S
> +++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> @@ -29,8 +29,10 @@
> # define libc_hidden_def(ignored)
> # endif
>
> -# undef weak_alias
> -# define weak_alias(ignored1, ignored2)
> +# ifdef USE_MULTIARCH
> +# undef weak_alias
> +# define weak_alias(ignored1, ignored2)
> +# endif
>
> # undef strong_alias
> # define strong_alias(ignored1, ignored2)
> diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c
> index 7757d1ec4e..bae29d8dad 100644
> --- a/sysdeps/x86_64/multiarch/memcmp.c
> +++ b/sysdeps/x86_64/multiarch/memcmp.c
> @@ -27,8 +27,6 @@
> # include "ifunc-memcmp.h"
>
> libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
> -# undef bcmp
> -weak_alias (memcmp, bcmp)
>
> # ifdef SHARED
> __hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
> diff --git a/sysdeps/x86_64/multiarch/memcmpeq.c b/sysdeps/x86_64/multiarch/memcmpeq.c
> index aa1c8ad4de..45cb6bad5b 100644
> --- a/sysdeps/x86_64/multiarch/memcmpeq.c
> +++ b/sysdeps/x86_64/multiarch/memcmpeq.c
> @@ -27,6 +27,8 @@
> # include "ifunc-memcmpeq.h"
>
> libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
> +# undef bcmp
> +weak_alias (__memcmpeq, bcmp)
>
> # ifdef SHARED
> __hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)
> --
> 2.34.1
>
Are we sure this is safe? One of the original rationales for adding
'__memcmpeq' as
a new interface instead of just optimizing 'bcmp' was that users may
be relying on
the fact that 'bcmp' has historically returned the LT/GT of the comparison.
As well, if we are going to change 'bcmp' should we do it for all
architectures so that
its consistent?
On Sun, Feb 6, 2022 at 11:34 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Sun, Feb 6, 2022 at 1:26 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > ---
> > sysdeps/x86_64/memcmp.S | 2 +-
> > sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
> > sysdeps/x86_64/multiarch/memcmp.c | 2 --
> > sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
> > 4 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S
> > index e02a53ea1e..2768d45f10 100644
> > --- a/sysdeps/x86_64/memcmp.S
> > +++ b/sysdeps/x86_64/memcmp.S
> > @@ -405,8 +405,8 @@ END(memcmp)
> >
> > #ifdef USE_AS_MEMCMPEQ
> > libc_hidden_def (memcmp)
> > -#else
> > # undef bcmp
> > weak_alias (memcmp, bcmp)
> > +#else
> > libc_hidden_builtin_def (memcmp)
> > #endif
> > diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > index e10555638d..d825c13ede 100644
> > --- a/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > +++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > @@ -29,8 +29,10 @@
> > # define libc_hidden_def(ignored)
> > # endif
> >
> > -# undef weak_alias
> > -# define weak_alias(ignored1, ignored2)
> > +# ifdef USE_MULTIARCH
> > +# undef weak_alias
> > +# define weak_alias(ignored1, ignored2)
> > +# endif
> >
> > # undef strong_alias
> > # define strong_alias(ignored1, ignored2)
> > diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c
> > index 7757d1ec4e..bae29d8dad 100644
> > --- a/sysdeps/x86_64/multiarch/memcmp.c
> > +++ b/sysdeps/x86_64/multiarch/memcmp.c
> > @@ -27,8 +27,6 @@
> > # include "ifunc-memcmp.h"
> >
> > libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
> > -# undef bcmp
> > -weak_alias (memcmp, bcmp)
> >
> > # ifdef SHARED
> > __hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
> > diff --git a/sysdeps/x86_64/multiarch/memcmpeq.c b/sysdeps/x86_64/multiarch/memcmpeq.c
> > index aa1c8ad4de..45cb6bad5b 100644
> > --- a/sysdeps/x86_64/multiarch/memcmpeq.c
> > +++ b/sysdeps/x86_64/multiarch/memcmpeq.c
> > @@ -27,6 +27,8 @@
> > # include "ifunc-memcmpeq.h"
> >
> > libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
> > +# undef bcmp
> > +weak_alias (__memcmpeq, bcmp)
> >
> > # ifdef SHARED
> > __hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)
> > --
> > 2.34.1
> >
>
> Are we sure this is safe? One of the original rationales for adding
> '__memcmpeq' as
> a new interface instead of just optimizing 'bcmp' was that users may
> be relying on
> the fact that 'bcmp' has historically returned the LT/GT of the comparison.
The bcmp man page has:
NAME
bcmp - compare byte sequences
SYNOPSIS
#include <strings.h>
int bcmp(const void *s1, const void *s2, size_t n);
DESCRIPTION
The bcmp() function compares the two byte sequences s1 and s2 of length
n each. If they are equal, and in particular if n is zero, bcmp() re‐
turns 0. Otherwise, it returns a nonzero result.
RETURN VALUE
The bcmp() function returns 0 if the byte sequences are equal, other‐
wise a nonzero result is returned.
> As well, if we are going to change 'bcmp' should we do it for all
> architectures so that
> its consistent?
Only x86-64 has __memcmpeq which isn't an alias of memcmp.
On Sun, Feb 6, 2022 at 1:57 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sun, Feb 6, 2022 at 11:34 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > On Sun, Feb 6, 2022 at 1:26 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > ---
> > > sysdeps/x86_64/memcmp.S | 2 +-
> > > sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
> > > sysdeps/x86_64/multiarch/memcmp.c | 2 --
> > > sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
> > > 4 files changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S
> > > index e02a53ea1e..2768d45f10 100644
> > > --- a/sysdeps/x86_64/memcmp.S
> > > +++ b/sysdeps/x86_64/memcmp.S
> > > @@ -405,8 +405,8 @@ END(memcmp)
> > >
> > > #ifdef USE_AS_MEMCMPEQ
> > > libc_hidden_def (memcmp)
> > > -#else
> > > # undef bcmp
> > > weak_alias (memcmp, bcmp)
> > > +#else
> > > libc_hidden_builtin_def (memcmp)
> > > #endif
> > > diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > > index e10555638d..d825c13ede 100644
> > > --- a/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > > +++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > > @@ -29,8 +29,10 @@
> > > # define libc_hidden_def(ignored)
> > > # endif
> > >
> > > -# undef weak_alias
> > > -# define weak_alias(ignored1, ignored2)
> > > +# ifdef USE_MULTIARCH
> > > +# undef weak_alias
> > > +# define weak_alias(ignored1, ignored2)
> > > +# endif
> > >
> > > # undef strong_alias
> > > # define strong_alias(ignored1, ignored2)
> > > diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c
> > > index 7757d1ec4e..bae29d8dad 100644
> > > --- a/sysdeps/x86_64/multiarch/memcmp.c
> > > +++ b/sysdeps/x86_64/multiarch/memcmp.c
> > > @@ -27,8 +27,6 @@
> > > # include "ifunc-memcmp.h"
> > >
> > > libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
> > > -# undef bcmp
> > > -weak_alias (memcmp, bcmp)
> > >
> > > # ifdef SHARED
> > > __hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
> > > diff --git a/sysdeps/x86_64/multiarch/memcmpeq.c b/sysdeps/x86_64/multiarch/memcmpeq.c
> > > index aa1c8ad4de..45cb6bad5b 100644
> > > --- a/sysdeps/x86_64/multiarch/memcmpeq.c
> > > +++ b/sysdeps/x86_64/multiarch/memcmpeq.c
> > > @@ -27,6 +27,8 @@
> > > # include "ifunc-memcmpeq.h"
> > >
> > > libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
> > > +# undef bcmp
> > > +weak_alias (__memcmpeq, bcmp)
> > >
> > > # ifdef SHARED
> > > __hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)
> > > --
> > > 2.34.1
> > >
> >
> > Are we sure this is safe? One of the original rationales for adding
> > '__memcmpeq' as
> > a new interface instead of just optimizing 'bcmp' was that users may
> > be relying on
> > the fact that 'bcmp' has historically returned the LT/GT of the comparison.
>
> The bcmp man page has:
Si, but it may be a situation like 'memcpy' vs 'memmove'. Sure not supporting
overlap is legal by the spec but if enough users/applications rely on
the existing
functionality issues arise changing it.
Not inherently opposed to this patch (in fact in favor) but think some broader
community support given the previous reservations is needed.
>
> NAME
> bcmp - compare byte sequences
>
> SYNOPSIS
> #include <strings.h>
>
> int bcmp(const void *s1, const void *s2, size_t n);
>
> DESCRIPTION
> The bcmp() function compares the two byte sequences s1 and s2 of length
> n each. If they are equal, and in particular if n is zero, bcmp() re‐
> turns 0. Otherwise, it returns a nonzero result.
>
> RETURN VALUE
> The bcmp() function returns 0 if the byte sequences are equal, other‐
> wise a nonzero result is returned.
>
> > As well, if we are going to change 'bcmp' should we do it for all
> > architectures so that
> > its consistent?
>
> Only x86-64 has __memcmpeq which isn't an alias of memcmp.
Fair. Guess it can be added as optimized implementations come online.
>
> --
> H.J.
@@ -405,8 +405,8 @@ END(memcmp)
#ifdef USE_AS_MEMCMPEQ
libc_hidden_def (memcmp)
-#else
# undef bcmp
weak_alias (memcmp, bcmp)
+#else
libc_hidden_builtin_def (memcmp)
#endif
@@ -29,8 +29,10 @@
# define libc_hidden_def(ignored)
# endif
-# undef weak_alias
-# define weak_alias(ignored1, ignored2)
+# ifdef USE_MULTIARCH
+# undef weak_alias
+# define weak_alias(ignored1, ignored2)
+# endif
# undef strong_alias
# define strong_alias(ignored1, ignored2)
@@ -27,8 +27,6 @@
# include "ifunc-memcmp.h"
libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
-# undef bcmp
-weak_alias (memcmp, bcmp)
# ifdef SHARED
__hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
@@ -27,6 +27,8 @@
# include "ifunc-memcmpeq.h"
libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
+# undef bcmp
+weak_alias (__memcmpeq, bcmp)
# ifdef SHARED
__hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)