[PATCH] RISC-V: Support GNU indirect functions.

[Nelson Chu]

Hi MaskRay,

On Wed, Jul 15, 2020 at 12:44 PM Fangrui Song <i@maskray.me> wrote:
> .text
> .globl func
> .type func, @gnu_indirect_function
> func:
>    ret
>
> .globl _start
> _start:
> .L:
>    auipc a0, %pcrel_hi(func)
>    addi a0, a0, %pcrel_lo(.L)
>
> % ld.lld -pie a.64.o -o a.64  # silent
> % ld-new -pie a.64.o -o a.64
> ....: warning: GNU indirect functions with DT_TEXTREL may result in a segfault at runtime; recompile with -fPIE

The warning and redundant R_RISCV_NOP can be fixed by the following
change, but haven't run all tests.

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index ca0dd11..73f44d8 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -808,9 +808,10 @@ riscv_elf_check_relocs (bfd *abfd, struct
bfd_link_info *info,

          if ((bfd_link_pic (info)
               && (sec->flags & SEC_ALLOC) != 0
               && ((r != NULL && ! r->pc_relative)
                   || (h != NULL
-                      && (! info->symbolic
+                      && (! (bfd_link_pie (info)
+                             || info->symbolic)
                           || h->root.type == bfd_link_hash_defweak
                           || !h->def_regular))))
              || (!bfd_link_pic (info)


> This test checks a non-GOT-non-PLT relocation to a STT_GNU_IFUNC.
> Such relocations can be used to take the address of the indirect function.
> To make sure pointer equality holds (i.e. taking the address from another module will get the same address),
> LLD will create a PLT (called "canonical PLT" among a few linker people),
> change func's type to STT_FUNC, and redirect all references to the PLT.
>
>       4: 0000000000001270     0 FUNC    GLOBAL DEFAULT    7 func
>
> This patch does not change func's type. If you link with ld-new -E -pie a.64.o -o a.64,
> func will be exported. Taking the address of func from another module will break pointer equality.
>
>      20: 00000000000003f0     0 IFUNC   GLOBAL DEFAULT    9 func

Consider Vincent Chen's ifunc support in GLIBC,
https://sourceware.org/pipermail/libc-alpha/2020-July/116059.html

I'm not sure why we need to change the symbol type from STT_IFUNC to
STT_FUNC, and then update the symbol value to the corresponding plt
entry.  If other modules want to call `func`, it will use
R_RISCV_JUMP_SLOT with the undef STT_FUNC `func`.  Glibc will know the
`func` is actually an STT_IFUNC according to the executable (probably
need --export-dynamic) or shared library which defines it, and then
handle the case just like what R_RISCV_IRELATIVE does.  If we want to
call the `ifunc`, and the `ifunc` is defined in the same
executable/shared library, then linker will generate the
R_RISCV_IRELATIVE for the call, it should be worked with or without
changing the STT_IFUNC to STT_FUNC.  So are there any cases that would
fail since we don't change the STT_IFUNC to STT_FUNC, and redirect it
to the plt entry?

I would be happy to do the changes, even if the GNU toolchain (gcc +
binutils + glibc) seems OK for now.  But this wouldn't be the first
time I was wrong, so please feel free to correct me.


> Regarding tests, it'd be nice to check whether this example works
> https://reviews.llvm.org/rG45acc35ac21323bafaf5d4367df10ebc4eed35f4
>
>    // gcc -fno-pie -no-pie a.c
>    // gcc -fPIE -pie a.c
>    #include <stdio.h>
>    static void impl(void) { puts("meow"); }
>    void thefunc(void) __attribute__((ifunc("resolver")));
>    void *resolver(void) { return &impl; }
>    int main(void) {
>      thefunc();
>      void (*theptr)(void) = &thefunc;
>      theptr();
>    }

I assume both `thefunc` and `theptr` should print "meow".  If the
result is expected, then our patches (Vincent Chen's glibc patch and
my binutils patch) should work.


Thanks
Nelson