* [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS @ 2019-03-12 21:52 Nathan Chancellor 2019-03-12 22:55 ` Nick Desaulniers ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: Nathan Chancellor @ 2019-03-12 21:52 UTC (permalink / raw) To: Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel Cc: Nick Desaulniers, Arnd Bergmann, James Y Knight, clang-built-linux, Nathan Chancellor, stable 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-12 21:52 [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS Nathan Chancellor @ 2019-03-12 22:55 ` Nick Desaulniers 2019-03-13 8:13 ` Rasmus Villemoes [not found] ` <20190325003834.2F24E2133F@mail.kernel.org> 2 siblings, 0 replies; 31+ messages in thread From: Nick Desaulniers @ 2019-03-12 22:55 UTC (permalink / raw) To: Nathan Chancellor, Masahiro Yamada Cc: Michal Marek, Linux Kbuild mailing list, LKML, Arnd Bergmann, James Y Knight, clang-built-linux, stable 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-12 21:52 [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS Nathan Chancellor 2019-03-12 22:55 ` Nick Desaulniers @ 2019-03-13 8:13 ` Rasmus Villemoes 2019-03-13 10:58 ` Masahiro Yamada 2019-03-13 13:44 ` Nathan Chancellor [not found] ` <20190325003834.2F24E2133F@mail.kernel.org> 2 siblings, 2 replies; 31+ messages in thread From: Rasmus Villemoes @ 2019-03-13 8:13 UTC (permalink / raw) To: Nathan Chancellor, Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel Cc: Nick Desaulniers, Arnd Bergmann, James Y Knight, clang-built-linux, stable 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-13 8:13 ` Rasmus Villemoes @ 2019-03-13 10:58 ` Masahiro Yamada 2019-03-13 13:44 ` Nathan Chancellor 1 sibling, 0 replies; 31+ messages in thread From: Masahiro Yamada @ 2019-03-13 10:58 UTC (permalink / raw) To: Rasmus Villemoes Cc: Nathan Chancellor, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, Nick Desaulniers, Arnd Bergmann, James Y Knight, clang-built-linux, stable 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-13 8:13 ` Rasmus Villemoes 2019-03-13 10:58 ` Masahiro Yamada @ 2019-03-13 13:44 ` Nathan Chancellor 2019-03-13 13:48 ` Arnd Bergmann 1 sibling, 1 reply; 31+ messages in thread From: Nathan Chancellor @ 2019-03-13 13:44 UTC (permalink / raw) To: Rasmus Villemoes Cc: Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel, Nick Desaulniers, Arnd Bergmann, James Y Knight, clang-built-linux, stable 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-13 13:44 ` Nathan Chancellor @ 2019-03-13 13:48 ` Arnd Bergmann 2019-03-13 15:32 ` Nathan Chancellor 0 siblings, 1 reply; 31+ messages in thread From: Arnd Bergmann @ 2019-03-13 13:48 UTC (permalink / raw) To: Nathan Chancellor Cc: Rasmus Villemoes, Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, Nick Desaulniers, James Y Knight, clang-built-linux, # 3.4.x 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-13 13:48 ` Arnd Bergmann @ 2019-03-13 15:32 ` Nathan Chancellor 2019-03-13 17:21 ` Nick Desaulniers 0 siblings, 1 reply; 31+ messages in thread From: Nathan Chancellor @ 2019-03-13 15:32 UTC (permalink / raw) To: Arnd Bergmann Cc: Rasmus Villemoes, Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, Nick Desaulniers, James Y Knight, clang-built-linux, # 3.4.x 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-13 15:32 ` Nathan Chancellor @ 2019-03-13 17:21 ` Nick Desaulniers 2019-03-13 17:27 ` Nick Desaulniers 0 siblings, 1 reply; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 17:21 UTC (permalink / raw) To: Nathan Chancellor Cc: Arnd Bergmann, Rasmus Villemoes, Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, James Y Knight, clang-built-linux, # 3.4.x 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-13 17:21 ` Nick Desaulniers @ 2019-03-13 17:27 ` Nick Desaulniers 2019-03-13 18:02 ` [PATCH] lib/string.c: implement a basic bcmp Nick Desaulniers 2019-03-13 19:38 ` [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS Arnd Bergmann 0 siblings, 2 replies; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 17:27 UTC (permalink / raw) To: Nathan Chancellor Cc: Arnd Bergmann, Rasmus Villemoes, Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, James Y Knight, clang-built-linux, # 3.4.x 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH] lib/string.c: implement a basic bcmp 2019-03-13 17:27 ` Nick Desaulniers @ 2019-03-13 18:02 ` Nick Desaulniers 2019-03-13 18:11 ` Nick Desaulniers 2019-03-13 19:38 ` [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS Arnd Bergmann 1 sibling, 1 reply; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 18:02 UTC (permalink / raw) To: akpm Cc: clang-built-linux, linux-kbuild, Nick Desaulniers, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Greg Kroah-Hartman, Alexander Shishkin, Andy Shevchenko, linux-kernel 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] lib/string.c: implement a basic bcmp 2019-03-13 18:02 ` [PATCH] lib/string.c: implement a basic bcmp Nick Desaulniers @ 2019-03-13 18:11 ` Nick Desaulniers 2019-03-13 18:17 ` [PATCH v2] " Nick Desaulniers 0 siblings, 1 reply; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 18:11 UTC (permalink / raw) To: Andrew Morton Cc: clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Greg Kroah-Hartman, Alexander Shishkin, Andy Shevchenko, LKML 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2] lib/string.c: implement a basic bcmp 2019-03-13 18:11 ` Nick Desaulniers @ 2019-03-13 18:17 ` Nick Desaulniers 2019-03-13 18:40 ` Steven Rostedt 0 siblings, 1 reply; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 18:17 UTC (permalink / raw) To: akpm Cc: clang-built-linux, linux-kbuild, Nick Desaulniers, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Alexander Shishkin, Ingo Molnar, Steven Rostedt (VMware), Greg Kroah-Hartman, Dan Williams, Andy Shevchenko, linux-kernel 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2] lib/string.c: implement a basic bcmp 2019-03-13 18:17 ` [PATCH v2] " Nick Desaulniers @ 2019-03-13 18:40 ` Steven Rostedt 2019-03-13 18:51 ` Nick Desaulniers 0 siblings, 1 reply; 31+ messages in thread From: Steven Rostedt @ 2019-03-13 18:40 UTC (permalink / raw) To: Nick Desaulniers Cc: akpm, clang-built-linux, linux-kbuild, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Alexander Shishkin, Ingo Molnar, Greg Kroah-Hartman, Dan Williams, Andy Shevchenko, linux-kernel 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 > + ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] lib/string.c: implement a basic bcmp 2019-03-13 18:40 ` Steven Rostedt @ 2019-03-13 18:51 ` Nick Desaulniers 2019-03-13 19:01 ` Steven Rostedt 0 siblings, 1 reply; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 18:51 UTC (permalink / raw) To: Steven Rostedt Cc: Andrew Morton, clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Alexander Shishkin, Ingo Molnar, Greg Kroah-Hartman, Dan Williams, Andy Shevchenko, LKML 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] lib/string.c: implement a basic bcmp 2019-03-13 18:51 ` Nick Desaulniers @ 2019-03-13 19:01 ` Steven Rostedt 2019-03-13 19:34 ` Rasmus Villemoes 0 siblings, 1 reply; 31+ messages in thread From: Steven Rostedt @ 2019-03-13 19:01 UTC (permalink / raw) To: Nick Desaulniers Cc: Andrew Morton, clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Alexander Shishkin, Ingo Molnar, Greg Kroah-Hartman, Dan Williams, Andy Shevchenko, LKML 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] lib/string.c: implement a basic bcmp 2019-03-13 19:01 ` Steven Rostedt @ 2019-03-13 19:34 ` Rasmus Villemoes 2019-03-13 20:12 ` Steven Rostedt 0 siblings, 1 reply; 31+ messages in thread From: Rasmus Villemoes @ 2019-03-13 19:34 UTC (permalink / raw) To: Steven Rostedt, Nick Desaulniers Cc: Andrew Morton, clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Alexander Shishkin, Ingo Molnar, Greg Kroah-Hartman, Dan Williams, Andy Shevchenko, LKML 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] lib/string.c: implement a basic bcmp 2019-03-13 19:34 ` Rasmus Villemoes @ 2019-03-13 20:12 ` Steven Rostedt 2019-03-13 20:37 ` [PATCH v3] " Nick Desaulniers 0 siblings, 1 reply; 31+ messages in thread From: Steven Rostedt @ 2019-03-13 20:12 UTC (permalink / raw) To: Rasmus Villemoes Cc: Nick Desaulniers, Andrew Morton, clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Alexander Shishkin, Ingo Molnar, Greg Kroah-Hartman, Dan Williams, Andy Shevchenko, LKML 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3] lib/string.c: implement a basic bcmp 2019-03-13 20:12 ` Steven Rostedt @ 2019-03-13 20:37 ` Nick Desaulniers 2019-03-13 21:03 ` Steven Rostedt 0 siblings, 1 reply; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 20:37 UTC (permalink / raw) To: akpm Cc: clang-built-linux, linux-kbuild, Nick Desaulniers, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Andy Shevchenko, Dan Williams, Namhyung Kim, Alexander Shishkin, Greg Kroah-Hartman, linux-kernel 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3] lib/string.c: implement a basic bcmp 2019-03-13 20:37 ` [PATCH v3] " Nick Desaulniers @ 2019-03-13 21:03 ` Steven Rostedt 2019-03-13 21:13 ` [PATCH v4] " Nick Desaulniers 0 siblings, 1 reply; 31+ messages in thread From: Steven Rostedt @ 2019-03-13 21:03 UTC (permalink / raw) To: Nick Desaulniers, akpm Cc: clang-built-linux, linux-kbuild, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Andy Shevchenko, Dan Williams, Namhyung Kim, Alexander Shishkin, Greg Kroah-Hartman, linux-kernel 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-13 21:03 ` Steven Rostedt @ 2019-03-13 21:13 ` Nick Desaulniers 2019-03-14 3:15 ` Nathan Chancellor ` (4 more replies) 0 siblings, 5 replies; 31+ messages in thread From: Nick Desaulniers @ 2019-03-13 21:13 UTC (permalink / raw) To: akpm Cc: clang-built-linux, linux-kbuild, Nick Desaulniers, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, linux-kernel 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-13 21:13 ` [PATCH v4] " Nick Desaulniers @ 2019-03-14 3:15 ` Nathan Chancellor 2019-03-14 5:00 ` Masahiro Yamada ` (3 subsequent siblings) 4 siblings, 0 replies; 31+ messages in thread From: Nathan Chancellor @ 2019-03-14 3:15 UTC (permalink / raw) To: Nick Desaulniers Cc: akpm, clang-built-linux, linux-kbuild, stable, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, linux-kernel 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 > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-13 21:13 ` [PATCH v4] " Nick Desaulniers 2019-03-14 3:15 ` Nathan Chancellor @ 2019-03-14 5:00 ` Masahiro Yamada 2019-03-14 8:33 ` Andy Shevchenko ` (2 subsequent siblings) 4 siblings, 0 replies; 31+ messages in thread From: Masahiro Yamada @ 2019-03-14 5:00 UTC (permalink / raw) To: Nick Desaulniers Cc: Andrew Morton, clang-built-linux, Linux Kbuild mailing list, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-13 21:13 ` [PATCH v4] " Nick Desaulniers 2019-03-14 3:15 ` Nathan Chancellor 2019-03-14 5:00 ` Masahiro Yamada @ 2019-03-14 8:33 ` Andy Shevchenko 2019-03-14 9:57 ` David Laight 2019-03-21 2:11 ` Andrew Morton 4 siblings, 0 replies; 31+ messages in thread From: Andy Shevchenko @ 2019-03-14 8:33 UTC (permalink / raw) To: Nick Desaulniers Cc: akpm, clang-built-linux, linux-kbuild, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, linux-kernel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-13 21:13 ` [PATCH v4] " Nick Desaulniers ` (2 preceding siblings ...) 2019-03-14 8:33 ` Andy Shevchenko @ 2019-03-14 9:57 ` David Laight 2019-03-14 11:07 ` Rasmus Villemoes 2019-03-21 2:11 ` Andrew Morton 4 siblings, 1 reply; 31+ messages in thread From: David Laight @ 2019-03-14 9:57 UTC (permalink / raw) To: 'Nick Desaulniers', akpm Cc: clang-built-linux, linux-kbuild, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, linux-kernel 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) ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-14 9:57 ` David Laight @ 2019-03-14 11:07 ` Rasmus Villemoes 0 siblings, 0 replies; 31+ messages in thread From: Rasmus Villemoes @ 2019-03-14 11:07 UTC (permalink / raw) To: David Laight, 'Nick Desaulniers', akpm Cc: clang-built-linux, linux-kbuild, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, linux-kernel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-13 21:13 ` [PATCH v4] " Nick Desaulniers ` (3 preceding siblings ...) 2019-03-14 9:57 ` David Laight @ 2019-03-21 2:11 ` Andrew Morton 2019-03-21 17:02 ` Nick Desaulniers 4 siblings, 1 reply; 31+ messages in thread From: Andrew Morton @ 2019-03-21 2:11 UTC (permalink / raw) To: Nick Desaulniers Cc: clang-built-linux, linux-kbuild, stable, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, linux-kernel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-21 2:11 ` Andrew Morton @ 2019-03-21 17:02 ` Nick Desaulniers 2019-03-21 17:20 ` Nick Desaulniers 2019-03-21 21:05 ` Andrew Morton 0 siblings, 2 replies; 31+ messages in thread From: Nick Desaulniers @ 2019-03-21 17:02 UTC (permalink / raw) To: Andrew Morton Cc: clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, LKML 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-21 17:02 ` Nick Desaulniers @ 2019-03-21 17:20 ` Nick Desaulniers 2019-03-21 21:05 ` Andrew Morton 1 sibling, 0 replies; 31+ messages in thread From: Nick Desaulniers @ 2019-03-21 17:20 UTC (permalink / raw) To: Andrew Morton Cc: clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, LKML 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v4] lib/string.c: implement a basic bcmp 2019-03-21 17:02 ` Nick Desaulniers 2019-03-21 17:20 ` Nick Desaulniers @ 2019-03-21 21:05 ` Andrew Morton 1 sibling, 0 replies; 31+ messages in thread From: Andrew Morton @ 2019-03-21 21:05 UTC (permalink / raw) To: Nick Desaulniers Cc: clang-built-linux, Linux Kbuild mailing list, # 3.4.x, Nathan Chancellor, Adhemerval Zanella, Arnd Bergmann, James Y Knight, Masahiro Yamada, Rasmus Villemoes, Steven Rostedt, Namhyung Kim, Greg Kroah-Hartman, Alexander Shishkin, Dan Williams, Andy Shevchenko, LKML 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS 2019-03-13 17:27 ` Nick Desaulniers 2019-03-13 18:02 ` [PATCH] lib/string.c: implement a basic bcmp Nick Desaulniers @ 2019-03-13 19:38 ` Arnd Bergmann 1 sibling, 0 replies; 31+ messages in thread From: Arnd Bergmann @ 2019-03-13 19:38 UTC (permalink / raw) To: Nick Desaulniers Cc: Nathan Chancellor, Rasmus Villemoes, Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, James Y Knight, clang-built-linux, # 3.4.x 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <20190325003834.2F24E2133F@mail.kernel.org>]
* Re: [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS [not found] ` <20190325003834.2F24E2133F@mail.kernel.org> @ 2019-03-25 14:02 ` Nathan Chancellor 0 siblings, 0 replies; 31+ messages in thread From: Nathan Chancellor @ 2019-03-25 14:02 UTC (permalink / raw) To: Sasha Levin; +Cc: Masahiro Yamada, Nick Desaulniers, stable On Mon, Mar 25, 2019 at 12:38:33AM +0000, Sasha Levin wrote: > Hi, > > [This is an automated email] > > This commit has been processed because it contains a -stable tag. > The stable tag indicates that it's relevant for the following trees: all > > The bot has tested the following trees: v5.0.3, v4.19.30, v4.14.107, v4.9.164, v4.4.176, v3.18.136. > > v5.0.3: Build OK! > v4.19.30: Build OK! > v4.14.107: Build OK! > v4.9.164: Build OK! > v4.4.176: Failed to apply! Possible dependencies: > 238bcbc4e07f ("kbuild: consolidate Clang compiler flags") > dbe27a002ef8 ("kbuild: add -no-integrated-as Clang option unconditionally") > > v3.18.136: Failed to apply! Possible dependencies: > 238bcbc4e07f ("kbuild: consolidate Clang compiler flags") > 35288e30ebca ("Kbuild: fix file name in comment about extra gcc checks") > 5631d9c42985 ("kbuild: Fix clang detection") > 665d92e38f65 ("kbuild: do not add $(call ...) to invoke cc-version or cc-fullversion") > 785f11aa595b ("kbuild: Add better clang cross build support") > 92d6cf2dab83 ("powerpc: Don't use -mno-strict-align on clang") > a37c45cd82e6 ("kbuild: clang: add -no-integrated-as to KBUILD_[AC]FLAGS") > ae6b289a3789 ("kbuild: Set KBUILD_CFLAGS before incl. arch Makefile") > bb3f38c3c5b7 ("kbuild: clang: fix build failures with sparse check") > bfb38988c51e ("kbuild: clang: Disable 'address-of-packed-member' warning") > dd33c03b18b3 ("kbuild: fix cc-ifversion macro") > e79c8385c878 ("powerpc: Don't do gcc version checks if we're building with clang") > ee4eb20dbce9 ("Makefile: Fix detection of clang when cross-compiling") > ef8c4ed9db80 ("kbuild: allow to use GCC toolchain not in Clang search path") > > > How should we proceed with this patch? > > -- > Thanks, > Sasha Hi Sasha, This patch will not be picked up, Nick's patch is a much better fix, which should apply cleanly to all stable branches after it makes it into mainline. https://lore.kernel.org/lkml/20190313211335.165605-1-ndesaulniers@google.com/ Thanks for letting us know! Nathan ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2019-03-25 14:02 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-12 21:52 [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS Nathan Chancellor 2019-03-12 22:55 ` Nick Desaulniers 2019-03-13 8:13 ` Rasmus Villemoes 2019-03-13 10:58 ` Masahiro Yamada 2019-03-13 13:44 ` Nathan Chancellor 2019-03-13 13:48 ` Arnd Bergmann 2019-03-13 15:32 ` Nathan Chancellor 2019-03-13 17:21 ` Nick Desaulniers 2019-03-13 17:27 ` Nick Desaulniers 2019-03-13 18:02 ` [PATCH] lib/string.c: implement a basic bcmp Nick Desaulniers 2019-03-13 18:11 ` Nick Desaulniers 2019-03-13 18:17 ` [PATCH v2] " Nick Desaulniers 2019-03-13 18:40 ` Steven Rostedt 2019-03-13 18:51 ` Nick Desaulniers 2019-03-13 19:01 ` Steven Rostedt 2019-03-13 19:34 ` Rasmus Villemoes 2019-03-13 20:12 ` Steven Rostedt 2019-03-13 20:37 ` [PATCH v3] " Nick Desaulniers 2019-03-13 21:03 ` Steven Rostedt 2019-03-13 21:13 ` [PATCH v4] " Nick Desaulniers 2019-03-14 3:15 ` Nathan Chancellor 2019-03-14 5:00 ` Masahiro Yamada 2019-03-14 8:33 ` Andy Shevchenko 2019-03-14 9:57 ` David Laight 2019-03-14 11:07 ` Rasmus Villemoes 2019-03-21 2:11 ` Andrew Morton 2019-03-21 17:02 ` Nick Desaulniers 2019-03-21 17:20 ` Nick Desaulniers 2019-03-21 21:05 ` Andrew Morton 2019-03-13 19:38 ` [PATCH] Makefile: Add '-fno-builtin-bcmp' to CLANG_FLAGS Arnd Bergmann [not found] ` <20190325003834.2F24E2133F@mail.kernel.org> 2019-03-25 14:02 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).