[PATCH] [lld] [mach-o]: Initial support for reading dylibs during link.

[kledzik at apple.com]

================
Comment at: lib/ReaderWriter/MachO/Atoms.h:128
@@ +127,3 @@
+    return false;
+  }
+
----------------
There is nothing to fix now.  But long term, what we need to have happen is when the Resolver/SymbolTable replaces an UndefinedAtom with a SharedLibraryAtom, if the UndefinedAtom was marked canBeNullAtRuntime, then the SharedLibraryAtom needs to be marked that way too.

================
Comment at: lib/ReaderWriter/MachO/Atoms.h:151
@@ +150,3 @@
+  StringRef _name;
+  StringRef _dylib;
+};
----------------
_dylib should be renamed _dylibInstallName.  It is not the path where the dylib was loaded from (that might be in an SDK).  It is the string from the dylib's LC_ID_DYLIB load command.

================
Comment at: lib/ReaderWriter/MachO/File.h:100
@@ +99,3 @@
+                                           bool dataSymbolOnly) const {
+    auto sym = _nameToAtom.find(name);
+
----------------
Add FIXME note that we need to track whether a symbol is data or code to implement dataSymbolOnly correctly.

================
Comment at: lib/ReaderWriter/MachO/File.h:102-106
@@ +101,7 @@
+
+    if (sym == _nameToAtom.end())
+      return nullptr;
+
+    sym->second =
+        new (_allocator) MachOSharedLibraryAtom(*this, name, _dylib_name);
+    return sym->second;
----------------
In the case where exports() has been called again with the same name, we should return the previously allocated atom (or assert).  This code allocates another copy of the atom.

================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp:439-441
@@ +438,5 @@
+
+  for (auto &sym : normalizedFile.globalSymbols) {
+    file->addSharedLibraryAtom(sym.name, copyRefs);
+  }
+
----------------
Probably should check visibility of symbol.  In dylibs, only global symbols should be in globalSymbols.  On the other hand, in .o files, hidden (scopeLinkageUnit) symbols  are also in globalSymbols. 

================
Comment at: test/mach-o/lit.local.cfg:4
@@ -3,1 +3,2 @@
 config.suffixes = ['.yaml']  
+config.excludes = ['Inputs']
----------------
Why the separate file for the dylib?  You can put multiple yaml "documents" in one .yaml file.  Just separate them with "---".  That is one of the reasons for using yaml for test cases in lld.  

You can also prune down that dylib yaml.  It does not need content - just symbols.

http://reviews.llvm.org/D4309