Introduce CommonSymbol.

[ruiu]

Summary

Previously, we handled common symbols as a kind of Defined symbol,
but what we were doing for common symbols is pretty different from
regular defined symbols.

Common symbol and defined symbol are probably as different as shared
symbol and defined symbols are different.

This patch introduces CommonSymbol to represent common symbols.
After symbols are resolved, they are converted to Defined symbols
residing in a .bss section.

[grimar]

This is interesting, could you please hold this on (in case you'll have LGTM from other person) until I'll look more carefully in my tomorrow (+24h)?

[MaskRay]

In D61895#1501273, @grimar wrote:

This is interesting, could you please hold this on (in case you'll have LGTM from other person) until I'll look more carefully in my tomorrow (+24h)?

You beat me to the draw:) I wanted to say the same. I also need a careful analysis of this...

[MaskRay]

lld/ELF/SymbolTable.cpp

155 ↗

(On Diff #199422)

mergeProperties(Old, New); can be moved after if (Old->isPlaceholder()) {

See my question in D61855. I think we should decrease the number of redundant assignments (if possible).

156 ↗

(On Diff #199422)

Not the fault of this patch, but this comment looks obscure..

isa<SharedSymbol>(Old) -> Old->isShared().

496 ↗

(On Diff #199422)

getDemangledSyms is transitively called by scanVersioScript before replaceCommonSymbols. Is a isCommon() check needed here?

511 ↗

(On Diff #199422)

Ditto. Is a isCommon() check needed here?

528 ↗

(On Diff #199422)

Ditto. Is a isCommon() check needed here?

[grimar]

lld/ELF/SymbolTable.cpp

136 ↗

(On Diff #199422)

Missing comment.

295 ↗

(On Diff #199422)

Not strong opinion, but perhaps !OldSym->Section && !NewSym->Section would be better (while you are here).

320 ↗

(On Diff #199422)

Lets use CommonSymbol instead of auto? OldSym has an obvious type here, but I feel sometimes that auto is something
that was invented for iterators or stuff like that and often used a bit too often than needed.

I.e. I think it should be used when the type is so complex in spelling that you should not think about it. Otherwise for readablility,
like in this place, probably it is better just to use the original type name. What do you think?

lld/ELF/Symbols.h

254 ↗

(On Diff #199422)

delcarations -> declarations
Neverthless -> Nevertheless
varaible -> variable

(actually this is my spell checker who complains, I was fine with the original version :])

[ruiu]

Looks like it is not easy to make changes to a patch in a patch series as it causes merge conflicts. It's still better than merging them into one big patch and send it for review, but splitting a patch makes my life a bit harder when it comes to updating it. Do you have any critical comments that need to be addressed? I'll make changes as you guys requested before committing, so if this patch looks generally OK, could you stamp on it for me?

lld/ELF/SymbolTable.cpp

496 ↗

(On Diff #199422)

Yes, I guess so. I'll do that.

[grimar]

Generally it looks good to me.

[ruiu]

Thanks! I'll commit this tomorrow.

[MaskRay]

lld/ELF/Driver.cpp

1697 ↗

(On Diff #199422)

regular defined symbols?

lld/ELF/Symbols.h

238 ↗

(On Diff #199422)

The comment may mention this is for C tentative definitions (its origin is Fortran COMMON blocks, which may predate Unix...)

[ruiu]

lld/ELF/SymbolTable.cpp

155 ↗

(On Diff #199422)

Currently the first symbol needs to be merged with itself. I'll try to fix later.

320 ↗

(On Diff #199422)

I'm fine in either way.

lld/trunk/ELF/Driver.cpp

if (auto *LO = dyn_cast<LazyObject>(Sym))
     MB = LO->File->MB;
   else
     MB = cast<LazyArchive>(Sym)->getMemberBuffer();
 
   if (isBitcode(MB))
     Symtab->fetchLazy<ELFT>(Sym);
 }
 
+// Replaces common symbols with defined symbols reside in .bss sections.
+// This function is called after all symbol names are resolved. As a
+// result, the passes after the symbol resolution won't see any
+// symbols of type CommonSymbol.
+static void replaceCommonSymbols() {
+  for (Symbol *Sym : Symtab->getSymbols()) {
+    auto *S = dyn_cast<CommonSymbol>(Sym);
+    if (!S)
+      continue;
+
+    auto *Bss = make<BssSection>("COMMON", S->Size, S->Alignment);
+    Bss->File = S->File;
+    Bss->Live = !Config->GcSections;
+    InputSections.push_back(Bss);
+
+    Defined New(S->File, S->getName(), S->Binding, S->StOther, S->Type,
+                /*Value=*/0, S->Size, Bss);
+    replaceSymbol(S, &New);
+  }
+}
+
 // If all references to a DSO happen to be weak, the DSO is not added
 // to DT_NEEDED. If that happens, we need to eliminate shared symbols
 // created from the DSO. Otherwise, they become dangling references
 // that point to a non-existent DSO.
 static void demoteSharedSymbols() {
   for (Symbol *Sym : Symtab->getSymbols()) {
     auto *S = dyn_cast<SharedSymbol>(Sym);
     if (!S || S->getFile().IsNeeded)
if (Config->ARMHasBlx == false)
            "feature detected");
   }
 
   // This adds a .comment section containing a version string. We have to add it
   // before mergeSections because the .comment section is a mergeable section.
   if (!Config->Relocatable)
     InputSections.push_back(createCommentSection());
 
+  // Replace common symbols with regular symbols.
+  replaceCommonSymbols();
+
   // Do size optimizations: garbage collection, merging of SHF_MERGE sections
   // and identical code folding.
   splitSections<ELFT>();
   markLive<ELFT>();
   demoteSharedSymbols();
   mergeSections();
   if (Config->ICF != ICFLevel::None) {
     findKeepUniqueSections<ELFT>(Args);

lld/trunk/ELF/InputFiles.cpp

template <class ELFT> Symbol *ObjFile<ELFT>::createSymbol(const Elf_Sym *Sym) {
   case SHN_UNDEF:
     return Symtab->addUndefined<ELFT>(
         Undefined{this, Name, Binding, StOther, Type});
   case SHN_COMMON:
     if (Value == 0 || Value >= UINT32_MAX)
       fatal(toString(this) + ": common symbol '" + Name +
             "' has invalid alignment: " + Twine(Value));
     return Symtab->addCommon(
-        Defined{this, Name, Binding, StOther, Type, Value, Size, nullptr});
+        CommonSymbol{this, Name, Binding, StOther, Type, Value, Size});
   }
 
   switch (Binding) {
   default:
     fatal(toString(this) + ": unexpected binding: " + Twine((int)Binding));
   case STB_GLOBAL:
   case STB_WEAK:
   case STB_GNU_UNIQUE:
static Symbol *createBitcodeSymbol(const std::vector<bool> &KeptComdats,
   if (ObjSym.isUndefined() || (C != -1 && !KeptComdats[C])) {
     Undefined New(&F, Name, Binding, Visibility, Type);
     if (CanOmitFromDynSym)
       New.ExportDynamic = false;
     return Symtab->addUndefined<ELFT>(New);
   }
 
   if (ObjSym.isCommon())
-    return Symtab->addCommon(Defined{&F, Name, Binding, Visibility, STT_OBJECT,
-                                     ObjSym.getCommonAlignment(),
-                                     ObjSym.getCommonSize(), nullptr});
+    return Symtab->addCommon(
+        CommonSymbol{&F, Name, Binding, Visibility, STT_OBJECT,
+                     ObjSym.getCommonAlignment(), ObjSym.getCommonSize()});
 
   Defined New(&F, Name, Binding, Visibility, Type, 0, 0, nullptr);
   if (CanOmitFromDynSym)
     New.ExportDynamic = false;
   return Symtab->addBitcode(New);
 }
 
 template <class ELFT>

lld/trunk/ELF/LinkerScript.cpp

void LinkerScript::addSymbol(SymbolAssignment *Cmd) {
   // We want to set symbol values early if we can. This allows us to
   // use symbols as variables in linker scripts. Doing so allows us to
   // write expressions like this: `alignment = 16; . = ALIGN(., alignment)`.
   uint64_t SymValue = Value.Sec ? 0 : Value.getValue();
 
   Defined New(nullptr, Cmd->Name, STB_GLOBAL, Visibility, STT_NOTYPE, SymValue,
               0, Sec);
 
-  Symbol *Sym;
-  std::tie(Sym, std::ignore) = Symtab->insert(New);
+  Symbol *Sym = Symtab->insert(New);
+  Symtab->mergeProperties(Sym, New);
   replaceSymbol(Sym, &New);
   Cmd->Sym = cast<Defined>(Sym);
 }
 
 // This function is called from LinkerScript::declareSymbols.
 // It creates a placeholder symbol if needed.
 static void declareSymbol(SymbolAssignment *Cmd) {
   if (!shouldDefineSym(Cmd))
     return;
 
   uint8_t Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
   Defined New(nullptr, Cmd->Name, STB_GLOBAL, Visibility, STT_NOTYPE, 0, 0,
               nullptr);
 
   // We can't calculate final value right now.
-  Symbol *Sym;
-  std::tie(Sym, std::ignore) = Symtab->insert(New);
+  Symbol *Sym = Symtab->insert(New);
+  Symtab->mergeProperties(Sym, New);
   replaceSymbol(Sym, &New);
 
   Cmd->Sym = cast<Defined>(Sym);
   Cmd->Provide = false;
   Sym->ScriptDefined = true;
 }
 
 // This method is used to handle INSERT AFTER statement. Here we rebuild

lld/trunk/ELF/SymbolTable.h

 #include "LTO.h"
 #include "lld/Common/Strings.h"
 #include "llvm/ADT/CachedHashString.h"
 #include "llvm/ADT/DenseMap.h"
 
 namespace lld {
 namespace elf {
 
+class CommonSymbol;
 class Defined;
 class LazyArchive;
 class LazyObject;
 class SectionBase;
 class SharedSymbol;
 class Undefined;
 
 // SymbolTable is a bucket of all known symbols, including defined,

 public:
   template <class ELFT> void addCombinedLTOObject();
   void wrap(Symbol *Sym, Symbol *Real, Symbol *Wrap);
 
   ArrayRef<Symbol *> getSymbols() const { return SymVector; }
 
   template <class ELFT> Symbol *addUndefined(const Undefined &New);
 
-  Defined *addDefined(const Defined &New);
+  Symbol *addDefined(const Defined &New);
 
   void addShared(const SharedSymbol &New);
 
   template <class ELFT> void addLazyArchive(const LazyArchive &New);
   template <class ELFT> void addLazyObject(const LazyObject &New);
 
   Symbol *addBitcode(const Defined &New);
-  Symbol *addCommon(const Defined &New);
+  Symbol *addCommon(const CommonSymbol &New);
 
-  std::pair<Symbol *, bool> insert(const Symbol &New);
+  Symbol *insert(const Symbol &New);
+  void mergeProperties(Symbol *Old, const Symbol &New);
 
   template <class ELFT> void fetchLazy(Symbol *Sym);
 
   void scanVersionScript();
 
   Symbol *find(StringRef Name);
 
   void trace(StringRef Name);

lld/trunk/ELF/SymbolTable.cpp

 static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) {
   if (VA == STV_DEFAULT)
     return VB;
   if (VB == STV_DEFAULT)
     return VA;
   return std::min(VA, VB);
 }
 
-// Find an existing symbol or create and insert a new one, then apply the given
-// attributes.
-std::pair<Symbol *, bool> SymbolTable::insert(const Symbol &New) {
-  // Find an existing symbol or create and insert a new one.
-
+Symbol *SymbolTable::insert(const Symbol &New) {
   // <name>@@<version> means the symbol is the default version. In that
   // case <name>@@<version> will be used to resolve references to <name>.
   //
   // Since this is a hot path, the following string search code is
   // optimized for speed. StringRef::find(char) is much faster than
   // StringRef::find(StringRef).
   StringRef Name = New.getName();
   size_t Pos = Name.find('@');
if (IsNew) {
     Old->ExportDynamic = false;
     Old->CanInline = true;
     Old->Traced = Traced;
     Old->ScriptDefined = false;
   } else {
     Old = SymVector[SymIndex];
   }
 
+  return Old;
+}
+
+// Merge symbol properties.
+//
+// When we have many symbols of the same name, we choose one of them,
+// and that's the result of symbol resolution. However, symbols that
+// were not chosen still affect some symbol properteis.
+void SymbolTable::mergeProperties(Symbol *Old, const Symbol &New) {
   // Merge symbol properties.
   Old->ExportDynamic = Old->ExportDynamic || New.ExportDynamic;
   Old->IsUsedInRegularObj = Old->IsUsedInRegularObj || New.IsUsedInRegularObj;
 
   // DSO symbols do not affect visibility in the output.
-  if (!isa<SharedSymbol>(&New))
+  if (!isa<SharedSymbol>(New))
     Old->Visibility = getMinVisibility(Old->Visibility, New.Visibility);
-
-  return {Old, IsNew};
 }
 
 template <class ELFT> Symbol *SymbolTable::addUndefined(const Undefined &New) {
-  Symbol *Old;
-  bool WasInserted;
-  std::tie(Old, WasInserted) = insert(New);
+  Symbol *Old = insert(New);
+  mergeProperties(Old, New);
+
+  if (Old->isPlaceholder()) {
+    replaceSymbol(Old, &New);
+    return Old;
+  }
 
   // An undefined symbol with non default visibility must be satisfied
   // in the same DSO.
-  if (WasInserted ||
-      (isa<SharedSymbol>(Old) && New.Visibility != STV_DEFAULT)) {
+  if (Old->isShared() && New.Visibility != STV_DEFAULT) {
     replaceSymbol(Old, &New);
     return Old;
   }
 
   if (Old->isShared() || Old->isLazy() ||
       (Old->isUndefined() && New.Binding != STB_WEAK))
     Old->Binding = New.Binding;
 
static int compareVersion(StringRef OldName, StringRef NewName) {
   bool B = NewName.contains("@@");
   if (!A && B)
     return 1;
   if (A && !B)
     return -1;
   return 0;
 }
 
-// We have a new defined symbol with the specified binding. Return 1 if the new
-// symbol should win, -1 if the new symbol should lose, or 0 if both symbols are
-// strong defined symbols.
-static int compareDefined(const Symbol *Old, const Symbol *New) {
-  if (!Old->isDefined())
+// Compare two symbols. Return 1 if the new symbol should win, -1 if
+// the new symbol should lose, or 0 if there is a conflict.
+static int compare(const Symbol *Old, const Symbol *New) {
+  assert(New->isDefined() || New->isCommon());
+
+  if (!Old->isDefined() && !Old->isCommon())
     return 1;
+
   if (int Cmp = compareVersion(Old->getName(), New->getName()))
     return Cmp;
-  if (New->Binding == STB_WEAK)
+
+  if (New->isWeak())
     return -1;
+
   if (Old->isWeak())
     return 1;
+
+  if (Old->isCommon() && New->isCommon()) {
+    if (Config->WarnCommon)
+      warn("multiple common of " + Old->getName());
     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(const Symbol *OldSym, const Defined *New) {
-  if (int Cmp = compareDefined(OldSym, New))
-    return Cmp;
-
-  if (auto *Old = dyn_cast<Defined>(OldSym)) {
-    if (Old->Section && isa<BssSection>(Old->Section)) {
-      // Non-common symbols take precedence over common symbols.
-      if (Config->WarnCommon)
-        warn("common " + Old->getName() + " is overridden");
-      return 1;
-    }
+  if (Old->isCommon()) {
 
-    if (New->File && isa<BitcodeFile>(New->File))
-      return 0;
+  if (New->isCommon()) {
+    if (Config->WarnCommon)
+      warn("common " + Old->getName() + " is overridden");
+    return -1;
+  }
+
+  auto *OldSym = cast<Defined>(Old);
+  auto *NewSym = cast<Defined>(New);
+
 
-    if (Old->Section == nullptr && New->Section == nullptr &&
-        Old->Value == New->Value && New->Binding == STB_GLOBAL)
-      return -1;
-  }
+  if (!OldSym->Section && !NewSym->Section && OldSym->Value == NewSym->Value &&
+      NewSym->Binding == STB_GLOBAL)
+
   return 0;
 }
 
-Symbol *SymbolTable::addCommon(const Defined &New) {
-  Symbol *Old;
-  bool WasInserted;
-  std::tie(Old, WasInserted) = insert(New);
-
-  auto Replace = [&] {
-    auto *Bss = make<BssSection>("COMMON", New.Size, New.Value);
-    Bss->File = New.File;
-    Bss->Live = !Config->GcSections;
-    InputSections.push_back(Bss);
-
-    Defined Sym = New;
-    Sym.Value = 0;
-    Sym.Section = Bss;
-    replaceSymbol(Old, &Sym);
-  };
+Symbol *SymbolTable::addCommon(const CommonSymbol &New) {
+  Symbol *Old = insert(New);
+  mergeProperties(Old, New);
 
-  if (WasInserted) {
-    Replace();
+  if (Old->isPlaceholder()) {
+    replaceSymbol(Old, &New);
     return Old;
   }
 
-  int Cmp = compareDefined(Old, &New);
+  int Cmp = compare(Old, &New);
   if (Cmp < 0)
     return Old;
 
   if (Cmp > 0) {
-    Replace();
-    return Old;
-  }
-
-  auto *D = cast<Defined>(Old);
-  auto *Bss = dyn_cast_or_null<BssSection>(D->Section);
-  if (!Bss) {
-    // Non-common symbols take precedence over common symbols.
-    if (Config->WarnCommon)
-      warn("common " + Old->getName() + " is overridden");
+    replaceSymbol(Old, &New);
     return Old;
   }
 
-  if (Config->WarnCommon)
-    warn("multiple common of " + D->getName());
+  CommonSymbol *OldSym = cast<CommonSymbol>(Old);
 
-  Bss->Alignment = std::max<uint32_t>(Bss->Alignment, New.Value);
-  if (New.Size > Bss->Size) {
-    D->File = Bss->File = New.File;
-    D->Size = Bss->Size = New.Size;
+  OldSym->Alignment = std::max(OldSym->Alignment, New.Alignment);
+  if (OldSym->Size < New.Size) {
+    OldSym->File = New.File;
+    OldSym->Size = New.Size;
   }
-  return Old;
+  return OldSym;
 }
 
 static void reportDuplicate(Symbol *Sym, InputFile *NewFile,
                             InputSectionBase *ErrSec, uint64_t ErrOffset) {
   if (Config->AllowMultipleDefinition)
     return;
 
   Defined *D = cast<Defined>(Sym);
if (!Src1.empty())
     Msg += Src1 + "\n>>>            ";
   Msg += Obj1 + "\n>>> defined at ";
   if (!Src2.empty())
     Msg += Src2 + "\n>>>            ";
   Msg += Obj2;
   error(Msg);
 }
 
-Defined *SymbolTable::addDefined(const Defined &New) {
-  Symbol *Old;
-  bool WasInserted;
-  std::tie(Old, WasInserted) = insert(New);
+Symbol *SymbolTable::addDefined(const Defined &New) {
+  Symbol *Old = insert(New);
+  mergeProperties(Old, New);
 
-  if (WasInserted) {
+  if (Old->isPlaceholder()) {
     replaceSymbol(Old, &New);
-    return cast<Defined>(Old);
+    return Old;
   }
 
-  int Cmp = compareDefinedNonCommon(Old, &New);
+  int Cmp = compare(Old, &New);
   if (Cmp > 0)
     replaceSymbol(Old, &New);
   else if (Cmp == 0)
     reportDuplicate(Old, New.File,
                     dyn_cast_or_null<InputSectionBase>(New.Section), New.Value);
-  return cast<Defined>(Old);
+  return Old;
 }
 
 void SymbolTable::addShared(const SharedSymbol &New) {
-  Symbol *Old;
-  bool WasInserted;
-  std::tie(Old, WasInserted) = insert(New);
+  Symbol *Old = insert(New);
+  mergeProperties(Old, New);
 
   // Make sure we preempt DSO symbols with default visibility.
   if (New.Visibility == STV_DEFAULT)
     Old->ExportDynamic = true;
 
-  if (WasInserted) {
+  if (Old->isPlaceholder()) {
     replaceSymbol(Old, &New);
-  } else if (Old->Visibility == STV_DEFAULT &&
-             (Old->isUndefined() || Old->isLazy())) {
+    return;
+  }
+
+  if (Old->Visibility == STV_DEFAULT && (Old->isUndefined() || Old->isLazy())) {
     // An undefined symbol with non default visibility must be satisfied
     // in the same DSO.
     uint8_t Binding = Old->Binding;
     replaceSymbol(Old, &New);
     Old->Binding = Binding;
   }
 }
 
 Symbol *SymbolTable::addBitcode(const Defined &New) {
-  Symbol *Old;
-  bool WasInserted;
-  std::tie(Old, WasInserted) = insert(New);
+  Symbol *Old = insert(New);
+  mergeProperties(Old, New);
 
-  if (WasInserted) {
+  if (Old->isPlaceholder()) {
     replaceSymbol(Old, &New);
     return Old;
   }
 
-  int Cmp = compareDefinedNonCommon(Old, &New);
+  int Cmp = compare(Old, &New);
   if (Cmp > 0)
     replaceSymbol(Old, &New);
   else if (Cmp == 0)
     reportDuplicate(Old, New.File, nullptr, 0);
   return Old;
 }
 
 Symbol *SymbolTable::find(StringRef Name) {
   auto It = SymMap.find(CachedHashStringRef(Name));
   if (It == SymMap.end())
     return nullptr;
   if (It->second == -1)
     return nullptr;
   return SymVector[It->second];
 }
 
 template <class ELFT, class LazyT> void SymbolTable::addLazy(const LazyT &New) {
-  Symbol *Old;
-  bool WasInserted;
-  std::tie(Old, WasInserted) = insert(New);
+  Symbol *Old = insert(New);
+  mergeProperties(Old, New);
 
-  if (WasInserted) {
+  if (Old->isPlaceholder()) {
     replaceSymbol(Old, &New);
     return;
   }
 
   if (!Old->isUndefined())
     return;
 
   // An undefined weak will not fetch archive members. See comment on Lazy in

 // "llvm::*::foo(int, ?)". Obviously, there's no way to handle this
 // other than trying to match a pattern against all demangled symbols.
 // So, if "extern C++" feature is used, we need to demangle all known
 // symbols.
 StringMap<std::vector<Symbol *>> &SymbolTable::getDemangledSyms() {
   if (!DemangledSyms) {
     DemangledSyms.emplace();
     for (Symbol *Sym : SymVector) {
-      if (!Sym->isDefined())
+      if (!Sym->isDefined() && !Sym->isCommon())
         continue;
       if (Optional<std::string> S = demangleItanium(Sym->getName()))
         (*DemangledSyms)[*S].push_back(Sym);
       else
         (*DemangledSyms)[Sym->getName()].push_back(Sym);
     }
   }
   return *DemangledSyms;
 }
 
 std::vector<Symbol *> SymbolTable::findByVersion(SymbolVersion Ver) {
   if (Ver.IsExternCpp)
     return getDemangledSyms().lookup(Ver.Name);
   if (Symbol *B = find(Ver.Name))
-    if (B->isDefined())
+    if (B->isDefined() || B->isCommon())
       return {B};
   return {};
 }
 
 std::vector<Symbol *> SymbolTable::findAllByVersion(SymbolVersion Ver) {
   std::vector<Symbol *> Res;
   StringMatcher M(Ver.Name);
 
   if (Ver.IsExternCpp) {
     for (auto &P : getDemangledSyms())
       if (M.match(P.first()))
         Res.insert(Res.end(), P.second.begin(), P.second.end());
     return Res;
   }
 
   for (Symbol *Sym : SymVector)
-    if (Sym->isDefined() && M.match(Sym->getName()))
+    if ((Sym->isDefined() || Sym->isCommon()) && M.match(Sym->getName()))
       Res.push_back(Sym);
   return Res;
 }
 
 // If there's only one anonymous version definition in a version
 // script file, the script does not actually define any symbol version,
 // but just specifies symbols visibilities.
 void SymbolTable::handleAnonymousVersion() {

lld/trunk/ELF/Symbols.h

 };
 
 // The base class for real symbol classes.
 class Symbol {
 public:
   enum Kind {
     PlaceholderKind,
     DefinedKind,
+    CommonKind,
     SharedKind,
     UndefinedKind,
     LazyArchiveKind,
     LazyObjectKind,
   };
 
   Kind kind() const { return static_cast<Kind>(SymbolKind); }
 
public:
   // True if this symbol is specified by --trace-symbol option.
   unsigned Traced : 1;
 
   bool includeInDynsym() const;
   uint8_t computeBinding() const;
   bool isWeak() const { return Binding == llvm::ELF::STB_WEAK; }
 
   bool isUndefined() const { return SymbolKind == UndefinedKind; }
+  bool isCommon() const { return SymbolKind == CommonKind; }
   bool isDefined() const { return SymbolKind == DefinedKind; }
   bool isShared() const { return SymbolKind == SharedKind; }
+  bool isPlaceholder() const { return SymbolKind == PlaceholderKind; }
+
   bool isLocal() const { return Binding == llvm::ELF::STB_LOCAL; }
 
   bool isLazy() const {
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
 
   // True if this is an undefined weak symbol. This only works once
   // all input files have been added.
public:
 
   static bool classof(const Symbol *S) { return S->isDefined(); }
 
   uint64_t Value;
   uint64_t Size;
   SectionBase *Section;
 };
 
+// Represents a common symbol.
+//
+// On Unix, it is traditionally allowed to write variable definitions
+// without initialization expressions (such as "int foo;") to header
+// files. Such definition is called "tentative definition".
+//
+// Using tentative definition is usually considered a bad practice
+// because you should write only declarations (such as "extern int
+// foo;") to header files. Nevertheless, the linker and the compiler
+// have to do something to support bad code by allowing duplicate
+// definitions for this particular case.
+//
+// Common symbols represent variable definitions without initializations.
+// The compiler creates common symbols when it sees varaible definitions
+// without initialization (you can suppress this behavior and let the
+// compiler create a regular defined symbol by -fno-common).
+//
+// The linker allows common symbols to be replaced by regular defined
+// symbols. If there are remaining common symbols after name resolution is
+// complete, they are converted to regular defined symbols in a .bss
+// section. (Therefore, the later passes don't see any CommonSymbols.)
+class CommonSymbol : public Symbol {
+public:
+  CommonSymbol(InputFile *File, StringRefZ Name, uint8_t Binding,
+               uint8_t StOther, uint8_t Type, uint64_t Alignment, uint64_t Size)
+      : Symbol(CommonKind, File, Name, Binding, StOther, Type),
+        Alignment(Alignment), Size(Size) {}
+
+  static bool classof(const Symbol *S) { return S->isCommon(); }
+
+  uint32_t Alignment;
+  uint64_t Size;
+};
+
 class Undefined : public Symbol {
 public:
   Undefined(InputFile *File, StringRefZ Name, uint8_t Binding, uint8_t StOther,
             uint8_t Type)
       : Symbol(UndefinedKind, File, Name, Binding, StOther, Type) {}
 
   static bool classof(const Symbol *S) { return S->kind() == UndefinedKind; }
 };

lld/trunk/ELF/Symbols.cpp

static uint64_t getSymVA(const Symbol &Sym, int64_t &Addend) {
   }
   case Symbol::SharedKind:
   case Symbol::UndefinedKind:
     return 0;
   case Symbol::LazyArchiveKind:
   case Symbol::LazyObjectKind:
     assert(Sym.IsUsedInRegularObj && "lazy symbol reached writer");
     return 0;
+  case Symbol::CommonKind:
+    llvm_unreachable("common symbol reached writer");
   case Symbol::PlaceholderKind:
     llvm_unreachable("placeholder symbol reached writer");
   }
   llvm_unreachable("invalid symbol kind");
 }
 
 uint64_t Symbol::getVA(int64_t Addend) const {
   uint64_t OutVA = getSymVA(*this, Addend);

 void elf::printTraceSymbol(Symbol *Sym) {
   std::string S;
   if (Sym->isUndefined())
     S = ": reference to ";
   else if (Sym->isLazy())
     S = ": lazy definition of ";
   else if (Sym->isShared())
     S = ": shared definition of ";
-  else if (dyn_cast_or_null<BssSection>(cast<Defined>(Sym)->Section))
+  else if (Sym->isCommon())
     S = ": common definition of ";
   else
     S = ": definition of ";
 
   message(toString(Sym->File) + S + Sym->getName());
 }
 
 void elf::maybeWarnUnorderableSymbol(const Symbol *Sym) {

lld/trunk/ELF/Writer.cpp

 
 static Defined *addOptionalRegular(StringRef Name, SectionBase *Sec,
                                    uint64_t Val, uint8_t StOther = STV_HIDDEN,
                                    uint8_t Binding = STB_GLOBAL) {
   Symbol *S = Symtab->find(Name);
   if (!S || S->isDefined())
     return nullptr;
 
-  return Symtab->addDefined(Defined{/*File=*/nullptr, Name, Binding, StOther,
-                                    STT_NOTYPE, Val,
-                                    /*Size=*/0, Sec});
+  return cast<Defined>(Symtab->addDefined(
+      Defined{/*File=*/nullptr, Name, Binding, StOther, STT_NOTYPE, Val,
+              /*Size=*/0, Sec}));
 }
 
 static Defined *addAbsolute(StringRef Name) {
-  return Symtab->addDefined(Defined{nullptr, Name, STB_GLOBAL, STV_HIDDEN,
-                                    STT_NOTYPE, 0, 0, nullptr});
+  Defined New(nullptr, Name, STB_GLOBAL, STV_HIDDEN, STT_NOTYPE, 0, 0, nullptr);
+  Symbol *Sym = Symtab->addDefined(New);
+  if (!Sym->isDefined())
+    error("duplicate symbol: " + toString(*Sym));
+  return cast<Defined>(Sym);
 }
 
 // The linker is expected to define some symbols depending on
 // the linking result. This function defines such symbols.
 void elf::addReservedSymbols() {
   if (Config->EMachine == EM_MIPS) {
     // Define _gp for MIPS. st_value of _gp symbol will be updated by Writer
     // so that it points to an absolute address which by default is relative
if (S->isDefined()) {
             GotSymName + "'");
       return;
     }
 
     uint64_t GotOff = 0;
     if (Config->EMachine == EM_PPC || Config->EMachine == EM_PPC64)
       GotOff = 0x8000;
 
-    ElfSym::GlobalOffsetTable =
-        Symtab->addDefined(Defined{/*File=*/nullptr, GotSymName, STB_GLOBAL,
-                                   STV_HIDDEN, STT_NOTYPE, GotOff,
-                                   /*Size=*/0, Out::ElfHeader});
+                               STV_HIDDEN, STT_NOTYPE, GotOff, /*Size=*/0,
+                               Out::ElfHeader});
+    ElfSym::GlobalOffsetTable = cast<Defined>(S);
   }
 
   // __ehdr_start is the location of ELF file headers. Note that we define
   // this symbol unconditionally even when using a linker script, which
   // differs from the behavior implemented by GNU linker which only define
   // this symbol if ELF headers are in the memory mapped segment.
   addOptionalRegular("__ehdr_start", Out::ElfHeader, 0, STV_HIDDEN);