[mac/lld] Include archive name in diagnostics

Also, for .o files, include full path as given on link command line.

Before:
    lld: error: undefined symbol [...], referenced from sandbox_logging.o

After:
    lld: error: undefined symbol [...], referenced from libseatbelt.a(sandbox_logging.o)

Move archiveName up to InputFile so we can consistently use toString()
to print InputFiles in diags, and pass it to the ObjFile ctor. This
matches the ELF and COFF ports.

Differential Revision: https://reviews.llvm.org/D92437

diff --git a/lld/ELF/InputFiles.h b/lld/ELF/InputFiles.h
index b1c83dd..e8fc689 100644 (file)
--- a/lld/ELF/InputFiles.h
+++ b/lld/ELF/InputFiles.h
@@ -95,9 +95,8 @@ public:
   // Get filename to use for linker script processing.
   StringRef getNameForScript() const;
 
-  // Filename of .a which contained this file. If this file was
-  // not in an archive file, it is the empty string. We use this
-  // string for creating error messages.
+  // If not empty, this stores the name of the archive containing this file.
+  // We use this string for creating error messages.
   std::string archiveName;
 
   // If this is an architecture-specific file, the following members

diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index a02be0c..c3b8224 100644 (file)
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -266,9 +266,8 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive) {
     if (config->allLoad || forceLoadArchive) {
       if (Optional<MemoryBufferRef> buffer = readFile(path)) {
         for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
-          auto file = make<ObjFile>(member.mbref, member.modTime);
-          file->archiveName = buffer->getBufferIdentifier();
-          inputFiles.push_back(file);
+          inputFiles.push_back(
+              make<ObjFile>(member.mbref, member.modTime, path));
         }
       }
     } else if (config->forceLoadObjC) {
@@ -280,17 +279,21 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive) {
       // we already found that it contains an ObjC symbol. We should also
       // consider creating a LazyObjFile class in order to avoid double-loading
       // these files here and below (as part of the ArchiveFile).
-      if (Optional<MemoryBufferRef> buffer = readFile(path))
-        for (const ArchiveMember &member : getArchiveMembers(*buffer))
-          if (hasObjCSection(member.mbref))
-            inputFiles.push_back(make<ObjFile>(member.mbref, member.modTime));
+      if (Optional<MemoryBufferRef> buffer = readFile(path)) {
+        for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
+          if (hasObjCSection(member.mbref)) {
+            inputFiles.push_back(
+                make<ObjFile>(member.mbref, member.modTime, path));
+          }
+        }
+      }
     }
 
     newFile = make<ArchiveFile>(std::move(file));
     break;
   }
   case file_magic::macho_object:
-    newFile = make<ObjFile>(mbref, getModTime(path));
+    newFile = make<ObjFile>(mbref, getModTime(path), "");
     break;
   case file_magic::macho_dynamically_linked_shared_lib:
   case file_magic::macho_dynamically_linked_shared_lib_stub:
@@ -737,7 +740,7 @@ bool macho::link(llvm::ArrayRef<const char *> argsArr, bool canExitEarly,
     parseOrderFile(orderFile);
 
   if (config->outputType == MH_EXECUTE && isa<Undefined>(config->entry)) {
-    error("undefined symbol: " + config->entry->getName());
+    error("undefined symbol: " + toString(*config->entry));
     return false;
   }
 

diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index cfb30cf..3a87a42 100644 (file)
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -74,6 +74,17 @@ using namespace llvm::sys;
 using namespace lld;
 using namespace lld::macho;
 
+// Returns "<internal>", "foo.a(bar.o)", or "baz.o".
+std::string lld::toString(const InputFile *f) {
+  if (!f)
+    return "<internal>";
+  if (f->archiveName.empty())
+    return std::string(f->getName());
+  return (path::filename(f->archiveName) + "(" + path::filename(f->getName()) +
+          ")")
+      .str();
+}
+
 std::vector<InputFile *> macho::inputFiles;
 std::unique_ptr<TarWriter> macho::tar;
 int InputFile::idCount = 0;
@@ -365,8 +376,10 @@ OpaqueFile::OpaqueFile(MemoryBufferRef mb, StringRef segName,
   subsections.push_back({{0, isec}});
 }
 
-ObjFile::ObjFile(MemoryBufferRef mb, uint32_t modTime)
+ObjFile::ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName)
     : InputFile(ObjKind, mb), modTime(modTime) {
+  this->archiveName = std::string(archiveName);
+
   auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
   auto *hdr = reinterpret_cast<const mach_header_64 *>(mb.getBufferStart());
 
@@ -402,9 +415,11 @@ void ObjFile::parseDebugInfo() {
 
   auto *ctx = make<DWARFContext>(
       std::move(dObj), "",
-      [&](Error err) { warn(getName() + ": " + toString(std::move(err))); },
+      [&](Error err) {
+        warn(toString(this) + ": " + toString(std::move(err)));
+      },
       [&](Error warning) {
-        warn(getName() + ": " + toString(std::move(warning)));
+        warn(toString(this) + ": " + toString(std::move(warning)));
       });
 
   // TODO: Since object files can contain a lot of DWARF info, we should verify
@@ -482,7 +497,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella)
     auto *c = reinterpret_cast<const dylib_command *>(cmd);
     dylibName = reinterpret_cast<const char *>(cmd) + read32le(&c->dylib.name);
   } else {
-    error("dylib " + getName() + " missing LC_ID_DYLIB load command");
+    error("dylib " + toString(this) + " missing LC_ID_DYLIB load command");
     return;
   }
 
@@ -500,7 +515,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella)
                                                    isWeakDef, isTlv));
               });
   } else {
-    error("LC_DYLD_INFO_ONLY not found in " + getName());
+    error("LC_DYLD_INFO_ONLY not found in " + toString(this));
     return;
   }
 
@@ -601,8 +616,7 @@ void ArchiveFile::fetch(const object::Archive::Symbol &sym) {
                                      "for the member defining symbol " +
                                      toMachOString(sym)));
 
-  auto file = make<ObjFile>(mb, modTime);
-  file->archiveName = getName();
+  auto file = make<ObjFile>(mb, modTime, getName());
 
   symbols.insert(symbols.end(), file->symbols.begin(), file->symbols.end());
   subsections.insert(subsections.end(), file->subsections.begin(),
@@ -613,8 +627,3 @@ BitcodeFile::BitcodeFile(MemoryBufferRef mbref)
     : InputFile(BitcodeKind, mbref) {
   obj = check(lto::InputFile::create(mbref));
 }
-
-// Returns "<internal>" or "baz.o".
-std::string lld::toString(const InputFile *file) {
-  return file ? std::string(file->getName()) : "<internal>";
-}

diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 6ca9a40..59638ac 100644 (file)
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -62,12 +62,17 @@ public:
   StringRef getName() const { return name; }
 
   MemoryBufferRef mb;
+
   std::vector<Symbol *> symbols;
   ArrayRef<llvm::MachO::section_64> sectionHeaders;
   std::vector<SubsectionMap> subsections;
   // Provides an easy way to sort InputFiles deterministically.
   const int id;
 
+  // If not empty, this stores the name of the archive containing this file.
+  // We use this string for creating error messages.
+  std::string archiveName;
+
 protected:
   InputFile(Kind kind, MemoryBufferRef mb)
       : mb(mb), id(idCount++), fileKind(kind), name(mb.getBufferIdentifier()) {}
@@ -94,11 +99,10 @@ private:
 // .o file
 class ObjFile : public InputFile {
 public:
-  explicit ObjFile(MemoryBufferRef mb, uint32_t modTime);
+  explicit ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName);
   static bool classof(const InputFile *f) { return f->kind() == ObjKind; }
 
   llvm::DWARFUnit *compileUnit = nullptr;
-  StringRef archiveName = "";
   const uint32_t modTime;
 
 private:

diff --git a/lld/MachO/LTO.cpp b/lld/MachO/LTO.cpp
index 3050e03..1932cf4 100644 (file)
--- a/lld/MachO/LTO.cpp
+++ b/lld/MachO/LTO.cpp
@@ -78,7 +78,7 @@ std::vector<ObjFile *> BitcodeCompiler::compile() {
   for (unsigned i = 0; i != maxTasks; ++i)
     if (!buf[i].empty())
       ret.push_back(
-          make<ObjFile>(MemoryBufferRef(buf[i], "lto.tmp"), /*modTime=*/0));
+          make<ObjFile>(MemoryBufferRef(buf[i], "lto.tmp"), /*modTime=*/0, ""));
 
   return ret;
 }

diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 39de749..254e278 100644 (file)
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -390,7 +390,7 @@ void Writer::scanRelocations() {
       if (auto *s = r.referent.dyn_cast<lld::macho::Symbol *>()) {
         if (isa<Undefined>(s))
           error("undefined symbol " + toString(*s) + ", referenced from " +
-                sys::path::filename(isec->file->getName()));
+                toString(isec->file));
         else
           target->prepareSymbolRelocation(s, isec, r);
       } else {
@@ -617,7 +617,7 @@ void Writer::createOutputSections() {
       if (ssec->isNeeded())
         getOrCreateOutputSegment(ssec->segname)->addOutputSection(ssec);
     } else {
-      error("section from " + it->second->firstSection()->file->getName() +
+      error("section from " + toString(it->second->firstSection()->file) +
             " conflicts with synthetic section " + ssec->segname + "," +
             ssec->name);
     }

diff --git a/lld/test/MachO/invalid/undefined-symbol.s b/lld/test/MachO/invalid/undefined-symbol.s
index 2a981bb..a1cc07f 100644 (file)
--- a/lld/test/MachO/invalid/undefined-symbol.s
+++ b/lld/test/MachO/invalid/undefined-symbol.s
@@ -1,8 +1,25 @@
 # REQUIRES: x86
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
-# RUN: not %lld -o %t %t.o 2>&1 | FileCheck %s -DBASENAME=%basename_t
-# CHECK: error: undefined symbol _foo, referenced from [[BASENAME]]
+# RUN: rm -rf %t
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/main.s -o %t/main.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
+# RUN: llvm-ar crs %t/foo.a %t/foo.o
+# RUN: not %lld -o /dev/null %t/main.o 2>&1 | \
+# RUN:     FileCheck %s -DSYM=_foo -DFILENAME=%t/main.o
+# RUN: not %lld -o /dev/null %t/main.o %t/foo.a 2>&1 | \
+# RUN:     FileCheck %s -DSYM=_bar -DFILENAME='foo.a(foo.o)'
+# RUN: not %lld -o /dev/null %t/main.o -force_load %t/foo.a 2>&1 | \
+# RUN:     FileCheck %s -DSYM=_bar -DFILENAME='foo.a(foo.o)'
+# CHECK: error: undefined symbol [[SYM]], referenced from [[FILENAME]]
 
+#--- foo.s
+.globl _foo
+.text
+_foo:
+  callq _bar
+  retq
+
+#--- main.s
 .globl _main
 .text
 _main: