riscvarchive / riscv-binutils-gdb

RISC-V backports for binutils-gdb. Development is done upstream at the FSF.
GNU General Public License v2.0
147 stars 233 forks source link

64-bit relocations? (Error: value of -549755813888 too large for field of 4 bytes at 0) #168

Open tommythorn opened 5 years ago

tommythorn commented 5 years ago

I briefly looked into this, but it looks hairy to me. If you try to assemble (with the riscv64 version of GAS)

        .section .text
        sw      t2, dummy - 0x8000000000, a0
        .data
dummy:  .dword  0

you hit

fail.s: Assembler messages:
fail.s:2: Error: value of -549755813888 too large for field of 4 bytes at 0
fail.s:2: Error: value of -549755813888 too large for field of 4 bytes at 0

The message comes from https://github.com/riscv/riscv-binutils-gdb/blob/riscv-binutils-2.31.1/gas/write.c#L1113 which looks like generic GAS.

aswaterman commented 5 years ago

Hmm, I agree you should be able to do that. Obviously, the final address dummy - 0x8000000000 needs to lie within 2 GiB of the auipc, but the magnitude of the offset shouldn't matter.

aswaterman commented 5 years ago

In bfd/elfxx-riscv.c, changing the size field in the R_RISCV_PCREL_HI20 reloc definition from 2 to 3 makes the error go away. This simple example appears to correctly link after doing so. (It also correctly issues an error if the final address is too far away.) However, I'm not confident this is the complete solution, and it needs to be tested with a complete bootstrap to make sure it doesn't break anything. I don't have time for that now :)

tommythorn commented 5 years ago

Thanks, I'll run it though and see what I find. What is the "complete bootstrap"?

aswaterman commented 5 years ago

Use it to build the linux kernel, glibc, and the user-space stuff.

jim-wilson commented 5 years ago

A size of 2 translates to 4 bytes. A size of 3 translates to 0 bytes. I think you mean a size of 4 which translates to 8 bytes. It might be useful to make this conditional on the target ISA size, as a size of 2 is always OK for rv32, it is only rv64 that can use a size of 4 here. Note that the only relocs that currently have a size of 3 are R_RISCV_NONE and R_RISCV_RELAX both of which are placeholder relocs that don't directly modify the binary.

It looks like a value of 3 works by accident, because gas disables the overflow check when a reloc has a size of 0. And in the linker we are handling this reloc directly in perform_relocation without using the reloc size. We shouldn't be using something that only works by accident.

A value of 4 fails because now the reloc size is larger than the instruction, and I get a "fixup not contained within frag" error because it thinks we are trying to write past the end of memory. So that can't be right either.

I think we need a different solution here. Like setting the fx_no_overflow bit in gas for this fixup, to disable the inappropriate overflow check. This may require doing our own overflow checking in md_apply_fix. Or maybe gas should be a little smarter, and disable the overflow check for pc-relative relocs, as you can't expect to detect overflow in this case in the assembler when we don't know the link time address yet. I see that the z8k port is setting fx_no_overflow for pc-relative relocs.

Actually, in the mips gas port, I see / These relocations can have an addend that won't fit in
4 octets for 64bit assembly.
/ if (GPR_SIZE == 64 ...) ip->fixp[0]->fx_no_overflow = 1; which looks like the right fix for this problem, but we need a list of affected RISC-V relocs, and we need to check to see if other assembler changes are required to make this work.

tommythorn commented 5 years ago

I can confirm that the patch below worked for my problem, but I gave up trying to claim that this is correct & sufficient:

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 987377ae81..9cf33395c8 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -940,6 +940,11 @@ append_insn (struct riscv_cl_insn *ip, expressionS *address_expr,
                                  address_expr, FALSE, reloc_type);

          ip->fixp->fx_tcbit = riscv_opts.relax;
+
+          if (abi_xlen == 64 && (reloc_type == BFD_RELOC_RISCV_PCREL_HI20
+                                 || reloc_type == BFD_RELOC_RISCV_PCREL_LO12_I
+                                 || reloc_type == BFD_RELOC_RISCV_PCREL_LO12_S))
+              ip->fixp->fx_no_overflow = 1;
        }
     }