Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Sergei Trofimovich <slyich@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2733 bytes --]

On Mon, 11 Jun 2018 14:50:24 +0200
Florian Weimer <fweimer@redhat.com> wrote:

> On 06/11/2018 02:35 PM, Sergei Trofimovich wrote:
> > On Mon, 11 Jun 2018 08:17:37 -0300
> > Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> >   
> >> On 11/06/2018 07:46, Florian Weimer wrote:  
> >>> On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:  
> >>>> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
> >>>> rtld fails to link due to missing memcpy symbol.
> >>>>
> >>>> This happens because .c files are built with -mfpu=neon flag but .S/.s files
> >>>> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
> >>>>
> >>>> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.  
> >>>
> >>> Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.  
> > 
> > I'm not sure just adding an arch flag is enough:
> >      https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy_neon.S;h=1a8d8bbe9e128618105be0652de42a34fbc52d73;hb=HEAD
> >     ...
> >     2 #ifndef __ARM_NEON__
> >     3 # define memcpy __memcpy_neon
> > Here memcpy_neon.S explicitly supports both neon-enabled and disabled flags.  
> 
> Does this mean your proposed patch fails to address the issue, too?

My proposed patch fixes ld.so linkage (and ld.so seems to work).

> The tricky part here is that the IFUNC selector is C, but the 
> alternative implementation is in assembler, so it's not a matter of 
> adding an #include for the rtld build.

As I understand IFUNC selector is not used at all for -mfpu=neon
target (due to "!defined (__ARM_NEON__)"):
    https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy.c;h=02776b6fe6ec9aa6aa7efb8c1770970d73fe50d5;hb=HEAD

  22 #if IS_IN (libc) && !defined (__ARM_NEON__)
  ...
  32 arm_libc_ifunc_redirected (__redirect_memcpy, memcpy, IFUNC_SELECTOR);
  33 
  34 arm_libc_ifunc_hidden_def (__redirect_memcpy, memcpy);
  35 #endif

Or in other words how glibc seems to work today:
- -mfpu=neon -> no IFUNC selection, memcpy_neon.S is supposed to define __memcpy
- no -mfpu=neon -> IFUNC selection is used, memcpy_neon.S is supposed to define __memcpy_neon

> I think a file
> 
> sysdeps/arm/armv7/multiarch/rtld-memcpy.S
> 
> with
> 
> #ifdef __ARM_NEON__
> # include <sysdeps/arm/armv7/multiarch/memcpy_neon.S>
> #else
> # include <sysdeps/arm/armv7/multiarch/memcpy_noneon.S>
> #endif
> 
> might fix this, without affecting the IFUNCs in the libc.so build for 
> !__ARM_NEON__.  Similarly for memchr.
> 
> Thanks,
> Florian

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]