[lld-macho] Canonicalize personality pointers in EH frames

[int3]

Summary

We already do this for personality pointers referenced from compact
unwind entries; this patch extends that behavior to personalities
referenced via EH frames as well.

This reduces the number of distinct personalities we need in the final
binary, and helps us avoid hitting the "too many personalities" error.

I renamed UnwindInfoSection::prepareRelocations() to simply prepare
since we now do some non-reloc-specific stuff within.

Fixes #58277.

[int3]

lld/test/MachO/eh-frame-personality-dedup.s

19

this used to print 2

[oontvoo]

LG - thanks!

lld/MachO/UnwindInfoSection.h

 namespace lld::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; }
   void addSymbol(const Defined *);
-  virtual void prepareRelocations() = 0;
+  virtual void prepare() = 0;
 
 protected:
   UnwindInfoSection();
 
   llvm::MapVector<std::pair<const InputSection *, uint64_t /*Defined::value*/>,
                   const Defined *>
       symbols;
   bool allEntriesAreOmitted = true;
 };
 
 UnwindInfoSection *makeUnwindInfoSection();
 
 } // namespace lld::macho
 
 #endif

lld/MachO/UnwindInfoSection.cpp

 };
 
 // UnwindInfoSectionImpl allows us to avoid cluttering our header file with a
 // lengthy definition of UnwindInfoSection.
 class UnwindInfoSectionImpl final : public UnwindInfoSection {
 public:
   UnwindInfoSectionImpl() : cuOffsets(target->wordSize) {}
   uint64_t getSize() const override { return unwindInfoSize; }
-  void prepareRelocations() override;
+  void prepare() override;
   void finalize() override;
   void writeTo(uint8_t *buf) const override;
 
 private:
   void prepareRelocations(ConcatInputSection *);
   void relocateCompactUnwind(std::vector<CompactUnwindEntry> &);
   void encodePersonalities();
+  Symbol *canonicalizePersonality(Symbol *);
 
   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.
void UnwindInfoSection::addSymbol(const Defined *d) {
   // If we have multiple symbols at the same address, only one of them can have
   // an associated unwind entry.
   if (!p.second && d->unwindEntry) {
     assert(!p.first->second->unwindEntry);
     p.first->second = d;
   }
 }
 
-void UnwindInfoSectionImpl::prepareRelocations() {
+void UnwindInfoSectionImpl::prepare() {
   // 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 &&
-        d->unwindEntry->getName() == section_names::compactUnwind)
+    if (d->unwindEntry) {
+      if (d->unwindEntry->getName() == section_names::compactUnwind) {
         prepareRelocations(d->unwindEntry);
+      } else {
+        // We don't have to add entries to the GOT here because FDEs have
+        // explicit GOT relocations, so Writer::scanRelocations() will add those
+        // GOT entries. However, we still need to canonicalize the personality
+        // pointers (like prepareRelocations() does for CU entries) in order
+        // to avoid overflowing the 3-personality limit.
+        FDE &fde = cast<ObjFile>(d->getFile())->fdes[d->unwindEntry];
+        fde.personality = canonicalizePersonality(fde.personality);
+      }
+    }
 }
 
 // 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.
 void UnwindInfoSectionImpl::prepareRelocations(ConcatInputSection *isec) {
if (auto *s = r.referent.dyn_cast<Symbol *>()) {
       }
       if (auto *undefined = dyn_cast<Undefined>(s)) {
         treatUndefinedSymbol(*undefined, isec, r.offset);
         // treatUndefinedSymbol() can replace s with a DylibSymbol; re-check.
         if (isa<Undefined>(s))
           continue;
       }
 
+      // Similar to canonicalizePersonality(), but we also register a GOT entry.
       if (auto *defined = dyn_cast<Defined>(s)) {
         // Check if we have created a synthetic symbol at the same address.
         Symbol *&personality =
             personalityTable[{defined->isec, defined->value}];
         if (personality == nullptr) {
           personality = defined;
           in.got->addEntry(defined);
         } else if (personality != defined) {
           r.referent = personality;
         }
         continue;
       }
+
       assert(isa<DylibSymbol>(s));
       in.got->addEntry(s);
       continue;
     }
 
     if (auto *referentIsec = r.referent.dyn_cast<InputSection *>()) {
       assert(!isCoalescedWeak(referentIsec));
       // Personality functions can be referenced via section relocations
if (auto *referentIsec = r.referent.dyn_cast<InputSection *>()) {
         in.got->addEntry(s);
       }
       r.referent = s;
       r.addend = 0;
     }
   }
 }
 
+Symbol *UnwindInfoSectionImpl::canonicalizePersonality(Symbol *personality) {
+  if (auto *defined = dyn_cast_or_null<Defined>(personality)) {
+    // Check if we have created a synthetic symbol at the same address.
+    Symbol *&synth = personalityTable[{defined->isec, defined->value}];
+    if (synth == nullptr)
+      synth = defined;
+    else if (synth != defined)
+      return synth;
+  }
+  return personality;
+}
+
 // 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.
 void UnwindInfoSectionImpl::relocateCompactUnwind(
     std::vector<CompactUnwindEntry> &cuEntries) {
   parallelFor(0, symbolsVec.size(), [&](size_t i) {
     CompactUnwindEntry &cu = cuEntries[i];

lld/MachO/Writer.cpp

for (auto it = isec->relocs.begin(); it != isec->relocs.end(); ++it) {
             in.chainedFixups->addRebase(isec, r.offset);
           else
             in.rebase->addEntry(isec, r.offset);
         }
       }
     }
   }
 
-  in.unwindInfo->prepareRelocations();
+  in.unwindInfo->prepare();
 }
 
 static void addNonWeakDefinition(const Defined *defined) {
   if (config->emitChainedFixups)
     in.chainedFixups->setHasNonWeakDefinition();
   else
     in.weakBinding->addNonWeakDefinition(defined);
 }

lld/test/MachO/eh-frame-personality-dedup.s

+# REQUIRES: x86
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/eh-frame.s -o %t/eh-frame.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/cu.s -o %t/cu.o
+# RUN: %lld -dylib %t/cu.o %t/eh-frame.o -o %t/out
+
+## Sanity check: we want our input to contain a section (and not symbol)
+## relocation for the personality reference.
+# RUN: llvm-readobj --relocations %t/cu.o | FileCheck %s --check-prefix=SECT-RELOC
+# SECT-RELOC:      Section __compact_unwind {
+# SECT-RELOC-NEXT:   __text
+# SECT-RELOC-NEXT:   __text
+# SECT-RELOC-NEXT: }
+
+## Verify that the personality referenced via a symbol reloc in eh-frame.s gets
+## dedup'ed with the personality referenced via a section reloc in cu.s.
+# RUN: llvm-objdump --macho --unwind-info %t/out | FileCheck %s
+# CHECK: Personality functions: (count = 1)
+
+#--- eh-frame.s
+_fun:
+  .cfi_startproc
+  .cfi_personality 155, _my_personality
+  ## cfi_escape cannot be encoded in compact unwind
+  .cfi_escape 0
+  ret
+  .cfi_endproc
+
+.subsections_via_symbols
+
+#--- cu.s
+.globl _my_personality
+_fun:
+  .cfi_startproc
+  .cfi_personality 155, _my_personality
+  .cfi_def_cfa_offset 16
+  ret
+  .cfi_endproc
+
+_my_personality:
+  nop
+
+.subsections_via_symbols