[lld] [lld-macho][NFC] Refactor insertions into inputSections (PR #85692)

via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 18 13:20:36 PDT 2024


https://github.com/alx32 created https://github.com/llvm/llvm-project/pull/85692

Before this change, after `InputSection` objects are created, they need to be added to the appropriate container for tracking.
The logic for selecting the appropriate container lives in `Driver.cpp` / `gatherInputSections`, where the `InputSection` is added to the matching container depending on the input config and the  type of  `InputSection`.

Also, multiple other locations also insert directly into `inputSections` array - assuming that that is the appropriate container for the `InputSection`'s they create. Currently this is the correct assumption, however an upcoming change will change this. 

For an upcoming feature (relative method lists), we need to route `InputSection`'s either to `inputSections` array or to a synthetic section, depending on weather the relative method list optimization is enabled or not.

We can achieve the above either by duplicating some of the logic or refactoring the routing and  `InputSection`'s and reusing that. 

The refactoring & code sharing approach seems the correct way to go - as such this diff performs the refactoring while not introducing any functional changes. Later on we can just call `addInputSection` and not have to worry about routing logic. 


>From 06d973c6e914beba75dd8e6f43fbf93aeb7c934d Mon Sep 17 00:00:00 2001
From: Alex B <alexborcan at meta.com>
Date: Mon, 18 Mar 2024 13:04:55 -0700
Subject: [PATCH] [lld-macho][NFC] Refactor insertions into inputSections

---
 lld/MachO/Driver.cpp            | 41 +++------------------------------
 lld/MachO/InputSection.cpp      | 37 +++++++++++++++++++++++++++++
 lld/MachO/InputSection.h        |  1 +
 lld/MachO/ObjC.cpp              | 11 +++++----
 lld/MachO/SyntheticSections.cpp |  4 ++--
 5 files changed, 49 insertions(+), 45 deletions(-)

diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 36248925d65ad2..7a9e4072bcf489 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -612,7 +612,7 @@ static void replaceCommonSymbols() {
     if (!osec)
       osec = ConcatOutputSection::getOrCreateForInput(isec);
     isec->parent = osec;
-    inputSections.push_back(isec);
+    addInputSection(isec);
 
     // FIXME: CommonSymbol should store isReferencedDynamically, noDeadStrip
     // and pass them on here.
@@ -1220,53 +1220,18 @@ static void createFiles(const InputArgList &args) {
 
 static void gatherInputSections() {
   TimeTraceScope timeScope("Gathering input sections");
-  int inputOrder = 0;
   for (const InputFile *file : inputFiles) {
     for (const Section *section : file->sections) {
       // Compact unwind entries require special handling elsewhere. (In
       // contrast, EH frames are handled like regular ConcatInputSections.)
       if (section->name == section_names::compactUnwind)
         continue;
-      ConcatOutputSection *osec = nullptr;
-      for (const Subsection &subsection : section->subsections) {
-        if (auto *isec = dyn_cast<ConcatInputSection>(subsection.isec)) {
-          if (isec->isCoalescedWeak())
-            continue;
-          if (config->emitInitOffsets &&
-              sectionType(isec->getFlags()) == S_MOD_INIT_FUNC_POINTERS) {
-            in.initOffsets->addInput(isec);
-            continue;
-          }
-          isec->outSecOff = inputOrder++;
-          if (!osec)
-            osec = ConcatOutputSection::getOrCreateForInput(isec);
-          isec->parent = osec;
-          inputSections.push_back(isec);
-        } else if (auto *isec =
-                       dyn_cast<CStringInputSection>(subsection.isec)) {
-          if (isec->getName() == section_names::objcMethname) {
-            if (in.objcMethnameSection->inputOrder == UnspecifiedInputOrder)
-              in.objcMethnameSection->inputOrder = inputOrder++;
-            in.objcMethnameSection->addInput(isec);
-          } else {
-            if (in.cStringSection->inputOrder == UnspecifiedInputOrder)
-              in.cStringSection->inputOrder = inputOrder++;
-            in.cStringSection->addInput(isec);
-          }
-        } else if (auto *isec =
-                       dyn_cast<WordLiteralInputSection>(subsection.isec)) {
-          if (in.wordLiteralSection->inputOrder == UnspecifiedInputOrder)
-            in.wordLiteralSection->inputOrder = inputOrder++;
-          in.wordLiteralSection->addInput(isec);
-        } else {
-          llvm_unreachable("unexpected input section kind");
-        }
-      }
+      for (const Subsection &subsection : section->subsections)
+        addInputSection(subsection.isec);
     }
     if (!file->objCImageInfo.empty())
       in.objCImageInfo->addFile(file);
   }
-  assert(inputOrder <= UnspecifiedInputOrder);
 }
 
 static void foldIdenticalLiterals() {
diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index 8f5affb1dc21d8..376e426e7c4f71 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -38,6 +38,43 @@ static_assert(sizeof(void *) != 8 ||
 
 std::vector<ConcatInputSection *> macho::inputSections;
 
+void lld::macho::addInputSection(InputSection *inputSection) {
+  // Used across function calls to impose section ordering
+  static uint64_t inputOrder = 0;
+
+  if (auto *isec = dyn_cast<ConcatInputSection>(inputSection)) {
+    if (isec->isCoalescedWeak())
+      return;
+    if (config->emitInitOffsets &&
+        sectionType(isec->getFlags()) == S_MOD_INIT_FUNC_POINTERS) {
+      in.initOffsets->addInput(isec);
+      return;
+    }
+    isec->outSecOff = inputOrder++;
+    auto *osec = ConcatOutputSection::getOrCreateForInput(isec);
+    isec->parent = osec;
+    inputSections.push_back(isec);
+  } else if (auto *isec = dyn_cast<CStringInputSection>(inputSection)) {
+    if (isec->getName() == section_names::objcMethname) {
+      if (in.objcMethnameSection->inputOrder == UnspecifiedInputOrder)
+        in.objcMethnameSection->inputOrder = inputOrder++;
+      in.objcMethnameSection->addInput(isec);
+    } else {
+      if (in.cStringSection->inputOrder == UnspecifiedInputOrder)
+        in.cStringSection->inputOrder = inputOrder++;
+      in.cStringSection->addInput(isec);
+    }
+  } else if (auto *isec = dyn_cast<WordLiteralInputSection>(inputSection)) {
+    if (in.wordLiteralSection->inputOrder == UnspecifiedInputOrder)
+      in.wordLiteralSection->inputOrder = inputOrder++;
+    in.wordLiteralSection->addInput(isec);
+  } else {
+    llvm_unreachable("unexpected input section kind");
+  }
+
+  assert(inputOrder <= UnspecifiedInputOrder);
+}
+
 uint64_t InputSection::getFileSize() const {
   return isZeroFill(getFlags()) ? 0 : getSize();
 }
diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index b25f0638f4c6cb..d1b60e7a7ebae9 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -369,6 +369,7 @@ constexpr const char addrSig[] = "__llvm_addrsig";
 
 } // namespace section_names
 
+void addInputSection(InputSection *inputSection);
 } // namespace macho
 
 std::string toString(const macho::InputSection *);
diff --git a/lld/MachO/ObjC.cpp b/lld/MachO/ObjC.cpp
index 9b2446360e4f7f..83bbd3d014aa79 100644
--- a/lld/MachO/ObjC.cpp
+++ b/lld/MachO/ObjC.cpp
@@ -8,6 +8,7 @@
 
 #include "ObjC.h"
 #include "ConcatOutputSection.h"
+#include "Driver.h"
 #include "InputFiles.h"
 #include "InputSection.h"
 #include "Layout.h"
@@ -785,7 +786,7 @@ void ObjcCategoryMerger::emitAndLinkProtocolList(
       infoCategoryWriter.catPtrListInfo.align);
   listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
   listSec->live = true;
-  allInputSections.push_back(listSec);
+  addInputSection(listSec);
 
   listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
 
@@ -843,7 +844,7 @@ void ObjcCategoryMerger::emitAndLinkPointerList(
       infoCategoryWriter.catPtrListInfo.align);
   listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
   listSec->live = true;
-  allInputSections.push_back(listSec);
+  addInputSection(listSec);
 
   listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
 
@@ -884,7 +885,7 @@ ObjcCategoryMerger::emitCatListEntrySec(const std::string &forCateogryName,
                                bodyData, infoCategoryWriter.catListInfo.align);
   newCatList->parent = infoCategoryWriter.catListInfo.outputSection;
   newCatList->live = true;
-  allInputSections.push_back(newCatList);
+  addInputSection(newCatList);
 
   newCatList->parent = infoCategoryWriter.catListInfo.outputSection;
 
@@ -922,7 +923,7 @@ Defined *ObjcCategoryMerger::emitCategoryBody(const std::string &name,
                                bodyData, infoCategoryWriter.catBodyInfo.align);
   newBodySec->parent = infoCategoryWriter.catBodyInfo.outputSection;
   newBodySec->live = true;
-  allInputSections.push_back(newBodySec);
+  addInputSection(newBodySec);
 
   std::string symName =
       objc::symbol_names::category + baseClassName + "_$_(" + name + ")";
@@ -1124,7 +1125,7 @@ void ObjcCategoryMerger::generateCatListForNonErasedCategories(
           infoCategoryWriter.catListInfo.align);
       listSec->parent = infoCategoryWriter.catListInfo.outputSection;
       listSec->live = true;
-      allInputSections.push_back(listSec);
+      addInputSection(listSec);
 
       std::string slotSymName = "<__objc_catlist slot for category ";
       slotSymName += nonErasedCatBody->getName();
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 7ee3261ce3075f..1b3694528de1dd 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -793,7 +793,7 @@ void StubHelperSection::setUp() {
 
   in.imageLoaderCache->parent =
       ConcatOutputSection::getOrCreateForInput(in.imageLoaderCache);
-  inputSections.push_back(in.imageLoaderCache);
+  addInputSection(in.imageLoaderCache);
   // Since this isn't in the symbol table or in any input file, the noDeadStrip
   // argument doesn't matter.
   dyldPrivate =
@@ -855,7 +855,7 @@ ConcatInputSection *ObjCSelRefsSection::makeSelRef(StringRef methname) {
                                 /*addend=*/static_cast<int64_t>(methnameOffset),
                                 /*referent=*/in.objcMethnameSection->isec});
   objcSelref->parent = ConcatOutputSection::getOrCreateForInput(objcSelref);
-  inputSections.push_back(objcSelref);
+  addInputSection(objcSelref);
   objcSelref->isFinal = true;
   methnameToSelref[CachedHashStringRef(methname)] = objcSelref;
   return objcSelref;



More information about the llvm-commits mailing list