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.
