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) }} This patch changes the allocation of commons to create a section per common symbol and let the section logic do the layout. Differential revision: https://reviews.llvm.org/D37489 llvm-svn: 312796
llvm · Sep 8, 2017 · 1e30f07 · 1e30f07
1 parent 500c25e
commit 1e30f07
Show file tree

Hide file tree

Showing 15 changed files with 243 additions and 71 deletions.
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
@@ -174,17 +174,17 @@ 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))
@@ -284,7 +284,7 @@ LinkerScript::computeInputSections(const InputSectionDescription *Cmd) {
       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))
@@ -328,8 +328,8 @@ LinkerScript::computeInputSections(const InputSectionDescription *Cmd) {
 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);
   }
@@ -868,7 +868,7 @@ ExprValue LinkerScript::getSymbolValue(const Twine &Loc, StringRef 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;

diff --git a/lld/ELF/MapFile.cpp b/lld/ELF/MapFile.cpp
@@ -57,7 +57,7 @@ template <class ELFT> static std::vector<Defined *> getSymbols() {
             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);
       }
     }
@@ -71,8 +71,8 @@ static SymbolMapTy getSectionSyms(ArrayRef<Defined *> Syms) {
   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

diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
@@ -99,11 +99,13 @@ static uint64_t getSymVA(const SymbolBody &Body, int64_t &Addend) {
     }
     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)
@@ -202,9 +204,9 @@ OutputSection *SymbolBody::getOutputSection() const {
     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;
   }
 

diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
@@ -27,6 +27,7 @@ namespace elf {
 
 class ArchiveFile;
 class BitcodeFile;
+class BssSection;
 class InputFile;
 class LazyObjFile;
 template <class ELFT> class ObjFile;
@@ -173,6 +174,7 @@ class DefinedCommon : public Defined {
   // Computed by the writer.
   uint64_t Offset;
   uint64_t Size;
+  BssSection *Section = nullptr;
 };
 
 // Regular defined symbols read from object file symbol tables.

diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
@@ -54,35 +54,22 @@ uint64_t SyntheticSection::getVA() const {
   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.
@@ -2321,7 +2308,6 @@ InputSection *InX::ARMAttributes;
 BssSection *InX::Bss;
 BssSection *InX::BssRelRo;
 BuildIdSection *InX::BuildId;
-InputSection *InX::Common;
 SyntheticSection *InX::Dynamic;
 StringTableSection *InX::DynStrTab;
 SymbolTableBaseSection *InX::DynSymTab;
@@ -2349,11 +2335,6 @@ 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>();

diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
@@ -740,7 +740,7 @@ class ThunkSection : public SyntheticSection {
   size_t Size = 0;
 };
 
-template <class ELFT> InputSection *createCommonSection();
+std::vector<InputSection *> createCommonSections();
 InputSection *createInterpSection();
 template <class ELFT> MergeInputSection *createCommentSection();
 void decompressAndMergeSections();
@@ -754,7 +754,6 @@ struct InX {
   static BssSection *Bss;
   static BssSection *BssRelRo;
   static BuildIdSection *BuildId;
-  static InputSection *Common;
   static SyntheticSection *Dynamic;
   static StringTableSection *DynStrTab;
   static SymbolTableBaseSection *DynSymTab;

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
@@ -299,9 +299,9 @@ template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
     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);

diff --git a/lld/test/ELF/common.s b/lld/test/ELF/common.s
@@ -12,37 +12,37 @@
 // 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

diff --git a/lld/test/ELF/linkerscript/Inputs/common-filespec1.s b/lld/test/ELF/linkerscript/Inputs/common-filespec1.s
@@ -0,0 +1,2 @@
+.comm common_uniq_1,8,8
+.comm common_multiple,16,8
diff --git a/lld/test/ELF/linkerscript/Inputs/common-filespec2.s b/lld/test/ELF/linkerscript/Inputs/common-filespec2.s
@@ -0,0 +1,2 @@
+.comm common_uniq_2,16,16
+.comm common_multiple,32,8
diff --git a/lld/test/ELF/linkerscript/common-exclude.s b/lld/test/ELF/linkerscript/common-exclude.s
@@ -0,0 +1,86 @@
+# 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