[PATCH] D76839: [lld-macho] Extend SyntheticSections to cover all segment load commands

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputSection.cpp:24
 
+uint32_t InputSection::getFileOffset() const {
+  return parent->fileOff + addr - parent->firstSection()->addr;
----------------
ruiu wrote:
> It is safer to use uint64_t to represent file offsets. At least on Linux, we actually create binaries that are larger than 4 GiB.
Sections are limited to 32-bit file offsets even in 64-bit MachO binaries. Even hidden sections addressed by e.g. LC_DYLD_LOAD_INFO must be 32-bit. Segments can have 64-bit file offsets though... so I've changed fileOff in OutputSegment but kept this 32-bit.


================
Comment at: lld/MachO/SyntheticSections.h:23-24
+
+constexpr const char *binding = "__binding";
+constexpr const char *header = "__mach_header";
+constexpr const char *pageZero = "__pagezero";
----------------
ruiu wrote:
> int3 wrote:
> > The names of hidden sections don't make it to the final executable; they're just for distinguishing the InputSections within lld code and so can be any unique string. But I've picked the same names as the corresponding Atoms in ld64 for easy reference.
> Actually this kind of one-shot temporary can be inlined, and in particular for these names, we don't have to have then as global constants because we are not really referring them.
they're referenced in `sectionOrder()`, so they're not exactly one-shot


================
Comment at: lld/MachO/SyntheticSections.h:76
 
+// Stores bind opcodes, which tell dyld which dylib symbols to load non-lazily.
+class BindingSection : public InputSection {
----------------
ruiu wrote:
> Can you expand the comment a bit? I'd explain what the bind opcode in more details.
hm, I think the explanation of bind opcodes would make more sense in `encode()`... I'll elaborate there


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76839