[ELF] LinkerScript: Implement custom output section factory

[evgeny777]

Summary

This patch implements extra output section factory for use in LinkerScript class. It attempts to merge input sections with the same name and different flags. When merge is impossible, error is raised.

See discussion here:
https://reviews.llvm.org/D22683

[grimar]

ELF/LinkerScript.cpp

61

You moved this and some other code. Was there a reason for that ? It creates some noise in the patch.

135

Is it intended ` ?

139

Ditto.

ELF/OutputSections.h

681

Doesn't seem you destroy factory using base class pointer. Virtual descructor is excessive.

683

Why not = 0 ?

705

Missing override keyword.

ELF/Writer.cpp

663

May be would be better just initialize Writer::Factory differently depending on whether script is used or not ?

[evgeny777]

Updated diff, so now it's little less noisy.
The main problem is that lot of code was moved from LinkerScript to OutputSectionFactory, so it's hard to make it look pretty

[ruiu]

Honestly this does not seem to simplify things. It may, but it also introduces new complexities. We now have BaseFactory and two Factory classes, and we are doing some object-oriented juggling there. Not sure if we can get rid of it (maybe we can just remove the base class and have two classes?) But at least for https://reviews.llvm.org/D22683, I'd say we want to just use the existing factory if this patch can't be simplified.

[evgeny777]

Honestly, I don't know how to radically simplify this. Using two separate classes involves code duplication:

• Output section creation (see OutputSectionFactoryBase::create)
• Handling .opd, .init_array and .fini_array (See Writer::finalizeSections)
• OwningSections vector (minor issue, but still ...)

So, I also think it makes sense to proceed with https://reviews.llvm.org/D22683 without this change.

ELF/Writer.cpp

663

Not sure. You'll have to maintain two different indexes with the same class.

[grimar]

Btw currenly when linking FreeBSD kernel LLD produces 7(!) different .rodata sections:

[ 6] .rodata           PROGBITS         ffffffff80c87c8c  00c86c8c
     00000000000e6e4d  0000000000000001 AMS       0     0     1
[ 7] .rodata           PROGBITS         ffffffff80d6eae0  00d6dae0
     0000000000246f00  0000000000000000   A       0     0     32
[ 8] .rodata           PROGBITS         ffffffff80fb59e0  00fb49e0
     00000000000001f0  0000000000000010  AM       0     0     16
[ 9] .rodata           PROGBITS         ffffffff80fb5bd0  00fb4bd0
     00000000000002a0  0000000000000004 AMS       0     0     16
[10] .rodata           PROGBITS         ffffffff80fb5e70  00fb4e70
     0000000000000108  0000000000000008  AM       0     0     8
[11] .rodata           PROGBITS         ffffffff80fb5f78  00fb4f78
     000000000000000c  0000000000000004 AMS       0     0     4
[12] .rodata           PROGBITS         ffffffff80fb5f88  00fb4f88
     0000000000001158  0000000000000001 AMS       0     0     8

While ld produces only one:
[ 6] .rodata PROGBITS ffffffff80e85100 00c85100

0000000000325aaa  0000000000000000   A       0     0     32

[davide]

In D22960#508621, @grimar wrote:

Btw currenly when linking FreeBSD kernel LLD produces 7(!) different .rodata sections:

[ 6] .rodata           PROGBITS         ffffffff80c87c8c  00c86c8c
     00000000000e6e4d  0000000000000001 AMS       0     0     1
[ 7] .rodata           PROGBITS         ffffffff80d6eae0  00d6dae0
     0000000000246f00  0000000000000000   A       0     0     32
[ 8] .rodata           PROGBITS         ffffffff80fb59e0  00fb49e0
     00000000000001f0  0000000000000010  AM       0     0     16
[ 9] .rodata           PROGBITS         ffffffff80fb5bd0  00fb4bd0
     00000000000002a0  0000000000000004 AMS       0     0     16
[10] .rodata           PROGBITS         ffffffff80fb5e70  00fb4e70
     0000000000000108  0000000000000008  AM       0     0     8
[11] .rodata           PROGBITS         ffffffff80fb5f78  00fb4f78
     000000000000000c  0000000000000004 AMS       0     0     4
[12] .rodata           PROGBITS         ffffffff80fb5f88  00fb4f88
     0000000000001158  0000000000000001 AMS       0     0     8

While ld produces only one:
[ 6] .rodata PROGBITS ffffffff80e85100 00c85100

0000000000325aaa  0000000000000000   A       0     0     32

I noticed the same. This is likely to be a bug. Many loadable sections are also not put in a PT_LOAD. And this makes the loader very angry.

[ruiu]

It means that we need to have a separate logic than the regular one to create output sections for the linker script, right?

[davide]

In D22960#509021, @ruiu wrote:

It means that we need to have a separate logic than the regular one to create output sections for the linker script, right?

Yes. I think so. Large part of the headache derives from the fact that ld.bfd is actually linker-script driven in any case.

[ruiu]

Yup. My gut is in many cases we want to separate code paths for linker scripts and non-linker scripts even if they can share code. It's because the complexity to fix the impedance mismatch between the two code paths tend to surpass the benefit of sharing a small amount of code.

[evgeny777]

Output sections are now merged in LinkerScript<ELFT>::createInputSectionList

ELF/LinkerScript.h

 //===- LinkerScript.h -------------------------------------------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_ELF_LINKER_SCRIPT_H
 #define LLD_ELF_LINKER_SCRIPT_H
 
+#include "OutputSections.h"
 #include "Writer.h"
 #include "lld/Core/LLVM.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <functional>
 
 namespace lld {
 namespace elf {
 class SymbolBody;
 template <class ELFT> class InputSectionBase;
 template <class ELFT> class OutputSectionBase;
-template <class ELFT> class OutputSectionFactory;
+template <class ELFT> class OutputSectionFactoryBase;
 template <class ELFT> class DefinedCommon;
 
 typedef std::function<uint64_t(uint64_t)> Expr;
 
 // Parses a linker script. Calling this function updates
 // Config and ScriptConfig.
 void readLinkerScript(MemoryBufferRef MB);
 
struct ScriptConfiguration {
 
   // List of section patterns specified with KEEP commands. They will
   // be kept even if they are unused and --gc-sections is specified.
   std::vector<StringRef> KeptSections;
 };
 
 extern ScriptConfiguration *ScriptConfig;
 
+// Output section factory used by linker script processor.
+template <class ELFT>
+class OutputSectionBuilder : public OutputSectionFactoryBase<ELFT> {
+  typedef typename ELFT::uint uintX_t;
+
+public:
+  void addMultiple(StringRef OutputName, const InputSectionDescription *I);
+  void addSingle(StringRef OutputName, InputSectionBase<ELFT> *S);
+  std::vector<OutputSectionBase<ELFT> *> finish();
+  OutputSectionBase<ELFT> *lookup(StringRef Name, uint32_t Type,
+                                  uintX_t Flags) override;
+
+private:
+  ScriptConfiguration &Opt = *ScriptConfig;
+  llvm::SmallDenseMap<StringRef, OutputSectionBase<ELFT> *> Sections;
+};
+
 // This is a runner of the linker script.
 template <class ELFT> class LinkerScript {
   typedef typename ELFT::uint uintX_t;
 
 public:
-  std::vector<OutputSectionBase<ELFT> *>
-  createSections(OutputSectionFactory<ELFT> &Factory);
+  std::vector<OutputSectionBase<ELFT> *> createSections();
 
   std::vector<PhdrEntry<ELFT>>
   createPhdrs(ArrayRef<OutputSectionBase<ELFT> *> S);
 
   ArrayRef<uint8_t> getFiller(StringRef Name);
   bool shouldKeep(InputSectionBase<ELFT> *S);
   void assignAddresses(ArrayRef<OutputSectionBase<ELFT> *> S);
   int compareSections(StringRef A, StringRef B);
   void addScriptedSymbols();
   bool hasPhdrsCommands();
+  OutputSectionFactoryBase<ELFT> *getFactory();
 
 private:
   std::vector<std::pair<StringRef, const InputSectionDescription *>>
   getSectionMap();
 
-  std::vector<InputSectionBase<ELFT> *>
-  getInputSections(const InputSectionDescription *);
-
   // "ScriptConfig" is a bit too long, so define a short name for it.
   ScriptConfiguration &Opt = *ScriptConfig;
 
   std::vector<OutputSectionBase<ELFT> *>
   filter(std::vector<OutputSectionBase<ELFT> *> &Sections);
 
   int getSectionIndex(StringRef Name);
   std::vector<size_t> getPhdrIndices(StringRef SectionName);
   size_t getPhdrIndex(StringRef PhdrName);
   void dispatchAssignment(SymbolAssignment *Cmd);
 
   uintX_t Dot;
+  std::unique_ptr<OutputSectionBuilder<ELFT>> Builder;
 };
 
 // Variable template is a C++14 feature, so we can't template
 // a global variable. Use a struct to workaround.
 template <class ELFT> struct Script { static LinkerScript<ELFT> *X; };
 template <class ELFT> LinkerScript<ELFT> *Script<ELFT>::X;
 
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/LinkerScript.cpp

 bool OutputSectionCommand::classof(const BaseCommand *C) {
   return C->Kind == OutputSectionKind;
 }
 
 bool InputSectionDescription::classof(const BaseCommand *C) {
   return C->Kind == InputSectionKind;
 }
 
-template <class ELFT> static bool isDiscarded(InputSectionBase<ELFT> *S) {
-  return !S || !S->Live;
-}
-
 template <class ELFT>
-bool LinkerScript<ELFT>::shouldKeep(InputSectionBase<ELFT> *S) {
-  for (StringRef Pat : Opt.KeptSections)
-    if (globMatch(Pat, S->getSectionName()))
-      return true;
-  return false;
+static inline bool isDiscarded(InputSectionBase<ELFT> *S) {
+  return !S || !S->Live;
 }
 
 static bool match(ArrayRef<StringRef> Patterns, StringRef S) {
   for (StringRef Pat : Patterns)
     if (globMatch(Pat, S))
       return true;
   return false;
 }
 
-// Create a vector of (<output section name>, <input section name patterns>).
-// For example, if a returned vector contains (".text" (".foo.*" ".bar.*")),
-// input sections start with ".foo." or ".bar." should be added to
-// ".text" section.
-template <class ELFT>
-std::vector<std::pair<StringRef, const InputSectionDescription *>>
-LinkerScript<ELFT>::getSectionMap() {
-  std::vector<std::pair<StringRef, const InputSectionDescription *>> Ret;
-
-  for (const std::unique_ptr<BaseCommand> &Base1 : Opt.Commands)
-    if (auto *Cmd1 = dyn_cast<OutputSectionCommand>(Base1.get()))
-      for (const std::unique_ptr<BaseCommand> &Base2 : Cmd1->Commands)
-        if (auto *Cmd2 = dyn_cast<InputSectionDescription>(Base2.get()))
-          Ret.emplace_back(Cmd1->Name, Cmd2);
-
-  return Ret;
-}
-
 static bool fileMatches(const InputSectionDescription *Desc,
                         StringRef Filename) {
   if (!globMatch(Desc->FilePattern, Filename))
     return false;
   return Desc->ExcludedFiles.empty() || !match(Desc->ExcludedFiles, Filename);
 }
 
-// Returns input sections filtered by given glob patterns.
 template <class ELFT>
-std::vector<InputSectionBase<ELFT> *>
-LinkerScript<ELFT>::getInputSections(const InputSectionDescription *I) {
+void OutputSectionBuilder<ELFT>::addMultiple(StringRef OutputName,
+                                             const InputSectionDescription *I) {
   ArrayRef<StringRef> Patterns = I->SectionPatterns;
   std::vector<InputSectionBase<ELFT> *> Ret;
   for (const std::unique_ptr<ObjectFile<ELFT>> &F :
        Symtab<ELFT>::X->getObjectFiles()) {
     if (fileMatches(I, sys::path::filename(F->getName())))
       for (InputSectionBase<ELFT> *S : F->getSections())
         if (!isDiscarded(S) && !S->OutSec &&
             match(Patterns, S->getSectionName()))
-          Ret.push_back(S);
+          addSingle(OutputName, S);
   }
-
   if ((llvm::find(Patterns, "COMMON") != Patterns.end()))
-    Ret.push_back(CommonInputSection<ELFT>::X);
-
-  return Ret;
+    addSingle(OutputName, CommonInputSection<ELFT>::X);
 }
 
 template <class ELFT>
-std::vector<OutputSectionBase<ELFT> *>
-LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
-  std::vector<OutputSectionBase<ELFT> *> Ret;
+std::vector<OutputSectionBase<ELFT> *> OutputSectionBuilder<ELFT>::finish() {
+  auto &Sections = OutputSectionFactoryBase<ELFT>::OwningSections;
+  std::vector<OutputSectionBase<ELFT> *> Result;
 
-  // Add input section to output section. If there is no output section yet,
-  // then create it and add to output section list.
-  auto Add = [&](InputSectionBase<ELFT> *C, StringRef Name) {
-    OutputSectionBase<ELFT> *Sec;
-    bool IsNew;
-    std::tie(Sec, IsNew) = Factory.create(C, Name);
-    if (IsNew)
-      Ret.push_back(Sec);
-    Sec->addSection(C);
-  };
-
-  for (auto &P : getSectionMap()) {
-    StringRef OutputName = P.first;
-    const InputSectionDescription *I = P.second;
-    for (InputSectionBase<ELFT> *S : getInputSections(I)) {
-      if (OutputName == "/DISCARD/") {
-        S->Live = false;
-        reportDiscarded(S);
-        continue;
-      }
-      Add(S, OutputName);
-    }
-  }
-
-  // Add all other input sections, which are not listed in script.
-  for (const std::unique_ptr<ObjectFile<ELFT>> &F :
-       Symtab<ELFT>::X->getObjectFiles())
-    for (InputSectionBase<ELFT> *S : F->getSections())
-      if (!isDiscarded(S) && !S->OutSec)
-        Add(S, getOutputSectionName(S));
-
-  // Remove from the output all the sections which did not meet
-  // the optional constraints.
-  return filter(Ret);
-}
-
-// Process ONLY_IF_RO and ONLY_IF_RW.
-template <class ELFT>
-std::vector<OutputSectionBase<ELFT> *>
-LinkerScript<ELFT>::filter(std::vector<OutputSectionBase<ELFT> *> &Sections) {
-  // In this loop, we remove output sections if they don't satisfy
-  // requested properties.
   for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get());
     if (!Cmd || Cmd->Name == "/DISCARD/")
       continue;
 
     if (Cmd->Constraint == ConstraintKind::NoConstraint)
       continue;
 
-    auto It = llvm::find_if(Sections, [&](OutputSectionBase<ELFT> *S) {
+    auto It = llvm::find_if(Sections,
+                            [&](std::unique_ptr<OutputSectionBase<ELFT>> &S) {
                               return S->getName() == Cmd->Name;
                             });
     if (It == Sections.end())
       continue;
 
-    OutputSectionBase<ELFT> *Sec = *It;
+    OutputSectionBase<ELFT> *Sec = (*It).get();
     bool Writable = (Sec->getFlags() & SHF_WRITE);
     bool RO = (Cmd->Constraint == ConstraintKind::ReadOnly);
     bool RW = (Cmd->Constraint == ConstraintKind::ReadWrite);
 
     if ((RO && Writable) || (RW && !Writable))
       Sections.erase(It);
   }
-  return Sections;
+
+  for (std::unique_ptr<OutputSectionBase<ELFT>> &S : Sections)
+    Result.push_back(S.get());
+
+  return Result;
+}
+
+template <class ELFT>
+static void checkCompatibility(OutputSectionBase<ELFT> *Out,
+                               InputSectionBase<ELFT> *In) {
+  const typename ELFT::Shdr *InHdr = In->getSectionHdr();
+
+  if (Out->getType() != InHdr->sh_type)
+    error("failed to mix sections with incompatible types within `" +
+          Out->getName());
+
+  if ((Out->getFlags() & SHF_TLS) != (InHdr->sh_flags & SHF_TLS))
+    error("failed to mix TLS and non-TLS sections within `" + Out->getName());
+}
+
+template <class ELFT>
+void OutputSectionBuilder<ELFT>::addSingle(StringRef OutputName,
+                                           InputSectionBase<ELFT> *S) {
+  if (OutputName == "/DISCARD/") {
+    S->Live = false;
+    reportDiscarded(S);
+    return;
+  }
+
+  OutputSectionBase<ELFT> *&Sec = Sections[OutputName];
+  if (!Sec) {
+    bool IsNew;
+    auto Key = OutputSectionFactoryBase<ELFT>::createKey(S, OutputName);
+    std::tie(Sec, IsNew) = OutputSectionFactoryBase<ELFT>::create(S, Key);
+  }
+  checkCompatibility(Sec, S);
+  if (HasError)
+    return;
+
+  const typename ELFT::Shdr *InHdr = S->getSectionHdr();
+  Sec->setFlags(Sec->getFlags() | InHdr->sh_flags);
+
+  Sec->addSection(S);
+}
+
+template <class ELFT>
+OutputSectionBase<ELFT> *OutputSectionBuilder<ELFT>::lookup(StringRef Name,
+                                                            uint32_t Type,
+                                                            uintX_t Flags) {
+  auto It = Sections.find(Name);
+  if (It == Sections.end())
+    return nullptr;
+
+  OutputSectionBase<ELFT> *Sec = (*It).second;
+
+  return (Sec->getFlags() == Flags) && (Sec->getType() == Type) ? Sec : nullptr;
+}
+
+template <class ELFT>
+bool LinkerScript<ELFT>::shouldKeep(InputSectionBase<ELFT> *S) {
+  for (StringRef Pat : Opt.KeptSections)
+    if (globMatch(Pat, S->getSectionName()))
+      return true;
+  return false;
+}
+
+// Create a vector of (<output section name>, <input section name patterns>).
+// For example, if a returned vector contains (".text" (".foo.*" ".bar.*")),
+// input sections start with ".foo." or ".bar." should be added to
+// ".text" section.
+template <class ELFT>
+std::vector<std::pair<StringRef, const InputSectionDescription *>>
+LinkerScript<ELFT>::getSectionMap() {
+  std::vector<std::pair<StringRef, const InputSectionDescription *>> Ret;
+
+  for (const std::unique_ptr<BaseCommand> &Base1 : Opt.Commands)
+    if (auto *Cmd1 = dyn_cast<OutputSectionCommand>(Base1.get()))
+      for (const std::unique_ptr<BaseCommand> &Base2 : Cmd1->Commands)
+        if (auto *Cmd2 = dyn_cast<InputSectionDescription>(Base2.get()))
+          Ret.emplace_back(Cmd1->Name, Cmd2);
+
+  return Ret;
+}
+
+template <class ELFT>
+std::vector<OutputSectionBase<ELFT> *> LinkerScript<ELFT>::createSections() {
+  std::vector<OutputSectionBase<ELFT> *> Ret;
+
+  Builder = llvm::make_unique<OutputSectionBuilder<ELFT>>();
+  for (auto &P : getSectionMap())
+    Builder->addMultiple(P.first, P.second);
+
+  // Add all other input sections, which are not listed in script.
+  for (const std::unique_ptr<ObjectFile<ELFT>> &F :
+       Symtab<ELFT>::X->getObjectFiles())
+    for (InputSectionBase<ELFT> *S : F->getSections())
+      if (!isDiscarded(S) && !S->OutSec)
+        Builder->addSingle(getOutputSectionName(S), S);
+
+  // Remove from the output all the sections which did not meet
+  // the optional constraints.
+  return Builder->finish();
 }
 
 template <class ELFT>
 void LinkerScript<ELFT>::assignAddresses(
     ArrayRef<OutputSectionBase<ELFT> *> Sections) {
   // Orphan sections are sections present in the input files which
   // are not explicitly placed into the output file by the linker script.
   // We place orphan sections at end of file.
for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     Cmd->Sym = Sym->body();
   }
 }
 
 template <class ELFT> bool LinkerScript<ELFT>::hasPhdrsCommands() {
   return !Opt.PhdrsCommands.empty();
 }
 
+template <class ELFT>
+OutputSectionFactoryBase<ELFT> *LinkerScript<ELFT>::getFactory() {
+  return Builder.get();
+}
+
 // Returns indices of ELF headers containing specific section, identified
 // by Name. Each index is a zero based number of ELF header listed within
 // PHDRS {} script block.
 template <class ELFT>
 std::vector<size_t> LinkerScript<ELFT>::getPhdrIndices(StringRef SectionName) {
   for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get());
     if (!Cmd || Cmd->Name != SectionName)

ELF/OutputSections.h

public:
   virtual void addSection(InputSectionBase<ELFT> *C) {}
 
   unsigned SectionIndex;
 
   // Returns the size of the section in the output file.
   uintX_t getSize() const { return Header.sh_size; }
   void setSize(uintX_t Val) { Header.sh_size = Val; }
   uintX_t getFlags() const { return Header.sh_flags; }
+  void setFlags(uintX_t Flags) { Header.sh_flags = Flags; }
   uint32_t getPhdrFlags() const;
   uintX_t getFileOff() const { return Header.sh_offset; }
   uintX_t getAlignment() const { return Header.sh_addralign; }
   uint32_t getType() const { return Header.sh_type; }
 
   void updateAlignment(uintX_t Alignment) {
     if (Alignment > Header.sh_addralign)
       Header.sh_addralign = Alignment;
template <bool Is64Bits> struct SectionKey {
   uintX_t Flags;
   uintX_t Alignment;
 };
 
 // This class knows how to create an output section for a given
 // input section. Output section type is determined by various
 // factors, including input section's sh_flags, sh_type and
 // linker scripts.
-template <class ELFT> class OutputSectionFactory {
+template <class ELFT> class OutputSectionFactoryBase {
+  typedef typename ELFT::uint uintX_t;
   typedef typename ELFT::Shdr Elf_Shdr;
+  typedef typename elf::SectionKey<ELFT::Is64Bits> Key;
+
+public:
+  virtual ~OutputSectionFactoryBase() = default;
+
+  virtual OutputSectionBase<ELFT> *lookup(StringRef Name, uint32_t Type,
+                                          uintX_t Flags) {
+    return nullptr;
+  }
+
+protected:
+  Key createKey(InputSectionBase<ELFT> *C, StringRef OutsecName);
+  std::pair<OutputSectionBase<ELFT> *, bool> create(InputSectionBase<ELFT> *C,
+                                                    const Key &Key);
+
+  std::vector<std::unique_ptr<OutputSectionBase<ELFT>>> OwningSections;
+};
+
+template <class ELFT>
+class OutputSectionFactory : public OutputSectionFactoryBase<ELFT> {
   typedef typename ELFT::uint uintX_t;
   typedef typename elf::SectionKey<ELFT::Is64Bits> Key;
 
 public:
   std::pair<OutputSectionBase<ELFT> *, bool> create(InputSectionBase<ELFT> *C,
                                                     StringRef OutsecName);
 
   OutputSectionBase<ELFT> *lookup(StringRef Name, uint32_t Type, uintX_t Flags);
 
 private:
-  Key createKey(InputSectionBase<ELFT> *C, StringRef OutsecName);
-
   llvm::SmallDenseMap<Key, OutputSectionBase<ELFT> *> Map;
-  std::vector<std::unique_ptr<OutputSectionBase<ELFT>>> OwningSections;
 };
 
 template <class ELFT> BuildIdSection<ELFT> *Out<ELFT>::BuildId;
 template <class ELFT> DynamicSection<ELFT> *Out<ELFT>::Dynamic;
 template <class ELFT> EhFrameHeader<ELFT> *Out<ELFT>::EhFrameHdr;
 template <class ELFT> EhOutputSection<ELFT> *Out<ELFT>::EhFrame;
 template <class ELFT> GnuHashTableSection<ELFT> *Out<ELFT>::GnuHashTab;
 template <class ELFT> GotPltSection<ELFT> *Out<ELFT>::GotPlt;

ELF/OutputSections.cpp

 void MipsOptionsOutputSection<ELFT>::addSection(InputSectionBase<ELFT> *C) {
   auto *S = cast<MipsOptionsInputSection<ELFT>>(C);
   if (S->Reginfo)
     GprMask |= S->Reginfo->ri_gprmask;
   S->OutSec = this;
 }
 
 template <class ELFT>
-std::pair<OutputSectionBase<ELFT> *, bool>
-OutputSectionFactory<ELFT>::create(InputSectionBase<ELFT> *C,
+SectionKey<ELFT::Is64Bits>
+OutputSectionFactoryBase<ELFT>::createKey(InputSectionBase<ELFT> *C,
                                           StringRef OutsecName) {
-  SectionKey<ELFT::Is64Bits> Key = createKey(C, OutsecName);
-  OutputSectionBase<ELFT> *&Sec = Map[Key];
-  if (Sec)
-    return {Sec, false};
+  const Elf_Shdr *H = C->getSectionHdr();
+  uintX_t Flags = H->sh_flags & ~SHF_GROUP & ~SHF_COMPRESSED;
+
+  // For SHF_MERGE we create different output sections for each alignment.
+  // This makes each output section simple and keeps a single level mapping from
+  // input to output.
+  uintX_t Alignment = 0;
+  if (isa<MergeInputSection<ELFT>>(C))
+    Alignment = std::max(H->sh_addralign, H->sh_entsize);
+
+  uint32_t Type = H->sh_type;
+  return SectionKey<ELFT::Is64Bits>{OutsecName, Type, Flags, Alignment};
+}
 
+template <class ELFT>
+std::pair<OutputSectionBase<ELFT> *, bool>
+OutputSectionFactoryBase<ELFT>::create(InputSectionBase<ELFT> *C,
+                                       const Key &Key) {
+  OutputSectionBase<ELFT> *Sec;
   switch (C->SectionKind) {
   case InputSectionBase<ELFT>::Regular:
     Sec = new OutputSection<ELFT>(Key.Name, Key.Type, Key.Flags);
     break;
   case InputSectionBase<ELFT>::EHFrame:
     return {Out<ELFT>::EhFrame, false};
   case InputSectionBase<ELFT>::Merge:
     Sec = new MergeOutputSection<ELFT>(Key.Name, Key.Type, Key.Flags,
                                        Key.Alignment);
     break;
   case InputSectionBase<ELFT>::MipsReginfo:
     Sec = new MipsReginfoOutputSection<ELFT>();
     break;
   case InputSectionBase<ELFT>::MipsOptions:
     Sec = new MipsOptionsOutputSection<ELFT>();
     break;
   }
   OwningSections.emplace_back(Sec);
   return {Sec, true};
 }
 
 template <class ELFT>
+std::pair<OutputSectionBase<ELFT> *, bool>
+OutputSectionFactory<ELFT>::create(InputSectionBase<ELFT> *C,
+                                   StringRef OutsecName) {
+  SectionKey<ELFT::Is64Bits> Key =
+      OutputSectionFactoryBase<ELFT>::createKey(C, OutsecName);
+  OutputSectionBase<ELFT> *&Sec = Map[Key];
+  if (Sec)
+    return {Sec, false};
+
+  bool IsNew;
+  std::tie(Sec, IsNew) = OutputSectionFactoryBase<ELFT>::create(C, Key);
+  return std::make_pair(Sec, IsNew);
+}
+
+template <class ELFT>
 OutputSectionBase<ELFT> *OutputSectionFactory<ELFT>::lookup(StringRef Name,
                                                             uint32_t Type,
                                                             uintX_t Flags) {
   return Map.lookup({Name, Type, Flags, 0});
 }
 
-template <class ELFT>
-SectionKey<ELFT::Is64Bits>
-OutputSectionFactory<ELFT>::createKey(InputSectionBase<ELFT> *C,
-                                      StringRef OutsecName) {
-  const Elf_Shdr *H = C->getSectionHdr();
-  uintX_t Flags = H->sh_flags & ~SHF_GROUP & ~SHF_COMPRESSED;
-
-  // For SHF_MERGE we create different output sections for each alignment.
-  // This makes each output section simple and keeps a single level mapping from
-  // input to output.
-  uintX_t Alignment = 0;
-  if (isa<MergeInputSection<ELFT>>(C))
-    Alignment = std::max(H->sh_addralign, H->sh_entsize);
-
-  uint32_t Type = H->sh_type;
-  return SectionKey<ELFT::Is64Bits>{OutsecName, Type, Flags, Alignment};
-}
-
 template <bool Is64Bits>
 typename lld::elf::SectionKey<Is64Bits>
 DenseMapInfo<lld::elf::SectionKey<Is64Bits>>::getEmptyKey() {
   return SectionKey<Is64Bits>{DenseMapInfo<StringRef>::getEmptyKey(), 0, 0, 0};
 }
 
 template <bool Is64Bits>
 typename lld::elf::SectionKey<Is64Bits>

ELF/Writer.cpp

 template <class ELFT> void Writer<ELFT>::run() {
   if (!Config->DiscardAll)
     copyLocalSymbols();
   addReservedSymbols();
 
   CommonInputSection<ELFT> Common(getCommonSymbols<ELFT>());
   CommonInputSection<ELFT>::X = &Common;
 
-  OutputSections = ScriptConfig->HasContents
-                       ? Script<ELFT>::X->createSections(Factory)
+  OutputSections = ScriptConfig->HasContents ? Script<ELFT>::X->createSections()
                                              : createSections();
   finalizeSections();
   if (HasError)
     return;
 
   if (Config->Relocatable) {
     assignFileOffsets();
   } else {
     Phdrs = Script<ELFT>::X->hasPhdrsCommands()
std::vector<OutputSectionBase<ELFT> *> Writer<ELFT>::createSections() {
   }
   return Result;
 }
 
 // Create output section objects and add them to OutputSections.
 template <class ELFT> void Writer<ELFT>::finalizeSections() {
   // Create output sections for input object file sections.
   std::vector<OutputSectionBase<ELFT> *> RegularSections = OutputSections;
+  OutputSectionFactoryBase<ELFT> *Fac =
+      ScriptConfig->HasContents ? Script<ELFT>::X->getFactory() : &Factory;
 
   // If we have a .opd section (used under PPC64 for function descriptors),
   // store a pointer to it here so that we can use it later when processing
   // relocations.
-  Out<ELFT>::Opd = Factory.lookup(".opd", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC);
+  Out<ELFT>::Opd = Fac->lookup(".opd", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC);
 
-  Out<ELFT>::Dynamic->PreInitArraySec = Factory.lookup(
-      ".preinit_array", SHT_PREINIT_ARRAY, SHF_WRITE | SHF_ALLOC);
+  Out<ELFT>::Dynamic->PreInitArraySec =
+      Fac->lookup(".preinit_array", SHT_PREINIT_ARRAY, SHF_WRITE | SHF_ALLOC);
   Out<ELFT>::Dynamic->InitArraySec =
-      Factory.lookup(".init_array", SHT_INIT_ARRAY, SHF_WRITE | SHF_ALLOC);
+      Fac->lookup(".init_array", SHT_INIT_ARRAY, SHF_WRITE | SHF_ALLOC);
   Out<ELFT>::Dynamic->FiniArraySec =
-      Factory.lookup(".fini_array", SHT_FINI_ARRAY, SHF_WRITE | SHF_ALLOC);
+      Fac->lookup(".fini_array", SHT_FINI_ARRAY, SHF_WRITE | SHF_ALLOC);
 
   // Sort section contents for __attribute__((init_priority(N)).
   sortInitFini(Out<ELFT>::Dynamic->InitArraySec);
   sortInitFini(Out<ELFT>::Dynamic->FiniArraySec);
-  sortCtorsDtors(Factory.lookup(".ctors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC));
-  sortCtorsDtors(Factory.lookup(".dtors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC));
+  sortCtorsDtors(Fac->lookup(".ctors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC));
+  sortCtorsDtors(Fac->lookup(".dtors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC));
 
   // The linker needs to define SECNAME_start, SECNAME_end and SECNAME_stop
   // symbols for sections, so that the runtime can get the start and end
   // addresses of each section by section name. Add such symbols.
   if (!Config->Relocatable) {
     addStartEndSymbols();
     for (OutputSectionBase<ELFT> *Sec : RegularSections)
       addStartStopSymbols(Sec);

test/ELF/linkerscript/linkerscript-repsection-va.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 
 # RUN: echo "SECTIONS {.foo : {*(.foo.*)} }" > %t.script
 # RUN: ld.lld -o %t1 --script %t.script %t
 # RUN: llvm-objdump -section-headers %t1 | FileCheck %s
 # CHECK:      Sections:
 # CHECK-NEXT: Idx Name          Size      Address          Type
 # CHECK-NEXT:   0               00000000 0000000000000000
-# CHECK-NEXT:   1 .foo          00000004 0000000000000158 DATA
-# CHECK-NEXT:   2 .foo          00000004 000000000000015c DATA
-# CHECK-NEXT:   3 .text         00000001 0000000000000160 TEXT DATA
+# CHECK-NEXT:   1 .foo          00000008 0000000000000158 DATA
+# CHECK-NEXT:   2 .text         00000001 0000000000000160 TEXT DATA
 
 .global _start
 _start:
  nop
 
 .section .foo.1,"a"
 foo1:
  .long 0
 
 .section .foo.2,"aw"
 foo2:
  .long 0