ld.lld: Demangle symbols from archives in diagnostics

[thakis]

Summary

This ports r366573 from COFF to ELF.

There are now to toString(Archive::Symbol), one doing MSVC demangling
in COFF and one doing Itanium demangling in ELF, so rename these two
to toCOFFString() and to toELFString() to not get a duplicate symbol.

Nothing ever passes a raw Archive::Symbol to CHECK(), so these not
being part of the normal toString() machinery seems ok.

There are two code paths in the ELF linker that emits this type of
diagnostic:

1. The "normal" one in InputFiles.cpp. This is covered by the tweaked test.

2. An additional one that's only used for libcalls if there's at least one bitcode in the link, and if the libcall symbol is lazy, and lazily loaded from an archive (i.e. not from a lazy .o file). (This code path was added in r339301.) Since all libcall names so far are C symbols and never mangled, the change there is not observable and hence not covered by tests.

[thakis]

lld/ELF/Symbols.h

36 ↗

(On Diff #211110)

This overload is also declared in InputFiles.h, and since its implementation is in InputFiles.cpp I removed this redundant declaration.

[ruiu]

LGTM

lld/COFF/Symbols.h

26 ↗

(On Diff #211110)

Can you leave a comment saying that we want two different stringize functions for ELF and COFF, so the function is named toCOFFString?

[thakis]

Thanks!

lld/trunk/COFF/Driver.cpp

 }
 
 void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
                                         const Archive::Symbol &sym,
                                         StringRef parentName) {
 
   auto reportBufferError = [=](Error &&e, StringRef childName) {
     fatal("could not get the buffer for the member defining symbol " +
-          toString(sym) + ": " + parentName + "(" + childName + "): " +
+          toCOFFString(sym) + ": " + parentName + "(" + childName + "): " +
           toString(std::move(e)));
   };
 
   if (!c.getParent()->isThin()) {
     uint64_t offsetInArchive = c.getChildOffset();
     Expected<MemoryBufferRef> mbOrErr = c.getMemoryBufferRef();
     if (!mbOrErr)
       reportBufferError(mbOrErr.takeError(), check(c.getFullName()));
     MemoryBufferRef mb = mbOrErr.get();
     enqueueTask([=]() {
-      driver->addArchiveBuffer(mb, toString(sym), parentName, offsetInArchive);
+      driver->addArchiveBuffer(mb, toCOFFString(sym), parentName,
+                               offsetInArchive);
     });
     return;
   }
 
   std::string childName = CHECK(
       c.getFullName(),
       "could not get the filename for the member defining symbol " +
-      toString(sym));
+      toCOFFString(sym));
   auto future = std::make_shared<std::future<MBErrPair>>(
       createFutureForFile(childName));
   enqueueTask([=]() {
     auto mbOrErr = future->get();
     if (mbOrErr.second)
       reportBufferError(errorCodeToError(mbOrErr.second), childName);
     driver->addArchiveBuffer(takeBuffer(std::move(mbOrErr.first)),
-                             toString(sym), parentName, /*OffsetInArchive=*/0);
+                             toCOFFString(sym), parentName,
+                             /*OffsetInArchive=*/0);
   });
 }
 
 static bool isDecorated(StringRef sym) {
   return sym.startswith("@") || sym.contains("@@") || sym.startswith("?") ||
          (!config->mingw && sym.contains('@'));
 }
 

lld/trunk/COFF/InputFiles.cpp

void ArchiveFile::parse() {
 
   // Read the symbol table to construct Lazy objects.
   for (const Archive::Symbol &sym : file->symbols())
     symtab->addLazy(this, sym);
 }
 
 // Returns a buffer pointing to a member file containing a given symbol.
 void ArchiveFile::addMember(const Archive::Symbol &sym) {
-  const Archive::Child &c = CHECK(
-      sym.getMember(), "could not get the member for symbol " + toString(sym));
+  const Archive::Child &c =
+      CHECK(sym.getMember(),
+            "could not get the member for symbol " + toCOFFString(sym));
 
   // Return an empty buffer if we have already returned the same buffer.
   if (!seen.insert(c.getChildOffset()).second)
     return;
 
   driver->enqueueArchiveMember(c, sym, getName());
 }
 

lld/trunk/COFF/Symbols.h

 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/COFF.h"
 #include <atomic>
 #include <memory>
 #include <vector>
 
 namespace lld {
+
+std::string toString(coff::Symbol &b);
+
+// There are two different ways to convert an Archive::Symbol to a string:
+// One for Microsoft name mangling and one for Itanium name mangling.
+// Call the functions toCOFFString and toELFString, not just toString.
+std::string toCOFFString(const coff::Archive::Symbol &b);
+
 namespace coff {
 
 using llvm::object::Archive;
 using llvm::object::COFFSymbolRef;
 using llvm::object::coff_import_header;
 using llvm::object::coff_symbol_generic;
 
 class ArchiveFile;
void replaceSymbol(Symbol *s, ArgT &&... arg) {
   static_assert(alignof(T) <= alignof(SymbolUnion),
                 "SymbolUnion not aligned enough");
   assert(static_cast<Symbol *>(static_cast<T *>(nullptr)) == nullptr &&
          "Not a Symbol");
   new (s) T(std::forward<ArgT>(arg)...);
 }
 } // namespace coff
 
-std::string toString(coff::Symbol &b);
-std::string toString(const coff::Archive::Symbol &b);
 } // namespace lld
 
 #endif

lld/trunk/COFF/Symbols.cpp

 // Returns a symbol name for an error message.
 static std::string demangle(StringRef symName) {
   if (config->demangle)
     if (Optional<std::string> s = demangleMSVC(symName))
       return *s;
   return symName;
 }
 std::string toString(coff::Symbol &b) { return demangle(b.getName()); }
-std::string toString(const Archive::Symbol &b) { return demangle(b.getName()); }
+std::string toCOFFString(const Archive::Symbol &b) {
+  return demangle(b.getName());
+}
 
 namespace coff {
 
 StringRef Symbol::getName() {
   // COFF symbol names are read lazily for a performance reason.
   // Non-external symbol names are never used by the linker except for logging
   // or debugging. Their internal references are resolved not by name but by
   // symbol index. And because they are not external, no one can refer them by

lld/trunk/ELF/InputFiles.cpp

for (const Archive::Symbol &sym : file->symbols())
     symtab->addSymbol(LazyArchive{*this, sym});
 }
 
 // Returns a buffer pointing to a member file containing a given symbol.
 void ArchiveFile::fetch(const Archive::Symbol &sym) {
   Archive::Child c =
       CHECK(sym.getMember(), toString(this) +
                                  ": could not get the member for symbol " +
-                                 sym.getName());
+                                 toELFString(sym));
 
   if (!seen.insert(c.getChildOffset()).second)
     return;
 
   MemoryBufferRef mb =
       CHECK(c.getMemoryBufferRef(),
             toString(this) +
                 ": could not get the buffer for the member defining symbol " +
-                sym.getName());
+                toELFString(sym));
 
   if (tar && c.getParent()->isThin())
     tar->append(relativeToRoot(CHECK(c.getFullName(), this)), mb.getBuffer());
 
   InputFile *file = createObjectFile(
       mb, getName(), c.getParent()->isThin() ? 0 : c.getChildOffset());
   file->groupId = groupId;
   parseFile(file);

lld/trunk/ELF/Symbols.h

 class LazyArchive;
 class LazyObject;
 class SharedSymbol;
 class Symbol;
 class Undefined;
 } // namespace elf
 
 std::string toString(const elf::Symbol &);
-std::string toString(const elf::InputFile *);
+
+// There are two different ways to convert an Archive::Symbol to a string:
+// One for Microsoft name mangling and one for Itanium name mangling.
+// Call the functions toCOFFString and toELFString, not just toString.
+std::string toELFString(const elf::Archive::Symbol &);
 
 namespace elf {
 
 // This is a StringRef-like container that doesn't run strlen().
 //
 // ELF string tables contain a lot of null-terminated strings. Most of them
 // are not necessary for the linker because they are names of local symbols,
 // and the linker doesn't use local symbol names for name resolution. So, we

lld/trunk/ELF/Symbols.cpp

 Defined *ElfSym::mipsGp;
 Defined *ElfSym::mipsGpDisp;
 Defined *ElfSym::mipsLocalGp;
 Defined *ElfSym::relaIpltStart;
 Defined *ElfSym::relaIpltEnd;
 Defined *ElfSym::riscvGlobalPointer;
 Defined *ElfSym::tlsModuleBase;
 
+// Returns a symbol for an error message.
+static std::string demangle(StringRef symName) {
+  if (config->demangle)
+    if (Optional<std::string> s = demangleItanium(symName))
+      return *s;
+  return symName;
+}
+namespace lld {
+std::string toString(const Symbol &b) { return demangle(b.getName()); }
+std::string toELFString(const Archive::Symbol &b) {
+  return demangle(b.getName());
+}
+} // namespace lld
+
 static uint64_t getSymVA(const Symbol &sym, int64_t &addend) {
   switch (sym.kind()) {
   case Symbol::DefinedKind: {
     auto &d = cast<Defined>(sym);
     SectionBase *isec = d.section;
 
     // This is an absolute symbol.
     if (!isec)
if (auto *sym = dyn_cast<LazyObject>(this)) {
     dyn_cast<LazyObjFile>(sym->file)->fetch();
     return;
   }
 
   llvm_unreachable("Symbol::fetch() is called on a non-lazy symbol");
 }
 
 MemoryBufferRef LazyArchive::getMemberBuffer() {
-  Archive::Child c = CHECK(
-      sym.getMember(), "could not get the member for symbol " + sym.getName());
+  Archive::Child c =
+      CHECK(sym.getMember(),
+            "could not get the member for symbol " + toELFString(sym));
 
   return CHECK(c.getMemoryBufferRef(),
                "could not get the buffer for the member defining symbol " +
-                   sym.getName());
+                   toELFString(sym));
 }
 
 uint8_t Symbol::computeBinding() const {
   if (config->relocatable)
     return binding;
   if (visibility != STV_DEFAULT && visibility != STV_PROTECTED)
     return STB_LOCAL;
   if (versionId == VER_NDX_LOCAL && isDefined() && !isPreemptible)
void elf::maybeWarnUnorderableSymbol(const Symbol *sym) {
   else if (d && !d->section)
     report(": unable to order absolute symbol: ");
   else if (d && isa<OutputSection>(d->section))
     report(": unable to order synthetic symbol: ");
   else if (d && !d->section->repl->isLive())
     report(": unable to order discarded symbol: ");
 }
 
-// Returns a symbol for an error message.
-std::string lld::toString(const Symbol &b) {
-  if (config->demangle)
-    if (Optional<std::string> s = demangleItanium(b.getName()))
-      return *s;
-  return b.getName();
-}
-
 static uint8_t getMinVisibility(uint8_t va, uint8_t vb) {
   if (va == STV_DEFAULT)
     return vb;
   if (vb == STV_DEFAULT)
     return va;
   return std::min(va, vb);
 }
 

lld/trunk/test/ELF/archive-thin-missing-member.s

 # REQUIRES: x86
 
 # RUN: rm -f %t-no-syms.a
 # RUN: rm -f %t-syms.a
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: llvm-ar rcTS %t-no-syms.a %t.o
 # RUN: llvm-ar rcT %t-syms.a %t.o
 # RUN: rm %t.o
 
 # Test error when loading symbols from missing thin archive member.
-# RUN: not ld.lld %t-no-syms.a -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR1
+# RUN: not ld.lld --entry=_Z1fi %t-no-syms.a -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR1
 # ERR1: {{.*}}-no-syms.a: could not get the buffer for a child of the archive: '{{.*}}.o': {{[Nn]}}o such file or directory
 
 # Test error when thin archive has symbol table but member is missing.
-# RUN: not ld.lld -m elf_amd64_fbsd %t-syms.a -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR2
-# ERR2: {{.*}}-syms.a: could not get the buffer for the member defining symbol _start: '{{.*}}.o': {{[Nn]}}o such file or directory
+# RUN: not ld.lld --entry=_Z1fi -m elf_amd64_fbsd %t-syms.a -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR2
+# ERR2: {{.*}}-syms.a: could not get the buffer for the member defining symbol f(int): '{{.*}}.o': {{[Nn]}}o such file or directory
+# RUN: not ld.lld --entry=_Z1fi --no-demangle -m elf_amd64_fbsd %t-syms.a -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR2MANGLE
+# ERR2MANGLE: {{.*}}-syms.a: could not get the buffer for the member defining symbol _Z1fi: '{{.*}}.o': {{[Nn]}}o such file or directory
 
 # Test error when thin archive is linked using --whole-archive but member is missing.
-# RUN: not ld.lld --whole-archive %t-syms.a -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR3
+# RUN: not ld.lld --entry=_Z1fi --whole-archive %t-syms.a -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR3
 # ERR3: {{.*}}-syms.a: could not get the buffer for a child of the archive: '{{.*}}.o': {{[Nn]}}o such file or directory
 
-.global _start
-_start:
+.global _Z1fi
+_Z1fi:
     nop