[lld-macho][nfc] Eliminate InputSection::Shared

Earlier in LLD's evolution, I tried to create the illusion that
subsections were indistinguishable from "top-level" sections. Thus, even
though the subsections shared many common field values, I hid those
common values away in a private Shared struct (see D105305). More
recently, however, @gkm added a public `Section` struct in D113241 that
served as an explicit way to store values that are common to an entire
set of subsections (aka InputSections). Now that we have another "common
value" struct, `Shared` has been rendered redundant. All its fields can
be moved into `Section` instead, and the pointer to `Shared` can be replaced
with a pointer to `Section`.

This `Section` pointer also has the advantage of letting us inspect other
subsections easily, simplifying the implementation of {D118798}.

P.S. I do think that having both `Section` and `InputSection` makes for
a slightly confusing naming scheme. I considered renaming `InputSection`
to `Subsection`, but that would break the symmetry with `OutputSection`.
It would also make us deviate from LLD-ELF's naming scheme.

This change is perf-neutral on my 3.2 GHz 16-Core Intel Xeon W machine:

             base           diff           difference (95% CI)
  sys_time   1.258 ± 0.031  1.248 ± 0.023  [  -1.6% ..   +0.1%]
  user_time  3.659 ± 0.047  3.658 ± 0.041  [  -0.5% ..   +0.4%]
  wall_time  4.640 ± 0.085  4.625 ± 0.063  [  -1.0% ..   +0.3%]
  samples    49             61

There's also no stat sig change in RSS (as measured by `time -l`):

           base                         diff                           difference (95% CI)
  time     998038627.097 ± 13567305.958 1003327715.556 ± 15210451.236  [  -0.2% ..   +1.2%]
  samples  31                           36

Reviewed By: #lld-macho, oontvoo

Differential Revision: https://reviews.llvm.org/D118797

Author: int3

lld/MachO/ConcatOutputSection.cpp

@@ -313,7 +313,7 @@ void ConcatOutputSection::finalize() {
         fatal(Twine(__FUNCTION__) + ": FIXME: thunk range overrun");
       }
       thunkInfo.isec =
-          make<ConcatInputSection>(isec->getSegName(), isec->getName());
+          makeSyntheticInputSection(isec->getSegName(), isec->getName());
       thunkInfo.isec->parent = this;
 
       // This code runs after dead code removal. Need to set the `live` bit

lld/MachO/Driver.cpp

@@ -540,17 +540,19 @@ static void replaceCommonSymbols() {
     // but it's not really worth supporting the linking of 64-bit programs on
     // 32-bit archs.
     ArrayRef<uint8_t> data = {nullptr, static_cast<size_t>(common->size)};
-    auto *isec = make<ConcatInputSection>(
-        segment_names::data, section_names::common, common->getFile(), data,
-        common->align, S_ZEROFILL);
+    // FIXME avoid creating one Section per symbol?
+    auto *section =
+        make<Section>(common->getFile(), segment_names::data,
+                      section_names::common, S_ZEROFILL, /*addr=*/0);
+    auto *isec = make<ConcatInputSection>(*section, data, common->align);
     if (!osec)
       osec = ConcatOutputSection::getOrCreateForInput(isec);
     isec->parent = osec;
     inputSections.push_back(isec);
 
     // FIXME: CommonSymbol should store isReferencedDynamically, noDeadStrip
     // and pass them on here.
-    replaceSymbol<Defined>(sym, sym->getName(), isec->getFile(), isec,
+    replaceSymbol<Defined>(sym, sym->getName(), common->getFile(), isec,
                            /*value=*/0,
                            /*size=*/0,
                            /*isWeakDef=*/false,
@@ -1005,8 +1007,8 @@ static void gatherInputSections() {
   TimeTraceScope timeScope("Gathering input sections");
   int inputOrder = 0;
   for (const InputFile *file : inputFiles) {
-    for (const Section &section : file->sections) {
-      const Subsections &subsections = section.subsections;
+    for (const Section *section : file->sections) {
+      const Subsections &subsections = section->subsections;
       if (subsections.empty())
         continue;
       if (subsections[0].isec->getName() == section_names::compactUnwind)

lld/MachO/InputFiles.cpp

@@ -277,28 +277,24 @@ void ObjFile::parseSections(ArrayRef<SectionHeader> sectionHeaders) {
     ArrayRef<uint8_t> data = {isZeroFill(sec.flags) ? nullptr
                                                     : buf + sec.offset,
                               static_cast<size_t>(sec.size)};
-    sections.push_back(sec.addr);
+    sections.push_back(make<Section>(this, segname, name, sec.flags, sec.addr));
     if (sec.align >= 32) {
       error("alignment " + std::to_string(sec.align) + " of section " + name +
             " is too large");
       continue;
     }
+    const Section &section = *sections.back();
     uint32_t align = 1 << sec.align;
-    uint32_t flags = sec.flags;
 
     auto splitRecords = [&](int recordSize) -> void {
       if (data.empty())
         return;
-      Subsections &subsections = sections.back().subsections;
+      Subsections &subsections = sections.back()->subsections;
       subsections.reserve(data.size() / recordSize);
-      auto *isec = make<ConcatInputSection>(
-          segname, name, this, data.slice(0, recordSize), align, flags);
-      subsections.push_back({0, isec});
-      for (uint64_t off = recordSize; off < data.size(); off += recordSize) {
-        // Copying requires less memory than constructing a fresh InputSection.
-        auto *copy = make<ConcatInputSection>(*isec);
-        copy->data = data.slice(off, recordSize);
-        subsections.push_back({off, copy});
+      for (uint64_t off = 0; off < data.size(); off += recordSize) {
+        auto *isec = make<ConcatInputSection>(
+            section, data.slice(off, recordSize), align);
+        subsections.push_back({off, isec});
       }
     };
 
@@ -312,19 +308,17 @@ void ObjFile::parseSections(ArrayRef<SectionHeader> sectionHeaders) {
 
       InputSection *isec;
       if (sectionType(sec.flags) == S_CSTRING_LITERALS) {
-        isec =
-            make<CStringInputSection>(segname, name, this, data, align, flags);
+        isec = make<CStringInputSection>(section, data, align);
         // FIXME: parallelize this?
         cast<CStringInputSection>(isec)->splitIntoPieces();
       } else {
-        isec = make<WordLiteralInputSection>(segname, name, this, data, align,
-                                             flags);
+        isec = make<WordLiteralInputSection>(section, data, align);
       }
-      sections.back().subsections.push_back({0, isec});
+      sections.back()->subsections.push_back({0, isec});
     } else if (auto recordSize = getRecordSize(segname, name)) {
       splitRecords(*recordSize);
       if (name == section_names::compactUnwind)
-        compactUnwindSection = &sections.back();
+        compactUnwindSection = sections.back();
     } else if (segname == segment_names::llvm) {
       if (name == "__cg_profile" && config->callGraphProfileSort) {
         TimeTraceScope timeScope("Parsing call graph section");
@@ -352,16 +346,15 @@ void ObjFile::parseSections(ArrayRef<SectionHeader> sectionHeaders) {
       // spurious duplicate symbol errors, we do not parse these sections.
       // TODO: Evaluate whether the bitcode metadata is needed.
     } else {
-      auto *isec =
-          make<ConcatInputSection>(segname, name, this, data, align, flags);
+      auto *isec = make<ConcatInputSection>(section, data, align);
       if (isDebugSection(isec->getFlags()) &&
           isec->getSegName() == segment_names::dwarf) {
         // Instead of emitting DWARF sections, we emit STABS symbols to the
         // object files that contain them. We filter them out early to avoid
         // parsing their relocations unnecessarily.
         debugSections.push_back(isec);
       } else {
-        sections.back().subsections.push_back({0, isec});
+        sections.back()->subsections.push_back({0, isec});
       }
     }
   }
@@ -506,7 +499,7 @@ void ObjFile::parseRelocations(ArrayRef<SectionHeader> sectionHeaders,
         referentOffset = totalAddend - referentSecHead.addr;
       }
       Subsections &referentSubsections =
-          sections[relInfo.r_symbolnum - 1].subsections;
+          sections[relInfo.r_symbolnum - 1]->subsections;
       r.referent =
           findContainingSubsection(referentSubsections, &referentOffset);
       r.addend = referentOffset;
@@ -547,7 +540,7 @@ void ObjFile::parseRelocations(ArrayRef<SectionHeader> sectionHeaders,
         uint64_t referentOffset =
             totalAddend - sectionHeaders[minuendInfo.r_symbolnum - 1].addr;
         Subsections &referentSubsectVec =
-            sections[minuendInfo.r_symbolnum - 1].subsections;
+            sections[minuendInfo.r_symbolnum - 1]->subsections;
         p.referent =
             findContainingSubsection(referentSubsectVec, &referentOffset);
         p.addend = referentOffset;
@@ -699,7 +692,7 @@ void ObjFile::parseSymbols(ArrayRef<typename LP::section> sectionHeaders,
 
     StringRef name = strtab + sym.n_strx;
     if ((sym.n_type & N_TYPE) == N_SECT) {
-      Subsections &subsections = sections[sym.n_sect - 1].subsections;
+      Subsections &subsections = sections[sym.n_sect - 1]->subsections;
       // parseSections() may have chosen not to parse this section.
       if (subsections.empty())
         continue;
@@ -712,7 +705,7 @@ void ObjFile::parseSymbols(ArrayRef<typename LP::section> sectionHeaders,
   }
 
   for (size_t i = 0; i < sections.size(); ++i) {
-    Subsections &subsections = sections[i].subsections;
+    Subsections &subsections = sections[i]->subsections;
     if (subsections.empty())
       continue;
     InputSection *lastIsec = subsections.back().isec;
@@ -823,12 +816,13 @@ OpaqueFile::OpaqueFile(MemoryBufferRef mb, StringRef segName,
     : InputFile(OpaqueKind, mb) {
   const auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
   ArrayRef<uint8_t> data = {buf, mb.getBufferSize()};
-  ConcatInputSection *isec =
-      make<ConcatInputSection>(segName.take_front(16), sectName.take_front(16),
-                               /*file=*/this, data);
+  sections.push_back(make<Section>(/*file=*/this, segName.take_front(16),
+                                   sectName.take_front(16),
+                                   /*flags=*/0, /*addr=*/0));
+  Section &section = *sections.back();
+  ConcatInputSection *isec = make<ConcatInputSection>(section, data);
   isec->live = true;
-  sections.push_back(0);
-  sections.back().subsections.push_back({0, isec});
+  section.subsections.push_back({0, isec});
 }
 
 ObjFile::ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName,
@@ -898,9 +892,9 @@ template <class LP> void ObjFile::parse() {
   // The relocations may refer to the symbols, so we parse them after we have
   // parsed all the symbols.
   for (size_t i = 0, n = sections.size(); i < n; ++i)
-    if (!sections[i].subsections.empty())
+    if (!sections[i]->subsections.empty())
       parseRelocations(sectionHeaders, sectionHeaders[i],
-                       sections[i].subsections);
+                       sections[i]->subsections);
 
   parseDebugInfo();
   if (compactUnwindSection)

lld/MachO/InputFiles.h

@@ -58,11 +58,24 @@ struct Subsection {
 };
 
 using Subsections = std::vector<Subsection>;
+class InputFile;
 
 struct Section {
-  uint64_t address = 0;
+  InputFile *file;
+  StringRef segname;
+  StringRef name;
+  uint32_t flags;
+  uint64_t addr;
   Subsections subsections;
-  Section(uint64_t addr) : address(addr){};
+
+  Section(InputFile *file, StringRef segname, StringRef name, uint32_t flags,
+          uint64_t addr)
+      : file(file), segname(segname), name(name), flags(flags), addr(addr) {}
+  // Ensure pointers to Sections are never invalidated.
+  Section(const Section &) = delete;
+  Section &operator=(const Section &) = delete;
+  Section(Section &&) = delete;
+  Section &operator=(Section &&) = delete;
 };
 
 // Represents a call graph profile edge.
@@ -93,7 +106,7 @@ class InputFile {
   MemoryBufferRef mb;
 
   std::vector<Symbol *> symbols;
-  std::vector<Section> sections;
+  std::vector<Section *> sections;
 
   // If not empty, this stores the name of the archive containing this file.
   // We use this string for creating error messages.

lld/MachO/InputSection.cpp

@@ -177,6 +177,18 @@ void ConcatInputSection::writeTo(uint8_t *buf) {
   }
 }
 
+ConcatInputSection *macho::makeSyntheticInputSection(StringRef segName,
+                                                     StringRef sectName,
+                                                     uint32_t flags,
+                                                     ArrayRef<uint8_t> data,
+                                                     uint32_t align) {
+  Section &section =
+      *make<Section>(/*file=*/nullptr, segName, sectName, flags, /*addr=*/0);
+  auto isec = make<ConcatInputSection>(section, data, align);
+  section.subsections.push_back({0, isec});
+  return isec;
+}
+
 void CStringInputSection::splitIntoPieces() {
   size_t off = 0;
   StringRef s = toStringRef(data);
@@ -211,13 +223,11 @@ uint64_t CStringInputSection::getOffset(uint64_t off) const {
   return piece.outSecOff + addend;
 }
 
-WordLiteralInputSection::WordLiteralInputSection(StringRef segname,
-                                                 StringRef name,
-                                                 InputFile *file,
+WordLiteralInputSection::WordLiteralInputSection(const Section &section,
                                                  ArrayRef<uint8_t> data,
-                                                 uint32_t align, uint32_t flags)
-    : InputSection(WordLiteralKind, segname, name, file, data, align, flags) {
-  switch (sectionType(flags)) {
+                                                 uint32_t align)
+    : InputSection(WordLiteralKind, section, data, align) {
+  switch (sectionType(getFlags())) {
   case S_4BYTE_LITERALS:
     power2LiteralSize = 2;
     break;