[binutils-gdb] RISC-V: Fix the assert fail when linking discarded sections under -pie for got
Nelson Chu
nelsonc1225@sourceware.org
Tue Jul 22 06:33:57 GMT 2025
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9e10fcf71c1101fb6422d0f52de5e615ed8df71d
commit 9e10fcf71c1101fb6422d0f52de5e615ed8df71d
Author: Nelson Chu <nelson@rivosinc.com>
Date: Thu Jul 3 11:16:24 2025 +0800
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:
---
bfd/elfnn-riscv.c | 88 ++++++++++++++++--------------
ld/testsuite/ld-riscv-elf/discard-exe.d | 6 ++
ld/testsuite/ld-riscv-elf/discard-pic.d | 9 +++
ld/testsuite/ld-riscv-elf/discard-pie.d | 9 +++
ld/testsuite/ld-riscv-elf/discard.ld | 13 +++++
ld/testsuite/ld-riscv-elf/discard.s | 20 +++++++
ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 4 ++
7 files changed, 109 insertions(+), 40 deletions(-)
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 4689f388927..84f121c1b14 100644
--- 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
index 00000000000..7bdb16b197d
--- /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
index 00000000000..9ac2cc6850f
--- /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
index 00000000000..cb95c4a9ffb
--- /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
index 00000000000..3afed216fbc
--- /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
index 00000000000..8dd15a83906
--- /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 70c9aa7f66a..bae1105cad6 100644
--- 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
More information about the Binutils-cvs
mailing list