Index: ELF/Driver.cpp =================================================================== --- ELF/Driver.cpp +++ ELF/Driver.cpp @@ -1293,7 +1293,7 @@ if (!S->getFile().IsNeeded) { bool Used = S->Used; replaceSymbol(S, nullptr, S->getName(), STB_WEAK, S->StOther, - S->Type); + S->Type, /*DiscardedSecIdx=*/0); S->Used = Used; } } @@ -1366,7 +1366,7 @@ template static Symbol *addUndefined(StringRef Name) { return Symtab->addUndefined(Name, STB_GLOBAL, STV_DEFAULT, 0, false, - nullptr); + nullptr, /*DiscardedSecIdx*/ 0); } // The --wrap option is a feature to rename symbols so that you can write Index: ELF/InputFiles.h =================================================================== --- ELF/InputFiles.h +++ ELF/InputFiles.h @@ -184,9 +184,6 @@ typedef typename ELFT::Word Elf_Word; typedef typename ELFT::CGProfile Elf_CGProfile; - StringRef getShtGroupSignature(ArrayRef Sections, - const Elf_Shdr &Sec); - public: static bool classof(const InputFile *F) { return F->kind() == Base::ObjKind; } @@ -194,7 +191,11 @@ ArrayRef getGlobalSymbols(); ObjFile(MemoryBufferRef M, StringRef ArchiveName); - void parse(llvm::DenseSet &ComdatGroups); + void parse(llvm::DenseMap + &ComdatGroups); + + StringRef getShtGroupSignature(ArrayRef Sections, + const Elf_Shdr &Sec); Symbol &getSymbol(uint32_t SymbolIndex) const { if (SymbolIndex >= this->Symbols.size()) @@ -235,8 +236,8 @@ ArrayRef CGProfile; private: - void - initializeSections(llvm::DenseSet &ComdatGroups); + void initializeSections(llvm::DenseMap &ComdatGroups); void initializeSymbols(); void initializeJustSymbols(); void initializeDwarf(); @@ -315,7 +316,8 @@ uint64_t OffsetInArchive); static bool classof(const InputFile *F) { return F->kind() == BitcodeKind; } template - void parse(llvm::DenseSet &ComdatGroups); + void parse(llvm::DenseMap + &ComdatGroups); std::unique_ptr Obj; }; Index: ELF/InputFiles.cpp =================================================================== --- ELF/InputFiles.cpp +++ ELF/InputFiles.cpp @@ -292,7 +292,8 @@ } template -void ObjFile::parse(DenseSet &ComdatGroups) { +void ObjFile::parse( + DenseMap &ComdatGroups) { // Read a section table. JustSymbols is usually false. if (this->JustSymbols) initializeJustSymbols(); @@ -398,7 +399,7 @@ template void ObjFile::initializeSections( - DenseSet &ComdatGroups) { + DenseMap &ComdatGroups) { const ELFFile &Obj = this->getObj(); ArrayRef ObjSections = CHECK(Obj.sections(), this); @@ -457,7 +458,8 @@ 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); @@ -801,7 +803,8 @@ StringRefZ Name = this->StringTable.data() + Sym->st_name; if (Sym->st_shndx == SHN_UNDEF) - return make(this, Name, Binding, StOther, Type); + return make(this, Name, Binding, StOther, Type, + /*DiscardedSecIdx=*/0); return make(this, Name, Binding, StOther, Type, Value, Size, Sec); } @@ -811,7 +814,8 @@ switch (Sym->st_shndx) { case SHN_UNDEF: return Symtab->addUndefined(Name, Binding, StOther, Type, - /*CanOmitFromDynSym=*/false, this); + /*CanOmitFromDynSym=*/false, this, + /*DiscardedSecIdx=*/0); case SHN_COMMON: if (Value == 0 || Value >= UINT32_MAX) fatal(toString(this) + ": common symbol '" + Name + @@ -825,9 +829,11 @@ case STB_GLOBAL: case STB_WEAK: case STB_GNU_UNIQUE: - if (Sec == &InputSection::Discarded) + if (Sec == &InputSection::Discarded) { return Symtab->addUndefined(Name, Binding, StOther, Type, - /*CanOmitFromDynSym=*/false, this); + /*CanOmitFromDynSym=*/false, this, + /*DiscardedSecIdx=*/SecIdx); + } return Symtab->addDefined(Name, StOther, Type, Value, Size, Binding, Sec, this); } @@ -1030,9 +1036,9 @@ } if (Sym.isUndefined()) { - Symbol *S = Symtab->addUndefined(Name, Sym.getBinding(), - Sym.st_other, Sym.getType(), - /*CanOmitFromDynSym=*/false, this); + Symbol *S = Symtab->addUndefined( + Name, Sym.getBinding(), Sym.st_other, Sym.getType(), + /*CanOmitFromDynSym=*/false, this, 0); S->ExportDynamic = true; continue; } @@ -1163,11 +1169,13 @@ int C = ObjSym.getComdatIndex(); if (C != -1 && !KeptComdats[C]) return Symtab->addUndefined(Name, Binding, Visibility, Type, - CanOmitFromDynSym, &F); + CanOmitFromDynSym, &F, + /*DiscardedSecIdx=*/0); if (ObjSym.isUndefined()) return Symtab->addUndefined(Name, Binding, Visibility, Type, - CanOmitFromDynSym, &F); + CanOmitFromDynSym, &F, + /*DiscardedSecIdx=*/0); if (ObjSym.isCommon()) return Symtab->addCommon(Name, ObjSym.getCommonSize(), @@ -1179,10 +1187,12 @@ } template -void BitcodeFile::parse(DenseSet &ComdatGroups) { +void BitcodeFile::parse( + DenseMap &ComdatGroups) { std::vector 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(KeptComdats, ObjSym, *this)); @@ -1342,10 +1352,14 @@ template void ArchiveFile::parse(); template void ArchiveFile::parse(); -template void BitcodeFile::parse(DenseSet &); -template void BitcodeFile::parse(DenseSet &); -template void BitcodeFile::parse(DenseSet &); -template void BitcodeFile::parse(DenseSet &); +template void +BitcodeFile::parse(DenseMap &); +template void +BitcodeFile::parse(DenseMap &); +template void +BitcodeFile::parse(DenseMap &); +template void +BitcodeFile::parse(DenseMap &); template void LazyObjFile::parse(); template void LazyObjFile::parse(); Index: ELF/LTO.cpp =================================================================== --- ELF/LTO.cpp +++ ELF/LTO.cpp @@ -154,7 +154,7 @@ static void undefine(Symbol *S) { replaceSymbol(S, nullptr, S->getName(), STB_GLOBAL, STV_DEFAULT, - S->Type); + S->Type, /*DiscardedSecIdx=*/0); } void BitcodeCompiler::add(BitcodeFile &F) { Index: ELF/Relocations.cpp =================================================================== --- ELF/Relocations.cpp +++ ELF/Relocations.cpp @@ -663,10 +663,38 @@ return Addend; } +// Custom error message if Sym is defined in a discarded section. +template +static std::string maybeReportDiscarded(Undefined &Sym, InputSectionBase &Sec, + const RelTy &Rel) { + auto *File = dyn_cast_or_null>(Sym.File); + if (!File || !Sym.DiscardedSecIdx || + File->getSections()[Sym.DiscardedSecIdx] != &InputSection::Discarded) + return ""; + ArrayRef> 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 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 static bool maybeReportUndefined(Symbol &Sym, InputSectionBase &Sec, - uint64_t Offset) { + const RelTy &Rel) { if (Sym.isLocal() || !Sym.isUndefined() || Sym.isWeak()) return false; @@ -675,19 +703,29 @@ 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 Src = Sec.getSrcMsg(Sym, Offset); + std::string Msg = + maybeReportDiscarded(cast(Sym), Sec, Rel); + if (Msg.empty()) + Msg = "undefined " + Visibility() + "symbol: " + toString(Sym); + + Msg += "\n>>> referenced by "; + std::string Src = Sec.getSrcMsg(Sym, Rel.r_offset); if (!Src.empty()) Msg += Src + "\n>>> "; - Msg += Sec.getObjMsg(Offset); + Msg += Sec.getObjMsg(Rel.r_offset); if (Sym.getName().startswith("_ZTV")) Msg += "\nthe vtable symbol may be undefined because the class is missing " @@ -1019,7 +1057,7 @@ return; // Skip if the target symbol is an erroneous undefined symbol. - if (maybeReportUndefined(Sym, Sec, Rel.r_offset)) + if (maybeReportUndefined(Sym, Sec, Rel)) return; const uint8_t *RelocatedAddr = Sec.data().begin() + Rel.r_offset; Index: ELF/SymbolTable.h =================================================================== --- ELF/SymbolTable.h +++ ELF/SymbolTable.h @@ -42,7 +42,8 @@ template Symbol *addUndefined(StringRef Name, uint8_t Binding, uint8_t StOther, - uint8_t Type, bool CanOmitFromDynSym, InputFile *File); + uint8_t Type, bool CanOmitFromDynSym, InputFile *File, + uint32_t DiscardedSecIdx); Defined *addDefined(StringRef Name, uint8_t StOther, uint8_t Type, uint64_t Value, uint64_t Size, uint8_t Binding, @@ -82,6 +83,11 @@ // Set of .so files to not link the same shared object file more than once. llvm::DenseMap 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 ComdatGroups; + private: std::pair insertName(StringRef Name); @@ -104,11 +110,6 @@ llvm::DenseMap SymMap; std::vector SymVector; - // 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. - llvm::DenseSet ComdatGroups; - // A map from demangled symbol names to their symbol objects. // This mapping is 1:N because two symbols with different versions // can have the same name. We use this map to handle "extern C++ {}" Index: ELF/SymbolTable.cpp =================================================================== --- ELF/SymbolTable.cpp +++ ELF/SymbolTable.cpp @@ -141,7 +141,7 @@ LTO->add(*F); for (InputFile *File : LTO->compile()) { - DenseSet DummyGroups; + DenseMap DummyGroups; auto *Obj = cast>(File); Obj->parse(DummyGroups); for (Symbol *Sym : Obj->getGlobalSymbols()) @@ -246,7 +246,8 @@ template Symbol *SymbolTable::addUndefined(StringRef Name, uint8_t Binding, uint8_t StOther, uint8_t Type, - bool CanOmitFromDynSym, InputFile *File) { + bool CanOmitFromDynSym, InputFile *File, + uint32_t DiscardedSecIdx) { Symbol *S; bool WasInserted; uint8_t Visibility = getVisibility(StOther); @@ -254,8 +255,13 @@ // An undefined symbol with non default visibility must be satisfied // in the same DSO. - if (WasInserted || (isa(S) && Visibility != STV_DEFAULT)) { - replaceSymbol(S, File, Name, Binding, StOther, Type); + // + // If this is a non-weak defined symbol in a discarded section, override the + // existing undefined symbol for better error message later. + if (WasInserted || (isa(S) && Visibility != STV_DEFAULT) || + (S->isUndefined() && Binding != STB_WEAK && DiscardedSecIdx)) { + replaceSymbol(S, File, Name, Binding, StOther, Type, + DiscardedSecIdx); return S; } @@ -766,13 +772,17 @@ template void SymbolTable::addFile(InputFile *); template Symbol *SymbolTable::addUndefined(StringRef, uint8_t, uint8_t, - uint8_t, bool, InputFile *); + uint8_t, bool, InputFile *, + uint32_t); template Symbol *SymbolTable::addUndefined(StringRef, uint8_t, uint8_t, - uint8_t, bool, InputFile *); + uint8_t, bool, InputFile *, + uint32_t); template Symbol *SymbolTable::addUndefined(StringRef, uint8_t, uint8_t, - uint8_t, bool, InputFile *); + uint8_t, bool, InputFile *, + uint32_t); template Symbol *SymbolTable::addUndefined(StringRef, uint8_t, uint8_t, - uint8_t, bool, InputFile *); + uint8_t, bool, InputFile *, + uint32_t); template void SymbolTable::addCombinedLTOObject(); template void SymbolTable::addCombinedLTOObject(); Index: ELF/Symbols.h =================================================================== --- ELF/Symbols.h +++ ELF/Symbols.h @@ -227,13 +227,18 @@ SectionBase *Section; }; +// Represents an undefined symbol or a defined symbol in a discarded section. 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) + : 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 { Index: test/ELF/comdat-discarded-error.s =================================================================== --- /dev/null +++ test/ELF/comdat-discarded-error.s @@ -0,0 +1,18 @@ +# REQUIRES: x86 +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t1.o +# RUN: echo '.section .text.foo,"axG",@progbits,foo,comdat; .globl foo; foo:' | \ +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux - -o %t2.o +# RUN: echo '.section .text.foo,"axG",@progbits,foo,comdat; .globl bar; bar:' | \ +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux - -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 Index: test/ELF/exclude-discarded-error.s =================================================================== --- /dev/null +++ test/ELF/exclude-discarded-error.s @@ -0,0 +1,15 @@ +# REQUIRES: x86 +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %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: Index: test/ELF/exclude-discarded-error2.s =================================================================== --- /dev/null +++ test/ELF/exclude-discarded-error2.s @@ -0,0 +1,14 @@ +# REQUIRES: x86 +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o +# RUN: echo '.section .foo,"ae"; .weak foo; foo:' | \ +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux - -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