diff mbox series

[v2] LoongArch: Fix the infinite loop caused by calling undefweak symbol

Message ID 20241120012843.446845-1-cailulu@loongson.cn
State New
Headers
Series [v2] LoongArch: Fix the infinite loop caused by calling undefweak symbol |

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

Lulu Cai Nov. 20, 2024, 1:28 a.m. UTC 
  The undefweak symbol value of non-default visibility is 0 and does
not use plt entry, and will not be relocated in the relocate_secion
function. As a result, an infinite loop is generated because
bl %plt(sym) => bl 0.

Fix this by converting the bl/call36 instructions to nop or removing
them.

---
Changes from v1:
  - Pass @var{again} to loongarch_relax_call_undefweak to continue the
    next relax.

v1: https://sourceware.org/pipermail/binutils/2024-November/137701.html
---
 bfd/elfnn-loongarch.c                         | 54 +++++++++++++++++++
 gas/config/tc-loongarch.c                     |  3 +-
 .../gas/loongarch/double_quotation_marks.d    |  1 +
 gas/testsuite/gas/loongarch/jmp_op.d          |  2 +
 gas/testsuite/gas/loongarch/reloc.d           |  4 ++
 gas/testsuite/gas/loongarch/relocs_64.d       |  1 +
 .../ld-loongarch-elf/call_undefweak.d         | 30 +++++++++++
 .../ld-loongarch-elf/call_undefweak.s         | 48 +++++++++++++++++
 ld/testsuite/ld-loongarch-elf/jmp_op.d        |  2 +
 .../ld-loongarch-elf/ld-loongarch-elf.exp     | 10 ++++
 10 files changed, 154 insertions(+), 1 deletion(-)
 create mode 100644 ld/testsuite/ld-loongarch-elf/call_undefweak.d
 create mode 100644 ld/testsuite/ld-loongarch-elf/call_undefweak.s

Comments

Xi Ruoyao Nov. 20, 2024, 1:53 a.m. UTC | #1 
On Wed, 2024-11-20 at 09:28 +0800, Lulu Cai wrote:
> The undefweak symbol value of non-default visibility is 0 and does
> not use plt entry, and will not be relocated in the relocate_secion
> function. As a result, an infinite loop is generated because
> bl %plt(sym) => bl 0.
> 
> Fix this by converting the bl/call36 instructions to nop or removing
> them.

IMO it's better to convert it to "jirl $ra, $zero, 0".  Semantically an
undefined weak symbol should be resolved to address 0 so we should make
a call to address 0, instead of silently ignore the call.

Also "b %plt(sym)" should be turned to "jirl $zero, $zero, 0" as well
when sym is non-default visibility and weak undefined.
mengqinggang Nov. 20, 2024, 2:22 a.m. UTC | #2 
在 2024/11/20 上午9:53, Xi Ruoyao 写道:
> On Wed, 2024-11-20 at 09:28 +0800, Lulu Cai wrote:
>> The undefweak symbol value of non-default visibility is 0 and does
>> not use plt entry, and will not be relocated in the relocate_secion
>> function. As a result, an infinite loop is generated because
>> bl %plt(sym) => bl 0.
>>
>> Fix this by converting the bl/call36 instructions to nop or removing
>> them.
> IMO it's better to convert it to "jirl $ra, $zero, 0".  Semantically an
> undefined weak symbol should be resolved to address 0 so we should make
> a call to address 0, instead of silently ignore the call.
>
> Also "b %plt(sym)" should be turned to "jirl $zero, $zero, 0" as well
> when sym is non-default visibility and weak undefined.

AArch64 optimizes  calls to undefined weak symbols to a NOP. It maybe a 
better choice.

In bfd/elfnn-aarch64.c:

  6046         /* A call to an undefined weak symbol is converted to a 
jump to
  6047            the next instruction unless a PLT entry will be created.
  6048            The jump to the next instruction is optimized as a NOP.
  6049            Do the same for local undefined symbols. */
  6050         if (weak_undef_p && ! via_plt_p)
  6051 {
  6052             bfd_putl32 (INSN_NOP, hit_data);
  6053             return bfd_reloc_ok;
  6054           }
Lulu Cai Nov. 21, 2024, 1:24 a.m. UTC | #3 
On 11/20/24 10:22 AM, mengqinggang wrote:
>
> 在 2024/11/20 上午9:53, Xi Ruoyao 写道:
>> On Wed, 2024-11-20 at 09:28 +0800, Lulu Cai wrote:
>>> The undefweak symbol value of non-default visibility is 0 and does
>>> not use plt entry, and will not be relocated in the relocate_secion
>>> function. As a result, an infinite loop is generated because
>>> bl %plt(sym) => bl 0.
>>>
>>> Fix this by converting the bl/call36 instructions to nop or removing
>>> them.
>> IMO it's better to convert it to "jirl $ra, $zero, 0". Semantically an
>> undefined weak symbol should be resolved to address 0 so we should make
>> a call to address 0, instead of silently ignore the call.
>>
>> Also "b %plt(sym)" should be turned to "jirl $zero, $zero, 0" as well
>> when sym is non-default visibility and weak undefined.
>
> AArch64 optimizes  calls to undefined weak symbols to a NOP. It maybe 
> a better choice.
>

Each port handles calls to non-default visibility undefweak symbols 
slightly differently.
x86-64 and riscv preserve the semantics of the symbol using "call 0" and 
"jalr 0", while
aarc64 optimizes the call to "nop". It is unclear why aarch64 optimizes 
instead of preserving
semantics.

> In bfd/elfnn-aarch64.c:
>
>  6046         /* A call to an undefined weak symbol is converted to a 
> jump to
>  6047            the next instruction unless a PLT entry will be created.
>  6048            The jump to the next instruction is optimized as a NOP.
>  6049            Do the same for local undefined symbols. */
>  6050         if (weak_undef_p && ! via_plt_p)
>  6051 {
>  6052             bfd_putl32 (INSN_NOP, hit_data);
>  6053             return bfd_reloc_ok;
>  6054           }
diff mbox series

Patch

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 608d179c168..2fe50db1813 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -222,6 +222,10 @@  loongarch_elf_new_section_hook (bfd *abfd, asection *sec)
    || (R_TYPE) == R_LARCH_TLS_LE64_LO20	  \
    || (R_TYPE) == R_LARCH_TLS_LE64_HI12)
 
+#define IS_CALL_RELOC(R_TYPE)	  \
+  ((R_TYPE) == R_LARCH_B26	  \
+   || (R_TYPE) == R_LARCH_CALL36)
+
 /* If TLS GD/IE need dynamic relocations, INDX will be the dynamic indx,
    and set NEED_RELOC to true used in allocate_dynrelocs and
    loongarch_elf_relocate_section for TLS GD/IE.  */
@@ -5277,6 +5281,38 @@  loongarch_relax_tls_ld_gd_desc (bfd *abfd, asection *sec, asection *sym_sec,
   return true;
 }
 
+/* Delete the call undefweak instruction without plt entries when relax
+   is enabled, otherwise it will be converted to nop.  */
+static bool
+loongarch_relax_call_undefweak (bfd *abfd, asection *sec,
+				Elf_Internal_Rela *rel,
+				struct bfd_link_info *info,
+				bool with_relax_reloc,
+				bool *again)
+{
+  bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
+  unsigned long r_type = ELFNN_R_TYPE (rel->r_info);
+
+  bfd_put (32, abfd, LARCH_NOP, contents + rel->r_offset);
+  /* Medium call uses pcaddu18i + jirl, so jirl also needs
+     to be converted to nop.  */
+  if (r_type == R_LARCH_CALL36)
+    bfd_put (32, abfd, LARCH_NOP, contents + rel->r_offset + 4);
+
+  if (with_relax_reloc && !info->disable_target_specific_optimizations)
+    {
+      /* Continue next relax trip.  */
+      *again = true;
+
+      loongarch_relax_delete_bytes (abfd, sec, rel->r_offset, 4, info);
+      if (r_type == R_LARCH_CALL36)
+	loongarch_relax_delete_bytes (abfd, sec, rel->r_offset, 4, info);
+    }
+
+  rel->r_info = ELFNN_R_INFO (0, R_LARCH_NONE);
+  return true;
+}
+
 /* Traverse all output sections and return the max alignment.  */
 
 static bfd_vma
@@ -5398,6 +5434,24 @@  loongarch_elf_relax_section (bfd *abfd, asection *sec,
 	  r_type = ELFNN_R_TYPE (rel->r_info);
 	}
 
+      /* The undefweak symbol value of non-default visibility is 0
+	 and does not use plt entry, and will not be relocated in
+	 the relocate_secion function. As a result, an infinite loop
+	 is generated because bl %plt(sym) => bl 0.  */
+      if (h != NULL
+	  && h->root.type == bfd_link_hash_undefweak
+	  && !h->needs_plt
+	  && IS_CALL_RELOC (r_type))
+      {
+	bool with_relax_reloc = false;
+	if (i + 1 != sec->reloc_count
+	    && ELFNN_R_TYPE (rel[1].r_info) == R_LARCH_RELAX)
+	  with_relax_reloc = true;
+
+	loongarch_relax_call_undefweak (abfd, sec, rel, info,
+					with_relax_reloc, again);
+	r_type = ELFNN_R_TYPE (rel->r_info);
+      }
       relax_func_t relax_func = NULL;
       if (info->relax_pass == 0)
 	{
diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
index 411c24b41f0..e5837d75f76 100644
--- a/gas/config/tc-loongarch.c
+++ b/gas/config/tc-loongarch.c
@@ -853,7 +853,8 @@  loongarch_args_parser_can_match_arg_helper (char esc_ch1, char esc_ch2,
 			|| BFD_RELOC_LARCH_TLS_LE_LO12 == reloc_type
 			|| BFD_RELOC_LARCH_TLS_LE64_LO20 == reloc_type
 			|| BFD_RELOC_LARCH_TLS_LE64_HI12 == reloc_type
-			|| BFD_RELOC_LARCH_CALL36 == reloc_type))
+			|| BFD_RELOC_LARCH_CALL36 == reloc_type
+			|| BFD_RELOC_LARCH_B26 == reloc_type))
 		{
 		  ip->reloc_info[ip->reloc_num].type = BFD_RELOC_LARCH_RELAX;
 		  ip->reloc_info[ip->reloc_num].value = const_0;
diff --git a/gas/testsuite/gas/loongarch/double_quotation_marks.d b/gas/testsuite/gas/loongarch/double_quotation_marks.d
index a42534b94a4..3a058921799 100644
--- a/gas/testsuite/gas/loongarch/double_quotation_marks.d
+++ b/gas/testsuite/gas/loongarch/double_quotation_marks.d
@@ -9,5 +9,6 @@  Disassembly of section .text:
 .* <.text>:
 [ 	]+0:[ 	]+50000000[ 	]+b[ 	]+0[ 	]+# 0x0
 [ 	]+0: R_LARCH_B26[ 	]+.L1
+[ 	]+0: R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+4:[ 	]+5800018d[ 	]+beq[ 	]+\$t0, \$t1, 0[ 	]+# 0x4
 [ 	]+4: R_LARCH_B16[ 	]+.L1
diff --git a/gas/testsuite/gas/loongarch/jmp_op.d b/gas/testsuite/gas/loongarch/jmp_op.d
index 21576072aab..2c2f8e68645 100644
--- a/gas/testsuite/gas/loongarch/jmp_op.d
+++ b/gas/testsuite/gas/loongarch/jmp_op.d
@@ -25,8 +25,10 @@  Disassembly of section .text:
 [ 	]+20:[ 	]+4c000080[ 	]+jr[ 	]+\$a0
 [ 	]+24:[ 	]+53ffdfff[ 	]+b[ 	]+-36[ 	]+#[ 	]+0[ 	]+<\.L1>
 [ 	]+24:[ 	]+R_LARCH_B26[ 	]+\.L1
+[ 	]+24:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+28:[ 	]+57ffdbff[ 	]+bl[ 	]+-40[ 	]+#[ 	]+0[ 	]+<\.L1>
 [ 	]+28:[ 	]+R_LARCH_B26[ 	]+\.L1
+[ 	]+28:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+2c:[ 	]+5bffd485[ 	]+beq[ 	]+\$a0,[ 	]+\$a1,[ 	]+-44[ 	]+#[ 	]+0[ 	]+<\.L1>
 [ 	]+2c:[ 	]+R_LARCH_B16[ 	]+\.L1
 [ 	]+30:[ 	]+5fffd085[ 	]+bne[ 	]+\$a0,[ 	]+\$a1,[ 	]+-48[ 	]+#[ 	]+0[ 	]+<\.L1>
diff --git a/gas/testsuite/gas/loongarch/reloc.d b/gas/testsuite/gas/loongarch/reloc.d
index 6a8f0e1f5d9..65fbac7bca9 100644
--- a/gas/testsuite/gas/loongarch/reloc.d
+++ b/gas/testsuite/gas/loongarch/reloc.d
@@ -29,8 +29,10 @@  Disassembly of section .text:
 [ 	]+24:[ 	]+R_LARCH_B21[ 	]+.L1
 [ 	]+28:[ 	]+50000000[ 	]+b[ 	]+0[ 	]+#[ 	]+0x28
 [ 	]+28:[ 	]+R_LARCH_B26[ 	]+.L1
+[ 	]+28:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+2c:[ 	]+54000000[ 	]+bl[ 	]+0[ 	]+#[ 	]+0x2c
 [ 	]+2c:[ 	]+R_LARCH_B26[ 	]+.L1
+[ 	]+2c:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+30:[ 	]+14000004[ 	]+lu12i.w[ 	]+\$a0,[ 	]+0
 [ 	]+30:[ 	]+R_LARCH_ABS_HI20[ 	]+.L1
 [ 	]+34:[ 	]+038000a4[ 	]+ori[ 	]+\$a0,[ 	]+\$a1,[ 	]+0x0
@@ -111,8 +113,10 @@  Disassembly of section .text:
 [ 	]+c0:[ 	]+R_LARCH_B21[ 	]+.L1\+0x8
 [ 	]+c4:[ 	]+50000000[ 	]+b[ 	]+0[ 	]+#[ 	]+0xc4
 [ 	]+c4:[ 	]+R_LARCH_B26[ 	]+.L1\+0x8
+[ 	]+c4:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+c8:[ 	]+54000000[ 	]+bl[ 	]+0[ 	]+#[ 	]+0xc8
 [ 	]+c8:[ 	]+R_LARCH_B26[ 	]+.L1\+0x8
+[ 	]+c8:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+cc:[ 	]+14000004[ 	]+lu12i.w[ 	]+\$a0,[ 	]+0
 [ 	]+cc:[ 	]+R_LARCH_ABS_HI20[ 	]+.L1\+0x8
 [ 	]+d0:[ 	]+038000a4[ 	]+ori[ 	]+\$a0,[ 	]+\$a1,[ 	]+0x0
diff --git a/gas/testsuite/gas/loongarch/relocs_64.d b/gas/testsuite/gas/loongarch/relocs_64.d
index ce5216a2b73..fb9eb2c86b6 100644
--- a/gas/testsuite/gas/loongarch/relocs_64.d
+++ b/gas/testsuite/gas/loongarch/relocs_64.d
@@ -14,6 +14,7 @@  Disassembly of section .text:
 [ 	]+4: R_LARCH_B21[ 	]+.L1
 [ 	]+8:[ 	]+50008400[ 	]+b[ 	]+132[ 	]+# 8c <.L1>
 [ 	]+8: R_LARCH_B26[ 	]+.L1
+[ 	]+8: R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+c:[ 	]+14000004[ 	]+lu12i.w[ 	]+\$a0, 0
 [ 	]+c: R_LARCH_ABS_HI20[ 	]+.L1
 [ 	]+10:[ 	]+038000a4[ 	]+ori[ 	]+\$a0, \$a1, 0x0
diff --git a/ld/testsuite/ld-loongarch-elf/call_undefweak.d b/ld/testsuite/ld-loongarch-elf/call_undefweak.d
new file mode 100644
index 00000000000..a8ab431cc74
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/call_undefweak.d
@@ -0,0 +1,30 @@ 
+#...
+Disassembly of section .plt:
+#...
+00000001200004d0 <fn3@plt>:
+   1200004d0:	1c00010f 	pcaddu12i   	\$t3, 8
+   1200004d4:	28ed01ef 	ld.d        	\$t3, \$t3, -1216
+   1200004d8:	4c0001ed 	jirl        	\$t1, \$t3, 0
+   1200004dc:	03400000 	nop
+
+Disassembly of section .text:
+#...
+0000000120000668 <main>:
+   120000668:	03400000 	nop
+   12000066c:	03400000 	nop
+   120000670:	57fe63ff 	bl          	-416	# 1200004d0 <fn3@plt>
+
+0000000120000674 <medium_call_nop>:
+   120000674:	03400000 	nop
+   120000678:	03400000 	nop
+   12000067c:	03400000 	nop
+   120000680:	03400000 	nop
+   120000684:	1e000001 	pcaddu18i   	\$ra, 0
+   120000688:	4ffe4c21 	jirl        	\$ra, \$ra, -436
+
+000000012000068c <normal_call_delete>:
+   12000068c:	57fe47ff 	bl          	-444	# 1200004d0 <fn3@plt>
+
+0000000120000690 <medium_call_delete>:
+   120000690:	57fe43ff 	bl          	-448	# 1200004d0 <fn3@plt>
+#pass
diff --git a/ld/testsuite/ld-loongarch-elf/call_undefweak.s b/ld/testsuite/ld-loongarch-elf/call_undefweak.s
new file mode 100644
index 00000000000..684b2964e49
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/call_undefweak.s
@@ -0,0 +1,48 @@ 
+	.text
+	.align	2
+	.globl	main
+	.type	main, @function
+main:
+	# undefweak symbol with .hidden and .protected
+	# do not need plt entry, Calls to these symbols
+	# are converted to nop when relax is disabled.
+nornal_call_nop:
+	.option norelax
+	bl	%plt(fn1)
+	bl	%plt(fn2)
+	bl	%plt(fn3)
+
+	# Medium call uses pcaddu18i+jirl, we need to
+	# convert two instructions to nop.
+medium_call_nop:
+	pcaddu18i $r1,%call36(fn1)
+	jirl	  $r1,$r1,0
+	pcaddu18i $r1,%call36(fn2)
+	jirl	  $r1,$r1,0
+	pcaddu18i $r1,%call36(fn3)
+	jirl	  $r1,$r1,0
+
+
+	# These insns call undefweak symbol without
+	# plt entry will be deleted when relax is
+	# enabled.
+normal_call_delete:
+	.option relax
+	bl	%plt(fn1)
+	bl	%plt(fn2)
+	bl	%plt(fn3)
+medium_call_delete:
+	pcaddu18i $r1,%call36(fn1)
+	jirl	  $r1,$r1,0
+	pcaddu18i $r1,%call36(fn2)
+	jirl	  $r1,$r1,0
+	pcaddu18i $r1,%call36(fn3)
+	jirl	  $r1,$r1,0
+
+	.weak	fn1
+	.hidden	fn1
+
+	.weak	fn2
+	.protected  fn2
+
+	.weak	fn3
diff --git a/ld/testsuite/ld-loongarch-elf/jmp_op.d b/ld/testsuite/ld-loongarch-elf/jmp_op.d
index 231d780923e..fe7715a4678 100644
--- a/ld/testsuite/ld-loongarch-elf/jmp_op.d
+++ b/ld/testsuite/ld-loongarch-elf/jmp_op.d
@@ -25,8 +25,10 @@  Disassembly of section .text:
 [ 	]+20:[ 	]+4c000080[ 	]+jirl[ 	]+\$zero,[ 	]+\$a0,[ 	]+0
 [ 	]+24:[ 	]+53ffdfff[ 	]+b[ 	]+-36[ 	]+#[ 	]+0[ 	]+<.L1>
 [ 	]+24:[ 	]+R_LARCH_B26[ 	]+.L1
+[ 	]+24:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+28:[ 	]+57ffdbff[ 	]+bl[ 	]+-40[ 	]+#[ 	]+0[ 	]+<.L1>
 [ 	]+28:[ 	]+R_LARCH_B26[ 	]+.L1
+[ 	]+28:[ 	]+R_LARCH_RELAX[ 	]+\*ABS\*
 [ 	]+2c:[ 	]+5bffd485[ 	]+beq[ 	]+\$a0,[ 	]+\$a1,[ 	]+-44[ 	]+#[ 	]+0[ 	]+<.L1>
 [ 	]+2c:[ 	]+R_LARCH_B16[ 	]+.L1
 [ 	]+30:[ 	]+5fffd085[ 	]+bne[ 	]+\$a0,[ 	]+\$a1,[ 	]+-48[ 	]+#[ 	]+0[ 	]+<.L1>
diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
index 3313f72eee4..8335158dc60 100644
--- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
+++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
@@ -142,6 +142,16 @@  if [istarget "loongarch64-*-*"] {
 	    "abs-global.out" \
 	] \
     ]
+
+  run_cc_link_tests [list \
+	[list \
+	    "call undefweak symbol" \
+	    "" "" \
+	    {call_undefweak.s} \
+	    {{objdump {-d} call_undefweak.d}} \
+	    "call_undefweak" \
+	] \
+    ]
 }
 
 if [istarget "loongarch64-*-*"] {