[lld-macho] Make __LINKEDIT sections contiguous

[int3]

Summary

codesign (or more specifically libstuff) checks that each section in
__LINKEDIT ends where the next one starts -- no gaps are permitted. This
diff achieves it by aligning every section's start and end points to
WordSize.

Remarks: ld64 appears to satisfy the constraint by adding padding bytes
when generating the __LINKEDIT data, e.g. by emitting BIND_OPCODE_DONE
(which is a 0x0 byte) repeatedly. I think the approach this diff takes
is a bit more elegant, but I'm not sure if it's too restrictive. In
particular, it assumes padding always uses the zero byte. But we can
revisit this later.

[compnerd]

I think that the assumption that you can use a 0 byte is dangerous (in theory, probably not practice). Please add a large comment explaining the reasoning and divergence into the code.

lld/MachO/SyntheticSections.h

public:
 
   static bool classof(const OutputSection *sec) {
     return sec->kind() == SyntheticKind;
   }
 
   const StringRef segname;
 };
 
+// All sections in __LINKEDIT should inherit from this.
+class LinkEditSection : public SyntheticSection {
+public:
+  LinkEditSection(const char *segname, const char *name)
+      : SyntheticSection(segname, name) {
+    align = WordSize;
+  }
+
+  virtual uint64_t getRawSize() const = 0;
+
+  // codesign (or more specifically libstuff) checks that each section in
+  // __LINKEDIT ends where the next one starts -- no gaps are permitted. We
+  // therefore align every section's start and end points to WordSize.
+  //
+  // NOTE: This assumes that the extra bytes required for alignment can be
+  // zero-valued bytes.
+  uint64_t getSize() const override final {
+    return llvm::alignTo(getRawSize(), WordSize);
+  }
+};
+
 // The header of the Mach-O file, which must have a file offset of zero.
 class MachHeaderSection : public SyntheticSection {
 public:
   MachHeaderSection();
   void addLoadCommand(LoadCommand *);
   bool isHidden() const override { return true; }
   uint64_t getSize() const override;
   void writeTo(uint8_t *buf) const override;
struct BindingEntry {
   uint64_t offset;
   int64_t addend;
   BindingEntry(const DylibSymbol *dysym, const InputSection *isec,
                uint64_t offset, int64_t addend)
       : dysym(dysym), isec(isec), offset(offset), addend(addend) {}
 };
 
 // Stores bind opcodes for telling dyld which symbols to load non-lazily.
-class BindingSection : public SyntheticSection {
+class BindingSection : public LinkEditSection {
 public:
   BindingSection();
   void finalizeContents();
-  uint64_t getSize() const override { return contents.size(); }
+  uint64_t getRawSize() const override { return contents.size(); }
   // Like other sections in __LINKEDIT, the binding section is special: its
   // offsets are recorded in the LC_DYLD_INFO_ONLY load command, instead of in
   // section headers.
   bool isHidden() const override { return true; }
   bool isNeeded() const override;
   void writeTo(uint8_t *buf) const override;
 
   void addEntry(const DylibSymbol *dysym, const InputSection *isec,

 class LazyPointerSection : public SyntheticSection {
 public:
   LazyPointerSection();
   uint64_t getSize() const override;
   bool isNeeded() const override;
   void writeTo(uint8_t *buf) const override;
 };
 
-class LazyBindingSection : public SyntheticSection {
+class LazyBindingSection : public LinkEditSection {
 public:
   LazyBindingSection();
   void finalizeContents();
-  uint64_t getSize() const override { return contents.size(); }
+  uint64_t getRawSize() const override { return contents.size(); }
   uint32_t encode(const DylibSymbol &);
   // Like other sections in __LINKEDIT, the lazy binding section is special: its
   // offsets are recorded in the LC_DYLD_INFO_ONLY load command, instead of in
   // section headers.
   bool isHidden() const override { return true; }
   bool isNeeded() const override;
   void writeTo(uint8_t *buf) const override;
 
 private:
   SmallVector<char, 128> contents;
   llvm::raw_svector_ostream os{contents};
 };
 
 // Stores a trie that describes the set of exported symbols.
-class ExportSection : public SyntheticSection {
+class ExportSection : public LinkEditSection {
 public:
   ExportSection();
   void finalizeContents();
-  uint64_t getSize() const override { return size; }
+  uint64_t getRawSize() const override { return size; }
   // Like other sections in __LINKEDIT, the export section is special: its
   // offsets are recorded in the LC_DYLD_INFO_ONLY load command, instead of in
   // section headers.
   bool isHidden() const override { return true; }
   void writeTo(uint8_t *buf) const override;
 
 private:
   TrieBuilder trieBuilder;
   size_t size = 0;
 };
 
 // Stores the strings referenced by the symbol table.
-class StringTableSection : public SyntheticSection {
+class StringTableSection : public LinkEditSection {
 public:
   StringTableSection();
   // Returns the start offset of the added string.
   uint32_t addString(StringRef);
-  uint64_t getSize() const override { return size; }
+  uint64_t getRawSize() const override { return size; }
   // Like other sections in __LINKEDIT, the string table section is special: its
   // offsets are recorded in the LC_SYMTAB load command, instead of in section
   // headers.
   bool isHidden() const override { return true; }
   void writeTo(uint8_t *buf) const override;
 
 private:
   // An n_strx value of 0 always indicates the empty string, so we must locate

lld/MachO/SyntheticSections.cpp

 
 void GotSection::writeTo(uint8_t *buf) const {
   for (size_t i = 0, n = entries.size(); i < n; ++i)
     if (auto *defined = dyn_cast<Defined>(entries[i]))
       write64le(&buf[i * WordSize], defined->getVA());
 }
 
 BindingSection::BindingSection()
-    : SyntheticSection(segment_names::linkEdit, section_names::binding) {}
+    : LinkEditSection(segment_names::linkEdit, section_names::binding) {}
 
 bool BindingSection::isNeeded() const {
   return bindings.size() != 0 || in.got->isNeeded();
 }
 
 namespace {
 struct Binding {
   OutputSegment *segment = nullptr;
for (const DylibSymbol *sym : in.stubs->getEntries()) {
     uint64_t stubHelperOffset = target->stubHelperHeaderSize +
                                 sym->stubsIndex * target->stubHelperEntrySize;
     write64le(buf + off, in.stubHelper->addr + stubHelperOffset);
     off += WordSize;
   }
 }
 
 LazyBindingSection::LazyBindingSection()
-    : SyntheticSection(segment_names::linkEdit, section_names::lazyBinding) {}
+    : LinkEditSection(segment_names::linkEdit, section_names::lazyBinding) {}
 
 bool LazyBindingSection::isNeeded() const { return in.stubs->isNeeded(); }
 
 void LazyBindingSection::finalizeContents() {
   // TODO: Just precompute output size here instead of writing to a temporary
   // buffer
   for (DylibSymbol *sym : in.stubs->getEntries())
     sym->lazyBindOffset = encode(*sym);
uint32_t LazyBindingSection::encode(const DylibSymbol &sym) {
   os << static_cast<uint8_t>(MachO::BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM)
      << sym.getName() << '\0'
      << static_cast<uint8_t>(MachO::BIND_OPCODE_DO_BIND)
      << static_cast<uint8_t>(MachO::BIND_OPCODE_DONE);
   return opstreamOffset;
 }
 
 ExportSection::ExportSection()
-    : SyntheticSection(segment_names::linkEdit, section_names::export_) {}
+    : LinkEditSection(segment_names::linkEdit, section_names::export_) {}
 
 void ExportSection::finalizeContents() {
   // TODO: We should check symbol visibility.
   for (const Symbol *sym : symtab->getSymbols())
     if (auto *defined = dyn_cast<Defined>(sym))
       trieBuilder.addSymbol(*defined);
   size = trieBuilder.build();
 }
 
 void ExportSection::writeTo(uint8_t *buf) const { trieBuilder.writeTo(buf); }
 
 SymtabSection::SymtabSection(StringTableSection &stringTableSection)
     : SyntheticSection(segment_names::linkEdit, section_names::symbolTable),
-      stringTableSection(stringTableSection) {
-  // TODO: When we introduce the SyntheticSections superclass, we should make
-  // all synthetic sections aligned to WordSize by default.
-  align = WordSize;
-}
+      stringTableSection(stringTableSection) {}
 
 uint64_t SymtabSection::getSize() const {
   return symbols.size() * sizeof(structs::nlist_64);
 }
 
 void SymtabSection::finalizeContents() {
   // TODO support other symbol types
   for (Symbol *sym : symtab->getSymbols())
if (auto *defined = dyn_cast<Defined>(entry.sym)) {
       // For the N_SECT symbol type, n_value is the address of the symbol
       nList->n_value = defined->value + defined->isec->getVA();
     }
     ++nList;
   }
 }
 
 StringTableSection::StringTableSection()
-    : SyntheticSection(segment_names::linkEdit, section_names::stringTable) {}
+    : LinkEditSection(segment_names::linkEdit, section_names::stringTable) {}
 
 uint32_t StringTableSection::addString(StringRef str) {
   uint32_t strx = size;
   strings.push_back(str);
   size += str.size() + 1; // account for null terminator
   return strx;
 }
 
 void StringTableSection::writeTo(uint8_t *buf) const {
   uint32_t off = 0;
   for (StringRef str : strings) {
     memcpy(buf + off, str.data(), str.size());
     off += str.size() + 1; // account for null terminator
   }
 }

lld/MachO/Writer.cpp

 static int sectionOrder(OutputSection *osec) {
   StringRef segname = osec->parent->name;
   // Sections are uniquely identified by their segment + section name.
   if (segname == segment_names::text) {
     if (osec->name == section_names::header)
       return -1;
   } else if (segname == segment_names::linkEdit) {
     return StringSwitch<int>(osec->name)
-        .Case(section_names::binding, -4)
+        .Case(section_names::binding, -5)
+        .Case(section_names::lazyBinding, -4)
         .Case(section_names::export_, -3)
         .Case(section_names::symbolTable, -2)
         .Case(section_names::stringTable, -1)
         .Default(0);
   }
   // ZeroFill sections must always be the at the end of their segments,
   // otherwise subsequent sections may get overwritten with zeroes at runtime.
   if (isZeroFill(osec->flags))

lld/test/MachO/linkedit-contiguity.s

+# REQUIRES: x86
+# RUN: mkdir -p %t
+
+## codesign requires that each setion in __LINKEDIT ends where the next one
+## starts. This test enforces that invariant.
+## TODO: Test other __LINKEDIT sections here as support for them gets added.
+## Examples of such sections include the data for LC_CODE_SIGNATURE and
+## LC_DATA_IN_CODE.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %p/Inputs/libhello.s \
+# RUN:   -o %t/libhello.o
+# RUN: lld -flavor darwinnew -dylib -L%S/Inputs/MacOSX.sdk/usr/lib \
+# RUN:   -install_name @executable_path/libhello.dylib %t/libhello.o \
+# RUN:   -o %t/libhello.dylib
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t/test.o
+# RUN: lld -flavor darwinnew -o %t/test \
+# RUN:   -L%S/Inputs/MacOSX.sdk/usr/lib -L%t -lhello %t/test.o -lSystem
+
+# RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s
+
+# CHECK:      cmd LC_DYLD_INFO_ONLY
+# CHECK-NEXT: cmdsize 48
+# CHECK-NEXT: rebase_off 0
+# CHECK-NEXT: rebase_size 0
+# CHECK-NEXT: bind_off [[#BIND_OFF:]]
+# CHECK-NEXT: bind_size [[#BIND_SIZE:]]
+# CHECK-NEXT: weak_bind_off 0
+# CHECK-NEXT: weak_bind_size 0
+# CHECK-NEXT: lazy_bind_off [[#LAZY_OFF: BIND_OFF + BIND_SIZE]]
+# CHECK-NEXT: lazy_bind_size [[#LAZY_SIZE:]]
+# CHECK-NEXT: export_off [[#EXPORT_OFF: LAZY_OFF + LAZY_SIZE]]
+# CHECK-NEXT: export_size [[#]]
+
+.text
+.globl _main
+_main:
+  sub $8, %rsp # 16-byte-align the stack; dyld checks for this
+  callq _print_hello
+  add $8, %rsp
+  ret