COFF: Define overloaded toString functions.
authorRui Ueyama <ruiu@google.com>
Wed, 7 Dec 2016 23:17:02 +0000 (23:17 +0000)
committerRui Ueyama <ruiu@google.com>
Wed, 7 Dec 2016 23:17:02 +0000 (23:17 +0000)
Previously, we had different way to stringize SymbolBody and InputFile
to construct error messages. This patch defines overloaded function
toString() so that we don't need to memorize all these different
function names.

With that change, it is now easy to include demangled names in error
messages. Now, if there is a symbol name conflict, we'll print out
both mangled and demangled names.

llvm-svn: 288992

lld/COFF/CMakeLists.txt
lld/COFF/Driver.cpp
lld/COFF/InputFiles.cpp
lld/COFF/InputFiles.h
lld/COFF/Strings.cpp [new file with mode: 0644]
lld/COFF/Strings.h [new file with mode: 0644]
lld/COFF/SymbolTable.cpp
lld/COFF/Symbols.cpp
lld/COFF/Symbols.h
lld/test/COFF/conflict-mangled.test [new file with mode: 0644]
lld/test/COFF/conflict.test

index a394f6a..8bcef96 100644 (file)
@@ -14,6 +14,7 @@ add_lld_library(lldCOFF
   MarkLive.cpp
   ModuleDef.cpp
   PDB.cpp
+  Strings.cpp
   SymbolTable.cpp
   Symbols.cpp
   Writer.cpp
index 01067b1..fbdb2d0 100644 (file)
@@ -581,7 +581,7 @@ void LinkerDriver::link(llvm::ArrayRef<const char *> ArgsArr) {
       continue;
     }
     if (Config->Machine != MT)
-      fatal(File->getShortName() + ": machine type " + machineToStr(MT) +
+      fatal(toString(File.get()) + ": machine type " + machineToStr(MT) +
             " conflicts with " + machineToStr(Config->Machine));
   }
   if (Config->Machine == IMAGE_FILE_MACHINE_UNKNOWN) {
index d832b57..a00d762 100644 (file)
@@ -45,29 +45,13 @@ namespace coff {
 
 int InputFile::NextIndex = 0;
 llvm::LLVMContext BitcodeFile::Context;
-
-// 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);
-}
-
-// Returns a string in the format of "foo.obj" or "foo.obj(bar.lib)".
-std::string InputFile::getShortName() {
-  if (ParentName == "")
-    return getName().lower();
-  std::string Res = (getBasename(ParentName) + "(" +
-                     getBasename(getName()) + ")").str();
-  return StringRef(Res).lower();
-}
+std::mutex BitcodeFile::Mu;
 
 ArchiveFile::ArchiveFile(MemoryBufferRef M) : InputFile(ArchiveKind, M) {}
 
 void ArchiveFile::parse() {
   // Parse a MemoryBufferRef as an archive file.
-  File = check(Archive::create(MB), getShortName());
+  File = check(Archive::create(MB), toString(this));
 
   // Allocate a buffer for Lazy objects.
   size_t NumSyms = File->getNumberOfSymbols();
@@ -84,7 +68,7 @@ void ArchiveFile::parse() {
   for (auto &Child : File->children(Err))
     Seen[Child.getChildOffset()].clear();
   if (Err)
-    fatal(Err, getShortName());
+    fatal(Err, toString(this));
 }
 
 // Returns a buffer pointing to a member file containing a given symbol.
@@ -113,14 +97,13 @@ MutableArrayRef<Lazy> ArchiveFile::getLazySymbols() { return LazySymbols; }
 
 void ObjectFile::parse() {
   // Parse a memory buffer as a COFF file.
-  std::unique_ptr<Binary> Bin =
-      check(createBinary(MB), getShortName());
+  std::unique_ptr<Binary> Bin = check(createBinary(MB), toString(this));
 
   if (auto *Obj = dyn_cast<COFFObjectFile>(Bin.get())) {
     Bin.release();
     COFFObj.reset(Obj);
   } else {
-    fatal(getShortName() + " is not a COFF file");
+    fatal(toString(this) + " is not a COFF file");
   }
 
   // Read section and symbol tables.
@@ -189,7 +172,7 @@ void ObjectFile::initializeSymbols() {
   for (uint32_t I = 0; I < NumSymbols; ++I) {
     // Get a COFFSymbolRef object.
     COFFSymbolRef Sym =
-        check(COFFObj->getSymbol(I), "broken object file: " + getShortName());
+        check(COFFObj->getSymbol(I), "broken object file: " + toString(this));
 
     const void *AuxP = nullptr;
     if (Sym.getNumberOfAuxSymbols())
@@ -251,12 +234,12 @@ Defined *ObjectFile::createDefined(COFFSymbolRef Sym, const void *AuxP,
 
   // Reserved sections numbers don't have contents.
   if (llvm::COFF::isReservedSectionNumber(SectionNumber))
-    fatal("broken object file: " + getShortName());
+    fatal("broken object file: " + toString(this));
 
   // This symbol references a section which is not present in the section
   // header.
   if ((uint32_t)SectionNumber >= SparseChunks.size())
-    fatal("broken object file: " + getShortName());
+    fatal("broken object file: " + toString(this));
 
   // Nothing else to do without a section chunk.
   auto *SC = cast_or_null<SectionChunk>(SparseChunks[SectionNumber]);
@@ -395,7 +378,26 @@ MachineTypes BitcodeFile::getMachineType() {
   }
 }
 
-std::mutex BitcodeFile::Mu;
+// 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);
+}
+
+// Returns a string in the format of "foo.obj" or "foo.obj(bar.lib)".
+std::string toString(InputFile *File) {
+  if (!File)
+    return "(internal)";
+  if (File->ParentName.empty())
+    return File->getName().lower();
+
+  std::string Res =
+      (getBasename(File->ParentName) + "(" + getBasename(File->getName()) + ")")
+          .str();
+  return StringRef(Res).lower();
+}
 
 } // namespace coff
 } // namespace lld
index 4cb9dd2..14aecd4 100644 (file)
@@ -61,13 +61,8 @@ public:
   // Returns the CPU type this file was compiled to.
   virtual MachineTypes getMachineType() { return IMAGE_FILE_MACHINE_UNKNOWN; }
 
-  // Returns a short, human-friendly filename. If this is a member of
-  // an archive file, a returned value includes parent's filename.
-  // Used for logging or debugging.
-  std::string getShortName();
-
-  // Sets a parent filename if this file is created from an archive.
-  void setParentName(StringRef N) { ParentName = N; }
+  // An archive file name if this file is created from an archive.
+  StringRef ParentName;
 
   // Returns .drectve section contents if exist.
   StringRef getDirectives() { return StringRef(Directives).trim(); }
@@ -86,7 +81,6 @@ protected:
 
 private:
   const Kind FileKind;
-  StringRef ParentName;
 };
 
 // .lib or .a file.
@@ -222,6 +216,8 @@ private:
   static std::mutex Mu;
 };
 
+std::string toString(InputFile *File);
+
 } // namespace coff
 } // namespace lld
 
diff --git a/lld/COFF/Strings.cpp b/lld/COFF/Strings.cpp
new file mode 100644 (file)
index 0000000..c7eaef0
--- /dev/null
@@ -0,0 +1,30 @@
+//===- Strings.cpp -------------------------------------------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Strings.h"
+
+#if defined(_MSC_VER)
+#include <DbgHelp.h>
+#include <Windows.h>
+#pragma comment(lib, "dbghelp.lib")
+#endif
+
+using namespace lld;
+using namespace lld::coff;
+using namespace llvm;
+
+Optional<std::string> coff::demangle(StringRef S) {
+#if defined(_MSC_VER)
+  char Buf[4096];
+  if (S.startswith("?"))
+    if (size_t Len = UnDecorateSymbolName(S.str().c_str(), Buf, sizeof(Buf), 0))
+      return std::string(Buf, Len);
+#endif
+  return None;
+}
diff --git a/lld/COFF/Strings.h b/lld/COFF/Strings.h
new file mode 100644 (file)
index 0000000..1f85f3e
--- /dev/null
@@ -0,0 +1,23 @@
+//===- Strings.h ------------------------------------------------*- C++ -*-===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLD_COFF_STRINGS_H
+#define LLD_COFF_STRINGS_H
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include <string>
+
+namespace lld {
+namespace coff {
+llvm::Optional<std::string> demangle(llvm::StringRef S);
+}
+}
+
+#endif
index 58c3618..c22126a 100644 (file)
@@ -71,7 +71,7 @@ void SymbolTable::readArchives() {
   for (std::future<ArchiveFile *> &Future : ArchiveQueue) {
     ArchiveFile *File = Future.get();
     if (Config->Verbose)
-      llvm::outs() << "Reading " << File->getShortName() << "\n";
+      llvm::outs() << "Reading " << toString(File) << "\n";
     for (Lazy &Sym : File->getLazySymbols())
       addLazy(&Sym, &LazySyms);
   }
@@ -92,7 +92,7 @@ void SymbolTable::readObjects() {
   for (size_t I = 0; I < ObjectQueue.size(); ++I) {
     InputFile *File = ObjectQueue[I].get();
     if (Config->Verbose)
-      llvm::outs() << "Reading " << File->getShortName() << "\n";
+      llvm::outs() << "Reading " << toString(File) << "\n";
     // Adding symbols may add more files to ObjectQueue
     // (but not to ArchiveQueue).
     for (SymbolBody *Sym : File->getSymbols())
@@ -102,8 +102,7 @@ void SymbolTable::readObjects() {
     if (!S.empty()) {
       Directives.push_back(S);
       if (Config->Verbose)
-        llvm::outs() << "Directives: " << File->getShortName()
-                     << ": " << S << "\n";
+        llvm::outs() << "Directives: " << toString(File) << ": " << S << "\n";
     }
   }
   ObjectQueue.clear();
@@ -161,8 +160,8 @@ void SymbolTable::reportRemainingUndefines(bool Resolve) {
     if (!isa<ArchiveFile>(File.get()))
       for (SymbolBody *Sym : File->getSymbols())
         if (Undefs.count(Sym->repl()))
-          llvm::errs() << File->getShortName() << ": undefined symbol: "
-                       << Sym->getName() << "\n";
+          llvm::errs() << toString(File.get())
+                       << ": undefined symbol: " << Sym->getName() << "\n";
   if (!Config->Force)
     fatal("link failed");
 }
@@ -211,8 +210,9 @@ void SymbolTable::addSymbol(SymbolBody *New) {
   // equivalent (conflicting), or more preferable, respectively.
   int Comp = Existing->compare(New);
   if (Comp == 0)
-    fatal("duplicate symbol: " + Existing->getDebugName() + " and " +
-          New->getDebugName());
+    fatal("duplicate symbol: " + toString(*Existing) + " in " +
+          toString(Existing->getFile()) + " and in " +
+          toString(New->getFile()));
   if (Comp < 0)
     Sym->Body = New;
 }
@@ -237,7 +237,7 @@ void SymbolTable::addMemberFile(Lazy *Body) {
   if (!File)
     return;
   if (Config->Verbose)
-    llvm::outs() << "Loaded " << File->getShortName() << " for "
+    llvm::outs() << "Loaded " << toString(File.get()) << " for "
                  << Body->getName() << "\n";
   addFile(std::move(File));
 }
@@ -321,7 +321,7 @@ DefinedAbsolute *SymbolTable::addAbsolute(StringRef Name, uint64_t VA) {
 
 void SymbolTable::printMap(llvm::raw_ostream &OS) {
   for (ObjectFile *File : ObjectFiles) {
-    OS << File->getShortName() << ":\n";
+    OS << toString(File) << ":\n";
     for (SymbolBody *Body : File->getSymbols())
       if (auto *R = dyn_cast<DefinedRegular>(Body))
         if (R->getChunk()->isLive())
index 6e2db66..5645862 100644 (file)
@@ -7,13 +7,15 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Symbols.h"
 #include "Error.h"
 #include "InputFiles.h"
-#include "Symbols.h"
+#include "Strings.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
+using namespace llvm;
 using namespace llvm::object;
 using llvm::sys::fs::identify_magic;
 using llvm::sys::fs::file_magic;
@@ -36,6 +38,16 @@ StringRef SymbolBody::getName() {
   return Name;
 }
 
+InputFile *SymbolBody::getFile() {
+  if (auto *Sym = dyn_cast<DefinedCOFF>(this))
+    return Sym->File;
+  if (auto *Sym = dyn_cast<DefinedBitcode>(this))
+    return Sym->File;
+  if (auto *Sym = dyn_cast<Lazy>(this))
+    return Sym->File;
+  return nullptr;
+}
+
 // Returns 1, 0 or -1 if this symbol should take precedence
 // over the Other, tie or lose, respectively.
 int SymbolBody::compare(SymbolBody *Other) {
@@ -150,18 +162,6 @@ int SymbolBody::compare(SymbolBody *Other) {
   llvm_unreachable("unknown symbol kind");
 }
 
-std::string SymbolBody::getDebugName() {
-  std::string N = getName().str();
-  if (auto *D = dyn_cast<DefinedCOFF>(this)) {
-    N += " ";
-    N += D->File->getShortName();
-  } else if (auto *D = dyn_cast<DefinedBitcode>(this)) {
-    N += " ";
-    N += D->File->getShortName();
-  }
-  return N;
-}
-
 COFFSymbolRef DefinedCOFF::getCOFFSymbol() {
   size_t SymSize = File->getCOFFObj()->getSymbolTableEntrySize();
   if (SymSize == sizeof(coff_symbol16))
@@ -201,7 +201,7 @@ std::unique_ptr<InputFile> Lazy::getMember() {
   else
     fatal("unknown file type: " + File->getName());
 
-  Obj->setParentName(File->getName());
+  Obj->ParentName = File->getName();
   return Obj;
 }
 
@@ -213,5 +213,12 @@ Defined *Undefined::getWeakAlias() {
   return nullptr;
 }
 
+// Returns a symbol name for an error message.
+std::string toString(SymbolBody &B) {
+  if (Optional<std::string> S = demangle(B.getName()))
+    return ("\"" + *S + "\" (" + B.getName() + ")").str();
+  return B.getName();
+}
+
 } // namespace coff
 } // namespace lld
index f96c1fb..a38e730 100644 (file)
@@ -75,6 +75,9 @@ public:
   // Returns the symbol name.
   StringRef getName();
 
+  // Returns the file from which this symbol was created.
+  InputFile *getFile();
+
   // A SymbolBody has a backreference to a Symbol. Originally they are
   // doubly-linked. A backreference will never change. But the pointer
   // in the Symbol may be mutated by the resolver. If you have a
@@ -89,10 +92,6 @@ public:
   // they are duplicate (conflicting) symbols.
   int compare(SymbolBody *Other);
 
-  // Returns a name of this symbol including source file name.
-  // Used only for debugging and logging.
-  std::string getDebugName();
-
 protected:
   explicit SymbolBody(Kind K, StringRef N = "")
       : SymbolKind(K), IsExternal(true), IsCOMDAT(false),
@@ -149,12 +148,14 @@ public:
     return S->kind() <= LastDefinedCOFFKind;
   }
 
+  ObjectFile *getFile() { return File; }
   int getFileIndex() { return File->Index; }
 
   COFFSymbolRef getCOFFSymbol();
 
-protected:
   ObjectFile *File;
+
+protected:
   const coff_symbol_generic *Sym;
 };
 
@@ -259,8 +260,9 @@ public:
 
   int getFileIndex() { return File->Index; }
 
-private:
   ArchiveFile *File;
+
+private:
   const Archive::Symbol Sym;
 };
 
@@ -368,7 +370,6 @@ public:
     return S->kind() == DefinedBitcodeKind;
   }
 
-private:
   BitcodeFile *File;
 };
 
@@ -397,6 +398,8 @@ inline uint64_t Defined::getRVA() {
   llvm_unreachable("unknown symbol kind");
 }
 
+std::string toString(SymbolBody &B);
+
 } // namespace coff
 } // namespace lld
 
diff --git a/lld/test/COFF/conflict-mangled.test b/lld/test/COFF/conflict-mangled.test
new file mode 100644 (file)
index 0000000..2f028ef
--- /dev/null
@@ -0,0 +1,37 @@
+# REQUIRES: system-windows
+# RUN: yaml2obj < %s > %t1.obj
+# RUN: yaml2obj < %s > %t2.obj
+# RUN: not lld-link /out:%t.exe %t1.obj %t2.obj >& %t.log
+# RUN: FileCheck %s < %t.log
+
+# CHECK: duplicate symbol: "int __cdecl mangled(void)" (?mangled@@YAHXZ) in {{.+}}1.obj and in {{.+}}2.obj
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     000000000000
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          6
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            '?mangled@@YAHXZ'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...
index 65dda51..a634c71 100644 (file)
@@ -8,7 +8,7 @@
 # RUN: not lld-link /out:%t.exe %t.lto1.obj %t.lto2.obj >& %t.log
 # RUN: FileCheck %s < %t.log
 
-# CHECK: duplicate symbol: foo {{.+}}1.obj and foo {{.+}}2.obj
+# CHECK: duplicate symbol: foo in {{.+}}1.obj and in {{.+}}2.obj
 
 --- !COFF
 header: