[RFC] riscv: Add support for STT_GNU_IFUNC symbols
Commit Message
This patch adds the support for STT_GNU_IFUNC symbols to riscv ports. The
Binutils' IFUNC patches are under reviewing and can be found in
https://sourceware.org/pipermail/binutils/2020-July/112186.html.
Any suggestions and feedback are welcome.
---
sysdeps/riscv/dl-irel.h | 53 ++++++++++++++++++++++++++++++++++++++++++++++
sysdeps/riscv/dl-machine.h | 22 ++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletion(-)
create mode 100644 sysdeps/riscv/dl-irel.h
Comments
On Jul 09 2020, Vincent Chen wrote:
> diff --git a/sysdeps/riscv/dl-irel.h b/sysdeps/riscv/dl-irel.h
> new file mode 100644
> index 0000000..9dd8de9
> --- /dev/null
> +++ b/sysdeps/riscv/dl-irel.h
> @@ -0,0 +1,53 @@
> +/* Machine-dependent ELF indirect relocation inline functions.
> + RISC-V version.
> + Copyright (C) 2012-2020 Free Software Foundation, Inc.
I think that should list only 2020 as copyright year.
> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
> index 03db94a..728e6d3 100644
> --- a/sysdeps/riscv/dl-machine.h
> +++ b/sysdeps/riscv/dl-machine.h
> @@ -248,7 +256,12 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
> break;
> }
> #endif
> -
> + case R_RISCV_IRELATIVE:
> + value = map->l_addr + reloc->r_addend;
> + if (__glibc_likely (!skip_ifunc))
> + value = elf_ifunc_invoke (value);
> + *addr_field = value;
> + break;
> case R_RISCV_JUMP_SLOT:
> case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
> *addr_field = value;
Please keep the empty line between each case bock.
Andreas.
On Thu, 9 Jul 2020, Vincent Chen wrote:
> This patch adds the support for STT_GNU_IFUNC symbols to riscv ports. The
> Binutils' IFUNC patches are under reviewing and can be found in
> https://sourceware.org/pipermail/binutils/2020-July/112186.html.
>
> Any suggestions and feedback are welcome.
We have the libc-abis mechanism that's supposed to ensure binaries using
newer ELF features don't run on older glibc versions not implementing
those features. So in theory an architecture newly getting IFUNC support
should get a libc-abis entry (at the end of the file, not with the other
IFUNC entries there). But given the various missing or broken pieces of
this mechanism (EI_ABIVERSION not set in the static linker for most
architectures, possibly not checked by glibc in some cases), that probably
doesn't matter much.
https://sourceware.org/legacy-ml/libc-ports/2010-03/msg00044.html
https://sourceware.org/legacy-ml/libc-alpha/2010-04/msg00016.html
https://sourceware.org/legacy-ml/libc-alpha/2014-01/msg00375.html
On Thu, Jul 9, 2020 at 1:26 AM Vincent Chen <vincent.chen@sifive.com> wrote:
> This patch adds the support for STT_GNU_IFUNC symbols to riscv ports. The
> Binutils' IFUNC patches are under reviewing and can be found in
> https://sourceware.org/pipermail/binutils/2020-July/112186.html.
There is an existing bug report with a patch, though that patch was
never tested with working binutils support. See
https://sourceware.org/bugzilla/show_bug.cgi?id=24868
Hopefully this patch fixes that bug.
Jim
On Thu, Jul 9, 2020 at 4:42 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Jul 09 2020, Vincent Chen wrote:
>
> > diff --git a/sysdeps/riscv/dl-irel.h b/sysdeps/riscv/dl-irel.h
> > new file mode 100644
> > index 0000000..9dd8de9
> > --- /dev/null
> > +++ b/sysdeps/riscv/dl-irel.h
> > @@ -0,0 +1,53 @@
> > +/* Machine-dependent ELF indirect relocation inline functions.
> > + RISC-V version.
> > + Copyright (C) 2012-2020 Free Software Foundation, Inc.
>
> I think that should list only 2020 as copyright year.
>
> > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
> > index 03db94a..728e6d3 100644
> > --- a/sysdeps/riscv/dl-machine.h
> > +++ b/sysdeps/riscv/dl-machine.h
>
> > @@ -248,7 +256,12 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
> > break;
> > }
> > #endif
> > -
> > + case R_RISCV_IRELATIVE:
> > + value = map->l_addr + reloc->r_addend;
> > + if (__glibc_likely (!skip_ifunc))
> > + value = elf_ifunc_invoke (value);
> > + *addr_field = value;
> > + break;
> > case R_RISCV_JUMP_SLOT:
> > case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
> > *addr_field = value;
>
> Please keep the empty line between each case bock.
>
> Andreas.
Thank you for pointing out my mistakes. I will modify them in my next
version patch
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
On Fri, Jul 10, 2020 at 12:40 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 9 Jul 2020, Vincent Chen wrote:
>
> > This patch adds the support for STT_GNU_IFUNC symbols to riscv ports. The
> > Binutils' IFUNC patches are under reviewing and can be found in
> > https://sourceware.org/pipermail/binutils/2020-July/112186.html.
> >
> > Any suggestions and feedback are welcome.
>
> We have the libc-abis mechanism that's supposed to ensure binaries using
> newer ELF features don't run on older glibc versions not implementing
> those features. So in theory an architecture newly getting IFUNC support
> should get a libc-abis entry (at the end of the file, not with the other
> IFUNC entries there). But given the various missing or broken pieces of
> this mechanism (EI_ABIVERSION not set in the static linker for most
> architectures, possibly not checked by glibc in some cases), that probably
> doesn't matter much.
>
> https://sourceware.org/legacy-ml/libc-ports/2010-03/msg00044.html
> https://sourceware.org/legacy-ml/libc-alpha/2010-04/msg00016.html
> https://sourceware.org/legacy-ml/libc-alpha/2014-01/msg00375.html
>
Thanks for your reminder. I will add an entry for riscv toolchain at
the end of libc-abis
On Sat, Jul 11, 2020 at 7:16 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Thu, Jul 9, 2020 at 1:26 AM Vincent Chen <vincent.chen@sifive.com> wrote:
> > This patch adds the support for STT_GNU_IFUNC symbols to riscv ports. The
> > Binutils' IFUNC patches are under reviewing and can be found in
> > https://sourceware.org/pipermail/binutils/2020-July/112186.html.
>
> There is an existing bug report with a patch, though that patch was
> never tested with working binutils support. See
> https://sourceware.org/bugzilla/show_bug.cgi?id=24868
> Hopefully this patch fixes that bug.
>
> Jim
I followed the descriptions in the Bugzilla to create the case and
executed it in the environment with IFUNC support. No matter which
binding scenario is used, (RTLD_NOW or RTLD_LAZY), the execution
results are as expected. Hence I think this patch can fix this issue.
Thank you.
new file mode 100644
@@ -0,0 +1,53 @@
+/* Machine-dependent ELF indirect relocation inline functions.
+ RISC-V version.
+ Copyright (C) 2012-2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _DL_IREL_H
+#define _DL_IREL_H
+
+#include <stdio.h>
+#include <unistd.h>
+#include <ldsodefs.h>
+#include <sysdep.h>
+
+#define ELF_MACHINE_IRELA 1
+
+static inline ElfW(Addr)
+__attribute ((always_inline))
+elf_ifunc_invoke (ElfW(Addr) addr)
+{
+ return ((ElfW(Addr) (*) (uint64_t)) (addr)) (GLRO(dl_hwcap));
+}
+
+static inline void
+__attribute ((always_inline))
+elf_irela (const ElfW(Rela) *reloc)
+{
+ ElfW(Addr) *const reloc_addr = (void *) reloc->r_offset;
+ const unsigned long int r_type = ELFW(R_TYPE) (reloc->r_info);
+
+ if (__glibc_likely (r_type == R_RISCV_IRELATIVE))
+ {
+ ElfW(Addr) value = elf_ifunc_invoke (reloc->r_addend);
+ *reloc_addr = value;
+ }
+ else
+ __libc_fatal ("Unexpected reloc type in static binary.\n");
+}
+
+#endif
@@ -25,6 +25,7 @@
#include <elf/elf.h>
#include <sys/asm.h>
#include <dl-tls.h>
+#include <dl-irel.h>
#ifndef _RTLD_PROLOGUE
# define _RTLD_PROLOGUE(entry) \
@@ -174,6 +175,13 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
if (sym_map != NULL)
value = SYMBOL_ADDRESS (sym_map, sym, true) + reloc->r_addend;
+ if (sym != NULL
+ && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC)
+ && __glibc_likely (sym->st_shndx != SHN_UNDEF)
+ && __glibc_likely (!skip_ifunc))
+ value = elf_ifunc_invoke (value);
+
+
switch (r_type)
{
#ifndef RTLD_BOOTSTRAP
@@ -248,7 +256,12 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
break;
}
#endif
-
+ case R_RISCV_IRELATIVE:
+ value = map->l_addr + reloc->r_addend;
+ if (__glibc_likely (!skip_ifunc))
+ value = elf_ifunc_invoke (value);
+ *addr_field = value;
+ break;
case R_RISCV_JUMP_SLOT:
case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
*addr_field = value;
@@ -290,6 +303,13 @@ elf_machine_lazy_rel (struct link_map *map, ElfW(Addr) l_addr,
else
*reloc_addr = map->l_mach.plt;
}
+ else if (__glibc_unlikely (r_type == R_RISCV_IRELATIVE))
+ {
+ ElfW(Addr) value = map->l_addr + reloc->r_addend;
+ if (__glibc_likely (!skip_ifunc))
+ value = elf_ifunc_invoke (value);
+ *reloc_addr = value;
+ }
else
_dl_reloc_bad_type (map, r_type, 1);
}