[PATCH] D87199: [lld-macho] Implement support for PIC

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

> Is there a funnel point that we know that all the rebase entries will pass through to ensure that we catch them rather than adding them at a couple of sites?

It may be clearer if we scan the GOT/LazyPointerSection inside `RebaseSection::finalizeContents()` for any absolute addresses, rather than relying on the code that's adding entries to either of those sections to also handle the rebasing. I will try that... however, there isn't a funnel point that will cover absolute addresses from both our SyntheticSections as well as from InputSections.



================
Comment at: lld/MachO/SyntheticSections.cpp:159
+
+  if (!locations.empty())
+    os << static_cast<uint8_t>(REBASE_OPCODE_SET_TYPE_IMM |
----------------
I should just return early instead of checking `locations.empty()` twice


================
Comment at: lld/MachO/SyntheticSections.h:187
+  Location target;
+  BindingEntry(const DylibSymbol *dysym, int64_t addend, Location target)
+      : dysym(dysym), addend(addend), target(std::move(target)) {}
----------------
compnerd wrote:
> What do you think of moving `addend` to after `target`?
What's the rationale behind your preference? I thought putting the addend next to the symbol made sense since they both determine what address gets written into the target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87199