Change the error message format for duplicate symbols.

[ruiu]

Summary

This patch is intended to improve readability of "duplicate symbol"
error messages.

Without this patch:

/ssd/clang/bin/ld.lld: error: /ssd/llvm-project/lld/ELF/Relocations.cpp:1054: duplicate symbol 'lld::elf::demangle(llvm::StringRef)'
/ssd/clang/bin/ld.lld: error: /ssd/llvm-project/lld/ELF/Strings.cpp:93: previous definition was here

With this patch:

/ssd/clang/bin/ld.lld: error: duplicate symbol: lld::elf::demangle(llvm::StringRef)
>>> defined at Strings.cpp:93 (/ssd/llvm-project/lld/ELF/Strings.cpp:93)
>>>            Strings.cpp.o:(lld::elf::demangle(llvm::StringRef)) in archive lib/liblldELF.a
>>> defined at Relocations.cpp:1054 (/ssd/llvm-project/lld/ELF/Relocations.cpp:1054)
>>>            Relocations.cpp.o:(.text+0x4C30) in archive lib/liblldELF.a

Discussion thread:
http://lists.llvm.org/pipermail/llvm-dev/2017-March/111459.html

[• rafael]

LGTM.

If we ever want to get fancier with our errors the only thing I can think of in accumulating info and printing at once during exit so that we can do things like

undefined foo, used in

>> use1
>> use2
>> use3

....

and correspondently for duplicates.

lld/trunk/ELF/SymbolTable.cpp

if (IsNew) {
     Sym->VersionId = Config->DefaultSymbolVersion;
     SymVector.push_back(Sym);
   } else {
     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 "'" + toString(*Existing) + "' in " + toString(Existing->File) +
-         " and " + toString(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));
+      ((Type == STT_TLS) != S->body()->isTls())) {
+    error("TLS attribute mismatch: " + toString(*S->body()) +
+          "\n>>> defined in " + toString(S->body()->File) +
+          "\n>>> defined in " + toString(File));
+  }
 
   return {S, WasInserted};
 }
 
 template <class ELFT> Symbol *SymbolTable<ELFT>::addUndefined(StringRef Name) {
   return addUndefined(Name, /*IsLocal=*/false, 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;
 }
 
-static void print(const Twine &Msg) {
+static void warnOrError(const Twine &Msg) {
   if (Config->AllowMultipleDefinition)
     warn(Msg);
   else
     error(Msg);
 }
 
-static void reportDuplicate(SymbolBody *Existing, InputFile *NewFile) {
-  print("duplicate symbol " + conflictMsg(Existing, NewFile));
+static void reportDuplicate(SymbolBody *Sym, InputFile *NewFile) {
+  warnOrError("duplicate symbol: " + toString(*Sym) +
+              "\n>>> defined in " + toString(Sym->File) +
+              "\n>>> defined in " + toString(NewFile));
 }
 
 template <class ELFT>
-static void reportDuplicate(SymbolBody *Existing, InputSectionBase *ErrSec,
+static void reportDuplicate(SymbolBody *Sym, InputSectionBase *ErrSec,
                             typename ELFT::uint ErrOffset) {
-  DefinedRegular *D = dyn_cast<DefinedRegular>(Existing);
+  DefinedRegular *D = dyn_cast<DefinedRegular>(Sym);
   if (!D || !D->Section || !ErrSec) {
-    reportDuplicate(Existing, ErrSec ? ErrSec->getFile<ELFT>() : nullptr);
+    reportDuplicate(Sym, ErrSec ? ErrSec->getFile<ELFT>() : nullptr);
     return;
   }
 
-  std::string OldLoc =
-      cast<InputSectionBase>(D->Section)->template getLocation<ELFT>(D->Value);
-  std::string NewLoc = ErrSec->getLocation<ELFT>(ErrOffset);
-
-  print(NewLoc + ": duplicate symbol '" + toString(*Existing) + "'");
-  print(OldLoc + ": previous definition was here");
+  // Construct and print an error message in the form of:
+  //
+  //   ld.lld: error: duplicate symbol: foo
+  //   >>> defined at bar.c:30
+  //   >>>            bar.o (/home/alice/src/bar.o)
+  //   >>> defined at baz.c:563
+  //   >>>            baz.o in archive libbaz.a
+  auto *Sec1 = cast<InputSectionBase>(D->Section);
+  std::string Src1 = Sec1->getSrcMsg<ELFT>(D->Value);
+  std::string Obj1 = Sec1->getObjMsg<ELFT>(D->Value);
+  std::string Src2 = ErrSec->getSrcMsg<ELFT>(ErrOffset);
+  std::string Obj2 = ErrSec->getObjMsg<ELFT>(ErrOffset);
+
+  std::string Msg = "duplicate symbol: " + toString(*Sym) + "\n>>> defined at ";
+  if (!Src1.empty())
+    Msg += Src1 + "\n>>>            ";
+  Msg += Obj1 + "\n>>> defined at ";
+  if (!Src2.empty())
+    Msg += Src2 + "\n>>>            ";
+  Msg += Obj2;
+  warnOrError(Msg);
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, uint8_t StOther,
                                       uint8_t Type, uint64_t Value,
                                       uint64_t Size, uint8_t Binding,
                                       SectionBase *Section, InputFile *File) {
   Symbol *S;

lld/trunk/test/ELF/abs-conflict.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: ld.lld %t.o %t.o -o %t.so -shared
 // RUN: llvm-readobj --dyn-symbols %t.so | FileCheck %s
 
 // CHECK:      Name: foo
 // CHECK-NEXT: Value: 0x123
 
 .global foo
 foo = 0x123
 
 // RUN: echo ".global foo; foo = 0x124" >  %t2.s
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %t2.s -o %t2.o
 // RUN: not ld.lld %t.o %t2.o -o %t.so -shared 2>&1 | FileCheck --check-prefix=DUP %s
 
-// DUP: duplicate symbol 'foo'
+// DUP:      duplicate symbol: foo
+// DUP-NEXT: >>> defined in {{.*}}.o
+// DUP-NEXT: >>> defined in (internal)

lld/trunk/test/ELF/basic.s

 # RUN: not ld.lld %t -o 2>&1 | FileCheck --check-prefix=NO_O_VAL %s
 # NO_O_VAL: -o: missing 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: {{.*}}:(.text+0x0): duplicate symbol '_start'
-# DUP: {{.*}}:(.text+0x0): previous definition was here
+# DUP:      duplicate symbol: _start
+# DUP-NEXT: >>> defined at {{.*}}:(.text+0x0)
+# DUP-NEXT: >>> defined at {{.*}}:(.text+0x0)
 
 # 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

lld/trunk/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:       {{.*}}:(.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
+# DEMANGLE:       duplicate symbol: mul(double, double)
+# DEMANGLE-NEXT:  >>> defined at {{.*}}:(.text+0x0)
+# DEMANGLE-NEXT:  >>> defined at {{.*}}:(.text+0x0)
+# DEMANGLE:       duplicate symbol: foo
+# DEMANGLE-NEXT:  >>> defined at {{.*}}:(.text+0x0)
+# DEMANGLE-NEXT:  >>> defined at {{.*}}:(.text+0x0)
 
 # RUN: not ld.lld %t1.o %t1.o -o %t2 --no-demangle 2>&1 | \
 # RUN:   FileCheck -check-prefix=NO_DEMANGLE %s
 
-# 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
+# NO_DEMANGLE:      duplicate symbol: _Z3muldd
+# NO_DEMANGLE-NEXT: >>> defined at {{.*}}:(.text+0x0)
+# NO_DEMANGLE-NEXT: >>> defined at {{.*}}:(.text+0x0)
+# NO_DEMANGLE:      duplicate symbol: foo
+# NO_DEMANGLE-NEXT: >>> defined at {{.*}}:(.text+0x0)
+# NO_DEMANGLE-NEXT: >>> defined at {{.*}}:(.text+0x0)
 
 # 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:      {{.*}}3.a({{.*}}2.o):(.text+0x0): duplicate symbol 'foo'
-# ARCHIVE-NEXT: {{.*}}1.o:(.text+0x0): previous definition was here
+# ARCHIVE:      duplicate symbol: foo
+# ARCHIVE-NEXT: >>> defined at {{.*}}:(.text+0x0)
+# ARCHIVE-NEXT: >>> defined at {{.*}}:(.text+0x0) in archive {{.*}}.a
 
 # 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
+# DBGINFO:      duplicate symbol: zed
+# DBGINFO-NEXT: >>> defined at conflict-debug.s:4
+# DBGINFO-NEXT: >>>            {{.*}}:(.text+0x0)
+# DBGINFO-NEXT: >>> defined at conflict-debug.s:4
+# DBGINFO-NEXT: >>>            {{.*}}:(.text+0x0)
 
 .globl _Z3muldd, foo
 _Z3muldd:
 foo:
   mov $60, %rax
   mov $42, %rdi
   syscall

lld/trunk/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
+; CHECK-NEXT: >>> defined in {{.*}}.o
+; CHECK-NEXT: >>> defined in {{.*}}.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
 }

lld/trunk/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: tlsvar
+// CHECK: >>> defined in
+// CHECK: >>> defined in
 
 .globl _start,tlsvar
 _start:
   movl tlsvar,%edx