[PATCH v2 1/2] RISC-V: Fix the assert fail when linking discarded sections under -pie for got
Nelson Chu
nelson@rivosinc.com
Tue Jul 22 06:34:40 GMT 2025
Committed, thanks.
Nelson
On Thu, Jul 3, 2025 at 11:16 AM Nelson Chu <nelson@rivosinc.com> wrote:
> 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.
> ---
> 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(-)
> create mode 100644 ld/testsuite/ld-riscv-elf/discard-exe.d
> create mode 100644 ld/testsuite/ld-riscv-elf/discard-pic.d
> create mode 100644 ld/testsuite/ld-riscv-elf/discard-pie.d
> create mode 100644 ld/testsuite/ld-riscv-elf/discard.ld
> create mode 100644 ld/testsuite/ld-riscv-elf/discard.s
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 790f0397cf5..26efe87d687 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;
> @@ -2872,6 +2871,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;
> @@ -2895,22 +2903,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);
> @@ -2918,6 +2911,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 e103df66e11..24d6e3badca 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -235,6 +235,10 @@ if [istarget "riscv*-*-*"] {
>
> run_dump_test "zicfilp-unlabeled-plt"
>
> + 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
> --
> 2.39.5 (Apple Git-154)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250722/365e98aa/attachment-0001.htm>
More information about the Binutils
mailing list