ELF: Remove DefinedCommon.

[pcc]

Summary

Common symbols are now represented with a DefinedRegular that points
to a BssSection, even during symbol resolution.

Depends on D39394

[ruiu]

LGTM

Very nice improvement! I haven't thought of this approach before, but this totally makes sense to me.

If you need my patch, please commit it.

lld/ELF/InputSection.h

57 ↗

(On Diff #121664)

Can you add this to InputSection?

[pcc]

lld/ELF/InputSection.h

57 ↗

(On Diff #121664)

Do you mean InputSectionBase? I put it here so that it would pack with the other bitfield.

lld/trunk/ELF/Driver.cpp

template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   Config->EFlags = Target->calcEFlags();
 
   // This adds a .comment section containing a version string. We have to add it
   // before decompressAndMergeSections because the .comment section is a
   // mergeable section.
   if (!Config->Relocatable)
     InputSections.push_back(createCommentSection<ELFT>());
 
-  // Create a .bss section for each common symbol and then replace the common
-  // symbol with a DefinedRegular symbol. As a result, all common symbols are
-  // "instantiated" as regular defined symbols, so that we don't need to care
-  // about common symbols beyond this point. Note that if -r is given, we just
-  // need to pass through common symbols as-is.
-  if (Config->DefineCommon)
-    createCommonSections<ELFT>();
-
   // Do size optimizations: garbage collection, merging of SHF_MERGE sections
   // and identical code folding.
   markLive<ELFT>();
   decompressSections();
   mergeSections();
   if (Config->ICF)
     doIcf<ELFT>();
 
   // Write the result to the file.
   writeResult<ELFT>();
 }

lld/trunk/ELF/InputSection.h

 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Object/ELF.h"
 #include "llvm/Support/Threading.h"
 #include <mutex>
 
 namespace lld {
 namespace elf {
 
-class DefinedCommon;
 class Symbol;
 struct SectionPiece;
 
 class DefinedRegular;
 class SyntheticSection;
 class MergeSyntheticSection;
 template <class ELFT> class ObjFile;
 class OutputSection;
public:
 
   // The next two bit fields are only used by InputSectionBase, but we
   // put them here so the struct packs better.
 
   // The garbage collector sets sections' Live bits.
   // If GC is disabled, all sections are considered live by default.
   unsigned Live : 1;
 
+  unsigned Bss : 1;
+
   // These corresponds to the fields in Elf_Shdr.
   uint32_t Alignment;
   uint64_t Flags;
   uint64_t Entsize;
   uint32_t Type;
   uint32_t Link;
   uint32_t Info;
 
   OutputSection *getOutputSection();
   const OutputSection *getOutputSection() const {
     return const_cast<SectionBase *>(this)->getOutputSection();
   }
 
   // Translate an offset in the input section to an offset in the output
   // section.
   uint64_t getOffset(uint64_t Offset) const;
 
   uint64_t getOffset(const DefinedRegular &Sym) const;
 
 protected:
   SectionBase(Kind SectionKind, StringRef Name, uint64_t Flags,
               uint64_t Entsize, uint64_t Alignment, uint32_t Type,
               uint32_t Info, uint32_t Link)
-      : Name(Name), SectionKind(SectionKind), Live(false),
+      : Name(Name), SectionKind(SectionKind), Live(false), Bss(false),
         Alignment(Alignment), Flags(Flags), Entsize(Entsize), Type(Type),
         Link(Link), Info(Info) {}
 };
 
 // This corresponds to a section of an input file.
 class InputSectionBase : public SectionBase {
 public:
   InputSectionBase()

lld/trunk/ELF/SymbolTable.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolTable.h"
 #include "Config.h"
 #include "LinkerScript.h"
 #include "Memory.h"
 #include "Symbols.h"
+#include "SyntheticSections.h"
 #include "lld/Common/ErrorHandler.h"
 #include "llvm/ADT/STLExtras.h"
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::ELF;
 
 using namespace lld;

 static int compareDefinedNonCommon(Symbol *S, bool WasInserted, uint8_t Binding,
                                    bool IsAbsolute, uint64_t Value,
                                    StringRef Name) {
   if (int Cmp = compareDefined(S, WasInserted, Binding, Name)) {
     if (Cmp > 0)
       S->Binding = Binding;
     return Cmp;
   }
-  if (isa<DefinedCommon>(S)) {
+  if (auto *R = dyn_cast<DefinedRegular>(S)) {
+    if (R->Section && isa<BssSection>(R->Section)) {
       // Non-common symbols take precedence over common symbols.
       if (Config->WarnCommon)
         warn("common " + S->getName() + " is overridden");
       return 1;
     }
-  if (auto *R = dyn_cast<DefinedRegular>(S)) {
     if (R->Section == nullptr && Binding == STB_GLOBAL && IsAbsolute &&
         R->Value == Value)
       return -1;
   }
   return 0;
 }
 
 Symbol *SymbolTable::addCommon(StringRef N, uint64_t Size, uint32_t Alignment,
                                uint8_t Binding, uint8_t StOther, uint8_t Type,
                                InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(N, Type, getVisibility(StOther),
                                     /*CanOmitFromDynSym*/ false, File);
   int Cmp = compareDefined(S, WasInserted, Binding, N);
   if (Cmp > 0) {
+    auto *Bss = make<BssSection>("COMMON", Size, Alignment);
+    Bss->File = File;
+    Bss->Live = !Config->GcSections;
+    InputSections.push_back(Bss);
+
     S->Binding = Binding;
-    replaceSymbol<DefinedCommon>(S, File, N, Size, Alignment, StOther, Type);
+    replaceSymbol<DefinedRegular>(S, File, N, /*IsLocal=*/false, StOther, Type,
+                                  0, Size, Bss);
   } else if (Cmp == 0) {
-    auto *C = dyn_cast<DefinedCommon>(S);
-    if (!C) {
+    auto *D = cast<DefinedRegular>(S);
+    auto *Bss = dyn_cast_or_null<BssSection>(D->Section);
+    if (!Bss) {
       // Non-common symbols take precedence over common symbols.
       if (Config->WarnCommon)
         warn("common " + S->getName() + " is overridden");
       return S;
     }
 
     if (Config->WarnCommon)
-      warn("multiple common of " + S->getName());
+      warn("multiple common of " + D->getName());
 
-    Alignment = C->Alignment = std::max(C->Alignment, Alignment);
-    if (Size > C->Size)
-      replaceSymbol<DefinedCommon>(S, File, N, Size, Alignment, StOther, Type);
+    Bss->Alignment = std::max(Bss->Alignment, Alignment);
+    if (Size > Bss->Size) {
+      D->File = Bss->File = File;
+      D->Size = Bss->Size = Size;
+    }
   }
   return S;
 }
 
 static void warnOrError(const Twine &Msg) {
   if (Config->AllowMultipleDefinition)
     warn(Msg);
   else

lld/trunk/ELF/Symbols.h

 template <class ELFT> class SharedFile;
 
 // The base class for real symbol classes.
 class Symbol {
 public:
   enum Kind {
     DefinedFirst,
     DefinedRegularKind = DefinedFirst,
-    DefinedCommonKind,
-    DefinedLast = DefinedCommonKind,
+    DefinedLast = DefinedRegularKind,
     SharedKind,
     UndefinedKind,
     LazyArchiveKind,
     LazyObjectKind,
   };
 
   Symbol(Kind K) : SymbolKind(K) {}
   Kind kind() const { return static_cast<Kind>(SymbolKind); }
public:
   InputFile *File = nullptr;
 
   bool includeInDynsym() const;
   uint8_t computeBinding() const;
   bool isWeak() const { return Binding == llvm::ELF::STB_WEAK; }
 
   bool isUndefined() const { return SymbolKind == UndefinedKind; }
   bool isDefined() const { return SymbolKind <= DefinedLast; }
-  bool isCommon() const { return SymbolKind == DefinedCommonKind; }
   bool isShared() const { return SymbolKind == SharedKind; }
   bool isLocal() const { return IsLocal; }
 
   bool isLazy() const {
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
 
   bool isInCurrentOutput() const {
-    return SymbolKind == DefinedRegularKind || SymbolKind == DefinedCommonKind;
+    return SymbolKind == DefinedRegularKind;
   }
 
   // True is this is an undefined weak symbol. This only works once
   // all input files have been added.
   bool isUndefWeak() const;
 
   InputFile *getFile() const;
   StringRef getName() const { return Name; }

 class Defined : public Symbol {
 public:
   Defined(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther, uint8_t Type)
       : Symbol(K, Name, IsLocal, StOther, Type) {}
 
   static bool classof(const Symbol *S) { return S->isDefined(); }
 };
 
-class DefinedCommon : public Defined {
-public:
-  DefinedCommon(StringRef Name, uint64_t Size, uint32_t Alignment,
-                uint8_t StOther, uint8_t Type)
-      : Defined(DefinedCommonKind, Name, /*IsLocal=*/false, StOther, Type),
-        Alignment(Alignment), Size(Size) {}
-
-  static bool classof(const Symbol *S) {
-    return S->kind() == DefinedCommonKind;
-  }
-
-  // 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 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(DefinedRegularKind, Name, IsLocal, StOther, Type), Value(Value),
         Size(Size), Section(Section) {}
 
struct ElfSym {
   static DefinedRegular *MipsLocalGp;
 };
 
 // A buffer class that is large enough to hold any Symbol-derived
 // object. We allocate memory using this class and instantiate a symbol
 // using the placement new.
 union SymbolUnion {
   alignas(DefinedRegular) char A[sizeof(DefinedRegular)];
-  alignas(DefinedCommon) char B[sizeof(DefinedCommon)];
   alignas(Undefined) char C[sizeof(Undefined)];
   alignas(SharedSymbol) char D[sizeof(SharedSymbol)];
   alignas(LazyArchive) char E[sizeof(LazyArchive)];
   alignas(LazyObject) char F[sizeof(LazyObject)];
 };
 
 void printTraceSymbol(Symbol *Sym);
 

lld/trunk/ELF/Symbols.cpp

case Symbol::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 Symbol::DefinedCommonKind:
-    llvm_unreachable("common are converted to bss");
   case Symbol::SharedKind: {
     auto &SS = cast<SharedSymbol>(Sym);
     if (SS.CopyRelSec)
       return SS.CopyRelSec->getParent()->Addr + SS.CopyRelSec->OutSecOff;
     if (SS.NeedsPltAddr)
       return Sym.getPltVA();
     return 0;
   }

 uint64_t Symbol::getPltVA() const {
   if (this->IsInIplt)
     return InX::Iplt->getVA() + PltIndex * Target->PltEntrySize;
   return InX::Plt->getVA() + Target->PltHeaderSize +
          PltIndex * Target->PltEntrySize;
 }
 
 uint64_t Symbol::getSize() const {
-  if (const auto *C = dyn_cast<DefinedCommon>(this))
-    return C->Size;
   if (const auto *DR = dyn_cast<DefinedRegular>(this))
     return DR->Size;
   if (const auto *S = dyn_cast<SharedSymbol>(this))
     return S->Size;
   return 0;
 }
 
 OutputSection *Symbol::getOutputSection() const {
   if (auto *S = dyn_cast<DefinedRegular>(this)) {
     if (S->Section)
       return S->Section->getOutputSection();
     return nullptr;
   }
 
   if (auto *S = dyn_cast<SharedSymbol>(this)) {
     if (S->CopyRelSec)
       return S->CopyRelSec->getParent();
     return nullptr;
   }
 
-  if (auto *S = dyn_cast<DefinedCommon>(this)) {
-    if (Config->DefineCommon)
-      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.
 void Symbol::parseSymbolVersion() {
   StringRef S = getName();
   size_t Pos = S.find('@');
bool Symbol::includeInDynsym() const {
   return ExportDynamic;
 }
 
 // Print out a log message for --trace-symbol.
 void elf::printTraceSymbol(Symbol *Sym) {
   std::string S;
   if (Sym->isUndefined())
     S = ": reference to ";
-  else if (Sym->isCommon())
-    S = ": common definition of ";
   else if (Sym->isLazy())
     S = ": lazy definition of ";
   else if (Sym->isShared())
     S = ": shared definition of ";
+  else if (dyn_cast_or_null<BssSection>(cast<DefinedRegular>(Sym)->Section))
+    S = ": common definition of ";
   else
     S = ": definition of ";
 
   message(toString(Sym->File) + S + Sym->getName());
 }
 
 // Returns a symbol for an error message.
 std::string lld::toString(const Symbol &B) {

lld/trunk/ELF/SyntheticSections.h

 // respectively.
 class BssSection final : public SyntheticSection {
 public:
   BssSection(StringRef Name, uint64_t Size, uint32_t Alignment);
   void writeTo(uint8_t *) override {}
   bool empty() const override { return getSize() == 0; }
   size_t getSize() const override { return Size; }
 
-private:
+  static bool classof(const SectionBase *S) { return S->Bss; }
   uint64_t Size;
 };
 
 class MipsGotSection final : public SyntheticSection {
 public:
   MipsGotSection();
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return Size; }
public:
   void writeTo(uint8_t *Buf) override;
   InputSection *getTargetInputSection() const;
 
 private:
   std::vector<const Thunk *> Thunks;
   size_t Size = 0;
 };
 
-template <class ELFT> void createCommonSections();
 InputSection *createInterpSection();
 template <class ELFT> MergeInputSection *createCommentSection();
 void decompressSections();
 void mergeSections();
 
 Symbol *addSyntheticLocal(StringRef Name, uint8_t Type, uint64_t Value,
                           uint64_t Size, InputSectionBase *Section);
 

lld/trunk/ELF/SyntheticSections.cpp

 }
 
 uint64_t SyntheticSection::getVA() const {
   if (OutputSection *Sec = getParent())
     return Sec->Addr + OutSecOff;
   return 0;
 }
 
-// Create a .bss section for each common symbol and replace the common symbol
-// with a DefinedRegular symbol.
-template <class ELFT> void elf::createCommonSections() {
-  for (Symbol *S : Symtab->getSymbols()) {
-    auto *Sym = dyn_cast<DefinedCommon>(S);
-
-    if (!Sym)
-      continue;
-
-    // Create a synthetic section for the common data.
-    auto *Section = make<BssSection>("COMMON", Sym->Size, Sym->Alignment);
-    Section->File = Sym->getFile();
-    Section->Live = !Config->GcSections;
-    InputSections.push_back(Section);
-
-    // Replace all DefinedCommon symbols with DefinedRegular symbols so that we
-    // don't have to care about DefinedCommon symbols beyond this point.
-    replaceSymbol<DefinedRegular>(
-        S, Sym->getFile(), Sym->getName(), static_cast<bool>(Sym->isLocal()),
-        Sym->StOther, Sym->Type, 0, Sym->getSize(), Section);
-  }
-}
-
 // 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");
   if (S.empty())
     S = Saver.save(Twine("Linker: ") + getLLDVersion());
void BuildIdSection::computeHash(
   });
 
   // Write to the final output buffer.
   HashFn(HashBuf, Hashes);
 }
 
 BssSection::BssSection(StringRef Name, uint64_t Size, uint32_t Alignment)
     : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, Alignment, Name) {
+  this->Bss = true;
   if (OutputSection *Sec = getParent())
     Sec->Alignment = std::max(Sec->Alignment, Alignment);
   this->Size = Size;
 }
 
 void BuildIdSection::writeBuildId(ArrayRef<uint8_t> Buf) {
   switch (Config->BuildId) {
   case BuildIdKind::Fast:
for (SymbolTableEntry &Ent : Symbols) {
     } else {
       ESym->setBindingAndType(Sym->computeBinding(), Sym->Type);
       ESym->setVisibility(Sym->Visibility);
     }
 
     ESym->st_name = Ent.StrTabOffset;
 
     // Set a section index.
-    if (const OutputSection *OutSec = Sym->getOutputSection())
+    BssSection *CommonSec = nullptr;
+    if (!Config->DefineCommon)
+      if (auto *D = dyn_cast<DefinedRegular>(Sym))
+        CommonSec = dyn_cast_or_null<BssSection>(D->Section);
+    if (CommonSec)
+      ESym->st_shndx = SHN_COMMON;
+    else if (const OutputSection *OutSec = Sym->getOutputSection())
       ESym->st_shndx = OutSec->SectionIndex;
     else if (isa<DefinedRegular>(Sym))
       ESym->st_shndx = SHN_ABS;
-    else if (isa<DefinedCommon>(Sym))
-      ESym->st_shndx = SHN_COMMON;
     else
       ESym->st_shndx = SHN_UNDEF;
 
     // Copy symbol size if it is a defined symbol. st_size is not significant
     // for undefined symbols, so whether copying it or not is up to us if that's
     // the case. We'll leave it as zero because by not setting a value, we can
     // get the exact same outputs for two sets of input files that differ only
     // in undefined symbol size in DSOs.
     if (ESym->st_shndx == SHN_UNDEF)
       ESym->st_size = 0;
     else
       ESym->st_size = Sym->getSize();
 
     // st_value is usually an address of a symbol, but that has a
     // special meaining for uninstantiated common symbols (this can
     // occur if -r is given).
-    if (!Config->DefineCommon && isa<DefinedCommon>(Sym))
-      ESym->st_value = cast<DefinedCommon>(Sym)->Alignment;
+    if (CommonSec)
+      ESym->st_value = CommonSec->Alignment;
     else
       ESym->st_value = Sym->getVA();
 
     ++ESym;
   }
 
   // On MIPS we need to mark symbol which has a PLT entry and requires
   // pointer equality by STO_MIPS_PLT flag. That is necessary to help

 template void EhFrameSection::addSection<ELF64LE>(InputSectionBase *);
 template void EhFrameSection::addSection<ELF64BE>(InputSectionBase *);
 
 template void PltSection::addEntry<ELF32LE>(Symbol &Sym);
 template void PltSection::addEntry<ELF32BE>(Symbol &Sym);
 template void PltSection::addEntry<ELF64LE>(Symbol &Sym);
 template void PltSection::addEntry<ELF64BE>(Symbol &Sym);
 
-template void elf::createCommonSections<ELF32LE>();
-template void elf::createCommonSections<ELF32BE>();
-template void elf::createCommonSections<ELF64LE>();
-template void elf::createCommonSections<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/test/ELF/relocatable-common.s

 
 # DEFCOMM:      Symbol {
 # DEFCOMM:        Name: common
 # DEFCOMM-NEXT:   Value: 0x0
 # DEFCOMM-NEXT:   Size: 4
 # DEFCOMM-NEXT:   Binding: Global
 # DEFCOMM-NEXT:   Type: Object
 # DEFCOMM-NEXT:   Other: 0
-# DEFCOMM-NEXT:   Section: COMMON (0x2)
+# DEFCOMM-NEXT:   Section: COMMON
 # DEFCOMM-NEXT: }
 
 .comm common,4,4