[lld-macho] Associate each Symbol with an InputFile

This makes our error messages more informative. But the bigger motivation is for
LTO symbol resolution, which will be in an upcoming diff. The changes in this
one are largely mechanical.

Reviewed By: #lld-macho, smeenai

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

lld/MachO/Driver.cpp

@@ -285,7 +285,8 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive) {
     } else if (config->forceLoadObjC) {
       for (const object::Archive::Symbol &sym : file->symbols())
         if (sym.getName().startswith(objc::klass))
-          symtab->addUndefined(sym.getName(), /*isWeakRef=*/false);
+          symtab->addUndefined(sym.getName(), /*file=*/nullptr,
+                               /*isWeakRef=*/false);
 
       // TODO: no need to look for ObjC sections for a given archive member if
       // we already found that it contains an ObjC symbol. We should also
@@ -510,7 +511,7 @@ static void replaceCommonSymbols() {
       continue;
 
     auto *isec = make<InputSection>();
-    isec->file = common->file;
+    isec->file = common->getFile();
     isec->name = section_names::common;
     isec->segname = segment_names::data;
     isec->align = common->align;
@@ -521,7 +522,7 @@ static void replaceCommonSymbols() {
     isec->flags = S_ZEROFILL;
     inputSections.push_back(isec);
 
-    replaceSymbol<Defined>(sym, sym->getName(), isec, /*value=*/0,
+    replaceSymbol<Defined>(sym, sym->getName(), isec->file, isec, /*value=*/0,
                            /*isWeakDef=*/false,
                            /*isExternal=*/true, common->privateExtern);
   }
@@ -721,6 +722,7 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   target = createTargetInfo(args);
 
   config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e, "_main"),
+                                       /*file=*/nullptr,
                                        /*isWeakRef=*/false);
   config->outputFile = args.getLastArgValue(OPT_o, "a.out");
   config->installName =

lld/MachO/ExportTrie.cpp

@@ -44,7 +44,6 @@
 #include "llvm/Support/LEB128.h"
 
 using namespace llvm;
-using namespace llvm::MachO;
 using namespace lld;
 using namespace lld::macho;
 
@@ -62,6 +61,7 @@ struct ExportInfo {
   uint8_t flags = 0;
   ExportInfo(const Symbol &sym, uint64_t imageBase)
       : address(sym.getVA() - imageBase) {
+    using namespace llvm::MachO;
     // Set the symbol type.
     if (sym.isWeakDef())
       flags |= EXPORT_SYMBOL_FLAGS_WEAK_DEFINITION;

lld/MachO/InputFiles.cpp

@@ -336,23 +336,23 @@ static macho::Symbol *createDefined(const structs::nlist_64 &sym,
 
   if (sym.n_type & (N_EXT | N_PEXT)) {
     assert((sym.n_type & N_EXT) && "invalid input");
-    return symtab->addDefined(name, isec, value, sym.n_desc & N_WEAK_DEF,
+    return symtab->addDefined(name, isec->file, isec, value,
-                              sym.n_type & N_PEXT);
+                              sym.n_desc & N_WEAK_DEF, sym.n_type & N_PEXT);
   }
-  return make<Defined>(name, isec, value, sym.n_desc & N_WEAK_DEF,
+  return make<Defined>(name, isec->file, isec, value, sym.n_desc & N_WEAK_DEF,
                        /*isExternal=*/false, /*isPrivateExtern=*/false);
 }
 
 // Absolute symbols are defined symbols that do not have an associated
 // InputSection. They cannot be weak.
 static macho::Symbol *createAbsolute(const structs::nlist_64 &sym,
-                                     StringRef name) {
+                                     InputFile *file, StringRef name) {
   if (sym.n_type & (N_EXT | N_PEXT)) {
     assert((sym.n_type & N_EXT) && "invalid input");
-    return symtab->addDefined(name, nullptr, sym.n_value, /*isWeakDef=*/false,
+    return symtab->addDefined(name, file, nullptr, sym.n_value,
-                              sym.n_type & N_PEXT);
+                              /*isWeakDef=*/false, sym.n_type & N_PEXT);
   }
-  return make<Defined>(name, nullptr, sym.n_value, /*isWeakDef=*/false,
+  return make<Defined>(name, file, nullptr, sym.n_value, /*isWeakDef=*/false,
                        /*isExternal=*/false, /*isPrivateExtern=*/false);
 }
 
@@ -362,12 +362,12 @@ macho::Symbol *ObjFile::parseNonSectionSymbol(const structs::nlist_64 &sym,
   switch (type) {
   case N_UNDF:
     return sym.n_value == 0
-               ? symtab->addUndefined(name, sym.n_desc & N_WEAK_REF)
+               ? symtab->addUndefined(name, this, sym.n_desc & N_WEAK_REF)
                : symtab->addCommon(name, this, sym.n_value,
                                    1 << GET_COMM_ALIGN(sym.n_desc),
                                    sym.n_type & N_PEXT);
   case N_ABS:
-    return createAbsolute(sym, name);
+    return createAbsolute(sym, this, name);
   case N_PBUD:
   case N_INDR:
     error("TODO: support symbols of type " + std::to_string(type));

lld/MachO/SymbolTable.cpp

@@ -37,9 +37,9 @@ std::pair<Symbol *, bool> SymbolTable::insert(StringRef name) {
   return {sym, true};
 }
 
-Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
+Symbol *SymbolTable::addDefined(StringRef name, InputFile *file,
-                                uint32_t value, bool isWeakDef,
+                                InputSection *isec, uint32_t value,
-                                bool isPrivateExtern) {
+                                bool isWeakDef, bool isPrivateExtern) {
   Symbol *s;
   bool wasInserted;
   bool overridesWeakDef = false;
@@ -54,8 +54,11 @@ Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
           defined->privateExtern &= isPrivateExtern;
         return s;
       }
-      if (!defined->isWeakDef())
+      if (!defined->isWeakDef()) {
-        error("duplicate symbol: " + name);
+        error("duplicate symbol: " + name + "\n>>> defined in " +
+              toString(defined->getFile()) + "\n>>> defined in " +
+              toString(file));
+      }
     } else if (auto *dysym = dyn_cast<DylibSymbol>(s)) {
       overridesWeakDef = !isWeakDef && dysym->isWeakDef();
     }
@@ -64,13 +67,14 @@ Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
   }
 
   Defined *defined =
-      replaceSymbol<Defined>(s, name, isec, value, isWeakDef,
+      replaceSymbol<Defined>(s, name, file, isec, value, isWeakDef,
                              /*isExternal=*/true, isPrivateExtern);
   defined->overridesWeakDef = overridesWeakDef;
   return s;
 }
 
-Symbol *SymbolTable::addUndefined(StringRef name, bool isWeakRef) {
+Symbol *SymbolTable::addUndefined(StringRef name, InputFile *file,
+                                  bool isWeakRef) {
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name);
@@ -78,7 +82,7 @@ Symbol *SymbolTable::addUndefined(StringRef name, bool isWeakRef) {
   auto refState = isWeakRef ? RefState::Weak : RefState::Strong;
 
   if (wasInserted)
-    replaceSymbol<Undefined>(s, name, refState);
+    replaceSymbol<Undefined>(s, name, file, refState);
   else if (auto *lazy = dyn_cast<LazySymbol>(s))
     lazy->fetchArchiveMember();
   else if (auto *dynsym = dyn_cast<DylibSymbol>(s))
@@ -162,11 +166,12 @@ Symbol *SymbolTable::addDSOHandle(const MachHeaderSection *header) {
   return s;
 }
 
-void lld::macho::treatUndefinedSymbol(StringRef symbolName,
+void lld::macho::treatUndefinedSymbol(const Undefined &sym) {
-                                      StringRef fileName) {
+  std::string message = "undefined symbol: " + toString(sym);
-  std::string message = ("undefined symbol: " + symbolName).str();
+  std::string fileName = toString(sym.getFile());
+
   if (!fileName.empty())
-    message += ("\n>>> referenced by " + fileName).str();
+    message += "\n>>> referenced by " + fileName;
   switch (config->undefinedSymbolTreatment) {
   case UndefinedSymbolTreatment::suppress:
     break;

lld/MachO/SymbolTable.h

@@ -20,9 +20,11 @@ namespace macho {
 class ArchiveFile;
 class DylibFile;
 class InputFile;
+class ObjFile;
 class InputSection;
 class MachHeaderSection;
 class Symbol;
+class Undefined;
 
 /*
  * Note that the SymbolTable handles name collisions by calling
@@ -32,10 +34,10 @@ class Symbol;
  */
 class SymbolTable {
 public:
-  Symbol *addDefined(StringRef name, InputSection *isec, uint32_t value,
+  Symbol *addDefined(StringRef name, InputFile *, InputSection *,
-                     bool isWeakDef, bool isPrivateExtern);
+                     uint32_t value, bool isWeakDef, bool isPrivateExtern);
 
-  Symbol *addUndefined(StringRef name, bool isWeakRef);
+  Symbol *addUndefined(StringRef name, InputFile *, bool isWeakRef);
 
   Symbol *addCommon(StringRef name, InputFile *, uint64_t size, uint32_t align,
                     bool isPrivateExtern);
@@ -56,7 +58,7 @@ private:
   std::vector<Symbol *> symVector;
 };
 
-extern void treatUndefinedSymbol(StringRef symbolName, StringRef fileName);
+void treatUndefinedSymbol(const Undefined &);
 
 extern SymbolTable *symtab;
 

lld/MachO/Symbols.cpp

@@ -44,7 +44,7 @@ uint64_t Defined::getFileOffset() const {
   return isec->getFileOffset() + value;
 }
 
-void LazySymbol::fetchArchiveMember() { file->fetch(sym); }
+void LazySymbol::fetchArchiveMember() { getFile()->fetch(sym); }
 
 uint64_t DSOHandle::getVA() const { return header->addr; }
 

lld/MachO/Symbols.h

@@ -9,6 +9,7 @@
 #ifndef LLD_MACHO_SYMBOLS_H
 #define LLD_MACHO_SYMBOLS_H
 
+#include "InputFiles.h"
 #include "InputSection.h"
 #include "Target.h"
 #include "lld/Common/ErrorHandler.h"
@@ -21,8 +22,6 @@ namespace macho {
 
 class InputSection;
 class MachHeaderSection;
-class DylibFile;
-class ArchiveFile;
 
 struct StringRefZ {
   StringRefZ(const char *s) : data(s), size(-1) {}
@@ -83,20 +82,23 @@ public:
 
   uint32_t symtabIndex = UINT32_MAX;
 
+  InputFile *getFile() const { return file; }
+
 protected:
-  Symbol(Kind k, StringRefZ name)
+  Symbol(Kind k, StringRefZ name, InputFile *file)
-      : symbolKind(k), nameData(name.data), nameSize(name.size) {}
+      : symbolKind(k), nameData(name.data), nameSize(name.size), file(file) {}
 
   Kind symbolKind;
   const char *nameData;
   mutable uint32_t nameSize;
+  InputFile *file;
 };
 
 class Defined : public Symbol {
 public:
-  Defined(StringRefZ name, InputSection *isec, uint32_t value, bool isWeakDef,
+  Defined(StringRefZ name, InputFile *file, InputSection *isec, uint32_t value,
-          bool isExternal, bool isPrivateExtern)
+          bool isWeakDef, bool isExternal, bool isPrivateExtern)
-      : Symbol(DefinedKind, name), isec(isec), value(value),
+      : Symbol(DefinedKind, name, file), isec(isec), value(value),
         overridesWeakDef(false), privateExtern(isPrivateExtern),
         weakDef(isWeakDef), external(isExternal) {}
 
@@ -116,6 +118,7 @@ public:
 
   static bool classof(const Symbol *s) { return s->kind() == DefinedKind; }
 
+  InputFile *file;
   InputSection *isec;
   uint32_t value;
 
@@ -136,8 +139,8 @@ enum class RefState : uint8_t { Unreferenced = 0, Weak = 1, Strong = 2 };
 
 class Undefined : public Symbol {
 public:
-  Undefined(StringRefZ name, RefState refState)
+  Undefined(StringRefZ name, InputFile *file, RefState refState)
-      : Symbol(UndefinedKind, name), refState(refState) {
+      : Symbol(UndefinedKind, name, file), refState(refState) {
     assert(refState != RefState::Unreferenced);
   }
 
@@ -167,7 +170,7 @@ class CommonSymbol : public Symbol {
 public:
   CommonSymbol(StringRefZ name, InputFile *file, uint64_t size, uint32_t align,
                bool isPrivateExtern)
-      : Symbol(CommonKind, name), file(file), size(size),
+      : Symbol(CommonKind, name, file), size(size),
         align(align != 1 ? align : llvm::PowerOf2Ceil(size)),
         privateExtern(isPrivateExtern) {
     // TODO: cap maximum alignment
@@ -175,7 +178,6 @@ public:
 
   static bool classof(const Symbol *s) { return s->kind() == CommonKind; }
 
-  InputFile *const file;
   const uint64_t size;
   const uint32_t align;
   const bool privateExtern;
@@ -185,18 +187,18 @@ class DylibSymbol : public Symbol {
 public:
   DylibSymbol(DylibFile *file, StringRefZ name, bool isWeakDef,
               RefState refState, bool isTlv)
-      : Symbol(DylibKind, name), file(file), refState(refState),
+      : Symbol(DylibKind, name, file), refState(refState), weakDef(isWeakDef),
-        weakDef(isWeakDef), tlv(isTlv) {}
+        tlv(isTlv) {}
 
   bool isWeakDef() const override { return weakDef; }
   bool isWeakRef() const override { return refState == RefState::Weak; }
   bool isReferenced() const { return refState != RefState::Unreferenced; }
   bool isTlv() const override { return tlv; }
   bool hasStubsHelper() const { return stubsHelperIndex != UINT32_MAX; }
+  DylibFile *getFile() const { return cast<DylibFile>(file); }
 
   static bool classof(const Symbol *s) { return s->kind() == DylibKind; }
 
-  DylibFile *file;
   uint32_t stubsHelperIndex = UINT32_MAX;
   uint32_t lazyBindOffset = UINT32_MAX;
 
@@ -210,14 +212,14 @@ private:
 class LazySymbol : public Symbol {
 public:
   LazySymbol(ArchiveFile *file, const llvm::object::Archive::Symbol &sym)
-      : Symbol(LazyKind, sym.getName()), file(file), sym(sym) {}
+      : Symbol(LazyKind, sym.getName(), file), sym(sym) {}
+
+  ArchiveFile *getFile() const { return cast<ArchiveFile>(file); }
+  void fetchArchiveMember();
 
   static bool classof(const Symbol *s) { return s->kind() == LazyKind; }
 
-  void fetchArchiveMember();
-
 private:
-  ArchiveFile *file;
   const llvm::object::Archive::Symbol sym;
 };
 
@@ -235,7 +237,7 @@ private:
 class DSOHandle : public Symbol {
 public:
   DSOHandle(const MachHeaderSection *header)
-      : Symbol(DSOHandleKind, name), header(header) {}
+      : Symbol(DSOHandleKind, name, nullptr), header(header) {}
 
   const MachHeaderSection *header;
 

lld/MachO/SyntheticSections.cpp

@@ -265,15 +265,15 @@ static void encodeBinding(const Symbol *sym, const OutputSection *osec,
 static void encodeDylibOrdinal(const DylibSymbol *dysym, Binding &lastBinding,
                                raw_svector_ostream &os) {
   using namespace llvm::MachO;
-  if (lastBinding.ordinal != dysym->file->ordinal) {
+  if (lastBinding.ordinal != dysym->getFile()->ordinal) {
-    if (dysym->file->ordinal <= BIND_IMMEDIATE_MASK) {
+    if (dysym->getFile()->ordinal <= BIND_IMMEDIATE_MASK) {
       os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_ORDINAL_IMM |
-                                 dysym->file->ordinal);
+                                 dysym->getFile()->ordinal);
     } else {
       os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB);
-      encodeULEB128(dysym->file->ordinal, os);
+      encodeULEB128(dysym->getFile()->ordinal, os);
     }
-    lastBinding.ordinal = dysym->file->ordinal;
+    lastBinding.ordinal = dysym->getFile()->ordinal;
   }
 }
 
@@ -442,10 +442,9 @@ void StubHelperSection::setup() {
   in.got->addEntry(stubBinder);
 
   inputSections.push_back(in.imageLoaderCache);
-  dyldPrivate =
+  dyldPrivate = make<Defined>("__dyld_private", nullptr, in.imageLoaderCache, 0,
-      make<Defined>("__dyld_private", in.imageLoaderCache, 0,
+                              /*isWeakDef=*/false,
-                    /*isWeakDef=*/false,
+                              /*isExternal=*/false, /*isPrivateExtern=*/false);
-                    /*isExternal=*/false, /*isPrivateExtern=*/false);
 }
 
 ImageLoaderCacheSection::ImageLoaderCacheSection() {
@@ -522,12 +521,12 @@ uint32_t LazyBindingSection::encode(const DylibSymbol &sym) {
   uint64_t offset = in.lazyPointers->addr - dataSeg->firstSection()->addr +
                     sym.stubsIndex * WordSize;
   encodeULEB128(offset, os);
-  if (sym.file->ordinal <= MachO::BIND_IMMEDIATE_MASK) {
+  if (sym.getFile()->ordinal <= MachO::BIND_IMMEDIATE_MASK) {
     os << static_cast<uint8_t>(MachO::BIND_OPCODE_SET_DYLIB_ORDINAL_IMM |
-                               sym.file->ordinal);
+                               sym.getFile()->ordinal);
   } else {
     os << static_cast<uint8_t>(MachO::BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB);
-    encodeULEB128(sym.file->ordinal, os);
+    encodeULEB128(sym.getFile()->ordinal, os);
   }
 
   uint8_t flags = MachO::BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM;
@@ -786,7 +785,7 @@ void SymtabSection::writeTo(uint8_t *buf) const {
       nList->n_desc |= defined->isExternalWeakDef() ? MachO::N_WEAK_DEF : 0;
     } else if (auto *dysym = dyn_cast<DylibSymbol>(entry.sym)) {
       uint16_t n_desc = nList->n_desc;
-      MachO::SET_LIBRARY_ORDINAL(n_desc, dysym->file->ordinal);
+      MachO::SET_LIBRARY_ORDINAL(n_desc, dysym->getFile()->ordinal);
       nList->n_type = MachO::N_EXT;
       n_desc |= dysym->isWeakRef() ? MachO::N_WEAK_REF : 0;
       nList->n_desc = n_desc;

lld/MachO/Writer.cpp

@@ -439,8 +439,8 @@ void Writer::scanRelocations() {
       if (target->hasAttr(r.type, RelocAttrBits::SUBTRAHEND))
         continue;
       if (auto *sym = r.referent.dyn_cast<lld::macho::Symbol *>()) {
-        if (isa<Undefined>(sym))
+        if (auto *undefined = dyn_cast<Undefined>(sym))
-          treatUndefinedSymbol(toString(*sym), toString(isec->file));
+          treatUndefinedSymbol(*undefined);
         else if (target->validateSymbolRelocation(sym, isec, r))
           prepareSymbolRelocation(sym, isec, r);
       } else {
@@ -458,7 +458,8 @@ void Writer::scanSymbols() {
       if (defined->overridesWeakDef)
         in.weakBinding->addNonWeakDefinition(defined);
     } else if (const auto *dysym = dyn_cast<DylibSymbol>(sym)) {
-      dysym->file->refState = std::max(dysym->file->refState, dysym->refState);
+      dysym->getFile()->refState =
+          std::max(dysym->getFile()->refState, dysym->refState);
     }
   }
 }

lld/test/MachO/invalid/abs-duplicate.s

@@ -4,7 +4,9 @@
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weakfoo.s -o %t/weakfoo.o
 # RUN: not %lld -lSystem %t/test.o %t/weakfoo.o -o %t/test 2>&1 | FileCheck %s
 
-# CHECK: lld: error: duplicate symbol: _weakfoo
+# CHECK:      error: duplicate symbol: _weakfoo
+# CHECK-NEXT: >>> defined in {{.*}}/test.o
+# CHECK-NEXT: >>> defined in {{.*}}/weakfoo.o
 
 #--- weakfoo.s
 .globl _weakfoo

lld/test/MachO/invalid/duplicate-symbol.s

@@ -1,10 +1,12 @@
 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t-dup.o
-# RUN: not %lld -o /dev/null %t-dup.o %t.o 2>&1 | FileCheck %s
+# RUN: not %lld -o /dev/null %t-dup.o %t.o 2>&1 | FileCheck %s -DFILE_1=%t-dup.o -DFILE_2=%t.o
-# RUN: not %lld -o /dev/null %t.o %t.o 2>&1 | FileCheck %s
+# RUN: not %lld -o /dev/null %t.o %t.o 2>&1 | FileCheck %s -DFILE_1=%t.o -DFILE_2=%t.o
 
-# CHECK: error: duplicate symbol: _main
+# CHECK:      error: duplicate symbol: _main
+# CHECK-NEXT: >>> defined in [[FILE_1]]
+# CHECK-NEXT: >>> defined in [[FILE_2]]
 
 .text
 .global _main