Port r363962 to COFF: Deduplicate undefined symbol diagnostics

[thakis]

Summary

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.

[thakis]

ruiu: ping :)

[ruiu]

LGTM

lld/COFF/Chunks.cpp

static void maybeReportRelocationToDiscarded(const SectionChunk *FromChunk,
   if (Sym) {
     Name = Sym->getName();
   } else {
     COFFSymbolRef COFFSym =
         check(File->getCOFFObj()->getSymbol(Rel.SymbolTableIndex));
     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 {
   if (!hasData())
     return;
   // Copy section contents from source object file to output file.
   ArrayRef<uint8_t> A = getContents();
   if (!A.empty())

lld/COFF/SymbolTable.h

private:
   std::vector<Symbol *> getSymsWithPrefix(StringRef Prefix);
 
   llvm::DenseMap<llvm::CachedHashStringRef, Symbol *> SymMap;
   std::unique_ptr<BitcodeCompiler> LTO;
 };
 
 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
 
 #endif

lld/COFF/SymbolTable.cpp

if (!D || D->getChunk() != SC || D->getValue() > Addr ||
       continue;
 
     Candidate = D;
   }
 
   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;
   };
   std::vector<Location> Locations;
 
   for (Chunk *C : File->getChunks()) {
     auto *SC = dyn_cast<SectionChunk>(C);
     if (!SC)
       continue;
     for (const coff_relocation &R : SC->getRelocs()) {
       if (R.SymbolTableIndex != SymIndex)
         continue;
       std::pair<StringRef, uint32_t> FileLine =
           getFileLine(SC, R.VirtualAddress);
       Symbol *Sym = getSymbol(SC, R.VirtualAddress);
       if (!FileLine.first.empty() || Sym)
         Locations.push_back({Sym, FileLine});
     }
   }
 
   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
          << "\n>>>               ";
     OS << toString(File);
     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() {
   for (auto &I : SymMap) {
     Symbol *Sym = I.second;
     auto *Undef = dyn_cast<Undefined>(Sym);
     if (!Undef)
       continue;
for (Symbol *B : Config->GCRoot) {
     if (Undefs.count(B))
       errorOrWarn("<root>: undefined symbol: " + toString(*B));
     if (Config->WarnLocallyDefinedImported)
       if (Symbol *Imp = LocalImports.lookup(B))
         warn("<root>: locally defined symbol imported: " + toString(*Imp) +
              " (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) +
                ": locally defined symbol imported: " + toString(*Imp) +
                " (defined in " + toString(Imp->getFile()) + ") [LNK4217]");
     }
   }
+
+  for (const UndefinedDiag& UndefDiag : UndefDiags)
+    reportUndefinedSymbol(UndefDiag);
 }
 
 std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name) {
   bool Inserted = false;
   Symbol *&Sym = SymMap[CachedHashStringRef(Name)];
   if (!Sym) {
     Sym = reinterpret_cast<Symbol *>(make<SymbolUnion>());
     Sym->IsUsedInRegularObj = false;

lld/test/COFF/undefined-symbol-multi.s

+# 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: