[lld-macho][nfc] Canonicalize all pointers to InputSections early on

[int3]

Summary

Having to remember to call canonical() all over the place is
error-prone; let's do it in a centralized location instead. It also
appears to improve performance slightly.

           base           diff           difference (95% CI)
sys_time   0.984 ± 0.009  0.983 ± 0.014  [  -0.8% ..   +0.6%]
user_time  6.508 ± 0.035  6.475 ± 0.036  [  -0.8% ..   -0.2%]
wall_time  5.321 ± 0.034  5.300 ± 0.033  [  -0.7% ..   -0.1%]
samples    36             23

[int3]

update

[int3]

update

[thakis]

lg

lld/MachO/InputSection.h

public:
   void markLive(uint64_t off) override { live = true; }
   bool isCoalescedWeak() const { return wasCoalesced && symbols.size() == 0; }
   bool shouldOmitFromOutput() const { return !live || isCoalescedWeak(); }
   bool isHashableForICF() const;
   void hashForICF();
   void writeTo(uint8_t *buf);
 
   void foldIdentical(ConcatInputSection *redundant);
-  InputSection *canonical() override {
+  ConcatInputSection *canonical() override {
     return replacement ? replacement : this;
   }
 
   static bool classof(const InputSection *isec) {
     return isec->kind() == ConcatKind;
   }
 
   // Points to the surviving section after this one is folded by ICF
-  InputSection *replacement = nullptr;
+  ConcatInputSection *replacement = nullptr;
   // Equivalence-class ID for ICF
   uint64_t icfEqClass[2] = {0, 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.

lld/MachO/Symbols.h

public:
   }
   bool isTlv() const override;
 
   bool isExternal() const { return external; }
   bool isAbsolute() const { return isec == nullptr; }
 
   uint64_t getVA() const override;
 
+  // Ensure this symbol's pointers to InputSections point to their canonical
+  // copies.
+  void canonicalize();
+
   static bool classof(const Symbol *s) { return s->kind() == DefinedKind; }
 
   InputSection *isec;
   // Contains the offset from the containing subsection. Note that this is
   // different from nlist::n_value, which is the absolute address of the symbol.
   uint64_t value;
   // size is only calculated for regular (non-bitcode) symbols.
   uint64_t size;

lld/MachO/Symbols.cpp

 }
 
 uint64_t Defined::getVA() const {
   assert(isLive() && "this should only be called for live symbols");
 
   if (isAbsolute())
     return value;
 
-  if (!isec->canonical()->isFinal) {
+  if (!isec->isFinal) {
     // A target arch that does not use thunks ought never ask for
     // the address of a function that has not yet been finalized.
     assert(target->usesThunks());
 
     // ConcatOutputSection::finalize() can seek the address of a
     // function before its address is assigned. The thunking algorithm
     // knows that unfinalized functions will be out of range, so it is
     // expedient to return a contrived out-of-range address.
     return TargetInfo::outOfRangeVA;
   }
-  return isec->canonical()->getVA(value);
+  return isec->getVA(value);
+}
+
+void Defined::canonicalize() {
+  if (compactUnwind)
+    compactUnwind = compactUnwind->canonical();
+  if (isec)
+    isec = isec->canonical();
 }
 
 uint64_t DylibSymbol::getVA() const {
   return isInStubs() ? getStubVA() : Symbol::getVA();
 }
 
 void LazySymbol::fetchArchiveMember() { getFile()->fetch(sym); }

lld/MachO/SyntheticSections.cpp

if (lastFile == nullptr || lastFile != file) {
         emitEndSourceStab();
       lastFile = file;
 
       emitBeginSourceStab(file->compileUnit);
       emitObjectFileStab(file);
     }
 
     StabsEntry symStab;
-    symStab.sect = defined->isec->canonical()->parent->index;
+    symStab.sect = defined->isec->parent->index;
     symStab.strx = stringTableSection.addString(defined->getName());
     symStab.value = defined->getVA();
 
     if (isCodeSection(isec)) {
       symStab.type = N_FUN;
       stabs.emplace_back(std::move(symStab));
       emitEndFunStab(defined);
     } else {
if (auto *defined = dyn_cast<Defined>(entry.sym)) {
       }
 
       if (defined->isAbsolute()) {
         nList->n_type = scope | N_ABS;
         nList->n_sect = NO_SECT;
         nList->n_value = defined->value;
       } else {
         nList->n_type = scope | N_SECT;
-        nList->n_sect = defined->isec->canonical()->parent->index;
+        nList->n_sect = defined->isec->parent->index;
         // For the N_SECT symbol type, n_value is the address of the symbol
         nList->n_value = defined->getVA();
       }
       nList->n_desc |= defined->thumb ? N_ARM_THUMB_DEF : 0;
       nList->n_desc |= defined->isExternalWeakDef() ? N_WEAK_DEF : 0;
       nList->n_desc |=
           defined->referencedDynamically ? REFERENCED_DYNAMICALLY : 0;
     } else if (auto *dysym = dyn_cast<DylibSymbol>(entry.sym)) {

lld/MachO/Writer.cpp

for (size_t i = 0; i < inputSections.size(); ++i) {
     }
   }
 
   in.unwindInfo->prepareRelocations();
 }
 
 void Writer::scanSymbols() {
   TimeTraceScope timeScope("Scan symbols");
-  for (const Symbol *sym : symtab->getSymbols()) {
-    if (const auto *defined = dyn_cast<Defined>(sym)) {
+  for (Symbol *sym : symtab->getSymbols()) {
+    if (auto *defined = dyn_cast<Defined>(sym)) {
       if (!defined->isLive())
         continue;
+      defined->canonicalize();
       if (defined->overridesWeakDef)
         in.weakBinding->addNonWeakDefinition(defined);
       if (!defined->isAbsolute() && isCodeSection(defined->isec))
         in.unwindInfo->addSymbol(defined);
     } else if (const auto *dysym = dyn_cast<DylibSymbol>(sym)) {
       // This branch intentionally doesn't check isLive().
       if (dysym->isDynamicLookup())
         continue;
       dysym->getFile()->refState =
           std::max(dysym->getFile()->refState, dysym->getRefState());
     }
   }
 
   for (const InputFile *file : inputFiles) {
     if (auto *objFile = dyn_cast<ObjFile>(file))
       for (Symbol *sym : objFile->symbols) {
-        if (auto *defined = dyn_cast_or_null<Defined>(sym))
-          if (!defined->isExternal() && defined->isLive() &&
-              !defined->isAbsolute() && isCodeSection(defined->isec))
+        if (auto *defined = dyn_cast_or_null<Defined>(sym)) {
+          if (!defined->isLive())
+            continue;
+          defined->canonicalize();
+          if (!defined->isExternal() && !defined->isAbsolute() &&
+              isCodeSection(defined->isec))
             in.unwindInfo->addSymbol(defined);
         }
       }
   }
+}
 
 // TODO: ld64 enforces the old load commands in a few other cases.
 static bool useLCBuildVersion(const PlatformInfo &platformInfo) {
   static const std::vector<std::pair<PlatformKind, VersionTuple>> minVersion = {
       {PlatformKind::macOS, VersionTuple(10, 14)},
       {PlatformKind::iOS, VersionTuple(12, 0)},
       {PlatformKind::iOSSimulator, VersionTuple(13, 0)},
       {PlatformKind::tvOS, VersionTuple(12, 0)},
void Writer::writeOutputFile() {
   if (auto e = buffer->commit())
     error("failed to write to the output file: " + toString(std::move(e)));
 }
 
 template <class LP> void Writer::run() {
   treatSpecialUndefineds();
   if (config->entry && !isa<Undefined>(config->entry))
     prepareBranchTarget(config->entry);
+  // Canonicalization of all pointers to InputSections should be handled by
+  // these two methods.
   scanSymbols();
   scanRelocations();
 
   // Do not proceed if there was an undefined symbol.
   if (errorCount())
     return;
 
   if (in.stubHelper->isNeeded())