[RFC] riscv: Add support for STT_GNU_IFUNC symbols

Message ID 1594283178-9047-1-git-send-email-vincent.chen@sifive.com
State New
Headers show
Series
  • [RFC] riscv: Add support for STT_GNU_IFUNC symbols
Related show

Commit Message

Vincent Chen July 9, 2020, 8:26 a.m.
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

-- 
2.7.4

Comments

Andreas Schwab July 9, 2020, 8:42 a.m. | #1
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.

-- 
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."
Joseph Myers July 9, 2020, 4:40 p.m. | #2
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

-- 
Joseph S. Myers
joseph@codesourcery.com
Jim Wilson July 10, 2020, 11:16 p.m. | #3
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
Vincent Chen July 17, 2020, 2:50 a.m. | #4
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."
Vincent Chen July 17, 2020, 2:53 a.m. | #5
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
Vincent Chen July 17, 2020, 3:05 a.m. | #6
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.

Patch

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.
+   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
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
@@ -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);
 }