Linker script handing of file patterns with COMMON defs

[dmikulin]

Summary

Currently lld creates a single section to collect all commons. There is no way to separate commons based on file name patterns. The following linker script construct does not work because commons are allocated before section placement is done and the only synthesized BssSection that holds all commons has no file associated with it:
SECTIONS { .common_0 : { *file0.o(COMMON) }}

The idea here is to create a common section for each set of commons matching a COMMON file pattern in the linker script. Each section will have a file name of one of the member symbols attached to it so that subsequent processing of SECTION commands can match against it and place it into an appropriate output section.

[ruiu]

Perhaps a more straightforward (though inefficient) way of doing it is to create a Common section for each symbol instead of trying to group them from beginning. It might be too inefficient, but I think it's worth a try.

lld/ELF/LinkerScript.cpp

331 ↗

(On Diff #113877)

I don't remember why you can't discard common sections. If it works without this piece of code, can you just remove the code?

[dmikulin]

My initial thought was to create a section for each common, but I immediately discarded it as too inefficient. Also, it would complicate sorting commons according to their alignment requirements (as it's currently done).

lld/ELF/LinkerScript.cpp

331 ↗

(On Diff #113877)

There's a test which specifically checks for attempts to discard COMMONs. Other than that, the rest of them pass with no issues. I can fix the test and remove the code that checks for common sections.

[ruiu]

Did you try the naive-but-possibly-inefficient algorithm? It may or may not work, but I wouldn't turn it down unless it is shown to be too inefficient.

As to sorting, I think finding sequences of Common sections and sorting them isn't too hard.

[dmikulin]

Intuitively it didn't seem like the right thing to do. Why do you think it's worth pursuing?
I've just made the change to allocate a section per common (without sorting) and it kind of works, failing some tests due to lack of sorting.
Is there a good real-life benchmark with lots of commons to quantify the efficiency? Or a generated synthetic test case good enough?

[ruiu]

I don't think it is intuitively a wrong approach. One advantage of the approach is that it eliminates dependency to linker scripts when creating Common sections as it'll instantiate all Common sections without knowing anything about linker scripts. As a benchmark, I'd first try to link Clang with/without a patch to see if there's a measurable difference.

[dmikulin]

Linking clang takes less than a second and link time is dominated by processing of things other than commons. If there is a difference, it's not very significant in the overall link time.
Linking a synthetic test with 100K commons almost doubles user time when section-per-common is used.

[ruiu]

What if you build LLVM/clang with debug info? (I don't think that makes a difference, but it would make link time longer from <1 seconds to ~10 seconds.)

[dmikulin]

I think debug link will mask processing of commons even more, as the number of commons is more or less the same, but link time is dominated by processing debug info.
The synthetic test case confirms the inefficiency of allocating a section per common. The question is: can we tolerate the inefficiency?

[dmikulin]

Changed the implementation to allocate a common section for each common symbol.
Per Rui's comment, removed a check for common sections in LinkerScript::discard() to allow discarding them.

A couple of data points:

• linking a synthetic test with 100K commons takes ~0.550secs of user time vs ~0.325secs with grouping symbols.
• memory consumption goes up by ~16M,
• linking clang shows no measurable difference between the two allocation schemes.

[ruiu]

When I link clang, lld doesn't actually create any Common section because no input file has common symbols. I haven't thought about that until now, but that makes sense -- I personally don't write code that depends on common symbols too. Maybe common symbols are not that common these days as people have been trained to add extern to global varaible declarations.

We are sorting common symbols because it is believed to pack common symbols well. But we didn't verify that when we add that code to lld. It's a copy of GNU linker's behavior. Maybe we should stop doing that.

[dmikulin]

Are there Fortran FEs to LLVM? Might still be fairly common there.
If it doesn't cost much, keeping the sorting doesn't hurt, may save us a few alignment holes. Most of the cost is from section per common, not from sorting.

[ruiu]

Our philosophy of re-implementing a linker is to not copy features without verifying that doing that makes sense. So I think we shouldn't have sorted common symbols in the first place. We can bring it back when it is proved to be useful, but until then, I think I prefer removing the code to sort common symbols.

[dmikulin]

Now without sorting the commons by alignment.

[ruiu]

lld/ELF/Symbols.h

177 ↗

(On Diff #114273)

Section = nullptr

lld/ELF/SyntheticSections.cpp

57–58 ↗

(On Diff #114273)

clang-format

59 ↗

(On Diff #114273)

By convention, Ret is preferred over SV. In general, I found that giving the same name to a variable regardless of its type is more readable than naming it based on type. So, I prefer naming a symbol (whether it is defined or not) Sym instead of DC or something like that.

64 ↗

(On Diff #114273)

So, let's name this Sym

lld/ELF/SyntheticSections.h

757 ↗

(On Diff #114273)

I think you can now remove this member.

[dmikulin]

Addressed latest comments.

[ruiu]

LGTM with these changes.

lld/ELF/SyntheticSections.cpp

58–60 ↗

(On Diff #114287)

I believe this is slightly more readable.

if (!Config->DefineCommon)
  return {};

std::vector<InputSection *> Ret;
for (Symbol *S : ...) {

lld/ELF/Writer.cpp

305–306 ↗

(On Diff #114287)

In lld, we don't use auto unless it is obvious in a very local context (typically RHS). This is not the case.

for (InputSection *S : createCommonSections())
  add(S)

[bd1976llvm]

Hi Dmitry, Ruiu - this change breaks lld for references to garbage collected common symbols.

Could you please take a look at: https://reviews.llvm.org/D37718.

lld/trunk/ELF/LinkerScript.cpp

 bool AssertCommand::classof(const BaseCommand *C) {
   return C->Kind == AssertKind;
 }
 
 bool BytesDataCommand::classof(const BaseCommand *C) {
   return C->Kind == BytesDataKind;
 }
 
-static std::string filename(InputSectionBase *S) {
-  if (!S->File)
+static std::string filename(InputFile *File) {
+  if (!File)
     return "";
-  if (S->File->ArchiveName.empty())
-    return S->File->getName();
-  return (S->File->ArchiveName + "(" + S->File->getName() + ")").str();
+  if (File->ArchiveName.empty())
+    return File->getName();
+  return (File->ArchiveName + "(" + File->getName() + ")").str();
 }
 
 bool LinkerScript::shouldKeep(InputSectionBase *S) {
   for (InputSectionDescription *ID : Opt.KeptSections) {
-    std::string Filename = filename(S);
+    std::string Filename = filename(S->File);
     if (ID->FilePat.match(Filename))
       for (SectionPattern &P : ID->SectionPatterns)
         if (P.SectionPat.match(S->Name))
           return true;
   }
   return false;
 }
 
for (InputSectionBase *Sec : InputSections) {
       }
 
       // For -emit-relocs we have to ignore entries like
       //   .rela.dyn : { *(.rela.data) }
       // which are common because they are in the default bfd script.
       if (Sec->Type == SHT_REL || Sec->Type == SHT_RELA)
         continue;
 
-      std::string Filename = filename(Sec);
+      std::string Filename = filename(Sec->File);
       if (!Cmd->FilePat.match(Filename) ||
           Pat.ExcludedFilePat.match(Filename) ||
           !Pat.SectionPat.match(Sec->Name))
         continue;
 
       Ret.push_back(cast<InputSection>(Sec));
       Sec->Assigned = true;
     }
for (const SectionPattern &Pat : Cmd->SectionPatterns) {
     }
   }
   return Ret;
 }
 
 void LinkerScript::discard(ArrayRef<InputSectionBase *> V) {
   for (InputSectionBase *S : V) {
     S->Live = false;
-    if (S == InX::ShStrTab || S == InX::Common || S == InX::Dynamic ||
-        S == InX::DynSymTab || S == InX::DynStrTab)
+    if (S == InX::ShStrTab || S == InX::Dynamic || S == InX::DynSymTab ||
+        S == InX::DynStrTab)
       error("discarding " + S->Name + " section is not allowed");
     discard(S->DependentSections);
   }
 }
 
 std::vector<InputSectionBase *>
 LinkerScript::createInputSectionList(OutputSection &OutCmd) {
   std::vector<InputSectionBase *> Ret;
if (CurAddressState)
               Loc};
     error(Loc + ": unable to get location counter value");
     return 0;
   }
   if (SymbolBody *B = Symtab->find(S)) {
     if (auto *D = dyn_cast<DefinedRegular>(B))
       return {D->Section, D->Value, Loc};
     if (auto *C = dyn_cast<DefinedCommon>(B))
-      return {InX::Common, C->Offset, Loc};
+      return {C->Section, C->Offset, Loc};
   }
   error(Loc + ": symbol not found: " + S);
   return 0;
 }
 
 bool LinkerScript::isDefined(StringRef S) { return Symtab->find(S) != nullptr; }
 
 static const size_t NoPhdr = -1;

lld/trunk/ELF/MapFile.cpp

template <class ELFT> static std::vector<Defined *> getSymbols() {
   std::vector<Defined *> V;
   for (ObjFile<ELFT> *File : ObjFile<ELFT>::Instances) {
     for (SymbolBody *B : File->getSymbols()) {
       if (auto *DR = dyn_cast<DefinedRegular>(B)) {
         if (DR->getFile() == File && !DR->isSection() && DR->Section &&
             DR->Section->Live)
           V.push_back(DR);
       } else if (auto *DC = dyn_cast<DefinedCommon>(B)) {
-        if (InX::Common)
+        if (DC->Section)
           V.push_back(DC);
       }
     }
   }
   return V;
 }
 
 // Returns a map from sections to their symbols.
 static SymbolMapTy getSectionSyms(ArrayRef<Defined *> Syms) {
   SymbolMapTy Ret;
   for (Defined *S : Syms) {
     if (auto *DR = dyn_cast<DefinedRegular>(S))
       Ret[DR->Section].push_back(S);
-    else
-      Ret[InX::Common].push_back(S);
+    else if (auto *DC = dyn_cast<DefinedCommon>(S))
+      Ret[DC->Section].push_back(S);
   }
 
   // Sort symbols by address. We want to print out symbols in the
   // order in the output file rather than the order they appeared
   // in the input files.
   for (auto &It : Ret) {
     SmallVectorImpl<Defined *> &V = It.second;
     std::sort(V.begin(), V.end(),

lld/trunk/ELF/Symbols.h

 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ELF.h"
 
 namespace lld {
 namespace elf {
 
 class ArchiveFile;
 class BitcodeFile;
+class BssSection;
 class InputFile;
 class LazyObjFile;
 template <class ELFT> class ObjFile;
 class OutputSection;
 template <class ELFT> class SharedFile;
 
 struct Symbol;
 
public:
 
   // The maximum alignment we have seen for this symbol.
   uint32_t Alignment;
 
   // The output offset of this common symbol in the output bss.
   // Computed by the writer.
   uint64_t Offset;
   uint64_t Size;
+  BssSection *Section = nullptr;
 };
 
 // Regular defined symbols read from object file symbol tables.
 class DefinedRegular : public Defined {
 public:
   DefinedRegular(StringRefZ Name, bool IsLocal, uint8_t StOther, uint8_t Type,
                  uint64_t Value, uint64_t Size, SectionBase *Section)
       : Defined(SymbolBody::DefinedRegularKind, Name, IsLocal, StOther, Type),

lld/trunk/ELF/Symbols.cpp

case SymbolBody::DefinedRegularKind: {
     if (D.isTls() && !Config->Relocatable) {
       if (!Out::TlsPhdr)
         fatal(toString(D.getFile()) +
               " has an STT_TLS symbol but doesn't have an SHF_TLS section");
       return VA - Out::TlsPhdr->p_vaddr;
     }
     return VA;
   }
-  case SymbolBody::DefinedCommonKind:
+  case SymbolBody::DefinedCommonKind: {
     if (!Config->DefineCommon)
       return 0;
-    return InX::Common->getParent()->Addr + InX::Common->OutSecOff +
-           cast<DefinedCommon>(Body).Offset;
+    auto DC = cast<DefinedCommon>(Body);
+    return DC.Section->getParent()->Addr + DC.Section->OutSecOff +
+           DC.Offset;
+  }
   case SymbolBody::SharedKind: {
     auto &SS = cast<SharedSymbol>(Body);
     if (SS.CopyRelSec)
       return SS.CopyRelSec->getParent()->Addr + SS.CopyRelSec->OutSecOff +
              SS.CopyRelSecOff;
     if (SS.NeedsPltAddr)
       return Body.getPltVA();
     return 0;
OutputSection *SymbolBody::getOutputSection() const {
   }
 
   if (auto *S = dyn_cast<SharedSymbol>(this)) {
     if (S->CopyRelSec)
       return S->CopyRelSec->getParent();
     return nullptr;
   }
 
-  if (isa<DefinedCommon>(this)) {
+  if (auto *S = dyn_cast<DefinedCommon>(this)) {
     if (Config->DefineCommon)
-      return InX::Common->getParent();
+      return S->Section->getParent();
     return nullptr;
   }
 
   return nullptr;
 }
 
 // If a symbol name contains '@', the characters after that is
 // a symbol version name. This function parses that.

lld/trunk/ELF/SyntheticSections.h

public:
   void writeTo(uint8_t *Buf) override;
   InputSection *getTargetInputSection() const;
 
 private:
   std::vector<const Thunk *> Thunks;
   size_t Size = 0;
 };
 
-template <class ELFT> InputSection *createCommonSection();
+std::vector<InputSection *> createCommonSections();
 InputSection *createInterpSection();
 template <class ELFT> MergeInputSection *createCommentSection();
 void decompressAndMergeSections();
 
 SymbolBody *addSyntheticLocal(StringRef Name, uint8_t Type, uint64_t Value,
                               uint64_t Size, InputSectionBase *Section);
 
 // Linker generated sections which can be used as inputs.
 struct InX {
   static InputSection *ARMAttributes;
   static BssSection *Bss;
   static BssSection *BssRelRo;
   static BuildIdSection *BuildId;
-  static InputSection *Common;
   static SyntheticSection *Dynamic;
   static StringTableSection *DynStrTab;
   static SymbolTableBaseSection *DynSymTab;
   static GnuHashTableSection *GnuHashTab;
   static InputSection *Interp;
   static GdbIndexSection *GdbIndex;
   static GotSection *Got;
   static GotPltSection *GotPlt;

lld/trunk/ELF/SyntheticSections.cpp

 using namespace lld::elf;
 
 uint64_t SyntheticSection::getVA() const {
   if (OutputSection *Sec = getParent())
     return Sec->Addr + OutSecOff;
   return 0;
 }
 
-template <class ELFT> static std::vector<DefinedCommon *> getCommonSymbols() {
-  std::vector<DefinedCommon *> V;
-  for (Symbol *S : Symtab->getSymbols())
-    if (auto *B = dyn_cast<DefinedCommon>(S->body()))
-      V.push_back(B);
-  return V;
-}
-
-// Find all common symbols and allocate space for them.
-template <class ELFT> InputSection *elf::createCommonSection() {
+std::vector<InputSection *> elf::createCommonSections() {
   if (!Config->DefineCommon)
-    return nullptr;
+    return {};
 
-  // Sort the common symbols by alignment as an heuristic to pack them better.
-  std::vector<DefinedCommon *> Syms = getCommonSymbols<ELFT>();
-  if (Syms.empty())
-    return nullptr;
-
-  std::stable_sort(Syms.begin(), Syms.end(),
-                   [](const DefinedCommon *A, const DefinedCommon *B) {
-                     return A->Alignment > B->Alignment;
-                   });
+  std::vector<InputSection *> Ret;
+  for (Symbol *S : Symtab->getSymbols()) {
+    auto *Sym = dyn_cast<DefinedCommon>(S->body());
+    if (!Sym || !Sym->Live)
+      continue;
 
-  // Allocate space for common symbols.
-  BssSection *Sec = make<BssSection>("COMMON");
-  for (DefinedCommon *Sym : Syms)
-    if (Sym->Live)
-      Sym->Offset = Sec->reserveSpace(Sym->Size, Sym->Alignment);
-  return Sec;
+    Sym->Section = make<BssSection>("COMMON");
+    Sym->Offset = Sym->Section->reserveSpace(Sym->Size, Sym->Alignment);
+    Sym->Section->File = Sym->getFile();
+    Ret.push_back(Sym->Section);
+  }
+  return Ret;
 }
 
 // Returns an LLD version string.
 static ArrayRef<uint8_t> getVersion() {
   // Check LLD_VERSION first for ease of testing.
   // You can get consitent output by using the environment variable.
   // This is only for testing.
   StringRef S = getenv("LLD_VERSION");
InputSection *ThunkSection::getTargetInputSection() const {
   const Thunk *T = Thunks.front();
   return T->getTargetInputSection();
 }
 
 InputSection *InX::ARMAttributes;
 BssSection *InX::Bss;
 BssSection *InX::BssRelRo;
 BuildIdSection *InX::BuildId;
-InputSection *InX::Common;
 SyntheticSection *InX::Dynamic;
 StringTableSection *InX::DynStrTab;
 SymbolTableBaseSection *InX::DynSymTab;
 InputSection *InX::Interp;
 GdbIndexSection *InX::GdbIndex;
 GotSection *InX::Got;
 GotPltSection *InX::GotPlt;
 GnuHashTableSection *InX::GnuHashTab;

 template GdbIndexSection *elf::createGdbIndex<ELF64LE>();
 template GdbIndexSection *elf::createGdbIndex<ELF64BE>();
 
 template void PltSection::addEntry<ELF32LE>(SymbolBody &Sym);
 template void PltSection::addEntry<ELF32BE>(SymbolBody &Sym);
 template void PltSection::addEntry<ELF64LE>(SymbolBody &Sym);
 template void PltSection::addEntry<ELF64BE>(SymbolBody &Sym);
 
-template InputSection *elf::createCommonSection<ELF32LE>();
-template InputSection *elf::createCommonSection<ELF32BE>();
-template InputSection *elf::createCommonSection<ELF64LE>();
-template InputSection *elf::createCommonSection<ELF64BE>();
-
 template MergeInputSection *elf::createCommentSection<ELF32LE>();
 template MergeInputSection *elf::createCommentSection<ELF32BE>();
 template MergeInputSection *elf::createCommentSection<ELF64LE>();
 template MergeInputSection *elf::createCommentSection<ELF64BE>();
 
 template class elf::MipsAbiFlagsSection<ELF32LE>;
 template class elf::MipsAbiFlagsSection<ELF32BE>;
 template class elf::MipsAbiFlagsSection<ELF64LE>;

lld/trunk/ELF/Writer.cpp

if (Config->Strip != StripPolicy::All) {
     InX::SymTab = make<SymbolTableSection<ELFT>>(*InX::StrTab);
   }
 
   if (Config->BuildId != BuildIdKind::None) {
     InX::BuildId = make<BuildIdSection>();
     Add(InX::BuildId);
   }
 
-  InX::Common = createCommonSection<ELFT>();
-  if (InX::Common)
-    Add(InX::Common);
+  auto Commons = createCommonSections();
+  for (InputSection *S : Commons)
+    Add(S);
 
   InX::Bss = make<BssSection>(".bss");
   Add(InX::Bss);
   InX::BssRelRo = make<BssSection>(".bss.rel.ro");
   Add(InX::BssRelRo);
 
   // Add MIPS-specific sections.
   bool HasDynSymTab = !SharedFile<ELFT>::Instances.empty() || Config->Pic ||

lld/trunk/test/ELF/common.s

 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/common.s -o %t2
 // RUN: ld.lld %t %t2 -o %t3
 // RUN: llvm-readobj -t -s %t3 | FileCheck %s
 // REQUIRES: x86
 
 // CHECK:      Name: .bss
 // CHECK-NEXT: Type: SHT_NOBITS
 // CHECK-NEXT: Flags [
 // CHECK-NEXT:   SHF_ALLOC
 // CHECK-NEXT:   SHF_WRITE
 // CHECK-NEXT: ]
 // CHECK-NEXT: Address: 0x201000
 // CHECK-NEXT: Offset:
-// CHECK-NEXT: Size: 22
+// CHECK-NEXT: Size: 36
 // CHECK-NEXT: Link: 0
 // CHECK-NEXT: Info: 0
 // CHECK-NEXT: AddressAlignment: 16
 
 // CHECK:      Name: sym1
-// CHECK-NEXT: Value: 0x201004
+// CHECK-NEXT: Value: 0x201000
 // CHECK-NEXT: Size: 8
 // CHECK-NEXT: Binding: Global
 // CHECK-NEXT: Type: Object
 // CHECK-NEXT: Other: 0
 // CHECK-NEXT: Section: .bss
 
 // CHECK:      Name: sym2
-// CHECK-NEXT: Value: 0x20100C
+// CHECK-NEXT: Value: 0x201008
 // CHECK-NEXT: Size: 8
 // CHECK-NEXT: Binding: Global
 // CHECK-NEXT: Type: Object
 // CHECK-NEXT: Other: 0
 // CHECK-NEXT: Section: .bss
 
 // CHECK:      Name: sym3
-// CHECK-NEXT: Value: 0x201014
+// CHECK-NEXT: Value: 0x201010
 // CHECK-NEXT: Size: 2
 // CHECK-NEXT: Binding: Global
 // CHECK-NEXT: Type: Object
 // CHECK-NEXT: Other: 0
 // CHECK-NEXT: Section: .bss
 
 // CHECK:      Name: sym4
-// CHECK-NEXT: Value: 0x201000
+// CHECK-NEXT: Value: 0x201020
 // CHECK-NEXT: Size: 4
 // CHECK-NEXT: Binding: Global
 // CHECK-NEXT: Type: Object
 // CHECK-NEXT: Other: 0
 // CHECK-NEXT: Section: .bss
 
 
 .globl _start
 _start:
 
 .comm sym1,4,4
 .comm sym2,8,4
 .comm sym3,2,2
 .comm sym4,4,2

lld/trunk/test/ELF/linkerscript/Inputs/common-filespec1.s

+.comm common_uniq_1,8,8
+.comm common_multiple,16,8

lld/trunk/test/ELF/linkerscript/Inputs/common-filespec2.s

+.comm common_uniq_2,16,16
+.comm common_multiple,32,8

lld/trunk/test/ELF/linkerscript/common-exclude.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %tfile0.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/common-filespec1.s -o %tfile1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/common-filespec2.s -o %tfile2.o
+# RUN: echo "SECTIONS { .common.incl : { *(EXCLUDE_FILE (*file2.o) COMMON) } .common.excl : { *(COMMON) } }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %tfile0.o %tfile1.o %tfile2.o
+# RUN: llvm-readobj -s -t %t1 | FileCheck %s
+
+# Commons from file0 and file1 are not excluded, so they must be in .common.incl
+# Commons from file2 are excluded from the first rule and should be caught by
+# the second in .common.excl
+# CHECK:       Section {
+# CHECK:         Index:
+# CHECK:         Name: .common.incl
+# CHECK-NEXT:    Type: SHT_NOBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_WRITE
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x8
+# CHECK-NEXT:    Offset: 0x
+# CHECK-NEXT:    Size: 16
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 8
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK:       Section {
+# CHECK:         Index:
+# CHECK:         Name: .common.excl
+# CHECK-NEXT:    Type: SHT_NOBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_WRITE
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x20
+# CHECK-NEXT:    Offset: 0x
+# CHECK-NEXT:    Size: 48
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 16
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: common_multiple
+# CHECK-NEXT:    Value: 0x20
+# CHECK-NEXT:    Size: 32
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common.excl
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: common_uniq_0
+# CHECK-NEXT:    Value: 0x8
+# CHECK-NEXT:    Size: 4
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common.incl
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: common_uniq_1
+# CHECK-NEXT:    Value: 0x10
+# CHECK-NEXT:    Size: 8
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common.incl
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: common_uniq_2
+# CHECK-NEXT:    Value: 0x40
+# CHECK-NEXT:    Size: 16
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common.excl
+# CHECK-NEXT:  }
+
+.globl _start
+_start:
+  jmp _start
+
+.comm common_uniq_0,4,4
+.comm common_multiple,8,8

lld/trunk/test/ELF/linkerscript/common-filespec.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %tfile0.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/common-filespec1.s -o %tfile1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/common-filespec2.s -o %tfile2.o
+# RUN: echo "SECTIONS { .common_0 : { *file0.o(COMMON) } .common_1 : { *file1.o(COMMON) } .common_2 : { *file2.o(COMMON) } }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %tfile0.o %tfile1.o %tfile2.o
+# RUN: llvm-readobj -s -t %t1 | FileCheck %s
+
+# Make sure all 3 sections are allocated and they have sizes and alignments
+# corresponding to the commons assigned to them
+# CHECK:       Section {
+# CHECK:         Index:
+# CHECK:         Name: .common_0
+# CHECK-NEXT:    Type: SHT_NOBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_WRITE
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x4
+# CHECK-NEXT:    Offset: 0x
+# CHECK-NEXT:    Size: 4
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 4
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK:       Section {
+# CHECK:         Index:
+# CHECK:         Name: .common_1
+# CHECK-NEXT:    Type: SHT_NOBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_WRITE
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x8
+# CHECK-NEXT:    Offset: 0x
+# CHECK-NEXT:    Size: 8
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 8
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK:       Section {
+# CHECK:         Index:
+# CHECK:         Name: .common_2
+# CHECK-NEXT:    Type: SHT_NOBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_WRITE
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x10
+# CHECK-NEXT:    Offset: 0x
+# CHECK-NEXT:    Size: 48
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 16
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+
+# Commons with unique name in each file must be assigned to that file's section.
+# For a common with multiple definitions, the largest one wins and it must be
+# assigned to the section from the file which provided the winning def
+# CHECK:       Symbol {
+# CHECK:         Name: common_multiple
+# CHECK-NEXT:    Value: 0x10
+# CHECK-NEXT:    Size: 32
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common_2
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: common_uniq_0
+# CHECK-NEXT:    Value: 0x4
+# CHECK-NEXT:    Size: 4
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common_0
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: common_uniq_1
+# CHECK-NEXT:    Value: 0x8
+# CHECK-NEXT:    Size: 8
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common_1
+# CHECK-NEXT:  }
+# CHECK:       Symbol {
+# CHECK:         Name: common_uniq_2
+# CHECK-NEXT:    Value: 0x30
+# CHECK-NEXT:    Size: 16
+# CHECK-NEXT:    Binding: Global
+# CHECK-NEXT:    Type: Object
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: .common_2
+# CHECK-NEXT:  }
+
+.globl _start
+_start:
+  jmp _start
+
+.comm common_uniq_0,4,4
+.comm common_multiple,8,8

lld/trunk/test/ELF/linkerscript/common.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 # RUN: echo "SECTIONS { . = SIZEOF_HEADERS; .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:
 # CHECK:         Name: .common
 # CHECK-NEXT:    Type: SHT_NOBITS
 # CHECK-NEXT:    Flags [
 # CHECK-NEXT:      SHF_ALLOC
 # CHECK-NEXT:      SHF_WRITE
 # CHECK-NEXT:    ]
 # CHECK-NEXT:    Address: 0x200
 # CHECK-NEXT:    Offset: 0x
-# CHECK-NEXT:    Size: 256
+# CHECK-NEXT:    Size: 384
 # CHECK-NEXT:    Link: 0
 # CHECK-NEXT:    Info: 0
 # CHECK-NEXT:    AddressAlignment: 256
 # CHECK-NEXT:    EntrySize: 0
 # CHECK-NEXT:  }
 # CHECK:       Symbol {
 # CHECK:         Name: q1
-# CHECK-NEXT:    Value: 0x280
+# CHECK-NEXT:    Value: 0x200
 # CHECK-NEXT:    Size: 128
 # CHECK-NEXT:    Binding: Global
 # CHECK-NEXT:    Type: Object
 # CHECK-NEXT:    Other: 0
 # CHECK-NEXT:    Section: .common
 # CHECK-NEXT:  }
 # CHECK-NEXT:  Symbol {
 # CHECK-NEXT:    Name: q2
-# CHECK-NEXT:    Value: 0x200
+# CHECK-NEXT:    Value: 0x300
 # CHECK-NEXT:    Size: 128
 # CHECK-NEXT:    Binding: Global
 # CHECK-NEXT:    Type: Object
 # CHECK-NEXT:    Other: 0
 # CHECK-NEXT:    Section: .common
 # CHECK-NEXT:  }
 
 .globl _start
 _start:
   jmp _start
 
 .comm q1,128,8
 .comm q2,128,256

lld/trunk/test/ELF/linkerscript/discard-section-err.s

 # RUN:   FileCheck -check-prefix=DYNSYM %s
 # DYNSYM: discarding .dynsym section is not allowed
 
 # RUN: echo "SECTIONS { /DISCARD/ : { *(.dynstr) } }" > %t.script
 # RUN: not ld.lld -pie -o %t --script %t.script %t.o 2>&1 | \
 # RUN:   FileCheck -check-prefix=DYNSTR %s
 # DYNSTR: discarding .dynstr section is not allowed
 
-# RUN: echo "SECTIONS { /DISCARD/ : { *(COMMON) } }" > %t.script
-# RUN: not ld.lld -pie -o %t --script %t.script %t.o 2>&1 | \
-# RUN:   FileCheck -check-prefix=COMMON %s
-# COMMON: discarding COMMON section is not allowed
-
 .comm foo,4,4

lld/trunk/test/ELF/map-file.s

 // CHECK-NEXT: 0000000000201011 0000000000000000     0                 bar
 // CHECK-NEXT: 0000000000201012 0000000000000000     1         {{.*}}{{/|\\}}map-file.s.tmp2.o:(.text.zed)
 // CHECK-NEXT: 0000000000201012 0000000000000000     0                 zed
 // CHECK-NEXT: 0000000000201014 0000000000000000     4         {{.*}}{{/|\\}}map-file.s.tmp3.o:(.text)
 // CHECK-NEXT: 0000000000201014 0000000000000000     0                 bah
 // CHECK-NEXT: 0000000000201014 0000000000000001     4         {{.*}}{{/|\\}}map-file.s.tmp4.a(map-file.s.tmp4.o):(.text)
 // CHECK-NEXT: 0000000000201014 0000000000000000     0                 baz
 // CHECK-NEXT: 0000000000202000 0000000000000004    16 .bss
-// CHECK-NEXT: 0000000000202000 0000000000000004    16         <internal>:(COMMON)
+// CHECK-NEXT: 0000000000202000 0000000000000004    16         {{.*}}{{/|\\}}map-file.s.tmp1.o:(COMMON)
 // CHECK-NEXT: 0000000000202000 0000000000000004     0                 common
 // CHECK-NEXT: 0000000000000000 0000000000000008     1 .comment
 // CHECK-NEXT: 0000000000000000 0000000000000008     1         <internal>:(.comment)
 // CHECK-NEXT: 0000000000000000 0000000000000120     8 .symtab
 // CHECK-NEXT: 0000000000000000 0000000000000120     8         <internal>:(.symtab)
 // CHECK-NEXT: 0000000000000000 0000000000000039     1 .shstrtab
 // CHECK-NEXT: 0000000000000000 0000000000000039     1         <internal>:(.shstrtab)
 // CHECK-NEXT: 0000000000000000 0000000000000038     1 .strtab
 // CHECK-NEXT: 0000000000000000 0000000000000038     1         <internal>:(.strtab)
 
 // RUN: not ld.lld %t1.o %t2.o %t3.o %t4.a -o %t -Map=/ 2>&1 \
 // RUN:  | FileCheck -check-prefix=FAIL %s
 // FAIL: cannot open map file /