Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

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

Open
tommythorn opened this issue Feb 28, 2019 · 6 comments

Comments

@tommythorn
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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

@aswaterman
Copy link
Contributor

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

@jim-wilson
Copy link
Collaborator

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
Copy link
Author

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;
        }
     }

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants