Reland r319090, "COFF: Do not create SectionChunks for discarded comdat sections." with a fix for debug sections.

[pcc]

Summary

If /debug was not specified, readSection will return a null
pointer for debug sections. If the debug section is associative with
another section, we need to make sure that the section returned from
readSection is not a null pointer before adding it as an associative
section.

[pcc]

Diff against previous patch:

diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 0ecfbdaf7373..d0c7bcaefe06 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -208,7 +208,8 @@ void ObjFile::readAssociativeDefinition(
   int32_t SectionNumber = Sym.getSectionNumber();
   if (Parent) {
     SparseChunks[SectionNumber] = readSection(SectionNumber, Def);
-    Parent->addAssociative(SparseChunks[SectionNumber]);
+    if (SparseChunks[SectionNumber])
+      Parent->addAssociative(SparseChunks[SectionNumber]);
   } else {
     SparseChunks[SectionNumber] = nullptr;
   }
diff --git a/lld/test/COFF/pdb-comdat.test b/lld/test/COFF/pdb-comdat.test
index a3283d206ab8..655f215e0199 100644
--- a/lld/test/COFF/pdb-comdat.test
+++ b/lld/test/COFF/pdb-comdat.test
@@ -107,3 +107,6 @@ REORDER-LABEL:   Mod 0001 | `{{.*}}pdb_comdat_main.obj`:
 REORDER:       c:\src\llvm-project\build\pdb_comdat_main.c
 REORDER-NOT:       c:\src\llvm-project\build\foo.h
 REORDER-LABEL:   Mod 0002 | `* Linker *`:
+
+Make sure that we don't crash on non-prevailing debug sections if -debug is not enabled.
+RUN: lld-link pdb_comdat_main.obj pdb_comdat_bar.obj -out:%t.exe -nodefaultlib -entry:main

[ruiu]

LGTM

lld/trunk/COFF/Chunks.h

public:
   // in a final output. It's supposed to print out a log message to stdout.
   void printDiscardedMessage() const;
 
   // Adds COMDAT associative sections to this COMDAT section. A chunk
   // and its children are treated as a group by the garbage collector.
   void addAssociative(SectionChunk *Child);
 
   StringRef getDebugName() override;
-  void setSymbol(DefinedRegular *S) { if (!Sym) Sym = S; }
 
-  // Returns true if the chunk was not dropped by GC or COMDAT deduplication.
-  bool isLive() { return Live && !Discarded; }
+  // Returns true if the chunk was not dropped by GC.
+  bool isLive() { return Live; }
 
   // Used by the garbage collector.
   void markLive() {
     assert(Config->DoGC && "should only mark things live from GC");
     assert(!isLive() && "Cannot mark an already live section!");
     Live = true;
   }
 
-  // Returns true if this chunk was dropped by COMDAT deduplication.
-  bool isDiscarded() const { return Discarded; }
-
-  // Used by the SymbolTable when discarding unused comdat sections. This is
-  // redundant when GC is enabled, as all comdat sections will start out dead.
-  void markDiscarded() { Discarded = true; }
-
   // True if this is a codeview debug info chunk. These will not be laid out in
   // the image. Instead they will end up in the PDB, if one is requested.
   bool isCodeView() const {
     return SectionName == ".debug" || SectionName.startswith(".debug$");
   }
 
   // True if this is a DWARF debug info or exception handling chunk.
   bool isDWARF() const {
public:
   // Auxiliary Format 5: Section Definitions. Used for ICF.
   uint32_t Checksum = 0;
 
   const coff_section *Header;
 
   // The file that this chunk was created from.
   ObjFile *File;
 
+  // The COMDAT leader symbol if this is a COMDAT chunk.
+  DefinedRegular *Sym = nullptr;
+
 private:
   StringRef SectionName;
   std::vector<SectionChunk *> AssocChildren;
   llvm::iterator_range<const coff_relocation *> Relocs;
   size_t NumRelocs;
 
-  // True if this chunk was discarded because it was a duplicate comdat section.
-  bool Discarded;
-
   // Used by the garbage collector.
   bool Live;
 
   // Used for ICF (Identical COMDAT Folding)
   void replace(SectionChunk *Other);
   uint32_t Class[2] = {0, 0};
-
-  // Sym points to a section symbol if this is a COMDAT chunk.
-  DefinedRegular *Sym = nullptr;
 };
 
 // A chunk for common symbols. Common chunks don't have actual data.
 class CommonChunk : public Chunk {
 public:
   CommonChunk(const COFFSymbolRef Sym);
   size_t getSize() const override { return Sym.getValue(); }
   bool hasData() const override { return false; }

lld/trunk/COFF/Chunks.cpp

SectionChunk::SectionChunk(ObjFile *F, const coff_section *H)
     : Chunk(SectionKind), Repl(this), Header(H), File(F),
       Relocs(File->getCOFFObj()->getRelocations(Header)),
       NumRelocs(std::distance(Relocs.begin(), Relocs.end())) {
   // Initialize SectionName.
   File->getCOFFObj()->getSectionName(Header, SectionName);
 
   Alignment = Header->getAlignment();
 
-  // Chunks may be discarded during comdat merging.
-  Discarded = false;
-
   // If linker GC is disabled, every chunk starts out alive.  If linker GC is
   // enabled, treat non-comdat sections as roots. Generally optimized object
   // files will be built with -ffunction-sections or /Gy, so most things worth
   // stripping will be in a comdat.
   Live = !Config->DoGC || !isCOMDAT();
 }
 
 static void add16(uint8_t *P, int16_t V) { write16le(P, read16le(P) + V); }

 
 bool SectionChunk::isCOMDAT() const {
   return Header->Characteristics & IMAGE_SCN_LNK_COMDAT;
 }
 
 void SectionChunk::printDiscardedMessage() const {
   // Removed by dead-stripping. If it's removed by ICF, ICF already
   // printed out the name, so don't repeat that here.
-  if (Sym && this == Repl) {
-    if (Discarded)
-      message("Discarded comdat symbol " + Sym->getName());
-    else if (!Live)
-      message("Discarded " + Sym->getName());
-  }
-}
+  if (Sym && this == Repl)
 
 StringRef SectionChunk::getDebugName() {
   if (Sym)
     return Sym->getName();
   return "";
 }
 
 ArrayRef<uint8_t> SectionChunk::getContents() const {

lld/trunk/COFF/InputFiles.h

public:
   // if we are not producing a PDB.
   llvm::pdb::DbiModuleDescriptorBuilder *ModuleDBI = nullptr;
 
 private:
   void initializeChunks();
   void initializeSymbols();
   void initializeSEH();
 
-  Symbol *createDefined(COFFSymbolRef Sym, const void *Aux, bool IsFirst);
+  SectionChunk *
+  readSection(uint32_t SectionNumber,
+              const llvm::object::coff_aux_section_definition *Def);
+
+  void readAssociativeDefinition(
+      COFFSymbolRef COFFSym,
+      const llvm::object::coff_aux_section_definition *Def);
+
+  llvm::Optional<Symbol *>
+  createDefined(COFFSymbolRef Sym,
+                std::vector<const llvm::object::coff_aux_section_definition *>
+                    &ComdatDefs);
+  Symbol *createRegular(COFFSymbolRef Sym);
   Symbol *createUndefined(COFFSymbolRef Sym);
 
   std::unique_ptr<COFFObjectFile> COFFObj;
   const coff_section *SXData = nullptr;
 
   // List of all chunks defined by this file. This includes both section
   // chunks and non-section chunks for common symbols.
   std::vector<Chunk *> Chunks;

lld/trunk/COFF/InputFiles.cpp

void ObjFile::parse() {
   }
 
   // Read section and symbol tables.
   initializeChunks();
   initializeSymbols();
   initializeSEH();
 }
 
+// We set SectionChunk pointers in the SparseChunks vector to this value
+// temporarily to mark comdat sections as having an unknown resolution. As we
+// walk the object file's symbol table, once we visit either a leader symbol or
+// an associative section definition together with the parent comdat's leader,
+// we set the pointer to either nullptr (to mark the section as discarded) or a
+// valid SectionChunk for that section.
+static SectionChunk *const PendingComdat = reinterpret_cast<SectionChunk *>(1);
+
 void ObjFile::initializeChunks() {
   uint32_t NumSections = COFFObj->getNumberOfSections();
   Chunks.reserve(NumSections);
   SparseChunks.resize(NumSections + 1);
   for (uint32_t I = 1; I < NumSections + 1; ++I) {
     const coff_section *Sec;
-    StringRef Name;
     if (auto EC = COFFObj->getSection(I, Sec))
       fatal("getSection failed: #" + Twine(I) + ": " + EC.message());
-    if (auto EC = COFFObj->getSectionName(Sec, Name))
-      fatal("getSectionName failed: #" + Twine(I) + ": " + EC.message());
-    if (Name == ".sxdata") {
-      SXData = Sec;
-      continue;
-    }
-    if (Name == ".drectve") {
-      ArrayRef<uint8_t> Data;
-      COFFObj->getSectionContents(Sec, Data);
-      Directives = std::string((const char *)Data.data(), Data.size());
-      continue;
-    }
+
+    if (Sec->Characteristics & IMAGE_SCN_LNK_COMDAT)
+      SparseChunks[I] = PendingComdat;
+    else
+      SparseChunks[I] = readSection(I, nullptr);
+  }
+}
+
+SectionChunk *ObjFile::readSection(uint32_t SectionNumber,
+                                   const coff_aux_section_definition *Def) {
+  const coff_section *Sec;
+  StringRef Name;
+  if (auto EC = COFFObj->getSection(SectionNumber, Sec))
+    fatal("getSection failed: #" + Twine(SectionNumber) + ": " + EC.message());
+    fatal("getSectionName failed: #" + Twine(SectionNumber) + ": " +
+          EC.message());
+    return nullptr;
+    return nullptr;
 
-    // Object files may have DWARF debug info or MS CodeView debug info
-    // (or both).
-    //
-    // DWARF sections don't need any special handling from the perspective
-    // of the linker; they are just a data section containing relocations.
-    // We can just link them to complete debug info.
-    //
-    // CodeView needs a linker support. We need to interpret and debug
-    // info, and then write it to a separate .pdb file.
 
-    // Ignore debug info unless /debug is given.
-    if (!Config->Debug && Name.startswith(".debug"))
-      continue;
+    return nullptr;
 
-    if (Sec->Characteristics & llvm::COFF::IMAGE_SCN_LNK_REMOVE)
-      continue;
-    auto *C = make<SectionChunk>(this, Sec);
+    return nullptr;
+  if (Def)
+    C->Checksum = Def->CheckSum;
 
-    // CodeView sections are stored to a different vector because they are not
-    // linked in the regular manner.
-    if (C->isCodeView())
-      DebugChunks.push_back(C);
-    else
-      Chunks.push_back(C);
 
-    SparseChunks[I] = C;
+  return C;
+}
+
+void ObjFile::readAssociativeDefinition(
+    COFFSymbolRef Sym, const coff_aux_section_definition *Def) {
+  SectionChunk *Parent = SparseChunks[Def->getNumber(Sym.isBigObj())];
+
+  // If the parent is pending, it probably means that its section definition
+  // appears after us in the symbol table. Leave the associated section as
+  // pending; we will handle it during the second pass in initializeSymbols().
+  if (Parent == PendingComdat)
+    return;
+
+  // Check whether the parent is prevailing. If it is, so are we, and we read
+  // the section; otherwise mark it as discarded.
+  int32_t SectionNumber = Sym.getSectionNumber();
+  if (Parent) {
+    SparseChunks[SectionNumber] = readSection(SectionNumber, Def);
+    if (SparseChunks[SectionNumber])
+      Parent->addAssociative(SparseChunks[SectionNumber]);
+  } else {
+    SparseChunks[SectionNumber] = nullptr;
+  }
+}
+
+Symbol *ObjFile::createRegular(COFFSymbolRef Sym) {
+  SectionChunk *SC = SparseChunks[Sym.getSectionNumber()];
+  if (Sym.isExternal()) {
+    StringRef Name;
+    COFFObj->getSymbolName(Sym, Name);
+    if (SC)
+      return Symtab->addRegular(this, Name, Sym.getGeneric(), SC);
+    return Symtab->addUndefined(Name, this, false);
   }
+  if (SC)
+    return make<DefinedRegular>(this, /*Name*/ "", false,
+                                /*IsExternal*/ false, Sym.getGeneric(), SC);
+  return nullptr;
 }
 
 void ObjFile::initializeSymbols() {
   uint32_t NumSymbols = COFFObj->getNumberOfSymbols();
   Symbols.resize(NumSymbols);
 
   SmallVector<std::pair<Symbol *, uint32_t>, 8> WeakAliases;
-  int32_t LastSectionNumber = 0;
+  std::vector<uint32_t> PendingIndexes;
+  PendingIndexes.reserve(NumSymbols);
+
+  std::vector<const coff_aux_section_definition *> ComdatDefs(
+      COFFObj->getNumberOfSections() + 1);
 
   for (uint32_t I = 0; I < NumSymbols; ++I) {
     COFFSymbolRef COFFSym = check(COFFObj->getSymbol(I));
-
-    const void *AuxP = nullptr;
-    if (COFFSym.getNumberOfAuxSymbols())
-      AuxP = check(COFFObj->getSymbol(I + 1)).getRawPtr();
-    bool IsFirst = (LastSectionNumber != COFFSym.getSectionNumber());
-
-    Symbol *Sym = nullptr;
     if (COFFSym.isUndefined()) {
-      Sym = createUndefined(COFFSym);
+      Symbols[I] = createUndefined(COFFSym);
     } else if (COFFSym.isWeakExternal()) {
-      Sym = createUndefined(COFFSym);
-      uint32_t TagIndex =
-          static_cast<const coff_aux_weak_external *>(AuxP)->TagIndex;
-      WeakAliases.emplace_back(Sym, TagIndex);
+      Symbols[I] = createUndefined(COFFSym);
+      uint32_t TagIndex = COFFSym.getAux<coff_aux_weak_external>()->TagIndex;
+      WeakAliases.emplace_back(Symbols[I], TagIndex);
+    } else if (Optional<Symbol *> OptSym = createDefined(COFFSym, ComdatDefs)) {
+      Symbols[I] = *OptSym;
     } else {
-      Sym = createDefined(COFFSym, AuxP, IsFirst);
+      // createDefined() returns None if a symbol belongs to a section that
+      // was pending at the point when the symbol was read. This can happen in
+      // two cases:
+      // 1) section definition symbol for a comdat leader;
+      // 2) symbol belongs to a comdat section associated with a section whose
+      //    section definition symbol appears later in the symbol table.
+      // In both of these cases, we can expect the section to be resolved by
+      // the time we finish visiting the remaining symbols in the symbol
+      // table. So we postpone the handling of this symbol until that time.
+      PendingIndexes.push_back(I);
     }
-    Symbols[I] = Sym;
     I += COFFSym.getNumberOfAuxSymbols();
-    LastSectionNumber = COFFSym.getSectionNumber();
+  }
+
+  for (uint32_t I : PendingIndexes) {
+    COFFSymbolRef Sym = check(COFFObj->getSymbol(I));
+    if (auto *Def = Sym.getSectionDefinition())
+      if (Def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
+        readAssociativeDefinition(Sym, Def);
+    Symbols[I] = createRegular(Sym);
   }
 
   for (auto &KV : WeakAliases) {
     Symbol *Sym = KV.first;
     uint32_t Idx = KV.second;
     checkAndSetWeakAlias(Symtab, this, Sym, Symbols[Idx]);
   }
 }
 
 Symbol *ObjFile::createUndefined(COFFSymbolRef Sym) {
   StringRef Name;
   COFFObj->getSymbolName(Sym, Name);
   return Symtab->addUndefined(Name, this, Sym.isWeakExternal());
 }
 
-Symbol *ObjFile::createDefined(COFFSymbolRef Sym, const void *AuxP,
-                               bool IsFirst) {
+Optional<Symbol *> ObjFile::createDefined(
+    COFFSymbolRef Sym,
+    std::vector<const coff_aux_section_definition *> &ComdatDefs) {
   StringRef Name;
   if (Sym.isCommon()) {
     auto *C = make<CommonChunk>(Sym);
     Chunks.push_back(C);
     COFFObj->getSymbolName(Sym, Name);
     Symbol *S =
         Symtab->addCommon(this, Name, Sym.getValue(), Sym.getGeneric(), C);
     return S;
Optional<Symbol *> ObjFile::createDefined(
   if (llvm::COFF::isReservedSectionNumber(SectionNumber))
     fatal("broken object file: " + toString(this));
 
   // This symbol references a section which is not present in the section
   // header.
   if ((uint32_t)SectionNumber >= SparseChunks.size())
     fatal("broken object file: " + toString(this));
 
-  // Nothing else to do without a section chunk.
-  auto *SC = SparseChunks[SectionNumber];
-  if (!SC)
-    return nullptr;
-
-  // Handle section definitions
-  if (IsFirst && AuxP) {
-    auto *Aux = reinterpret_cast<const coff_aux_section_definition *>(AuxP);
-    if (Aux->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
-      if (auto *ParentSC = SparseChunks[Aux->getNumber(Sym.isBigObj())]) {
-        ParentSC->addAssociative(SC);
-        // If we already discarded the parent, discard the child.
-        if (ParentSC->isDiscarded())
-          SC->markDiscarded();
-      }
-    SC->Checksum = Aux->CheckSum;
+  // Handle comdat leader symbols.
+  if (const coff_aux_section_definition *Def = ComdatDefs[SectionNumber]) {
+    ComdatDefs[SectionNumber] = nullptr;
+    Symbol *Leader;
+    bool Prevailing;
+    if (Sym.isExternal()) {
+      COFFObj->getSymbolName(Sym, Name);
+      std::tie(Leader, Prevailing) =
+          Symtab->addComdat(this, Name, Sym.getGeneric());
+    } else {
+      Leader = make<DefinedRegular>(this, /*Name*/ "", false,
+                                    /*IsExternal*/ false, Sym.getGeneric());
+      Prevailing = true;
+    }
+    if (Prevailing) {
+      SectionChunk *C = readSection(SectionNumber, Def);
+      SparseChunks[SectionNumber] = C;
+      C->Sym = cast<DefinedRegular>(Leader);
+      cast<DefinedRegular>(Leader)->Data = &C->Repl;
+    } else {
+      SparseChunks[SectionNumber] = nullptr;
+    return Leader;
   }
 
-  DefinedRegular *B;
-  if (Sym.isExternal()) {
-    COFFObj->getSymbolName(Sym, Name);
-    Symbol *S =
-        Symtab->addRegular(this, Name, SC->isCOMDAT(), Sym.getGeneric(), SC);
-    B = cast<DefinedRegular>(S);
-  } else
-    B = make<DefinedRegular>(this, /*Name*/ "", SC->isCOMDAT(),
-                             /*IsExternal*/ false, Sym.getGeneric(), SC);
-  if (SC->isCOMDAT() && Sym.getValue() == 0 && !AuxP)
-    SC->setSymbol(B);
+  // Read associative section definitions and prepare to handle the comdat
+  // leader symbol by setting the section's ComdatDefs pointer if we encounter a
+  // non-associative comdat.
+  if (SparseChunks[SectionNumber] == PendingComdat) {
+    if (auto *Def = Sym.getSectionDefinition()) {
+      if (Def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
+        readAssociativeDefinition(Sym, Def);
+      else
+        ComdatDefs[SectionNumber] = Def;
+    }
+  }
 
-  return B;
+  if (SparseChunks[SectionNumber] == PendingComdat)
+    return None;
+  return createRegular(Sym);
 }
 
 void ObjFile::initializeSEH() {
   if (!SEHCompat || !SXData)
     return;
   ArrayRef<uint8_t> A;
   COFFObj->getSectionContents(SXData, A);
   if (A.size() % 4 != 0)
void ImportFile::parse() {
   // DLL functions just like regular non-DLL functions.)
   if (Hdr->getType() == llvm::COFF::IMPORT_CODE)
     ThunkSym = Symtab->addImportThunk(Name, ImpSym, Hdr->Machine);
 }
 
 void BitcodeFile::parse() {
   Obj = check(lto::InputFile::create(MemoryBufferRef(
       MB.getBuffer(), Saver.save(ParentName + MB.getBufferIdentifier()))));
+  std::vector<std::pair<Symbol *, bool>> Comdat(Obj->getComdatTable().size());
+  for (size_t I = 0; I != Obj->getComdatTable().size(); ++I)
+    Comdat[I] = Symtab->addComdat(this, Saver.save(Obj->getComdatTable()[I]));
   for (const lto::InputFile::Symbol &ObjSym : Obj->symbols()) {
     StringRef SymName = Saver.save(ObjSym.getName());
+    int ComdatIndex = ObjSym.getComdatIndex();
     Symbol *Sym;
     if (ObjSym.isUndefined()) {
       Sym = Symtab->addUndefined(SymName, this, false);
     } else if (ObjSym.isCommon()) {
       Sym = Symtab->addCommon(this, SymName, ObjSym.getCommonSize());
     } else if (ObjSym.isWeak() && ObjSym.isIndirect()) {
       // Weak external.
       Sym = Symtab->addUndefined(SymName, this, true);
       std::string Fallback = ObjSym.getCOFFWeakExternalFallback();
       Symbol *Alias = Symtab->addUndefined(Saver.save(Fallback));
       checkAndSetWeakAlias(Symtab, this, Sym, Alias);
+    } else if (ComdatIndex != -1) {
+      if (SymName == Obj->getComdatTable()[ComdatIndex])
+        Sym = Comdat[ComdatIndex].first;
+      else if (Comdat[ComdatIndex].second)
+        Sym = Symtab->addRegular(this, SymName);
+      else
+        Sym = Symtab->addUndefined(SymName, this, false);
     } else {
-      bool IsCOMDAT = ObjSym.getComdatIndex() != -1;
-      Sym = Symtab->addRegular(this, SymName, IsCOMDAT);
+      Sym = Symtab->addRegular(this, SymName);
     }
     SymbolBodies.push_back(Sym);
   }
   Directives = Obj->getCOFFLinkerOpts();
 }
 
 MachineTypes BitcodeFile::getMachineType() {
   switch (Triple(Obj->getTargetTriple()).getArch()) {

lld/trunk/COFF/MarkLive.cpp

void markLive(const std::vector<Chunk *> &Chunks) {
   };
 
   // Add GC root chunks.
   for (Symbol *B : Config->GCRoot)
     AddSym(B);
 
   while (!Worklist.empty()) {
     SectionChunk *SC = Worklist.pop_back_val();
-
-    // If this section was discarded, there are relocations referring to
-    // discarded sections. Ignore these sections to avoid crashing. They will be
-    // diagnosed during relocation processing.
-    if (SC->isDiscarded())
-      continue;
-
     assert(SC->isLive() && "We mark as live when pushing onto the worklist!");
 
     // Mark all symbols listed in the relocation table for this section.
     for (Symbol *B : SC->symbols())
       if (B)
         AddSym(B);
 
     // Mark associative sections if any.
     for (SectionChunk *C : SC->children())
       Enqueue(C);
   }
 }
 
 }
 }

lld/trunk/COFF/SymbolTable.h

public:
   Symbol *addUndefined(StringRef Name);
 
   Symbol *addSynthetic(StringRef N, Chunk *C);
   Symbol *addAbsolute(StringRef N, uint64_t VA);
 
   Symbol *addUndefined(StringRef Name, InputFile *F, bool IsWeakAlias);
   void addLazy(ArchiveFile *F, const Archive::Symbol Sym);
   Symbol *addAbsolute(StringRef N, COFFSymbolRef S);
-  Symbol *addRegular(InputFile *F, StringRef N, bool IsCOMDAT,
+  Symbol *addRegular(InputFile *F, StringRef N,
                      const llvm::object::coff_symbol_generic *S = nullptr,
                      SectionChunk *C = nullptr);
+  std::pair<Symbol *, bool>
+  addComdat(InputFile *F, StringRef N,
+            const llvm::object::coff_symbol_generic *S = nullptr);
   Symbol *addCommon(InputFile *F, StringRef N, uint64_t Size,
                     const llvm::object::coff_symbol_generic *S = nullptr,
                     CommonChunk *C = nullptr);
   DefinedImportData *addImportData(StringRef N, ImportFile *F);
   DefinedImportThunk *addImportThunk(StringRef Name, DefinedImportData *S,
                                      uint16_t Machine);
 
   void reportDuplicate(Symbol *Existing, InputFile *NewFile);

lld/trunk/COFF/SymbolTable.cpp

 #include "llvm/Support/raw_ostream.h"
 #include <utility>
 
 using namespace llvm;
 
 namespace lld {
 namespace coff {
 
-enum SymbolPreference {
-  SP_EXISTING = -1,
-  SP_CONFLICT = 0,
-  SP_NEW = 1,
-};
-
-/// Checks if an existing symbol S should be kept or replaced by a new symbol.
-/// Returns SP_EXISTING when S should be kept, SP_NEW when the new symbol
-/// should be kept, and SP_CONFLICT if no valid resolution exists.
-static SymbolPreference compareDefined(Symbol *S, bool WasInserted,
-                                       bool NewIsCOMDAT) {
-  // If the symbol wasn't previously known, the new symbol wins by default.
-  if (WasInserted || !isa<Defined>(S))
-    return SP_NEW;
-
-  // If the existing symbol is a DefinedRegular, both it and the new symbol
-  // must be comdats. In that case, we have no reason to prefer one symbol
-  // over the other, and we keep the existing one. If one of the symbols
-  // is not a comdat, we report a conflict.
-  if (auto *R = dyn_cast<DefinedRegular>(S)) {
-    if (NewIsCOMDAT && R->isCOMDAT())
-      return SP_EXISTING;
-    else
-      return SP_CONFLICT;
-  }
-
-  // Existing symbol is not a DefinedRegular; new symbol wins.
-  return SP_NEW;
-}
-
 SymbolTable *Symtab;
 
 void SymbolTable::addFile(InputFile *File) {
   log("Reading " + toString(File));
   File->parse();
 
   MachineTypes MT = File->getMachineType();
   if (Config->Machine == IMAGE_FILE_MACHINE_UNKNOWN) {
Symbol *SymbolTable::addSynthetic(StringRef N, Chunk *C) {
   S->IsUsedInRegularObj = true;
   if (WasInserted || isa<Undefined>(S) || isa<Lazy>(S))
     replaceSymbol<DefinedSynthetic>(S, N, C);
   else if (!isa<DefinedCOFF>(S))
     reportDuplicate(S, nullptr);
   return S;
 }
 
-Symbol *SymbolTable::addRegular(InputFile *F, StringRef N, bool IsCOMDAT,
+Symbol *SymbolTable::addRegular(InputFile *F, StringRef N,
                                 const coff_symbol_generic *Sym,
                                 SectionChunk *C) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(N);
   if (!isa<BitcodeFile>(F))
     S->IsUsedInRegularObj = true;
-  SymbolPreference SP = compareDefined(S, WasInserted, IsCOMDAT);
-  if (SP == SP_CONFLICT) {
+  if (WasInserted || !isa<DefinedRegular>(S))
+    replaceSymbol<DefinedRegular>(S, F, N, /*IsCOMDAT*/ false,
+                                  /*IsExternal*/ true, Sym, C);
+  else
     reportDuplicate(S, F);
-  } else if (SP == SP_NEW) {
-    replaceSymbol<DefinedRegular>(S, F, N, IsCOMDAT, /*IsExternal*/ true, Sym,
-                                  C);
-  } else if (SP == SP_EXISTING && IsCOMDAT && C) {
-    C->markDiscarded();
-    // Discard associative chunks that we've parsed so far. No need to recurse
-    // because an associative section cannot have children.
-    for (SectionChunk *Child : C->children())
-      Child->markDiscarded();
-  }
   return S;
 }
 
+std::pair<Symbol *, bool>
+SymbolTable::addComdat(InputFile *F, StringRef N,
+                       const coff_symbol_generic *Sym) {
+  Symbol *S;
+  bool WasInserted;
+  std::tie(S, WasInserted) = insert(N);
+  if (!isa<BitcodeFile>(F))
+    S->IsUsedInRegularObj = true;
+  if (WasInserted || !isa<DefinedRegular>(S)) {
+    replaceSymbol<DefinedRegular>(S, F, N, /*IsCOMDAT*/ true,
+                                  /*IsExternal*/ true, Sym, nullptr);
+    return {S, true};
+  }
+  if (!cast<DefinedRegular>(S)->isCOMDAT())
+    reportDuplicate(S, F);
+  return {S, false};
+}
+
 Symbol *SymbolTable::addCommon(InputFile *F, StringRef N, uint64_t Size,
                                const coff_symbol_generic *Sym, CommonChunk *C) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(N);
   if (!isa<BitcodeFile>(F))
     S->IsUsedInRegularObj = true;
   if (WasInserted || !isa<DefinedCOFF>(S))

lld/trunk/COFF/Symbols.h

static bool classof(const Symbol *S) {
     return S->kind() == DefinedRegularKind;
   }
 
   uint64_t getRVA() const { return (*Data)->getRVA() + Sym->Value; }
   bool isCOMDAT() const { return IsCOMDAT; }
   SectionChunk *getChunk() const { return *Data; }
   uint32_t getValue() const { return Sym->Value; }
 
-private:
   SectionChunk **Data;
 };
 
 class DefinedCommon : public DefinedCOFF {
 public:
   DefinedCommon(InputFile *F, StringRef N, uint64_t Size,
                 const coff_symbol_generic *S = nullptr,
                 CommonChunk *C = nullptr)

lld/trunk/test/COFF/pdb-comdat.test

 
 REORDER-LABEL:   Mod 0000 | `{{.*}}pdb_comdat_bar.obj`:
 REORDER:       c:\src\llvm-project\build\pdb_comdat_bar.c (MD5: 365279DB4FCBEDD721BBFC3B14A953C2)
 REORDER:       c:\src\llvm-project\build\foo.h (MD5: D74D834EFAC3AE2B45E606A8320B1D5C)
 REORDER-LABEL:   Mod 0001 | `{{.*}}pdb_comdat_main.obj`:
 REORDER:       c:\src\llvm-project\build\pdb_comdat_main.c
 REORDER-NOT:       c:\src\llvm-project\build\foo.h
 REORDER-LABEL:   Mod 0002 | `* Linker *`:
+
+Make sure that we don't crash on non-prevailing debug sections if -debug is not enabled.
+RUN: lld-link pdb_comdat_main.obj pdb_comdat_bar.obj -out:%t.exe -nodefaultlib -entry:main

lld/trunk/test/COFF/reloc-discarded.s

# CHECK: error: relocation against symbol in discarded section: assoc_global
   .section  .bss,"bw",discard,main_global
   .globl  main_global
   .p2align  2
 main_global:
   .long 0
 
   .section  .CRT$XCU,"dr",associative,main_global
   .p2align  3
-  .globl assoc_global
 assoc_global:
   .quad main_global
 
   .text
   .globl main
 main:
   movq assoc_global(%rip), %rax
   movl (%rax), %eax
   retq

llvm/trunk/include/llvm/Object/COFF.h

 struct coff_symbol_generic {
   union {
     char ShortName[COFF::NameSize];
     StringTableOffset Offset;
   } Name;
   support::ulittle32_t Value;
 };
 
+struct coff_aux_section_definition;
+
 class COFFSymbolRef {
 public:
   COFFSymbolRef() = default;
   COFFSymbolRef(const coff_symbol16 *CS) : CS16(CS) {}
   COFFSymbolRef(const coff_symbol32 *CS) : CS32(CS) {}
 
   const void *getRawPtr() const {
     return CS16 ? static_cast<const void *>(CS16) : CS32;
public:
   }
 
   uint8_t getBaseType() const { return getType() & 0x0F; }
 
   uint8_t getComplexType() const {
     return (getType() & 0xF0) >> COFF::SCT_COMPLEX_TYPE_SHIFT;
   }
 
+  template <typename T> const T *getAux() const {
+    return CS16 ? reinterpret_cast<const T *>(CS16 + 1)
+                : reinterpret_cast<const T *>(CS32 + 1);
+  }
+
+  const coff_aux_section_definition *getSectionDefinition() const {
+    if (!getNumberOfAuxSymbols() ||
+        getStorageClass() != COFF::IMAGE_SYM_CLASS_STATIC)
+      return nullptr;
+    return getAux<coff_aux_section_definition>();
+  }
+
   bool isAbsolute() const {
     return getSectionNumber() == -1;
   }
 
   bool isExternal() const {
     return getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL;
   }