[ELF] - Print absolute file name in errors when possible.

[grimar]

Summary

Currently LLD prints basename of source file name in error messages,
for example:
$ mkdir foo
$ echo 'void _start(void) { foobar(); }' > foo/bar.c
$ gcc -g -c foo/bar.c
$ bin/ld.lld -o out bar.o
bin/ld.lld: error: bar.c:1: undefined symbol 'foobar'
$
This should say:
bin/ld.lld: error: foo/bar.c:1: undefined symbol 'foobar'

This is PR31299

[grimar]

• Convert path separators to *nix style.

[ruiu]

LGTM

ELF/InputFiles.cpp

95 ↗

(On Diff #80551)

Add a blank line before #ifdef.

test/ELF/Inputs/undef-debug.s

1 ↗

(On Diff #80551)

nit: "folder" as a term is probably windows-ish too? probably "dir" or something.

[grimar]

ELF/InputFiles.cpp

95 ↗

(On Diff #80551)

I am going to export convertToUnixPathSeparator from \llvm\tools\lld\lib\Core\Reproduce.cpp as was requested by Rafael and use it here instead.

[ruiu]

ELF/InputFiles.cpp

95 ↗

(On Diff #80551)

OK, but if it involves packing into SmallString and unpack it to std::string, it would be uglier than this, so please upload a patch after you make a change. Thanks.

[grimar]

• Exported and reused convertToUnixPathSeparator().

[grimar]

ELF/InputFiles.cpp

95 ↗

(On Diff #80551)

It did not involve packing/unpacking..

[ruiu]

LGTM

ELF/InputFiles.cpp

95 ↗

(On Diff #80551)

Use Ret.data() instead of &Ret[0] because the latter is invalid if Ret is empty. (That shouldn't happen in this case, but always avoiding &Ret[0] is a good idea.)

[grimar]

ELF/InputFiles.cpp

95 ↗

(On Diff #80551)

That is true, but data() returns const char*, that why I did not use it as it requires cast then.
It is not very clean to remove const, though in c++ 11 it is safe to modify pointer returned by data() afaik.

Do you really prefer:

convertToUnixPathSeparator({(char*)Ret.data(), Ret.size()});

?

[ruiu]

I prefer a more functional approach. We don't need to modify a string in place. We can create a new string and return it.

[ruiu]

By the way, the commit message was not accurate. It does not print out an absolute path. An absolute path is a path starts with '/' or a drive letter. This patch is to include path components in error messages instead of just base names.

[grimar]

In D27506#616268, @ruiu wrote:

By the way, the commit message was not accurate. It does not print out an absolute path. An absolute path is a path starts with '/' or a drive letter. This patch is to include path components in error messages instead of just base names.

Technically you probably correct. My naming was based on "DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath" I am using.

lld/trunk/ELF/InputFiles.cpp

std::string elf::ObjectFile<ELFT>::getLineInfo(InputSectionBase<ELFT> *S,
   // The offset to CU is 0.
   const DWARFDebugLine::LineTable *Tbl = DwarfLine->getLineTable(0);
   if (!Tbl)
     return "";
 
   // Use fake address calcuated by adding section file offset and offset in
   // section. See comments for ObjectInfo class.
   DILineInfo Info;
-  DILineInfoSpecifier Spec;
-  Tbl->getFileLineInfoForAddress(S->Offset + Offset, nullptr, Spec.FLIKind,
-                                 Info);
+  Tbl->getFileLineInfoForAddress(
+      S->Offset + Offset, nullptr,
+      DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, Info);
   if (Info.Line == 0)
     return "";
-  return Info.FileName + ":" + std::to_string(Info.Line);
+  std::string Ret = Info.FileName + ":" + std::to_string(Info.Line);
+  convertToUnixPathSeparator({(char*)Ret.data(), Ret.size()});
+  return Ret;
 }
 
 // Returns "(internal)", "foo.a(bar.o)" or "baz.o".
 std::string elf::toString(const InputFile *F) {
   if (!F)
     return "(internal)";
   if (!F->ArchiveName.empty())
     return (F->ArchiveName + "(" + F->getName() + ")").str();

lld/trunk/include/lld/Core/Reproduce.h

 //===- Reproduce.h - Utilities for creating reproducers ---------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_CORE_REPRODUCE_H
 #define LLD_CORE_REPRODUCE_H
 
 #include "lld/Core/LLVM.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 
 class raw_fd_ostream;
 

 
 // Rewrite the given path if a file exists with that pathname, otherwise
 // returns the original path.
 std::string rewritePath(StringRef S);
 
 // Returns the string form of the given argument.
 std::string stringize(llvm::opt::Arg *Arg);
 
+// Converts path to use unix path separators.
+void convertToUnixPathSeparator(llvm::MutableArrayRef<char> Path);
+
 }
 
 #endif

lld/trunk/lib/Core/Reproduce.cpp

static void writeMember(raw_fd_ostream &OS, StringRef Path, StringRef Data) {
   OS << "000000";                        // c_rdev
   OS << "00000000000";                   // c_mtime
   OS << format("%06o", Path.size() + 1); // c_namesize
   OS << format("%011o", Data.size());    // c_filesize
   OS << Path << '\0';                    // c_name
   OS << Data;                            // c_filedata
 }
 
-// Converts path to use unix path separators so the cpio can be extracted on
-// both unix and windows.
-static void convertToUnixPathSeparator(SmallString<128> &Path) {
-#ifdef LLVM_ON_WIN32
-  std::replace(Path.begin(), Path.end(), '\\', '/');
-#endif
-}
-
 void CpioFile::append(StringRef Path, StringRef Data) {
   if (!Seen.insert(Path).second)
     return;
 
   // Construct an in-archive filename so that /home/foo/bar is stored
   // as baz/home/foo/bar where baz is the basename of the output file.
   // (i.e. in that case we are creating baz.cpio.)
   SmallString<128> Fullpath;
std::string lld::stringize(opt::Arg *Arg) {
   std::string K = Arg->getSpelling();
   if (Arg->getNumValues() == 0)
     return K;
   std::string V = quote(Arg->getValue());
   if (Arg->getOption().getRenderStyle() == opt::Option::RenderJoinedStyle)
     return K + V;
   return K + " " + V;
 }
+
+void lld::convertToUnixPathSeparator(MutableArrayRef<char> Path) {
+#ifdef LLVM_ON_WIN32
+  std::replace(Path.begin(), Path.end(), '\\', '/');
+#endif
+}

lld/trunk/test/ELF/Inputs/undef-debug.s

-.file 1 "undef-debug.s"
+.file 1 "dir/undef-debug.s"
 .loc 1 3
         .quad zed3
 
 .section .text.1,"ax"
 .loc 1 7
         .quad zed4
 
 .section .text.2,"ax"
 .loc 1 11
         .quad zed5

lld/trunk/test/ELF/undef.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: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/undef-debug.s -o %t3.o
 # RUN: llvm-ar rc %t2.a %t2.o
 # RUN: not ld.lld %t.o %t2.a %t3.o -o %t.exe 2>&1 | FileCheck %s
 # RUN: not ld.lld -pie %t.o %t2.a %t3.o -o %t.exe 2>&1 | FileCheck %s
 # CHECK: error: undef.s:(.text+0x1): undefined symbol 'foo'
 # CHECK: error: undef.s:(.text+0x6): undefined symbol 'bar'
 # CHECK: error: undef.s:(.text+0x10): undefined symbol 'foo(int)'
 # CHECK: error: {{.*}}2.a({{.*}}.o):(.text+0x0): undefined symbol 'zed2'
-# CHECK: error: undef-debug.s:3: undefined symbol 'zed3'
-# CHECK: error: undef-debug.s:7: undefined symbol 'zed4'
-# CHECK: error: undef-debug.s:11: undefined symbol 'zed5'
+# CHECK: error: dir/undef-debug.s:3: undefined symbol 'zed3'
+# CHECK: error: dir/undef-debug.s:7: undefined symbol 'zed4'
+# CHECK: error: dir/undef-debug.s:11: undefined symbol 'zed5'
 
 # RUN: not ld.lld %t.o %t2.a -o %t.exe -no-demangle 2>&1 | \
 # RUN:   FileCheck -check-prefix=NO-DEMANGLE %s
 # NO-DEMANGLE: error: undef.s:(.text+0x10): undefined symbol '_Z3fooi'
 
 .file "undef.s"
 
   .globl _start
 _start:
   call foo
   call bar
   call zed1
   call _Z3fooi