[LLD][COFF] Partial sections

[aganea]

Summary

As suggested by @rnk here, I persisted the Map variable, previously in lld::coff::Writer::createSections(), now as a member in class lld::coff::Writer and converted its data into a PartialSection.

PartialSections are simply sections that contribute to an OutputSection.
This is needed later by D54802 to generate COFF groups in the * Linker * module.

I used a std::set below instead of a DenseSet because we need to retain a sorted order when enumerating.

[rnk]

I used a std::map below instead of a DenseHash because we need to retain a sorted order when enumerating.

Yep, I looked at removing this before. It might actually be worth it for mingw, because GCC outputs sections named like ".text$_Z3foov", so you can imagine that this map is going to have one entry per symbol, so one extra memory allocation per symbol.

@mstorsjo, I assume that ld.bfd doesn't actually sort the .text (and .pdata and .xdata) section contributions alphabetically. Should we drop the "$_Z3foov" portion of the object file section name before making the unmerged section? It might matter for performance.

COFF/Writer.cpp

335

LLVM generally tries to keep anon namespaces small and use static instead:
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

619–620

What is NC supposed to reference here and below?

703

Eh, this is small, since it's only the real output sections of the PE. I guess it doesn't matter if we use std::map for simplicity.

[rnk]

I just hit send before saying that, at a high level, looks good, but we should check with @ruiu and @pcc about the terminology.

[mstorsjo]

In D55293#1328930, @rnk wrote:

Yep, I looked at removing this before. It might actually be worth it for mingw, because GCC outputs sections named like ".text$_Z3foov", so you can imagine that this map is going to have one entry per symbol, so one extra memory allocation per symbol.

@mstorsjo, I assume that ld.bfd doesn't actually sort the .text (and .pdata and .xdata) section contributions alphabetically. Should we drop the "$_Z3foov" portion of the object file section name before making the unmerged section? It might matter for performance.

ld.bfd does sort the section contributions alphabetically. I tested with a test snippet like this:

$ cat test.cpp
inline int foo1() {
    return 1;
}
inline int foo3() {
    return 3;
}
inline int foo2() {
    return 2;
}
int main(int argc, char* argv[]) {
    return foo1() + foo3() + foo2();
}
$ x86_64-w64-mingw32-g++ test.cpp -c
$ x86_64-w64-mingw32-g++ test.o -o test.exe -Wl,-Map,map.txt
$ cat map.txt
...
 .text          0x0000000000402da0        0x0 /Users/martin/mingw64/lib/gcc/x86_64-w64-mingw32/5.4.0/crtend.o
 *(SORT(.text$*))
 .text$_Z4foo1v
                0x0000000000402da0       0x10 test.o
                0x0000000000402da0                foo1()
 .text$_Z4foo2v
                0x0000000000402db0       0x10 test.o
                0x0000000000402db0                foo2()
 .text$_Z4foo3v
                0x0000000000402dc0       0x10 test.o
                0x0000000000402dc0                foo3()

So it does seem to be sorted, intentionally.

COFF/Writer.cpp

222

I think the parameter is unnecessary when converting this from a static function to a method, as the class already has got the same IdataContents Idata member.

674–687

I'd maybe name it ImportTableSec or so - everything in .idata is relating to imports so ImportSec is a bit generic as it would go for all of them. This one specifically refers to the sections that ImportTableStart/ImportTableSize refers to.

COFF/Writer.h

82 ↗

(On Diff #176699)

Maybe leave out the ContribSections part from this patch - as far as I see it's not actually needed/used at this stage yet, and introducing it here makes the patch a bit more confusing.

[ruiu]

COFF/Writer.h

82 ↗

(On Diff #176699)

I don't think I understand the point of this change. Should the fact that some section is not assigned to some output section be a C++ class? It seems to me that that's a bit weird concept to be a class. Also, OutputSection has a list of unmerged sections, which seems odd to me too because once they are added, they are no longer unmerged.

I guess that first of all, I don't understand the intention of this change. Are you changing the code so that you can handle sections in input files and synthetic sections in the same way?

[rnk]

@ruiu, the motivation is to have some abstraction over the collection of chunks before "merging" section names. So, this would allow us to enumerate all chunks contributing to the .CRT$XCU section before it is concatenated with all the other .CRT$* sections and then merged into the .rdata section by our builtin /merge: rule. We accumulate those sections in a plain vector currently, and I felt it would be useful to have some name for that grouping.

[ruiu]

Is .CRT$XCU just an example, or do you want to do something special for .CRT$XCU? If latter is the case, have you considered just directly implementing what you want do?

COFF/Writer.h

82 ↗

(On Diff #176699)

It seems you are not using this member.

[rnk]

In D55293#1333805, @ruiu wrote:

Is .CRT$XCU just an example, or do you want to do something special for .CRT$XCU? If latter is the case, have you considered just directly implementing what you want do?

It is just an example. I believe the PDB has this information for all unmerged sections.

[aganea]

COFF/Writer.cpp

703

This is essentially due to the new NameAndChars structure. If we want to use a (Small)DenseMap here, it needs a bit of boilerplate for DenseMapInfo<NameAndChars>, only to be used here. I don't think space or perf. will be an issue here.

COFF/Writer.h

82 ↗

(On Diff #176699)

You are right, this member shouldn't be part of the current patch, I will remove it.
@ruiu The overall intention of this patch is to support Coff Groups, as proposed in D54802.

Mod 0003 | `* Linker *`:
[...]
   588 | S_SECTION [size = 28] `.text`
         length = 38, alignment = 12, rva = 4096, section # = 1
         characteristics =
           code
           execute permissions
           read permissions
   616 | S_COFFGROUP [size = 24] `.text`
         length = 8, addr = 0001:0000
         characteristics =
           code
           execute permissions
           read permissions
   640 | S_SECTION [size = 28] `.rdata`
         length = 61, alignment = 12, rva = 8192, section # = 2
         characteristics =
           initialized data
           read permissions
   668 | S_SECTION [size = 28] `.idata`
         length = 145, alignment = 12, rva = 12288, section # = 3
         characteristics =
           initialized data
           read permissions
   696 | S_COFFGROUP [size = 28] `.idata$2`
         length = 40, addr = 0003:0000
         characteristics =
           initialized data
           read permissions
           write permissions
   724 | S_COFFGROUP [size = 28] `.idata$4`
         length = 24, addr = 0003:0040
         characteristics =
           initialized data
           read permissions
           write permissions
   752 | S_COFFGROUP [size = 28] `.idata$5`
         length = 24, addr = 0003:0064
         characteristics =
           initialized data
           read permissions
           write permissions
   780 | S_COFFGROUP [size = 28] `.idata$6`
         length = 24, addr = 0003:0088
         characteristics =
           initialized data
           read permissions
           write permissions
   808 | S_COFFGROUP [size = 28] `.idata$7`
         length = 33, addr = 0003:0112
         characteristics =
           initialized data
           read permissions
           write permissions

Without this, the unmerged sections are otherwise lost in Writer::mergeSections(), and are not available later in PDBLinker::addSections().

[aganea]

COFF/Writer.cpp

619–620

It's a std::pair<NameAndChars, UnmergedSection *>. I changed it back to auto &Pair like it was before.

674–687

I changed the .idata$2 reference to ImportDirs , and .idata$5 reference to ImportAddresses. Does that sound better?

[mstorsjo]

COFF/Writer.cpp

674–687

Sounds good to me.

[aganea]

Gentle ping. Any further comments? Thank you in advance for taking the time to review this.

[aganea]

Hi @ruiu ! This is waiting for your final review / approval. Could you possibly take a quick look please? Thank you!

[rnk]

In D55293#1346895, @aganea wrote:

Hi @ruiu ! This is waiting for your final review / approval. Could you possibly take a quick look please? Thank you!

@ruiu, ping

[ruiu]

COFF/Writer.cpp

161

Format.

165

NC -> Other by convention.

169

nit: I'd stick with < when defining operator<. You can just return the result of the comparison of characteristics.

if (C == 0)
 return Characteristics < NC.Characteritics.

611

80 cols?

I recommend you use clang-format.

724–725

Why did you have to rename the function?

COFF/Writer.h

28–38 ↗

(On Diff #178652)

Since no one except one .cpp file uses this class declaration, you don't need to write it in a header. Please move to the .cpp file.

30 ↗

(On Diff #178652)

It feels that UnmergedSection is still a bit weird name. You don't remove members from "Unmerged" once they are "merged" right?

37 ↗

(On Diff #178652)

You have an unused member variable.

[aganea]

Updated following @ruiu 's comments - thank you!

I wasn't happy with the UnmergedSection name either - I changed that into PartialSection. If could also be PartialOutSection if we want to better designate the destination.

I also removed the struct NameAndChars because the concept was overlapping with data in PartialSection; that changed the std::map into a std::set and a custom comparer which uses part of PartialSection 's data as the "key".

COFF/Writer.cpp

724–725

Reverted.

COFF/Writer.h

30 ↗

(On Diff #178652)

What about PartialSection?

[rnk]

lgtm I like the new name. I think this is an overall readability improvement.

[ruiu]

COFF/Writer.cpp

166

Now you can refer this class as just StringRef instead of llvm::StringRef.

182

Can this null condition happen?

[aganea]

COFF/Writer.cpp

182

No, it cannot. I can transform the null checks into an assert, if you’re fine with that? The intent will be better expressed.

[ruiu]

COFF/Writer.cpp

182

I don't think you even have to have a null check. Dereferencing a pointer clearly delivers the intent that the pointer cannot be null.

COFF/Writer.cpp

void writeTo(uint8_t *B) const override {
     if (!Config->PDBAltPath.empty())
       memcpy(P, Config->PDBAltPath.data(), Config->PDBAltPath.size());
     P[Config->PDBAltPath.size()] = '\0';
   }
 
   mutable codeview::DebugInfo *BuildId = nullptr;
 };
 
+// PartialSection represents a group of chunks that contribute to an
+// OutputSection. Collating a collection of PartialSections of same name and
+// characteristics constitutes the OutputSection.
+class PartialSection {
+public:
+  PartialSection(StringRef N, uint32_t Chars)
+      : Name(N), Characteristics(Chars) {}
+  StringRef Name;
+  unsigned Characteristics;
+  std::vector<Chunk *> Chunks;
+
+  bool operator<(const PartialSection &Other) const {
+    int C = Name.compare(Other.Name);
+    if (C == 1)
+      return false;
+    if (C == 0)
+      return Characteristics < Other.Characteristics;
+    return true;
+  }
+};
+
+struct PartialLess {
+  bool operator()(PartialSection *L, PartialSection *R) const {
+    return *L < *R;
+  }
+};
+
 // The writer writes a SymbolTable result to a file.
 class Writer {
 public:
   Writer() : Buffer(errorHandler().OutputBuffer) {}
   void run();
 
 private:
   void createSections();
   void createMiscChunks();
   void createImportTables();
   void appendImportThunks();
-  void locateImportTables(
-      std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map);
+  void locateImportTables();
   void createExportTable();
   void mergeSections();
   void readRelocTargets();
   void removeUnusedSections();
   void assignAddresses();
   void finalizeAddresses();
   void removeEmptySections();
   void createSymbolAndStringTable();
   void openFile(StringRef OutputPath);
   template <typename PEHeaderTy> void writeHeader();
   void createSEHTable();
   void createRuntimePseudoRelocs();
   void insertCtorDtorSymbols();
   void createGuardCFTables();
   void markSymbolsForRVATable(ObjFile *File,
                               ArrayRef<SectionChunk *> SymIdxChunks,
                               SymbolRVASet &TableSymbols);
   void maybeAddRVATable(SymbolRVASet TableSymbols, StringRef TableSym,
                         StringRef CountSym);
   void setSectionPermissions();
   void writeSections();
   void writeBuildId();
   void sortExceptionTable();
   void sortCRTSectionChunks(std::vector<Chunk *> &Chunks);
+  void addSyntheticIdata();
+  bool fixGnuImportChunks();
+  PartialSection *createPartialSection(StringRef Name, uint32_t OutChars);
+  PartialSection *findPartialSection(StringRef Name, uint32_t OutChars);
 
   llvm::Optional<coff_symbol16> createSymbol(Defined *D);
   size_t addEntryToStringTable(StringRef Str);
 
   OutputSection *findSection(StringRef Name);
   void addBaserels();
   void addBaserelBlocks(std::vector<Baserel> &V);
 
   uint32_t getSizeOfInitializedData();
-  std::map<StringRef, std::vector<DefinedImportData *>> binImports();
 
   std::unique_ptr<FileOutputBuffer> &Buffer;
+  std::set<PartialSection *, PartialLess> PartialSections;
   std::vector<OutputSection *> OutputSections;
   std::vector<char> Strtab;
   std::vector<llvm::object::coff_symbol16> OutputSymtab;
   IdataContents Idata;
   Chunk *ImportTableStart = nullptr;
   uint64_t ImportTableSize = 0;
   Chunk *IATStart = nullptr;
   uint64_t IATSize = 0;
if (StringTableOff) {
     strncpy(Hdr->Name, Name.data(),
             std::min(Name.size(), (size_t)COFF::NameSize));
   }
 }
 
 } // namespace coff
 } // namespace lld
 
 // Check whether the target address S is in range from a relocation
 // of type RelType at address P.
 static bool isInRange(uint16_t RelType, uint64_t S, uint64_t P, int Margin) {
   assert(Config->Machine == ARMNT);
   int64_t Diff = AbsoluteDifference(S, P + 4) + Margin;
   switch (RelType) {
   case IMAGE_REL_ARM_BRANCH20T:
     return isInt<21>(Diff);
   case IMAGE_REL_ARM_BRANCH24T:

 // GNU binutils doesn't use short import files, but instead produces import
 // libraries that consist of object files, with section chunks for the .idata$*
 // sections. These are linked just as regular static libraries. Each import
 // library consists of one header object, one object file for every imported
 // symbol, and one trailer object. In order for the .idata tables/lists to
 // be formed correctly, the section chunks within each .idata$* section need
 // to be grouped by library, and sorted alphabetically within each library
 // (which makes sure the header comes first and the trailer last).
-static bool fixGnuImportChunks(
-    std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map) {
+bool Writer::fixGnuImportChunks() {
   uint32_t RDATA = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ;
 
   // Make sure all .idata$* section chunks are mapped as RDATA in order to
   // be sorted into the same sections as our own synthesized .idata chunks.
-  for (auto &Pair : Map) {
-    StringRef SectionName = Pair.first.first;
-    uint32_t OutChars = Pair.first.second;
-    if (!SectionName.startswith(".idata"))
-      continue;
-    if (OutChars == RDATA)
-      continue;
-    std::vector<Chunk *> &SrcVect = Pair.second;
-    std::vector<Chunk *> &DestVect = Map[{SectionName, RDATA}];
-    DestVect.insert(DestVect.end(), SrcVect.begin(), SrcVect.end());
-    SrcVect.clear();
+  for (PartialSection *PSec : PartialSections) {
+    if (!PSec->Name.startswith(".idata"))
+      continue;
+    if (PSec->Characteristics == RDATA)
+      continue;
+    PartialSection *RDataSec = createPartialSection(PSec->Name, RDATA);
+    RDataSec->Chunks.insert(RDataSec->Chunks.end(), PSec->Chunks.begin(),
+                            PSec->Chunks.end());
+    PSec->Chunks.clear();
   }
 
   bool HasIdata = false;
   // Sort all .idata$* chunks, grouping chunks from the same library,
   // with alphabetical ordering of the object fils within a library.
-  for (auto &Pair : Map) {
-    StringRef SectionName = Pair.first.first;
-    if (!SectionName.startswith(".idata"))
+  for (PartialSection *PSec : PartialSections) {
+    if (!PSec->Name.startswith(".idata"))
       continue;
 
-    std::vector<Chunk *> &Chunks = Pair.second;
+    std::vector<Chunk *> &Chunks = PSec->Chunks;
     if (!Chunks.empty())
       HasIdata = true;
     std::stable_sort(Chunks.begin(), Chunks.end(), [&](Chunk *S, Chunk *T) {
       SectionChunk *SC1 = dyn_cast_or_null<SectionChunk>(S);
       SectionChunk *SC2 = dyn_cast_or_null<SectionChunk>(T);
       if (!SC1 || !SC2) {
         // if SC1, order them ascending. If SC2 or both null,
         // S is not less than T.
std::stable_sort(Chunks.begin(), Chunks.end(), [&](Chunk *S, Chunk *T) {
       return Key1 < Key2;
     });
   }
   return HasIdata;
 }
 
 // Add generated idata chunks, for imported symbols and DLLs, and a
 // terminator in .idata$2.
-static void addSyntheticIdata(
-    IdataContents &Idata,
-    std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map) {
+void Writer::addSyntheticIdata() {
   uint32_t RDATA = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ;
   Idata.create();
 
   // Add the .idata content in the right section groups, to allow
   // chunks from other linked in object files to be grouped together.
   // See Microsoft PE/COFF spec 5.4 for details.
   auto Add = [&](StringRef N, std::vector<Chunk *> &V) {
-    std::vector<Chunk *> &DestVect = Map[{N, RDATA}];
-    DestVect.insert(DestVect.end(), V.begin(), V.end());
+    PartialSection *PSec = createPartialSection(N, RDATA);
+    PSec->Chunks.insert(PSec->Chunks.end(), V.begin(), V.end());
   };
 
   // The loader assumes a specific order of data.
   // Add each type in the correct order.
   Add(".idata$2", Idata.Dirs);
   Add(".idata$4", Idata.Lookups);
   Add(".idata$5", Idata.Addresses);
   Add(".idata$6", Idata.Hints);
   Add(".idata$7", Idata.DLLNames);
 }
 
 // Locate the first Chunk and size of the import directory list and the
 // IAT.
-void Writer::locateImportTables(
-    std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map) {
+void Writer::locateImportTables() {
   uint32_t RDATA = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ;
-  std::vector<Chunk *> &ImportTables = Map[{".idata$2", RDATA}];
-  if (!ImportTables.empty())
-    ImportTableStart = ImportTables.front();
-  for (Chunk *C : ImportTables)
+
+  if (PartialSection *ImportDirs = findPartialSection(".idata$2", RDATA)) {
+    if (!ImportDirs->Chunks.empty())
+      ImportTableStart = ImportDirs->Chunks.front();
+    for (Chunk *C : ImportDirs->Chunks)
       ImportTableSize += C->getSize();
+  }
 
-  std::vector<Chunk *> &IAT = Map[{".idata$5", RDATA}];
-  if (!IAT.empty())
-    IATStart = IAT.front();
-  for (Chunk *C : IAT)
+  if (PartialSection *ImportAddresses = findPartialSection(".idata$5", RDATA)) {
+    if (!ImportAddresses->Chunks.empty())
+      IATStart = ImportAddresses->Chunks.front();
+    for (Chunk *C : ImportAddresses->Chunks)
       IATSize += C->getSize();
   }
+}
 
 // Create output section objects and add them to OutputSections.
 void Writer::createSections() {
   // First, create the builtin sections.
   const uint32_t DATA = IMAGE_SCN_CNT_INITIALIZED_DATA;
   const uint32_t BSS = IMAGE_SCN_CNT_UNINITIALIZED_DATA;
   const uint32_t CODE = IMAGE_SCN_CNT_CODE;
   const uint32_t DISCARDABLE = IMAGE_SCN_MEM_DISCARDABLE;
   const uint32_t R = IMAGE_SCN_MEM_READ;
   const uint32_t W = IMAGE_SCN_MEM_WRITE;
   const uint32_t X = IMAGE_SCN_MEM_EXECUTE;
 
   SmallDenseMap<std::pair<StringRef, uint32_t>, OutputSection *> Sections;
   auto CreateSection = [&](StringRef Name, uint32_t OutChars) {
     OutputSection *&Sec = Sections[{Name, OutChars}];
     if (!Sec) {
       Sec = make<OutputSection>(Name, OutChars);
       OutputSections.push_back(Sec);
     }
     return Sec;
   };
 
   // Try to match the section order used by link.exe.
   TextSec = CreateSection(".text", CODE | R | X);
   CreateSection(".bss", BSS | R | W);
   RdataSec = CreateSection(".rdata", DATA | R);
   BuildidSec = CreateSection(".buildid", DATA | R);
   DataSec = CreateSection(".data", DATA | R | W);
   PdataSec = CreateSection(".pdata", DATA | R);
   IdataSec = CreateSection(".idata", DATA | R);
   EdataSec = CreateSection(".edata", DATA | R);
   DidatSec = CreateSection(".didat", DATA | R);
   RsrcSec = CreateSection(".rsrc", DATA | R);
   RelocSec = CreateSection(".reloc", DATA | DISCARDABLE | R);
   CtorsSec = CreateSection(".ctors", DATA | R | W);
   DtorsSec = CreateSection(".dtors", DATA | R | W);
 
   // Then bin chunks by name and output characteristics.
-  std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> Map;
   for (Chunk *C : Symtab->getChunks()) {
     auto *SC = dyn_cast<SectionChunk>(C);
     if (SC && !SC->Live) {
       if (Config->Verbose)
         SC->printDiscardedMessage();
       continue;
     }
-    Map[{C->getSectionName(), C->getOutputCharacteristics()}].push_back(C);
+    PartialSection *PSec = createPartialSection(C->getSectionName(),
+                                                C->getOutputCharacteristics());
+    PSec->Chunks.push_back(C);
   }
 
   // Even in non MinGW cases, we might need to link against GNU import
   // libraries.
-  bool HasIdata = fixGnuImportChunks(Map);
+  bool HasIdata = fixGnuImportChunks();
   if (!Idata.empty())
     HasIdata = true;
 
   if (HasIdata)
-    addSyntheticIdata(Idata, Map);
+    addSyntheticIdata();
 
   // Process an /order option.
   if (!Config->Order.empty())
-    for (auto &Pair : Map)
-      sortBySectionOrder(Pair.second);
+    for (PartialSection *PSec : PartialSections)
+      sortBySectionOrder(PSec->Chunks);
 
   if (HasIdata)
-    locateImportTables(Map);
+    locateImportTables();
 
   // Then create an OutputSection for each section.
   // '$' and all following characters in input section names are
   // discarded when determining output section. So, .text$foo
   // contributes to .text, for example. See PE/COFF spec 3.2.
-  for (auto &Pair : Map) {
-    StringRef Name = getOutputSectionName(Pair.first.first);
-    uint32_t OutChars = Pair.first.second;
+  for (PartialSection *PSec : PartialSections) {
+    StringRef Name = getOutputSectionName(PSec->Name);
+    uint32_t OutChars = PSec->Characteristics;
 
     if (Name == ".CRT") {
       // In link.exe, there is a special case for the I386 target where .CRT
       // sections are treated as if they have output characteristics DATA | R if
       // their characteristics are DATA | R | W. This implements the same
       // special case for all architectures.
       OutChars = DATA | R;
 
-      log("Processing section " + Pair.first.first + " -> " + Name);
+      log("Processing section " + PSec->Name + " -> " + Name);
 
-      sortCRTSectionChunks(Pair.second);
+      sortCRTSectionChunks(PSec->Chunks);
     }
 
     OutputSection *Sec = CreateSection(Name, OutChars);
-    std::vector<Chunk *> &Chunks = Pair.second;
-    for (Chunk *C : Chunks)
+    for (Chunk *C : PSec->Chunks)
       Sec->addChunk(C);
   }
 
   // Finally, move some output sections to the end.
   auto SectionOrder = [&](OutputSection *S) {
     // Move DISCARDABLE (or non-memory-mapped) sections to the end of file because
     // the loader cannot handle holes. Stripping can remove other discardable ones
     // than .reloc, which is first of them (created early).
for (size_t E = V.size(); J < E; ++J) {
     RelocSec->addChunk(make<BaserelChunk>(Page, &V[I], &V[0] + J));
     I = J;
     Page = P;
   }
   if (I == J)
     return;
   RelocSec->addChunk(make<BaserelChunk>(Page, &V[I], &V[0] + J));
 }
+
+PartialSection *Writer::createPartialSection(StringRef Name,
+                                             uint32_t OutChars) {
+  PartialSection *PSec = findPartialSection(Name, OutChars);
+  if (PSec)
+    return PSec;
+  PSec = make<PartialSection>(Name, OutChars);
+  PartialSections.insert(PSec);
+  return PSec;
+}
+
+PartialSection *Writer::findPartialSection(StringRef Name, uint32_t OutChars) {
+  auto It = find_if(PartialSections, [&](PartialSection *P) {
+    return P->Name == Name && P->Characteristics == OutChars;
+  });
+  if (It != PartialSections.end())
+    return *It;
+  return nullptr;
+}