[1/2] riscv: Add support for GNU indirect function

Message ID 1606818180-17436-2-git-send-email-vincent.chen@sifive.com
State Superseded
Headers
Series riscv: add support for GNU indirect function |

Commit Message

Vincent Chen Dec. 1, 2020, 10:22 a.m. UTC
  Enable riscv Glibc to support GNU indirect function
---
 libc-abis                  |  1 +
 sysdeps/riscv/dl-irel.h    | 53 ++++++++++++++++++++++++++++++++++++++
 sysdeps/riscv/dl-machine.h | 22 ++++++++++++++++
 3 files changed, 76 insertions(+)
 create mode 100644 sysdeps/riscv/dl-irel.h
  

Comments

Andreas Schwab Dec. 1, 2020, 11:14 a.m. UTC | #1
On Dez 01 2020, Vincent Chen wrote:

> Enable riscv Glibc to support GNU indirect function
> ---
>  libc-abis                  |  1 +
>  sysdeps/riscv/dl-irel.h    | 53 ++++++++++++++++++++++++++++++++++++++
>  sysdeps/riscv/dl-machine.h | 22 ++++++++++++++++
>  3 files changed, 76 insertions(+)
>  create mode 100644 sysdeps/riscv/dl-irel.h
>
> diff --git a/libc-abis b/libc-abis
> index e702f6ae24..fb772499b2 100644
> --- a/libc-abis
> +++ b/libc-abis
> @@ -46,5 +46,6 @@ IFUNC		powerpc64-*-linux*
>  IFUNC		powerpc-*-linux*
>  IFUNC		sparc64-*-linux*
>  IFUNC		sparc-*-linux*
> +IFUNC		riscv*-linux*
>  # Absolute (SHN_ABS) symbols working correctly.
>  ABSOLUTE

Please read the comment and follow it.  Always append, never insert.

Andreas.
  
Florian Weimer Dec. 1, 2020, 11:42 a.m. UTC | #2
* Vincent Chen:

> diff --git a/libc-abis b/libc-abis
> index e702f6ae24..fb772499b2 100644
> --- a/libc-abis
> +++ b/libc-abis
> @@ -46,5 +46,6 @@ IFUNC		powerpc64-*-linux*
>  IFUNC		powerpc-*-linux*
>  IFUNC		sparc64-*-linux*
>  IFUNC		sparc-*-linux*
> +IFUNC		riscv*-linux*
>  # Absolute (SHN_ABS) symbols working correctly.
>  ABSOLUTE

Does this really work?  Isn't RISC-V already at the ABSOLUTE level?

Thanks,
Florian
  
Szabolcs Nagy Dec. 1, 2020, 1:13 p.m. UTC | #3
The 12/01/2020 18:22, Vincent Chen wrote:
> +elf_ifunc_invoke (ElfW(Addr) addr)
> +{
> +  return ((ElfW(Addr) (*) (uint64_t)) (addr)) (GLRO(dl_hwcap));

based on the aarch64 experience i'd pass at
least a (void*)0 second arg so once the
hwcap bits are used up the ifunc call abi
can be extended. (note that there is no
other way to pass data to an ifunc resolver
than the arguments: calling extern function
or object symbols don't work.)
  
Vincent Chen Dec. 4, 2020, 3:29 a.m. UTC | #4
On Tue, Dec 1, 2020 at 7:42 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Vincent Chen:
>
> > diff --git a/libc-abis b/libc-abis
> > index e702f6ae24..fb772499b2 100644
> > --- a/libc-abis
> > +++ b/libc-abis
> > @@ -46,5 +46,6 @@ IFUNC               powerpc64-*-linux*
> >  IFUNC                powerpc-*-linux*
> >  IFUNC                sparc64-*-linux*
> >  IFUNC                sparc-*-linux*
> > +IFUNC                riscv*-linux*
> >  # Absolute (SHN_ABS) symbols working correctly.
> >  ABSOLUTE
>
> Does this really work?  Isn't RISC-V already at the ABSOLUTE level?
>
Sorry, I could not really catch what you mean. The SHN_ABS symbols
could work correctly in riscv, so did you mean I should add the entry
"+IFUNC                riscv*-linux*" below the "ABSOLUTE" entry?
  
Vincent Chen Dec. 4, 2020, 3:35 a.m. UTC | #5
On Tue, Dec 1, 2020 at 9:14 PM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 12/01/2020 18:22, Vincent Chen wrote:
> > +elf_ifunc_invoke (ElfW(Addr) addr)
> > +{
> > +  return ((ElfW(Addr) (*) (uint64_t)) (addr)) (GLRO(dl_hwcap));
>
> based on the aarch64 experience i'd pass at
> least a (void*)0 second arg so once the
> hwcap bits are used up the ifunc call abi
> can be extended. (note that there is no
> other way to pass data to an ifunc resolver
> than the arguments: calling extern function
> or object symbols don't work.)

I think this is a good suggestion to reserve the room for future
extension. I will try to implement it in my next version patch. Thank
you very much.
  
Florian Weimer Dec. 4, 2020, 5:57 a.m. UTC | #6
* Vincent Chen:

> On Tue, Dec 1, 2020 at 7:42 PM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Vincent Chen:
>>
>> > diff --git a/libc-abis b/libc-abis
>> > index e702f6ae24..fb772499b2 100644
>> > --- a/libc-abis
>> > +++ b/libc-abis
>> > @@ -46,5 +46,6 @@ IFUNC               powerpc64-*-linux*
>> >  IFUNC                powerpc-*-linux*
>> >  IFUNC                sparc64-*-linux*
>> >  IFUNC                sparc-*-linux*
>> > +IFUNC                riscv*-linux*
>> >  # Absolute (SHN_ABS) symbols working correctly.
>> >  ABSOLUTE
>>
>> Does this really work?  Isn't RISC-V already at the ABSOLUTE level?
>>
> Sorry, I could not really catch what you mean. The SHN_ABS symbols
> could work correctly in riscv, so did you mean I should add the entry
> "+IFUNC                riscv*-linux*" below the "ABSOLUTE" entry?

It's the same issue that Andreas raised.  He knows much more about this
than I do.  I think you have to come up with a new IFUNC_2 ABI version
that comes after ABSOLUTE.

Thanks,
Florian
  

Patch

diff --git a/libc-abis b/libc-abis
index e702f6ae24..fb772499b2 100644
--- a/libc-abis
+++ b/libc-abis
@@ -46,5 +46,6 @@  IFUNC		powerpc64-*-linux*
 IFUNC		powerpc-*-linux*
 IFUNC		sparc64-*-linux*
 IFUNC		sparc-*-linux*
+IFUNC		riscv*-linux*
 # Absolute (SHN_ABS) symbols working correctly.
 ABSOLUTE
diff --git a/sysdeps/riscv/dl-irel.h b/sysdeps/riscv/dl-irel.h
new file mode 100644
index 0000000000..7dcfffae6a
--- /dev/null
+++ b/sysdeps/riscv/dl-irel.h
@@ -0,0 +1,53 @@ 
+/* Machine-dependent ELF indirect relocation inline functions.
+   RISC-V version.
+   Copyright (C) 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 511140864e..0ae45dd0c3 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)						\
@@ -176,6 +177,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
@@ -251,6 +259,13 @@  elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
     }
 #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;
@@ -292,6 +307,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);
 }