Omit path to lld binary from lld's error and warning diagnostics.

[thakis]

Summary

lld currently prepends the absolute path to itself to every diagnostic it emits. This path can be longer than the diagnostic, and makes the actual error message hard to read.

There isn't a good reason for printing this path: if you want to know which lld you're running, pass -v to clang – chances are that if you're unsure of this, you're not only unsure when it errors out. Some people want an indication that the diagnostic is from the linker though, so instead print just the basename of the linker's path.

Before:

$ out/bin/clang -target x86_64-unknown-linux -x c++ /dev/null -fuse-ld=lld 
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: cannot open crt1.o: No such file or directory
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: cannot open crti.o: No such file or directory
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: cannot open crtbegin.o: No such file or directory
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: unable to find library -lgcc
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: unable to find library -lgcc_s
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: unable to find library -lc
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: unable to find library -lgcc
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: unable to find library -lgcc_s
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: cannot open crtend.o: No such file or directory
/Users/thakis/src/llvm-mono/out/bin/ld.lld: error: cannot open crtn.o: No such file or directory
clang: error: linker command failed with exit code 1 (use -v to see invocation)

After:

$ out/bin/clang -target x86_64-unknown-linux -x c++ /dev/null -fuse-ld=lld 
ld.lld: error: cannot open crt1.o: No such file or directory
ld.lld: error: cannot open crti.o: No such file or directory
ld.lld: error: cannot open crtbegin.o: No such file or directory
ld.lld: error: unable to find library -lgcc
ld.lld: error: unable to find library -lgcc_s
ld.lld: error: unable to find library -lc
ld.lld: error: unable to find library -lgcc
ld.lld: error: unable to find library -lgcc_s
ld.lld: error: cannot open crtend.o: No such file or directory
ld.lld: error: cannot open crtn.o: No such file or directory
clang: error: linker command failed with exit code 1 (use -v to see invocation)

(The path is still printed for log messages.)

[ruiu]

I think we should at least include "ld.lld: " part in the error message, because without that, it is sometimes hard to identify what command is emitting an error in a long build log.

[smeenai]

In D49189#1159019, @ruiu wrote:

I think we should at least include "ld.lld: " part in the error message, because without that, it is sometimes hard to identify what command is emitting an error in a long build log.

+1 – it's useful to be able to immediately identify linker errors.

Do you plan to do something similar for lld-link as well?

[ruiu]

How about changing the Clang driver so that it invokes a linker using a relative path instead of an absolute one when appropriate?

[thakis]

Thanks for taking a look!

I think we should at least include "ld.lld: " part in the error message, because without that, it is sometimes hard to identify what command is emitting an error in a long build log.

In what cases? clang doesn't prepend its errors with "clang:" (except for driver-level diags) either. For failed build steps, most build tools output the failing build step above the error so it's clear from context where the error comes from.

I see how this is is useful in the abstract, but I can't think of concrete examples (it's well possible I'm just lacking imagination), so I thought not printing anything is the best approach: If there's some concrete case where it helps, we'll learn about it and can add something back, and if there isn't then we end up with easier to read output. If we include some prefix and it's not needed, we'll always be stuck with it.

In D49189#1159020, @smeenai wrote:

In D49189#1159019, @ruiu wrote:

I think we should at least include "ld.lld: " part in the error message, because without that, it is sometimes hard to identify what command is emitting an error in a long build log.

+1 – it's useful to be able to immediately identify linker errors.

Do you plan to do something similar for lld-link as well?

This patch here modifies lld-link's behavior as well.

[thakis]

In D49189#1159027, @ruiu wrote:

How about changing the Clang driver so that it invokes a linker using a relative path instead of an absolute one when appropriate?

On Windows, build systems usually call the linker directly, instead of through the driver. That path can still be quite long. (Also, see above.)

[ruiu]

In D49189#1159054, @thakis wrote:

Thanks for taking a look!

I think we should at least include "ld.lld: " part in the error message, because without that, it is sometimes hard to identify what command is emitting an error in a long build log.

In what cases? clang doesn't prepend its errors with "clang:" (except for driver-level diags) either. For failed build steps, most build tools output the failing build step above the error so it's clear from context where the error comes from.

I see how this is is useful in the abstract, but I can't think of concrete examples (it's well possible I'm just lacking imagination), so I thought not printing anything is the best approach: If there's some concrete case where it helps, we'll learn about it and can add something back, and if there isn't then we end up with easier to read output. If we include some prefix and it's not needed, we'll always be stuck with it.

If you invoke the linker directly, you can just search for the linker name in a log and find an error, but if you do that through the compiler driver, is it easy to determine whether it's a link error or not, I guess. I don't a good example in hand, but the first thing I usually do to search for a link error is to search for "lld" after Ctrl-F. It's handy.

As to not printing out anything, I believe clang's behavior is somewhat unusual, as most command usually includes argv[0] in their error messages. It feels like we probably should follow the usual convention than clang.

Maybe printing out the basename of argv[0] could get the right balance. What do you think?

[smeenai]

In D49189#1159171, @ruiu wrote:

Maybe printing out the basename of argv[0] could get the right balance. What do you think?

I'm in favor of this. It's what gcc, ld.bfd, and ld.gold appear to do as well, so there's some precedent.

[smeenai]

Actually, I lied; bfd and gold appear to print their full path. gcc does print the basename though, and that seems like a good compromise.

[thakis]

print basename

[thakis]

Ok, basename is already a big improvement, so let's start there :-)

While finding the filename() function, I noticed that COFF/InputFiles.cpp could use it for slightly simpler code as well. If this change looks fine to you, I'll land it as 2 commits (one for the log message change, and one for the InputFiles.cpp simplification).

[ruiu]

LGTM

[thakis]

r337633 and r337634, thanks!

lld/COFF/Driver.cpp

 namespace coff {
 
 static Timer InputFileTimer("Input File Reading", Timer::root());
 
 Configuration *Config;
 LinkerDriver *Driver;
 
 bool link(ArrayRef<const char *> Args, bool CanExitEarly, raw_ostream &Diag) {
-  errorHandler().LogName = Args[0];
+  errorHandler().LogName = sys::path::filename(Args[0]);
   errorHandler().ErrorOS = &Diag;
   errorHandler().ColorDiagnostics = Diag.has_colors();
   errorHandler().ErrorLimitExceededMsg =
       "too many errors emitted, stopping now"
       " (use /errorlimit:0 to see all errors)";
   errorHandler().ExitEarly = CanExitEarly;
   Config = make<Configuration>();
   Config->Argv = {Args.begin(), Args.end()};

lld/COFF/InputFiles.cpp

 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Target/TargetOptions.h"
 #include <cstring>
 #include <system_error>
 #include <utility>
 
 using namespace llvm;
 using namespace llvm::COFF;
 using namespace llvm::object;
default:
     return IMAGE_FILE_MACHINE_UNKNOWN;
   }
 }
 } // namespace coff
 } // namespace lld
 
 // Returns the last element of a path, which is supposed to be a filename.
 static StringRef getBasename(StringRef Path) {
-  size_t Pos = Path.find_last_of("\\/");
-  if (Pos == StringRef::npos)
-    return Path;
-  return Path.substr(Pos + 1);
+  return sys::path::filename(Path, sys::path::Style::windows);
 }
 
 // Returns a string in the format of "foo.obj" or "foo.obj(bar.lib)".
 std::string lld::toString(const coff::InputFile *File) {
   if (!File)
     return "<internal>";
   if (File->ParentName.empty())
     return File->getName();
 
   return (getBasename(File->ParentName) + "(" + getBasename(File->getName()) +
           ")")
       .str();
 }

lld/ELF/Driver.cpp

 
 Configuration *elf::Config;
 LinkerDriver *elf::Driver;
 
 static void setConfigs(opt::InputArgList &Args);
 
 bool elf::link(ArrayRef<const char *> Args, bool CanExitEarly,
                raw_ostream &Error) {
-  errorHandler().LogName = Args[0];
+  errorHandler().LogName = sys::path::filename(Args[0]);
   errorHandler().ErrorLimitExceededMsg =
       "too many errors emitted, stopping now (use "
       "-error-limit=0 to see all errors)";
   errorHandler().ErrorOS = &Error;
   errorHandler().ExitEarly = CanExitEarly;
   errorHandler().ColorDiagnostics = Error.has_colors();
 
   InputSections.clear();

lld/lib/Driver/DarwinLdDriver.cpp

for (auto i = Files.rbegin(), e = Files.rend(); i != e; ++i) {
     auto &members = ctx.getNodes();
     members.insert(members.begin(), llvm::make_unique<FileNode>(std::move(*i)));
   }
 }
 
 /// This is where the link is actually performed.
 bool link(llvm::ArrayRef<const char *> args, bool CanExitEarly,
           raw_ostream &Error) {
-  errorHandler().LogName = args[0];
+  errorHandler().LogName = llvm::sys::path::filename(args[0]);
   errorHandler().ErrorLimitExceededMsg =
       "too many errors emitted, stopping now (use "
       "'-error-limit 0' to see all errors)";
   errorHandler().ErrorOS = &Error;
   errorHandler().ExitEarly = CanExitEarly;
   errorHandler().ColorDiagnostics = Error.has_colors();
 
   MachOLinkingContext ctx;

lld/wasm/Driver.cpp

private:
   void addFile(StringRef Path);
   void addLibrary(StringRef Name);
   std::vector<InputFile *> Files;
 };
 } // anonymous namespace
 
 bool lld::wasm::link(ArrayRef<const char *> Args, bool CanExitEarly,
                      raw_ostream &Error) {
-  errorHandler().LogName = Args[0];
+  errorHandler().LogName = sys::path::filename(Args[0]);
   errorHandler().ErrorOS = &Error;
   errorHandler().ColorDiagnostics = Error.has_colors();
   errorHandler().ErrorLimitExceededMsg =
       "too many errors emitted, stopping now (use "
       "-error-limit=0 to see all errors)";
 
   Config = make<Configuration>();
   Symtab = make<SymbolTable>();