[PATCH] D106128: [lld-macho] Use immediate encodings for bind opcodes

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.

Thanks!



================
Comment at: lld/MachO/SyntheticSections.cpp:370
+    if ((p->opcode == BIND_OPCODE_DO_BIND_ADD_ADDR_ULEB) &&
+        (p->data < (BIND_IMMEDIATE_MASK * offsetWidth)) &&
+        ((p->data % offsetWidth) == 0)) {
----------------
thevinster wrote:
> int3 wrote:
> > how about `p->data / offsetWidth <= BIND_IMMEDIATE_MASK`, to mirror the assignment below?
> I changed it to `p->data / target->wordSize < BIND_IMMEDIATE_MASK`. I removed the equals comparison because when dyld uncompacts, it seems to add an extra `sizeof(intptr_t)`. 
> 
> See https://opensource.apple.com/source/dyld/dyld-852/src/ImageLoaderMachOCompressed.cpp.auto.html and search for `address += immediate*sizeof(intptr_t) + sizeof(intptr_t);`
I think the previous implementation was the right one. The DO_BIND opcode itself adds a `sizeof(intptr_t)`, which is probably why dyld is doing that.

>From http://www.m4b.io/reverse/engineering/mach/binaries/2015/03/29/mach-binaries.html 's description of DO_BIND:

> Push the current record onto the "import stack", and then increment the current record's address offset by the size of the platform pointer (32 or 64 bit)

It would be good to have a test case that covers this edge case :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106128