[ELF] Improve error message for relocations to symbols defined in discarded sections

[MaskRay]

Summary

Rather than report "undefined symbol: ", give more informative message
about the object file that defines the discarded section.

In particular, PR41133, if the section is a discarded COMDAT, print the
section group signature and the object file with the prevailing
definition. This is useful to track down some ODR issues.

We need to

• add uint32_t DiscardedSecIdx to Undefined for this feature.
• make ComdatGroups public and change its type to DenseMap<CachedHashStringRef, const InputFile *>

[smeenai]

Thank you!

If I'm understanding correctly, for the case where the relocation is not in the same object file as the discarded section, we'll still get the "undefined symbol" error message. I don't know if it would be possible to enhance that case to print the "relocation against discarded section" message as well. (gold does print that message in this case; it just doesn't print the additional information about the section group signature and prevailing definition.)

[ruiu]

ELF/Relocations.cpp

698 ↗

(On Diff #191716)

This is a big chunk of new code. Is there any way to factor out?

709–711 ↗

(On Diff #191716)

I think the following message would be better:

relocation refers to a symbol in a discarded section: <symbol name here>

[MaskRay]

Factor out maybeReportDiscardedComdat

[MaskRay]

In D59649#1438330, @smeenai wrote:

Thank you!

If I'm understanding correctly, for the case where the relocation is not in the same object file as the discarded section, we'll still get the "undefined symbol" error message. I don't know if it would be possible to enhance that case to print the "relocation against discarded section" message as well. (gold does print that message in this case; it just doesn't print the additional information about the section group signature and prevailing definition.)

Symbol has the File field but not the section index (st_shndx). If the relocation is not in the same object as the discarded section, we can't get the defined section easily.

[smeenai]

In D59649#1438809, @MaskRay wrote:

In D59649#1438330, @smeenai wrote:

Thank you!

If I'm understanding correctly, for the case where the relocation is not in the same object file as the discarded section, we'll still get the "undefined symbol" error message. I don't know if it would be possible to enhance that case to print the "relocation against discarded section" message as well. (gold does print that message in this case; it just doesn't print the additional information about the section group signature and prevailing definition.)

Symbol has the File field but not the section index (st_shndx). If the relocation is not in the same object as the discarded section, we can't get the defined section easily.

Could we do so via symbol table lookup? Rui said in https://bugs.llvm.org/show_bug.cgi?id=41133 that "We already insert all symbols (including ones for discarded sections) to the symbol table".

[MaskRay]

In D59649#1438837, @smeenai wrote:

In D59649#1438809, @MaskRay wrote:

In D59649#1438330, @smeenai wrote:

Thank you!

If I'm understanding correctly, for the case where the relocation is not in the same object file as the discarded section, we'll still get the "undefined symbol" error message. I don't know if it would be possible to enhance that case to print the "relocation against discarded section" message as well. (gold does print that message in this case; it just doesn't print the additional information about the section group signature and prevailing definition.)

Symbol has the File field but not the section index (st_shndx). If the relocation is not in the same object as the discarded section, we can't get the defined section easily.

Could we do so via symbol table lookup? Rui said in https://bugs.llvm.org/show_bug.cgi?id=41133 that "We already insert all symbols (including ones for discarded sections) to the symbol table".

We do insert all symbols into the symbol table (InputFiles.cpp:ObjFile<ELFT>::initializeSymbols()), however, Symbol does not encode all fields of Elf_Sym. Symbol has several derived classes, among them, only Defined keeps st_shndx, Undefined doesn't. Symbols defined in discarded COMDAT sections are represented by Undefined so the section index is lost. I think I can probably add SectionBase *Sec to Undefined so that we do not have the limitation.

[MaskRay]

generalize to SHF_EXCLUDE and other discarded sections

[MaskRay]

.

[MaskRay]

Binding != STB_WEAK

[MaskRay]

.

[grimar]

ELF/Driver.cpp

1296 ↗

(On Diff #191830)

I would use something like 0 /*DiscardedSecIdx*/ because it is not clear what is 0.
(here and below)

ELF/Relocations.cpp

682 ↗

(On Diff #191830)

Early return?

684 ↗

(On Diff #191830)

Use lookup?

691 ↗

(On Diff #191830)

I.e. this code seems a bit better looking as:

// If the discarded section is a COMDAT.
Elf_Shdr_Impl<ELFT> ELFSec = ObjSections[Sym.DiscardedSecIdx - 1];
if (ELFSec.sh_type != SHT_GROUP)
  return Msg;

StringRef Signature = File->getShtGroupSignature(ObjSections, ELFSec);
if (const InputFile *InFile =
        Symtab->ComdatGroups.lookup(CachedHashStringRef(Signature)))
  Msg += "\n>>> section group signature: " + Signature.str() +
         "\n>>> prevailing definition is in " + toString(InFile);
return Msg;

707 ↗

(On Diff #191830)

If you'll do -> std::string here, then ...

723 ↗

(On Diff #191830)

... you can remove .str() here.

test/ELF/exclude-discarded-error2.s

9 ↗

(On Diff #191830)

What about testing the full error message explicitly here?

[MaskRay]

Address grimar's comments

[MaskRay]

ELF/Relocations.cpp

684 ↗

(On Diff #191830)

Thank you! I didn't know the useful lookup...

[MaskRay]

0 -> /*DiscardedSecIdx=*/0

[grimar]

I have no more comments, thanks!

ELF/Driver.cpp

1296 ↗

(On Diff #191830)

Side note: Hmmm. We are not consistent. We use both /*DiscardedSecIdx*/ and /*DiscardedSecIdx=*/ forms across LLD.

[MaskRay]

/*DiscardedSecIdx*/ 0 -> /*DiscardedSecIdx=*/0

[MaskRay]

ELF/Driver.cpp

1296 ↗

(On Diff #191830)

Missed that case. Fixed!

[smeenai]

Thank you! This does even better than gold now :)

[MaskRay]

Ping :)

[MaskRay]

Ping :) I had come across an ODR violation case (which took me quite a while to figure out the cause) before I noticed smeenai's report so I believe this improved error message will be helpful.

[ruiu]

I'm honestly not very happy about the plumbing work in this patch, as the new value, DiscardedSecIdx, is not semantically required but handed over to many functions just to get to the location where the error message is generated.

I wonder if we can do this without that variable. When we need that variable, we are generating an error message, so the code doesn't have to be fast. I.e. you can scan through object file's symbol table to find an undefined symbol, for example. What do you think?

[MaskRay]

I wonder if we can do this without that variable. ... you can scan through object file's symbol table to find an undefined symbol, for example. What do you think?

I have thought about this option. I believe that will make code slower and more complex.

We use Undefined for two purposes: 1) undefined symbol 2) symbol in a discarded section (has only one occurrence: InputFiles.cpp: ObjFile<ELFT>::createSymbol). (I added some comments in this patch).

If we don't store Undefined::DiscardedSecIdx, we can iterate the symbol table of Symbol::File to figure out the section index and then check if the section is discarded. That would repeat some work done by ObjFile<ELFT>::createSymbol. Because we may emit many maybeReportUndefined errors, we don't want symbol table iterating to be too slow. We may end up with some cache. I think that is more complex than an extra DiscardedSecIdx.

If /*DiscardedSecIdx=0*/ looks too messy, it can be made a default argument.
That is simpler than having two member functions SymbolTable::addSymbolInDiscarded and SymbolTable::addUndefined.

[MaskRay]

explict /*DiscardedSecIdx=0*/ -> default argument

[MaskRay]

Does the default argument DiscardedSecIdx look good?

[MaskRay]

Remove template parameter class RelTy

[MaskRay]

Rebase after recent SymbolTable changes. Combined with D61583, we will give good warnings for PR41693

[MaskRay]

🏓 Ping:)

[ruiu]

Overall looking good.

Somewhat unrelated, but maybe we should clear ComdatGroups map once name resolution pass is done? COMDAT signatures are long strings, and discarding them from memory might save nontrivial amount of memory.

ELF/InputFiles.cpp

117 ↗

(On Diff #200506)

Why did you move this to Symtab?

[MaskRay]

In D59649#1511494, @ruiu wrote:

Overall looking good.

Somewhat unrelated, but maybe we should clear ComdatGroups map once name resolution pass is done? COMDAT signatures are long strings, and discarding them from memory might save nontrivial amount of memory.

Good point. I can do that in a separate change.

ELF/InputFiles.cpp

117 ↗

(On Diff #200506)

Relocations.cpp needs to access the variable to check which section is prevailing.

It was originally in Symtab until you moved it to a function-local static variable.. It can also be a global variable if you prefer.

[ruiu]

LGTM

[MaskRay]

lld/trunk/ELF/Relocations.cpp

688

We may improve the search for SHT_GROUP. Probably some validation is required. Sym.DiscardedSecIdx - 1 may not find the belonged SHT_GROUP...

lld/trunk/ELF/InputFiles.h

 template <class ELFT> class ObjFile : public ELFFileBase {
   using Elf_Rel = typename ELFT::Rel;
   using Elf_Rela = typename ELFT::Rela;
   using Elf_Sym = typename ELFT::Sym;
   using Elf_Shdr = typename ELFT::Shdr;
   using Elf_Word = typename ELFT::Word;
   using Elf_CGProfile = typename ELFT::CGProfile;
 
-  StringRef getShtGroupSignature(ArrayRef<Elf_Shdr> Sections,
-                                 const Elf_Shdr &Sec);
-
 public:
   static bool classof(const InputFile *F) { return F->kind() == ObjKind; }
 
   llvm::object::ELFFile<ELFT> getObj() const {
     return this->ELFFileBase::getObj<ELFT>();
   }
 
   ArrayRef<Symbol *> getLocalSymbols();
   ArrayRef<Symbol *> getGlobalSymbols();
 
   ObjFile(MemoryBufferRef M, StringRef ArchiveName);
-  void parse(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
+  void parse(llvm::DenseMap<llvm::CachedHashStringRef, const InputFile *>
+                 &ComdatGroups);
+
+  StringRef getShtGroupSignature(ArrayRef<Elf_Shdr> Sections,
+                                 const Elf_Shdr &Sec);
 
   Symbol &getSymbol(uint32_t SymbolIndex) const {
     if (SymbolIndex >= this->Symbols.size())
       fatal(toString(this) + ": invalid symbol index");
     return *this->Symbols[SymbolIndex];
   }
 
   uint32_t getSectionIndex(const Elf_Sym &Sym) const;
public:
 
   // Pointer to this input file's .llvm_addrsig section, if it has one.
   const Elf_Shdr *AddrsigSec = nullptr;
 
   // SHT_LLVM_CALL_GRAPH_PROFILE table
   ArrayRef<Elf_CGProfile> CGProfile;
 
 private:
-  void
-  initializeSections(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
+  void initializeSections(llvm::DenseMap<llvm::CachedHashStringRef,
+                                         const InputFile *> &ComdatGroups);
   void initializeSymbols();
   void initializeJustSymbols();
   void initializeDwarf();
   InputSectionBase *getRelocTarget(const Elf_Shdr &Sec);
   InputSectionBase *createInputSection(const Elf_Shdr &Sec);
   StringRef getSectionName(const Elf_Shdr &Sec);
 
   bool shouldMerge(const Elf_Shdr &Sec);

 };
 
 class BitcodeFile : public InputFile {
 public:
   BitcodeFile(MemoryBufferRef M, StringRef ArchiveName,
               uint64_t OffsetInArchive);
   static bool classof(const InputFile *F) { return F->kind() == BitcodeKind; }
   template <class ELFT>
-  void parse(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
+  void parse(llvm::DenseMap<llvm::CachedHashStringRef, const InputFile *>
+                 &ComdatGroups);
   std::unique_ptr<llvm::lto::InputFile> Obj;
 };
 
 // .so file.
 class SharedFile : public ELFFileBase {
 public:
   // This is actually a vector of Elf_Verdef pointers.
   std::vector<const void *> Verdefs;

lld/trunk/ELF/InputFiles.cpp

if (!Config->Emulation.empty()) {
 
     error(toString(File) + " is incompatible with " + toString(Existing));
   }
 
   return false;
 }
 
 template <class ELFT> static void doParseFile(InputFile *File) {
-  // Comdat groups define "link once" sections. If two comdat groups have the
-  // same name, only one of them is linked, and the other is ignored. This set
-  // is used to uniquify them.
-  static llvm::DenseSet<llvm::CachedHashStringRef> ComdatGroups;
-
   if (!isCompatible(File))
     return;
 
   // Binary file
   if (auto *F = dyn_cast<BinaryFile>(File)) {
     BinaryFiles.push_back(F);
     F->parse();
     return;
template <class ELFT> static void doParseFile(InputFile *File) {
   if (auto *F = dyn_cast<SharedFile>(File)) {
     F->parse<ELFT>();
     return;
   }
 
   // LLVM bitcode file
   if (auto *F = dyn_cast<BitcodeFile>(File)) {
     BitcodeFiles.push_back(F);
-    F->parse<ELFT>(ComdatGroups);
+    F->parse<ELFT>(Symtab->ComdatGroups);
     return;
   }
 
   // Regular object file
   ObjectFiles.push_back(File);
-  cast<ObjFile<ELFT>>(File)->parse(ComdatGroups);
+  cast<ObjFile<ELFT>>(File)->parse(Symtab->ComdatGroups);
 }
 
 // Add symbols in File to the symbol table.
 void elf::parseFile(InputFile *File) {
   switch (Config->EKind) {
   case ELF32LEKind:
     doParseFile<ELF32LE>(File);
     return;
template <class ELFT> ArrayRef<Symbol *> ObjFile<ELFT>::getLocalSymbols() {
   return makeArrayRef(this->Symbols).slice(1, this->FirstGlobal - 1);
 }
 
 template <class ELFT> ArrayRef<Symbol *> ObjFile<ELFT>::getGlobalSymbols() {
   return makeArrayRef(this->Symbols).slice(this->FirstGlobal);
 }
 
 template <class ELFT>
-void ObjFile<ELFT>::parse(DenseSet<CachedHashStringRef> &ComdatGroups) {
+void ObjFile<ELFT>::parse(
+    DenseMap<CachedHashStringRef, const InputFile *> &ComdatGroups) {
   // Read a section table. JustSymbols is usually false.
   if (this->JustSymbols)
     initializeJustSymbols();
   else
     initializeSections(ComdatGroups);
 
   // Read a symbol table.
   initializeSymbols();
static void addDependentLibrary(StringRef Specifier, const InputFile *F) {
   else
     error(toString(F) +
           ": unable to find library from dependent library specifier: " +
           Specifier);
 }
 
 template <class ELFT>
 void ObjFile<ELFT>::initializeSections(
-    DenseSet<CachedHashStringRef> &ComdatGroups) {
+    DenseMap<CachedHashStringRef, const InputFile *> &ComdatGroups) {
   const ELFFile<ELFT> &Obj = this->getObj();
 
   ArrayRef<Elf_Shdr> ObjSections = CHECK(Obj.sections(), this);
   uint64_t Size = ObjSections.size();
   this->Sections.resize(Size);
   this->SectionStringTable =
       CHECK(Obj.getSectionStringTable(ObjSections), this);
 
case SHT_GROUP: {
       // An group with the empty flag doesn't define anything; such sections
       // are just skipped.
       if (Entries[0] == 0)
         continue;
 
       if (Entries[0] != GRP_COMDAT)
         fatal(toString(this) + ": unsupported SHT_GROUP format");
 
-      bool IsNew = ComdatGroups.insert(CachedHashStringRef(Signature)).second;
+      bool IsNew =
+          ComdatGroups.try_emplace(CachedHashStringRef(Signature), this).second;
       if (IsNew) {
         if (Config->Relocatable)
           this->Sections[I] = createInputSection(Sec);
         continue;
       }
 
       // Otherwise, discard group members.
       for (uint32_t SecIndex : Entries.slice(1)) {
if (Binding == STB_LOCAL) {
     StringRefZ Name = this->StringTable.data() + Sym->st_name;
     if (Sym->st_shndx == SHN_UNDEF)
       return make<Undefined>(this, Name, Binding, StOther, Type);
     return make<Defined>(this, Name, Binding, StOther, Type, Value, Size, Sec);
   }
 
   StringRef Name = CHECK(Sym->getName(this->StringTable), this);
 
-  if (Sym->st_shndx == SHN_UNDEF || Sec == &InputSection::Discarded)
+  if (Sym->st_shndx == SHN_UNDEF)
     return Symtab->addSymbol(Undefined{this, Name, Binding, StOther, Type});
 
+  if (Sec == &InputSection::Discarded)
+    return Symtab->addSymbol(Undefined{this, Name, Binding, StOther, Type,
+                                       /*DiscardedSecIdx=*/SecIdx});
+
   if (Sym->st_shndx == SHN_COMMON) {
     if (Value == 0 || Value >= UINT32_MAX)
       fatal(toString(this) + ": common symbol '" + Name +
             "' has invalid alignment: " + Twine(Value));
     return Symtab->addSymbol(
         CommonSymbol{this, Name, Binding, StOther, Type, Value, Size});
   }
 
static Symbol *createBitcodeSymbol(const std::vector<bool> &KeptComdats,
 
   Defined New(&F, Name, Binding, Visibility, Type, 0, 0, nullptr);
   if (CanOmitFromDynSym)
     New.ExportDynamic = false;
   return Symtab->addSymbol(New);
 }
 
 template <class ELFT>
-void BitcodeFile::parse(DenseSet<CachedHashStringRef> &ComdatGroups) {
+void BitcodeFile::parse(
+    DenseMap<CachedHashStringRef, const InputFile *> &ComdatGroups) {
   std::vector<bool> KeptComdats;
   for (StringRef S : Obj->getComdatTable())
-    KeptComdats.push_back(ComdatGroups.insert(CachedHashStringRef(S)).second);
+    KeptComdats.push_back(
+        ComdatGroups.try_emplace(CachedHashStringRef(S), this).second);
 
   for (const lto::InputFile::Symbol &ObjSym : Obj->symbols())
     Symbols.push_back(createBitcodeSymbol<ELFT>(KeptComdats, ObjSym, *this));
 
   for (auto L : Obj->getDependentLibraries())
     addDependentLibrary(L, this);
 }
 
std::string elf::replaceThinLTOSuffix(StringRef Path) {
   StringRef Suffix = Config->ThinLTOObjectSuffixReplace.first;
   StringRef Repl = Config->ThinLTOObjectSuffixReplace.second;
 
   if (Path.consume_back(Suffix))
     return (Path + Repl).str();
   return Path;
 }
 
-template void BitcodeFile::parse<ELF32LE>(DenseSet<CachedHashStringRef> &);
-template void BitcodeFile::parse<ELF32BE>(DenseSet<CachedHashStringRef> &);
-template void BitcodeFile::parse<ELF64LE>(DenseSet<CachedHashStringRef> &);
-template void BitcodeFile::parse<ELF64BE>(DenseSet<CachedHashStringRef> &);
+template void
+BitcodeFile::parse<ELF32LE>(DenseMap<CachedHashStringRef, const InputFile *> &);
+template void
+BitcodeFile::parse<ELF32BE>(DenseMap<CachedHashStringRef, const InputFile *> &);
+template void
+BitcodeFile::parse<ELF64LE>(DenseMap<CachedHashStringRef, const InputFile *> &);
+template void
+BitcodeFile::parse<ELF64BE>(DenseMap<CachedHashStringRef, const InputFile *> &);
 
 template void LazyObjFile::parse<ELF32LE>();
 template void LazyObjFile::parse<ELF32BE>();
 template void LazyObjFile::parse<ELF64LE>();
 template void LazyObjFile::parse<ELF64BE>();
 
 template class elf::ObjFile<ELF32LE>;
 template class elf::ObjFile<ELF32BE>;
 template class elf::ObjFile<ELF64LE>;
 template class elf::ObjFile<ELF64BE>;
 
 template void SharedFile::parse<ELF32LE>();
 template void SharedFile::parse<ELF32BE>();
 template void SharedFile::parse<ELF64LE>();
 template void SharedFile::parse<ELF64BE>();

lld/trunk/ELF/Relocations.cpp

static int64_t computeAddend(const RelTy &Rel, const RelTy *End,
   if (Config->EMachine == EM_PPC64 && Config->Pic && Type == R_PPC64_TOC)
     Addend += getPPC64TocBase();
   if (Config->EMachine == EM_MIPS)
     Addend += computeMipsAddend<ELFT>(Rel, End, Sec, Expr, IsLocal);
 
   return Addend;
 }
 
+// Custom error message if Sym is defined in a discarded section.
+template <class ELFT>
+static std::string maybeReportDiscarded(Undefined &Sym, InputSectionBase &Sec,
+                                        uint64_t Offset) {
+  auto *File = dyn_cast_or_null<ObjFile<ELFT>>(Sym.File);
+  if (!File || !Sym.DiscardedSecIdx ||
+      File->getSections()[Sym.DiscardedSecIdx] != &InputSection::Discarded)
+    return "";
+  ArrayRef<Elf_Shdr_Impl<ELFT>> ObjSections =
+      CHECK(File->getObj().sections(), File);
+  std::string Msg =
+      "relocation refers to a symbol in a discarded section: " + toString(Sym) +
+      "\n>>> defined in " + toString(File);
+
+  Elf_Shdr_Impl<ELFT> ELFSec = ObjSections[Sym.DiscardedSecIdx - 1];
+  if (ELFSec.sh_type != SHT_GROUP)
+    return Msg;
+
+  // If the discarded section is a COMDAT.
+  StringRef Signature = File->getShtGroupSignature(ObjSections, ELFSec);
+  if (const InputFile *Prevailing =
+          Symtab->ComdatGroups.lookup(CachedHashStringRef(Signature)))
+    Msg += "\n>>> section group signature: " + Signature.str() +
+           "\n>>> prevailing definition is in " + toString(Prevailing);
+  return Msg;
+}
+
 // Report an undefined symbol if necessary.
 // Returns true if this function printed out an error message.
+template <class ELFT>
 static bool maybeReportUndefined(Symbol &Sym, InputSectionBase &Sec,
                                  uint64_t Offset) {
   if (!Sym.isUndefined() || Sym.isWeak())
     return false;
 
   bool CanBeExternal = !Sym.isLocal() && Sym.computeBinding() != STB_LOCAL &&
                        Sym.Visibility == STV_DEFAULT;
   if (Config->UnresolvedSymbols == UnresolvedPolicy::Ignore && CanBeExternal)
     return false;
 
-  std::string Msg = "undefined ";
-  if (Sym.Visibility == STV_INTERNAL)
-    Msg += "internal ";
-  else if (Sym.Visibility == STV_HIDDEN)
-    Msg += "hidden ";
-  else if (Sym.Visibility == STV_PROTECTED)
-    Msg += "protected ";
-  Msg += "symbol: " + toString(Sym) + "\n>>> referenced by ";
+  auto Visibility = [&]() -> std::string {
+    switch (Sym.Visibility) {
+    case STV_INTERNAL:
+      return "internal ";
+    case STV_HIDDEN:
+      return "hidden ";
+    case STV_PROTECTED:
+      return "protected ";
+    default:
+      return "";
+    }
+  };
 
+  std::string Msg =
+      maybeReportDiscarded<ELFT>(cast<Undefined>(Sym), Sec, Offset);
+  if (Msg.empty())
+    Msg = "undefined " + Visibility() + "symbol: " + toString(Sym);
+
+  Msg += "\n>>> referenced by ";
   std::string Src = Sec.getSrcMsg(Sym, Offset);
   if (!Src.empty())
     Msg += Src + "\n>>>               ";
   Msg += Sec.getObjMsg(Offset);
 
   if (Sym.getName().startswith("_ZTV"))
     Msg += "\nthe vtable symbol may be undefined because the class is missing "
            "its key function (see https://lld.llvm.org/missingkeyfunction)";
static void scanReloc(InputSectionBase &Sec, OffsetGetter &GetOffset, RelTy *&I,
 
   // Get an offset in an output section this relocation is applied to.
   uint64_t Offset = GetOffset.get(Rel.r_offset);
   if (Offset == uint64_t(-1))
     return;
 
   // Error if the target symbol is undefined. Symbol index 0 may be used by
   // marker relocations, e.g. R_*_NONE and R_ARM_V4BX. Don't error on them.
-  if (SymIndex != 0 && maybeReportUndefined(Sym, Sec, Rel.r_offset))
+  if (SymIndex != 0 && maybeReportUndefined<ELFT>(Sym, Sec, Rel.r_offset))
     return;
 
   const uint8_t *RelocatedAddr = Sec.data().begin() + Rel.r_offset;
   RelExpr Expr = Target->getRelExpr(Type, Sym, RelocatedAddr);
 
   // Ignore "hint" relocations because they are only markers for relaxation.
   if (oneof<R_HINT, R_NONE>(Expr))
     return;

lld/trunk/ELF/SymbolTable.h

public:
 
   void trace(StringRef Name);
 
   void handleDynamicList();
 
   // Set of .so files to not link the same shared object file more than once.
   llvm::DenseMap<StringRef, SharedFile *> SoNames;
 
+  // Comdat groups define "link once" sections. If two comdat groups have the
+  // same name, only one of them is linked, and the other is ignored. This map
+  // is used to uniquify them.
+  llvm::DenseMap<llvm::CachedHashStringRef, const InputFile *> ComdatGroups;
+
 private:
   std::vector<Symbol *> findByVersion(SymbolVersion Ver);
   std::vector<Symbol *> findAllByVersion(SymbolVersion Ver);
 
   llvm::StringMap<std::vector<Symbol *>> &getDemangledSyms();
   void handleAnonymousVersion();
   void assignExactVersion(SymbolVersion Ver, uint16_t VersionId,
                           StringRef VersionName);

lld/trunk/ELF/SymbolTable.cpp

 // to the compiler at once, it can do whole-program optimization.
 template <class ELFT> void SymbolTable::addCombinedLTOObject() {
   // Compile bitcode files and replace bitcode symbols.
   LTO.reset(new BitcodeCompiler);
   for (BitcodeFile *F : BitcodeFiles)
     LTO->add(*F);
 
   for (InputFile *File : LTO->compile()) {
-    DenseSet<CachedHashStringRef> DummyGroups;
+    DenseMap<CachedHashStringRef, const InputFile *> DummyGroups;
     auto *Obj = cast<ObjFile<ELFT>>(File);
     Obj->parse(DummyGroups);
     for (Symbol *Sym : Obj->getGlobalSymbols())
       Sym->parseSymbolVersion();
     ObjectFiles.push_back(File);
   }
 }
 
Symbol *SymbolTable::addSymbol(const Symbol &New) {
   Symbol *Old = Symtab->insert(New.getName());
   resolveSymbol(Old, New);
   return Old;
 }
 
 static void addUndefined(Symbol *Old, const Undefined &New) {
   // An undefined symbol with non default visibility must be satisfied
   // in the same DSO.
-  if (Old->isShared() && New.Visibility != STV_DEFAULT) {
+  //
+  // If this is a non-weak defined symbol in a discarded section, override the
+  // existing undefined symbol for better error message later.
+  if ((Old->isShared() && New.Visibility != STV_DEFAULT) ||
+      (Old->isUndefined() && New.Binding != STB_WEAK && New.DiscardedSecIdx)) {
     Old->replace(New);
     return;
   }
 
   if (Old->isShared() || Old->isLazy() ||
       (Old->isUndefined() && New.Binding != STB_WEAK))
     Old->Binding = New.Binding;
 

lld/trunk/ELF/Symbols.h

public:
 
   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) {}
+            uint8_t Type, uint32_t DiscardedSecIdx = 0)
+      : Symbol(UndefinedKind, File, Name, Binding, StOther, Type),
+        DiscardedSecIdx(DiscardedSecIdx) {}
 
   static bool classof(const Symbol *S) { return S->kind() == UndefinedKind; }
+
+  // The section index if in a discarded section, 0 otherwise.
+  uint32_t DiscardedSecIdx;
 };
 
 class SharedSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) { return S->kind() == SharedKind; }
 
   SharedSymbol(InputFile &File, StringRef Name, uint8_t Binding,
                uint8_t StOther, uint8_t Type, uint64_t Value, uint64_t Size,

lld/trunk/test/ELF/comdat-discarded-error.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t1.o
+# RUN: echo '.section .text.foo,"axG",@progbits,foo,comdat; .globl foo; foo:' | \
+# RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t2.o
+# RUN: echo '.section .text.foo,"axG",@progbits,foo,comdat; .globl bar; bar:' | \
+# RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t3.o
+
+# RUN: not ld.lld %t1.o %t2.o %t3.o -o /dev/null 2>&1 | FileCheck %s
+
+# CHECK:      error: relocation refers to a symbol in a discarded section: bar
+# CHECK-NEXT: >>> defined in {{.*}}3.o
+# CHECK-NEXT: >>> section group signature: foo
+# CHECK-NEXT: >>> prevailing definition is in {{.*}}2.o
+# CHECK-NEXT: >>> referenced by {{.*}}1.o:(.text+0x1)
+
+.globl _start
+_start:
+  jmp bar

lld/trunk/test/ELF/exclude-discarded-error.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
+
+# CHECK:      error: relocation refers to a symbol in a discarded section: foo
+# CHECK-NEXT: >>> defined in {{.*}}.o
+# CHECK-NEXT: >>> referenced by {{.*}}.o:(.text+0x1)
+
+.globl _start
+_start:
+  jmp foo
+
+.section .foo,"ae"
+.globl foo
+foo:

lld/trunk/test/ELF/exclude-discarded-error2.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: echo '.section .foo,"ae"; .weak foo; foo:' | \
+# RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t1.o
+# RUN: not ld.lld %t.o %t1.o -o /dev/null 2>&1 | FileCheck %s
+
+# Because foo defined in %t1.o is weak, it does not override global undefined
+# in %t.o
+# CHECK-NOT: discarded section
+# CHECK: undefined symbol: foo
+
+.globl _start
+_start:
+  jmp foo