Port r363962 to COFF: Deduplicate undefined symbol diagnostics

lld/coff already deduplicated undefined symbols on a TU level: It would
group all references to a symbol from a single TU. This makes it so that
references from all TUs to a single symbol are grouped together.

Since lld/coff almost did what I thought it did already, the patch is
much smaller than the elf version. The only not local change is that
getSymbolLocations() now returns a vector<string> instead of a string,
so that the undefined symbol reporting code can know how many references
to a symbol exist in a given TU.

Fixes PR42260 for lld/coff.

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

llvm-svn: 364285

4 files changed, 116 insertions, 11 deletions

diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index f41354436fe..fbae6f9a272 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -336,8 +336,15 @@ static void maybeReportRelocationToDiscarded(const SectionChunk *FromChunk,
     File->getCOFFObj()->getSymbolName(COFFSym, Name);
   }
 
-  error("relocation against symbol in discarded section: " + Name +
-        getSymbolLocations(File, Rel.SymbolTableIndex));
+  std::vector<std::string> SymbolLocations =
+      getSymbolLocations(File, Rel.SymbolTableIndex);
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  OS << "relocation against symbol in discarded section: " + Name;
+  for (const std::string &S : SymbolLocations)
+    OS << S;
+  error(OS.str());
 }
 
 void SectionChunk::writeTo(uint8_t *Buf) const {
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 27bf127a8ad..529562d807d 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -79,7 +79,11 @@ static Symbol *getSymbol(SectionChunk *SC, uint32_t Addr) {
   return Candidate;
 }
 
-std::string getSymbolLocations(ObjFile *File, uint32_t SymIndex) {
+// Given a file and the index of a symbol in that file, returns a description
+// of all references to that symbol from that file. If no debug information is
+// available, returns just the name of the file, else one string per actual
+// reference as described in the debug info.
+std::vector<std::string> getSymbolLocations(ObjFile *File, uint32_t SymIndex) {
   struct Location {
     Symbol *Sym;
     std::pair<StringRef, uint32_t> FileLine;
@@ -102,11 +106,12 @@ std::string getSymbolLocations(ObjFile *File, uint32_t SymIndex) {
   }
 
   if (Locations.empty())
-    return "\n>>> referenced by " + toString(File);
+    return std::vector<std::string>({"\n>>> referenced by " + toString(File)});
 
-  std::string Out;
-  llvm::raw_string_ostream OS(Out);
+  std::vector<std::string> SymbolLocations(Locations.size());
+  size_t I = 0;
   for (Location Loc : Locations) {
+    llvm::raw_string_ostream OS(SymbolLocations[I++]);
     OS << "\n>>> referenced by ";
     if (!Loc.FileLine.first.empty())
       OS << Loc.FileLine.first << ":" << Loc.FileLine.second
@@ -115,7 +120,41 @@ std::string getSymbolLocations(ObjFile *File, uint32_t SymIndex) {
     if (Loc.Sym)
       OS << ":(" << toString(*Loc.Sym) << ')';
   }
-  return OS.str();
+  return SymbolLocations;
+}
+
+// For an undefined symbol, stores all files referencing it and the index of
+// the undefined symbol in each file.
+struct UndefinedDiag {
+  Symbol *Sym;
+  struct File {
+    ObjFile *File;
+    uint64_t SymIndex;
+  };
+  std::vector<File> Files;
+};
+
+static void reportUndefinedSymbol(const UndefinedDiag &UndefDiag) {
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  OS << "undefined symbol: " << toString(*UndefDiag.Sym);
+
+  const size_t MaxUndefReferences = 10;
+  size_t I = 0, NumRefs = 0;
+  for (const UndefinedDiag::File &Ref : UndefDiag.Files) {
+    std::vector<std::string> SymbolLocations =
+        getSymbolLocations(Ref.File, Ref.SymIndex);
+    NumRefs += SymbolLocations.size();
+    for (const std::string &S : SymbolLocations) {
+      if (I >= MaxUndefReferences)
+        break;
+      OS << S;
+      I++;
+    }
+  }
+  if (I < NumRefs)
+    OS << "\n>>> referenced " << NumRefs - I << " more times";
+  errorOrWarn(OS.str());
 }
 
 void SymbolTable::loadMinGWAutomaticImports() {
@@ -263,15 +302,24 @@ void SymbolTable::reportRemainingUndefines() {
              " (defined in " + toString(Imp->getFile()) + ") [LNK4217]");
   }
 
+  std::vector<UndefinedDiag> UndefDiags;
+  DenseMap<Symbol *, int> FirstDiag;
+
   for (ObjFile *File : ObjFile::Instances) {
     size_t SymIndex = (size_t)-1;
     for (Symbol *Sym : File->getSymbols()) {
       ++SymIndex;
       if (!Sym)
         continue;
-      if (Undefs.count(Sym))
-        errorOrWarn("undefined symbol: " + toString(*Sym) +
-                    getSymbolLocations(File, SymIndex));
+      if (Undefs.count(Sym)) {
+        auto it = FirstDiag.find(Sym);
+        if (it == FirstDiag.end()) {
+          FirstDiag[Sym] = UndefDiags.size();
+          UndefDiags.push_back({Sym, {{File, SymIndex}}});
+        } else {
+          UndefDiags[it->second].Files.push_back({File, SymIndex});
+        }
+      }
       if (Config->WarnLocallyDefinedImported)
         if (Symbol *Imp = LocalImports.lookup(Sym))
           warn(toString(File) +
@@ -279,6 +327,9 @@ void SymbolTable::reportRemainingUndefines() {
                " (defined in " + toString(Imp->getFile()) + ") [LNK4217]");
     }
   }
+
+  for (const UndefinedDiag& UndefDiag : UndefDiags)
+    reportUndefinedSymbol(UndefDiag);
 }
 
 std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name) {
diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h
index d01bac1c252..05417cfefea 100644
--- a/lld/COFF/SymbolTable.h
+++ b/lld/COFF/SymbolTable.h
@@ -123,7 +123,7 @@ private:
 
 extern SymbolTable *Symtab;
 
-std::string getSymbolLocations(ObjFile *File, uint32_t SymIndex);
+std::vector<std::string> getSymbolLocations(ObjFile *File, uint32_t SymIndex);
 
 } // namespace coff
 } // namespace lld
diff --git a/lld/test/COFF/undefined-symbol-multi.s b/lld/test/COFF/undefined-symbol-multi.s
new file mode 100644
index 00000000000..5942972a025
--- /dev/null
+++ b/lld/test/COFF/undefined-symbol-multi.s
@@ -0,0 +1,47 @@
+# REQUIRES: x86
+# RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t.obj %s
+
+# All references to a single undefined symbol count as a single error -- but
+# at most 10 references are printed.
+# RUN: echo ".globl bar" > %t.moreref.s
+# RUN: echo "bar:" >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: echo '  call "?foo@@YAHXZ"' >> %t.moreref.s
+# RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t2.obj %t.moreref.s
+# RUN: not lld-link /out:/dev/null  %t.obj %t2.obj 2>&1 | FileCheck %s
+
+# CHECK: error: undefined symbol: int __cdecl foo(void)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp.obj:(main)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp.obj:(main)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.obj:(bar)
+# CHECK-NEXT: >>> referenced 2 more times
+# CHECK-EMPTY:
+# CHECK-NEXT: error: undefined symbol: int __cdecl bar(void)
+# CHECK-NEXT: >>> referenced by {{.*}}.obj:(main)
+# CHECK-NEXT: >>> referenced by {{.*}}.obj:(f1)
+
+        .section        .text,"xr",one_only,main
+.globl main
+main:
+	call	"?foo@@YAHXZ"
+	call	"?foo@@YAHXZ"
+	call	"?bar@@YAHXZ"
+
+f1:
+	call	"?bar@@YAHXZ"
+.Lfunc_end1: