[lld] 7f2ba39 - [lld-macho][nfc] Move liveness-tracking fields into ConcatInputSection

Jez Ng via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 11 16:50:38 PDT 2021


Author: Jez Ng
Date: 2021-06-11T19:50:08-04:00
New Revision: 7f2ba39b1688230c663986485f605cf8df90f895

URL: https://github.com/llvm/llvm-project/commit/7f2ba39b1688230c663986485f605cf8df90f895
DIFF: https://github.com/llvm/llvm-project/commit/7f2ba39b1688230c663986485f605cf8df90f895.diff

LOG: [lld-macho][nfc] Move liveness-tracking fields into ConcatInputSection

These fields currently live in the parent InputSection class,
but they should be specific to ConcatInputSection, since the other
InputSection classes (that contain literals) aren't atomically live or
dead -- rather their component string/int literals should have
individual liveness states. (An upcoming diff will add liveness bits for
StringPieces and fixed-sized literals.)

I also factored out some asserts for isCoalescedWeak() in MarkLive.cpp.
We now avoid putting coalesced sections in the `inputSections` vector,
so we don't have to check/assert against it everywhere.

Reviewed By: #lld-macho, thakis

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

Added: 
    

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/InputSection.h
    lld/MachO/MarkLive.cpp
    lld/MachO/SymbolTable.cpp
    lld/MachO/Symbols.cpp
    lld/MachO/Symbols.h
    lld/MachO/UnwindInfoSection.cpp
    lld/MachO/UnwindInfoSection.h
    lld/MachO/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 17bb81d0ca62..23c297b7cfe7 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1293,9 +1293,14 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
       TimeTraceScope timeScope("Gathering input sections");
       // Gather all InputSections into one vector.
       for (const InputFile *file : inputFiles) {
-        for (const SubsectionMap &map : file->subsections)
-          for (const SubsectionEntry &subsectionEntry : map)
-            inputSections.push_back(subsectionEntry.isec);
+        for (const SubsectionMap &map : file->subsections) {
+          for (const SubsectionEntry &entry : map) {
+            if (auto concatIsec = dyn_cast<ConcatInputSection>(entry.isec))
+              if (concatIsec->isCoalescedWeak())
+                continue;
+            inputSections.push_back(entry.isec);
+          }
+        }
       }
     }
 

diff  --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 539c60a97f4b..bd9a2323a1fa 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -41,6 +41,8 @@ class InputSection {
   // The offset from the beginning of the file.
   virtual uint64_t getFileOffset(uint64_t off) const = 0;
   uint64_t getVA(uint64_t off) const;
+  // Whether the data at \p off in this InputSection is live.
+  virtual bool isLive(uint64_t off) const = 0;
 
   void writeTo(uint8_t *buf);
 
@@ -55,20 +57,6 @@ class InputSection {
   uint32_t callSiteCount = 0;
   bool isFinal = false; // is address assigned?
 
-  // How many symbols refer to this InputSection.
-  uint32_t numRefs = 0;
-
-  // With subsections_via_symbols, most symbols have their own InputSection,
-  // and for weak symbols (e.g. from inline functions), only the
-  // InputSection from one translation unit will make it to the output,
-  // while all copies in other translation units are coalesced into the
-  // first and not copied to the output.
-  bool wasCoalesced = false;
-
-  bool isCoalescedWeak() const { return wasCoalesced && numRefs == 0; }
-  bool shouldOmitFromOutput() const { return !live || isCoalescedWeak(); }
-
-  bool live = !config->deadStrip;
 
   ArrayRef<uint8_t> data;
   std::vector<Reloc> relocs;
@@ -89,11 +77,24 @@ class ConcatInputSection : public InputSection {
   uint64_t getFileOffset(uint64_t off) const override;
   uint64_t getOffset(uint64_t off) const override { return outSecOff + off; }
   uint64_t getVA() const { return InputSection::getVA(0); }
+  // ConcatInputSections are entirely live or dead, so the offset is irrelevant.
+  bool isLive(uint64_t off) const override { return live; }
+  bool isCoalescedWeak() const { return wasCoalesced && numRefs == 0; }
+  bool shouldOmitFromOutput() const { return !live || isCoalescedWeak(); }
 
   static bool classof(const InputSection *isec) {
     return isec->kind() == ConcatKind;
   }
 
+  // With subsections_via_symbols, most symbols have their own InputSection,
+  // and for weak symbols (e.g. from inline functions), only the
+  // InputSection from one translation unit will make it to the output,
+  // while all copies in other translation units are coalesced into the
+  // first and not copied to the output.
+  bool wasCoalesced = false;
+  bool live = !config->deadStrip;
+  // How many symbols refer to this InputSection.
+  uint32_t numRefs = 0;
   uint64_t outSecOff = 0;
   uint64_t outSecFileOff = 0;
 };
@@ -125,6 +126,8 @@ class CStringInputSection : public InputSection {
   CStringInputSection() : InputSection(CStringLiteralKind) {}
   uint64_t getFileOffset(uint64_t off) const override;
   uint64_t getOffset(uint64_t off) const override;
+  // FIXME implement this
+  bool isLive(uint64_t off) const override { return true; }
   // Find the StringPiece that contains this offset.
   const StringPiece &getStringPiece(uint64_t off) const;
   // Split at each null byte.
@@ -152,6 +155,8 @@ class WordLiteralInputSection : public InputSection {
   WordLiteralInputSection() : InputSection(WordLiteralKind) {}
   uint64_t getFileOffset(uint64_t off) const override;
   uint64_t getOffset(uint64_t off) const override;
+  // FIXME implement this
+  bool isLive(uint64_t off) const override { return true; }
 
   static bool classof(const InputSection *isec) {
     return isec->kind() == WordLiteralKind;

diff  --git a/lld/MachO/MarkLive.cpp b/lld/MachO/MarkLive.cpp
index b42590666897..73bd215e75f7 100644
--- a/lld/MachO/MarkLive.cpp
+++ b/lld/MachO/MarkLive.cpp
@@ -30,13 +30,18 @@ void markLive() {
   // We build up a worklist of sections which have been marked as live. We only
   // push into the worklist when we discover an unmarked section, and we mark
   // as we push, so sections never appear twice in the list.
-  SmallVector<InputSection *, 256> worklist;
-
-  auto enqueue = [&](InputSection *s) {
-    if (s->live)
-      return;
-    s->live = true;
-    worklist.push_back(s);
+  // Literal sections cannot contain references to other sections, so we only
+  // store ConcatInputSections in our worklist.
+  SmallVector<ConcatInputSection *, 256> worklist;
+
+  auto enqueue = [&](InputSection *isec) {
+    if (auto s = dyn_cast<ConcatInputSection>(isec)) {
+      assert(!s->isCoalescedWeak());
+      if (s->live)
+        return;
+      s->live = true;
+      worklist.push_back(s);
+    }
   };
 
   auto addSym = [&](Symbol *s) {
@@ -119,7 +124,8 @@ void markLive() {
     // See also scanEhFrameSection() in lld/ELF/MarkLive.cpp.
     if (isec->segname == segment_names::ld &&
         isec->name == section_names::compactUnwind) {
-      isec->live = true;
+      auto concatIsec = cast<ConcatInputSection>(isec);
+      concatIsec->live = true;
       const int compactUnwindEntrySize =
           target->wordSize == 8 ? sizeof(CompactUnwindEntry<uint64_t>)
                                 : sizeof(CompactUnwindEntry<uint32_t>);
@@ -131,11 +137,8 @@ void markLive() {
 
         if (auto *s = r.referent.dyn_cast<Symbol *>())
           addSym(s);
-        else {
-          auto *referentIsec = r.referent.get<InputSection *>();
-          assert(!referentIsec->isCoalescedWeak());
-          enqueue(referentIsec);
-        }
+        else
+          enqueue(r.referent.get<InputSection *>());
       }
       continue;
     }
@@ -144,27 +147,27 @@ void markLive() {
   do {
     // Mark things reachable from GC roots as live.
     while (!worklist.empty()) {
-      InputSection *s = worklist.pop_back_val();
+      ConcatInputSection *s = worklist.pop_back_val();
       assert(s->live && "We mark as live when pushing onto the worklist!");
 
       // Mark all symbols listed in the relocation table for this section.
       for (const Reloc &r : s->relocs) {
-        if (auto *s = r.referent.dyn_cast<Symbol *>()) {
+        if (auto *s = r.referent.dyn_cast<Symbol *>())
           addSym(s);
-        } else {
-          auto *referentIsec = r.referent.get<InputSection *>();
-          assert(!referentIsec->isCoalescedWeak());
-          enqueue(referentIsec);
-        }
+        else
+          enqueue(r.referent.get<InputSection *>());
       }
     }
 
     // S_ATTR_LIVE_SUPPORT sections are live if they point _to_ a live section.
     // Process them in a second pass.
     for (InputSection *isec : inputSections) {
+      if (!isa<ConcatInputSection>(isec))
+        continue;
+      auto concatIsec = cast<ConcatInputSection>(isec);
       // FIXME: Check if copying all S_ATTR_LIVE_SUPPORT sections into a
       // separate vector and only walking that here is faster.
-      if (!(isec->flags & S_ATTR_LIVE_SUPPORT) || isec->live)
+      if (!(concatIsec->flags & S_ATTR_LIVE_SUPPORT) || concatIsec->live)
         continue;
 
       for (const Reloc &r : isec->relocs) {
@@ -172,7 +175,7 @@ void markLive() {
         if (auto *s = r.referent.dyn_cast<Symbol *>())
           referentLive = s->isLive();
         else
-          referentLive = r.referent.get<InputSection *>()->live;
+          referentLive = r.referent.get<InputSection *>()->isLive(r.addend);
         if (referentLive)
           enqueue(isec);
       }

diff  --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index 985188556f83..c0ae11bfd2f0 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -71,8 +71,8 @@ Defined *SymbolTable::addDefined(StringRef name, InputFile *file,
           //        We could do this if either is weak (but getting the
           //        case where !isWeakDef && defined->isWeakDef() right
           //        requires some care and testing).
-          if (isec)
-            isec->wasCoalesced = true;
+          if (auto concatIsec = dyn_cast_or_null<ConcatInputSection>(isec))
+            concatIsec->wasCoalesced = true;
         }
 
         return defined;

diff  --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp
index 0c439edf28b5..ccb57ae2eafd 100644
--- a/lld/MachO/Symbols.cpp
+++ b/lld/MachO/Symbols.cpp
@@ -40,7 +40,7 @@ bool Symbol::isLive() const {
     // no_dead_strip or live_support. In that case, the section will know
     // that it's live but `used` might be false. Non-absolute symbols always
     // have to use the section's `live` bit as source of truth.
-    return d->isAbsolute() ? used : d->isec->live;
+    return d->isAbsolute() ? used : d->isec->isLive(d->value);
   }
 
   assert(!isa<CommonSymbol>(this) &&

diff  --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h
index 29eaad808cae..02e27e1a4566 100644
--- a/lld/MachO/Symbols.h
+++ b/lld/MachO/Symbols.h
@@ -124,8 +124,8 @@ class Defined : public Symbol {
         includeInSymtab(true), thumb(isThumb),
         referencedDynamically(isReferencedDynamically),
         noDeadStrip(noDeadStrip), weakDef(isWeakDef), external(isExternal) {
-    if (isec)
-      isec->numRefs++;
+    if (auto concatIsec = dyn_cast_or_null<ConcatInputSection>(isec))
+      concatIsec->numRefs++;
   }
 
   bool isWeakDef() const override { return weakDef; }

diff  --git a/lld/MachO/UnwindInfoSection.cpp b/lld/MachO/UnwindInfoSection.cpp
index c485c9a02c9e..705b0dfdae27 100644
--- a/lld/MachO/UnwindInfoSection.cpp
+++ b/lld/MachO/UnwindInfoSection.cpp
@@ -105,7 +105,7 @@ struct SecondLevelPage {
 
 template <class Ptr> class UnwindInfoSectionImpl : public UnwindInfoSection {
 public:
-  void prepareRelocations(InputSection *) override;
+  void prepareRelocations(ConcatInputSection *) override;
   void finalize() override;
   void writeTo(uint8_t *buf) const override;
 
@@ -132,7 +132,7 @@ template <class Ptr> class UnwindInfoSectionImpl : public UnwindInfoSection {
 // actually end up in the final binary. Second, personality pointers always
 // reside in the GOT and must be treated specially.
 template <class Ptr>
-void UnwindInfoSectionImpl<Ptr>::prepareRelocations(InputSection *isec) {
+void UnwindInfoSectionImpl<Ptr>::prepareRelocations(ConcatInputSection *isec) {
   assert(isec->segname == segment_names::ld &&
          isec->name == section_names::compactUnwind);
   assert(!isec->shouldOmitFromOutput() &&
@@ -200,10 +200,12 @@ void UnwindInfoSectionImpl<Ptr>::prepareRelocations(InputSection *isec) {
 // finalization of __DATA. Moreover, the finalization of unwind info depends on
 // the exact addresses that it references. So it is safe for compact unwind to
 // reference addresses in __TEXT, but not addresses in any other segment.
-static void checkTextSegment(InputSection *isec) {
+static ConcatInputSection *checkTextSegment(InputSection *isec) {
   if (isec->segname != segment_names::text)
     error("compact unwind references address in " + toString(isec) +
           " which is not in segment __TEXT");
+  // __text should always be a ConcatInputSection.
+  return cast<ConcatInputSection>(isec);
 }
 
 // We need to apply the relocations to the pre-link compact unwind section
@@ -234,8 +236,8 @@ relocateCompactUnwind(ConcatOutputSection *compactUnwindSection,
           referentVA = referentSym->gotIndex + 1;
         }
       } else if (auto *referentIsec = r.referent.dyn_cast<InputSection *>()) {
-        checkTextSegment(referentIsec);
-        if (referentIsec->shouldOmitFromOutput())
+        ConcatInputSection *concatIsec = checkTextSegment(referentIsec);
+        if (concatIsec->shouldOmitFromOutput())
           referentVA = UINT64_MAX; // Tombstone value
         else
           referentVA = referentIsec->getVA(r.addend);

diff  --git a/lld/MachO/UnwindInfoSection.h b/lld/MachO/UnwindInfoSection.h
index d530cddf1d90..4f181099a583 100644
--- a/lld/MachO/UnwindInfoSection.h
+++ b/lld/MachO/UnwindInfoSection.h
@@ -32,7 +32,7 @@ class UnwindInfoSection : public SyntheticSection {
 public:
   bool isNeeded() const override { return compactUnwindSection != nullptr; }
   uint64_t getSize() const override { return unwindInfoSize; }
-  virtual void prepareRelocations(InputSection *) = 0;
+  virtual void prepareRelocations(ConcatInputSection *) = 0;
 
   void setCompactUnwindSection(ConcatOutputSection *cuSection) {
     compactUnwindSection = cuSection;

diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 1a1c2145811d..93d6af607209 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -566,11 +566,15 @@ static void prepareSymbolRelocation(Symbol *sym, const InputSection *isec,
 void Writer::scanRelocations() {
   TimeTraceScope timeScope("Scan relocations");
   for (InputSection *isec : inputSections) {
-    if (isec->shouldOmitFromOutput())
+    if (!isa<ConcatInputSection>(isec))
       continue;
+    auto concatIsec = cast<ConcatInputSection>(isec);
 
-    if (isec->segname == segment_names::ld) {
-      in.unwindInfo->prepareRelocations(isec);
+    if (concatIsec->shouldOmitFromOutput())
+      continue;
+
+    if (concatIsec->segname == segment_names::ld) {
+      in.unwindInfo->prepareRelocations(concatIsec);
       continue;
     }
 
@@ -591,7 +595,6 @@ void Writer::scanRelocations() {
           prepareSymbolRelocation(sym, isec, r);
       } else {
         assert(r.referent.is<InputSection *>());
-        assert(!r.referent.get<InputSection *>()->shouldOmitFromOutput());
         if (!r.pcrel)
           in.rebase->addEntry(isec, r.offset);
       }
@@ -861,10 +864,10 @@ template <class LP> void Writer::createOutputSections() {
   DenseMap<NamePair, ConcatOutputSection *> concatOutputSections;
   for (const auto &p : enumerate(inputSections)) {
     InputSection *isec = p.value();
-    if (isec->shouldOmitFromOutput())
-      continue;
     OutputSection *osec;
     if (auto *concatIsec = dyn_cast<ConcatInputSection>(isec)) {
+      if (concatIsec->shouldOmitFromOutput())
+        continue;
       NamePair names = maybeRenameSection({isec->segname, isec->name});
       ConcatOutputSection *&concatOsec = concatOutputSections[names];
       if (concatOsec == nullptr)


        


More information about the llvm-commits mailing list