[PATCH v2 0/8] RISC-V: Support GNU indirect functions

Fri Aug 7 03:57:05 GMT 2020

Hi binutils,

I got some good suggestions about ifunc in the previous mails.  Therefore,
I have updated the RISC-V ifunc binutils implementation recently.  The
toolchain regressions and rv64 glibc ifunc testcases are passed.  There
are eight binutils patches as follows,

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

Basically, this is the v1 ifunc implementation.  I just fix a conflict since
the upstream code is changed.  Otherwise, this patch is the same as before.

commit f1dfbfdbc6c07a8ee4ea185a2f15615600857531
elf: Add sym_cache to elf_link_hash_table

Since many ELF backends have sym_cache to their link hash tables, add
sym_cache to elf_link_hash_table.  Also use sdynbss and srelbss in
elf_link_hash_table.

[PATCH-2] RISC-V: Treat R_RISCV_CALL and R_RISCV_CALL_PLT as the same.

In fact, we can treate these two relocation as the same one in the
riscv_elf_check_relocs.  I have heard that RISC-V lld had made this
improvement, and so had GNU AARCH64, they only need the R_AARCH64_CALL26
for calls rather than two seperate relocation.  I have tested the change,
and everything seems fine for now.

[PATCH-3] RISC-V: Resolve the TEXTREL warning and redundant R_RISCV_NONE for IFUNC.

The R_RISCV_PCREL reloc refers to IFUNC symbol may cause redundant
R_RISCV_NONE relocs in the text section.  The TEXTREL warning is issued since
it may result in a segfault at runtime.  To resolve this problem, do not let
R_RISCV_PCREL fall through to the static_reloc check in riscv_elf_check_relocs.
For now, we should check static_reloc only for R_RISCV_32 and R_RISCV_64 relocs.

[PATCH-4] RISC-V: Use NEED_IFUNC_SECTIONS to check if we need IFUNC sections or not.

The generic relocation kind `NEED_IFUNC_SECTIONS` should help to understand
what these relocation are used to do.  We need to create the IFUNC sections,
iplt and ipltgot, for these relocation when generating the static executable.

[PATCH-5] RISC-V: Remove the IFUNC testcases since their name are hard to understand.

I just keep the original naming when porting them from ld/testsuite/ld-ifunc/
before.  But for me at least, the naming are hard to understand and maintain.
Therefore, I remove them in this patch, and then rewrite the RISC-V ifunc
testcases in the following three patches.

[PATCH-6] RISC-V: Rewrite the IFUNC testcases.
[PATCH-7] RISC-V: Consider the different module testcases for IFUNC.

I don't rewrite the tests for the .hidden/.weak IFUNC symbols and
--gc-sections.  I think the ld/testsuite/ld-ifunc C tests may have
tested them, or we can add them in the future patches.  The generic
GNU IFUNC code have helped to handle most things, so I think these
should be run well.  There are three types of IFUNC testcases,

1). ifunc-reloc-*: Only check the single type of relocation refers to
IFUNC symbol.

* ifunc-reloc-call: R_RISCV_CALL and R_RISCV_CALL_PLT.
* ifunc-reloc-data: R_RISCV_32 and R_RISCV_64.
* ifunc-reloc-got: R_RISCV_GOT_HI20 and R_RISCV_PCREL_LO_I/S.
* ifunc-reloc-pcrel: R_RISCV_PCREL_HI20 and R_RISCV_PCREL_LO_I/S.

2). ifunc-[nonplt|plt]-*: Divide the relocation into PLT and non-PLT.
If we don't have PLT relocs, then don't need to create the PLT and
it's .plt entries.

* ifunc-nonplt: ifunc-reloc-got and ifunc-reloc-data.
* ifunc-plt: ifunc-nonplt, ifunc-reloc-call and ifunc-reloc-pcrel.

3).  ifunc-seperate-*: The IFUNC callers and resolver are in the
different modules.  That is, we compile the IFUNC resolver to the
shared library first, and then link it with the IFUNC callers.  The
opposite cases are relatively rare, so I didn't write the testcases
to test them.  The output(result) of IFUNC callers should be the same
as the normal FUNC cases (undef FUNC), and the shared IFUNC resolver
should define the symbols as IFUNC.  The ld.so will decide which module
to load.  If we load the IFUNC resolver module, then ld.so will call
elf_ifunc_invoke to handle the IFUNC.  And if we load the normal FUNC
resolver, then ld.so will deal with them as usual PLT.

[PATCH-8] RISC-V: Change bfd_link_executable back to !bfd_link_pic in check_relocs.
In my first patch, I had changed the !bfd_link_pic to the bfd_link_executable
for PLT static_reloc checking in the check_relocs (x86 also do the same
checks).  But it will cause different PIE results from the previous
toolchain.  The static_reloc check will affect whether to create the PLT and
it's entry for the non-PLT data relocs (R_RISCV_32/64).  For the cases that the
IFUNC callers and resolver are in the same module, the change is fine.  But
consider the different modules cases, the bfd_link_executable change will try
to create PLT for the non-PLT relocs when enabling PIE, but the previous
toolchain won't do that.  Therefore, I prefer to change the checking back to
the !bfd_link_pic to maintain the compatibility.

Therefore, the 3). ifunc-seperate-*,
* ifunc-seperate-resolver.s: The IFUNC resolver.  It will be a shared library.
* ifunc-seperate-caller-[plt|nonplt].s: The IFUNC callers.
* ifunc-seperate-[plt|nonplt]-[exe|pie|pic].d: In the past toolchain, we should
only try to create the PLT for the R_RISCV_32/64 when creating executable (rather
than PIE and PIC).  It should be fixed in the eight patch.


However, there are some difference between ld and lld, but I think both can run
well with Vincent Chen's glibc IFUNC patch.  I will send another mail to talk
about the difference, so we can discuss them later.


[Note-1] If you want to enable the ifunc in GCC, then you need to update the
gcc/config.gcc at least,

>From c7ce689c15cc8b4609abcad476ea330eaff4308f Mon Sep 17 00:00:00 2001
From: Nelson Chu <nelson.chu@sifive.com>
Date: Mon, 18 May 2020 19:01:29 -0700
Subject: [PATCH] RISC-V: Enable ifunc attribute by default for RISC-V.

---
 gcc/config.gcc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index ddd3b8f..a07c5b46 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -3281,7 +3281,7 @@ case ${target} in
         ;;
 *-*-linux*)
        case ${target} in
-       aarch64*-* | arm*-* | i[34567]86-* | powerpc*-* | s390*-* | sparc*-* | x86_64-*)
+       aarch64*-* | arm*-* | i[34567]86-* | powerpc*-* | riscv*-*-* | s390*-* | sparc*-* | x86_64-*)
                default_gnu_indirect_function=yes
                ;;
        esac


[Note-2] Also, Vincent Chen had finished the ifunc in the glibc, and had
sent the patch to glibc upstream.
https://patches-gcc.linaro.org/patch/38531/

Thanks
Nelson

