[binutils-gdb] RISC-V: Relax PCREL to GPREL while doing other relaxations is dangerous.
Nelson Chu
nelsonc1225@sourceware.org
Sat Nov 21 01:50:10 GMT 2020
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=abd20cb637008da9d32018b4b03973e119388a0a
commit abd20cb637008da9d32018b4b03973e119388a0a
Author: Nelson Chu <nelson.chu@sifive.com>
Date: Tue Nov 17 19:39:52 2020 -0800
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.
Diff:
---
bfd/ChangeLog | 6 ++++++
bfd/elfnn-riscv.c | 31 +++++++++++++++++-------------
ld/ChangeLog | 8 ++++++++
ld/emultempl/riscvelf.em | 2 +-
ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 +
ld/testsuite/ld-riscv-elf/pcgp-relax.d | 16 +++++++++++++++
ld/testsuite/ld-riscv-elf/pcgp-relax.s | 29 ++++++++++++++++++++++++++++
7 files changed, 79 insertions(+), 14 deletions(-)
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index b2553a65166..36a048a872a 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2020-11-21 Nelson Chu <nelson.chu@sifive.com>
+
+ * 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 <nick.alcock@oracle.com>
* elflink.c (elf_finalize_dynstr): Call examine_strtab after
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index a5b2c8a198c..4e8714045a4 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;
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 5e81fa267ea..4686c592e4c 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,11 @@
+2020-11-21 Nelson Chu <nelson.chu@sifive.com>
+
+ * 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.
+
2020-11-20 Nick Alcock <nick.alcock@oracle.com>
* testsuite/ld-ctf/data-func-conflicted.d: Shrink the expected
diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
index da5c934d25d..077f976492c 100644
--- a/ld/emultempl/riscvelf.em
+++ b/ld/emultempl/riscvelf.em
@@ -42,7 +42,7 @@ riscv_elf_before_allocation (void)
ENABLE_RELAXATION;
}
- link_info.relax_pass = 3;
+ link_info.relax_pass = 4;
}
static void
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 9834041acf6..86910e60ec3 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -62,6 +62,7 @@ proc run_dump_test_ifunc { name target output} {
if [istarget "riscv*-*-*"] {
run_dump_test "call-relax"
+ run_dump_test "pcgp-relax"
run_dump_test "c-lui"
run_dump_test "c-lui-2"
run_dump_test "disas-jalr"
diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax.d b/ld/testsuite/ld-riscv-elf/pcgp-relax.d
new file mode 100644
index 00000000000..dae2b62b9a7
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcgp-relax.d
@@ -0,0 +1,16 @@
+#source: pcgp-relax.s
+#ld: --relax
+#objdump: -d -Mno-aliases
+
+.*:[ ]+file format .*
+
+
+Disassembly of section \.text:
+
+0+[0-9a-f]+ <_start>:
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
+.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,gp,\-[0-9]+ # [0-9a-f]+ <data_g>
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,gp,\-[0-9]+ # [0-9a-f]+ <data_g>
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
+.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax.s b/ld/testsuite/ld-riscv-elf/pcgp-relax.s
new file mode 100644
index 00000000000..fab6a5bf140
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcgp-relax.s
@@ -0,0 +1,29 @@
+ .text
+ .globl _start
+_start:
+ addi a0, a0, %pcrel_lo(.L2)
+
+ call _start
+.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
+ .globl data_g
+data_g:
+ .word 0x1
+
+ .section .tbss
+data_t:
+ .word 0x0
More information about the Binutils-cvs
mailing list