Message ID | 20230901235224.3304592-4-evan@rivosinc.com |
---|---|
State | New |
Headers | show |
Series | RISC-V: ifunced memcpy using new kernel hwprobe interface | expand |
On Fri, Sep 01, 2023 at 04:52:21PM -0700, Evan Green wrote: > The new __riscv_hwprobe() function is designed to be used by ifunc > selector functions. This presents a challenge for applications and > libraries, as ifunc selectors are invoked before all relocations have > been performed, so an external call to __riscv_hwprobe() from an ifunc > selector won't work. To address this, pass a pointer to the > __riscv_hwprobe() vDSO function into ifunc selectors as the second > argument (alongside dl_hwcap, which was already being passed). > > Include a typedef as well for convenience, so that ifunc users don't > have to go through contortions to call this routine. Users will need to > remember to check the second argument for NULL, both to account for > older glibcs that don't pass the function, and older kernels that don't > have the vDSO pointer. This makes the IFUNC interface Linux-specific, which seems gratuitous. Being able to write portable IFUNC resolvers across Linux, BSDs and other similar OSes is valuable, and this would preclude that. In FreeBSD there are no issues with calling functions from IFUNC resolvers; we just process all non-IRELATIVE relocations before IRELATIVE ones. It sounds like glibc could benefit from the same; then you wouldn't have to hack things up like this. Alternatively we could have a standardised "what extensions do I have" interface passed in the pointer instead, but the Linux's hwprobe (with the Linux-defined IDs for every extension Linux chooses to support and poor extensibility for vendors) is not and will never be that. I urge you to not continue down the "who cares about portability" path. Jess > Signed-off-by: Evan Green <evan@rivosinc.com> > > --- > > (no changes since v7) > > Changes in v7: > - Remove __THROW from function pointer type, as it creates warnings > together with __fortified_attr_access. > > sysdeps/riscv/dl-irel.h | 8 ++++---- > sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h | 10 ++++++++++ > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/sysdeps/riscv/dl-irel.h b/sysdeps/riscv/dl-irel.h > index eaeec5467c..2147504458 100644 > --- a/sysdeps/riscv/dl-irel.h > +++ b/sysdeps/riscv/dl-irel.h > @@ -31,10 +31,10 @@ static inline ElfW(Addr) > __attribute ((always_inline)) > elf_ifunc_invoke (ElfW(Addr) addr) > { > - /* The second argument is a void pointer to preserve the extension > - fexibility. */ > - return ((ElfW(Addr) (*) (uint64_t, void *)) (addr)) > - (GLRO(dl_hwcap), NULL); > + /* The third argument is a void pointer to preserve the extension > + flexibility. */ > + return ((ElfW(Addr) (*) (uint64_t, void *, void *)) (addr)) > + (GLRO(dl_hwcap), GLRO(dl_vdso_riscv_hwprobe), NULL); > } > > static inline void > diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h > index aa189a4818..fd3be5a411 100644 > --- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h > +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h > @@ -67,6 +67,16 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count, > __fortified_attr_access (__read_write__, 1, 2) > __fortified_attr_access (__read_only__, 4, 3); > > +/* A pointer to the __riscv_hwprobe vDSO function is passed as the second > + argument to ifunc selector routines. Include a function pointer type for > + convenience in calling the function in those settings. */ > +typedef int (*__riscv_hwprobe_t) (struct riscv_hwprobe *__pairs, size_t __pair_count, > + size_t __cpu_count, unsigned long int *__cpus, > + unsigned int __flags) > + __nonnull ((1)) __wur > + __fortified_attr_access (__read_write__, 1, 2) > + __fortified_attr_access (__read_only__, 4, 3); > + > __END_DECLS > > #endif /* sys/hwprobe.h */ > -- > 2.34.1 >
* Jessica Clarke: > Being able to write portable IFUNC resolvers across Linux, BSDs and > other similar OSes is valuable, and this would preclude that. In FreeBSD > there are no issues with calling functions from IFUNC resolvers; we just > process all non-IRELATIVE relocations before IRELATIVE ones. Even across multiple objects? In glibc, we rely on relocation order by the link editor to put IRELATIVE last, so we don't have that particular problem for IRELATIVE, but we have it for regular symbol-based relocations that happen to refer to IFUNC symbols because the relocation order my not reflect the symbol binding order. Thanks, Florian
On 29 Sep 2023, at 20:40, Florian Weimer <fweimer@redhat.com> wrote: > > * Jessica Clarke: > >> Being able to write portable IFUNC resolvers across Linux, BSDs and >> other similar OSes is valuable, and this would preclude that. In FreeBSD >> there are no issues with calling functions from IFUNC resolvers; we just >> process all non-IRELATIVE relocations before IRELATIVE ones. > > Even across multiple objects? In glibc, we rely on relocation order by > the link editor to put IRELATIVE last, so we don't have that particular > problem for IRELATIVE, but we have it for regular symbol-based > relocations that happen to refer to IFUNC symbols because the relocation > order my not reflect the symbol binding order. No, just within an object, but that’s fine so long as you relocate objects in the order they appear in the link map. But you do need to take care with relocations that happen to refer to IFUNCs, as you say; they get deferred in FreeBSD too. This also means we’re not dependent on the static linker’s relocation order for IRELATIVE. I believe those guarantees are sufficient for allowing an IFUNC resolver to call into libc functions like __riscv_hwprobe if in libc itself or in a library linked against libc. The only issue would be something like libgcc_s needing to do so, but that doesn’t seem like a big deal, and can be worked around if you really need an IFUNC in libgcc_s. Jess
On 2023-09-29, Florian Weimer wrote: >* Jessica Clarke: > >> Being able to write portable IFUNC resolvers across Linux, BSDs and >> other similar OSes is valuable, and this would preclude that. In FreeBSD >> there are no issues with calling functions from IFUNC resolvers; we just >> process all non-IRELATIVE relocations before IRELATIVE ones. > >Even across multiple objects? In glibc, we rely on relocation order by >the link editor to put IRELATIVE last, so we don't have that particular >problem for IRELATIVE, but we have it for regular symbol-based >relocations that happen to refer to IFUNC symbols because the relocation >order my not reflect the symbol binding order. > >Thanks, >Florian As a resolution to https://sourceware.org/bugzilla/show_bug.cgi?id=13302 ("IRELATIVE relocation should come last"), GNU ld places IRELATIVE in .rela.plt (strange, as IRELATIVE is not lazy). I think relyong on this is too brittle, as IRELATIVE may be in .rela.dyn . I have just filed https://sourceware.org/bugzilla/show_bug.cgi?id=30976 ("rtld: resolve ifunc relocations after JUMP_SLOT/GLOB_DAT/etc") based on my examples at https://maskray.me/blog/2021-01-18-gnu-indirect-function#relocation-resolving-order FreeBSD rtld uses multiple phases and in my analysis > The approach turns out to be very robust. Many segfault examples on > glibc work on FreeBSD. glibc rtld probably does not need to do so much. Just refactoring the loop in elf/dynamic-link.h so that IRELATIVE, regardless of .rela.dyn or .rela.plt, will work for my examples. Then the outcome will look good enough to me. for (int ranges_index = 0; ranges_index < 2; ++ranges_index)
diff --git a/sysdeps/riscv/dl-irel.h b/sysdeps/riscv/dl-irel.h index eaeec5467c..2147504458 100644 --- a/sysdeps/riscv/dl-irel.h +++ b/sysdeps/riscv/dl-irel.h @@ -31,10 +31,10 @@ static inline ElfW(Addr) __attribute ((always_inline)) elf_ifunc_invoke (ElfW(Addr) addr) { - /* The second argument is a void pointer to preserve the extension - fexibility. */ - return ((ElfW(Addr) (*) (uint64_t, void *)) (addr)) - (GLRO(dl_hwcap), NULL); + /* The third argument is a void pointer to preserve the extension + flexibility. */ + return ((ElfW(Addr) (*) (uint64_t, void *, void *)) (addr)) + (GLRO(dl_hwcap), GLRO(dl_vdso_riscv_hwprobe), NULL); } static inline void diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h index aa189a4818..fd3be5a411 100644 --- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h @@ -67,6 +67,16 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count, __fortified_attr_access (__read_write__, 1, 2) __fortified_attr_access (__read_only__, 4, 3); +/* A pointer to the __riscv_hwprobe vDSO function is passed as the second + argument to ifunc selector routines. Include a function pointer type for + convenience in calling the function in those settings. */ +typedef int (*__riscv_hwprobe_t) (struct riscv_hwprobe *__pairs, size_t __pair_count, + size_t __cpu_count, unsigned long int *__cpus, + unsigned int __flags) + __nonnull ((1)) __wur + __fortified_attr_access (__read_write__, 1, 2) + __fortified_attr_access (__read_only__, 4, 3); + __END_DECLS #endif /* sys/hwprobe.h */
The new __riscv_hwprobe() function is designed to be used by ifunc selector functions. This presents a challenge for applications and libraries, as ifunc selectors are invoked before all relocations have been performed, so an external call to __riscv_hwprobe() from an ifunc selector won't work. To address this, pass a pointer to the __riscv_hwprobe() vDSO function into ifunc selectors as the second argument (alongside dl_hwcap, which was already being passed). Include a typedef as well for convenience, so that ifunc users don't have to go through contortions to call this routine. Users will need to remember to check the second argument for NULL, both to account for older glibcs that don't pass the function, and older kernels that don't have the vDSO pointer. Signed-off-by: Evan Green <evan@rivosinc.com> --- (no changes since v7) Changes in v7: - Remove __THROW from function pointer type, as it creates warnings together with __fortified_attr_access. sysdeps/riscv/dl-irel.h | 8 ++++---- sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h | 10 ++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-)