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

[Fangrui Song]

On 2020-07-08, Nelson Chu wrote:
>Hi MaskRay,
>
>
>On Wed, Jul 8, 2020 at 3:22 PM Fangrui Song <i@maskray.me> wrote:
>>
>> I think I know ifunc and its implementation in LLD quite well
>> (https://reviews.llvm.org/D74022 ). However, I know little about GNU ld
>> and its terminology, so please bear with me.
>
>I would like to thank you for your suggestions and the great ideas.
>So please feel free to join the discussion.
>
>
>> The test naming is really terrible. It is difficult to understand the
>> purpose of individual files. Can they have more descriptive names?
>
>Agreed... I just keep the original naming when porting them from
>ld/testsuite/ld-ifunc/.  I do plan to improve these, the descriptive
>names should help, or we probably can add some comments to describe
>them in the ld-riscv-elf.exp.  I will add some comments first in the
>ld-riscv-elf.exp, and then will try to give them the more descriptive
>names.  However, thanks for the suggestion.
>
>
>> ifunc-1.s and ifunc-3.s are identical except a visibility setting.
>> Can the tests be somehow combined? For example,
>> echo '.hidden foo' > a.s and then concatenate two assembly files
>
>This may be one of the ways to combine the assembly files.  Another
>way is used in the ifunc-10.s, it should also work.  We can define the
>symbols by "-defsym xxx=1" to assembler, and then use ".ifdef xxx" to
>decide which code patterns are prefered to be generated.  I remember
>not only ifunc-1.s and ifunc-3.s can be combined, so we probably can
>reduce the entire number of the ifunc testcases.
>
>
>> >+      if (h != NULL)
>> >+      {
>> >+        switch (r_type)
>> >+          {
>> >+          case R_RISCV_32:
>> >+          case R_RISCV_64:
>> >+          case R_RISCV_CALL:
>> >+          case R_RISCV_CALL_PLT:
>> >+          case R_RISCV_HI20:
>> >+          case R_RISCV_GOT_HI20:
>> >+          case R_RISCV_PCREL_HI20:
>>
>> The list has appears more than once with variance, I think a generic
>> relocation kind may help.
>
>I assume you mean something like "_bfd_elf_x86_valid_reloc_p" or some
>macros defined in the bfd/elfxx-x86.h.  If so, yeah that's a good
>idea.  I think generic relocation kinds are helpful.
>
>
>> (Off-topic
>> I think -z separate-code being default on linux x86 since binutils 2.31
>> is not the best choice. GNU ld should have --rosegment like gold and LLD.
>> -z separate-code should be responsible for overlapping p_offset
>>
>> https://reviews.llvm.org/D64903)
>
>Thanks for the information.  I will figure this out.
>
>
>Thank you very much, and other suggestions are also welcome.
>Nelson

Hi Nelson,

For this test
https://github.com/llvm/llvm-project/blob/master/lld/test/ELF/riscv-ifunc-nonpreemptible.s

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

Do you know why it reports this (spurious) diagnostic?

% 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

There is also a spurious R_RISCV_NONE dynamic relocation, which should be fixed.

% readelf -Wr a.64

Relocation section '.rela.dyn' at offset 0x210 contains 1 entry:
     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000000  0000000000000000 R_RISCV_NONE                              0

Relocation section '.rela.plt' at offset 0x228 contains 1 entry:
     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000002010  000000000000003a R_RISCV_IRELATIVE                         270


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



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