This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] RISC-V: Give error for ignored pcrel_lo addend.


This fixes a problem reported at
  https://github.com/riscv/riscv-binutils-gdb/issues/138

This fixes a problem found by the LLVM port.  When a %pcrel_lo operator has
an addend, GNU ld can silently produce incorrect output.  This is unfortunately
difficult to fix as it interacts with the deletion of bytes during linker
relaxation.  To get this right, we would have to search through all relocations
and adjust them, whereas currently we only search through symbols.  There is
also a problem with the code searching for a matching %pcrel_hi reloc, as it
is ignoring the addends.  There is a proposal to modify the ABI to make this
construct invalid.  Meanwhile, I'm modifying GNU ld to give an error instead
of incorrect output.

I also noticed that the misc other errors weren't handled correctly, because
they were generating warnings instead of the intended errors, so I fixed them
to call einfo instead of warning, and added %X%P to give more conventional
looking error messages.

This was tested with 32/64 elf/linux binutils/gcc builds and make checks.  And
also a linux kernel/buildroot/qemu build and boot.  There were no regressions.

Committed.

Jim

	bfd/
	* elfnn-riscv.c (riscv_elf_relocate_section): Use bfd_reloc_dangerous
	when pcrel_lo reloc has an addend.  Use reloc_dangerous callback for
	bfd_reloc_dangerous.  Use einfo instead of warning callback for errors.
	Add %X%P to error messages.

	ld/
	* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Run pcrel-lo-addend test.
	* testsuite/ld-riscv-elf/pcrel-lo-addend.d: New.
	* testsuite/ld-riscv-elf/pcrel-lo-addend.s: New.
---
 bfd/elfnn-riscv.c                           | 24 ++++++++++++++++++------
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp  |  1 +
 ld/testsuite/ld-riscv-elf/pcrel-lo-addend.d |  5 +++++
 ld/testsuite/ld-riscv-elf/pcrel-lo-addend.s | 17 +++++++++++++++++
 4 files changed, 41 insertions(+), 6 deletions(-)
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend.d
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend.s

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index dd9c300b5e..931bd1d89d 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -1993,6 +1993,16 @@ riscv_elf_relocate_section (bfd *output_bfd,
 
 	case R_RISCV_PCREL_LO12_I:
 	case R_RISCV_PCREL_LO12_S:
+	  /* Addends are not allowed, because then riscv_relax_delete_bytes
+	     would have to search through all relocs to update the addends.
+	     Also, riscv_resolve_pcrel_lo_relocs does not support addends
+	     when searching for a matching hi reloc.  */
+	  if (rel->r_addend)
+	    {
+	      r = bfd_reloc_dangerous;
+	      break;
+	    }
+
 	  if (riscv_record_pcrel_lo_reloc (&pcrel_relocs, input_section, info,
 					   howto, rel, relocation, name,
 					   contents))
@@ -2234,25 +2244,27 @@ riscv_elf_relocate_section (bfd *output_bfd,
 	  break;
 
 	case bfd_reloc_outofrange:
-	  msg = _("internal error: out of range error");
+	  msg = _("%X%P: internal error: out of range error\n");
 	  break;
 
 	case bfd_reloc_notsupported:
-	  msg = _("internal error: unsupported relocation error");
+	  msg = _("%X%P: internal error: unsupported relocation error\n");
 	  break;
 
 	case bfd_reloc_dangerous:
-	  msg = _("internal error: dangerous relocation");
+	  info->callbacks->reloc_dangerous
+	    (info, "%pcrel_lo with addend", input_bfd, input_section,
+	     rel->r_offset);
 	  break;
 
 	default:
-	  msg = _("internal error: unknown error");
+	  msg = _("%X%P: internal error: unknown error\n");
 	  break;
 	}
 
       if (msg)
-	info->callbacks->warning
-	  (info, msg, name, input_bfd, input_section, rel->r_offset);
+	info->callbacks->einfo (msg);
+
       /* We already reported the error via a callback, so don't try to report
 	 it again by returning false.  That leads to spurious errors.  */
       ret = TRUE;
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 2b6a1d78fe..cd11680b55 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -22,6 +22,7 @@
 if [istarget "riscv*-*-*"] {
     run_dump_test "c-lui"
     run_dump_test "disas-jalr"
+    run_dump_test "pcrel-lo-addend"
 
     # The following tests require shared library support.
     if ![check_shared_lib_support] {
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend.d b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend.d
new file mode 100644
index 0000000000..bd61b4bbff
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend.d
@@ -0,0 +1,5 @@
+#name: %pcrel_lo with an addend
+#source: pcrel-lo-addend.s
+#as: -march=rv32ic
+#ld: -melf32lriscv
+#error: .*dangerous relocation: %pcrel_lo with addend
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend.s b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend.s
new file mode 100644
index 0000000000..50cdccceed
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend.s
@@ -0,0 +1,17 @@
+  .text
+  .globl _start
+_start:
+  auipc ra, %pcrel_hi(tdata)
+  addi ra, ra, %pcrel_lo(.text)
+  lb t1, 0(ra)
+foo:
+  auipc ra, %pcrel_hi(tdata)
+  addi ra, ra, %pcrel_lo(.text+12)
+  lb t2, 1(ra)
+
+  .data
+tdata:
+  .byte 0xff
+  .byte 0x00
+  .byte 0xf0
+  .byte 0x0f
-- 
2.14.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]