[lld] ed39fd5 - [lld-macho] Use source information in duplicate symbol errors

[Nico Weber via llvm-commits]

Author: Daniel Bertalan
Date: 2022-06-23T11:07:15-04:00
New Revision: ed39fd515a9e5cd7668847359dd3b5ce35ab5d0a

URL: https://github.com/llvm/llvm-project/commit/ed39fd515a9e5cd7668847359dd3b5ce35ab5d0a
DIFF: https://github.com/llvm/llvm-project/commit/ed39fd515a9e5cd7668847359dd3b5ce35ab5d0a.diff

LOG: [lld-macho] Use source information in duplicate symbol errors

Similarly to how undefined symbol diagnostics were changed in D128184,
we now show where in the source file duplicate symbols are defined at:

  ld64.lld: error: duplicate symbol: _foo
  >> defined in bar.c:42
  >>            /path/to/bar.o
  >> defined in baz.c:1
  >>            /path/to/libbaz.a(baz.o)

For objects that don't contain DWARF data, the format is unchanged.

A slight difference to undefined symbol diagnostics is that we don't
print the name of the symbol on the third line, as it's already
contained on the first line.

Differential Revision: https://reviews.llvm.org/D128425

Added: 
    lld/test/MachO/invalid/duplicate-symbol-debug.s

Modified: 
    lld/MachO/InputSection.cpp
    lld/MachO/SymbolTable.cpp
    lld/MachO/Symbols.cpp
    lld/MachO/Symbols.h

Removed: 
    


################################################################################
diff  --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index ab76b8fc94026..e683e0dbe3b7f 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -85,7 +85,7 @@ std::string InputSection::getLocation(uint64_t off) const {
 }
 
 std::string InputSection::getSourceLocation(uint64_t off) const {
-  auto *obj = dyn_cast<ObjFile>(getFile());
+  auto *obj = dyn_cast_or_null<ObjFile>(getFile());
   if (!obj)
     return {};
 

diff  --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index 3c95aacadf846..d309f66c119fb 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -83,9 +83,17 @@ Defined *SymbolTable::addDefined(StringRef name, InputFile *file,
           concatIsec->symbols.erase(llvm::find(concatIsec->symbols, defined));
         }
       } else {
-        error("duplicate symbol: " + toString(*defined) + "\n>>> defined in " +
-              toString(defined->getFile()) + "\n>>> defined in " +
-              toString(file));
+        std::string src1 = defined->getSourceLocation();
+        std::string src2 = isec ? isec->getSourceLocation(value) : "";
+
+        std::string message =
+            "duplicate symbol: " + toString(*defined) + "\n>>> defined in ";
+        if (!src1.empty())
+          message += src1 + "\n>>>            ";
+        message += toString(defined->getFile()) + "\n>>> defined in ";
+        if (!src2.empty())
+          message += src2 + "\n>>>            ";
+        error(message + toString(file));
       }
 
     } else if (auto *dysym = dyn_cast<DylibSymbol>(s)) {

diff  --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp
index 31ee4c65fe6d4..40e99cb6dde5d 100644
--- a/lld/MachO/Symbols.cpp
+++ b/lld/MachO/Symbols.cpp
@@ -100,6 +100,12 @@ void Defined::canonicalize() {
     isec = isec->canonical();
 }
 
+std::string Defined::getSourceLocation() {
+  if (!isec)
+    return {};
+  return isec->getSourceLocation(value);
+}
+
 uint64_t DylibSymbol::getVA() const {
   return isInStubs() ? getStubVA() : Symbol::getVA();
 }

diff  --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h
index 227244711c08e..c661913becf48 100644
--- a/lld/MachO/Symbols.h
+++ b/lld/MachO/Symbols.h
@@ -132,6 +132,8 @@ class Defined : public Symbol {
 
   uint64_t getVA() const override;
 
+  std::string getSourceLocation();
+
   // Ensure this symbol's pointers to InputSections point to their canonical
   // copies.
   void canonicalize();

diff  --git a/lld/test/MachO/invalid/duplicate-symbol-debug.s b/lld/test/MachO/invalid/duplicate-symbol-debug.s
new file mode 100644
index 0000000000000..576ab75fadb77
--- /dev/null
+++ b/lld/test/MachO/invalid/duplicate-symbol-debug.s
@@ -0,0 +1,43 @@
+# REQUIRES: aarch64
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %s -o %t-dup.o
+# RUN: not %lld -dylib -arch arm64 -o /dev/null %t.o %t-dup.o 2>&1 | FileCheck %s -DFILE_1=%t.o -DFILE_2=%t-dup.o
+
+# CHECK:      error: duplicate symbol: _foo
+# CHECK-NEXT: >>> defined in duplicate-symbol-debug.s:20
+# CHECK-NEXT: >>>            [[FILE_1]]
+# CHECK-NEXT: >>> defined in duplicate-symbol-debug.s:20
+# CHECK-NEXT: >>>            [[FILE_2]]
+
+## Test case adapted from lld/test/ELF/Inputs/vs-diagnostics-duplicate2.s
+
+.file 1 "" "duplicate-symbol-debug.s"
+
+.text
+
+.globl _foo
+.loc 1 20
+_foo:
+  nop
+
+.section __DWARF,__debug_abbrev,regular,debug
+  .byte  1                  ; Abbreviation code
+  .byte 17                  ; DW_TAG_compile_unit
+  .byte  0                  ; DW_CHILDREN_no
+  .byte 16                  ; DW_AT_stmt_list
+  .byte 23                  ; DW_FORM_sec_offset
+  .byte  0                  ; EOM(1)
+  .byte  0                  ; EOM(2)
+  .byte  0                  ; EOM(3)
+
+.section __DWARF,__debug_info,regular,debug
+  .long Lend0 - Lbegin0     ; Length of Unit
+Lbegin0:
+  .short 4                  ; DWARF version number
+  .long  __debug_abbrev     ; Offset Into Abbrev. Section
+  .byte  8                  ; Address Size (in bytes)
+  .byte  1                  ; Abbrev [1] 0xb:0x1f DW_TAG_compile_unit
+  .long  __debug_line       ; DW_AT_stmt_list
+Lend0:
+  .section __DWARF,__debug_line,regular,debug
+