[PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Nathan Chancellor]

After LLVM revision r355672 [1], all known working kernel configurations
fail to link [2]:

ld: init/do_mounts.o: in function `prepare_namespace':
do_mounts.c:(.init.text+0x5ca): undefined reference to `bcmp'
ld: do_mounts.c:(.init.text+0x5e6): undefined reference to `bcmp'
ld: init/initramfs.o: in function `do_header':
initramfs.c:(.init.text+0x6e0): undefined reference to `bcmp'
ld: initramfs.c:(.init.text+0x6f8): undefined reference to `bcmp'
ld: arch/x86/kernel/setup.o: in function `setup_arch':
setup.c:(.init.text+0x21d): undefined reference to `bcmp'

Commit 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to
builtin") removed '-ffreestanding' globally and the kernel doesn't
provide a bcmp definition so the linker cannot find a reference to it.

Fix this by explicitly telling LLVM through Clang not to emit bcmp
references. This flag does not need to be behind 'cc-option' because all
working versions of Clang support this flag.

[1]: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
[2]: https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/104027249

Link: https://github.com/ClangBuiltLinux/linux/issues/416
Link: https://bugs.llvm.org/show_bug.cgi?id=41035
Cc: stable@vger.kernel.org
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 9ef547fc7ffe..6645a274b6e3 100644
--- a/Makefile
+++ b/Makefile
@@ -501,6 +501,7 @@ ifneq ($(GCC_TOOLCHAIN),)
 CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
 endif
 CLANG_FLAGS	+= -no-integrated-as
+CLANG_FLAGS	+= -fno-builtin-bcmp
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)
 export CLANG_FLAGS
-- 
2.21.0

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Nick Desaulniers]

On Tue, Mar 12, 2019 at 2:53 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> After LLVM revision r355672 [1], all known working kernel configurations
> fail to link [2]:
>
> ld: init/do_mounts.o: in function `prepare_namespace':
> do_mounts.c:(.init.text+0x5ca): undefined reference to `bcmp'
> ld: do_mounts.c:(.init.text+0x5e6): undefined reference to `bcmp'
> ld: init/initramfs.o: in function `do_header':
> initramfs.c:(.init.text+0x6e0): undefined reference to `bcmp'
> ld: initramfs.c:(.init.text+0x6f8): undefined reference to `bcmp'
> ld: arch/x86/kernel/setup.o: in function `setup_arch':
> setup.c:(.init.text+0x21d): undefined reference to `bcmp'
>
> Commit 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to
> builtin") removed '-ffreestanding' globally and the kernel doesn't
> provide a bcmp definition so the linker cannot find a reference to it.
>
> Fix this by explicitly telling LLVM through Clang not to emit bcmp
> references. This flag does not need to be behind 'cc-option' because all
> working versions of Clang support this flag.
>
> [1]: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
> [2]: https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/104027249
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/416
> Link: https://bugs.llvm.org/show_bug.cgi?id=41035
> Cc: stable@vger.kernel.org
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

Thanks for this patch.  Can the maintainers please consider this an
emergency patch; without it, the recent change to LLVM has caused ALL
of our CI targets to go red.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>


> ---
>  Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Makefile b/Makefile
> index 9ef547fc7ffe..6645a274b6e3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -501,6 +501,7 @@ ifneq ($(GCC_TOOLCHAIN),)
>  CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  CLANG_FLAGS    += -no-integrated-as
> +CLANG_FLAGS    += -fno-builtin-bcmp
>  KBUILD_CFLAGS  += $(CLANG_FLAGS)
>  KBUILD_AFLAGS  += $(CLANG_FLAGS)
>  export CLANG_FLAGS
> --
> 2.21.0
>


--
Thanks,
~Nick Desaulniers

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Rasmus Villemoes]

On 12/03/2019 22.52, Nathan Chancellor wrote:
> After LLVM revision r355672 [1], all known working kernel configurations
> fail to link [2]:
> 
> ld: init/do_mounts.o: in function `prepare_namespace':
> do_mounts.c:(.init.text+0x5ca): undefined reference to `bcmp'
> ld: do_mounts.c:(.init.text+0x5e6): undefined reference to `bcmp'
> ld: init/initramfs.o: in function `do_header':
> initramfs.c:(.init.text+0x6e0): undefined reference to `bcmp'
> ld: initramfs.c:(.init.text+0x6f8): undefined reference to `bcmp'
> ld: arch/x86/kernel/setup.o: in function `setup_arch':
> setup.c:(.init.text+0x21d): undefined reference to `bcmp'
> 
> Commit 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to
> builtin") removed '-ffreestanding' globally and the kernel doesn't
> provide a bcmp definition so the linker cannot find a reference to it.
>

> Fix this by explicitly telling LLVM through Clang not to emit bcmp
> references. This flag does not need to be behind 'cc-option' because all
> working versions of Clang support this flag.

Wouldn't it be better to just define bcmp as an alias for memcmp? They
seem to have compatible prototypes, and then somebody might someday sit
down and implement some word-at-a-time version of bcmp making use of the
weaker guarantees about the return value to gain some performance. But I
suppose that can also be done later.

Rasmus

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Masahiro Yamada]

On Wed, Mar 13, 2019 at 5:13 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 12/03/2019 22.52, Nathan Chancellor wrote:
> > After LLVM revision r355672 [1], all known working kernel configurations
> > fail to link [2]:
> >
> > ld: init/do_mounts.o: in function `prepare_namespace':
> > do_mounts.c:(.init.text+0x5ca): undefined reference to `bcmp'
> > ld: do_mounts.c:(.init.text+0x5e6): undefined reference to `bcmp'
> > ld: init/initramfs.o: in function `do_header':
> > initramfs.c:(.init.text+0x6e0): undefined reference to `bcmp'
> > ld: initramfs.c:(.init.text+0x6f8): undefined reference to `bcmp'
> > ld: arch/x86/kernel/setup.o: in function `setup_arch':
> > setup.c:(.init.text+0x21d): undefined reference to `bcmp'
> >
> > Commit 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to
> > builtin") removed '-ffreestanding' globally and the kernel doesn't
> > provide a bcmp definition so the linker cannot find a reference to it.
> >
>
> > Fix this by explicitly telling LLVM through Clang not to emit bcmp
> > references. This flag does not need to be behind 'cc-option' because all
> > working versions of Clang support this flag.
>
> Wouldn't it be better to just define bcmp as an alias for memcmp? They
> seem to have compatible prototypes, and then somebody might someday sit
> down and implement some word-at-a-time version of bcmp making use of the
> weaker guarantees about the return value to gain some performance. But I
> suppose that can also be done later.


I also thought of this.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Nathan Chancellor]

On Wed, Mar 13, 2019 at 09:13:11AM +0100, Rasmus Villemoes wrote:
> On 12/03/2019 22.52, Nathan Chancellor wrote:
> > After LLVM revision r355672 [1], all known working kernel configurations
> > fail to link [2]:
> > 
> > ld: init/do_mounts.o: in function `prepare_namespace':
> > do_mounts.c:(.init.text+0x5ca): undefined reference to `bcmp'
> > ld: do_mounts.c:(.init.text+0x5e6): undefined reference to `bcmp'
> > ld: init/initramfs.o: in function `do_header':
> > initramfs.c:(.init.text+0x6e0): undefined reference to `bcmp'
> > ld: initramfs.c:(.init.text+0x6f8): undefined reference to `bcmp'
> > ld: arch/x86/kernel/setup.o: in function `setup_arch':
> > setup.c:(.init.text+0x21d): undefined reference to `bcmp'
> > 
> > Commit 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to
> > builtin") removed '-ffreestanding' globally and the kernel doesn't
> > provide a bcmp definition so the linker cannot find a reference to it.
> >
> 
> > Fix this by explicitly telling LLVM through Clang not to emit bcmp
> > references. This flag does not need to be behind 'cc-option' because all
> > working versions of Clang support this flag.
> 
> Wouldn't it be better to just define bcmp as an alias for memcmp? They
> seem to have compatible prototypes, and then somebody might someday sit
> down and implement some word-at-a-time version of bcmp making use of the
> weaker guarantees about the return value to gain some performance. But I
> suppose that can also be done later.
> 
> Rasmus

Hi Rasmus,

Thank you much for the review, I didn't even realize this was possible :)

I'd certainly like to explore it as that is what glibc does. How would
you suggest going about it here?

Thanks,
Nathan

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Arnd Bergmann]

On Wed, Mar 13, 2019 at 2:44 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
> On Wed, Mar 13, 2019 at 09:13:11AM +0100, Rasmus Villemoes wrote:
> > Wouldn't it be better to just define bcmp as an alias for memcmp? They
> > seem to have compatible prototypes, and then somebody might someday sit
> > down and implement some word-at-a-time version of bcmp making use of the
> > weaker guarantees about the return value to gain some performance. But I
> > suppose that can also be done later.
>
> Thank you much for the review, I didn't even realize this was possible :)
>
> I'd certainly like to explore it as that is what glibc does. How would
> you suggest going about it here?

I suggested a possible implementation (likely contains bugs) and
an alias for architectures that require strict alignment, see
https://bugs.llvm.org/show_bug.cgi?id=41035#c11

We could start out with just the alias.

       Arnd

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Nathan Chancellor]

On Wed, Mar 13, 2019 at 02:48:49PM +0100, Arnd Bergmann wrote:
> On Wed, Mar 13, 2019 at 2:44 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> > On Wed, Mar 13, 2019 at 09:13:11AM +0100, Rasmus Villemoes wrote:
> > > Wouldn't it be better to just define bcmp as an alias for memcmp? They
> > > seem to have compatible prototypes, and then somebody might someday sit
> > > down and implement some word-at-a-time version of bcmp making use of the
> > > weaker guarantees about the return value to gain some performance. But I
> > > suppose that can also be done later.
> >
> > Thank you much for the review, I didn't even realize this was possible :)
> >
> > I'd certainly like to explore it as that is what glibc does. How would
> > you suggest going about it here?
> 
> I suggested a possible implementation (likely contains bugs) and
> an alias for architectures that require strict alignment, see
> https://bugs.llvm.org/show_bug.cgi?id=41035#c11
> 
> We could start out with just the alias.
> 
>        Arnd

So I've been messing around with this for a bit (forgive me, I'm still
learning all of the intricacies around here) and this is what I came up
with for when __ARCH_HAVE_MEMCMP is unset (not particularly difficult
obviously). I can compile, link, and boot an x86 in QEMU.

However, I am not sure how to handle memcmp definitions that are written
in assembly like arm64, as the alias attribute only works when the alias
is defined in the same translation unit. Thoughts?

Cheers,
Nathan

========================================

diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..69a9daca9179 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -864,6 +864,9 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
        return res;
 }
 EXPORT_SYMBOL(memcmp);
+
+int bcmp(const void *cs, const void *ct, size_t count) __weak __alias(memcmp);
+EXPORT_SYMBOL(bcmp);
 #endif
 
 #ifndef __HAVE_ARCH_MEMSCAN

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Nick Desaulniers]

On Wed, Mar 13, 2019 at 8:32 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Wed, Mar 13, 2019 at 02:48:49PM +0100, Arnd Bergmann wrote:
> > On Wed, Mar 13, 2019 at 2:44 PM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > > On Wed, Mar 13, 2019 at 09:13:11AM +0100, Rasmus Villemoes wrote:
> > > > Wouldn't it be better to just define bcmp as an alias for memcmp? They
> > > > seem to have compatible prototypes, and then somebody might someday sit
> > > > down and implement some word-at-a-time version of bcmp making use of the
> > > > weaker guarantees about the return value to gain some performance. But I
> > > > suppose that can also be done later.
> > >
> > > Thank you much for the review, I didn't even realize this was possible :)
> > >
> > > I'd certainly like to explore it as that is what glibc does. How would
> > > you suggest going about it here?
> >
> > I suggested a possible implementation (likely contains bugs) and
> > an alias for architectures that require strict alignment, see
> > https://bugs.llvm.org/show_bug.cgi?id=41035#c11
> >
> > We could start out with just the alias.
> >
> >        Arnd
>
> So I've been messing around with this for a bit (forgive me, I'm still
> learning all of the intricacies around here) and this is what I came up
> with for when __ARCH_HAVE_MEMCMP is unset (not particularly difficult
> obviously). I can compile, link, and boot an x86 in QEMU.
>
> However, I am not sure how to handle memcmp definitions that are written
> in assembly like arm64, as the alias attribute only works when the alias
> is defined in the same translation unit. Thoughts?

I hit this, too:
./arch/arm64/include/asm/string.h:40:15: error: alias must point to a
defined variable
      or function

since memcmp is only declared (not defined) in that header, clang is
not happy to alias to memcmp.  If __HAVE_ARCH_MEMCMP is defined, then
we can just return a call to memcmp.  Thoughts (I need to add comments
above bcmp, anything else)?  Do we like the typeof trick (stolen from
glibc) or no?

diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..e6c1954f2716 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -845,7 +845,13 @@ void *memmove(void *dest, const void *src, size_t count)
 EXPORT_SYMBOL(memmove);
 #endif

-#ifndef __HAVE_ARCH_MEMCMP
+#ifdef __HAVE_ARCH_MEMCMP
+int bcmp(const void *cs, const void *ct, size_t n)
+{
+       return memcmp(cs, ct, n);
+}
+EXPORT_SYMBOL(bcmp);
+#else
 /**
  * memcmp - Compare two areas of memory
  * @cs: One area of memory
@@ -864,6 +870,8 @@ __visible int memcmp(const void *cs, const void
*ct, size_t count)
        return res;
 }
 EXPORT_SYMBOL(memcmp);
+__weak __alias(memcmp) typeof(memcmp) bcmp;
+EXPORT_SYMBOL(bcmp);
 #endif

 #ifndef __HAVE_ARCH_MEMSCAN


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Nick Desaulniers]

On Wed, Mar 13, 2019 at 10:21 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Mar 13, 2019 at 8:32 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Wed, Mar 13, 2019 at 02:48:49PM +0100, Arnd Bergmann wrote:
> > > On Wed, Mar 13, 2019 at 2:44 PM Nathan Chancellor
> > > <natechancellor@gmail.com> wrote:
> > > > On Wed, Mar 13, 2019 at 09:13:11AM +0100, Rasmus Villemoes wrote:
> > > > > Wouldn't it be better to just define bcmp as an alias for memcmp? They
> > > > > seem to have compatible prototypes, and then somebody might someday sit
> > > > > down and implement some word-at-a-time version of bcmp making use of the
> > > > > weaker guarantees about the return value to gain some performance. But I
> > > > > suppose that can also be done later.
> > > >
> > > > Thank you much for the review, I didn't even realize this was possible :)
> > > >
> > > > I'd certainly like to explore it as that is what glibc does. How would
> > > > you suggest going about it here?
> > >
> > > I suggested a possible implementation (likely contains bugs) and
> > > an alias for architectures that require strict alignment, see
> > > https://bugs.llvm.org/show_bug.cgi?id=41035#c11
> > >
> > > We could start out with just the alias.
> > >
> > >        Arnd
> >
> > So I've been messing around with this for a bit (forgive me, I'm still
> > learning all of the intricacies around here) and this is what I came up
> > with for when __ARCH_HAVE_MEMCMP is unset (not particularly difficult
> > obviously). I can compile, link, and boot an x86 in QEMU.
> >
> > However, I am not sure how to handle memcmp definitions that are written
> > in assembly like arm64, as the alias attribute only works when the alias
> > is defined in the same translation unit. Thoughts?
>
> I hit this, too:
> ./arch/arm64/include/asm/string.h:40:15: error: alias must point to a
> defined variable
>       or function
>
> since memcmp is only declared (not defined) in that header, clang is
> not happy to alias to memcmp.  If __HAVE_ARCH_MEMCMP is defined, then
> we can just return a call to memcmp.  Thoughts (I need to add comments
> above bcmp, anything else)?  Do we like the typeof trick (stolen from
> glibc) or no?
>
> diff --git a/lib/string.c b/lib/string.c
> index 38e4ca08e757..e6c1954f2716 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -845,7 +845,13 @@ void *memmove(void *dest, const void *src, size_t count)
>  EXPORT_SYMBOL(memmove);
>  #endif
>
> -#ifndef __HAVE_ARCH_MEMCMP
> +#ifdef __HAVE_ARCH_MEMCMP
> +int bcmp(const void *cs, const void *ct, size_t n)
> +{
> +       return memcmp(cs, ct, n);
> +}
> +EXPORT_SYMBOL(bcmp);
> +#else
>  /**
>   * memcmp - Compare two areas of memory
>   * @cs: One area of memory
> @@ -864,6 +870,8 @@ __visible int memcmp(const void *cs, const void
> *ct, size_t count)
>         return res;
>  }
>  EXPORT_SYMBOL(memcmp);
> +__weak __alias(memcmp) typeof(memcmp) bcmp;
> +EXPORT_SYMBOL(bcmp);
>  #endif
>
>  #ifndef __HAVE_ARCH_MEMSCAN

Alternatively, just not worrying about __alias makes this simpler and
seems to work (need to add comments, thoughts?):


diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..2112108ecc35 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -866,6 +866,15 @@ __visible int memcmp(const void *cs, const void
*ct, size_t count)
 EXPORT_SYMBOL(memcmp);
 #endif

+#ifndef __HAVE_ARCH_BCMP
+#undef bcmp
+int bcmp(const void *cs, const void *ct, size_t count)
+{
+       return memcmp(cs, ct, count);
+}
+EXPORT_SYMBOL(bcmp);
+#endif
+
 #ifndef __HAVE_ARCH_MEMSCAN
 /**
  * memscan - Find a character in an area of memory.

-- 
Thanks,
~Nick Desaulniers

[PATCH] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

A recent optimization in Clang (r355672) lowers comparisons of the
return value of memcmp against zero to comparisons of the return value
of bcmp against zero.  This helps some platforms that implement bcmp
more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
an optimized implementation is in the works.

This results in linkage failures for all targets with Clang due to the
undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
to unbreak the build.  This routine can be further optimized in the
future.

Other ideas discussed:
* A weak alias was discussed, but breaks for architectures that define
their own implementations of memcmp since aliases to declarations are
not permitted (only definitions).  Arch-specific memcmp implementations
typically declare memcmp in C headers, but implement them in assembly.
* -ffreestanding also is used sporadically throughout the kernel.
* -fno-builtin-bcmp doesn't work when doing LTO.

Link: https://bugs.llvm.org/show_bug.cgi?id=41035
Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
Link: https://github.com/ClangBuiltLinux/linux/issues/416
Cc: stable@vger.kernel.org
Reported-by: Nathan Chancellor <natechancellor@gmail.com>
Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: James Y Knight <jyknight@google.com>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 lib/string.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..5b2377d7143f 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -866,6 +866,21 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
 EXPORT_SYMBOL(memcmp);
 #endif
 
+#ifndef __HAVE_ARCH_BCMP
+/**
+ * bcmp - Like memcmp but the return code simply indicates a non-match.
+ * @cs: One area of memory.
+ * @ct: Another area of memory.
+ * @count: The size of the areas.
+ */
+#undef bcmp
+int bcmp(const void *cs, const void *ct, size_t count)
+{
+	return memcmp(cs, ct, count);
+}
+EXPORT_SYMBOL(bcmp);
+#endif
+
 #ifndef __HAVE_ARCH_MEMSCAN
 /**
  * memscan - Find a character in an area of memory.
-- 
2.21.0.360.g471c308f928-goog

Re: [PATCH] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

On Wed, Mar 13, 2019 at 11:02 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> A recent optimization in Clang (r355672) lowers comparisons of the
> return value of memcmp against zero to comparisons of the return value
> of bcmp against zero.  This helps some platforms that implement bcmp
> more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
> an optimized implementation is in the works.
>
> This results in linkage failures for all targets with Clang due to the
> undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
> to unbreak the build.  This routine can be further optimized in the
> future.
>
> Other ideas discussed:
> * A weak alias was discussed, but breaks for architectures that define
> their own implementations of memcmp since aliases to declarations are
> not permitted (only definitions).  Arch-specific memcmp implementations
> typically declare memcmp in C headers, but implement them in assembly.
> * -ffreestanding also is used sporadically throughout the kernel.
> * -fno-builtin-bcmp doesn't work when doing LTO.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=41035
> Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
> Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
> Link: https://github.com/ClangBuiltLinux/linux/issues/416
> Cc: stable@vger.kernel.org
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: James Y Knight <jyknight@google.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  lib/string.c | 15 +++++++++++++++

ah, looks like I should provide a declaration in
include/linux/string.h, too, in case others within the kernel would
like to use bcmp.  WIll send a v2.

>  1 file changed, 15 insertions(+)
>
> diff --git a/lib/string.c b/lib/string.c
> index 38e4ca08e757..5b2377d7143f 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -866,6 +866,21 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
>  EXPORT_SYMBOL(memcmp);
>  #endif
>
> +#ifndef __HAVE_ARCH_BCMP
> +/**
> + * bcmp - Like memcmp but the return code simply indicates a non-match.
> + * @cs: One area of memory.
> + * @ct: Another area of memory.
> + * @count: The size of the areas.
> + */
> +#undef bcmp
> +int bcmp(const void *cs, const void *ct, size_t count)
> +{
> +       return memcmp(cs, ct, count);
> +}
> +EXPORT_SYMBOL(bcmp);
> +#endif
> +
>  #ifndef __HAVE_ARCH_MEMSCAN
>  /**
>   * memscan - Find a character in an area of memory.
> --
> 2.21.0.360.g471c308f928-goog
>


-- 
Thanks,
~Nick Desaulniers

[PATCH v2] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

A recent optimization in Clang (r355672) lowers comparisons of the
return value of memcmp against zero to comparisons of the return value
of bcmp against zero.  This helps some platforms that implement bcmp
more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
an optimized implementation is in the works.

This results in linkage failures for all targets with Clang due to the
undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
to unbreak the build.  This routine can be further optimized in the
future.

Other ideas discussed:
* A weak alias was discussed, but breaks for architectures that define
their own implementations of memcmp since aliases to declarations are
not permitted (only definitions).  Arch-specific memcmp implementations
typically declare memcmp in C headers, but implement them in assembly.
* -ffreestanding also is used sporadically throughout the kernel.
* -fno-builtin-bcmp doesn't work when doing LTO.

Link: https://bugs.llvm.org/show_bug.cgi?id=41035
Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
Link: https://github.com/ClangBuiltLinux/linux/issues/416
Cc: stable@vger.kernel.org
Reported-by: Nathan Chancellor <natechancellor@gmail.com>
Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: James Y Knight <jyknight@google.com>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1 -> V2:
* Add declaration to include/linux/string.h.
* Reword comment above bcmp.

 include/linux/string.h |  3 +++
 lib/string.c           | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 7927b875f80c..6ab0a6fa512e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
 #ifndef __HAVE_ARCH_MEMCMP
 extern int memcmp(const void *,const void *,__kernel_size_t);
 #endif
+#ifndef __HAVE_ARCH_BCMP
+extern int bcmp(const void *,const void *,__kernel_size_t);
+#endif
 #ifndef __HAVE_ARCH_MEMCHR
 extern void * memchr(const void *,int,__kernel_size_t);
 #endif
diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..b882692bac04 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -866,6 +866,21 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
 EXPORT_SYMBOL(memcmp);
 #endif
 
+#ifndef __HAVE_ARCH_BCMP
+/**
+ * bcmp - Like memcmp but a non-zero return code simply indicates a non-match.
+ * @cs: One area of memory.
+ * @ct: Another area of memory.
+ * @count: The size of the areas.
+ */
+#undef bcmp
+int bcmp(const void *cs, const void *ct, size_t count)
+{
+	return memcmp(cs, ct, count);
+}
+EXPORT_SYMBOL(bcmp);
+#endif
+
 #ifndef __HAVE_ARCH_MEMSCAN
 /**
  * memscan - Find a character in an area of memory.
-- 
2.21.0.360.g471c308f928-goog

Re: [PATCH v2] lib/string.c: implement a basic bcmp

[Steven Rostedt]

On Wed, 13 Mar 2019 11:17:15 -0700
Nick Desaulniers <ndesaulniers@google.com> wrote:

> +#ifndef __HAVE_ARCH_BCMP
> +/**
> + * bcmp - Like memcmp but a non-zero return code simply indicates a non-match.
> + * @cs: One area of memory.
> + * @ct: Another area of memory.
> + * @count: The size of the areas.
> + */
> +#undef bcmp
> +int bcmp(const void *cs, const void *ct, size_t count)
> +{
> +	return memcmp(cs, ct, count);

This is confusing where the comment says "like memcmp but .." and then
just returns memcmp() unmodified. If anything, I would expect to see

	return !!memcmp(cs, ct, conut);

or have a better comment explaining why its the same.

-- Steve

> +}
> +EXPORT_SYMBOL(bcmp);
> +#endif
> +

Re: [PATCH v2] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

On Wed, Mar 13, 2019 at 11:40 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 13 Mar 2019 11:17:15 -0700
> Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> > +#ifndef __HAVE_ARCH_BCMP
> > +/**
> > + * bcmp - Like memcmp but a non-zero return code simply indicates a non-match.
> > + * @cs: One area of memory.
> > + * @ct: Another area of memory.
> > + * @count: The size of the areas.
> > + */
> > +#undef bcmp
> > +int bcmp(const void *cs, const void *ct, size_t count)
> > +{
> > +     return memcmp(cs, ct, count);
>
> This is confusing where the comment says "like memcmp but .." and then
> just returns memcmp() unmodified. If anything, I would expect to see
>
>         return !!memcmp(cs, ct, conut);

That's more work than strictly needed. memcmp already provides the
semantics of bcmp.  memcmp just provides more meaning to the
signedness of the return code, whereas bcmp does not.

>
> or have a better comment explaining why its the same.

I could add something about "the signedness of the return code not
providing any meaning."  What would you like to see in such a comment?
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] lib/string.c: implement a basic bcmp

[Steven Rostedt]

On Wed, 13 Mar 2019 11:51:09 -0700
Nick Desaulniers <ndesaulniers@google.com> wrote:


> > This is confusing where the comment says "like memcmp but .." and then
> > just returns memcmp() unmodified. If anything, I would expect to see
> >
> >         return !!memcmp(cs, ct, conut);  
> 
> That's more work than strictly needed. memcmp already provides the
> semantics of bcmp.  memcmp just provides more meaning to the
> signedness of the return code, whereas bcmp does not.

I figured you would say as much ;-)

> 
> >
> > or have a better comment explaining why its the same.  
> 
> I could add something about "the signedness of the return code not
> providing any meaning."  What would you like to see in such a comment?

I think it's the wording that bothers me:

+ * bcmp - Like memcmp but a non-zero return code simply indicates a non-match.

What about:

  * bcmp - Like memcmp but non-zero only means a non-match

Then in the description say that bcmp() callers must not expect
anything more than zero and non-zero, as different implementations only
need to return non-zero for non matches. The non-zero has no other
meaning like it does in memcmp(). You could add that memcmp() itself is
one implementation of bcmp() but not vice versa.

-- Steve

Re: [PATCH v2] lib/string.c: implement a basic bcmp

[Rasmus Villemoes]

On 13/03/2019 20.01, Steven Rostedt wrote:
> On Wed, 13 Mar 2019 11:51:09 -0700
> Nick Desaulniers <ndesaulniers@google.com> wrote:
> 
>>>
>>> or have a better comment explaining why its the same.  
>>
>> I could add something about "the signedness of the return code not
>> providing any meaning."  What would you like to see in such a comment?
> 
> I think it's the wording that bothers me:
> 
> + * bcmp - Like memcmp but a non-zero return code simply indicates a non-match.
> 
> What about:
> 
>   * bcmp - Like memcmp but non-zero only means a non-match
> 
> Then in the description say that bcmp() callers must not expect
> anything more than zero and non-zero,

Yes, but let's completely avoid mentioning memcmp in the summary.

bcmp - return 0 if and only if the buffers have identical contents
@a: pointer to first buffer
@b: pointer to second buffer
@len: size of buffers

The sign or magnitude of a non-zero return value has no particular
meaning, and architectures may implement their own more efficient
bcmp(). So while this particular implementation is a simple (tail) call
to memcmp, do not rely on anything but whether the return value is zero
or non-zero.

Rasmus

Re: [PATCH v2] lib/string.c: implement a basic bcmp

[Steven Rostedt]

On Wed, 13 Mar 2019 20:34:11 +0100
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> Yes, but let's completely avoid mentioning memcmp in the summary.
> 
> bcmp - return 0 if and only if the buffers have identical contents
> @a: pointer to first buffer
> @b: pointer to second buffer
> @len: size of buffers
> 
> The sign or magnitude of a non-zero return value has no particular
> meaning, and architectures may implement their own more efficient
> bcmp(). So while this particular implementation is a simple (tail) call
> to memcmp, do not rely on anything but whether the return value is zero
> or non-zero.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Thanks!

-- Steve

[PATCH v3] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

A recent optimization in Clang (r355672) lowers comparisons of the
return value of memcmp against zero to comparisons of the return value
of bcmp against zero.  This helps some platforms that implement bcmp
more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
an optimized implementation is in the works.

This results in linkage failures for all targets with Clang due to the
undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
to unbreak the build.  This routine can be further optimized in the
future.

Other ideas discussed:
* A weak alias was discussed, but breaks for architectures that define
their own implementations of memcmp since aliases to declarations are
not permitted (only definitions).  Arch-specific memcmp implementations
typically declare memcmp in C headers, but implement them in assembly.
* -ffreestanding also is used sporadically throughout the kernel.
* -fno-builtin-bcmp doesn't work when doing LTO.

Link: https://bugs.llvm.org/show_bug.cgi?id=41035
Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
Link: https://github.com/ClangBuiltLinux/linux/issues/416
Cc: stable@vger.kernel.org
Reported-by: Nathan Chancellor <natechancellor@gmail.com>
Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: James Y Knight <jyknight@google.com>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Changes V2 -> V3:
* Adjust comment as per Steven to Rasmus' sugguestion.
* Pick up Steven's Ack.
Changes V1 -> V2:
* Add declaration to include/linux/string.h.
* Reword comment above bcmp.

 include/linux/string.h |  3 +++
 lib/string.c           | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 7927b875f80c..6ab0a6fa512e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
 #ifndef __HAVE_ARCH_MEMCMP
 extern int memcmp(const void *,const void *,__kernel_size_t);
 #endif
+#ifndef __HAVE_ARCH_BCMP
+extern int bcmp(const void *,const void *,__kernel_size_t);
+#endif
 #ifndef __HAVE_ARCH_MEMCHR
 extern void * memchr(const void *,int,__kernel_size_t);
 #endif
diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..48a43a42d3f2 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -866,6 +866,21 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
 EXPORT_SYMBOL(memcmp);
 #endif
 
+#ifndef __HAVE_ARCH_BCMP
+/**
+ * bcmp - returns 0 if and only if the buffers have identical contents.
+ * @cs: One area of memory.
+ * @ct: Another area of memory.
+ * @count: The size of the areas.
+ */
+#undef bcmp
+int bcmp(const void *cs, const void *ct, size_t count)
+{
+	return memcmp(cs, ct, count);
+}
+EXPORT_SYMBOL(bcmp);
+#endif
+
 #ifndef __HAVE_ARCH_MEMSCAN
 /**
  * memscan - Find a character in an area of memory.
-- 
2.21.0.360.g471c308f928-goog

Re: [PATCH v3] lib/string.c: implement a basic bcmp

[Steven Rostedt]

On March 13, 2019 4:37:37 PM EDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
>A recent optimization in Clang (r355672) lowers comparisons of the
>return value of memcmp against zero to comparisons of the return value
>of bcmp against zero.  This helps some platforms that implement bcmp
>more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
>an optimized implementation is in the works.
>
>This results in linkage failures for all targets with Clang due to the
>undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
>to unbreak the build.  This routine can be further optimized in the
>future.
>
>Other ideas discussed:
>* A weak alias was discussed, but breaks for architectures that define
>their own implementations of memcmp since aliases to declarations are
>not permitted (only definitions).  Arch-specific memcmp implementations
>typically declare memcmp in C headers, but implement them in assembly.
>* -ffreestanding also is used sporadically throughout the kernel.
>* -fno-builtin-bcmp doesn't work when doing LTO.
>
>Link: https://bugs.llvm.org/show_bug.cgi?id=41035
>Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
>Link:
>https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
>Link: https://github.com/ClangBuiltLinux/linux/issues/416
>Cc: stable@vger.kernel.org
>Reported-by: Nathan Chancellor <natechancellor@gmail.com>
>Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>Suggested-by: Arnd Bergmann <arnd@arndb.de>
>Suggested-by: James Y Knight <jyknight@google.com>
>Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
>Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>---
>Changes V2 -> V3:
>* Adjust comment as per Steven to Rasmus' sugguestion.
>* Pick up Steven's Ack.
>Changes V1 -> V2:
>* Add declaration to include/linux/string.h.
>* Reword comment above bcmp.
>
> include/linux/string.h |  3 +++
> lib/string.c           | 15 +++++++++++++++
> 2 files changed, 18 insertions(+)
>
>diff --git a/include/linux/string.h b/include/linux/string.h
>index 7927b875f80c..6ab0a6fa512e 100644
>--- a/include/linux/string.h
>+++ b/include/linux/string.h
>@@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
> #ifndef __HAVE_ARCH_MEMCMP
> extern int memcmp(const void *,const void *,__kernel_size_t);
> #endif
>+#ifndef __HAVE_ARCH_BCMP
>+extern int bcmp(const void *,const void *,__kernel_size_t);
>+#endif
> #ifndef __HAVE_ARCH_MEMCHR
> extern void * memchr(const void *,int,__kernel_size_t);
> #endif
>diff --git a/lib/string.c b/lib/string.c
>index 38e4ca08e757..48a43a42d3f2 100644
>--- a/lib/string.c
>+++ b/lib/string.c
>@@ -866,6 +866,21 @@ __visible int memcmp(const void *cs, const void
>*ct, size_t count)
> EXPORT_SYMBOL(memcmp);
> #endif
> 
>+#ifndef __HAVE_ARCH_BCMP
>+/**
>+ * bcmp - returns 0 if and only if the buffers have identical
>contents.
>+ * @cs: One area of memory.
>+ * @ct: Another area of memory.
>+ * @count: The size of the areas.

Where's the rest of the comment that I gave my ack to?

-- Steve

>+ */
>+#undef bcmp
>+int bcmp(const void *cs, const void *ct, size_t count)
>+{
>+	return memcmp(cs, ct, count);
>+}
>+EXPORT_SYMBOL(bcmp);
>+#endif
>+
> #ifndef __HAVE_ARCH_MEMSCAN
> /**
>  * memscan - Find a character in an area of memory.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity and top posting.

[PATCH v4] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

A recent optimization in Clang (r355672) lowers comparisons of the
return value of memcmp against zero to comparisons of the return value
of bcmp against zero.  This helps some platforms that implement bcmp
more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
an optimized implementation is in the works.

This results in linkage failures for all targets with Clang due to the
undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
to unbreak the build.  This routine can be further optimized in the
future.

Other ideas discussed:
* A weak alias was discussed, but breaks for architectures that define
their own implementations of memcmp since aliases to declarations are
not permitted (only definitions).  Arch-specific memcmp implementations
typically declare memcmp in C headers, but implement them in assembly.
* -ffreestanding also is used sporadically throughout the kernel.
* -fno-builtin-bcmp doesn't work when doing LTO.

Link: https://bugs.llvm.org/show_bug.cgi?id=41035
Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
Link: https://github.com/ClangBuiltLinux/linux/issues/416
Cc: stable@vger.kernel.org
Reported-by: Nathan Chancellor <natechancellor@gmail.com>
Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: James Y Knight <jyknight@google.com>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Changes V3 -> V4:
* Include the entirety of Rasmus' sugguestion, as per Steven.
* Change formal parameter identifiers to match the comment.
Changes V2 -> V3:
* Adjust comment as per Steven to Rasmus' sugguestion.
* Pick up Steven's Ack.
Changes V1 -> V2:
* Add declaration to include/linux/string.h.
* Reword comment above bcmp.

 include/linux/string.h |  3 +++
 lib/string.c           | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 7927b875f80c..6ab0a6fa512e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
 #ifndef __HAVE_ARCH_MEMCMP
 extern int memcmp(const void *,const void *,__kernel_size_t);
 #endif
+#ifndef __HAVE_ARCH_BCMP
+extern int bcmp(const void *,const void *,__kernel_size_t);
+#endif
 #ifndef __HAVE_ARCH_MEMCHR
 extern void * memchr(const void *,int,__kernel_size_t);
 #endif
diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..3ab861c1a857 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -866,6 +866,26 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
 EXPORT_SYMBOL(memcmp);
 #endif
 
+#ifndef __HAVE_ARCH_BCMP
+/**
+ * bcmp - returns 0 if and only if the buffers have identical contents.
+ * @a: pointer to first buffer.
+ * @b: pointer to second buffer.
+ * @len: size of buffers.
+ *
+ * The sign or magnitude of a non-zero return value has no particular
+ * meaning, and architectures may implement their own more efficient bcmp(). So
+ * while this particular implementation is a simple (tail) call to memcmp, do
+ * not rely on anything but whether the return value is zero or non-zero.
+ */
+#undef bcmp
+int bcmp(const void *a, const void *b, size_t len)
+{
+	return memcmp(a, b, len);
+}
+EXPORT_SYMBOL(bcmp);
+#endif
+
 #ifndef __HAVE_ARCH_MEMSCAN
 /**
  * memscan - Find a character in an area of memory.
-- 
2.21.0.360.g471c308f928-goog

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Nathan Chancellor]

On Wed, Mar 13, 2019 at 02:13:31PM -0700, Nick Desaulniers wrote:
> A recent optimization in Clang (r355672) lowers comparisons of the
> return value of memcmp against zero to comparisons of the return value
> of bcmp against zero.  This helps some platforms that implement bcmp
> more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
> an optimized implementation is in the works.
> 
> This results in linkage failures for all targets with Clang due to the
> undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
> to unbreak the build.  This routine can be further optimized in the
> future.
> 
> Other ideas discussed:
> * A weak alias was discussed, but breaks for architectures that define
> their own implementations of memcmp since aliases to declarations are
> not permitted (only definitions).  Arch-specific memcmp implementations
> typically declare memcmp in C headers, but implement them in assembly.
> * -ffreestanding also is used sporadically throughout the kernel.
> * -fno-builtin-bcmp doesn't work when doing LTO.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=41035
> Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
> Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
> Link: https://github.com/ClangBuiltLinux/linux/issues/416
> Cc: stable@vger.kernel.org
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: James Y Knight <jyknight@google.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

Thanks for fixing this better than I did (or tried to)!

> ---
> Changes V3 -> V4:
> * Include the entirety of Rasmus' sugguestion, as per Steven.
> * Change formal parameter identifiers to match the comment.
> Changes V2 -> V3:
> * Adjust comment as per Steven to Rasmus' sugguestion.
> * Pick up Steven's Ack.
> Changes V1 -> V2:
> * Add declaration to include/linux/string.h.
> * Reword comment above bcmp.
> 
>  include/linux/string.h |  3 +++
>  lib/string.c           | 20 ++++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 7927b875f80c..6ab0a6fa512e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
>  #ifndef __HAVE_ARCH_MEMCMP
>  extern int memcmp(const void *,const void *,__kernel_size_t);
>  #endif
> +#ifndef __HAVE_ARCH_BCMP
> +extern int bcmp(const void *,const void *,__kernel_size_t);
> +#endif
>  #ifndef __HAVE_ARCH_MEMCHR
>  extern void * memchr(const void *,int,__kernel_size_t);
>  #endif
> diff --git a/lib/string.c b/lib/string.c
> index 38e4ca08e757..3ab861c1a857 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -866,6 +866,26 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
>  EXPORT_SYMBOL(memcmp);
>  #endif
>  
> +#ifndef __HAVE_ARCH_BCMP
> +/**
> + * bcmp - returns 0 if and only if the buffers have identical contents.
> + * @a: pointer to first buffer.
> + * @b: pointer to second buffer.
> + * @len: size of buffers.
> + *
> + * The sign or magnitude of a non-zero return value has no particular
> + * meaning, and architectures may implement their own more efficient bcmp(). So
> + * while this particular implementation is a simple (tail) call to memcmp, do
> + * not rely on anything but whether the return value is zero or non-zero.
> + */
> +#undef bcmp
> +int bcmp(const void *a, const void *b, size_t len)
> +{
> +	return memcmp(a, b, len);
> +}
> +EXPORT_SYMBOL(bcmp);
> +#endif
> +
>  #ifndef __HAVE_ARCH_MEMSCAN
>  /**
>   * memscan - Find a character in an area of memory.
> -- 
> 2.21.0.360.g471c308f928-goog
> 

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Masahiro Yamada]

On Thu, Mar 14, 2019 at 6:13 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> A recent optimization in Clang (r355672) lowers comparisons of the
> return value of memcmp against zero to comparisons of the return value
> of bcmp against zero.  This helps some platforms that implement bcmp
> more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
> an optimized implementation is in the works.
>
> This results in linkage failures for all targets with Clang due to the
> undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
> to unbreak the build.  This routine can be further optimized in the
> future.
>
> Other ideas discussed:
> * A weak alias was discussed, but breaks for architectures that define
> their own implementations of memcmp since aliases to declarations are
> not permitted (only definitions).  Arch-specific memcmp implementations
> typically declare memcmp in C headers, but implement them in assembly.
> * -ffreestanding also is used sporadically throughout the kernel.
> * -fno-builtin-bcmp doesn't work when doing LTO.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=41035
> Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
> Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
> Link: https://github.com/ClangBuiltLinux/linux/issues/416
> Cc: stable@vger.kernel.org
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: James Y Knight <jyknight@google.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>


Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>




> ---
> Changes V3 -> V4:
> * Include the entirety of Rasmus' sugguestion, as per Steven.
> * Change formal parameter identifiers to match the comment.
> Changes V2 -> V3:
> * Adjust comment as per Steven to Rasmus' sugguestion.
> * Pick up Steven's Ack.
> Changes V1 -> V2:
> * Add declaration to include/linux/string.h.
> * Reword comment above bcmp.
>
>  include/linux/string.h |  3 +++
>  lib/string.c           | 20 ++++++++++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 7927b875f80c..6ab0a6fa512e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
>  #ifndef __HAVE_ARCH_MEMCMP
>  extern int memcmp(const void *,const void *,__kernel_size_t);
>  #endif
> +#ifndef __HAVE_ARCH_BCMP
> +extern int bcmp(const void *,const void *,__kernel_size_t);
> +#endif
>  #ifndef __HAVE_ARCH_MEMCHR
>  extern void * memchr(const void *,int,__kernel_size_t);
>  #endif
> diff --git a/lib/string.c b/lib/string.c
> index 38e4ca08e757..3ab861c1a857 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -866,6 +866,26 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
>  EXPORT_SYMBOL(memcmp);
>  #endif
>
> +#ifndef __HAVE_ARCH_BCMP
> +/**
> + * bcmp - returns 0 if and only if the buffers have identical contents.
> + * @a: pointer to first buffer.
> + * @b: pointer to second buffer.
> + * @len: size of buffers.
> + *
> + * The sign or magnitude of a non-zero return value has no particular
> + * meaning, and architectures may implement their own more efficient bcmp(). So
> + * while this particular implementation is a simple (tail) call to memcmp, do
> + * not rely on anything but whether the return value is zero or non-zero.
> + */
> +#undef bcmp
> +int bcmp(const void *a, const void *b, size_t len)
> +{
> +       return memcmp(a, b, len);
> +}
> +EXPORT_SYMBOL(bcmp);
> +#endif
> +
>  #ifndef __HAVE_ARCH_MEMSCAN
>  /**
>   * memscan - Find a character in an area of memory.
> --
> 2.21.0.360.g471c308f928-goog
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Andy Shevchenko]

On Wed, Mar 13, 2019 at 02:13:31PM -0700, Nick Desaulniers wrote:
> A recent optimization in Clang (r355672) lowers comparisons of the
> return value of memcmp against zero to comparisons of the return value
> of bcmp against zero.  This helps some platforms that implement bcmp
> more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
> an optimized implementation is in the works.
> 
> This results in linkage failures for all targets with Clang due to the
> undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
> to unbreak the build.  This routine can be further optimized in the
> future.
> 
> Other ideas discussed:
> * A weak alias was discussed, but breaks for architectures that define
> their own implementations of memcmp since aliases to declarations are
> not permitted (only definitions).  Arch-specific memcmp implementations
> typically declare memcmp in C headers, but implement them in assembly.
> * -ffreestanding also is used sporadically throughout the kernel.
> * -fno-builtin-bcmp doesn't work when doing LTO.

Thanks!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=41035
> Link: https://code.woboq.org/userspace/glibc/string/memcmp.c.html#bcmp
> Link: https://github.com/llvm/llvm-project/commit/8e16d73346f8091461319a7dfc4ddd18eedcff13
> Link: https://github.com/ClangBuiltLinux/linux/issues/416
> Cc: stable@vger.kernel.org
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Reported-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: James Y Knight <jyknight@google.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Changes V3 -> V4:
> * Include the entirety of Rasmus' sugguestion, as per Steven.
> * Change formal parameter identifiers to match the comment.
> Changes V2 -> V3:
> * Adjust comment as per Steven to Rasmus' sugguestion.
> * Pick up Steven's Ack.
> Changes V1 -> V2:
> * Add declaration to include/linux/string.h.
> * Reword comment above bcmp.
> 
>  include/linux/string.h |  3 +++
>  lib/string.c           | 20 ++++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 7927b875f80c..6ab0a6fa512e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
>  #ifndef __HAVE_ARCH_MEMCMP
>  extern int memcmp(const void *,const void *,__kernel_size_t);
>  #endif
> +#ifndef __HAVE_ARCH_BCMP
> +extern int bcmp(const void *,const void *,__kernel_size_t);
> +#endif
>  #ifndef __HAVE_ARCH_MEMCHR
>  extern void * memchr(const void *,int,__kernel_size_t);
>  #endif
> diff --git a/lib/string.c b/lib/string.c
> index 38e4ca08e757..3ab861c1a857 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -866,6 +866,26 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
>  EXPORT_SYMBOL(memcmp);
>  #endif
>  
> +#ifndef __HAVE_ARCH_BCMP
> +/**
> + * bcmp - returns 0 if and only if the buffers have identical contents.
> + * @a: pointer to first buffer.
> + * @b: pointer to second buffer.
> + * @len: size of buffers.
> + *
> + * The sign or magnitude of a non-zero return value has no particular
> + * meaning, and architectures may implement their own more efficient bcmp(). So
> + * while this particular implementation is a simple (tail) call to memcmp, do
> + * not rely on anything but whether the return value is zero or non-zero.
> + */
> +#undef bcmp
> +int bcmp(const void *a, const void *b, size_t len)
> +{
> +	return memcmp(a, b, len);
> +}
> +EXPORT_SYMBOL(bcmp);
> +#endif
> +
>  #ifndef __HAVE_ARCH_MEMSCAN
>  /**
>   * memscan - Find a character in an area of memory.
> -- 
> 2.21.0.360.g471c308f928-goog
> 

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v4] lib/string.c: implement a basic bcmp

[David Laight]

From: Nick Desaulniers
> Sent: 13 March 2019 21:14
...
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 7927b875f80c..6ab0a6fa512e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
>  #ifndef __HAVE_ARCH_MEMCMP
>  extern int memcmp(const void *,const void *,__kernel_size_t);
>  #endif
> +#ifndef __HAVE_ARCH_BCMP
> +extern int bcmp(const void *,const void *,__kernel_size_t);
> +#endif

Shouldn't that prototype always be present?
Any architecture specific implementation better have the same definition.
This is particularly true here since the compiler is going to assume
the default calling convention.

The only time you wouldn't want it is when the architecture specific
implementation is inline asm.
If that were an option the default implementation would need to be
excluded using a different #define.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Rasmus Villemoes]

On 14/03/2019 10.57, David Laight wrote:
> From: Nick Desaulniers
>> Sent: 13 March 2019 21:14
> ...
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 7927b875f80c..6ab0a6fa512e 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -150,6 +150,9 @@ extern void * memscan(void *,int,__kernel_size_t);
>>  #ifndef __HAVE_ARCH_MEMCMP
>>  extern int memcmp(const void *,const void *,__kernel_size_t);
>>  #endif
>> +#ifndef __HAVE_ARCH_BCMP
>> +extern int bcmp(const void *,const void *,__kernel_size_t);
>> +#endif
> 
> Shouldn't that prototype always be present?

Yes and no. The problem is that asm/string.h may decide to implement
bcmp (or memcpy, memset, strcpy, ...) as a macro, which would break
rather badly when used to declare the function. And we can't undef the
macro temporarily. So I think the convention is that asm/string.h
provides the prototype before it #defines the macro, see e.g.
arch/x86/include/asm/string_32.h which has a #define of memcpy.

There is a fix for the "may be defined as a function-like macro", which is

extern int (bcmp)(const void *,const void *,__kernel_size_t);

I'd like to see that used rather than the ad hoc convention, but it is
of course somewhat unconventional C.

Rasmus

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Andrew Morton]

On Wed, 13 Mar 2019 14:13:31 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:

> A recent optimization in Clang (r355672) lowers comparisons of the
> return value of memcmp against zero to comparisons of the return value
> of bcmp against zero.  This helps some platforms that implement bcmp
> more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
> an optimized implementation is in the works.
> 
> This results in linkage failures for all targets with Clang due to the
> undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
> to unbreak the build.  This routine can be further optimized in the
> future.
> 
> Other ideas discussed:
> * A weak alias was discussed, but breaks for architectures that define
> their own implementations of memcmp since aliases to declarations are
> not permitted (only definitions).  Arch-specific memcmp implementations
> typically declare memcmp in C headers, but implement them in assembly.
> * -ffreestanding also is used sporadically throughout the kernel.
> * -fno-builtin-bcmp doesn't work when doing LTO.

I guess we should backport this into -stable so that older kernels can
be built with newer Clang.

> ...
>
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -866,6 +866,26 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
>  EXPORT_SYMBOL(memcmp);
>  #endif
>  
> +#ifndef __HAVE_ARCH_BCMP
> +/**
> + * bcmp - returns 0 if and only if the buffers have identical contents.
> + * @a: pointer to first buffer.
> + * @b: pointer to second buffer.
> + * @len: size of buffers.
> + *
> + * The sign or magnitude of a non-zero return value has no particular
> + * meaning, and architectures may implement their own more efficient bcmp(). So
> + * while this particular implementation is a simple (tail) call to memcmp, do
> + * not rely on anything but whether the return value is zero or non-zero.
> + */
> +#undef bcmp

What is the undef for?

> +int bcmp(const void *a, const void *b, size_t len)
> +{
> +	return memcmp(a, b, len);
> +}
> +EXPORT_SYMBOL(bcmp);
> +#endif
> +
>  #ifndef __HAVE_ARCH_MEMSCAN
>  /**
>   * memscan - Find a character in an area of memory.
> -- 
> 2.21.0.360.g471c308f928-goog

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

On Wed, Mar 20, 2019 at 7:11 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Wed, 13 Mar 2019 14:13:31 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> > A recent optimization in Clang (r355672) lowers comparisons of the
> > return value of memcmp against zero to comparisons of the return value
> > of bcmp against zero.  This helps some platforms that implement bcmp
> > more efficiently than memcmp. glibc simply aliases bcmp to memcmp, but
> > an optimized implementation is in the works.
> >
> > This results in linkage failures for all targets with Clang due to the
> > undefined symbol.  For now, just implement bcmp as a tailcail to memcmp
> > to unbreak the build.  This routine can be further optimized in the
> > future.
> >
> > Other ideas discussed:
> > * A weak alias was discussed, but breaks for architectures that define
> > their own implementations of memcmp since aliases to declarations are
> > not permitted (only definitions).  Arch-specific memcmp implementations
> > typically declare memcmp in C headers, but implement them in assembly.
> > * -ffreestanding also is used sporadically throughout the kernel.
> > * -fno-builtin-bcmp doesn't work when doing LTO.
>
> I guess we should backport this into -stable so that older kernels can
> be built with newer Clang.

Ah, you're right.  I always forget.  Is it too late to add

Cc: stable@vger.kernel.org

to the patch in your tree?

>
> > ...
> >
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -866,6 +866,26 @@ __visible int memcmp(const void *cs, const void *ct, size_t count)
> >  EXPORT_SYMBOL(memcmp);
> >  #endif
> >
> > +#ifndef __HAVE_ARCH_BCMP
> > +/**
> > + * bcmp - returns 0 if and only if the buffers have identical contents.
> > + * @a: pointer to first buffer.
> > + * @b: pointer to second buffer.
> > + * @len: size of buffers.
> > + *
> > + * The sign or magnitude of a non-zero return value has no particular
> > + * meaning, and architectures may implement their own more efficient bcmp(). So
> > + * while this particular implementation is a simple (tail) call to memcmp, do
> > + * not rely on anything but whether the return value is zero or non-zero.
> > + */
> > +#undef bcmp
>
> What is the undef for?

Used the same convention as memcpy.  Looking at the rest of the
translation unit, I see that there's a mix of functions that do or do
not undef their symbol.  Rasmus pointed out that memcpy is implemented
as a macro for x86 (arch/x86/include/asm/string_32.h).  But looking
closer now, I suspect it's not needed (anyone declaring memcmp as a
macro should be setting __HAVE_ARCH_MEMCMP).

Shall I send you a cleanup removing the undefs for bcmp, memcmp,
strcat, strcpy, and strcmp?  Of those, I only see memcmp being
`#defined` in arch/m68k/include/asm/string.h, arch/x86/boot/string.h,
and arch/x86/include/asm/string_32.h.

Further, I can drop some of the __GNUC__ < 4 code in
arch/x86/include/asm/string_32.h. (grepping for __GNUC__, looks like
there's a fair amount of code that can be cleaned up).  We should
probably check it since Clang lies about being GCC 4.2 compatible,
which will surely break in horrific ways at some point.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Nick Desaulniers]

On Thu, Mar 21, 2019 at 10:02 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Mar 20, 2019 at 7:11 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > I guess we should backport this into -stable so that older kernels can
> > be built with newer Clang.
>
> Ah, you're right.  I always forget.  Is it too late to add
>
> Cc: stable@vger.kernel.org
>
> to the patch in your tree?

Just noticed now. Thank you for adding it!
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Andrew Morton]

On Thu, 21 Mar 2019 10:02:37 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:

> Shall I send you a cleanup removing the undefs for bcmp, memcmp,
> strcat, strcpy, and strcmp?  Of those, I only see memcmp being
> `#defined` in arch/m68k/include/asm/string.h, arch/x86/boot/string.h,
> and arch/x86/include/asm/string_32.h.
> 
> Further, I can drop some of the __GNUC__ < 4 code in
> arch/x86/include/asm/string_32.h. (grepping for __GNUC__, looks like
> there's a fair amount of code that can be cleaned up).  We should
> probably check it since Clang lies about being GCC 4.2 compatible,
> which will surely break in horrific ways at some point.\

All sounds good.  Some time, when convenient, thanks.

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Rasmus Villemoes]

On 21/03/2019 18.02, Nick Desaulniers wrote:
> On Wed, Mar 20, 2019 at 7:11 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>>
> 
> Further, I can drop some of the __GNUC__ < 4 code in
> arch/x86/include/asm/string_32.h. 

Already on its way to Linus:

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=88ca66d8540ca26119b1428cddb96b37925bdf01

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=2e905c7abdcd5ff09b9df33d25eb7148c85bed2a

Re: [PATCH v4] lib/string.c: implement a basic bcmp

[Sedat Dilek]

On Fri, Mar 22, 2019 at 8:17 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 21/03/2019 18.02, Nick Desaulniers wrote:
> > On Wed, Mar 20, 2019 at 7:11 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >>
> >
> > Further, I can drop some of the __GNUC__ < 4 code in
> > arch/x86/include/asm/string_32.h.
>
> Already on its way to Linus:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=88ca66d8540ca26119b1428cddb96b37925bdf01
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=2e905c7abdcd5ff09b9df33d25eb7148c85bed2a
>

Upstream:

"x86/asm: Remove dead __GNUC__ conditionals"
https://git.kernel.org/linus/88ca66d8540ca26119b1428cddb96b37925bdf01

"x86/asm: Remove unused __constant_c_x_memset() macro and inlines"
https://git.kernel.org/linus/2e905c7abdcd5ff09b9df33d25eb7148c85bed2a

- Sedat -

Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS

[Arnd Bergmann]

On Wed, Mar 13, 2019 at 6:27 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:

> > diff --git a/lib/string.c b/lib/string.c
> > index 38e4ca08e757..e6c1954f2716 100644
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -845,7 +845,13 @@ void *memmove(void *dest, const void *src, size_t count)
> >  EXPORT_SYMBOL(memmove);
> >  #endif
> >
> > -#ifndef __HAVE_ARCH_MEMCMP
> > +#ifdef __HAVE_ARCH_MEMCMP
> > +int bcmp(const void *cs, const void *ct, size_t n)
> > +{
> > +       return memcmp(cs, ct, n);
> > +}
> > +EXPORT_SYMBOL(bcmp);
> > +#else
> >  /**
> >   * memcmp - Compare two areas of memory
> >   * @cs: One area of memory
> > @@ -864,6 +870,8 @@ __visible int memcmp(const void *cs, const void
> > *ct, size_t count)
> >         return res;
> >  }
> >  EXPORT_SYMBOL(memcmp);
> > +__weak __alias(memcmp) typeof(memcmp) bcmp;
> > +EXPORT_SYMBOL(bcmp);
> >  #endif
> >
> >  #ifndef __HAVE_ARCH_MEMSCAN
>
> Alternatively, just not worrying about __alias makes this simpler and
> seems to work (need to add comments, thoughts?):

Either way seems fine to me. If we don't plan to provide an
optimized version, I'd go with the simpler definition rather than
the alias.

       Arnd