RISC-V: Fix the assert fail when linking discarded sections under -pie for got

Considering the following case,
% cat tmp.s
.option pic
.text
.global _start
_start:
nop
.section .discard.s, "ax"
la x1, _start
% cat tmp.ld
OUTPUT_ARCH(riscv)
ENTRY(_start)
SECTIONS
{
  /DISCARD/ : { *(.discard.*) }
  . = 0x10000;
  .text : { *(.text) }
  . = 0x20000;
  .got  : { *(.got) *(.got.plt)}
  . = 0x30000;
  .data : { *(.data) *(.data.*) }
}
% riscv64-unknown-linux-gnu-as tmp.s -o tmp.o
% riscv64-unknown-linux-gnu-ld -pie -Ttmp.ld tmp.o
riscv64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.44.50.20250624 assertion fail binutils-gdb/bfd/elfnn-riscv.c:3638

This happens when pie and the input sections, which refers to the global
symbol by got, are all discarded.  Since referenced sections are all discarded,
we won't go into relocate_section for those sections, the got entry also won't
be initialized.  Therefore, we will get assert fail when adding the RELATIVE
reloc in the finish_dynamic_symbol.

After seeing other target codes, there are two root causes as follows,

1. risc-v may call bfd_elf_link_record_dynamic_symbol in the allocate_dynrelocs
   for not only undefweak symbols.

2. risc-v is missing the code to add RELATIVE to R_RISCV_GOT entries in the
   relocate_section if a symbol is not dynamic and is not undefined weak under
   pic and pie.

If we call bfd_elf_link_record_dynamic_symbol, then the global symbol will be
forced to dynamic, so the h->dynindx will forced to be a number rather than -1,
even it should be -1.  Once h->dynindx != -1 and pic/pie, it will go into
finish_dynamic_symbol and insert RELATIVE/64 relocs for the got entry;  For the
above case there are two issues,

1. The global symbol _start is forced to be dynamic in the allocate_dynrelocs.
   when pie and all the referenced section are discarded, it won't go into
   relocate_section to initialize the got entry, so it will cause assert fail
   when adding RELATIVE reloc in the finish_dynamic_symbol.  The assert fail
   represents another problem - if we don't initialize the got entry in the
   relocate_section under pie, which means we don't need to go into the
   finish_dynamic_symbol and don't need a RELATIVE reloc for the got entry,
   it should be NONE reloc.

2. Without linking any discarded section, it originally forces every RELATIVE
   relocs added for every got by the finish_dynamic_symbol.  Even The final
   result looks correct under pie (genearte a RELATIVE reloc for got entry),
   not sure if it may cause other problems for some special cases, excpet the
   above one.

Therefore, this patch try to fix the above assert fail, and also clarify the
behavior of the allocate_dynrelocs which should only call bfd_elf_link_record_dynamic_symbol
for undefweak symbols, and add the missing code to generate RELATIVE reloc to
R_RISCV_GOT entries in the relocate_section if a symbol is not dynamic and is
not undefined weak under pic and pie.

Passed the gcc/binutils regressions of riscv-gnu-toolchain at least.

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 4689f3889275e59e9df3ede419dcfde5ba9d788f..84f121c1b14e22699e0bc046a955032425f97aad 100644 (file)
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -1441,11 +1441,10 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
       /* Make sure this symbol is output as a dynamic symbol.
         Undefined weak syms won't yet be marked as dynamic.  */
       if (h->dynindx == -1
-         && !h->forced_local)
-       {
-         if (! bfd_elf_link_record_dynamic_symbol (info, h))
-           return false;
-       }
+         && !h->forced_local
+         && h->root.type == bfd_link_hash_undefweak
+         && !bfd_elf_link_record_dynamic_symbol (info, h))
+       return false;
 
       if (WILL_CALL_FINISH_DYNAMIC_SYMBOL (1, bfd_link_pic (info), h))
        {
@@ -1497,21 +1496,20 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
   if (h->got.refcount > 0)
     {
       asection *s;
-      bool dyn;
+      bool dyn = htab->elf.dynamic_sections_created;
       int tls_type = riscv_elf_hash_entry (h)->tls_type;
 
       /* Make sure this symbol is output as a dynamic symbol.
         Undefined weak syms won't yet be marked as dynamic.  */
-      if (h->dynindx == -1
-         && !h->forced_local)
-       {
-         if (! bfd_elf_link_record_dynamic_symbol (info, h))
-           return false;
-       }
+      if (dyn
+         && h->dynindx == -1
+         && !h->forced_local
+         && h->root.type == bfd_link_hash_undefweak
+         && !bfd_elf_link_record_dynamic_symbol (info, h))
+       return false;
 
       s = htab->elf.sgot;
       h->got.offset = s->size;
-      dyn = htab->elf.dynamic_sections_created;
       if (tls_type & (GOT_TLS_GD | GOT_TLS_IE | GOT_TLSDESC))
        {
          int indx = 0;
@@ -1545,7 +1543,10 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
       else
        {
          s->size += GOT_ENTRY_SIZE;
-         if (WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, bfd_link_pic (info), h)
+         if ((ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
+              || h->root.type != bfd_link_hash_undefweak)
+             && (bfd_link_pic (info)
+                 || WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, 0, h))
              && ! UNDEFWEAK_NO_DYNAMIC_RELOC (info, h))
            htab->elf.srelgot->size += sizeof (ElfNN_External_Rela);
        }
@@ -1591,11 +1592,9 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
          /* Make sure undefined weak symbols are output as a dynamic
             symbol in PIEs.  */
          else if (h->dynindx == -1
-                  && !h->forced_local)
-           {
-             if (! bfd_elf_link_record_dynamic_symbol (info, h))
-               return false;
-           }
+                  && !h->forced_local
+                  && !bfd_elf_link_record_dynamic_symbol (info, h))
+           return false;
        }
     }
   else
@@ -1614,11 +1613,10 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
          /* Make sure this symbol is output as a dynamic symbol.
             Undefined weak syms won't yet be marked as dynamic.  */
          if (h->dynindx == -1
-             && !h->forced_local)
-           {
-             if (! bfd_elf_link_record_dynamic_symbol (info, h))
-               return false;
-           }
+             && !h->forced_local
+             && h->root.type == bfd_link_hash_undefweak
+             && !bfd_elf_link_record_dynamic_symbol (info, h))
+           return false;
 
          /* If that succeeded, we know we'll be keeping all the
             relocs.  */
@@ -2459,6 +2457,7 @@ riscv_elf_relocate_section (bfd *output_bfd,
       const char *msg = NULL;
       bool resolved_to_zero;
       bool via_plt = false;
+      bool relative_got = false;
 
       if (howto == NULL)
        continue;
@@ -2873,6 +2872,15 @@ riscv_elf_relocate_section (bfd *output_bfd,
                    off &= ~1;
                  else
                    {
+                     /* If a symbol is not dynamic and is not undefined weak,
+                        bind it locally and generate a RELATIVE relocation
+                        under PIC mode.  */
+                     if (h->dynindx == -1
+                         && !h->forced_local
+                         && h->root.type != bfd_link_hash_undefweak
+                         && bfd_link_pic (info))
+                       relative_got = true;
+
                      bfd_put_NN (output_bfd, relocation,
                                  htab->elf.sgot->contents + off);
                      h->got.offset |= 1;
@@ -2896,22 +2904,7 @@ riscv_elf_relocate_section (bfd *output_bfd,
              else
                {
                  if (bfd_link_pic (info))
-                   {
-                     asection *s;
-                     Elf_Internal_Rela outrel;
-
-                     /* We need to generate a R_RISCV_RELATIVE reloc
-                        for the dynamic linker.  */
-                     s = htab->elf.srelgot;
-                     BFD_ASSERT (s != NULL);
-
-                     outrel.r_offset = sec_addr (htab->elf.sgot) + off;
-                     outrel.r_info =
-                       ELFNN_R_INFO (0, R_RISCV_RELATIVE);
-                     outrel.r_addend = relocation;
-                     relocation = 0;
-                     riscv_elf_append_rela (output_bfd, s, &outrel);
-                   }
+                   relative_got = true;
 
                  bfd_put_NN (output_bfd, relocation,
                              htab->elf.sgot->contents + off);
@@ -2919,6 +2912,21 @@ riscv_elf_relocate_section (bfd *output_bfd,
                }
            }
 
+         /* We need to generate a R_RISCV_RELATIVE relocation later in the
+            riscv_elf_finish_dynamic_symbol if h->dynindx != -1;  Otherwise,
+            generate a R_RISCV_RELATIVE relocation here now.  */
+         if (relative_got)
+           {
+             asection *s = htab->elf.srelgot;
+             BFD_ASSERT (s != NULL);
+
+             Elf_Internal_Rela outrel;
+             outrel.r_offset = sec_addr (htab->elf.sgot) + off;
+             outrel.r_info = ELFNN_R_INFO (0, R_RISCV_RELATIVE);
+             outrel.r_addend = relocation;
+             riscv_elf_append_rela (output_bfd, s, &outrel);
+           }
+
          if (rel->r_addend != 0)
            {
              msg = _("The addend isn't allowed for R_RISCV_GOT_HI20");

diff --git a/ld/testsuite/ld-riscv-elf/discard-exe.d b/ld/testsuite/ld-riscv-elf/discard-exe.d
new file mode 100644 (file)
index 0000000..7bdb16b
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/discard-exe.d
@@ -0,0 +1,6 @@
+#source: discard.s
+#as:
+#ld: -Tdiscard.ld
+#readelf: -rW
+
+There are no relocations in this file.

diff --git a/ld/testsuite/ld-riscv-elf/discard-pic.d b/ld/testsuite/ld-riscv-elf/discard-pic.d
new file mode 100644 (file)
index 0000000..9ac2cc6
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/discard-pic.d
@@ -0,0 +1,9 @@
+#source: discard.s
+#as:
+#ld: -shared -Tdiscard.ld
+#readelf: -rW
+
+Relocation section '\.rela\.dyn'.*
+[      ]+Offset[       ]+Info[         ]+Type.*
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0
+0+(20008|20010)[       ]+[0-9a-f]+[    ]+R_RISCV_(32|64)[      ]+0+10008[      ]+sym_global \+ 0

diff --git a/ld/testsuite/ld-riscv-elf/discard-pie.d b/ld/testsuite/ld-riscv-elf/discard-pie.d
new file mode 100644 (file)
index 0000000..cb95c4a
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/discard-pie.d
@@ -0,0 +1,9 @@
+#source: discard.s
+#as:
+#ld: -pie -Tdiscard.ld
+#readelf: -rW
+
+Relocation section '\.rela\.dyn'.*
+[      ]+Offset[       ]+Info[         ]+Type.*
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0
+0+0[   ]+0+0[  ]+R_RISCV_NONE[         ]+0

diff --git a/ld/testsuite/ld-riscv-elf/discard.ld b/ld/testsuite/ld-riscv-elf/discard.ld
new file mode 100644 (file)
index 0000000..3afed21
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/discard.ld
@@ -0,0 +1,13 @@
+OUTPUT_ARCH(riscv)
+ENTRY(_start)
+SECTIONS
+{
+  /DISCARD/ : { *(.discard.*) }
+
+  . = 0x10000;
+  .text : { *(.text) }
+  . = 0x20000;
+  .got  : { *(.got) *(.got.plt)}
+  . = 0x30000;
+  .data : { *(.data) *(.data.*) }
+}

diff --git a/ld/testsuite/ld-riscv-elf/discard.s b/ld/testsuite/ld-riscv-elf/discard.s
new file mode 100644 (file)
index 0000000..8dd15a8
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/discard.s
@@ -0,0 +1,20 @@
+.text
+.option pic
+.option norvc
+.p2align 3
+.global _start
+_start:
+       nop
+
+sym_local:
+       nop
+
+.global sym_global
+sym_global:
+       nop
+
+.section .discard.got_local, "ax"
+       la      x1, sym_local
+
+.section .discard.got_global, "ax"
+       la      x1, sym_global

diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 70c9aa7f66adce9d3b73ab5834b29ec013792018..bae1105cad6a081a5dcb739bc994aca430095643 100644 (file)
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -238,6 +238,10 @@ if [istarget "riscv*-*-*"] {
     run_dump_test "pie-bind-locally-rv32"
     run_dump_test "pie-bind-locally-rv64"
 
+    run_dump_test "discard-exe"
+    run_dump_test "discard-pie"
+    run_dump_test "discard-pic"
+
     # IFUNC testcases.
     # Check IFUNC by single type relocs.
     run_dump_test_ifunc "ifunc-reloc-call-01" rv32 exe