Index: ELF/InputFiles.h =================================================================== --- ELF/InputFiles.h +++ ELF/InputFiles.h @@ -184,9 +184,6 @@ using Elf_Word = typename ELFT::Word; using Elf_CGProfile = typename ELFT::CGProfile; - StringRef getShtGroupSignature(ArrayRef Sections, - const Elf_Shdr &Sec); - public: static bool classof(const InputFile *F) { return F->kind() == ObjKind; } @@ -198,7 +195,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()) @@ -245,8 +246,8 @@ ArrayRef CGProfile; private: - void - initializeSections(llvm::DenseSet &ComdatGroups); + void initializeSections(llvm::DenseMap &ComdatGroups); void initializeSymbols(); void initializeJustSymbols(); void initializeDwarf(); @@ -339,7 +340,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 @@ -294,7 +294,8 @@ } template -void ObjFile::parse(DenseSet &ComdatGroups) { +void ObjFile::parse( + DenseMap &ComdatGroups) { // Read a section table. JustSymbols is usually false. if (this->JustSymbols) initializeJustSymbols(); @@ -400,7 +401,7 @@ template void ObjFile::initializeSections( - DenseSet &ComdatGroups) { + DenseMap &ComdatGroups) { const ELFFile &Obj = this->getObj(); ArrayRef ObjSections = CHECK(Obj.sections(), this); @@ -459,7 +460,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); @@ -829,7 +831,8 @@ case STB_GNU_UNIQUE: 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); } @@ -1195,10 +1198,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)); @@ -1364,10 +1369,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/Relocations.cpp =================================================================== --- ELF/Relocations.cpp +++ ELF/Relocations.cpp @@ -669,8 +669,36 @@ return Addend; } +// Custom error message if Sym is defined in a discarded section. +template +static std::string maybeReportDiscarded(Undefined &Sym, InputSectionBase &Sec, + uint64_t Offset) { + 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) { if (Sym.isLocal() || !Sym.isUndefined() || Sym.isWeak()) @@ -681,15 +709,25 @@ 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(cast(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>>> "; @@ -1027,7 +1065,7 @@ return; // Skip if the target symbol is an erroneous undefined symbol. - if (maybeReportUndefined(Sym, Sec, Rel.r_offset)) + if (maybeReportUndefined(Sym, Sec, Rel.r_offset)) 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 = 0); Defined *addDefined(StringRef Name, uint8_t StOther, uint8_t Type, uint64_t Value, uint64_t Size, uint8_t Binding, @@ -81,6 +82,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); @@ -103,11 +109,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 @@ -127,7 +127,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()) @@ -232,7 +232,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); @@ -240,8 +241,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; } @@ -751,13 +757,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 @@ -226,13 +226,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 = 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 { 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