[lld] r361359 - [ELF] Improve error message for relocations to symbols defined in discarded sections
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Wed May 22 02:06:42 PDT 2019
Author: maskray
Date: Wed May 22 02:06:42 2019
New Revision: 361359
URL: http://llvm.org/viewvc/llvm-project?rev=361359&view=rev
Log:
[ELF] Improve error message for relocations to symbols defined in discarded sections
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 *>
Reviewed By: ruiu
Differential Revision: https://reviews.llvm.org/D59649
Added:
lld/trunk/test/ELF/comdat-discarded-error.s
lld/trunk/test/ELF/exclude-discarded-error.s
lld/trunk/test/ELF/exclude-discarded-error2.s
Modified:
lld/trunk/ELF/InputFiles.cpp
lld/trunk/ELF/InputFiles.h
lld/trunk/ELF/Relocations.cpp
lld/trunk/ELF/SymbolTable.cpp
lld/trunk/ELF/SymbolTable.h
lld/trunk/ELF/Symbols.h
Modified: lld/trunk/ELF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=361359&r1=361358&r2=361359&view=diff
==============================================================================
--- lld/trunk/ELF/InputFiles.cpp (original)
+++ lld/trunk/ELF/InputFiles.cpp Wed May 22 02:06:42 2019
@@ -111,11 +111,6 @@ static bool isCompatible(InputFile *File
}
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;
@@ -151,13 +146,13 @@ template <class ELFT> static void doPars
// 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.
@@ -396,7 +391,8 @@ template <class ELFT> ArrayRef<Symbol *>
}
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();
@@ -523,7 +519,7 @@ static void addDependentLibrary(StringRe
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);
@@ -582,7 +578,8 @@ void ObjFile<ELFT>::initializeSections(
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);
@@ -949,9 +946,13 @@ template <class ELFT> Symbol *ObjFile<EL
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 +
@@ -1333,10 +1334,12 @@ static Symbol *createBitcodeSymbol(const
}
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));
@@ -1504,10 +1507,14 @@ std::string elf::replaceThinLTOSuffix(St
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>();
Modified: lld/trunk/ELF/InputFiles.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.h?rev=361359&r1=361358&r2=361359&view=diff
==============================================================================
--- lld/trunk/ELF/InputFiles.h (original)
+++ lld/trunk/ELF/InputFiles.h Wed May 22 02:06:42 2019
@@ -187,9 +187,6 @@ template <class ELFT> class ObjFile : pu
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; }
@@ -201,7 +198,11 @@ public:
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())
@@ -244,8 +245,8 @@ public:
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();
@@ -338,7 +339,8 @@ public:
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;
};
Modified: lld/trunk/ELF/Relocations.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Relocations.cpp?rev=361359&r1=361358&r2=361359&view=diff
==============================================================================
--- lld/trunk/ELF/Relocations.cpp (original)
+++ lld/trunk/ELF/Relocations.cpp Wed May 22 02:06:42 2019
@@ -671,8 +671,36 @@ static int64_t computeAddend(const RelTy
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())
@@ -683,15 +711,25 @@ static bool maybeReportUndefined(Symbol
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>>> ";
@@ -1031,7 +1069,7 @@ static void scanReloc(InputSectionBase &
// 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;
Modified: lld/trunk/ELF/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=361359&r1=361358&r2=361359&view=diff
==============================================================================
--- lld/trunk/ELF/SymbolTable.cpp (original)
+++ lld/trunk/ELF/SymbolTable.cpp Wed May 22 02:06:42 2019
@@ -46,7 +46,7 @@ template <class ELFT> void SymbolTable::
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())
@@ -135,7 +135,11 @@ Symbol *SymbolTable::addSymbol(const Sym
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;
}
Modified: lld/trunk/ELF/SymbolTable.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.h?rev=361359&r1=361358&r2=361359&view=diff
==============================================================================
--- lld/trunk/ELF/SymbolTable.h (original)
+++ lld/trunk/ELF/SymbolTable.h Wed May 22 02:06:42 2019
@@ -62,6 +62,11 @@ public:
// 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);
Modified: lld/trunk/ELF/Symbols.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.h?rev=361359&r1=361358&r2=361359&view=diff
==============================================================================
--- lld/trunk/ELF/Symbols.h (original)
+++ lld/trunk/ELF/Symbols.h Wed May 22 02:06:42 2019
@@ -280,10 +280,14 @@ public:
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 {
Added: lld/trunk/test/ELF/comdat-discarded-error.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/comdat-discarded-error.s?rev=361359&view=auto
==============================================================================
--- lld/trunk/test/ELF/comdat-discarded-error.s (added)
+++ lld/trunk/test/ELF/comdat-discarded-error.s Wed May 22 02:06:42 2019
@@ -0,0 +1,18 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t1.o
+# RUN: echo '.section .text.foo,"axG", at progbits,foo,comdat; .globl foo; foo:' | \
+# RUN: llvm-mc -filetype=obj -triple=x86_64 - -o %t2.o
+# RUN: echo '.section .text.foo,"axG", at 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
Added: lld/trunk/test/ELF/exclude-discarded-error.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/exclude-discarded-error.s?rev=361359&view=auto
==============================================================================
--- lld/trunk/test/ELF/exclude-discarded-error.s (added)
+++ lld/trunk/test/ELF/exclude-discarded-error.s Wed May 22 02:06:42 2019
@@ -0,0 +1,15 @@
+# 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:
Added: lld/trunk/test/ELF/exclude-discarded-error2.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/exclude-discarded-error2.s?rev=361359&view=auto
==============================================================================
--- lld/trunk/test/ELF/exclude-discarded-error2.s (added)
+++ lld/trunk/test/ELF/exclude-discarded-error2.s Wed May 22 02:06:42 2019
@@ -0,0 +1,14 @@
+# 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
More information about the llvm-commits
mailing list