[LLD] Do not print additional newlines after reaching error limit

[arichardson]

Summary

This could previously happen if errors that are emitted after reaching the
error limit. In that case, the flag inside the newline() function will be
set to true which causes the next call to print a newline even though the
actual message will be discarded.

[grimar]

Can you add a test case?

[MaskRay]

lld/Common/ErrorHandler.cpp

166

The comment may be too verbose... It should be obvious from the

if .. else if ...

pattern (there is no else branch).

The test may look like:

env LLD_IN_TEST=0 ld.lld --error-limit=1 %t.o -o /dev/null 2>&1 | FileCheck %s

(You need to find a multi-line error that can be easily replicated.)

[arichardson]

lld/Common/ErrorHandler.cpp

166

I agree this is quite long. I added the comment here to avoid future refactoring from introducing errors. With the test it is probably fine without a comment.
I'll see if I can find a message.

[arichardson]

add test

[ruiu]

LGTM

lld/Common/ErrorHandler.cpp

166

Comments work best when explaining why code works some way. In this case, the correct behavior is somewhat self-explanatory, and a unit test works best.

lld/Common/ErrorHandler.cpp

void ErrorHandler::warn(const Twine &msg) {
   std::lock_guard<std::mutex> lock(mu);
   newline(errorOS, msg);
   printHeader("warning: ", raw_ostream::MAGENTA, msg);
   *errorOS << msg << "\n";
 }
 
 void ErrorHandler::error(const Twine &msg) {
   std::lock_guard<std::mutex> lock(mu);
-  newline(errorOS, msg);
 
   if (errorLimit == 0 || errorCount < errorLimit) {
+    newline(errorOS, msg);
     printHeader("error: ", raw_ostream::RED, msg);
     *errorOS << msg << "\n";
   } else if (errorCount == errorLimit) {
+    newline(errorOS, msg);
     printHeader("error: ", raw_ostream::RED, msg);
     *errorOS << errorLimitExceededMsg << "\n";
     if (exitEarly)
       exitLld(1);
   }
 
   ++errorCount;
 }
 
 void ErrorHandler::fatal(const Twine &msg) {
   error(msg);
   exitLld(1);
 }

lld/test/ELF/error-limit-no-newlines.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t1.o
+# RUN: not ld.lld --error-limit=1 %t1.o %t1.o %t1.o -o /dev/null 2>%t.output
+# RUN: echo "END" >> %t.output
+# RUN: FileCheck %s -input-file=%t.output
+
+# CHECK:      error: duplicate symbol: _start
+# CHECK-NEXT: >>> defined at {{.*}}1.o:(.text+0x0)
+# CHECK-NEXT: >>> defined at {{.*}}1.o:(.text+0x0)
+# CHECK-EMPTY:
+# CHECK-NEXT: ld.lld: error: too many errors emitted, stopping now (use -error-limit=0 to see all errors)
+## Ensure that there isn't an additional newline before the next message:
+# CHECK-NEXT: END
+.globl _start
+_start:
+  nop
+
+.globl foo
+foo:
+  nop
+
+.globl bar
+bar:
+  nop