lld/elf: Deduplicate undefined symbol diagnostics

[thakis]

Summary

Before:

ld.lld: error: undefined symbol: f()
>>> referenced by test.cc:3
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-9c0808.o:(g())

ld.lld: error: undefined symbol: f()
>>> referenced by test.cc:4
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-9c0808.o:(h())

ld.lld: error: undefined symbol: f()
>>> referenced by test.cc:5
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-9c0808.o:(j())

ld.lld: error: undefined symbol: k()
>>> referenced by test.cc:5
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-9c0808.o:(j())

ld.lld: error: undefined symbol: f()
>>> referenced by test2.cc:2
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test2-07b391.o:(asdf())
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Now:

ld.lld: error: undefined symbol: f()
>>> referenced by test.cc:3
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-0e07ba.o:(g())
>>> referenced by test.cc:4
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-0e07ba.o:(h())
>>> referenced by test.cc:5
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-0e07ba.o:(j())
>>> referenced by test2.cc:2
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test2-6bdb24.o:(asdf())

ld.lld: error: undefined symbol: k()
>>> referenced by test.cc:5
>>>               /var/folders/c5/8d7sdn1x2mg92mj0rndghhdr0000gn/T/test-0e07ba.o:(j())
clang: error: linker command failed with exit code 1 (use -v to see invocation)

If there are more than 10 references to an undefined symbol, only the
first 10 are printed.

Fixes PR42260.

[ruiu]

I think that the three patches make sense only when they are combined, so could you merge them as a single patch?

lld/Common/ErrorHandler.cpp

137 ↗

(On Diff #204800)

To me, reporting a single undefined symbol that is used by multiple object files is still counted as a single error. That kind of error is actually likely to be caused by a single error to forget defining a symbol or something.

I don't feel strongly about that, though. But in that case, I'd choose a simpler one. Handle all error() calls as a single erorr is simpler.

[thakis]

squash commits, and remove error limit logic

[thakis]

lld/Common/ErrorHandler.cpp

137 ↗

(On Diff #204800)

I was unsure about this too. I think the motivation for -error-limit is probably to prevent unbounded error output by default, and if a single undef symbol can now produce an unboundedly long list of references, we lose that property. But the implementation was not very elegant. Another idea would be to only omit the first (say) 3 references and then say ">> and 532 more references". If we wanted we could then add a "-undef-limit" flag for people who want to see more than 3. WDYT?

[ruiu]

I agree that that's perhaps better to omit an error message as >> and 532 more references if it is too large, but I don't know how large it can be. Can this be hundreds of lines?

lld/ELF/Relocations.cpp

720

nit: add a blank line

In lld we honestly don't care too much about eliminating global initializers, so it can probably be just a global variable, but you need to erase the contents before the first use so that multiple runs of lld::elf::main in the same process works as expected.

784

Replace auto with a concrete type.

814–815

You can define bool IsWarning instead of constructing an entire Undef here

818

Is this really called by a multithreaded code?

821

Then you can construct an UndefinedDiag as an unnamed struct

Undefs().push_back({&Sym, {{&Sec, Offset}}, IsWarning});

[thakis]

comments

[thakis]

I made it so that it prints at most 10 references. Most of the time when I get this diagnostic, it's because the symbol doesn't exist, and seeing many references to it isn't useful.

lld/ELF/Relocations.cpp

720

Made it a global. (You might want to reconsider it at some point, though. The main cost of static initializers, other than that initialization order is undefined, is that the code for the initializer needs to be paged in. For example, the coff linker needs to load all the bits of the elf linker that is for static initializers, even though they'll never be used. See also https://web.archive.org/web/20160811161720/https://blog.mozilla.org/tglek/2010/05/27/startup-backward-constructors/ / https://glandium.org/blog/?cat=13&paged=3)

818

Hm, I hadn't really checked, I had believed the comment on the bug. It's called from here http://llvm-cs.pcc.me.uk/tools/lld/ELF/Writer.cpp#1741 , but it looks like forEachRelSec() doesn't do any parallel for loops, so probably not. Removed the lock.

[ruiu]

Generally looking good, but I have one question.

Before this patch, undefined symbols are reported as we find them. Now, we buffer error messages and print them out after visiting all relocations. So the delay to get a first error message if there's an undefined symbol will increase with this patch. How long is the increase? If it is only a few hundred milliseconds, it is okay, but if it the second time scale, we may have to do this in a different way.

lld/ELF/Relocations.cpp

747–756

k -> K

or just remove k

748

i -> I

805

I

806

auto -> UndefinedDiag

[grimar]

lld/ELF/Relocations.cpp

717

Seems this field can be calculated at any time, does it worth to keep the precalculated value?

804

I'd use llvm::DenseMap. It is probably a bit more consistent with the rest LLD ELF code,
and allows then to use its lookup method below:

if (Symbol *S = FirstRef.lookup(Undef.Sym)) {
...
}

824

It is not clang formatted.

[MaskRay]

lld/test/ELF/undef-multi.s

5

%t.exe -> /dev/null if you don't need the output.

18

Just a suggestion. It is now common to use ## for comments that are not RUN/CHECK lines.

[MaskRay]

lld/ELF/Relocations.cpp

717

IsWarning is computed on a per-symbol basis:

bool CanBeExternal = !Sym.isLocal() && Sym.computeBinding() != STB_LOCAL &&
                       Sym.Visibility == STV_DEFAULT;

  bool IsWarning =
      (Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal) ||
      Config->NoinhibitExec;

[grimar]

lld/ELF/Relocations.cpp

717

Yes and we have Symbol * UndefinedDiag::Sym here, so can calculate it on fly I think.

[thakis]

comments

[thakis]

How long is the increase?

That's a good question. Intuitively I'd say it shouldn't be all that much: Undefined diags were emitted doing relocation scanning, which I think happens fairly late in the link, and now it happens after it. So the max latency added is the time it takes to scan relocations. I'd expect that relocation scanning time is small compared to total link time. But I'll measure and report back.

lld/ELF/Relocations.cpp

717

It's needed as a return value; doing the computation twice seems less good to me?

804

Done, I think. (I had to change the value type to UndefinedDiag, and do if (UndefinedDiag *... to make it work; that's my best guess at what you meant. Maybe I got it wrong.)

lld/test/ELF/undef-multi.s

18

$ git grep -l '^# ' lld/test/ | wc -l
    1388
$ git grep -l '^## ' lld/test/ | wc -l
     201

In LLVM, it's 4766/176.

So it's an order of magnitude more common to not do this. Unless there's an effort to rewrite all of llvm's existing tests to this new style and enforce it, using ## for comments seems like it decreases llvm's self-consistency and I suggest everyone adding comments like this to stop doing it.

[grimar]

lld/ELF/Relocations.cpp

717

I think usually we do not store things that can be computated trivially. Lets see what others think about this one though.

804

You got it right.

[ruiu]

In D63344#1548106, @thakis wrote:

How long is the increase?

That's a good question. Intuitively I'd say it shouldn't be all that much: Undefined diags were emitted doing relocation scanning, which I think happens fairly late in the link, and now it happens after it. So the max latency added is the time it takes to scan relocations. I'd expect that relocation scanning time is small compared to total link time. But I'll measure and report back.

Thanks. I have a feeling that the relocation scan pass is one of the heaviest pass in lld, so it's worth measuring.

[thakis]

In D63344#1548150, @ruiu wrote:

In D63344#1548106, @thakis wrote:

How long is the increase?

That's a good question. Intuitively I'd say it shouldn't be all that much: Undefined diags were emitted doing relocation scanning, which I think happens fairly late in the link, and now it happens after it. So the max latency added is the time it takes to scan relocations. I'd expect that relocation scanning time is small compared to total link time. But I'll measure and report back.

Thanks. I have a feeling that the relocation scan pass is one of the heaviest pass in lld, so it's worth measuring.

I hooked up the ScopedTimer stuff that lld/COFF uses to lld/ELF (if there's interest, I can send a patch for that), and here's the output for linking libblink_core.so in a debug build:

  Reloc scanning:               239 ms (  4.0%)
-------------------------------------------------
Total Link Time:               5975 ms (100.0%)

[ruiu]

In D63344#1548151, @thakis wrote:

In D63344#1548150, @ruiu wrote:

In D63344#1548106, @thakis wrote:

How long is the increase?

That's a good question. Intuitively I'd say it shouldn't be all that much: Undefined diags were emitted doing relocation scanning, which I think happens fairly late in the link, and now it happens after it. So the max latency added is the time it takes to scan relocations. I'd expect that relocation scanning time is small compared to total link time. But I'll measure and report back.

Thanks. I have a feeling that the relocation scan pass is one of the heaviest pass in lld, so it's worth measuring.

I hooked up the ScopedTimer stuff that lld/COFF uses to lld/ELF (if there's interest, I can send a patch for that), and here's the output for linking libblink_core.so in a debug build:

  Reloc scanning:               239 ms (  4.0%)
-------------------------------------------------
Total Link Time:               5975 ms (100.0%)

Ah, this number seems completely acceptable. Thank you very much for measuring it.

[thakis]

Apologies, that number is wrong, I had a duplicate call to ScopedTimer in the main function. Here's the correct number:

  Reloc scanning:               220 ms (  7.0%)
-------------------------------------------------
Total Link Time:               3147 ms (100.0%)

I'll also do a static library build of chrome.

[ruiu]

lld/ELF/Relocations.cpp

717

Repeating the same code in two functions may be bad, but if you factor the code out and define a predicate function to determine whether we should report an error or a warning, it might look good. It may be worth trying.

[thakis]

For a static debug build of chrome (since the link time is a bit long, this configuration isn't officially supported and hits an assert("don't use this config") that I commented out):

  Reloc scanning:              1304 ms (  7.0%)
-------------------------------------------------
Total Link Time:              18744 ms (100.0%)

So it's not nothing, but it's also < 10 %, and this isn't link slowdown but latency for seeing undefined symbol diagnostics. Personally, I've never started looking at undefined symbols while they're still scrolling by, and latency for being done with printing undefined symbols is unchanged. So I'd say this is fine, but it's a bit of a judgement call.

[ruiu]

In D63344#1548162, @thakis wrote:

For a static debug build of chrome (since the link time is a bit long, this configuration isn't officially supported and hits an assert("don't use this config") that I commented out):

  Reloc scanning:              1304 ms (  7.0%)
-------------------------------------------------
Total Link Time:              18744 ms (100.0%)

So it's not nothing, but it's also < 10 %, and this isn't link slowdown but latency for seeing undefined symbol diagnostics. Personally, I've never started looking at undefined symbols while they're still scrolling by, and latency for being done with printing undefined symbols is unchanged. So I'd say this is fine, but it's a bit of a judgement call.

I'd think that what you should actually compare is the time to get an error message before this patch and after this patch, instead of the total link time. If lld spends zero time before relocation processing (which is unrealistic but just an example), users would have been able to get an error message immediately (assuming that relocations errors are everywhere) and they would have to wait 1300 ms after this patch.

But if you wait for 10 seconds for lld to start relocation processing, additional 1 second may not be a big deal.

[thakis]

use predicate function instead of storing IsWarning

[thakis]

actually use predicate function

[thakis]

The latest patch set uses predicate functions instead of storing IsWarning(). It looks less clear to me, but let me know what you think.

[thakis]

I'd think that what you should actually compare is the time to get an error message before this patch and after this patch, instead of the total link time. If lld spends zero time before relocation processing (which is unrealistic but just an example), users would have been able to get an error message immediately (assuming that relocations errors are everywhere) and they would have to wait 1300 ms after this patch.

But if you wait for 10 seconds for lld to start relocation processing, additional 1 second may not be a big deal.

Ah, I see. I added more timers:

  Before reloc scanning:      12705 ms ( 66.5%)
  Reloc scanning:              1362 ms (  7.1%)
  After reloc scanning:        5046 ms ( 26.4%)
-------------------------------------------------
Total Link Time:              19113 ms (100.0%)

So before you'd get an error 12.7s-14.0s in, now it's consistently after 14s. (But if there are multiple errors that scroll by, chances are you could only act after 14s previously as well.)

grimar, should I keep the current version with the predicate, or the version with the IsWarning flag? I don't have a strong preference.

And would there be interest in a -time-report flag that adds a summary like the above? (Not with "before reloc scanning" and "after reloc scanning", but with timers like "reading inputs" etc, like lld/COFF has.)

[grimar]

In D63344#1548225, @thakis wrote:

grimar, should I keep the current version with the predicate, or the version with the IsWarning flag? I don't have a strong preference.

My opinion is that original version was slightly simpler, so I would return to using IsWarning flag probably. Unless there is a better way to use predicates.

[thakis]

back to IsWarning

[MaskRay]

lld/ELF/Relocations.cpp

802

I think this loop can be changed to range-based

[thakis]

use range loop

[thakis]

All comments addressed :) Can I get an lg?

[MaskRay]

In D63344#1548314, @thakis wrote:

All comments addressed :) Can I get an lg?

I think you will:)

I have no more comments on this patch.

[ruiu]

LGTM

lld/ELF/Relocations.cpp

750

nit: you can do I++ here.

[thakis]

Thanks!

lld/ELF/Relocations.cpp

750

It's not quite the same: If the break is taken, I is incremented then, and it isn't currently. I could adjust for that in the if below, or start I at -1 (and change the init value at -1 and the type to int and add a cat to the if below), but everything I could think of so far is more complicated than what I currently have.

lld/ELF/Relocations.h

 struct Relocation {
   RelExpr Expr;
   RelType Type;
   uint64_t Offset;
   int64_t Addend;
   Symbol *Sym;
 };
 
+// This function writes undefined symbol diagnostics to an internal buffer.
+// Call reportUndefinedSymbols() after calling scanRelocations() to emit
+// the diagnostics.
 template <class ELFT> void scanRelocations(InputSectionBase &);
 
+template <class ELFT> void reportUndefinedSymbols();
+
 void addIRelativeRelocs();
 
 class ThunkSection;
 class Thunk;
 struct InputSectionDescription;
 
 class ThunkCreator {
 public:

lld/ELF/Relocations.cpp

static std::string maybeReportDiscarded(Undefined &Sym) {
   StringRef Signature = File->getShtGroupSignature(ObjSections, ELFSec);
   if (const InputFile *Prevailing =
           Symtab->ComdatGroups.lookup(CachedHashStringRef(Signature)))
     Msg += "\n>>> section group signature: " + Signature.str() +
            "\n>>> prevailing definition is in " + toString(Prevailing);
   return Msg;
 }
 
-// Report an undefined symbol if necessary.
-// Returns true if this function printed out an error message.
-template <class ELFT>
-static bool maybeReportUndefined(Symbol &Sym, InputSectionBase &Sec,
-                                 uint64_t Offset) {
-  if (!Sym.isUndefined() || Sym.isWeak())
-    return false;
-
-  bool CanBeExternal = !Sym.isLocal() && Sym.computeBinding() != STB_LOCAL &&
-                       Sym.Visibility == STV_DEFAULT;
-  if (Config->UnresolvedSymbols == UnresolvedPolicy::Ignore && CanBeExternal)
-    return false;
+// Undefined diagnostics are collected in a vector and emitted once all of
+// them are known, so that some postprocessing on the list of undefined symbols
+// can happen before lld emits diagnostics.
+struct UndefinedDiag {
+  Symbol *Sym;
+  struct Loc {
+    InputSectionBase *Sec;
+    uint64_t Offset;
+  };
+  std::vector<Loc> Locs;
+  bool IsWarning;
+};
+static std::vector<UndefinedDiag>& Undefs() {
+  // Function-local static to not require a global initializer.
+  static std::vector<UndefinedDiag> UndefsVector;
+  return UndefsVector;
+}
 
-  // clang (as of 2019-06-12) / gcc (as of 8.2.1) PPC64 may emit a .rela.toc
-  // which references a switch table in a discarded .rodata/.text section. The
-  // .toc and the .rela.toc are incorrectly not placed in the comdat. The ELF
-  // spec says references from outside the group to a STB_LOCAL symbol are not
-  // allowed. Work around the bug.
-  if (Config->EMachine == EM_PPC64 &&
-      cast<Undefined>(Sym).DiscardedSecIdx != 0 && Sec.Name == ".toc")
-    return false;
+template <class ELFT>
+static void reportUndefinedSymbol(const UndefinedDiag &Undef) {
+  Symbol &Sym = *Undef.Sym;
 
   auto Visibility = [&]() -> std::string {
     switch (Sym.Visibility) {
     case STV_INTERNAL:
       return "internal ";
     case STV_HIDDEN:
       return "hidden ";
     case STV_PROTECTED:
       return "protected ";
     default:
       return "";
     }
   };
 
   std::string Msg = maybeReportDiscarded<ELFT>(cast<Undefined>(Sym));
   if (Msg.empty())
     Msg = "undefined " + Visibility() + "symbol: " + toString(Sym);
 
+  for (UndefinedDiag::Loc L : Undef.Locs) {
+    InputSectionBase &Sec = *L.Sec;
+    uint64_t Offset = L.Offset;
+
     Msg += "\n>>> referenced by ";
     std::string Src = Sec.getSrcMsg(Sym, Offset);
     if (!Src.empty())
       Msg += Src + "\n>>>               ";
     Msg += Sec.getObjMsg(Offset);
+  }
 
   if (Sym.getName().startswith("_ZTV"))
     Msg += "\nthe vtable symbol may be undefined because the class is missing "
            "its key function (see https://lld.llvm.org/missingkeyfunction)";
 
-  if ((Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal) ||
-      Config->NoinhibitExec) {
+  if (Undef.IsWarning)
     warn(Msg);
+  else
+    error(Msg);
+}
+
+template <class ELFT>
+void elf::reportUndefinedSymbols() {
+  // Find the first "undefined symbol" diagnostic for each diagnostic, and
+  // collect all "referenced from" lines at the first diagnostic.
+  std::map<Symbol*, size_t> FirstRef;
+  for (size_t i = 0; i < Undefs().size(); ++i) {
+    auto& Undef = Undefs()[i];
+    auto it = FirstRef.find(Undef.Sym);
+    if (it == FirstRef.end()) {
+      FirstRef[Undef.Sym] = i;
+    } else {
+      Undefs()[it->second].Locs.push_back(Undef.Locs[0]);
+      Undef.Locs.clear();
+    }
+  }
+
+  for (const auto& Undef : Undefs()) {
+    if (!Undef.Locs.empty())
+      reportUndefinedSymbol<ELFT>(Undef);
+  }
+}
+
+// Report an undefined symbol if necessary.
+// Returns true if this function printed out an error message.
+template <class ELFT>
+static bool maybeReportUndefined(Symbol &Sym, InputSectionBase &Sec,
+                                 uint64_t Offset) {
+  if (!Sym.isUndefined() || Sym.isWeak())
+    return false;
+
+  bool CanBeExternal = !Sym.isLocal() && Sym.computeBinding() != STB_LOCAL &&
+                       Sym.Visibility == STV_DEFAULT;
+  if (Config->UnresolvedSymbols == UnresolvedPolicy::Ignore && CanBeExternal)
     return false;
+
+  // clang (as of 2019-06-12) / gcc (as of 8.2.1) PPC64 may emit a .rela.toc
+  // which references a switch table in a discarded .rodata/.text section. The
+  // .toc and the .rela.toc are incorrectly not placed in the comdat. The ELF
+  // spec says references from outside the group to a STB_LOCAL symbol are not
+  // allowed. Work around the bug.
+  if (Config->EMachine == EM_PPC64 &&
+      cast<Undefined>(Sym).DiscardedSecIdx != 0 && Sec.Name == ".toc")
+    return false;
+
+
+  UndefinedDiag Undef = { &Sym, {{&Sec, Offset}}, /*IsWarning=*/false };
+  if ((Config->UnresolvedSymbols == UnresolvedPolicy::Warn && CanBeExternal) ||
+      Config->NoinhibitExec)
+    Undef.IsWarning = true;
+
+  {
+    static std::mutex Mu;
+    std::lock_guard<std::mutex> Lock(Mu);
+    Undefs().push_back(Undef);
   }
 
-  error(Msg);
-  return true;
+  return !Undef.IsWarning;
 }
 
 // MIPS N32 ABI treats series of successive relocations with the same offset
 // as a single relocation. The similar approach used by N64 ABI, but this ABI
 // packs all relocations into the single relocation record. Here we emulate
 // this for the N32 ABI. Iterate over relocation with the same offset and put
 // theirs types into the single bit-set.
 template <class RelTy> static RelType getMipsN32RelType(RelTy *&Rel, RelTy *End) {
bool ThunkCreator::createThunks(ArrayRef<OutputSection *> OutputSections) {
   ++Pass;
   return AddressesChanged;
 }
 
 template void elf::scanRelocations<ELF32LE>(InputSectionBase &);
 template void elf::scanRelocations<ELF32BE>(InputSectionBase &);
 template void elf::scanRelocations<ELF64LE>(InputSectionBase &);
 template void elf::scanRelocations<ELF64BE>(InputSectionBase &);
+template void elf::reportUndefinedSymbols<ELF32LE>();
+template void elf::reportUndefinedSymbols<ELF32BE>();
+template void elf::reportUndefinedSymbols<ELF64LE>();
+template void elf::reportUndefinedSymbols<ELF64BE>();

lld/ELF/Writer.cpp

 
   Symtab->forEachSymbol([](Symbol *S) {
     if (!S->IsPreemptible)
       S->IsPreemptible = computeIsPreemptible(*S);
   });
 
   // Scan relocations. This must be done after every symbol is declared so that
   // we can correctly decide if a dynamic relocation is needed.
-  if (!Config->Relocatable)
+  if (!Config->Relocatable) {
     forEachRelSec(scanRelocations<ELFT>);
+    reportUndefinedSymbols<ELFT>();
+  }
 
   addIRelativeRelocs();
 
   if (In.Plt && In.Plt->isNeeded())
     In.Plt->addSymbols();
   if (In.Iplt && In.Iplt->isNeeded())
     In.Iplt->addSymbols();
 

lld/test/ELF/debug-line-obj.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux -dwarf-version=5 %s -o %t.o
 # RUN: not ld.lld %t.o -o %t1 2>&1 | FileCheck %s
 
 # When compiling with -ffunction-sections, .debug_line may contain descriptions
 # of locations from the different text sections. Until relocated such
 # descriptions might contain overlapping offsets. Check LLD is able to report
 # the error locations correctly in this case.
 
 # CHECK:      error: undefined symbol: foo()
 # CHECK-NEXT: >>> referenced by test.cpp:2
 # CHECK-NEXT: >>>               {{.*}}.o:(bar())
-# CHECK:      error: undefined symbol: foo()
 # CHECK-NEXT: >>> referenced by test.cpp:3
 # CHECK-NEXT: >>>               {{.*}}.o:(baz())
 
 # The code below is the reduced version of the output
 # from the following invocation and source:
 #
 # // test.cpp:
 # int foo();

lld/test/ELF/undef-multi.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/undef.s -o %t2.o
+# RUN: not ld.lld %t.o %t2.o -o %t.exe 2>&1 | FileCheck %s
+
+# CHECK: error: undefined symbol: zed2
+# CHECK-NEXT: >>> referenced by undef-multi.s
+# CHECK-NEXT: >>>               {{.*}}:(.text+0x1)
+# CHECK-NEXT: >>> referenced by undef-multi.s
+# CHECK-NEXT: >>>               {{.*}}:(.text+0x6)
+# CHECK-NEXT: >>> referenced by undef-multi.s
+# CHECK-NEXT: >>>               {{.*}}:(.text+0xB)
+# CHECK-NEXT: >>> referenced by undef-multi.s
+# CHECK-NEXT: >>>               {{.*}}:(.text+0x10)
+# CHECK-NEXT: >>> referenced by {{.*}}tmp2.o:(.text+0x0)
+
+# All references to a single undefined symbol count as a single error.
+# RUN: not ld.lld %t.o %t2.o -o %t.exe -error-limit=2 2>&1 | \
+# RUN:     FileCheck --check-prefix=LIMIT %s
+
+# LIMIT: error: undefined symbol: zed2
+# LIMIT-NEXT: >>> referenced by undef-multi.s
+# LIMIT-NEXT: >>>               {{.*}}:(.text+0x1)
+# LIMIT-NEXT: >>> referenced by undef-multi.s
+# LIMIT-NEXT: >>>               {{.*}}:(.text+0x6)
+# LIMIT-NEXT: >>> referenced by undef-multi.s
+# LIMIT-NEXT: >>>               {{.*}}:(.text+0xB)
+# LIMIT-NEXT: >>> referenced by undef-multi.s
+# LIMIT-NEXT: >>>               {{.*}}:(.text+0x10)
+# LIMIT-NEXT: >>> referenced by {{.*}}tmp2.o:(.text+0x0)
+
+.file "undef-multi.s"
+
+  .globl _start
+_start:
+  call zed2
+
+  .globl _f
+_f:
+  call zed2
+
+  .globl _g
+_g:
+  call zed2
+
+  .globl _h
+_h:
+  call zed2