From abd20cb637008da9d32018b4b03973e119388a0a Mon Sep 17 00:00:00 2001 From: Nelson Chu Date: Tue, 17 Nov 2020 19:39:52 -0800 Subject: RISC-V: Relax PCREL to GPREL while doing other relaxations is dangerous. I get the feedback recently that enable linker relaxations may fail to build some program. Consider the following case, .text foo: addi a0, a0, %pcrel_lo(.L2) call foo .L1: auipc a1, %pcrel_hi(data_g) addi a1, a1, %pcrel_lo(.L1) lui a2, %hi(data_g) addi a2, a2, %lo(data_g) lui a3, %tprel_hi(data_t) add a3, a3, tp, %tprel_add(data_t) addi a3, a3, %tprel_lo(data_t) .L2: auipc a0, %pcrel_hi(data_g) .data .word 0x0 .global data_g data_g: .word 0x1 .section .tbss data_t: .word 0x0 The current ld reports `dangerous relocation error` when doing the pcgp relaxation, test.o: in function `foo': (.text+0x0): dangerous relocation: %pcrel_lo missing matching %pcrel_hi The .L2 auipc should not be removed since it is behind the corresponding addi, so we record the information in the pcgp_relocs table to avoid removing the auipc later. But current ld still remove it since we do not update the pcgp_relocs table while doing other relaxations. I have two solutions to fix the problem, 1. Update the pcgp_relocs table once we actually delete the code. 2. Add new relax pass to do the pcgp relaxations At first I tried to do the first solution, and we need to update at least three information - hi_sec_off of riscv_pcgp_lo_reloc, hi_sec_off and hi_addr (symbol value) of riscv_pcgp_hi_reloc. Update the hi_sec_off is simple, but it is more complicate to update the symbol value, since we almost have to do parts the same works of _bfd_riscv_relax_call again in the riscv_relax_delete_bytes to get the correct symbol value. Compared with the first solution, the second one is more intuitive and simple. We add a new relax pass to do the pcgp relaxations later, so we will get all the information correctly in the _bfd_riscv_relax_call, including the symbol value, without changing so much code. I do not see any penalty by adding a new relax pass for now, so it should be fine to delay the pcgp relaxations. Besides, I have pass all riscv-gnu-toolchain regressions for this patch. bfd/ * elfnn-riscv.c (_bfd_riscv_relax_section): Add a new relax pass to do the pcgp relaxation later, after the lui and call relaxations, but before the delete and alignment relaxations. ld/ * emultempl/riscvelf.em (riscv_elf_before_allocation): Change link_info.relax_pass from 3 to 4. * testsuite/ld-riscv-elf/pcgp-relax.d: New testcase. * testsuite/ld-riscv-elf/pcgp-relax.s: Likewise. * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. --- bfd/ChangeLog | 6 ++++++ bfd/elfnn-riscv.c | 31 ++++++++++++++++++------------- 2 files changed, 24 insertions(+), 13 deletions(-) (limited to 'bfd') diff --git a/bfd/ChangeLog b/bfd/ChangeLog index b2553a6..36a048a 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,9 @@ +2020-11-21 Nelson Chu + + * elfnn-riscv.c (_bfd_riscv_relax_section): Add a new relax pass + to do the pcgp relaxation later, after the lui and call relaxations, + but before the delete and alignment relaxations. + 2020-11-20 Nick Alcock * elflink.c (elf_finalize_dynstr): Call examine_strtab after diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c index a5b2c8a..4e87140 100644 --- a/bfd/elfnn-riscv.c +++ b/bfd/elfnn-riscv.c @@ -4607,7 +4607,7 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, || (sec->flags & SEC_RELOC) == 0 || sec->reloc_count == 0 || (info->disable_target_specific_optimizations - && info->relax_pass == 0)) + && info->relax_pass < 2)) return TRUE; riscv_init_pcgp_relocs (&pcgp_relocs); @@ -4645,17 +4645,13 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, relax_func = NULL; if (info->relax_pass == 0) { - if (type == R_RISCV_CALL || type == R_RISCV_CALL_PLT) + if (type == R_RISCV_CALL + || type == R_RISCV_CALL_PLT) relax_func = _bfd_riscv_relax_call; else if (type == R_RISCV_HI20 || type == R_RISCV_LO12_I || type == R_RISCV_LO12_S) relax_func = _bfd_riscv_relax_lui; - else if (!bfd_link_pic(info) - && (type == R_RISCV_PCREL_HI20 - || type == R_RISCV_PCREL_LO12_I - || type == R_RISCV_PCREL_LO12_S)) - relax_func = _bfd_riscv_relax_pc; else if (type == R_RISCV_TPREL_HI20 || type == R_RISCV_TPREL_ADD || type == R_RISCV_TPREL_LO12_I @@ -4663,7 +4659,22 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, relax_func = _bfd_riscv_relax_tls_le; else continue; + } + else if (info->relax_pass == 1 + && !bfd_link_pic(info) + && (type == R_RISCV_PCREL_HI20 + || type == R_RISCV_PCREL_LO12_I + || type == R_RISCV_PCREL_LO12_S)) + relax_func = _bfd_riscv_relax_pc; + else if (info->relax_pass == 2 && type == R_RISCV_DELETE) + relax_func = _bfd_riscv_relax_delete; + else if (info->relax_pass == 3 && type == R_RISCV_ALIGN) + relax_func = _bfd_riscv_relax_align; + else + continue; + if (info->relax_pass < 2) + { /* Only relax this reloc if it is paired with R_RISCV_RELAX. */ if (i == sec->reloc_count - 1 || ELFNN_R_TYPE ((rel + 1)->r_info) != R_RISCV_RELAX @@ -4673,12 +4684,6 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, /* Skip over the R_RISCV_RELAX. */ i++; } - else if (info->relax_pass == 1 && type == R_RISCV_DELETE) - relax_func = _bfd_riscv_relax_delete; - else if (info->relax_pass == 2 && type == R_RISCV_ALIGN) - relax_func = _bfd_riscv_relax_align; - else - continue; data->relocs = relocs; -- cgit v1.1