[lld-macho] Address Feedback #1

Author: Alex B

lld/MachO/Driver.cpp

@@ -340,7 +340,7 @@ static InputFile *addFile(StringRef path, LoadType loadType,
       }
     } else if (isCommandLineLoad && config->forceLoadObjC) {
       for (const object::Archive::Symbol &sym : file->getArchive().symbols())
-        if (sym.getName().starts_with(objc::klass))
+        if (sym.getName().starts_with(objc::symbol_names::klass))
           file->fetch(sym);
 
       // TODO: no need to look for ObjC sections for a given archive member if
@@ -395,7 +395,7 @@ static InputFile *addFile(StringRef path, LoadType loadType,
     if ((isa<ObjFile>(newFile) || isa<BitcodeFile>(newFile)) && newFile->lazy &&
         config->forceLoadObjC) {
       for (Symbol *sym : newFile->symbols)
-        if (sym && sym->getName().starts_with(objc::klass)) {
+        if (sym && sym->getName().starts_with(objc::symbol_names::klass)) {
           extract(*newFile, "-ObjC");
           break;
         }
@@ -1437,6 +1437,8 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     resetOutputSegments();
     resetWriter();
     InputFile::resetIdCount();
+
+    objc::doCleanup();
   };
 
   ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]);
@@ -1984,6 +1986,8 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     if (args.hasArg(OPT_check_category_conflicts))
       objc::checkCategories();
 
+    // Category merging uses "->live = false" to erase old category data, so
+    // it has to run after dead-stripping (markLive).
     if (args.hasArg(OPT_objc_category_merging, OPT_no_objc_category_merging))
       objc::mergeCategories();
 

lld/MachO/InputFiles.cpp

@@ -1921,14 +1921,14 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
     case EncodeKind::ObjectiveCClass:
       // XXX ld64 only creates these symbols when -ObjC is passed in. We may
       // want to emulate that.
-      addSymbol(*symbol, objc::klass + symbol->getName());
-      addSymbol(*symbol, objc::metaclass + symbol->getName());
+      addSymbol(*symbol, objc::symbol_names::klass + symbol->getName());
+      addSymbol(*symbol, objc::symbol_names::metaclass + symbol->getName());
       break;
     case EncodeKind::ObjectiveCClassEHType:
-      addSymbol(*symbol, objc::ehtype + symbol->getName());
+      addSymbol(*symbol, objc::symbol_names::ehtype + symbol->getName());
       break;
     case EncodeKind::ObjectiveCInstanceVariable:
-      addSymbol(*symbol, objc::ivar + symbol->getName());
+      addSymbol(*symbol, objc::symbol_names::ivar + symbol->getName());
       break;
     }
   }

lld/MachO/InputSection.h

@@ -24,11 +24,6 @@
 namespace lld {
 namespace macho {
 
-enum LinkerOptReason : uint8_t {
-  NotOptimized,
-  CategoryMerging,
-};
-
 class InputFile;
 class OutputSection;
 
@@ -65,7 +60,6 @@ class InputSection {
   // Whether the data at \p off in this InputSection is live.
   virtual bool isLive(uint64_t off) const = 0;
   virtual void markLive(uint64_t off) = 0;
-  virtual bool isLinkOptimizedAway() const { return false; }
   virtual InputSection *canonical() { return this; }
   virtual const InputSection *canonical() const { return this; }
 
@@ -120,12 +114,7 @@ class ConcatInputSection final : public InputSection {
   bool isLive(uint64_t off) const override { return live; }
   void markLive(uint64_t off) override { live = true; }
   bool isCoalescedWeak() const { return wasCoalesced && symbols.empty(); }
-  bool isLinkOptimizedAway() const override {
-    return linkerOptimizeReason != LinkerOptReason::NotOptimized;
-  }
-  bool shouldOmitFromOutput() const {
-    return isLinkOptimizedAway() || !live || isCoalescedWeak();
-  }
+  bool shouldOmitFromOutput() const { return !live || isCoalescedWeak(); }
   void writeTo(uint8_t *buf);
 
   void foldIdentical(ConcatInputSection *redundant);
@@ -152,11 +141,6 @@ class ConcatInputSection final : public InputSection {
   // first and not copied to the output.
   bool wasCoalesced = false;
   bool live = !config->deadStrip;
-  // Flag to specify if a linker optimzation flagged this section to be
-  // discarded. Need a separate flag from live as live specifically means
-  // 'dead-stripped' which is rellevant in contexts such as linker map
-  // generation
-  LinkerOptReason linkerOptimizeReason = LinkerOptReason::NotOptimized;
   bool hasCallSites = false;
   // This variable has two usages. Initially, it represents the input order.
   // After assignAddresses is called, it represents the offset from the
@@ -192,20 +176,10 @@ struct StringPiece {
   // Only set if deduplicating literals
   uint32_t hash : 31;
   // Offset from the start of the containing output section.
-  uint64_t outSecOff : 48;
-  // Have to declare the 'linkerOptimizeReason' and 'live' as uint64_t so that
-  // the MSVC compiler will merge the storage of it and 'outSecOff' above.
-  uint64_t /*LinkerOptReason*/ linkerOptimizeReason : 8;
-
-  bool shouldOmitFromOutput() const {
-    return !live || linkerOptimizeReason != LinkerOptReason::NotOptimized;
-  }
+  uint64_t outSecOff = 0;
 
   StringPiece(uint64_t off, uint32_t hash)
-      : inSecOff(off), live(!config->deadStrip), hash(hash) {
-    outSecOff = 0;
-    linkerOptimizeReason = LinkerOptReason::NotOptimized;
-  }
+      : inSecOff(off), live(!config->deadStrip), hash(hash) {}
 };
 
 static_assert(sizeof(StringPiece) == 16, "StringPiece is too big!");

lld/MachO/MapFile.cpp

@@ -80,7 +80,7 @@ static MapInfo gatherMapInfo() {
           if (d->isec && d->getFile() == file &&
               !isa<CStringInputSection>(d->isec)) {
             isReferencedFile = true;
-            if (!d->isLive() && (!d->isec || !d->isec->isLinkOptimizedAway()))
+            if (!d->isLive())
               info.deadSymbols.push_back(d);
           }
       }
@@ -93,8 +93,6 @@ static MapInfo gatherMapInfo() {
           if (auto isec = dyn_cast<CStringInputSection>(subsec.isec)) {
             auto &liveCStrings = info.liveCStringsForSection[isec->parent];
             for (const auto &[i, piece] : llvm::enumerate(isec->pieces)) {
-              if (piece.linkerOptimizeReason != LinkerOptReason::NotOptimized)
-                continue;
               if (piece.live)
                 liveCStrings.push_back({isec->parent->addr + piece.outSecOff,
                                         {fileIndex, isec->getStringRef(i)}});
@@ -205,8 +203,6 @@ void macho::writeMapFile() {
     for (const OutputSection *osec : seg->getSections()) {
       if (auto *concatOsec = dyn_cast<ConcatOutputSection>(osec)) {
         for (const InputSection *isec : concatOsec->inputs) {
-          if (isec->isLinkOptimizedAway())
-            continue;
           for (Defined *sym : isec->symbols)
             if (!(isPrivateLabel(sym->getName()) && sym->size == 0))
               os << format("0x%08llX\t0x%08llX\t[%3u] %s\n", sym->getVA(),

lld/MachO/MarkLive.cpp

@@ -259,8 +259,6 @@ void markLive() {
           dyn_cast_or_null<DylibSymbol>(symtab->find("dyld_stub_binder")))
     marker->addSym(stubBinder);
   for (ConcatInputSection *isec : inputSections) {
-    if (isec->isLinkOptimizedAway())
-      continue;
     // Sections marked no_dead_strip
     if (isec->getFlags() & S_ATTR_NO_DEAD_STRIP) {
       marker->enqueue(isec, 0);