<div dir="ltr"><div>Umm, I probably found the root cause, so the workaround in this patch should be wrong.  I will send another patch for this issue later after passing some regressions.</div><div><br></div><div>Thanks</div><div>Nelson</div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, Jun 20, 2025 at 1:28 PM Nelson Chu <<a href="mailto:nelson@rivosinc.com">nelson@rivosinc.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi guys,</div><div dir="ltr"><br></div><div dir="ltr">I plan to commit this one, and the latter one "RISC-V: Don't allocate dynamic relocation if it's section was discarded", if there are no objections until next weekend ;)</div><div dir="ltr"><br></div><div>Thanks</div><div>Nelson</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jun 7, 2025 at 4:36 PM Nelson Chu <<a href="mailto:nelson@rivosinc.com" target="_blank">nelson@rivosinc.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This happened when I was porting the relr stuff, and caused problem for<br>
relr-discard testcases.  Considering there is an got entry needs an dynamic<br>
RELATIVE relocation when pie, we used to have an assert to make sure that the<br>
h->got.offset was marked as done in the relocate_section function.  But if the<br>
sections of related GOT_HO20 are all discared by linker script, all related<br>
GOT_HI20 will be skipped and converted to R_RISCV_NONE in the relocate_section,<br>
so that the h->got.offset won't be marked as done, and then break the link.<br>
<br>
According to the relr-discard testcases of aarch64 and loongarch, it seems<br>
still need to generate a dynamic RELATIVE for this case.  The loongarch seems<br>
doesn't have the assert, but aarch64 and x86 have.  I don't know how it works<br>
with the assert check for this case, so I just ignore the assert until someone<br>
clarify and figure it out.<br>
---<br>
 bfd/elfnn-riscv.c                          | 17 ++++-<br>
 ld/testsuite/ld-riscv-elf/discard-pic.d    | 15 +++++<br>
 ld/testsuite/ld-riscv-elf/discard-pie.d    | 15 +++++<br>
 ld/testsuite/ld-riscv-elf/discard.ld       | 13 ++++<br>
 ld/testsuite/ld-riscv-elf/discard.s        | 75 ++++++++++++++++++++++<br>
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp |  3 +<br>
 6 files changed, 135 insertions(+), 3 deletions(-)<br>
 create mode 100644 ld/testsuite/ld-riscv-elf/discard-pic.d<br>
 create mode 100644 ld/testsuite/ld-riscv-elf/discard-pie.d<br>
 create mode 100644 ld/testsuite/ld-riscv-elf/discard.ld<br>
 create mode 100644 ld/testsuite/ld-riscv-elf/discard.s<br>
<br>
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c<br>
index 1c494f5f986..db9372c2b10 100644<br>
--- a/bfd/elfnn-riscv.c<br>
+++ b/bfd/elfnn-riscv.c<br>
@@ -3446,9 +3446,20 @@ riscv_elf_finish_dynamic_symbol (bfd *output_bfd,<br>
          /* If this is a local symbol reference, we just want to emit<br>
             a RELATIVE reloc.  This can happen if it is a -Bsymbolic link,<br>
             or a pie link, or the symbol was forced to be local because<br>
-            of a version file.  The entry in the global offset table will<br>
-            already have been initialized in the relocate_section function.  */<br>
-         BFD_ASSERT ((h->got.offset & 1) != 0);<br>
+            of a version file.  The entry in the global offset table "should"<br>
+            already have been initialized in the relocate_section function,<br>
+            except the sections of the related relocation are all discarded.  */<br>
+<br>
+         /* If the sections of the related relocation are all discarded, then<br>
+            the RELOC_AGAINST_DISCARDED_SECTION cleans all related relocation<br>
+            to R_RISCV_NONE in the relocate_section, so the h->got.offset will<br>
+            not be marked as done in the relocate_section function.  In this<br>
+            case most of the targets seems still generate a RELATIVE reloc,<br>
+            but I don't know how that works with the following assert, so just<br>
+            ignore it until someone clarify and figure this out.<br>
+<br>
+         BFD_ASSERT ((h->got.offset & 1) != 0);  */<br>
+<br>
          asection *sec = h->root.u.def.section;<br>
          rela.r_info = ELFNN_R_INFO (0, R_RISCV_RELATIVE);<br>
          rela.r_addend = (h->root.u.def.value<br>
diff --git a/ld/testsuite/ld-riscv-elf/discard-pic.d b/ld/testsuite/ld-riscv-elf/discard-pic.d<br>
new file mode 100644<br>
index 00000000000..f876cdf06ba<br>
--- /dev/null<br>
+++ b/ld/testsuite/ld-riscv-elf/discard-pic.d<br>
@@ -0,0 +1,15 @@<br>
+#source: discard.s<br>
+#as: -march=rv64i -mabi=lp64<br>
+#ld: -shared -T discard.ld -melf64lriscv<br>
+#readelf: -rW<br>
+<br>
+Relocation section '\.rela\.dyn'.*<br>
+[      ]+Offset[       ]+Info[         ]+Type.*<br>
+0000000000020010[      ]+[0-9a-f]+[    ]+R_RISCV_RELATIVE[     ]+10004<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0000000000020018[      ]+[0-9a-f]+[    ]+R_RISCV_64[   ]+000000000001000c[     ]+sym_global \+ 0<br>
+0000000000020020[      ]+[0-9a-f]+[    ]+R_RISCV_64[   ]+000000000001000c[     ]+sym_global_keep \+ 0<br>
diff --git a/ld/testsuite/ld-riscv-elf/discard-pie.d b/ld/testsuite/ld-riscv-elf/discard-pie.d<br>
new file mode 100644<br>
index 00000000000..4e0d8cfb495<br>
--- /dev/null<br>
+++ b/ld/testsuite/ld-riscv-elf/discard-pie.d<br>
@@ -0,0 +1,15 @@<br>
+#source: discard.s<br>
+#as: -march=rv64i -mabi=lp64<br>
+#ld: -pie -T discard.ld -melf64lriscv<br>
+#readelf: -rW<br>
+<br>
+Relocation section '\.rela\.dyn'.*<br>
+[      ]+Offset[       ]+Info[         ]+Type.*<br>
+0000000000020010[      ]+[0-9a-f]+[    ]+R_RISCV_RELATIVE[     ]+10004<br>
+0000000000020018[      ]+[0-9a-f]+[    ]+R_RISCV_RELATIVE[     ]+1000c<br>
+0000000000020020[      ]+[0-9a-f]+[    ]+R_RISCV_RELATIVE[     ]+1000c<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0<br>
diff --git a/ld/testsuite/ld-riscv-elf/discard.ld b/ld/testsuite/ld-riscv-elf/discard.ld<br>
new file mode 100644<br>
index 00000000000..3afed216fbc<br>
--- /dev/null<br>
+++ b/ld/testsuite/ld-riscv-elf/discard.ld<br>
@@ -0,0 +1,13 @@<br>
+OUTPUT_ARCH(riscv)<br>
+ENTRY(_start)<br>
+SECTIONS<br>
+{<br>
+  /DISCARD/ : { *(.discard.*) }<br>
+<br>
+  . = 0x10000;<br>
+  .text : { *(.text) }<br>
+  . = 0x20000;<br>
+  .got  : { *(.got) *(.got.plt)}<br>
+  . = 0x30000;<br>
+  .data : { *(.data) *(.data.*) }<br>
+}<br>
diff --git a/ld/testsuite/ld-riscv-elf/discard.s b/ld/testsuite/ld-riscv-elf/discard.s<br>
new file mode 100644<br>
index 00000000000..729a5f9995a<br>
--- /dev/null<br>
+++ b/ld/testsuite/ld-riscv-elf/discard.s<br>
@@ -0,0 +1,75 @@<br>
+.text<br>
+.option pic<br>
+.p2align 3<br>
+.global _start<br>
+_start:<br>
+       nop<br>
+<br>
+sym_local:<br>
+sym_local_keep:<br>
+       nop<br>
+<br>
+.global sym_hidden<br>
+.hidden sym_hidden<br>
+sym_hidden:<br>
+       nop<br>
+<br>
+.global sym_global<br>
+.global sym_global_keep<br>
+sym_global:<br>
+sym_global_keep:<br>
+       nop<br>
+<br>
+.global sym_global_abs<br>
+.set sym_global_abs, 42<br>
+<br>
+.global sym_weak_undef<br>
+.weak sym_weak_undef<br>
+<br>
+.section .got_local_keep,"ax"<br>
+        la      x1, sym_local_keep<br>
+.section .discard.got_local_keep,"ax"<br>
+       la      x1, sym_local_keep<br>
+<br>
+.section .got_global_keep,"ax"<br>
+        la      x1, sym_global_keep<br>
+.section .discard.got_global_keep,"ax"<br>
+       la      x1, sym_global_keep<br>
+<br>
+# All relocation refer to sym_local are discarded.<br>
+.section .discard.got_local,"ax"<br>
+       la      x1, sym_local<br>
+<br>
+# All relocation refer to sym_global are discarded.<br>
+.section .discard.got_global,"ax"<br>
+       la      x1, sym_global<br>
+<br>
+.section .discard.local,"a"<br>
+.p2align 1<br>
+discard_local:<br>
+.quad sym_local<br>
+<br>
+.section .discard.hidden,"a"<br>
+.p2align 1<br>
+discard_hidden:<br>
+.quad sym_hidden<br>
+<br>
+.section .discard.global,"a"<br>
+.p2align 1<br>
+discard_global:<br>
+.quad sym_global<br>
+<br>
+.section .discard.global_abs,"a"<br>
+.p2align 1<br>
+discard_global_abs:<br>
+.quad sym_global_abs<br>
+<br>
+.section .discard.weak_undef,"a"<br>
+.p2align 1<br>
+discard_weak_undef:<br>
+.quad sym_weak_undef<br>
+<br>
+.section .discard._DYNAMIC,"a"<br>
+.p2align 1<br>
+discard_DYNAMIC:<br>
+.quad _DYNAMIC<br>
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp<br>
index 9cb847d3d8a..d68a9145dcd 100644<br>
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp<br>
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp<br>
@@ -227,6 +227,9 @@ if [istarget "riscv*-*-*"] {<br>
     run_dump_test "data-reloc-rv64-addr32-pic"<br>
     run_dump_test "data-reloc-rv64-undef32-pic"<br>
<br>
+    run_dump_test "discard-pie"<br>
+    run_dump_test "discard-pic"<br>
+<br>
     # IFUNC testcases.<br>
     # Check IFUNC by single type relocs.<br>
     run_dump_test_ifunc "ifunc-reloc-call-01" rv32 exe<br>
-- <br>
2.39.5 (Apple Git-154)<br>
<br>
</blockquote></div></div>
</div>
</blockquote></div></div>