Page MenuHomePhabricator

[mips] Support 64-bit relative relocations
ClosedPublic

Authored by atanasyan on May 21 2020, 11:10 AM.

Details

Summary

MIPS 64-bit ABI does not provide special PC-relative relocation like R_MIPS_PC32 in 32-bit case. But we can use a "chain of relocation" defined by N64 ABIs. In that case one relocation record might contain up to three relocations which applied sequentially. Width of a final relocation mask applied to the result of relocation depends on the last relocation in the chain. In case of 64-bit PC-relative relocation we need the following chain: R_MIPS_PC32 | R_MIPS_64. The first relocation calculates an offset, but does not truncate the result. The second relocation just apply calculated result as a 64-bit value.

The 64-bit PC-relative relocation might be useful in generation of .eh_frame sections to escape passing -Wl,-z,notext flags to linker.

Diff Detail

Event Timeline

atanasyan created this revision.May 21 2020, 11:10 AM
Herald added a project: Restricted Project. · View Herald Transcript
MaskRay added a comment.EditedMay 21 2020, 12:15 PM

Honestly it is not that I care much about MIPS, but the lack of PC64 support from MIPS makes me feel uncomfortable. It imposes much burden on many toolchain components.
So I register some complaint on those ISA/ABI makers.
That said, I very much appreciate your toolchain work here. Disclamer: I know really little about MIPS.

A bit archaeology:

https://sourceware.org/git/?p=binutils-gdb.git;a=blobdiff;f=include/elf/mips.h;h=9aba63e420c06b55f8b2fb589fefacb4104ee2a1;hp=ce43158123fdc21bd3359cc014a41287f8e11c38;hb=4030e8f62467c29b782aa2480e1e452b8e458699;hpb=38cc9c28f4c40dee2dad46cc1669f97dc17a2638 removed RELOC_NUMBER (R_MIPS_PC64, 249). 249 is now occupied by R_MIPS_EH.

On https://sourceware.org/pipermail/binutils/2020-February/109879.html I asked whether we can get R_MIPS_PC64 back,
before I was told that a relocation chain can probably replace R_MIPS_PC64. Is it a perfect replacement?
Is there any downside using a relocation chain (disclaimer: I haven't even looked up what the term means)?

  • .quad foo - . is representable
  • ld.so supports such relocation chain.
  • D78590 can be fixed for MIPS
  • D72228 can get rid of the FDECFIEncoding = dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4; hack
  • For various relocation processing tool, it supporting a relocation chain as easy as supporting an addition relocation type (R_MIPS_PC64), by, adding case R_MIPS_PC64:?
  • .quad foo - . is representable
  • ld.so supports such relocation chain.
  • D78590 can be fixed for MIPS
  • D72228 can get rid of the FDECFIEncoding = dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4; hack
  • For various relocation processing tool, it supporting a relocation chain as easy as supporting an addition relocation type (R_MIPS_PC64), by, adding case R_MIPS_PC64:?

Let's wait result of testing by Joe Holden. Interesting question - should we generate these 64-bit pc-relative relocations in .eh_frame always or provide some command line option to switch behaviour?

MIPS "relocation chain" is a standard feature defined by ABIs and should be supported by various tools including linkers. BFD and Gold linkers support it even better then LLD because they can handle arbitrary set of relocations in a chain.

zjiaz added a subscriber: zjiaz.May 25 2020, 2:00 AM
  • .quad foo - . is representable
  • ld.so supports such relocation chain.
  • D78590 can be fixed for MIPS
  • D72228 can get rid of the FDECFIEncoding = dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4; hack
  • For various relocation processing tool, it supporting a relocation chain as easy as supporting an addition relocation type (R_MIPS_PC64), by, adding case R_MIPS_PC64:?

Let's wait result of testing by Joe Holden. Interesting question - should we generate these 64-bit pc-relative relocations in .eh_frame always or provide some command line option to switch behaviour?

MIPS "relocation chain" is a standard feature defined by ABIs and should be supported by various tools including linkers. BFD and Gold linkers support it even better then LLD because they can handle arbitrary set of relocations in a chain.

Hi guys,

I've tested both reviews on mips64r2 and it looks like the result is as expected, it can successfully link itself with LLD rather than BFD as well as a handful of other software where this was previously not possible.

If you want me to do any specific testing let me know!

Thanks

It a relocation chain is indeed a perfect solution, this looks good to me!

lld/test/ELF/mips-pc64.s
11

Probably avoid single dash -script which is uncommon. Use the double dash form.

Actually, I start to use -T which is much shorter.

The executable can just be %t.

12

llvm-readelf -x .data to be more specific about the tested section.

22

The labels can be dropped. They aren't used

Ok so, just stumbled upon one issue but I'm not yet sure where the problem lies given how bad mips support is in Go:

ld: error: section type mismatch for .debug_line

/tmp/go-link-985122218/000010.o:(.debug_line): SHT_MIPS_DWARF
output section .debug_line: SHT_PROGBITS

I'm still testing though, so I'll see what I uncover...

Ok so, just stumbled upon one issue but I'm not yet sure where the problem lies given how bad mips support is in Go:

ld: error: section type mismatch for .debug_line

/tmp/go-link-985122218/000010.o:(.debug_line): SHT_MIPS_DWARF
output section .debug_line: SHT_PROGBITS

I'm still testing though, so I'll see what I uncover...

According to https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L404 , .debug_line on MIPS should use SHT_MIPS_DWARF (different from all other targets). Go produced assembly should do the same. LLD has such a rigorous check for section type compatibility.

MaskRay added inline comments.May 30 2020, 9:49 AM
lld/test/ELF/mips-pc64.s
8

.text 0x10000 : { *(.text) }
ditto for .data

Single quotes are preferred for outermost quotes. There are sometimes needs for double quotes. Outermost single quotes avoid escaping.

Ok so, just stumbled upon one issue but I'm not yet sure where the problem lies given how bad mips support is in Go:

ld: error: section type mismatch for .debug_line

/tmp/go-link-985122218/000010.o:(.debug_line): SHT_MIPS_DWARF
output section .debug_line: SHT_PROGBITS

I'm still testing though, so I'll see what I uncover...

According to https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L404 , .debug_line on MIPS should use SHT_MIPS_DWARF (different from all other targets). Go produced assembly should do the same. LLD has such a rigorous check for section type compatibility.

Go actually produces binaries that trip LLVM up quite often - another example is llvm-strip, I should probably file another bug for that though

GNU binutils are much more relaxed when it comes to these checks, so I expect Go elf tools don't get properly tested

MaskRay added a comment.EditedMay 30 2020, 10:09 AM

Ok so, just stumbled upon one issue but I'm not yet sure where the problem lies given how bad mips support is in Go:

ld: error: section type mismatch for .debug_line

/tmp/go-link-985122218/000010.o:(.debug_line): SHT_MIPS_DWARF
output section .debug_line: SHT_PROGBITS

I'm still testing though, so I'll see what I uncover...

According to https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L404 , .debug_line on MIPS should use SHT_MIPS_DWARF (different from all other targets). Go produced assembly should do the same. LLD has such a rigorous check for section type compatibility.

Go actually produces binaries that trip LLVM up quite often - another example is llvm-strip, I should probably file another bug for that though

GNU binutils are much more relaxed when it comes to these checks, so I expect Go elf tools don't get properly tested

I'd prefer we don't touch the section compatibility code in LLD. This should be easily fixable in Go assembly emitter.

FWIW GNU as 2.35 made things more rigorous as well https://sourceware.org/pipermail/binutils/2020-February/109945.html. GNU ld is still loose (even mixing SHT_REL and SHT_RELA is ok). They might keep that behavior for compatibility reasons. In LLD, these things don't work currently and there is no need to appease old versions of them.

Ok so, just stumbled upon one issue but I'm not yet sure where the problem lies given how bad mips support is in Go:

ld: error: section type mismatch for .debug_line

/tmp/go-link-985122218/000010.o:(.debug_line): SHT_MIPS_DWARF
output section .debug_line: SHT_PROGBITS

I'm still testing though, so I'll see what I uncover...

According to https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L404 , .debug_line on MIPS should use SHT_MIPS_DWARF (different from all other targets). Go produced assembly should do the same. LLD has such a rigorous check for section type compatibility.

Go actually produces binaries that trip LLVM up quite often - another example is llvm-strip, I should probably file another bug for that though

GNU binutils are much more relaxed when it comes to these checks, so I expect Go elf tools don't get properly tested

I'd prefer we don't touch the section compatibility code in LLD. This should be easily fixable in Go assembly emitter.

FWIW GNU as 2.35 made things more rigorous as well https://sourceware.org/pipermail/binutils/2020-February/109945.html. GNU ld is still loose (even mixing SHT_REL and SHT_RELA is ok). They might keep that behavior for compatibility reasons. In LLD, these things don't work currently and there is no need to appease old versions of them.

Yeah I'm totally in agreement, I 100% blame Go for this - maybe they will pay attention if gnu starts breaking things

I just tested building and booting FreeBSD MIPS64 on QEMU MALTA with this and D80392. Everything seems to work as before.

I did not try linking with ld.bfd instead of lld, but building with clang and linking with ld.bfd is not something I need for our MIPS builds.
For CheriBSD (FreeBSD ported to work on CHERI-MIPS and CHERI-RISC-V) we need lld anyway since we add new relocations that are only supported by lld.

arichardson accepted this revision.Jun 1 2020, 4:23 AM

LGTM once all other reviewers are happy with this patch.

This revision is now accepted and ready to land.Jun 1 2020, 4:23 AM

As a side note, I fixed the Go linker and everything looks awesome now.

Thanks!

LGTM once all other reviewers are happy with this patch.

You don't really need to wait for all of us but you should probably wait for @MaskRay and @atanasyan as they had comments.

In particular, I'm unlikely to dig into the detail as I'm somewhat out of touch with the details of MIPS linking these days. I also doubt you'll get a response from Rafael as he left the LLVM community a couple years ago.

atanasyan retitled this revision from [WIP][mips] Support 64-bit relative relocations to [mips] Support 64-bit relative relocations.Jun 2 2020, 12:49 AM
atanasyan removed a reviewer: espindola.
atanasyan updated this revision to Diff 267807.Jun 2 2020, 12:50 AM
atanasyan edited the summary of this revision. (Show Details)

Thanks for review, testing and comments.

This revision was automatically updated to reflect the committed changes.