[PATCH 0/2] riscv: add support for GNU indirect function

[PATCH 0/2] riscv: add support for GNU indirect function

[Vincent Chen]

The riscv Binutils supported GNU indirect function in the commit
02dd9d25. In order to make the whole mechanism work properly, the
first patch adds the required ports to enable Glibc to address the
riscv IRELATIVE relocation. 
In addition, since the dynamic linker resolves the IRELATIVE relocation
before jumping into the executable program, the $gp register has not
been initialized when the IFUNC resolver function is executed. It cause
that some unexpected error may occur when the dynamic loader resolves the
IREALTIVE relocation of the position-dependent executable program. To
solve this problem, a new RISCV specific dynamic entry, DT_RISCV_GP,
created by Binutils is used to record the gp address. The dynamic
linker can use it to initialize $gp register before resolving the
IREALTIVE relocation of the PDE program.
 
The related patch of DT_RISCV_GP in Binutils could be found in
https://sourceware.org/pipermail/binutils/2020-December/114346.html

Vincent Chen (2):
  riscv: Add support for GNU indirect function
  riscv: Initialize $gp before resolving the IRELATIVE relocation

 elf/elf.h                    |  4 +++
 libc-abis                    |  1 +
 sysdeps/riscv/dl-dtprocnum.h | 22 +++++++++++++++
 sysdeps/riscv/dl-irel.h      | 53 ++++++++++++++++++++++++++++++++++++
 sysdeps/riscv/dl-machine.h   | 31 +++++++++++++++++++++
 5 files changed, 111 insertions(+)
 create mode 100644 sysdeps/riscv/dl-dtprocnum.h
 create mode 100644 sysdeps/riscv/dl-irel.h

-- 
2.29.2

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

[Vincent Chen]

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
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);
 }
-- 
2.29.2

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

[Andreas Schwab]

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.

-- 
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."

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

[Florian Weimer]

* 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
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

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

[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?

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

[Florian Weimer]

* 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
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

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

[Szabolcs Nagy]

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.)

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

[Vincent Chen]

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.

[PATCH 2/2] riscv: Initialize $gp before resolving the IRELATIVE relocation

[Vincent Chen]

RISC-V $gp register may be used to access the global variable in
the PDE program. Currently, the initialization of $gp register
is addressed in the _start function. However, the IFUNC resolver is
executed before the _start function. In this condition, if a IFUNC
resolver function uses the uninitialized $gp to access global variable,
an unexpected error will occur. This patch makes ld.so initialize
the $gp with the DT_RISCV_GP dynamic entry before resolving the
IRELATIVE symbol.
---
 elf/elf.h                    |  4 ++++
 sysdeps/riscv/dl-dtprocnum.h | 22 ++++++++++++++++++++++
 sysdeps/riscv/dl-machine.h   |  9 +++++++++
 3 files changed, 35 insertions(+)
 create mode 100644 sysdeps/riscv/dl-dtprocnum.h

diff --git a/elf/elf.h b/elf/elf.h
index 6439c1a4d5..7566da2dca 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -3955,6 +3955,10 @@ enum
 
 #define R_RISCV_NUM		59
 
+/* RISCV specific values for the Dyn d_tag field.  */
+#define DT_RISCV_GP		(DT_LOPROC + 0)
+#define DT_RISCV_NUM		1
+
 /* BPF specific declarations.  */
 
 #define R_BPF_NONE		0	/* No reloc */
diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h
new file mode 100644
index 0000000000..5b5f99571a
--- /dev/null
+++ b/sysdeps/riscv/dl-dtprocnum.h
@@ -0,0 +1,22 @@
+/* Configuration of lookup 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/>.  */
+
+/* Number of extra dynamic section entries for this architecture.  By
+   default there are none.  */
+#define DT_THISPROCNUM  DT_RISCV_NUM
+
diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
index 0ae45dd0c3..4b1b8e1444 100644
--- a/sysdeps/riscv/dl-machine.h
+++ b/sysdeps/riscv/dl-machine.h
@@ -54,6 +54,9 @@
 #define ELF_MACHINE_NO_REL 1
 #define ELF_MACHINE_NO_RELA 0
 
+/* Translate a RISC-V specific dynamic tag to the index in l_info array.  */
+#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM)
+
 /* Return nonzero iff ELF header is compatible with the running host.  */
 static inline int __attribute_used__
 elf_machine_matches_host (const ElfW(Ehdr) *ehdr)
@@ -341,6 +344,12 @@ elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
     }
 #endif
 
+  if (l->l_info[DT_RISCV(GP)])
+    asm (
+      "mv gp, %0\n"
+      :
+      :"r"(D_PTR (l, l_info[DT_RISCV(GP)]))
+    );
   return lazy;
 }
 
-- 
2.29.2

Re: [PATCH 0/2] riscv: add support for GNU indirect function

[Jim Wilson]

On Tue, Dec 1, 2020 at 2:23 AM Vincent Chen <vincent.chen@sifive.com> wrote:

> In addition, since the dynamic linker resolves the IRELATIVE relocation
> before jumping into the executable program, the $gp register has not
> been initialized when the IFUNC resolver function is executed. It cause
> that some unexpected error may occur when the dynamic loader resolves the
> IREALTIVE relocation of the position-dependent executable program. To
> solve this problem, a new RISCV specific dynamic entry, DT_RISCV_GP,
> created by Binutils is used to record the gp address. The dynamic
> linker can use it to initialize $gp register before resolving the
> IREALTIVE relocation of the PDE program.
>

We should discuss DT_RISCV_GP with RISC-V International since we don't own
the ISA.  I opened a psabi issue.
    https://github.com/riscv/riscv-elf-psabi-doc/issues/165
The llvm/lld and FreeBSD developers also care about this stuff.

Jim

Re: [PATCH 0/2] riscv: add support for GNU indirect function

[Jim Wilson]

On Tue, Dec 1, 2020 at 6:52 PM Jim Wilson <jimw@sifive.com> wrote:

> On Tue, Dec 1, 2020 at 2:23 AM Vincent Chen <vincent.chen@sifive.com>
> wrote:
>
>> IREALTIVE relocation of the position-dependent executable program. To
>> solve this problem, a new RISCV specific dynamic entry, DT_RISCV_GP,
>> created by Binutils is used to record the gp address. The dynamic
>> linker can use it to initialize $gp register before resolving the
>> IREALTIVE relocation of the PDE program.
>>
>
> We should discuss DT_RISCV_GP with RISC-V International since we don't own
> the ISA.  I opened a psabi issue.
>     https://github.com/riscv/riscv-elf-psabi-doc/issues/165
> The llvm/lld and FreeBSD developers also care about this stuff.
>

Just to follow up on this, in the psabi discussion, the lld and FreeBSD
linker experts asked for a DT_SYMTAB solution to get the gp value, instead
of adding a new dynamic tag which is an ABI change.

Jim