From 9e10fcf71c1101fb6422d0f52de5e615ed8df71d Mon Sep 17 00:00:00 2001 From: Nelson Chu Date: Thu, 3 Jul 2025 11:16:24 +0800 Subject: [PATCH] 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. --- 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 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 -- 2.47.2