[lld-macho][rfc] Have Symbol::getVA() return a non-relative virtual address

[int3]

Summary

Currently, getVA() returns a virtual address with the assumption that
the ImageBase is zero. As I understand, this is what lld-ELF is doing.
However, under our current design, it seems like an awkward setup --
I'm finding that I have to add and subtract ImageBase in several places
to make things work out.

As such, I think it's simpler to have getVA() return a non-relative VA,
but I'm not sure if I'm missing something. Would love to hear more from
folks familiar with lld-ELF.

[smeenai]

I'm in favor of this, since it makes the VA actually be a VA and not an RVA :)

[int3]

subtract ImageBase for entryoff

[int3]

rebase

[smeenai]

This one needs to be properly stacked, I think.

[int3]

rebase

[int3]

rebase

lld/MachO/ExportTrie.cpp

if (lastPos != pos && (isTerminal || prefixesDiverge)) {
     lastPos = pos;
   }
 
   sortAndBuild(vec.slice(0, i), lastPos, pos, node);
   sortAndBuild(vec.slice(j), lastPos, pos, node);
 
   if (isTerminal) {
     assert(j - i == 1); // no duplicate symbols
-    node->info = {pivotSymbol->getVA() + ImageBase};
+    node->info = {pivotSymbol->getVA()};
   } else {
     // This is the tail-call-optimized version of the following:
     // multikeySort(vec.slice(i, j - i), lastPos, pos + 1, node);
     vec = vec.slice(i, j - i);
     ++pos;
     goto tailcall;
   }
 }

lld/MachO/InputSection.cpp

 
 void InputSection::writeTo(uint8_t *buf) {
   memcpy(buf, data.data(), data.size());
 
   for (Reloc &r : relocs) {
     uint64_t va = 0;
     if (auto *s = r.target.dyn_cast<Symbol *>()) {
       if (auto *dylibSymbol = dyn_cast<DylibSymbol>(s)) {
-        va = in.got->addr - ImageBase + dylibSymbol->gotIndex * WordSize;
+        va = in.got->addr + dylibSymbol->gotIndex * WordSize;
       } else {
         va = s->getVA();
       }
     } else if (auto *isec = r.target.dyn_cast<InputSection *>()) {
       va = isec->addr;
     } else {
       llvm_unreachable("Unknown relocation target");
     }
 
     uint64_t val = va + r.addend;
     if (1) // TODO: handle non-pcrel relocations
-      val -= addr - ImageBase + r.offset;
+      val -= addr + r.offset;
     target->relocateOne(buf + r.offset, r.type, val);
   }
 }

lld/MachO/Symbols.h

public:
   static bool classof(const Symbol *s) { return s->kind() == DylibKind; }
 
   DylibFile *file;
   uint32_t gotIndex = UINT32_MAX;
 };
 
 inline uint64_t Symbol::getVA() const {
   if (auto *d = dyn_cast<Defined>(this))
-    return d->isec->addr + d->value - ImageBase;
+    return d->isec->addr + d->value;
   return 0;
 }
 
 union SymbolUnion {
   alignas(Defined) char a[sizeof(Defined)];
   alignas(Undefined) char b[sizeof(Undefined)];
   alignas(DylibSymbol) char c[sizeof(DylibSymbol)];
 };

lld/MachO/Writer.cpp

 
 class LCMain : public LoadCommand {
   uint32_t getSize() const override { return sizeof(entry_point_command); }
 
   void writeTo(uint8_t *buf) const override {
     auto *c = reinterpret_cast<entry_point_command *>(buf);
     c->cmd = LC_MAIN;
     c->cmdsize = getSize();
-    c->entryoff = config->entry->getVA();
+    c->entryoff = config->entry->getVA() - ImageBase;
     c->stacksize = 0;
   }
 };
 
 class LCSymtab : public LoadCommand {
 public:
   LCSymtab(SymtabSection *symtabSection, StringTableSection *stringTableSection)
       : symtabSection(symtabSection), stringTableSection(stringTableSection) {}