[PATCH] D79311: [lld-macho] Support X86_64_RELOC_SIGNED_{1,2,4}

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

I think it makes a difference for non-extern relocations.

Our relocation computation for pcrel relocations (the only type we support right now) is currently `VA of target address + implicit addend - VA of relocation`. We want the end result to be `VA of target address + implicit addend - address of next instruction`, since PC-relative relocations on x86-64 are relative to the start of the next instruction, and we accomplish this by subtracting 4 in `X86_64::relocateOne`. That's assumes the relocation is at the end of the instruction though, which isn't correct for X86_64_RELOC_SIGNED_{1,2,4}.

We're getting away with this right now because for extern relocations, the implicit addend negates this. E.g. for an `X86_64_RELOC_SIGNED_1` (where there's 1 byte after the relocation), our computed offset would normally have been too high by 1 (since we'd be computing `target VA - (address of next instruction - 1)`), but the implicit addend is `-1`, so this works out. For non-extern relocations though, the implicit addend doesn't appear to have this adjustment, so we'll need to take the relocation type into account there.

Given that we only support extern relocations right now, this LGTM. @int3, are you good with this going in first and adjusting D79211 <https://reviews.llvm.org/D79211> after?

An interesting aside: if you change the `_s+2` to `_s+1`, the object file just contains a regular `X86_64_RELOC_SIGNED`. I wonder if that can cause any issues.



================
Comment at: lld/test/MachO/x86-64-reloc-signed.s:17
+_main:
+  movl $0x434241, _s(%rip)  # X86_64_RELOC_SIGNED4
+  callq _f
----------------
int3 wrote:
> nit: X86_64_RELOC_SIGNED_4 (missing underscore)
(same nit for the two below)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79311/new/

https://reviews.llvm.org/D79311