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 --git a/bfd/ChangeLog b/bfd/ChangeLog
index b2553a651663c65d6deeccb91cb69bec3fe710e8..36a048a872a307118f32ea098442935580599bb1 100644 (file)
--- 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 a5b2c8a198cefaf029ff8c2088b0af34a44cd9de..4e8714045a4901736fee8ea7b93bd03b9259a6ee 100644 (file)
--- 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 5e81fa267eac7f1a4e22215f03ea169ae58d0bfb..4686c592e4c08ed3512877fca0ab4eb4d0af7a97 100644 (file)
--- 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 da5c934d25d163225627bc4627977064931b4554..077f976492c0944e502b20a4487c608c4e4dd492 100644 (file)
--- 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 9834041acf6e4bde91faa048a1bf00e34c95b31b..86910e60ec3f33b8703e8f3527b89372b8edab33 100644 (file)
--- 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 (file)
index 0000000..dae2b62
--- /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 (file)
index 0000000..fab6a5b
--- /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