Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

On 28/08/17 19:25, Steve Ellcey wrote:
> On Fri, 2017-08-25 at 15:37 +0100, Szabolcs Nagy wrote:
> 
>> the use of ifunc in gcc target libraries was a mistake
>> in my opinion, there are several known bugs in the ifunc
>> design and uclibc/musl/bionic don't plan to support it.
>> but at this point i dont have a better proposal for doing
>> runtime selection of optimal atomics code.
>>
>> however in this patch i don't see why would the ctor run
>> before ifunc resolvers. how does this work on x86_64?
>> (there the different 16byte atomics are not even compatible,
>> so if ifunc resolvers in different dsos return different
>> result because one ran before the ctor, another after then
>> the runtime behaviour is broken. this can happen when one
>> dso is bindnow so ifunc relocation is processed before
>> ctors and another runs resolvers lazily or dlopened later..
>> but i didnt test it just looks broken)
> 
> I am not sure I followed everything here.  My basic testing all
> worked, init_cpu_revision got run when libatomic was loaded and
> then the IFUNC resolver gets called after that when one of the IFUNC
> atomic functions is called.  init_cpu_revision sets libat_have_lse
> which, I assume, will not be used by any other libraries.  If other
> libraries have IFUNCs they would have their own static constructors and
> set their own variables to be checked by their own IFUNCs.  So I am
> not sure how one DSO is going to break another DSO.
> 

this can only possibly work with lazy binding of the
libatomic calls (there are many reasons why that may
not be the case starting from static linking to compiling
with -fno-plt or setting LD_BIND_NOW at runtime) as i
expected this is broken on x86 see the execution test
i added to pr 60790:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60790

>> note that aarch64 ifunc resolvers get hwcap as an argument
>> so all this brokenness can be avoided (and if we want to
>> disable hwcap flags then probably glibc should take care
>> of that before passing hwcap to the ifunc resolver).
> 
> I looked at the IFUNC memcpy resolver in glibc and it does not look
> like it gets hwcap as an argument.  When I preprocess everything I see:
> 
> void *__libc_memcpy_ifunc (void) __asm__ ("__libc_memcpy");
> void *__libc_memcpy_ifunc (void)
> {
>   uint64_t __attribute__((unused)) midr = _dl_aarch64_cpu_features.midr_el1;
>   *res = ** expression that looks at midr value and returns function pointer **;
>   return res;
> }
> __asm__ (".type " "__libc_memcpy" ", %gnu_indirect_function");
> extern __typeof (__libc_memcpy) memcpy __attribute__ ((alias ("__libc_memcpy")));
> 

it's a general bug that most ifunc users (e.g. in binutils
tests) declare the ifunc resolvers incorrectly, the correct
prototype is the way the dynamic linker invokes the resolver
in sysdeps/aarch64/dl-irel.h:

static inline ElfW(Addr)
__attribute ((always_inline))
elf_ifunc_invoke (ElfW(Addr) addr)
{
  return ((ElfW(Addr) (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
}

(that argument should be uint64_t for ilp32, but that's a
separate issue)

in glibc the hwcap is not used, because it has accesses to
cached dispatch info, but in libatomic using the hwcap
argument is the right way.