[ELF] Show error location for 'duplicate symbol' error

[evgeny777]

Summary

This patch shows new and previous locations for duplicate symbol in a same way it's being done for undefined symbols

[• rafael]

LGTM

awesome, thanks!

[ruiu]

This is very useful.

I had a chat with Sean during the LLVM dev meeting, and he pointed out that if we can provide a better error diagnostics than exiting linkers, this could be an incentive for people to switch to a new linker. I agreed. Thank you for your your work on improving diagnostics.

ELF/Relocations.h

 
 template <class ELFT>
 void scanRelocations(InputSectionBase<ELFT> &, const typename ELFT::Shdr &);
 
 template <class ELFT>
 void createThunks(InputSectionBase<ELFT> &, const typename ELFT::Shdr &);
 
 template <class ELFT>
+std::string getLocation(SymbolBody *Sym, InputSectionBase<ELFT> &S,
+                        typename ELFT::uint Offset);
+
+template <class ELFT>
 static inline typename ELFT::uint getAddend(const typename ELFT::Rel &Rel) {
   return 0;
 }
 
 template <class ELFT>
 static inline typename ELFT::uint getAddend(const typename ELFT::Rela &Rel) {
   return Rel.r_addend;
 }
 }
 }
 
 #endif

ELF/Relocations.cpp

for (SymbolBody *B : S->getFile()->getSymbols())
     if (auto *D = dyn_cast<DefinedRegular<ELFT>>(B))
       if (D->Value <= Offset && D->Value + D->Size > Offset && D->Section == S)
         return D;
 
   return nullptr;
 }
 
 template <class ELFT>
-static std::string getLocation(SymbolBody &Sym, InputSectionBase<ELFT> &S,
-                               typename ELFT::uint Offset) {
+std::string getLocation(SymbolBody *Sym, InputSectionBase<ELFT> &S,
   ObjectFile<ELFT> *File = S.getFile();
 
   // First check if we can get desired values from debugging information.
   std::string LineInfo = File->getLineInfo(&S, Offset);
   if (!LineInfo.empty())
     return LineInfo;
 
   // If don't have STT_FILE typed symbol in object file then
   // use object file name.
   std::string SrcFile = File->SourceFile;
   if (SrcFile.empty())
-    SrcFile = Sym.File ? getFilename(Sym.File) : getFilename(File);
+    SrcFile = Sym && Sym->File ? getFilename(Sym->File) : getFilename(File);
 
   // Find a symbol at a given location.
   DefinedRegular<ELFT> *Encl = getSymbolAt(&S, Offset);
   if (Encl && Encl->Type == STT_FUNC) {
     StringRef Func = getSymbolName(*File, *Encl);
     return SrcFile + " (function " + maybeDemangle(Func) + ")";
   }
 
   // If there's no symbol, print out the offset instead of a symbol name.
   return (SrcFile + " (" + S.Name + "+0x" + Twine::utohexstr(Offset) + ")")
       .str();
 }
 
 template <class ELFT>
 static void reportUndefined(SymbolBody &Sym, InputSectionBase<ELFT> &S,
                             typename ELFT::uint Offset) {
   if (Config->UnresolvedSymbols == UnresolvedPolicy::Ignore)
     return;
 
   if (Config->Shared && Sym.symbol()->Visibility == STV_DEFAULT &&
       Config->UnresolvedSymbols != UnresolvedPolicy::NoUndef)
     return;
 
-  std::string Msg = getLocation(Sym, S, Offset) + ": undefined symbol '" +
+  std::string Msg = getLocation(&Sym, S, Offset) + ": undefined symbol '" +
                     maybeDemangle(Sym.getName()) + "'";
 
   if (Config->UnresolvedSymbols == UnresolvedPolicy::Warn)
     warn(Msg);
   else
     error(Msg);
 }
 

 template void createThunks<ELF32LE>(InputSectionBase<ELF32LE> &,
                                     const ELF32LE::Shdr &);
 template void createThunks<ELF32BE>(InputSectionBase<ELF32BE> &,
                                     const ELF32BE::Shdr &);
 template void createThunks<ELF64LE>(InputSectionBase<ELF64LE> &,
                                     const ELF64LE::Shdr &);
 template void createThunks<ELF64BE>(InputSectionBase<ELF64BE> &,
                                     const ELF64BE::Shdr &);
+
+template std::string getLocation<ELF32LE>(SymbolBody *Sym,
+                                          InputSectionBase<ELF32LE> &S,
+                                          uint32_t Offset);
+template std::string getLocation<ELF32BE>(SymbolBody *Sym,
+                                          InputSectionBase<ELF32BE> &S,
+                                          uint32_t Offset);
+template std::string getLocation<ELF64LE>(SymbolBody *Sym,
+                                          InputSectionBase<ELF64LE> &S,
+                                          uint64_t Offset);
+template std::string getLocation<ELF64BE>(SymbolBody *Sym,
+                                          InputSectionBase<ELF64BE> &S,
+                                          uint64_t Offset);
 }
 }

ELF/SymbolTable.h

 
 private:
   std::vector<SymbolBody *> findAll(const StringMatcher &M);
   std::pair<Symbol *, bool> insert(StringRef &Name);
   std::pair<Symbol *, bool> insert(StringRef &Name, uint8_t Type,
                                    uint8_t Visibility, bool CanOmitFromDynSym,
                                    InputFile *File);
 
-  void reportDuplicate(SymbolBody *Existing, InputFile *NewFile);
-
   std::map<std::string, std::vector<SymbolBody *>> getDemangledSyms();
   void handleAnonymousVersion();
 
   struct SymIndex {
     SymIndex(int Idx, bool Traced) : Idx(Idx), Traced(Traced) {}
     int Idx : 31;
     unsigned Traced : 1;
   };

ELF/SymbolTable.cpp

if (IsNew) {
     Sym = SymVector[V.Idx];
   }
   return {Sym, IsNew};
 }
 
 // Construct a string in the form of "Sym in File1 and File2".
 // Used to construct an error message.
 static std::string conflictMsg(SymbolBody *Existing, InputFile *NewFile) {
-  return maybeDemangle(Existing->getName()) + " in " +
+  return "'" + maybeDemangle(Existing->getName()) + "' in " +
          getFilename(Existing->File) + " and " + getFilename(NewFile);
 }
 
 // Find an existing symbol or create and insert a new one, then apply the given
 // attributes.
 template <class ELFT>
 std::pair<Symbol *, bool>
 SymbolTable<ELFT>::insert(StringRef &Name, uint8_t Type, uint8_t Visibility,
                           bool CanOmitFromDynSym, InputFile *File) {
   bool IsUsedInRegularObj = !File || File->kind() == InputFile::ObjectKind;
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
 
   // Merge in the new symbol's visibility.
   S->Visibility = getMinVisibility(S->Visibility, Visibility);
   if (!CanOmitFromDynSym && (Config->Shared || Config->ExportDynamic))
     S->ExportDynamic = true;
   if (IsUsedInRegularObj)
     S->IsUsedInRegularObj = true;
   if (!WasInserted && S->body()->Type != SymbolBody::UnknownType &&
       ((Type == STT_TLS) != S->body()->isTls()))
-    error("TLS attribute mismatch for symbol: " + conflictMsg(S->body(), File));
+    error("TLS attribute mismatch for symbol " + conflictMsg(S->body(), File));
 
   return {S, WasInserted};
 }
 
 template <class ELFT> Symbol *SymbolTable<ELFT>::addUndefined(StringRef Name) {
   return addUndefined(Name, STB_GLOBAL, STV_DEFAULT, /*Type*/ 0,
                       /*CanOmitFromDynSym*/ false, /*File*/ nullptr);
 }
if (Cmp > 0) {
 
     Alignment = C->Alignment = std::max(C->Alignment, Alignment);
     if (Size > C->Size)
       replaceBody<DefinedCommon>(S, N, Size, Alignment, StOther, Type, File);
   }
   return S;
 }
 
-template <class ELFT>
-void SymbolTable<ELFT>::reportDuplicate(SymbolBody *Existing,
-                                        InputFile *NewFile) {
-  std::string Msg = "duplicate symbol: " + conflictMsg(Existing, NewFile);
+static void reportDuplicate(const std::string &Msg) {
   if (Config->AllowMultipleDefinition)
     warn(Msg);
   else
     error(Msg);
 }
 
+static void reportDuplicate(SymbolBody *Existing, InputFile *NewFile) {
+  reportDuplicate("duplicate symbol " + conflictMsg(Existing, NewFile));
+}
+
+template <class ELFT>
+static void reportDuplicate(SymbolBody *Existing,
+                            InputSectionBase<ELFT> *ErrSec,
+                            typename ELFT::uint ErrOffset) {
+  DefinedRegular<ELFT> *D = dyn_cast<DefinedRegular<ELFT>>(Existing);
+  if (!D || !D->Section || !ErrSec) {
+    reportDuplicate(Existing, ErrSec ? ErrSec->getFile() : nullptr);
+    return;
+  }
+
+  std::string OldLoc = getLocation(Existing, *D->Section, D->Value);
+  std::string NewLoc = getLocation(nullptr, *ErrSec, ErrOffset);
+
+  reportDuplicate(NewLoc + ": duplicate symbol '" +
+                  maybeDemangle(Existing->getName()) + "'");
+  reportDuplicate(OldLoc + ": previous definition was here");
+}
+
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, const Elf_Sym &Sym,
                                       InputSectionBase<ELFT> *Section) {
   return addRegular(Name, Sym.st_other, Sym.getType(), Sym.st_value,
                     Sym.st_size, Sym.getBinding(), Section);
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, uint8_t StOther,
                                       uint8_t Type, uintX_t Value, uintX_t Size,
                                       uint8_t Binding,
                                       InputSectionBase<ELFT> *Section) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, Type, StOther & 3,
                                     /*CanOmitFromDynSym*/ false,
                                     Section ? Section->getFile() : nullptr);
   int Cmp = compareDefinedNonCommon(S, WasInserted, Binding);
   if (Cmp > 0)
     replaceBody<DefinedRegular<ELFT>>(S, Name, StOther, Type, Value, Size,
                                       Section);
   else if (Cmp == 0)
-    reportDuplicate(S->body(), Section->getFile());
+    reportDuplicate(S->body(), Section, Value);
   return S;
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, uint8_t StOther,
                                       InputSectionBase<ELFT> *Section,
                                       uint8_t Binding, uint8_t Type,
                                       uintX_t Value) {

test/ELF/Inputs/conflict-debug.s

+.file 1 "conflict-debug.s"
+.globl zed
+.loc 1 4
+zed:
+  nop

test/ELF/basic.s

 # RUN: not ld.lld %t -o 2>&1 | FileCheck --check-prefix=NO_O_VAL %s
 # NO_O_VAL: missing arg value for "-o", expected 1 argument.
 
 # RUN: not ld.lld --foo 2>&1 | FileCheck --check-prefix=UNKNOWN %s
 # UNKNOWN: unknown argument: --foo
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 # RUN: not ld.lld %t %t -o %t2 2>&1 | FileCheck --check-prefix=DUP %s
-# DUP: duplicate symbol: _start in {{.*}} and {{.*}}
+# DUP: {{.*}} (.text+0x0): duplicate symbol '_start'
+# DUP: {{.*}} (.text+0x0): previous definition was here
 
 # RUN: not ld.lld %t -o %t -m wrong_emul_fbsd 2>&1 | FileCheck --check-prefix=UNKNOWN_EMUL %s
 # UNKNOWN_EMUL: unknown emulation: wrong_emul_fbsd
 
 # RUN: not ld.lld %t --lto-partitions=0 2>&1 | FileCheck --check-prefix=NOTHREADS %s
 # NOTHREADS: --lto-partitions: number of threads must be > 0
 
 # RUN: not ld.lld %t --thinlto-jobs=0 2>&1 | FileCheck --check-prefix=NOTHREADSTHIN %s
 # NOTHREADSTHIN: --thinlto-jobs: number of threads must be > 0

test/ELF/conflict.s

 # REQUIRES: x86
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
 # RUN: not ld.lld %t1.o %t1.o -o %t2 2>&1 | FileCheck -check-prefix=DEMANGLE %s
 
-# DEMANGLE:    duplicate symbol: mul(double, double) in
-# DEMANGLE:    duplicate symbol: foo in
+# DEMANGLE:       {{.*}} (.text+0x0): duplicate symbol 'mul(double, double)'
+# DEMANGLE-NEXT:  {{.*}} (.text+0x0): previous definition was here
+# DEMANGLE-NEXT:  {{.*}} (.text+0x0): duplicate symbol 'foo'
+# DEMANGLE-NEXT:  {{.*}} (.text+0x0): previous definition was here
 
 # RUN: not ld.lld %t1.o %t1.o -o %t2 --no-demangle 2>&1 | \
 # RUN:   FileCheck -check-prefix=NO_DEMANGLE %s
 
-# NO_DEMANGLE: duplicate symbol: _Z3muldd in
-# NO_DEMANGLE: duplicate symbol: foo in
+# NO_DEMANGLE: {{.*}} (.text+0x0): duplicate symbol '_Z3muldd'
+# NO_DEMANGLE-NEXT: {{.*}} (.text+0x0): previous definition was here
+# NO_DEMANGLE-NEXT: {{.*}} (.text+0x0): duplicate symbol 'foo'
+# NO_DEMANGLE-NEXT: {{.*}} (.text+0x0): previous definition was here
 
 # RUN: not ld.lld %t1.o %t1.o -o %t2 --demangle --no-demangle 2>&1 | \
 # RUN:   FileCheck -check-prefix=NO_DEMANGLE %s
 # RUN: not ld.lld %t1.o %t1.o -o %t2 --no-demangle --demangle 2>&1 | \
 # RUN:   FileCheck -check-prefix=DEMANGLE %s
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/conflict.s -o %t2.o
 # RUN: llvm-ar rcs %t3.a %t2.o
 # RUN: not ld.lld %t1.o %t3.a -u baz -o %t2 2>&1 | FileCheck -check-prefix=ARCHIVE %s
 
-# ARCHIVE: duplicate symbol: foo in {{.*}}1.o and {{.*}}3.a({{.*}}2.o)
+# ARCHIVE:      {{.*}}3.a({{.*}}2.o) (.text+0x0): duplicate symbol 'foo'
+# ARCHIVE-NEXT: {{.*}}1.o (.text+0x0): previous definition was here
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/conflict-debug.s -o %t-dbg.o
+# RUN: not ld.lld %t-dbg.o %t-dbg.o -o %t-dbg 2>&1 | FileCheck -check-prefix=DBGINFO %s
+
+# DBGINFO:      conflict-debug.s (4): duplicate symbol 'zed'
+# DBGINFO-NEXT: conflict-debug.s (4): previous definition was here
 
 .globl _Z3muldd, foo
 _Z3muldd:
 foo:
   mov $60, %rax
   mov $42, %rdi
   syscall

test/ELF/lto/duplicated.ll

 ; REQUIRES: x86
 ; RUN: llvm-as %s -o %t.o
 ; RUN: not ld.lld -m elf_x86_64 %t.o %t.o -o %t.so -shared 2>&1 | FileCheck %s
-; CHECK: duplicate symbol: f in {{.*}}.o and {{.*}}.o
+; CHECK: duplicate symbol 'f' in {{.*}}.o and {{.*}}.o
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 define void @f() {
   ret void
 }

test/ELF/tls-mismatch.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/tls-mismatch.s -o %t2
 // RUN: not ld.lld %t %t2 -o %t3 2>&1 | FileCheck %s
-// CHECK: TLS attribute mismatch for symbol: tlsvar
+// CHECK: TLS attribute mismatch for symbol 'tlsvar'
 
 .globl _start,tlsvar
 _start:
   movl tlsvar,%edx