[lld-macho][nfc] De-templatize UnwindInfoSection

[int3]

Summary

Follow-on to D123276: [lld-macho] Use fewer indirections in UnwindInfo implementation. Now that we work with an internal
representation of compact unwind entries, we no longer need to template
our UnwindInfoSectionImpl code based on the pointer size of the target
architecture.

I've still kept the split between UnwindInfoSectionImpl and
UnwindInfoSection. I'd introduced that split in order to do type
erasure, but I think it's still useful to have in order to keep
UnwindInfoSection's definition in the header file clean.

[oontvoo]

lld/MachO/UnwindInfoSection.cpp

106–123

nit: I'm not a big fan of macros and all these lines are quite repetitive, so i'm wondering if maybe we could shorten them to a helper function like this ^
(technically shorter :) )

107

👍 nicer than the previous struct (i.e hide the template stuff)

170

Why do you need another symbolsVec? (shouldh't this be inheritted from UnwindInfoSection?)

[int3]

address comments

lld/MachO/UnwindInfoSection.cpp

106–123

good suggestion, thanks!

170

oh good catch. the symbolsVec in the parent class is the redundant one :)

[oontvoo]

LG thanks!

lld/MachO/UnwindInfoSection.h

 namespace lld {
 namespace macho {
 
 class UnwindInfoSection : public SyntheticSection {
 public:
   // If all functions are free of unwind info, we can omit the unwind info
   // section entirely.
   bool isNeeded() const override { return !allEntriesAreOmitted; }
-  uint64_t getSize() const override { return unwindInfoSize; }
   void addSymbol(const Defined *);
-  void prepareRelocations();
+  virtual void prepareRelocations() = 0;
 
 protected:
   UnwindInfoSection();
-  virtual void prepareRelocations(ConcatInputSection *) = 0;
 
   llvm::MapVector<std::pair<const InputSection *, uint64_t /*Defined::value*/>,
                   const Defined *>
       symbols;
-  std::vector<decltype(symbols)::value_type> symbolsVec;
-  uint64_t unwindInfoSize = 0;
   bool allEntriesAreOmitted = true;
 };
 
 UnwindInfoSection *makeUnwindInfoSection();
 
 } // namespace macho
 } // namespace lld
 
 #endif

lld/MachO/UnwindInfoSection.cpp

 //
 // Refer to the definition of unwind_info_section_header in
 // compact_unwind_encoding.h for an overview of the format we are encoding
 // here.
 
 // TODO(gkm): prune __eh_frame entries superseded by __unwind_info, PR50410
 // TODO(gkm): how do we align the 2nd-level pages?
 
-template <class Ptr> struct CompactUnwindLayout {
+// The offsets of various fields in the on-disk representation of each compact
+// unwind entry.
+struct CompactUnwindOffsets {
+  uint32_t functionAddress;
+  uint32_t functionLength;
+  uint32_t encoding;
+  uint32_t personality;
+  uint32_t lsda;
+
+  CompactUnwindOffsets(size_t wordSize) {
+    if (wordSize == 8)
+      init<uint64_t>();
+    else {
+      assert(wordSize == 4);
+      init<uint32_t>();
+    }
+  }
+
+private:
+  template <class Ptr> void init() {
+    functionAddress = offsetof(Layout<Ptr>, functionAddress);
+    functionLength = offsetof(Layout<Ptr>, functionLength);
+    encoding = offsetof(Layout<Ptr>, encoding);
+    personality = offsetof(Layout<Ptr>, personality);
+    lsda = offsetof(Layout<Ptr>, lsda);
+  }
+
+  template <class Ptr> struct Layout {
     Ptr functionAddress;
     uint32_t functionLength;
     compact_unwind_encoding_t encoding;
     Ptr personality;
     Ptr lsda;
   };
+};
 
+// LLD's internal representation of a compact unwind entry.
 struct CompactUnwindEntry {
   uint64_t functionAddress;
   uint32_t functionLength;
   compact_unwind_encoding_t encoding;
   Symbol *personality;
   InputSection *lsda;
 };
 
 using EncodingMap = DenseMap<compact_unwind_encoding_t, size_t>;
 
 struct SecondLevelPage {
   uint32_t kind;
   size_t entryIndex;
   size_t entryCount;
   size_t byteCount;
   std::vector<compact_unwind_encoding_t> localEncodings;
   EncodingMap localEncodingIndexes;
 };
 
-template <class Ptr>
+// UnwindInfoSectionImpl allows us to avoid cluttering our header file with a
+// lengthy definition of UnwindInfoSection.
 class UnwindInfoSectionImpl final : public UnwindInfoSection {
 public:
-  void prepareRelocations(ConcatInputSection *) override;
-  void relocateCompactUnwind(std::vector<CompactUnwindEntry> &);
-  void encodePersonalities();
+  UnwindInfoSectionImpl() : cuOffsets(target->wordSize) {}
+  uint64_t getSize() const override { return unwindInfoSize; }
+  void prepareRelocations() override;
   void finalize() override;
   void writeTo(uint8_t *buf) const override;
 
 private:
+  void prepareRelocations(ConcatInputSection *);
+  void relocateCompactUnwind(std::vector<CompactUnwindEntry> &);
+  void encodePersonalities();
+
+  uint64_t unwindInfoSize = 0;
+  std::vector<decltype(symbols)::value_type> symbolsVec;
+  CompactUnwindOffsets cuOffsets;
   std::vector<std::pair<compact_unwind_encoding_t, size_t>> commonEncodings;
   EncodingMap commonEncodingIndexes;
   // The entries here will be in the same order as their originating symbols
   // in symbolsVec.
   std::vector<CompactUnwindEntry> cuEntries;
   // Indices into the cuEntries vector.
   std::vector<size_t> cuIndices;
   std::vector<Symbol *> personalities;
   SmallDenseMap<std::pair<InputSection *, uint64_t /* addend */>, Symbol *>
       personalityTable;
   // Indices into cuEntries for CUEs with a non-null LSDA.
   std::vector<size_t> entriesWithLsda;
   // Map of cuEntries index to an index within the LSDA array.
   DenseMap<size_t, uint32_t> lsdaIndex;
   std::vector<SecondLevelPage> secondLevelPages;
   uint64_t level2PagesOffset = 0;
 };
 
 UnwindInfoSection::UnwindInfoSection()
     : SyntheticSection(segment_names::text, section_names::unwindInfo) {
   align = 4;
 }
 
-void UnwindInfoSection::prepareRelocations() {
-  // This iteration needs to be deterministic, since prepareRelocations may add
-  // entries to the GOT. Hence the use of a MapVector for
-  // UnwindInfoSection::symbols.
-  for (const Defined *d : make_second_range(symbols))
-    if (d->unwindEntry)
-      prepareRelocations(d->unwindEntry);
-}
-
 // Record function symbols that may need entries emitted in __unwind_info, which
 // stores unwind data for address ranges.
 //
 // Note that if several adjacent functions have the same unwind encoding, LSDA,
 // and personality function, they share one unwind entry. For this to work,
 // functions without unwind info need explicit "no unwind info" unwind entries
 // -- else the unwinder would think they have the unwind info of the closest
 // function with unwind info right before in the image. Thus, we add function
void UnwindInfoSection::addSymbol(const Defined *d) {
   // If we have multiple symbols at the same address, only one of them can have
   // an associated CUE.
   if (!p.second && d->unwindEntry) {
     assert(!p.first->second->unwindEntry);
     p.first->second = d;
   }
 }
 
+void UnwindInfoSectionImpl::prepareRelocations() {
+  // This iteration needs to be deterministic, since prepareRelocations may add
+  // entries to the GOT. Hence the use of a MapVector for
+  // UnwindInfoSection::symbols.
+  for (const Defined *d : make_second_range(symbols))
+    if (d->unwindEntry)
+      prepareRelocations(d->unwindEntry);
+}
+
 // Compact unwind relocations have different semantics, so we handle them in a
 // separate code path from regular relocations. First, we do not wish to add
 // rebase opcodes for __LD,__compact_unwind, because that section doesn't
 // 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(ConcatInputSection *isec) {
+void UnwindInfoSectionImpl::prepareRelocations(ConcatInputSection *isec) {
   assert(!isec->shouldOmitFromOutput() &&
          "__compact_unwind section should not be omitted");
 
   // FIXME: Make this skip relocations for CompactUnwindEntries that
   // point to dead-stripped functions. That might save some amount of
   // work. But since there are usually just few personality functions
   // that are referenced from many places, at least some of them likely
   // live, it wouldn't reduce number of got entries.
   for (size_t i = 0; i < isec->relocs.size(); ++i) {
     Reloc &r = isec->relocs[i];
     assert(target->hasAttr(r.type, RelocAttrBits::UNSIGNED));
 
     // Functions and LSDA entries always reside in the same object file as the
     // compact unwind entries that references them, and thus appear as section
     // relocs. There is no need to prepare them. We only prepare relocs for
     // personality functions.
-    if (r.offset != offsetof(CompactUnwindLayout<Ptr>, personality))
+    if (r.offset != cuOffsets.personality)
       continue;
 
     if (auto *s = r.referent.dyn_cast<Symbol *>()) {
       // Personality functions are nearly always system-defined (e.g.,
       // ___gxx_personality_v0 for C++) and relocated as dylib symbols.  When an
       // application provides its own personality function, it might be
       // referenced by an extern Defined symbol reloc, or a local section reloc.
       if (auto *defined = dyn_cast<Defined>(s)) {
for (size_t i = 0; i < isec->relocs.size(); ++i) {
     }
   }
 }
 
 // We need to apply the relocations to the pre-link compact unwind section
 // before converting it to post-link form. There should only be absolute
 // relocations here: since we are not emitting the pre-link CU section, there
 // is no source address to make a relative location meaningful.
-template <class Ptr>
-void UnwindInfoSectionImpl<Ptr>::relocateCompactUnwind(
+void UnwindInfoSectionImpl::relocateCompactUnwind(
     std::vector<CompactUnwindEntry> &cuEntries) {
   parallelForEachN(0, symbolsVec.size(), [&](size_t i) {
     CompactUnwindEntry &cu = cuEntries[i];
     const Defined *d = symbolsVec[i].second;
     cu.functionAddress = d->getVA();
     if (!d->unwindEntry)
       return;
 
     auto buf = reinterpret_cast<const uint8_t *>(d->unwindEntry->data.data()) -
-               sizeof(Ptr);
-    cu.functionLength = support::endian::read32le(
-        buf + offsetof(CompactUnwindLayout<Ptr>, functionLength));
-    cu.encoding = support::endian::read32le(
-        buf + offsetof(CompactUnwindLayout<Ptr>, encoding));
+               target->wordSize;
+    cu.functionLength =
+        support::endian::read32le(buf + cuOffsets.functionLength);
+    cu.encoding = support::endian::read32le(buf + cuOffsets.encoding);
     for (const Reloc &r : d->unwindEntry->relocs) {
-      if (r.offset == offsetof(CompactUnwindLayout<Ptr>, personality)) {
+      if (r.offset == cuOffsets.personality) {
         cu.personality = r.referent.get<Symbol *>();
-      } else if (r.offset == offsetof(CompactUnwindLayout<Ptr>, lsda)) {
+      } else if (r.offset == cuOffsets.lsda) {
         if (auto *referentSym = r.referent.dyn_cast<Symbol *>())
           cu.lsda = cast<Defined>(referentSym)->isec;
         else
           cu.lsda = r.referent.get<InputSection *>();
       }
     }
   });
 }
 
 // There should only be a handful of unique personality pointers, so we can
 // encode them as 2-bit indices into a small array.
-template <class Ptr> void UnwindInfoSectionImpl<Ptr>::encodePersonalities() {
+void UnwindInfoSectionImpl::encodePersonalities() {
   for (size_t idx : cuIndices) {
     CompactUnwindEntry &cu = cuEntries[idx];
     if (cu.personality == nullptr)
       continue;
     // Linear search is fast enough for a small array.
     auto it = find(personalities, cu.personality);
     uint32_t personalityIndex; // 1-based index
     if (it != personalities.end()) {
if ((target->cpuType == CPU_TYPE_X86_64 || target->cpuType == CPU_TYPE_X86) &&
     // actually different.
     return false;
   }
   return true;
 }
 
 // Scan the __LD,__compact_unwind entries and compute the space needs of
 // __TEXT,__unwind_info and __TEXT,__eh_frame.
-template <class Ptr> void UnwindInfoSectionImpl<Ptr>::finalize() {
+void UnwindInfoSectionImpl::finalize() {
   if (symbols.empty())
     return;
 
   // At this point, the address space for __TEXT,__text has been
   // assigned, so we can relocate the __LD,__compact_unwind entries
   // into a temporary buffer. Relocation is necessary in order to sort
   // the CU entries by function address. Sorting is necessary so that
   // we can fold adjacent CU entries with identical
level2PagesOffset = sizeof(unwind_info_section_header) +
                       entriesWithLsda.size() *
                           sizeof(unwind_info_section_header_lsda_index_entry);
   unwindInfoSize =
       level2PagesOffset + secondLevelPages.size() * SECOND_LEVEL_PAGE_BYTES;
 }
 
 // All inputs are relocated and output addresses are known, so write!
 
-template <class Ptr>
-void UnwindInfoSectionImpl<Ptr>::writeTo(uint8_t *buf) const {
+void UnwindInfoSectionImpl::writeTo(uint8_t *buf) const {
   assert(!cuIndices.empty() && "call only if there is unwind info");
 
   // section header
   auto *uip = reinterpret_cast<unwind_info_section_header *>(buf);
   uip->version = 1;
   uip->commonEncodingsArraySectionOffset = sizeof(unwind_info_section_header);
   uip->commonEncodingsArrayCount = commonEncodings.size();
   uip->personalityArraySectionOffset =
if (page.kind == UNWIND_SECOND_LEVEL_COMPRESSED) {
         *ep++ = cue.encoding;
       }
     }
     pp += SECOND_LEVEL_PAGE_WORDS;
   }
 }
 
 UnwindInfoSection *macho::makeUnwindInfoSection() {
-  if (target->wordSize == 8)
-    return make<UnwindInfoSectionImpl<uint64_t>>();
-  else
-    return make<UnwindInfoSectionImpl<uint32_t>>();
+  return make<UnwindInfoSectionImpl>();
 }