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

[Nick Desaulniers <ndesaulniers@google.com>]

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