[2/2] riscv: Fix R_RISCV_IRELATIVE overwrite and order issues
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Testing passed
|
Commit Message
This commit resolved two issues:
1. When an ifunc is referenced by a pointer, the relocation of
the pointer in .rela.plt would be overwritten by normal ifunc call.
2. R_RISCV_IRELATIVE should come last.
See https://sourceware.org/bugzilla/show_bug.cgi?id=13302
This patch fixes above issues with the implementation similar to x86.
That is, by adding two variables to record the next relocation index for
R_RISCV_IRELATIVE and R_RISCV_JUMP_SLOT.
A previous commit partially fixed the overwrite issue:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=51a8a7c2e3cc0730831963651a55d23d1fae624d
Details below:
There are three ways to access a local (non-preemptible) ifunc:
(1) Through PLT + GOT, i.e. use normal function call.
(2) Through GOT, i.e. use a local function pointer.
(3) Through a global function pointer.
For (1) and (2), a R_RISCV_IRELATIVE is created for GOT entry.
For (3), a R_RISCV_IRELATIVE is created for the global pointer.
The relocations overwrite issue is that, the R_RISCV_IRELATIVE created
for (1) overwrite the relocations for (2) and (3).
The previous commit partially fixed the overwrite issue for (2), but not
for (3).
---
bfd/elfnn-riscv.c | 65 +++++++++----------
ld/testsuite/ld-riscv-elf/ifunc-macro.s | 19 ++++++
.../ifunc-plt-got-overwrite-02-exe.rd | 4 ++
.../ifunc-plt-got-overwrite-02-pic.rd | 11 ++++
.../ifunc-plt-got-overwrite-02-pie.rd | 7 ++
.../ld-riscv-elf/ifunc-plt-got-overwrite-02.d | 16 +++++
.../ld-riscv-elf/ifunc-plt-got-overwrite-02.s | 34 ++++++++++
ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 7 ++
ld/testsuite/ld-riscv-elf/variant_cc-now.d | 4 +-
ld/testsuite/ld-riscv-elf/variant_cc-shared.d | 4 +-
10 files changed, 132 insertions(+), 39 deletions(-)
create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-macro.s
create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
Comments
Can you provide the testcase to show the case which I mentioned in the
pr13302? Which is that an ifunc with a dynamic jump slot calls another
ifunc, and are not in the rel.dyn. Since according to the testcase below,
it seems no requirement to apply this fix though.
Thanks
Nelson
On Mon, May 6, 2024 at 12:46 PM Hau Hsu <hau.hsu@sifive.com> wrote:
> This commit resolved two issues:
>
> 1. When an ifunc is referenced by a pointer, the relocation of
> the pointer in .rela.plt would be overwritten by normal ifunc call.
> 2. R_RISCV_IRELATIVE should come last.
> See https://sourceware.org/bugzilla/show_bug.cgi?id=13302
>
> This patch fixes above issues with the implementation similar to x86.
> That is, by adding two variables to record the next relocation index for
> R_RISCV_IRELATIVE and R_RISCV_JUMP_SLOT.
>
> A previous commit partially fixed the overwrite issue:
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=51a8a7c2e3cc0730831963651a55d23d1fae624d
>
> Details below:
>
> There are three ways to access a local (non-preemptible) ifunc:
> (1) Through PLT + GOT, i.e. use normal function call.
> (2) Through GOT, i.e. use a local function pointer.
> (3) Through a global function pointer.
>
> For (1) and (2), a R_RISCV_IRELATIVE is created for GOT entry.
> For (3), a R_RISCV_IRELATIVE is created for the global pointer.
> The relocations overwrite issue is that, the R_RISCV_IRELATIVE created
> for (1) overwrite the relocations for (2) and (3).
> The previous commit partially fixed the overwrite issue for (2), but not
> for (3).
> ---
> bfd/elfnn-riscv.c | 65 +++++++++----------
> ld/testsuite/ld-riscv-elf/ifunc-macro.s | 19 ++++++
> .../ifunc-plt-got-overwrite-02-exe.rd | 4 ++
> .../ifunc-plt-got-overwrite-02-pic.rd | 11 ++++
> .../ifunc-plt-got-overwrite-02-pie.rd | 7 ++
> .../ld-riscv-elf/ifunc-plt-got-overwrite-02.d | 16 +++++
> .../ld-riscv-elf/ifunc-plt-got-overwrite-02.s | 34 ++++++++++
> ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 7 ++
> ld/testsuite/ld-riscv-elf/variant_cc-now.d | 4 +-
> ld/testsuite/ld-riscv-elf/variant_cc-shared.d | 4 +-
> 10 files changed, 132 insertions(+), 39 deletions(-)
> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-macro.s
> create mode 100644
> ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
> create mode 100644
> ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
> create mode 100644
> ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 3a30b7a4bd9..d9243648d24 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -224,8 +224,12 @@ struct riscv_elf_link_hash_table
> htab_t loc_hash_table;
> void * loc_hash_memory;
>
> - /* The index of the last unused .rel.iplt slot. */
> - bfd_vma last_iplt_index;
> + /* The index of the next R_RISCV_JUMP_SLOT entry in .rela.plt. */
> + bfd_vma next_jump_slot_index;
> +
> + /* R_RISCV_IRELATIVE entry in .rela.plt comes last.
> + Use this to record the index of the next R_RISCV_IRELATIVE entry. */
> + bfd_vma next_irelative_index;
>
> /* The data segment phase, don't relax the section
> when it is exp_seg_relro_adjust. */
> @@ -1273,6 +1277,7 @@ allocate_dynrelocs (struct elf_link_hash_entry *h,
> void *inf)
>
> /* We also need to make an entry in the .rela.plt section. */
> htab->elf.srelplt->size += sizeof (ElfNN_External_Rela);
> + htab->elf.srelplt->reloc_count ++;
>
> /* If this symbol is not defined in a regular file, and we are
> not generating a shared library, then set the symbol to this
> @@ -1622,10 +1627,14 @@ riscv_elf_late_size_sections (bfd *output_bfd,
> struct bfd_link_info *info)
> local ifunc symbols. */
> htab_traverse (htab->loc_hash_table, allocate_local_ifunc_dynrelocs,
> info);
>
> - /* Used to resolve the dynamic relocs overwite problems when
> - generating static executable. */
> - if (htab->elf.irelplt)
> - htab->last_iplt_index = htab->elf.irelplt->reloc_count - 1;
> +
> + htab->next_jump_slot_index = 0;
> +
> + if (htab->elf.srelplt)
> + htab->next_irelative_index = htab->elf.srelplt->reloc_count - 1;
> + else if (htab->elf.irelplt)
> + htab->next_irelative_index = htab->elf.irelplt->reloc_count - 1;
> +
>
> if (htab->elf.sgotplt)
> {
> @@ -3201,15 +3210,14 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>
> if (h->plt.offset != (bfd_vma) -1)
> {
> - /* We've decided to create a PLT entry for this symbol. */
> + /* We've decided to create a PLT entry for this symbol.
> + Add a PLT entry, a GOT entry and a relocation entry. */
> bfd_byte *loc;
> bfd_vma i, header_address, plt_idx, got_offset, got_address;
> uint32_t plt_entry[PLT_ENTRY_INSNS];
> Elf_Internal_Rela rela;
> asection *plt, *gotplt, *relplt;
>
> - /* When building a static executable, use .iplt, .igot.plt and
> - .rela.iplt sections for STT_GNU_IFUNC symbols. */
> if (htab->elf.splt != NULL)
> {
> plt = htab->elf.splt;
> @@ -3218,6 +3226,8 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> }
> else
> {
> + /* When building a static executable, use .iplt, .igot.plt and
> + .rela.iplt sections for STT_GNU_IFUNC symbols. */
> plt = htab->elf.iplt;
> gotplt = htab->elf.igotplt;
> relplt = htab->elf.irelplt;
> @@ -3238,7 +3248,8 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> header_address = sec_addr (plt);
>
> /* Calculate the index of the entry and the offset of .got.plt
> entry.
> - For static executables, we don't reserve anything. */
> + The index of .got.plt is the same as .plt (exclude the header
> entries),
> + since this section is dedicated for plt. */
> if (plt == htab->elf.splt)
> {
> plt_idx = (h->plt.offset - PLT_HEADER_SIZE) / PLT_ENTRY_SIZE;
> @@ -3246,6 +3257,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> }
> else
> {
> + /* For static executables, we don't need to reserve the header
> entry. */
> plt_idx = h->plt.offset / PLT_ENTRY_SIZE;
> got_offset = plt_idx * GOT_ENTRY_SIZE;
> }
> @@ -3269,6 +3281,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> loc = gotplt->contents + (got_address - sec_addr (gotplt));
> bfd_put_NN (output_bfd, sec_addr (plt), loc);
>
> + /* Fill in the relocation entry. */
> rela.r_offset = got_address;
> if (PLT_LOCAL_IFUNC_P (info, h))
> {
> @@ -3283,17 +3296,20 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> rela.r_addend = h->root.u.def.value
> + sec->output_section->vma
> + sec->output_offset;
> + bfd_vma rela_idx = htab->next_irelative_index--;
> + loc = relplt->contents + rela_idx * sizeof
> (ElfNN_External_Rela);
> + bed->s->swap_reloca_out (output_bfd, &rela, loc);
> }
> else
> {
> /* Fill in the entry in the .rela.plt section. */
> rela.r_info = ELFNN_R_INFO (h->dynindx, R_RISCV_JUMP_SLOT);
> rela.r_addend = 0;
> + bfd_vma rela_idx = htab->next_jump_slot_index++;
> + loc = relplt->contents + rela_idx * sizeof
> (ElfNN_External_Rela);
> + bed->s->swap_reloca_out (output_bfd, &rela, loc);
> }
>
> - loc = relplt->contents + plt_idx * sizeof (ElfNN_External_Rela);
> - bed->s->swap_reloca_out (output_bfd, &rela, loc);
> -
> if (!h->def_regular)
> {
> /* Mark the symbol as undefined, rather than as defined in
> @@ -3315,7 +3331,6 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> asection *sgot;
> asection *srela;
> Elf_Internal_Rela rela;
> - bool use_elf_append_rela = true;
>
> /* This symbol has an entry in the GOT. Set it up. */
>
> @@ -3338,10 +3353,6 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> /* Use .rela.iplt section to store .got relocations
> in static executable. */
> srela = htab->elf.irelplt;
> -
> - /* Do not use riscv_elf_append_rela to add dynamic
> - relocs. */
> - use_elf_append_rela = false;
> }
>
> if (SYMBOL_REFERENCES_LOCAL (info, h))
> @@ -3417,23 +3428,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
> bfd_put_NN (output_bfd, 0,
> sgot->contents + (h->got.offset & ~(bfd_vma) 1));
>
> - if (use_elf_append_rela)
> - riscv_elf_append_rela (output_bfd, srela, &rela);
> - else
> - {
> - /* Use riscv_elf_append_rela to add the dynamic relocs into
> - .rela.iplt may cause the overwrite problems. Since we insert
> - the relocs for PLT didn't handle the reloc_index of
> .rela.iplt,
> - but the riscv_elf_append_rela adds the relocs to the place
> - that are calculated from the reloc_index (in seqential).
> -
> - One solution is that add these dynamic relocs (GOT IFUNC)
> - from the last of .rela.iplt section. */
> - bfd_vma iplt_idx = htab->last_iplt_index--;
> - bfd_byte *loc = srela->contents
> - + iplt_idx * sizeof (ElfNN_External_Rela);
> - bed->s->swap_reloca_out (output_bfd, &rela, loc);
> - }
> + riscv_elf_append_rela (output_bfd, srela, &rela);
> }
>
> if (h->needs_copy)
> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-macro.s
> b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
> new file mode 100644
> index 00000000000..88935a55814
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
> @@ -0,0 +1,19 @@
> +/* Define macros to handle similar behaviors for rv32/rv64.
> + Assumes macro "__64_bit__" defined for rv64.
> + The macro is specifically defined for ifunc tests in ld-riscv-elf.exp.
> */
> +
> +.macro PTR_DATA name
> +.ifdef __64_bit__
> + .quad \name
> +.else
> + .long \name
> +.endif
> +.endm
> +
> +.macro LOAD rd, rs, offset
> +.ifdef __64_bit__
> + ld \rd, \offset (\rs)
> +.else
> + lw \rd, \offset (\rs)
> +.endif
> +.endm
> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
> b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
> new file mode 100644
> index 00000000000..0de47a4009f
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
> @@ -0,0 +1,4 @@
> +Relocation section '.rela.plt' at .*
> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
> b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
> new file mode 100644
> index 00000000000..d7ecd4a4a69
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
> @@ -0,0 +1,11 @@
> +Relocation section '.rela.got' at .*
> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_(32|64)[ ]+foo1\(\)[
> ]+foo1 \+ 0
> +#...
> +Relocation section '.rela.ifunc' at .*
> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_(32|64)[ ]+foo2\(\)[
> ]+foo2 \+ 0
> +#...
> +Relocation section '.rela.plt' at .*
> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+foo1\(\)[
> ]+foo1 \+ 0
> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
> b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
> new file mode 100644
> index 00000000000..532cbf8a86a
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
> @@ -0,0 +1,7 @@
> +Relocation section '.rela.ifunc' at .*
> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
> +
> +Relocation section '.rela.plt' at .*
> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
> b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
> new file mode 100644
> index 00000000000..3e33ac619d4
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
> @@ -0,0 +1,16 @@
> +#...
> +Disassembly of section .plt:
> +#...
> +0+[0-9a-f]+ <(\*ABS\*\+0x[0-9a-f]+@plt|foo@plt|.plt)>:
> +#...
> +Disassembly of section .text:
> +#...
> +0+[0-9a-f]+ <foo_resolver>:
> +.*:[ ]+[0-9a-f]+[ ]+ret
> +
> +0+[0-9a-f]+ <_start>:
> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+.*
> +.*:[ ]+[0-9a-f]+[ ]+(lw|ld)[ ]+.*<(.*)>
> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+.*
> +.*:[ ]+[0-9a-f]+[ ]+jalr[ ]+.*<(.*plt.*)>
> +.*:[ ]+[0-9a-f]+[ ]+ret
> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
> b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
> new file mode 100644
> index 00000000000..ff6d171591c
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
> @@ -0,0 +1,34 @@
> +/* There are 2 ifuncs: foo1 and foo2.
> + foo1 is accessed by a function call.
> + foo2 is referenced by a global pointer (foo2_addr). */
> + .include "ifunc-macro.s"
> + .globl foo2_addr
> + .section .data
> + .type foo2_addr, @object
> +foo2_addr:
> + PTR_DATA foo2
> +
> + .text
> + .type foo_resolver, @function
> +foo_resolver:
> + ret
> + .size foo_resolver, .-foo_resolver
> +
> + .globl foo1
> + .type foo1, %gnu_indirect_function
> + .set foo1, foo_resolver
> +
> + .globl foo2
> + .type foo2, %gnu_indirect_function
> + .set foo2, foo_resolver
> +
> + .globl _start
> + .type _start, @function
> +_start:
> +.L1:
> + auipc x1, %got_pcrel_hi (foo1)
> + LOAD x1, x1, %pcrel_lo (.L1)
> + call foo1
> +
> + ret
> + .size _start, .-_start
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index a1dd0e5e37e..10011445683 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -284,6 +284,13 @@ if [istarget "riscv*-*-*"] {
> run_dump_test_ifunc "ifunc-plt-got-overwrite" rv64 pie
> run_dump_test_ifunc "ifunc-plt-got-overwrite" rv64 pic
>
> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 exe
> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 pie
> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 pic
> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 exe
> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 pie
> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 pic
> +
> # TODO: Make the following tests work under RV32.
> if [istarget "riscv32-*-*"] {
> return
> diff --git a/ld/testsuite/ld-riscv-elf/variant_cc-now.d
> b/ld/testsuite/ld-riscv-elf/variant_cc-now.d
> index 9453554a159..3ed5ab3cae0 100644
> --- a/ld/testsuite/ld-riscv-elf/variant_cc-now.d
> +++ b/ld/testsuite/ld-riscv-elf/variant_cc-now.d
> @@ -7,11 +7,11 @@ Relocation section '.rela.plt' at .*
> #...
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[
> ]+nocc_global_default_undef \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[
> ]+cc_global_default_undef \+ 0
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[
> ]+cc_global_default_def \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[
> ]+nocc_global_default_def \+ 0
> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
> diff --git a/ld/testsuite/ld-riscv-elf/variant_cc-shared.d
> b/ld/testsuite/ld-riscv-elf/variant_cc-shared.d
> index ffb69a392f2..6b73cf1de20 100644
> --- a/ld/testsuite/ld-riscv-elf/variant_cc-shared.d
> +++ b/ld/testsuite/ld-riscv-elf/variant_cc-shared.d
> @@ -7,11 +7,11 @@ Relocation section '.rela.plt' at .*
> #...
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[
> ]+nocc_global_default_undef \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[
> ]+cc_global_default_undef \+ 0
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[
> ]+cc_global_default_def \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[
> ]+nocc_global_default_def \+ 0
> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[
> ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
> --
> 2.37.1
>
>
Hi Nelson,
Sorry for the late reply.
> On May 8, 2024, at 9:00 AM, Nelson Chu <nelson@rivosinc.com> wrote:
>
> Can you provide the testcase to show the case which I mentioned in the pr13302? Which is that an ifunc with a dynamic jump slot calls another ifunc, and are not in the rel.dyn.
I am not quite sure what's the issue you mentioned in PR13302.
You mean the order issue of cause by one ifunc (generates JUMP_SLOT[32|64]) calls another JUMP_SLOT[32|64] ifunc?
> Since according to the testcase below, it seems no requirement to apply this fix though.
The new test case uses two methods to call ifuncs:
1. Through a normal function call: PLT + GOT
2. Through a global function pointer: GOT only
Without the fix, the relocation of the first method overwrites the second's.
The relocation section of my test case would be:
Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
Offset Info Type Sym. Value Symbol's Name + Addend
000110dc 0000003a R_RISCV_IRELATIVE 100c0
00000000 00000000 R_RISCV_NONE 0
With the fix, it becomes
Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
Offset Info Type Sym. Value Symbol's Name + Addend
000110e0 0000003a R_RISCV_IRELATIVE 100c0
000110dc 0000003a R_RISCV_IRELATIVE 100c0
>
> Thanks
> Nelson
>
> On Mon, May 6, 2024 at 12:46 PM Hau Hsu <hau.hsu@sifive.com <mailto:hau.hsu@sifive.com>> wrote:
>> This commit resolved two issues:
>>
>> 1. When an ifunc is referenced by a pointer, the relocation of
>> the pointer in .rela.plt would be overwritten by normal ifunc call.
>> 2. R_RISCV_IRELATIVE should come last.
>> See https://sourceware.org/bugzilla/show_bug.cgi?id=13302
>>
>> This patch fixes above issues with the implementation similar to x86.
>> That is, by adding two variables to record the next relocation index for
>> R_RISCV_IRELATIVE and R_RISCV_JUMP_SLOT.
>>
>> A previous commit partially fixed the overwrite issue:
>> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=51a8a7c2e3cc0730831963651a55d23d1fae624d
>>
>> Details below:
>>
>> There are three ways to access a local (non-preemptible) ifunc:
>> (1) Through PLT + GOT, i.e. use normal function call.
>> (2) Through GOT, i.e. use a local function pointer.
>> (3) Through a global function pointer.
>>
>> For (1) and (2), a R_RISCV_IRELATIVE is created for GOT entry.
>> For (3), a R_RISCV_IRELATIVE is created for the global pointer.
>> The relocations overwrite issue is that, the R_RISCV_IRELATIVE created
>> for (1) overwrite the relocations for (2) and (3).
>> The previous commit partially fixed the overwrite issue for (2), but not
>> for (3).
>> ---
>> bfd/elfnn-riscv.c | 65 +++++++++----------
>> ld/testsuite/ld-riscv-elf/ifunc-macro.s | 19 ++++++
>> .../ifunc-plt-got-overwrite-02-exe.rd | 4 ++
>> .../ifunc-plt-got-overwrite-02-pic.rd | 11 ++++
>> .../ifunc-plt-got-overwrite-02-pie.rd | 7 ++
>> .../ld-riscv-elf/ifunc-plt-got-overwrite-02.d | 16 +++++
>> .../ld-riscv-elf/ifunc-plt-got-overwrite-02.s | 34 ++++++++++
>> ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 7 ++
>> ld/testsuite/ld-riscv-elf/variant_cc-now.d | 4 +-
>> ld/testsuite/ld-riscv-elf/variant_cc-shared.d | 4 +-
>> 10 files changed, 132 insertions(+), 39 deletions(-)
>> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-macro.s
>> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
>> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
>> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
>> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
>> create mode 100644 ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
>>
>> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
>> index 3a30b7a4bd9..d9243648d24 100644
>> --- a/bfd/elfnn-riscv.c
>> +++ b/bfd/elfnn-riscv.c
>> @@ -224,8 +224,12 @@ struct riscv_elf_link_hash_table
>> htab_t loc_hash_table;
>> void * loc_hash_memory;
>>
>> - /* The index of the last unused .rel.iplt slot. */
>> - bfd_vma last_iplt_index;
>> + /* The index of the next R_RISCV_JUMP_SLOT entry in .rela.plt. */
>> + bfd_vma next_jump_slot_index;
>> +
>> + /* R_RISCV_IRELATIVE entry in .rela.plt comes last.
>> + Use this to record the index of the next R_RISCV_IRELATIVE entry. */
>> + bfd_vma next_irelative_index;
>>
>> /* The data segment phase, don't relax the section
>> when it is exp_seg_relro_adjust. */
>> @@ -1273,6 +1277,7 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
>>
>> /* We also need to make an entry in the .rela.plt section. */
>> htab->elf.srelplt->size += sizeof (ElfNN_External_Rela);
>> + htab->elf.srelplt->reloc_count ++;
>>
>> /* If this symbol is not defined in a regular file, and we are
>> not generating a shared library, then set the symbol to this
>> @@ -1622,10 +1627,14 @@ riscv_elf_late_size_sections (bfd *output_bfd, struct bfd_link_info *info)
>> local ifunc symbols. */
>> htab_traverse (htab->loc_hash_table, allocate_local_ifunc_dynrelocs, info);
>>
>> - /* Used to resolve the dynamic relocs overwite problems when
>> - generating static executable. */
>> - if (htab->elf.irelplt)
>> - htab->last_iplt_index = htab->elf.irelplt->reloc_count - 1;
>> +
>> + htab->next_jump_slot_index = 0;
>> +
>> + if (htab->elf.srelplt)
>> + htab->next_irelative_index = htab->elf.srelplt->reloc_count - 1;
>> + else if (htab->elf.irelplt)
>> + htab->next_irelative_index = htab->elf.irelplt->reloc_count - 1;
>> +
>>
>> if (htab->elf.sgotplt)
>> {
>> @@ -3201,15 +3210,14 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>>
>> if (h->plt.offset != (bfd_vma) -1)
>> {
>> - /* We've decided to create a PLT entry for this symbol. */
>> + /* We've decided to create a PLT entry for this symbol.
>> + Add a PLT entry, a GOT entry and a relocation entry. */
>> bfd_byte *loc;
>> bfd_vma i, header_address, plt_idx, got_offset, got_address;
>> uint32_t plt_entry[PLT_ENTRY_INSNS];
>> Elf_Internal_Rela rela;
>> asection *plt, *gotplt, *relplt;
>>
>> - /* When building a static executable, use .iplt, .igot.plt and
>> - .rela.iplt sections for STT_GNU_IFUNC symbols. */
>> if (htab->elf.splt != NULL)
>> {
>> plt = htab->elf.splt;
>> @@ -3218,6 +3226,8 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> }
>> else
>> {
>> + /* When building a static executable, use .iplt, .igot.plt and
>> + .rela.iplt sections for STT_GNU_IFUNC symbols. */
>> plt = htab->elf.iplt;
>> gotplt = htab->elf.igotplt;
>> relplt = htab->elf.irelplt;
>> @@ -3238,7 +3248,8 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> header_address = sec_addr (plt);
>>
>> /* Calculate the index of the entry and the offset of .got.plt entry.
>> - For static executables, we don't reserve anything. */
>> + The index of .got.plt is the same as .plt (exclude the header entries),
>> + since this section is dedicated for plt. */
>> if (plt == htab->elf.splt)
>> {
>> plt_idx = (h->plt.offset - PLT_HEADER_SIZE) / PLT_ENTRY_SIZE;
>> @@ -3246,6 +3257,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> }
>> else
>> {
>> + /* For static executables, we don't need to reserve the header entry. */
>> plt_idx = h->plt.offset / PLT_ENTRY_SIZE;
>> got_offset = plt_idx * GOT_ENTRY_SIZE;
>> }
>> @@ -3269,6 +3281,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> loc = gotplt->contents + (got_address - sec_addr (gotplt));
>> bfd_put_NN (output_bfd, sec_addr (plt), loc);
>>
>> + /* Fill in the relocation entry. */
>> rela.r_offset = got_address;
>> if (PLT_LOCAL_IFUNC_P (info, h))
>> {
>> @@ -3283,17 +3296,20 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> rela.r_addend = h->root.u.def.value
>> + sec->output_section->vma
>> + sec->output_offset;
>> + bfd_vma rela_idx = htab->next_irelative_index--;
>> + loc = relplt->contents + rela_idx * sizeof (ElfNN_External_Rela);
>> + bed->s->swap_reloca_out (output_bfd, &rela, loc);
>> }
>> else
>> {
>> /* Fill in the entry in the .rela.plt section. */
>> rela.r_info = ELFNN_R_INFO (h->dynindx, R_RISCV_JUMP_SLOT);
>> rela.r_addend = 0;
>> + bfd_vma rela_idx = htab->next_jump_slot_index++;
>> + loc = relplt->contents + rela_idx * sizeof (ElfNN_External_Rela);
>> + bed->s->swap_reloca_out (output_bfd, &rela, loc);
>> }
>>
>> - loc = relplt->contents + plt_idx * sizeof (ElfNN_External_Rela);
>> - bed->s->swap_reloca_out (output_bfd, &rela, loc);
>> -
>> if (!h->def_regular)
>> {
>> /* Mark the symbol as undefined, rather than as defined in
>> @@ -3315,7 +3331,6 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> asection *sgot;
>> asection *srela;
>> Elf_Internal_Rela rela;
>> - bool use_elf_append_rela = true;
>>
>> /* This symbol has an entry in the GOT. Set it up. */
>>
>> @@ -3338,10 +3353,6 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> /* Use .rela.iplt section to store .got relocations
>> in static executable. */
>> srela = htab->elf.irelplt;
>> -
>> - /* Do not use riscv_elf_append_rela to add dynamic
>> - relocs. */
>> - use_elf_append_rela = false;
>> }
>>
>> if (SYMBOL_REFERENCES_LOCAL (info, h))
>> @@ -3417,23 +3428,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
>> bfd_put_NN (output_bfd, 0,
>> sgot->contents + (h->got.offset & ~(bfd_vma) 1));
>>
>> - if (use_elf_append_rela)
>> - riscv_elf_append_rela (output_bfd, srela, &rela);
>> - else
>> - {
>> - /* Use riscv_elf_append_rela to add the dynamic relocs into
>> - .rela.iplt may cause the overwrite problems. Since we insert
>> - the relocs for PLT didn't handle the reloc_index of .rela.iplt,
>> - but the riscv_elf_append_rela adds the relocs to the place
>> - that are calculated from the reloc_index (in seqential).
>> -
>> - One solution is that add these dynamic relocs (GOT IFUNC)
>> - from the last of .rela.iplt section. */
>> - bfd_vma iplt_idx = htab->last_iplt_index--;
>> - bfd_byte *loc = srela->contents
>> - + iplt_idx * sizeof (ElfNN_External_Rela);
>> - bed->s->swap_reloca_out (output_bfd, &rela, loc);
>> - }
>> + riscv_elf_append_rela (output_bfd, srela, &rela);
>> }
>>
>> if (h->needs_copy)
>> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-macro.s b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
>> new file mode 100644
>> index 00000000000..88935a55814
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
>> @@ -0,0 +1,19 @@
>> +/* Define macros to handle similar behaviors for rv32/rv64.
>> + Assumes macro "__64_bit__" defined for rv64.
>> + The macro is specifically defined for ifunc tests in ld-riscv-elf.exp. */
>> +
>> +.macro PTR_DATA name
>> +.ifdef __64_bit__
>> + .quad \name
>> +.else
>> + .long \name
>> +.endif
>> +.endm
>> +
>> +.macro LOAD rd, rs, offset
>> +.ifdef __64_bit__
>> + ld \rd, \offset (\rs)
>> +.else
>> + lw \rd, \offset (\rs)
>> +.endif
>> +.endm
>> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
>> new file mode 100644
>> index 00000000000..0de47a4009f
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-exe.rd
>> @@ -0,0 +1,4 @@
>> +Relocation section '.rela.plt' at .*
>> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
>> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
>> new file mode 100644
>> index 00000000000..d7ecd4a4a69
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pic.rd
>> @@ -0,0 +1,11 @@
>> +Relocation section '.rela.got' at .*
>> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_(32|64)[ ]+foo1\(\)[ ]+foo1 \+ 0
>> +#...
>> +Relocation section '.rela.ifunc' at .*
>> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_(32|64)[ ]+foo2\(\)[ ]+foo2 \+ 0
>> +#...
>> +Relocation section '.rela.plt' at .*
>> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+foo1\(\)[ ]+foo1 \+ 0
>> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
>> new file mode 100644
>> index 00000000000..532cbf8a86a
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02-pie.rd
>> @@ -0,0 +1,7 @@
>> +Relocation section '.rela.ifunc' at .*
>> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
>> +
>> +Relocation section '.rela.plt' at .*
>> +[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
>> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
>> new file mode 100644
>> index 00000000000..3e33ac619d4
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.d
>> @@ -0,0 +1,16 @@
>> +#...
>> +Disassembly of section .plt:
>> +#...
>> +0+[0-9a-f]+ <(\*ABS\*\+0x[0-9a-f]+@plt|foo@plt|.plt)>:
>> +#...
>> +Disassembly of section .text:
>> +#...
>> +0+[0-9a-f]+ <foo_resolver>:
>> +.*:[ ]+[0-9a-f]+[ ]+ret
>> +
>> +0+[0-9a-f]+ <_start>:
>> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+.*
>> +.*:[ ]+[0-9a-f]+[ ]+(lw|ld)[ ]+.*<(.*)>
>> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+.*
>> +.*:[ ]+[0-9a-f]+[ ]+jalr[ ]+.*<(.*plt.*)>
>> +.*:[ ]+[0-9a-f]+[ ]+ret
>> diff --git a/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
>> new file mode 100644
>> index 00000000000..ff6d171591c
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-plt-got-overwrite-02.s
>> @@ -0,0 +1,34 @@
>> +/* There are 2 ifuncs: foo1 and foo2.
>> + foo1 is accessed by a function call.
>> + foo2 is referenced by a global pointer (foo2_addr). */
>> + .include "ifunc-macro.s"
>> + .globl foo2_addr
>> + .section .data
>> + .type foo2_addr, @object
>> +foo2_addr:
>> + PTR_DATA foo2
>> +
>> + .text
>> + .type foo_resolver, @function
>> +foo_resolver:
>> + ret
>> + .size foo_resolver, .-foo_resolver
>> +
>> + .globl foo1
>> + .type foo1, %gnu_indirect_function
>> + .set foo1, foo_resolver
>> +
>> + .globl foo2
>> + .type foo2, %gnu_indirect_function
>> + .set foo2, foo_resolver
>> +
>> + .globl _start
>> + .type _start, @function
>> +_start:
>> +.L1:
>> + auipc x1, %got_pcrel_hi (foo1)
>> + LOAD x1, x1, %pcrel_lo (.L1)
>> + call foo1
>> +
>> + ret
>> + .size _start, .-_start
>> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> index a1dd0e5e37e..10011445683 100644
>> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> @@ -284,6 +284,13 @@ if [istarget "riscv*-*-*"] {
>> run_dump_test_ifunc "ifunc-plt-got-overwrite" rv64 pie
>> run_dump_test_ifunc "ifunc-plt-got-overwrite" rv64 pic
>>
>> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 exe
>> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 pie
>> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 pic
>> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 exe
>> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 pie
>> + run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 pic
>> +
>> # TODO: Make the following tests work under RV32.
>> if [istarget "riscv32-*-*"] {
>> return
>> diff --git a/ld/testsuite/ld-riscv-elf/variant_cc-now.d b/ld/testsuite/ld-riscv-elf/variant_cc-now.d
>> index 9453554a159..3ed5ab3cae0 100644
>> --- a/ld/testsuite/ld-riscv-elf/variant_cc-now.d
>> +++ b/ld/testsuite/ld-riscv-elf/variant_cc-now.d
>> @@ -7,11 +7,11 @@ Relocation section '.rela.plt' at .*
>> #...
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+nocc_global_default_undef \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+cc_global_default_undef \+ 0
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+cc_global_default_def \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+nocc_global_default_def \+ 0
>> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
>> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
>> diff --git a/ld/testsuite/ld-riscv-elf/variant_cc-shared.d b/ld/testsuite/ld-riscv-elf/variant_cc-shared.d
>> index ffb69a392f2..6b73cf1de20 100644
>> --- a/ld/testsuite/ld-riscv-elf/variant_cc-shared.d
>> +++ b/ld/testsuite/ld-riscv-elf/variant_cc-shared.d
>> @@ -7,11 +7,11 @@ Relocation section '.rela.plt' at .*
>> #...
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+nocc_global_default_undef \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+cc_global_default_undef \+ 0
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
>> +[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+cc_global_default_def \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+nocc_global_default_def \+ 0
>> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
>> -[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
>> [0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
>> --
>> 2.37.1
>>
On Thu, May 16, 2024 at 2:16 PM Hau Hsu <hau.hsu@sifive.com> wrote:
> Hi Nelson,
>
> Sorry for the late reply.
>
> On May 8, 2024, at 9:00 AM, Nelson Chu <nelson@rivosinc.com> wrote:
>
> Can you provide the testcase to show the case which I mentioned in the
> pr13302? Which is that an ifunc with a dynamic jump slot calls another
> ifunc, and are not in the rel.dyn.
>
> I am not quite sure what's the issue you mentioned in PR13302.
> You mean the order issue of cause by one ifunc (generates
> JUMP_SLOT[32|64]) calls another JUMP_SLOT[32|64] ifunc?
>
> Since according to the testcase below, it seems no requirement to apply
> this fix though.
>
> The new test case uses two methods to call ifuncs:
> 1. Through a normal function call: PLT + GOT
> 2. Through a global function pointer: GOT only
>
> Without the fix, the relocation of the first method overwrites the
> second's.
> The relocation section of my test case would be:
>
> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
> Offset Info Type Sym. Value Symbol's Name + Addend
> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
> 00000000 00000000 R_RISCV_NONE 0
>
>
> With the fix, it becomes
>
> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
> Offset Info Type Sym. Value Symbol's Name + Addend
> 000110e0 0000003a R_RISCV_IRELATIVE 100c0
> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
>
>
So it seems like the overwrite problem, not the order problem we were
discussing in the pr13302...
> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
>> @@ -0,0 +1,19 @@
>> +/* Define macros to handle similar behaviors for rv32/rv64.
>> + Assumes macro "__64_bit__" defined for rv64.
>> + The macro is specifically defined for ifunc tests in
>> ld-riscv-elf.exp. */
>> +
>> +.macro PTR_DATA name
>> +.ifdef __64_bit__
>> + .quad \name
>> +.else
>> + .long \name
>> +.endif
>> +.endm
>> +
>> +.macro LOAD rd, rs, offset
>> +.ifdef __64_bit__
>> + ld \rd, \offset (\rs)
>> +.else
>> + lw \rd, \offset (\rs)
>> +.endif
>> +.endm
>
>
Btw, can we not use these macroes?
Nelson
> On May 16, 2024, at 3:07 PM, Nelson Chu <nelson@rivosinc.com> wrote:
>
> On Thu, May 16, 2024 at 2:16 PM Hau Hsu <hau.hsu@sifive.com <mailto:hau.hsu@sifive.com>> wrote:
>> Hi Nelson,
>>
>> Sorry for the late reply.
>>
>>> On May 8, 2024, at 9:00 AM, Nelson Chu <nelson@rivosinc.com <mailto:nelson@rivosinc.com>> wrote:
>>>
>>> Can you provide the testcase to show the case which I mentioned in the pr13302? Which is that an ifunc with a dynamic jump slot calls another ifunc, and are not in the rel.dyn.
>> I am not quite sure what's the issue you mentioned in PR13302.
>> You mean the order issue of cause by one ifunc (generates JUMP_SLOT[32|64]) calls another JUMP_SLOT[32|64] ifunc?
>>
>>> Since according to the testcase below, it seems no requirement to apply this fix though.
>> The new test case uses two methods to call ifuncs:
>> 1. Through a normal function call: PLT + GOT
>> 2. Through a global function pointer: GOT only
>>
>> Without the fix, the relocation of the first method overwrites the second's.
>> The relocation section of my test case would be:
>>
>> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
>> Offset Info Type Sym. Value Symbol's Name + Addend
>> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
>> 00000000 00000000 R_RISCV_NONE 0
>>
>>
>> With the fix, it becomes
>>
>> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
>> Offset Info Type Sym. Value Symbol's Name + Addend
>> 000110e0 0000003a R_RISCV_IRELATIVE 100c0
>> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
>>
>
> So it seems like the overwrite problem, not the order problem we were discussing in the pr13302...
Yes. Sorry that I didn't explain the whole story well.
This PR is originally to fix the overwrite problem, as my commit message says:
> This commit resolved two issues:
> 1. When an ifunc is referenced by a pointer, the relocation of
> the pointer in .rela.plt would be overwritten by normal ifunc call.
We found the issue when building glibc testbench statically.
To fix this issue, I sent a PR (https://sourceware.org/pipermail/binutils/2023-July/128485.html) about a year ago.
The PR use the method smilier to your previous commit, i.e.
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=51a8a7c2e3cc0730831963651a55d23d1fae624d
Then you suggested to check whether the relocation order is correct.
After that I checked the X86 implementation, I sent this PR.
>
>>>> --- /dev/null
>>>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
>>>> @@ -0,0 +1,19 @@
>>>> +/* Define macros to handle similar behaviors for rv32/rv64.
>>>> + Assumes macro "__64_bit__" defined for rv64.
>>>> + The macro is specifically defined for ifunc tests in ld-riscv-elf.exp. */
>>>> +
>>>> +.macro PTR_DATA name
>>>> +.ifdef __64_bit__
>>>> + .quad \name
>>>> +.else
>>>> + .long \name
>>>> +.endif
>>>> +.endm
>>>> +
>>>> +.macro LOAD rd, rs, offset
>>>> +.ifdef __64_bit__
>>>> + ld \rd, \offset (\rs)
>>>> +.else
>>>> + lw \rd, \offset (\rs)
>>>> +.endif
>>>> +.endm
>
> Btw, can we not use these macroes?
No problem. I just want to avoid similar codes in the ifunc tests.
>
> Nelson
I think the quite easy solution is - don't use riscv_elf_append_rela to
emit dynamic IRELATIVE for R_RISCV_32/64 into iplt section. Seems like
commit 51a8a7c2e3cc only handles the finish_dynamic_symbol, but forgot to
apply a similar fix in the relocate_section.
https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-riscv.c#L2408
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 604f6de4511..dca446c0495 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -2399,13 +2399,16 @@ riscv_elf_relocate_section (bfd *output_bfd,
2. .rela.got section in dynamic executable.
3. .rela.iplt section in static executable. */
if (bfd_link_pic (info))
- sreloc = htab->elf.irelifunc;
+ riscv_elf_append_rela (output_bfd,
htab->elf.irelifunc, &outrel);
else if (htab->elf.splt != NULL)
- sreloc = htab->elf.srelgot;
+ riscv_elf_append_rela (output_bfd, htab->elf.srelgot,
&outrel);
else
- sreloc = htab->elf.irelplt;
-
- riscv_elf_append_rela (output_bfd, sreloc, &outrel);
+ {
+ const struct elf_backend_data *bed =
get_elf_backend_data (output_bfd);
+ bfd_vma iplt_idx = htab->last_iplt_index--;
+ bfd_byte *loc = htab->elf.irelplt->contents +
iplt_idx * sizeof (ElfNN_External_Rela);
+ bed->s->swap_reloca_out (output_bfd, &outrel, loc);
+ }
/* If this reloc is against an external symbol, we
do not want to fiddle with the addend. Otherwise,
The above changes seem to fix the testcase you provided, but without
testing fully riscv-gnu-toolchain regressions.
Or we should find a way to handle reloc_index for iplt, and all use
riscv_elf_append_rela to emit the dynamic relocation.
Nelson
On Thu, May 16, 2024 at 3:30 PM Hau Hsu <hau.hsu@sifive.com> wrote:
>
> On May 16, 2024, at 3:07 PM, Nelson Chu <nelson@rivosinc.com> wrote:
>
> On Thu, May 16, 2024 at 2:16 PM Hau Hsu <hau.hsu@sifive.com> wrote:
>
>> Hi Nelson,
>>
>> Sorry for the late reply.
>>
>> On May 8, 2024, at 9:00 AM, Nelson Chu <nelson@rivosinc.com> wrote:
>>
>> Can you provide the testcase to show the case which I mentioned in the
>> pr13302? Which is that an ifunc with a dynamic jump slot calls another
>> ifunc, and are not in the rel.dyn.
>>
>> I am not quite sure what's the issue you mentioned in PR13302.
>> You mean the order issue of cause by one ifunc (generates
>> JUMP_SLOT[32|64]) calls another JUMP_SLOT[32|64] ifunc?
>>
>> Since according to the testcase below, it seems no requirement to apply
>> this fix though.
>>
>> The new test case uses two methods to call ifuncs:
>> 1. Through a normal function call: PLT + GOT
>> 2. Through a global function pointer: GOT only
>>
>> Without the fix, the relocation of the first method overwrites the
>> second's.
>> The relocation section of my test case would be:
>>
>> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
>> Offset Info Type Sym. Value Symbol's Name + Addend
>> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
>> 00000000 00000000 R_RISCV_NONE 0
>>
>>
>> With the fix, it becomes
>>
>> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
>> Offset Info Type Sym. Value Symbol's Name + Addend
>> 000110e0 0000003a R_RISCV_IRELATIVE 100c0
>> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
>>
>>
> So it seems like the overwrite problem, not the order problem we were
> discussing in the pr13302...
>
>
> Yes. Sorry that I didn't explain the whole story well.
>
> This PR is originally to fix the overwrite problem, as my commit message
> says:
> > This commit resolved two issues:
> > 1. When an ifunc is referenced by a pointer, the relocation of
> > the pointer in .rela.plt would be overwritten by normal ifunc call.
> We found the issue when building glibc testbench statically.
>
> To fix this issue, I sent a PR (
> https://sourceware.org/pipermail/binutils/2023-July/128485.html) about a
> year ago.
> The PR use the method smilier to your previous commit, i.e.
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=51a8a7c2e3cc0730831963651a55d23d1fae624d
> Then you suggested to check whether the relocation order is correct.
> After that I checked the X86 implementation, I sent this PR.
>
>
>
>> --- /dev/null
>>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
>>> @@ -0,0 +1,19 @@
>>> +/* Define macros to handle similar behaviors for rv32/rv64.
>>> + Assumes macro "__64_bit__" defined for rv64.
>>> + The macro is specifically defined for ifunc tests in
>>> ld-riscv-elf.exp. */
>>> +
>>> +.macro PTR_DATA name
>>> +.ifdef __64_bit__
>>> + .quad \name
>>> +.else
>>> + .long \name
>>> +.endif
>>> +.endm
>>> +
>>> +.macro LOAD rd, rs, offset
>>> +.ifdef __64_bit__
>>> + ld \rd, \offset (\rs)
>>> +.else
>>> + lw \rd, \offset (\rs)
>>> +.endif
>>> +.endm
>>
>>
> Btw, can we not use these macroes?
>
>
> No problem. I just want to avoid similar codes in the ifunc tests.
>
>
> Nelson
>
>
>
Let me run more toolchain integrated tests.
Thanks!
Hau
> On May 16, 2024, at 4:28 PM, Nelson Chu <nelson@rivosinc.com> wrote:
>
> I think the quite easy solution is - don't use riscv_elf_append_rela to emit dynamic IRELATIVE for R_RISCV_32/64 into iplt section. Seems like commit 51a8a7c2e3cc only handles the finish_dynamic_symbol, but forgot to apply a similar fix in the relocate_section.
>
> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-riscv.c#L2408
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 604f6de4511..dca446c0495 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -2399,13 +2399,16 @@ riscv_elf_relocate_section (bfd *output_bfd,
> 2. .rela.got section in dynamic executable.
> 3. .rela.iplt section in static executable. */
> if (bfd_link_pic (info))
> - sreloc = htab->elf.irelifunc;
> + riscv_elf_append_rela (output_bfd, htab->elf.irelifunc, &outrel);
> else if (htab->elf.splt != NULL)
> - sreloc = htab->elf.srelgot;
> + riscv_elf_append_rela (output_bfd, htab->elf.srelgot, &outrel);
> else
> - sreloc = htab->elf.irelplt;
> -
> - riscv_elf_append_rela (output_bfd, sreloc, &outrel);
> + {
> + const struct elf_backend_data *bed = get_elf_backend_data (output_bfd);
> + bfd_vma iplt_idx = htab->last_iplt_index--;
> + bfd_byte *loc = htab->elf.irelplt->contents + iplt_idx * sizeof (ElfNN_External_Rela);
> + bed->s->swap_reloca_out (output_bfd, &outrel, loc);
> + }
>
> /* If this reloc is against an external symbol, we
> do not want to fiddle with the addend. Otherwise,
>
> The above changes seem to fix the testcase you provided, but without testing fully riscv-gnu-toolchain regressions.
> Or we should find a way to handle reloc_index for iplt, and all use riscv_elf_append_rela to emit the dynamic relocation.
>
> Nelson
>
> On Thu, May 16, 2024 at 3:30 PM Hau Hsu <hau.hsu@sifive.com <mailto:hau.hsu@sifive.com>> wrote:
>>
>>> On May 16, 2024, at 3:07 PM, Nelson Chu <nelson@rivosinc.com <mailto:nelson@rivosinc.com>> wrote:
>>>
>>> On Thu, May 16, 2024 at 2:16 PM Hau Hsu <hau.hsu@sifive.com <mailto:hau.hsu@sifive.com>> wrote:
>>>> Hi Nelson,
>>>>
>>>> Sorry for the late reply.
>>>>
>>>>> On May 8, 2024, at 9:00 AM, Nelson Chu <nelson@rivosinc.com <mailto:nelson@rivosinc.com>> wrote:
>>>>>
>>>>> Can you provide the testcase to show the case which I mentioned in the pr13302? Which is that an ifunc with a dynamic jump slot calls another ifunc, and are not in the rel.dyn.
>>>> I am not quite sure what's the issue you mentioned in PR13302.
>>>> You mean the order issue of cause by one ifunc (generates JUMP_SLOT[32|64]) calls another JUMP_SLOT[32|64] ifunc?
>>>>
>>>>> Since according to the testcase below, it seems no requirement to apply this fix though.
>>>> The new test case uses two methods to call ifuncs:
>>>> 1. Through a normal function call: PLT + GOT
>>>> 2. Through a global function pointer: GOT only
>>>>
>>>> Without the fix, the relocation of the first method overwrites the second's.
>>>> The relocation section of my test case would be:
>>>>
>>>> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
>>>> Offset Info Type Sym. Value Symbol's Name + Addend
>>>> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
>>>> 00000000 00000000 R_RISCV_NONE 0
>>>>
>>>>
>>>> With the fix, it becomes
>>>>
>>>> Relocation section '.rela.plt' at offset 0x94 contains 2 entries:
>>>> Offset Info Type Sym. Value Symbol's Name + Addend
>>>> 000110e0 0000003a R_RISCV_IRELATIVE 100c0
>>>> 000110dc 0000003a R_RISCV_IRELATIVE 100c0
>>>>
>>>
>>> So it seems like the overwrite problem, not the order problem we were discussing in the pr13302...
>>
>> Yes. Sorry that I didn't explain the whole story well.
>>
>> This PR is originally to fix the overwrite problem, as my commit message says:
>> > This commit resolved two issues:
>> > 1. When an ifunc is referenced by a pointer, the relocation of
>> > the pointer in .rela.plt would be overwritten by normal ifunc call.
>> We found the issue when building glibc testbench statically.
>>
>> To fix this issue, I sent a PR (https://sourceware.org/pipermail/binutils/2023-July/128485.html) about a year ago.
>> The PR use the method smilier to your previous commit, i.e.
>> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=51a8a7c2e3cc0730831963651a55d23d1fae624d
>> Then you suggested to check whether the relocation order is correct.
>> After that I checked the X86 implementation, I sent this PR.
>>
>>>
>>>>>> --- /dev/null
>>>>>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-macro.s
>>>>>> @@ -0,0 +1,19 @@
>>>>>> +/* Define macros to handle similar behaviors for rv32/rv64.
>>>>>> + Assumes macro "__64_bit__" defined for rv64.
>>>>>> + The macro is specifically defined for ifunc tests in ld-riscv-elf.exp. */
>>>>>> +
>>>>>> +.macro PTR_DATA name
>>>>>> +.ifdef __64_bit__
>>>>>> + .quad \name
>>>>>> +.else
>>>>>> + .long \name
>>>>>> +.endif
>>>>>> +.endm
>>>>>> +
>>>>>> +.macro LOAD rd, rs, offset
>>>>>> +.ifdef __64_bit__
>>>>>> + ld \rd, \offset (\rs)
>>>>>> +.else
>>>>>> + lw \rd, \offset (\rs)
>>>>>> +.endif
>>>>>> +.endm
>>>
>>> Btw, can we not use these macroes?
>>
>> No problem. I just want to avoid similar codes in the ifunc tests.
>>
>>>
>>> Nelson
>>
@@ -224,8 +224,12 @@ struct riscv_elf_link_hash_table
htab_t loc_hash_table;
void * loc_hash_memory;
- /* The index of the last unused .rel.iplt slot. */
- bfd_vma last_iplt_index;
+ /* The index of the next R_RISCV_JUMP_SLOT entry in .rela.plt. */
+ bfd_vma next_jump_slot_index;
+
+ /* R_RISCV_IRELATIVE entry in .rela.plt comes last.
+ Use this to record the index of the next R_RISCV_IRELATIVE entry. */
+ bfd_vma next_irelative_index;
/* The data segment phase, don't relax the section
when it is exp_seg_relro_adjust. */
@@ -1273,6 +1277,7 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
/* We also need to make an entry in the .rela.plt section. */
htab->elf.srelplt->size += sizeof (ElfNN_External_Rela);
+ htab->elf.srelplt->reloc_count ++;
/* If this symbol is not defined in a regular file, and we are
not generating a shared library, then set the symbol to this
@@ -1622,10 +1627,14 @@ riscv_elf_late_size_sections (bfd *output_bfd, struct bfd_link_info *info)
local ifunc symbols. */
htab_traverse (htab->loc_hash_table, allocate_local_ifunc_dynrelocs, info);
- /* Used to resolve the dynamic relocs overwite problems when
- generating static executable. */
- if (htab->elf.irelplt)
- htab->last_iplt_index = htab->elf.irelplt->reloc_count - 1;
+
+ htab->next_jump_slot_index = 0;
+
+ if (htab->elf.srelplt)
+ htab->next_irelative_index = htab->elf.srelplt->reloc_count - 1;
+ else if (htab->elf.irelplt)
+ htab->next_irelative_index = htab->elf.irelplt->reloc_count - 1;
+
if (htab->elf.sgotplt)
{
@@ -3201,15 +3210,14 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
if (h->plt.offset != (bfd_vma) -1)
{
- /* We've decided to create a PLT entry for this symbol. */
+ /* We've decided to create a PLT entry for this symbol.
+ Add a PLT entry, a GOT entry and a relocation entry. */
bfd_byte *loc;
bfd_vma i, header_address, plt_idx, got_offset, got_address;
uint32_t plt_entry[PLT_ENTRY_INSNS];
Elf_Internal_Rela rela;
asection *plt, *gotplt, *relplt;
- /* When building a static executable, use .iplt, .igot.plt and
- .rela.iplt sections for STT_GNU_IFUNC symbols. */
if (htab->elf.splt != NULL)
{
plt = htab->elf.splt;
@@ -3218,6 +3226,8 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
}
else
{
+ /* When building a static executable, use .iplt, .igot.plt and
+ .rela.iplt sections for STT_GNU_IFUNC symbols. */
plt = htab->elf.iplt;
gotplt = htab->elf.igotplt;
relplt = htab->elf.irelplt;
@@ -3238,7 +3248,8 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
header_address = sec_addr (plt);
/* Calculate the index of the entry and the offset of .got.plt entry.
- For static executables, we don't reserve anything. */
+ The index of .got.plt is the same as .plt (exclude the header entries),
+ since this section is dedicated for plt. */
if (plt == htab->elf.splt)
{
plt_idx = (h->plt.offset - PLT_HEADER_SIZE) / PLT_ENTRY_SIZE;
@@ -3246,6 +3257,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
}
else
{
+ /* For static executables, we don't need to reserve the header entry. */
plt_idx = h->plt.offset / PLT_ENTRY_SIZE;
got_offset = plt_idx * GOT_ENTRY_SIZE;
}
@@ -3269,6 +3281,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
loc = gotplt->contents + (got_address - sec_addr (gotplt));
bfd_put_NN (output_bfd, sec_addr (plt), loc);
+ /* Fill in the relocation entry. */
rela.r_offset = got_address;
if (PLT_LOCAL_IFUNC_P (info, h))
{
@@ -3283,17 +3296,20 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
rela.r_addend = h->root.u.def.value
+ sec->output_section->vma
+ sec->output_offset;
+ bfd_vma rela_idx = htab->next_irelative_index--;
+ loc = relplt->contents + rela_idx * sizeof (ElfNN_External_Rela);
+ bed->s->swap_reloca_out (output_bfd, &rela, loc);
}
else
{
/* Fill in the entry in the .rela.plt section. */
rela.r_info = ELFNN_R_INFO (h->dynindx, R_RISCV_JUMP_SLOT);
rela.r_addend = 0;
+ bfd_vma rela_idx = htab->next_jump_slot_index++;
+ loc = relplt->contents + rela_idx * sizeof (ElfNN_External_Rela);
+ bed->s->swap_reloca_out (output_bfd, &rela, loc);
}
- loc = relplt->contents + plt_idx * sizeof (ElfNN_External_Rela);
- bed->s->swap_reloca_out (output_bfd, &rela, loc);
-
if (!h->def_regular)
{
/* Mark the symbol as undefined, rather than as defined in
@@ -3315,7 +3331,6 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
asection *sgot;
asection *srela;
Elf_Internal_Rela rela;
- bool use_elf_append_rela = true;
/* This symbol has an entry in the GOT. Set it up. */
@@ -3338,10 +3353,6 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
/* Use .rela.iplt section to store .got relocations
in static executable. */
srela = htab->elf.irelplt;
-
- /* Do not use riscv_elf_append_rela to add dynamic
- relocs. */
- use_elf_append_rela = false;
}
if (SYMBOL_REFERENCES_LOCAL (info, h))
@@ -3417,23 +3428,7 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,
bfd_put_NN (output_bfd, 0,
sgot->contents + (h->got.offset & ~(bfd_vma) 1));
- if (use_elf_append_rela)
- riscv_elf_append_rela (output_bfd, srela, &rela);
- else
- {
- /* Use riscv_elf_append_rela to add the dynamic relocs into
- .rela.iplt may cause the overwrite problems. Since we insert
- the relocs for PLT didn't handle the reloc_index of .rela.iplt,
- but the riscv_elf_append_rela adds the relocs to the place
- that are calculated from the reloc_index (in seqential).
-
- One solution is that add these dynamic relocs (GOT IFUNC)
- from the last of .rela.iplt section. */
- bfd_vma iplt_idx = htab->last_iplt_index--;
- bfd_byte *loc = srela->contents
- + iplt_idx * sizeof (ElfNN_External_Rela);
- bed->s->swap_reloca_out (output_bfd, &rela, loc);
- }
+ riscv_elf_append_rela (output_bfd, srela, &rela);
}
if (h->needs_copy)
new file mode 100644
@@ -0,0 +1,19 @@
+/* Define macros to handle similar behaviors for rv32/rv64.
+ Assumes macro "__64_bit__" defined for rv64.
+ The macro is specifically defined for ifunc tests in ld-riscv-elf.exp. */
+
+.macro PTR_DATA name
+.ifdef __64_bit__
+ .quad \name
+.else
+ .long \name
+.endif
+.endm
+
+.macro LOAD rd, rs, offset
+.ifdef __64_bit__
+ ld \rd, \offset (\rs)
+.else
+ lw \rd, \offset (\rs)
+.endif
+.endm
new file mode 100644
@@ -0,0 +1,4 @@
+Relocation section '.rela.plt' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
new file mode 100644
@@ -0,0 +1,11 @@
+Relocation section '.rela.got' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_(32|64)[ ]+foo1\(\)[ ]+foo1 \+ 0
+#...
+Relocation section '.rela.ifunc' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_(32|64)[ ]+foo2\(\)[ ]+foo2 \+ 0
+#...
+Relocation section '.rela.plt' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+foo1\(\)[ ]+foo1 \+ 0
new file mode 100644
@@ -0,0 +1,7 @@
+Relocation section '.rela.ifunc' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
+
+Relocation section '.rela.plt' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+[0-9a-f]*
new file mode 100644
@@ -0,0 +1,16 @@
+#...
+Disassembly of section .plt:
+#...
+0+[0-9a-f]+ <(\*ABS\*\+0x[0-9a-f]+@plt|foo@plt|.plt)>:
+#...
+Disassembly of section .text:
+#...
+0+[0-9a-f]+ <foo_resolver>:
+.*:[ ]+[0-9a-f]+[ ]+ret
+
+0+[0-9a-f]+ <_start>:
+.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+.*
+.*:[ ]+[0-9a-f]+[ ]+(lw|ld)[ ]+.*<(.*)>
+.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+.*
+.*:[ ]+[0-9a-f]+[ ]+jalr[ ]+.*<(.*plt.*)>
+.*:[ ]+[0-9a-f]+[ ]+ret
new file mode 100644
@@ -0,0 +1,34 @@
+/* There are 2 ifuncs: foo1 and foo2.
+ foo1 is accessed by a function call.
+ foo2 is referenced by a global pointer (foo2_addr). */
+ .include "ifunc-macro.s"
+ .globl foo2_addr
+ .section .data
+ .type foo2_addr, @object
+foo2_addr:
+ PTR_DATA foo2
+
+ .text
+ .type foo_resolver, @function
+foo_resolver:
+ ret
+ .size foo_resolver, .-foo_resolver
+
+ .globl foo1
+ .type foo1, %gnu_indirect_function
+ .set foo1, foo_resolver
+
+ .globl foo2
+ .type foo2, %gnu_indirect_function
+ .set foo2, foo_resolver
+
+ .globl _start
+ .type _start, @function
+_start:
+.L1:
+ auipc x1, %got_pcrel_hi (foo1)
+ LOAD x1, x1, %pcrel_lo (.L1)
+ call foo1
+
+ ret
+ .size _start, .-_start
@@ -284,6 +284,13 @@ if [istarget "riscv*-*-*"] {
run_dump_test_ifunc "ifunc-plt-got-overwrite" rv64 pie
run_dump_test_ifunc "ifunc-plt-got-overwrite" rv64 pic
+ run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 exe
+ run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 pie
+ run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv32 pic
+ run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 exe
+ run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 pie
+ run_dump_test_ifunc "ifunc-plt-got-overwrite-02" rv64 pic
+
# TODO: Make the following tests work under RV32.
if [istarget "riscv32-*-*"] {
return
@@ -7,11 +7,11 @@ Relocation section '.rela.plt' at .*
#...
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+nocc_global_default_undef \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+cc_global_default_undef \+ 0
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+cc_global_default_def \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+nocc_global_default_def \+ 0
-[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
-[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
@@ -7,11 +7,11 @@ Relocation section '.rela.plt' at .*
#...
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+nocc_global_default_undef \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+0000[ ]+cc_global_default_undef \+ 0
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+cc_global_default_def \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+0+8000[ ]+nocc_global_default_def \+ 0
-[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+cc_global_default_ifunc\(\)[ ]+cc_global_default_ifunc \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
-[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_JUMP_SLOT[ ]+nocc_global_default_ifunc\(\)[ ]+nocc_global_default_ifunc \+ 0
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8000
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050
[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_RISCV_IRELATIVE[ ]+8050