[lld] b2f00f2 - [mac/lld] Include archive name in diagnostics

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 1 20:03:34 PST 2020


Author: Nico Weber
Date: 2020-12-01T23:00:25-05:00
New Revision: b2f00f24a3c4983a977ff3d975962e7f9dfb887e

URL: https://github.com/llvm/llvm-project/commit/b2f00f24a3c4983a977ff3d975962e7f9dfb887e
DIFF: https://github.com/llvm/llvm-project/commit/b2f00f24a3c4983a977ff3d975962e7f9dfb887e.diff

LOG: [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

Added: 
    

Modified: 
    lld/ELF/InputFiles.h
    lld/MachO/Driver.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/InputFiles.h
    lld/MachO/LTO.cpp
    lld/MachO/Writer.cpp
    lld/test/MachO/invalid/undefined-symbol.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/InputFiles.h b/lld/ELF/InputFiles.h
index b1c83ddf384f..e8fc68900db1 100644
--- a/lld/ELF/InputFiles.h
+++ b/lld/ELF/InputFiles.h
@@ -95,9 +95,8 @@ class InputFile {
   // 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 a02be0c9a67c..c3b8224750ab 100644
--- 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 cfb30cfc35f5..3a87a42ded08 100644
--- 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 6ca9a40a49b0..59638ac197cf 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -62,12 +62,17 @@ class InputFile {
   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 @@ class InputFile {
 // .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 3050e03f2f24..1932cf46661d 100644
--- 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 39de749e2735..254e2787e893 100644
--- 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 2a981bb91da4..a1cc07f37553 100644
--- 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:


        


More information about the llvm-commits mailing list