[PATCH 2/2] riscv: Fix R_RISCV_IRELATIVE overwrite and order issues

[Hau Hsu <hau.hsu@sifive.com>]

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