[2/5] LoongArch: Fix bad reloc with mixed visibility ifunc symbols in shared libraries

Message ID 20240622100307.9498-3-xry111@xry111.site
State New
Headers
Series LoongArch: Add DT_RELR (packing relative relocs) support |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Xi Ruoyao June 22, 2024, 10:03 a.m. UTC
  With a simple test case:

    .globl  ifunc
    .globl  ifunc_hidden
    .hidden ifunc_hidden
    .type   ifunc, %gnu_indirect_function
    .type   ifunc_hidden, %gnu_indirect_function

    .text
    .align  2
    ifunc:  ret
    ifunc_hidden: ret

    test:
      bl ifunc
      bl ifunc_hidden

"ld -shared" produces a shared object with one R_LARCH_NONE (instead of
R_LARCH_JUMP_SLOT as we expect) to relocate the GOT entry of "ifunc".
It's because the indices in .plt and .rela.plt mismatches for
STV_DEFAULT STT_IFUNC symbols when another PLT entry exists for a
STV_HIDDEN STT_IFUNC symbol, and such a mismatch breaks the logic of
loongarch_elf_finish_dynamic_symbol.  Fix the issue by reordering .plt
so the indices no longer mismatch.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 bfd/elfnn-loongarch.c                         | 77 ++++++++++++++++---
 ld/testsuite/ld-loongarch-elf/ifunc-reloc.d   | 19 +++++
 ld/testsuite/ld-loongarch-elf/ifunc-reloc.s   | 55 +++++++++++++
 .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
 4 files changed, 140 insertions(+), 12 deletions(-)
 create mode 100644 ld/testsuite/ld-loongarch-elf/ifunc-reloc.d
 create mode 100644 ld/testsuite/ld-loongarch-elf/ifunc-reloc.s
  

Patch

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 3a55ac93e20..e0b3e9f5ee6 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -1691,9 +1691,10 @@  local_allocate_ifunc_dyn_relocs (struct bfd_link_info *info,
    ifunc dynamic relocs.  */
 
 static bool
-elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h, void *inf)
+elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h,
+				struct bfd_link_info *info,
+				bool ref_local)
 {
-  struct bfd_link_info *info;
   /* An example of a bfd_link_hash_indirect symbol is versioned
      symbol. For example: __gxx_personality_v0(bfd_link_hash_indirect)
      -> __gxx_personality_v0(bfd_link_hash_defined)
@@ -1709,20 +1710,18 @@  elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h, void *inf)
   if (h->root.type == bfd_link_hash_warning)
     h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
-  info = (struct bfd_link_info *) inf;
-
   /* Since STT_GNU_IFUNC symbol must go through PLT, we handle it
      here if it is defined and referenced in a non-shared object.  */
   if (h->type == STT_GNU_IFUNC && h->def_regular)
     {
-      if (SYMBOL_REFERENCES_LOCAL (info, h))
+      if (ref_local && SYMBOL_REFERENCES_LOCAL (info, h))
 	return local_allocate_ifunc_dyn_relocs (info, h,
 						&h->dyn_relocs,
 						PLT_ENTRY_SIZE,
 						PLT_HEADER_SIZE,
 						GOT_ENTRY_SIZE,
 						false);
-      else
+      else if (!ref_local && !SYMBOL_REFERENCES_LOCAL (info, h))
 	return _bfd_elf_allocate_ifunc_dyn_relocs (info, h,
 						   &h->dyn_relocs,
 						   PLT_ENTRY_SIZE,
@@ -1734,6 +1733,23 @@  elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h, void *inf)
   return true;
 }
 
+static bool
+elfNN_allocate_ifunc_dynrelocs_ref_local (struct elf_link_hash_entry *h,
+					  void *info)
+{
+  return elfNN_allocate_ifunc_dynrelocs (h, (struct bfd_link_info *) info,
+					 true);
+}
+
+static bool
+elfNN_allocate_ifunc_dynrelocs_ref_global (struct elf_link_hash_entry *h,
+					   void *info)
+{
+  return elfNN_allocate_ifunc_dynrelocs (h, (struct bfd_link_info *) info,
+					 false);
+}
+
+
 /* Allocate space in .plt, .got and associated reloc sections for
    ifunc dynamic relocs.  */
 
@@ -1749,7 +1765,7 @@  elfNN_allocate_local_ifunc_dynrelocs (void **slot, void *inf)
       || h->root.type != bfd_link_hash_defined)
     abort ();
 
-  return elfNN_allocate_ifunc_dynrelocs (h, inf);
+  return elfNN_allocate_ifunc_dynrelocs_ref_local (h, inf);
 }
 
 /* Set DF_TEXTREL if we find any dynamic relocs that apply to
@@ -1909,11 +1925,48 @@  loongarch_elf_late_size_sections (bfd *output_bfd,
      sym dynamic relocs.  */
   elf_link_hash_traverse (&htab->elf, allocate_dynrelocs, info);
 
-  /* Allocate global ifunc sym .plt and .got entries, and space for global
-     ifunc sym dynamic relocs.  */
-  elf_link_hash_traverse (&htab->elf, elfNN_allocate_ifunc_dynrelocs, info);
-
-  /* Allocate .plt and .got entries, and space for local ifunc symbols.  */
+  /* Allocate global ifunc sym .plt and .got entries, and space for
+     *preemptable* ifunc sym dynamic relocs.  Note that we must do it
+     for *all* preemptable ifunc (including local ifuncs and STV_HIDDEN
+     ifuncs) before doing it for any unpreemptable ifunc symbol:
+     assuming we are not so careful, when we link a shared library the
+     correlation of .plt and .rela.plt might look like:
+
+				idx in .plt	idx in .rela.plt
+	ext_func1@plt		0		0
+	ext_func2@plt		1		1
+	ext_func3@plt		2		2
+	hidden_ifunc1@plt	3		None: it's in .rela.got
+	hidden_ifunc2@plt	4		None: it's in .rela.got
+	normal_ifunc1@plt	5	!=	3
+	normal_ifunc2@plt	6	!=	4
+	local_ifunc@plt		7		None: it's in .rela.got
+
+     Now oops the indices for normal_ifunc{1,2} in .rela.plt were different
+     from the indices in .plt :(.  This would break finish_dynamic_symbol
+     which assumes the index in .rela.plt matches the index in .plt.
+
+     So let's be careful and make it correct:
+
+				idx in .plt	idx in .rela.plt
+	ext_func1@plt		0		0
+	ext_func2@plt		1		1
+	ext_func3@plt		2		2
+	normal_ifunc1@plt	3		3
+	normal_ifunc2@plt	4		4
+	hidden_ifunc1@plt	5		None: it's in .rela.got
+	hidden_ifunc2@plt	6		None: it's in .rela.got
+	local_ifunc@plt		7		None: it's in .rela.got
+
+     Now normal_ifuncs first.  */
+  elf_link_hash_traverse (&htab->elf,
+			  elfNN_allocate_ifunc_dynrelocs_ref_global, info);
+
+  /* Now hidden_ifuncs ifuncs.  */
+  elf_link_hash_traverse (&htab->elf,
+			  elfNN_allocate_ifunc_dynrelocs_ref_local, info);
+
+  /* Finally local ifuncs.  */
   htab_traverse (htab->loc_hash_table,
 		 elfNN_allocate_local_ifunc_dynrelocs, info);
 
diff --git a/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d
new file mode 100644
index 00000000000..cb592874b1e
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d
@@ -0,0 +1,19 @@ 
+#ld: -shared
+#readelf: -Wr
+
+#...
+.*'\.rela\.dyn'.*
+#...
+.* R_LARCH_RELATIVE .*
+.* R_LARCH_IRELATIVE .*
+.* R_LARCH_IRELATIVE .*
+.* R_LARCH_IRELATIVE .*
+#...
+.*'\.rela\.plt'.*
+#...
+.* R_LARCH_JUMP_SLOT .*
+.* R_LARCH_JUMP_SLOT .*
+.* R_LARCH_JUMP_SLOT .*
+.* R_LARCH_JUMP_SLOT .*
+.* R_LARCH_JUMP_SLOT .*
+.* R_LARCH_JUMP_SLOT .*
diff --git a/ld/testsuite/ld-loongarch-elf/ifunc-reloc.s b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.s
new file mode 100644
index 00000000000..e59f2b20e36
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.s
@@ -0,0 +1,55 @@ 
+.globl	foo
+.globl	foo_hidden1
+.globl	foo_hidden2
+.globl	foo_protected
+
+.type	foo, %gnu_indirect_function
+.type	foo_hidden1, %gnu_indirect_function
+.type	foo_hidden2, %gnu_indirect_function
+.type	foo_protected, %gnu_indirect_function
+.type	foo_internal, %gnu_indirect_function
+
+.hidden	foo_hidden1
+.hidden	foo_hidden2
+
+.protected	foo_protected
+
+.globl	ext_ifunc1
+.globl	ext_ifunc2
+.type	ext_ifunc1, %gnu_indirect_function
+.type	ext_ifunc2, %gnu_indirect_function
+
+.text
+.align	2
+foo:
+	ret
+
+foo_hidden1:
+	ret
+
+foo_hidden2:
+	ret
+
+foo_protected:
+	ret
+
+foo_internal:
+	ret
+
+test:
+	la.got	$a0, num
+	# The order is deliberately shuffled.
+	bl	ext_ifunc1
+	bl	foo
+	bl	foo_hidden1
+	bl	ext_func1
+	bl	foo_protected
+	bl	foo_internal
+	bl	foo_hidden2
+	bl	ext_func2
+	bl	ext_ifunc2
+
+.data
+.align	3
+num:
+	.quad	114514
diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
index 70625fa8dfe..b8721f0eaff 100644
--- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
+++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
@@ -140,6 +140,7 @@  if [istarget "loongarch64-*-*"] {
     run_dump_test "reloc_ler_with_shared"
     run_dump_test "reloc_abs_with_shared"
     run_dump_test "r_larch_32_elf64"
+    run_dump_test "ifunc-reloc"
   }
 
   if [check_pie_support] {