[lld] d613417 - [lld-macho] Group undefined symbol diagnostics by symbol

[Nico Weber via llvm-commits]

Author: Daniel Bertalan
Date: 2022-06-14T16:38:11-04:00
New Revision: d61341768cf0cff7ceeaddecc2f769b5c1b901c4

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

LOG: [lld-macho] Group undefined symbol diagnostics by symbol

ld64.lld used to print the "undefined symbol" line for each reference to
an undefined symbol previously:

  ld64.lld: error: undefined symbol: _foo
  >>> referenced by /path/to/bar.o:(symbol _baz+0x0)

  ld64.lld: error: undefined symbol: _foo
  >>> referenced by /path/to/bar.o:(symbol _quux+0x1)

Now they are deduplicated:

  ld64.lld: error: undefined symbol: _foo
  >>> referenced by /path/to/bar.o:(symbol _baz+0x0)
  >>> referenced by /path/to/bar.o:(symbol _quux+0x1)

As with the other lld ports, only the first 3 references are printed.

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

Added: 
    lld/test/MachO/invalid/undef-multi.s

Modified: 
    lld/MachO/SymbolTable.cpp
    lld/MachO/SymbolTable.h
    lld/MachO/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index b0c83a568eb69..7705fbd5f6679 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -345,35 +345,80 @@ static bool recoverFromUndefinedSymbol(const Undefined &sym) {
   return false;
 }
 
-static void printUndefinedDiagnostic(StringRef name, StringRef source) {
-  std::string message = "undefined symbol";
-  if (config->archMultiple)
-    message += (" for arch " + getArchitectureName(config->arch())).str();
-  message += (": " + name + "\n>>> referenced by " + source).str();
-
-  if (config->undefinedSymbolTreatment == UndefinedSymbolTreatment::error)
-    error(message);
-  else if (config->undefinedSymbolTreatment ==
-           UndefinedSymbolTreatment::warning)
-    warn(message);
-  else
-    assert(false && "diagnostics make sense for -undefined error|warning only");
+struct UndefinedDiag {
+  struct SectionAndOffset {
+    const InputSection *isec;
+    uint64_t offset;
+  };
+
+  std::vector<SectionAndOffset> codeReferences;
+  std::vector<std::string> otherReferences;
+};
+
+static MapVector<const Undefined *, UndefinedDiag> undefs;
+
+void macho::reportPendingUndefinedSymbols() {
+  for (const auto &undef : undefs) {
+    const UndefinedDiag &locations = undef.second;
+
+    std::string message = "undefined symbol";
+    if (config->archMultiple)
+      message += (" for arch " + getArchitectureName(config->arch())).str();
+    message += ": " + toString(*undef.first);
+
+    const size_t maxUndefinedReferences = 3;
+    size_t i = 0;
+    for (const std::string &loc : locations.otherReferences) {
+      if (i >= maxUndefinedReferences)
+        break;
+      message += "\n>>> referenced by " + loc;
+      ++i;
+    }
+
+    for (const UndefinedDiag::SectionAndOffset &loc :
+         locations.codeReferences) {
+      if (i >= maxUndefinedReferences)
+        break;
+      // TODO: Get source file/line from debug information.
+      message += "\n>>> referenced by " + loc.isec->getLocation(loc.offset);
+      ++i;
+    }
+
+    size_t totalReferences =
+        locations.otherReferences.size() + locations.codeReferences.size();
+    if (totalReferences > i)
+      message +=
+          ("\n>>> referenced " + Twine(totalReferences - i) + " more times")
+              .str();
+
+    if (config->undefinedSymbolTreatment == UndefinedSymbolTreatment::error)
+      error(message);
+    else if (config->undefinedSymbolTreatment ==
+             UndefinedSymbolTreatment::warning)
+      warn(message);
+    else
+      assert(false &&
+             "diagnostics make sense for -undefined error|warning only");
+  }
+
+  // This function is called multiple times during execution. Clear the printed
+  // diagnostics to avoid printing the same things again the next time.
+  undefs.clear();
 }
 
-void lld::macho::treatUndefinedSymbol(const Undefined &sym, StringRef source) {
+void macho::treatUndefinedSymbol(const Undefined &sym, StringRef source) {
   if (recoverFromUndefinedSymbol(sym))
     return;
-  printUndefinedDiagnostic(sym.getName(), source);
+
+  undefs[&sym].otherReferences.push_back(source.str());
 }
 
-void lld::macho::treatUndefinedSymbol(const Undefined &sym,
-                                      const InputSection *isec,
-                                      uint64_t offset) {
+void macho::treatUndefinedSymbol(const Undefined &sym, const InputSection *isec,
+                                 uint64_t offset) {
   if (recoverFromUndefinedSymbol(sym))
     return;
 
-  // TODO: Get source file/line from debug information.
-  printUndefinedDiagnostic(toString(sym), isec->getLocation(offset));
+  undefs[&sym].codeReferences.push_back({isec, offset});
 }
 
 std::unique_ptr<SymbolTable> macho::symtab;

diff  --git a/lld/MachO/SymbolTable.h b/lld/MachO/SymbolTable.h
index a433f41318eae..0ecfa6dcd093a 100644
--- a/lld/MachO/SymbolTable.h
+++ b/lld/MachO/SymbolTable.h
@@ -69,6 +69,9 @@ class SymbolTable {
   std::vector<Symbol *> symVector;
 };
 
+void reportPendingUndefinedSymbols();
+
+// Call reportPendingUndefinedSymbols() to emit diagnostics.
 void treatUndefinedSymbol(const Undefined &, StringRef source);
 void treatUndefinedSymbol(const Undefined &, const InputSection *,
                           uint64_t offset);

diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index f2634076e0803..74612409bc53d 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -1133,6 +1133,7 @@ void Writer::writeCodeSignature() {
 void Writer::writeOutputFile() {
   TimeTraceScope timeScope("Write output file");
   openFile();
+  reportPendingUndefinedSymbols();
   if (errorCount())
     return;
   writeSections();
@@ -1155,6 +1156,7 @@ template <class LP> void Writer::run() {
   scanRelocations();
 
   // Do not proceed if there was an undefined symbol.
+  reportPendingUndefinedSymbols();
   if (errorCount())
     return;
 

diff  --git a/lld/test/MachO/invalid/undef-multi.s b/lld/test/MachO/invalid/undef-multi.s
new file mode 100644
index 0000000000000..c59bff2a5e633
--- /dev/null
+++ b/lld/test/MachO/invalid/undef-multi.s
@@ -0,0 +1,28 @@
+# REQUIRES: aarch64
+
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %s -o %t.o
+# RUN: not %lld -arch arm64 %t.o -o /dev/null 2>&1 | FileCheck -DFILE=%t.o %s
+
+# CHECK: error: undefined symbol: _undef
+# CHECK-NEXT: >>> referenced by [[FILE]]:(symbol _main+0x0)
+# CHECK-NEXT: >>> referenced by [[FILE]]:(symbol _foo+0x0)
+# CHECK-NEXT: >>> referenced by [[FILE]]:(symbol _bar+0x0)
+# CHECK-NEXT: >>> referenced 1 more times
+
+.globl _main
+_main:
+    b _undef
+
+.globl _foo
+_foo:
+    b _undef
+
+.global _bar
+_bar:
+    b _undef
+
+.globl _baz
+_baz:
+    b _undef
+
+.subsections_via_symbols