[ELF] Linkerscript: allow setting custom output section for common symbols, instead of .bss

[evgeny777]

Summary

This patch allows common symbols to be placed to arbitrary section, specified in linker script. Special rule *(COMMON) is used for this case.
Only one output section is supported.

The patch introduces artificial input section object (CommonInputSection<ELFT>), which is used to reserve certain amount of space in the output section.

[grimar]

May be it is possible just to rename/reuse .bss section if script contains *(COMMON) ?
Though FreeBSD script for example uses .bss for commons:
https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?revision=284870&view=markup#l157
So even not sure that renaming is really needed.

[ruiu]

ELF/InputSection.cpp

689

The size computed here is not always going to be the same as the actual size, because actual size is computed after sorting common symbols.

ELF/InputSection.h

260–262

You introduced this class, but you are using this only for the linker script. If linker scirpts are not used, common symbols directly belong to .bss, but if there are, they belong to some output sectino through this CommonInputSection.

Why don't you use this section both in LinkerScript.cpp and in Writer.cpp? You could create a CommonInputSection, which virtually contains all common symbols in Writer.cpp and add that input sectino to .bss, no?

[evgeny777]

ELF/InputSection.h

260–262

Makes sense.

[evgeny777]

Review updated. Now common section is instantiated in Writer and is added to .bss in case linker script processor hasn't added it to some other section.
Also test case now checks for common section size and alignment

[grimar]

ELF/LinkerScript.cpp

111

llvm::find(Patterns, "COMMON") returns iterator. Condition is always true,
for cases when Patterns does not contain COMMON it is equal to

if (Patterns.end() && ... )

You probably want to update testcase also.

ELF/Symbols.h

188

Since you always call fixSymbols() I think you can skip initialization.

ELF/Writer.cpp

225–229

May be factory should own and create this ?
That way it would be consistent with OwningSections it has and also
you'll dont need to add argument in call below, you will be able to take it directly from factory.

[evgeny777]

ELF/LinkerScript.cpp

111

I think you're not right. This is ArrayRef<T>, which iterator is a pointer to element. This means that if pointer is nullptr then there is no such element in the array. If you try to do this with STL container, for example std::vector you'll get compilation error, because STL iterator does not define operator bool().

ELF/Symbols.h

188

If someone forgets to set Section pointer there will be a crash instead of undefined behavior, which is always good

ELF/Writer.cpp

225–229

This is *input* section and I guess you're talking about OutputSectionFactory, right?

[grimar]

ELF/LinkerScript.cpp

111

Right, I missed that it is ArrayRef, but it is even simpler than:
you are searching pointer among pointers, it will always return true
since when find return end() it will be some non-zero pointer.

[evgeny777]

George found a bug, review updated.

It turned out, that ArrayRef has this code for end():
iterator end() const { return Data + Length; }

So it is never nullptr.

[ruiu]

Overall looking good. I think I like the idea of CommonInputSection.

ELF/InputSection.cpp

673–677

build is called only from the constructor, so I don't see a reason to separate the two functions. You could merge build with the constructor.

684–687

for (Symbol *S : Symtab<ELFT>::X->getSymbols())
  if (auto *C = dyn_cast<DefinedCommon<ELFT>>(S->body()))
    Symbols.push_back(C);

704

It's weird that we have two variables for one thing -- can you use only this->Alignment?

707–712

If you add an pointer to the CommonInputSection to DefinedCommon symbol type and use it in Symbol::getVA, you dont' need to adjust offsets (i.e. you can remove this function.)

ELF/InputSection.h

272

= {} to initialize with zeros, otherwise this change will trigger asan, because you are passing a pointer to this member to InputSectionBase's ctor and the ctor uses it.

ELF/Symbols.h

188

You can remove this variable, no?

[evgeny777]

ELF/Symbols.h

188

So you suggest to replace it with pointer to CommonInputSection and use OutSec and OutSecOff, correct?

[ruiu]

ELF/Symbols.h

188

Yes. Or, more radially, you can make a pointer to a CommonInputSection a global varaible since it's a singleton class, but it's probably too much for this change, so I wouldn't do it at least now.

[evgeny777]

Review updated

[ruiu]

LGTM with the following changes.

ELF/InputSection.cpp

674

val -> Val

681

Remove the blank line.

ELF/LinkerScript.cpp

111

Do you need to check the size of the common input section? It seems that you can safely add an empty section to the output section. If so, please remove that check.

ELF/OutputSections.cpp

1451

CS should be named Section for consistency with DefiendRegular class.

ELF/Writer.cpp

741

Remove the size check if not necessary.

[evgeny777]

ELF/Writer.cpp

741

Looks like this check is really not necessary. After removing it I had to add this->Live = true to CommonInputSection constructor to make tests pass when --gc-sections flag is set.

ELF/InputSection.h

 
 namespace lld {
 namespace elf {
 
 class SymbolBody;
 
 template <class ELFT> class ICF;
 template <class ELFT> class DefinedRegular;
+template <class ELFT> class DefinedCommon;
 template <class ELFT> class ObjectFile;
 template <class ELFT> class OutputSection;
 template <class ELFT> class OutputSectionBase;
 
 // This corresponds to a section of an input file.
 template <class ELFT> class InputSectionBase {
 protected:
   typedef typename ELFT::Chdr Elf_Chdr;

 
 public:
   MipsOptionsInputSection(ObjectFile<ELFT> *F, const Elf_Shdr *Hdr);
   static bool classof(const InputSectionBase<ELFT> *S);
 
   const llvm::object::Elf_Mips_RegInfo<ELFT> *Reginfo = nullptr;
 };
 
+// A special kind of section used to store common symbols
+template <class ELFT> class CommonInputSection : public InputSection<ELFT> {
+  typedef typename ELFT::uint uintX_t;
+
+public:
+  CommonInputSection();
+
+private:
+  typename ELFT::Shdr Hdr;
+};
+
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/InputSection.cpp

 //===- InputSection.cpp ---------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "InputSection.h"
 #include "Config.h"
 #include "EhFrame.h"
 #include "Error.h"
 #include "InputFiles.h"
 #include "LinkerScript.h"
 #include "OutputSections.h"
+#include "SymbolTable.h"
 #include "Target.h"
 #include "Thunks.h"
 
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Endian.h"
 
 using namespace llvm;
 using namespace llvm::ELF;
MipsOptionsInputSection<ELFT>::MipsOptionsInputSection(elf::ObjectFile<ELFT> *F,
   }
 }
 
 template <class ELFT>
 bool MipsOptionsInputSection<ELFT>::classof(const InputSectionBase<ELFT> *S) {
   return S->SectionKind == InputSectionBase<ELFT>::MipsOptions;
 }
 
+// Due to initialization order in C++ memberwise initialization or
+// construction is invoked after base class construction. This helper
+// function is needed to zero initialize Elf_Shdr, before passing it
+// to InputSection<ELFT> constructor
+template <class T> static T *zero(T *val) {
+  return static_cast<T *>(memset(val, 0, sizeof(*val)));
+}
+
+template <class ELFT>
+CommonInputSection<ELFT>::CommonInputSection()
+    : InputSection<ELFT>(nullptr, zero(&Hdr)) {
+
+  std::vector<DefinedCommon<ELFT> *> Symbols;
+  Hdr.sh_size = 0;
+  Hdr.sh_type = SHT_NOBITS;
+  Hdr.sh_flags = SHF_ALLOC | SHF_WRITE;
+
+  for (Symbol *S : Symtab<ELFT>::X->getSymbols())
+    if (auto *C = dyn_cast<DefinedCommon<ELFT>>(S->body()))
+      Symbols.push_back(C);
+
+  std::stable_sort(
+      Symbols.begin(), Symbols.end(),
+      [](const DefinedCommon<ELFT> *A, const DefinedCommon<ELFT> *B) {
+        return A->Alignment > B->Alignment;
+      });
+
+  for (DefinedCommon<ELFT> *C : Symbols) {
+    this->Alignment = std::max<uintX_t>(this->Alignment, C->Alignment);
+    Hdr.sh_size = alignTo(Hdr.sh_size, C->Alignment);
+
+    // Compute symbol offset relative to beginning of input section.
+    C->Offset = Hdr.sh_size;
+    C->CS = this;
+    Hdr.sh_size += C->Size;
+  }
+}
+
 template class elf::InputSectionBase<ELF32LE>;
 template class elf::InputSectionBase<ELF32BE>;
 template class elf::InputSectionBase<ELF64LE>;
 template class elf::InputSectionBase<ELF64BE>;
 
 template class elf::InputSection<ELF32LE>;
 template class elf::InputSection<ELF32BE>;
 template class elf::InputSection<ELF64LE>;
 template class elf::InputSection<ELF64BE>;
 
 template class elf::EhInputSection<ELF32LE>;
 template class elf::EhInputSection<ELF32BE>;
 template class elf::EhInputSection<ELF64LE>;
 template class elf::EhInputSection<ELF64BE>;
 
 template class elf::MergeInputSection<ELF32LE>;
 template class elf::MergeInputSection<ELF32BE>;
 template class elf::MergeInputSection<ELF64LE>;
 template class elf::MergeInputSection<ELF64BE>;
 
 template class elf::MipsReginfoInputSection<ELF32LE>;
 template class elf::MipsReginfoInputSection<ELF32BE>;
 template class elf::MipsReginfoInputSection<ELF64LE>;
 template class elf::MipsReginfoInputSection<ELF64BE>;
 
 template class elf::MipsOptionsInputSection<ELF32LE>;
 template class elf::MipsOptionsInputSection<ELF32BE>;
 template class elf::MipsOptionsInputSection<ELF64LE>;
 template class elf::MipsOptionsInputSection<ELF64BE>;
+
+template class elf::CommonInputSection<ELF32LE>;
+template class elf::CommonInputSection<ELF32BE>;
+template class elf::CommonInputSection<ELF64LE>;
+template class elf::CommonInputSection<ELF64BE>;

ELF/LinkerScript.h

 #include "llvm/Support/MemoryBuffer.h"
 #include <functional>
 
 namespace lld {
 namespace elf {
 template <class ELFT> class InputSectionBase;
 template <class ELFT> class OutputSectionBase;
 template <class ELFT> class OutputSectionFactory;
+template <class ELFT> class DefinedCommon;
+template <class ELFT> class CommonInputSection;
 
 typedef std::function<uint64_t(uint64_t)> Expr;
 
 // Parses a linker script. Calling this function updates
 // Config and ScriptConfig.
 void readLinkerScript(MemoryBufferRef MB);
 
 class ScriptParser;

 extern ScriptConfiguration *ScriptConfig;
 
 // 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);
+  createSections(OutputSectionFactory<ELFT> &Factory,
+                 CommonInputSection<ELFT> *Common);
 
   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();
 
 private:
   std::vector<std::pair<StringRef, const InputSectionDescription *>>
   getSectionMap();
 
   std::vector<InputSectionBase<ELFT> *>
-  getInputSections(const InputSectionDescription *);
+  getInputSections(const InputSectionDescription *, CommonInputSection<ELFT> *);
 
   // "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);

ELF/LinkerScript.cpp

if (auto *Cmd1 = dyn_cast<OutputSectionCommand>(Base1.get()))
           Ret.emplace_back(Cmd1->Name, Cmd2);
 
   return Ret;
 }
 
 // Returns input sections filtered by given glob patterns.
 template <class ELFT>
 std::vector<InputSectionBase<ELFT> *>
-LinkerScript<ELFT>::getInputSections(const InputSectionDescription *I) {
+LinkerScript<ELFT>::getInputSections(const InputSectionDescription *I,
+                                     CommonInputSection<ELFT> *Common) {
   ArrayRef<StringRef> Patterns = I->Patterns;
   ArrayRef<StringRef> ExcludedFiles = I->ExcludedFiles;
   std::vector<InputSectionBase<ELFT> *> Ret;
   for (const std::unique_ptr<ObjectFile<ELFT>> &F :
        Symtab<ELFT>::X->getObjectFiles())
     for (InputSectionBase<ELFT> *S : F->getSections())
       if (!isDiscarded(S) && !S->OutSec && match(Patterns, S->getSectionName()))
         if (ExcludedFiles.empty() ||
             !match(ExcludedFiles, sys::path::filename(F->getName())))
           Ret.push_back(S);
+
+  if ((llvm::find(Patterns, "COMMON") != Patterns.end()) && Common->getSize())
+    Ret.push_back(Common);
+
   return Ret;
 }
 
 template <class ELFT>
 std::vector<OutputSectionBase<ELFT> *>
-LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
+LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory,
+                                   CommonInputSection<ELFT> *Common) {
   std::vector<OutputSectionBase<ELFT> *> Ret;
 
   // 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)) {
+    for (InputSectionBase<ELFT> *S : getInputSections(I, Common)) {
       if (OutputName == "/DISCARD/") {
         S->Live = false;
         reportDiscarded(S);
         continue;
       }
       Add(S, OutputName);
     }
   }

ELF/OutputSections.cpp

case SymbolBody::DefinedSyntheticKind:
     return cast<DefinedSynthetic<ELFT>>(Sym)->Section;
   case SymbolBody::DefinedRegularKind: {
     auto &D = cast<DefinedRegular<ELFT>>(*Sym);
     if (D.Section)
       return D.Section->OutSec;
     break;
   }
   case SymbolBody::DefinedCommonKind:
-    return Out<ELFT>::Bss;
+    return cast<DefinedCommon<ELFT>>(Sym)->CS->OutSec;
   case SymbolBody::SharedKind:
     if (cast<SharedSymbol<ELFT>>(Sym)->needsCopy())
       return Out<ELFT>::Bss;
     break;
   case SymbolBody::UndefinedKind:
   case SymbolBody::LazyArchiveKind:
   case SymbolBody::LazyObjectKind:
     break;

ELF/SymbolTable.cpp

static int compareDefined(Symbol *S, bool WasInserted, uint8_t Binding) {
   if (S->isWeak())
     return 1;
   return 0;
 }
 
 // We have a new non-common defined symbol with the specified binding. Return 1
 // if the new symbol should win, -1 if the new symbol should lose, or 0 if there
 // is a conflict. If the new symbol wins, also update the binding.
-static int compareDefinedNonCommon(Symbol *S, bool WasInserted, uint8_t Binding) {
+template <class ELFT>
+static int compareDefinedNonCommon(Symbol *S, bool WasInserted,
+                                   uint8_t Binding) {
   if (int Cmp = compareDefined(S, WasInserted, Binding)) {
     if (Cmp > 0)
       S->Binding = Binding;
     return Cmp;
   }
-  if (isa<DefinedCommon>(S->body())) {
+  if (isa<DefinedCommon<ELFT>>(S->body())) {
     // Non-common symbols take precedence over common symbols.
     if (Config->WarnCommon)
       warning("common " + S->body()->getName() + " is overridden");
     return 1;
   }
   return 0;
 }
 
 template <class ELFT>
 Symbol *SymbolTable<ELFT>::addCommon(StringRef N, uint64_t Size,
                                      uint64_t Alignment, uint8_t Binding,
                                      uint8_t StOther, uint8_t Type,
                                      InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
       insert(N, Type, StOther & 3, /*CanOmitFromDynSym*/ false,
              /*IsUsedInRegularObj*/ true, File);
   int Cmp = compareDefined(S, WasInserted, Binding);
   if (Cmp > 0) {
     S->Binding = Binding;
-    replaceBody<DefinedCommon>(S, N, Size, Alignment, StOther, Type, File);
+    replaceBody<DefinedCommon<ELFT>>(S, N, Size, Alignment, StOther, Type,
+                                     File);
   } else if (Cmp == 0) {
-    auto *C = dyn_cast<DefinedCommon>(S->body());
+    auto *C = dyn_cast<DefinedCommon<ELFT>>(S->body());
     if (!C) {
       // Non-common symbols take precedence over common symbols.
       if (Config->WarnCommon)
         warning("common " + S->body()->getName() + " is overridden");
       return S;
     }
 
     if (Config->WarnCommon)

 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, const Elf_Sym &Sym,
                                       InputSectionBase<ELFT> *Section) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
       insert(Name, Sym.getType(), Sym.getVisibility(),
              /*CanOmitFromDynSym*/ false, /*IsUsedInRegularObj*/ true,
              Section ? Section->getFile() : nullptr);
-  int Cmp = compareDefinedNonCommon(S, WasInserted, Sym.getBinding());
+  int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, Sym.getBinding());
   if (Cmp > 0)
     replaceBody<DefinedRegular<ELFT>>(S, Name, Sym, Section);
   else if (Cmp == 0)
     reportDuplicate(S->body(), Section->getFile());
   return S;
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, uint8_t Binding,
                                       uint8_t StOther) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
       insert(Name, STT_NOTYPE, StOther & 3, /*CanOmitFromDynSym*/ false,
              /*IsUsedInRegularObj*/ true, nullptr);
-  int Cmp = compareDefinedNonCommon(S, WasInserted, Binding);
+  int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, Binding);
   if (Cmp > 0)
     replaceBody<DefinedRegular<ELFT>>(S, Name, StOther);
   else if (Cmp == 0)
     reportDuplicate(S->body(), nullptr);
   return S;
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addSynthetic(StringRef N,
                                         OutputSectionBase<ELFT> *Section,
                                         uintX_t Value) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
       insert(N, STT_NOTYPE, STV_HIDDEN, /*CanOmitFromDynSym*/ false,
              /*IsUsedInRegularObj*/ true, nullptr);
-  int Cmp = compareDefinedNonCommon(S, WasInserted, STB_GLOBAL);
+  int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, STB_GLOBAL);
   if (Cmp > 0)
     replaceBody<DefinedSynthetic<ELFT>>(S, N, Value, Section);
   else if (Cmp == 0)
     reportDuplicate(S->body(), nullptr);
   return S;
 }
 
 template <typename ELFT>

 template <class ELFT>
 Symbol *SymbolTable<ELFT>::addBitcode(StringRef Name, bool IsWeak,
                                       uint8_t StOther, uint8_t Type,
                                       bool CanOmitFromDynSym, BitcodeFile *F) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, Type, StOther & 3, CanOmitFromDynSym,
                                     /*IsUsedInRegularObj*/ false, F);
-  int Cmp =
-      compareDefinedNonCommon(S, WasInserted, IsWeak ? STB_WEAK : STB_GLOBAL);
+  int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted,
+                                          IsWeak ? STB_WEAK : STB_GLOBAL);
   if (Cmp > 0)
     replaceBody<DefinedBitcode>(S, Name, StOther, Type, F);
   else if (Cmp == 0)
     reportDuplicate(S->body(), F);
   return S;
 }
 
 template <class ELFT> SymbolBody *SymbolTable<ELFT>::find(StringRef Name) {

ELF/Symbols.h

 // The defined symbol in LLVM bitcode files.
 class DefinedBitcode : public Defined {
 public:
   DefinedBitcode(StringRef Name, uint8_t StOther, uint8_t Type, BitcodeFile *F);
   static bool classof(const SymbolBody *S);
   BitcodeFile *file() { return (BitcodeFile *)this->File; }
 };
 
-class DefinedCommon : public Defined {
+template <class ELFT> class DefinedCommon : public Defined {
 public:
   DefinedCommon(StringRef N, uint64_t Size, uint64_t Alignment, uint8_t StOther,
                 uint8_t Type, InputFile *File);
 
   static bool classof(const SymbolBody *S) {
     return S->kind() == SymbolBody::DefinedCommonKind;
   }
 
   // The output offset of this common symbol in the output bss. Computed by the
   // writer.
-  uint64_t OffsetInBss;
+  uint64_t Offset;
 
   // The maximum alignment we have seen for this symbol.
   uint64_t Alignment;
 
   uint64_t Size;
+
+  // Virtual input section for common symbols.
+  CommonInputSection<ELFT> *CS;
 };
 
 // Regular defined symbols read from object file symbol tables.
 template <class ELFT> class DefinedRegular : public Defined {
   typedef typename ELFT::Sym Elf_Sym;
   typedef typename ELFT::uint uintX_t;
 
 public:
struct Symbol {
   bool isWeak() const { return Binding == llvm::ELF::STB_WEAK; }
 
   // This field is used to store the Symbol's SymbolBody. This instantiation of
   // AlignedCharArrayUnion gives us a struct with a char array field that is
   // large and aligned enough to store any derived class of SymbolBody. We
   // assume that the size and alignment of ELF64LE symbols is sufficient for any
   // ELFT, and we verify this with the static_asserts in replaceBody.
   llvm::AlignedCharArrayUnion<
-      DefinedBitcode, DefinedCommon, DefinedRegular<llvm::object::ELF64LE>,
+      DefinedBitcode, DefinedCommon<llvm::object::ELF64LE>,
+      DefinedRegular<llvm::object::ELF64LE>,
       DefinedSynthetic<llvm::object::ELF64LE>, Undefined,
       SharedSymbol<llvm::object::ELF64LE>, LazyArchive, LazyObject>
       Body;
 
   SymbolBody *body() { return reinterpret_cast<SymbolBody *>(Body.buffer); }
   const SymbolBody *body() const { return const_cast<Symbol *>(this)->body(); }
 };
 

ELF/Symbols.cpp

if (D.isSection()) {
       Offset += Addend;
       Addend = 0;
     }
     uintX_t VA = SC->OutSec->getVA() + SC->getOffset(Offset);
     if (D.isTls())
       return VA - Out<ELFT>::TlsPhdr->p_vaddr;
     return VA;
   }
-  case SymbolBody::DefinedCommonKind:
-    return Out<ELFT>::Bss->getVA() + cast<DefinedCommon>(Body).OffsetInBss;
+  case SymbolBody::DefinedCommonKind: {
+    auto &D = cast<DefinedCommon<ELFT>>(Body);
+    return D.CS->OutSec->getVA() + D.CS->OutSecOff + D.Offset;
+  }
   case SymbolBody::SharedKind: {
     auto &SS = cast<SharedSymbol<ELFT>>(Body);
     if (!SS.NeedsCopyOrPltAddr)
       return 0;
     if (SS.isFunc())
       return Body.getPltVA<ELFT>();
     return Out<ELFT>::Bss->getVA() + SS.OffsetInBss;
   }
template <class ELFT> typename ELFT::uint SymbolBody::getThunkVA() const {
   if (const auto *DR = dyn_cast<DefinedRegular<ELFT>>(this))
     return DR->ThunkData->getVA();
   if (const auto *S = dyn_cast<SharedSymbol<ELFT>>(this))
     return S->ThunkData->getVA();
   fatal("getThunkVA() not supported for Symbol class\n");
 }
 
 template <class ELFT> typename ELFT::uint SymbolBody::getSize() const {
-  if (const auto *C = dyn_cast<DefinedCommon>(this))
+  if (const auto *C = dyn_cast<DefinedCommon<ELFT>>(this))
     return C->Size;
   if (const auto *DR = dyn_cast<DefinedRegular<ELFT>>(this))
     return DR->Size;
   if (const auto *S = dyn_cast<SharedSymbol<ELFT>>(this))
     return S->Sym.st_size;
   return 0;
 }
 

 }
 
 template <typename ELFT>
 DefinedSynthetic<ELFT>::DefinedSynthetic(StringRef N, uintX_t Value,
                                          OutputSectionBase<ELFT> *Section)
     : Defined(SymbolBody::DefinedSyntheticKind, N, STV_HIDDEN, 0 /* Type */),
       Value(Value), Section(Section) {}
 
-DefinedCommon::DefinedCommon(StringRef N, uint64_t Size, uint64_t Alignment,
-                             uint8_t StOther, uint8_t Type, InputFile *File)
+template <class ELFT>
+DefinedCommon<ELFT>::DefinedCommon(StringRef N, uint64_t Size,
+                                   uint64_t Alignment, uint8_t StOther,
+                                   uint8_t Type, InputFile *File)
     : Defined(SymbolBody::DefinedCommonKind, N, StOther, Type),
       Alignment(Alignment), Size(Size) {
   this->File = File;
 }
 
 std::unique_ptr<InputFile> Lazy::fetch() {
   if (auto *S = dyn_cast<LazyArchive>(this))
     return S->fetch();

 template uint32_t SymbolBody::template getSize<ELF32BE>() const;
 template uint64_t SymbolBody::template getSize<ELF64LE>() const;
 template uint64_t SymbolBody::template getSize<ELF64BE>() const;
 
 template class elf::DefinedSynthetic<ELF32LE>;
 template class elf::DefinedSynthetic<ELF32BE>;
 template class elf::DefinedSynthetic<ELF64LE>;
 template class elf::DefinedSynthetic<ELF64BE>;
+
+template class elf::DefinedCommon<ELF32LE>;
+template class elf::DefinedCommon<ELF32BE>;
+template class elf::DefinedCommon<ELF64LE>;
+template class elf::DefinedCommon<ELF64BE>;

ELF/Writer.cpp

private:
   void fixHeaders();
   void fixSectionAlignments();
   void fixAbsoluteSymbols();
   void openFile();
   void writeHeader();
   void writeSections();
   void writeBuildId();
 
-  void addCommonSymbols(std::vector<DefinedCommon *> &Syms);
-
   std::unique_ptr<FileOutputBuffer> Buffer;
 
   BumpPtrAllocator Alloc;
   std::vector<OutputSectionBase<ELFT> *> OutputSections;
+  std::unique_ptr<CommonInputSection<ELFT>> CommonSection;
   OutputSectionFactory<ELFT> Factory;
 
   void addRelIpltSymbols();
   void addStartEndSymbols();
   void addStartStopSymbols(OutputSectionBase<ELFT> *Sec);
 
   SymbolTable<ELFT> &Symtab;
   std::vector<Phdr> Phdrs;

 }
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
   if (!Config->DiscardAll)
     copyLocalSymbols();
   addReservedSymbols();
 
-  OutputSections = ScriptConfig->DoLayout
-                       ? Script<ELFT>::X->createSections(Factory)
-                       : createSections();
+  CommonSection = llvm::make_unique<CommonInputSection<ELFT>>();
+  OutputSections =
+      ScriptConfig->DoLayout
+          ? Script<ELFT>::X->createSections(Factory, CommonSection.get())
   finalizeSections();
   if (HasError)
     return;
 
   if (Config->Relocatable) {
     assignFileOffsets();
   } else {
     Phdrs = Script<ELFT>::X->hasPhdrsCommands()

 template<class ELFT>
 void PhdrEntry<ELFT>::add(OutputSectionBase<ELFT> *Sec) {
   Last = Sec;
   if (!First)
     First = Sec;
   H.p_align = std::max<typename ELFT::uint>(H.p_align, Sec->getAlignment());
 }
 
-// Until this function is called, common symbols do not belong to any section.
-// This function adds them to end of BSS section.
-template <class ELFT>
-void Writer<ELFT>::addCommonSymbols(std::vector<DefinedCommon *> &Syms) {
-  if (Syms.empty())
-    return;
-
-  // Sort the common symbols by alignment as an heuristic to pack them better.
-  std::stable_sort(Syms.begin(), Syms.end(),
-                   [](const DefinedCommon *A, const DefinedCommon *B) {
-                     return A->Alignment > B->Alignment;
-                   });
-
-  uintX_t Off = Out<ELFT>::Bss->getSize();
-  for (DefinedCommon *C : Syms) {
-    Off = alignTo(Off, C->Alignment);
-    Out<ELFT>::Bss->updateAlignment(C->Alignment);
-    C->OffsetInBss = Off;
-    Off += C->Size;
-  }
-
-  Out<ELFT>::Bss->setSize(Off);
-}
-
 template <class ELFT>
 static Symbol *addOptionalSynthetic(SymbolTable<ELFT> &Table, StringRef Name,
                                     OutputSectionBase<ELFT> *Sec,
                                     typename ELFT::uint Val) {
   SymbolBody *S = Table.find(Name);
   if (!S)
     return nullptr;
   if (!S->isUndefined() && !S->isShared())
for (OutputSectionBase<ELFT> *Sec : OutputSections)
     Sec->assignOffsets();
 
   // Scan relocations. This must be done after every symbol is declared so that
   // we can correctly decide if a dynamic relocation is needed.
   forEachRelSec(scanRelocations<ELFT>);
 
   // Now that we have defined all possible symbols including linker-
   // synthesized ones. Visit all symbols to give the finishing touches.
-  std::vector<DefinedCommon *> CommonSymbols;
   for (Symbol *S : Symtab.getSymbols()) {
     SymbolBody *Body = S->body();
 
     // We only report undefined symbols in regular objects. This means that we
     // will accept an undefined reference in bitcode if it can be optimized out.
     if (S->IsUsedInRegularObj && Body->isUndefined() && !S->isWeak())
       reportUndefined<ELFT>(Symtab, Body);
 
-    if (auto *C = dyn_cast<DefinedCommon>(Body))
-      CommonSymbols.push_back(C);
-
     if (!includeInSymtab<ELFT>(*Body))
       continue;
     if (Out<ELFT>::SymTab)
       Out<ELFT>::SymTab->addSymbol(Body);
 
     if (isOutputDynamic<ELFT>() && S->includeInDynsym()) {
       Out<ELFT>::DynSymTab->addSymbol(Body);
       if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(Body))
         if (SS->file()->isNeeded())
           Out<ELFT>::VerNeed->addSymbol(SS);
     }
   }
 
   // Do not proceed if there was an undefined symbol.
   if (HasError)
     return;
 
-  addCommonSymbols(CommonSymbols);
+  // If linker script processor hasn't added common symbol section yet,
+  // then add it to .bss now.
+  if (!CommonSection->OutSec && CommonSection->getSize()) {
+    Out<ELFT>::Bss->addSection(CommonSection.get());
+    Out<ELFT>::Bss->assignOffsets();
+  }
 
   // So far we have added sections from input object files.
   // This function adds linker-created Out<ELFT>::* sections.
   addPredefinedSections();
 
   std::stable_sort(OutputSections.begin(), OutputSections.end(),
                    compareSections<ELFT>);
 

test/ELF/linkerscript/linkerscript-common.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { .common : { *(COMMON) } }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-readobj -s -t %t1 | FileCheck %s
+
+# q2 alignment is greater than q1, so it should have smaller offset
+# because of sorting
+# CHECK:       Section {
+# CHECK:         Index: 1
+# CHECK-NEXT:    Name: .common (1)
+# CHECK-NEXT:    Type: SHT_NOBITS (0x8)
+# CHECK-NEXT:    Flags [ (0x3)
+# CHECK-NEXT:      SHF_ALLOC (0x2)
+# CHECK-NEXT:      SHF_WRITE (0x1)
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x200
+# CHECK-NEXT:    Offset: 0x158
+# CHECK-NEXT:    Size: 256
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 256
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: q1 (8)
+# CHECK-NEXT:    Value: 0x280
+# CHECK-NEXT:    Size: 128
+# CHECK-NEXT:    Binding: Global (0x1)
+# CHECK-NEXT:    Type: Object (0x1)
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common (0x1)
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Symbol {
+# CHECK-NEXT:    Name: q2 (11)
+# CHECK-NEXT:    Value: 0x200
+# CHECK-NEXT:    Size: 128
+# CHECK-NEXT:    Binding: Global (0x1)
+# CHECK-NEXT:    Type: Object (0x1)
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common (0x1)
+# CHECK-NEXT:  }
+
+.globl _start
+_start:
+  jmp _start
+
+.comm q1,128,8
+.comm q2,128,256