Change when we choose to add an LC_LOAD_DYLIB to the final image.

Currently we do this when an atom is used, but we need to do it when a
dylib is referenced on the cmdline as this matches ld64.

This fixes much confusion over which maps are indexed with installName
vs path.  There is likely other confusion so i'll be seeing if i can remove
path() completely in a future commit as path() shouldn't really be needed by anyone.

llvm-svn: 278396

5 files changed, 61 insertions, 17 deletions

diff --git a/lld/include/lld/ReaderWriter/MachOLinkingContext.h b/lld/include/lld/ReaderWriter/MachOLinkingContext.h
index 7b673f0dad3..a9e80f50b23 100644
--- a/lld/include/lld/ReaderWriter/MachOLinkingContext.h
+++ b/lld/include/lld/ReaderWriter/MachOLinkingContext.h
@@ -377,6 +377,10 @@ public:
 
   uint32_t dylibCompatVersion(StringRef installName) const;
 
+  ArrayRef<mach_o::MachODylibFile*> allDylibs() const {
+    return _allDylibs;
+  }
+
   /// Creates a copy (owned by this MachOLinkingContext) of a string.
   StringRef copy(StringRef str) { return str.copy(_allocator); }
 
@@ -485,7 +489,7 @@ private:
   mutable std::unique_ptr<Writer> _writer;
   std::vector<SectionAlign> _sectAligns;
   mutable llvm::StringMap<mach_o::MachODylibFile*> _pathToDylibMap;
-  mutable std::set<mach_o::MachODylibFile*> _allDylibs;
+  mutable std::vector<mach_o::MachODylibFile*> _allDylibs;
   mutable std::set<mach_o::MachODylibFile*> _upwardDylibs;
   mutable std::vector<std::unique_ptr<File>> _indirectDylibs;
   mutable std::mutex _dylibsMutex;
diff --git a/lld/lib/ReaderWriter/MachO/File.h b/lld/lib/ReaderWriter/MachO/File.h
index 82c73e35e76..2bdd6342b47 100644
--- a/lld/lib/ReaderWriter/MachO/File.h
+++ b/lld/lib/ReaderWriter/MachO/File.h
@@ -311,7 +311,7 @@ public:
     _reExportedDylibs.emplace_back(dylibPath);
   }
 
-  StringRef installName() { return _installName; }
+  StringRef installName() const { return _installName; }
   uint32_t currentVersion() { return _currentVersion; }
   uint32_t compatVersion() { return _compatVersion; }
 
diff --git a/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp b/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp
index ebd4bf3abf9..1fb66558aa3 100644
--- a/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp
+++ b/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp
@@ -772,7 +772,10 @@ void MachOLinkingContext::createImplicitFiles(
 void MachOLinkingContext::registerDylib(MachODylibFile *dylib,
                                         bool upward) const {
   std::lock_guard<std::mutex> lock(_dylibsMutex);
-  _allDylibs.insert(dylib);
+
+  if (std::find(_allDylibs.begin(),
+                _allDylibs.end(), dylib) == _allDylibs.end())
+    _allDylibs.push_back(dylib);
   _pathToDylibMap[dylib->installName()] = dylib;
   // If path is different than install name, register path too.
   if (!dylib->path().equals(dylib->installName()))
diff --git a/lld/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp b/lld/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
index 73873258d12..f315e2c6077 100644
--- a/lld/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
+++ b/lld/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
@@ -1213,15 +1213,15 @@ void Util::addIndirectSymbols(const lld::File &atomFile, NormalizedFile &file) {
   }
 }
 
-void Util::addDependentDylibs(const lld::File &atomFile,NormalizedFile &nFile) {
+void Util::addDependentDylibs(const lld::File &atomFile,
+                              NormalizedFile &nFile) {
   // Scan all imported symbols and build up list of dylibs they are from.
   int ordinal = 1;
-  for (const SharedLibraryAtom *slAtom : atomFile.sharedLibrary()) {
-    StringRef loadPath = slAtom->loadName();
-    DylibPathToInfo::iterator pos = _dylibInfo.find(loadPath);
+  for (const auto *dylib : _ctx.allDylibs()) {
+    DylibPathToInfo::iterator pos = _dylibInfo.find(dylib->installName());
     if (pos == _dylibInfo.end()) {
       DylibInfo info;
-      bool flatNamespaceAtom = &slAtom->file() == _ctx.flatNamespaceFile();
+      bool flatNamespaceAtom = dylib == _ctx.flatNamespaceFile();
 
       // If we're in -flat_namespace mode (or this atom came from the flat
       // namespace file under -undefined dynamic_lookup) then use the flat
@@ -1230,24 +1230,22 @@ void Util::addDependentDylibs(const lld::File &atomFile,NormalizedFile &nFile) {
         info.ordinal = BIND_SPECIAL_DYLIB_FLAT_LOOKUP;
       else
         info.ordinal = ordinal++;
-      info.hasWeak = slAtom->canBeNullAtRuntime();
+      info.hasWeak = false;
       info.hasNonWeak = !info.hasWeak;
-      _dylibInfo[loadPath] = info;
+      _dylibInfo[dylib->installName()] = info;
 
       // Unless this was a flat_namespace atom, record the source dylib.
       if (!flatNamespaceAtom) {
         DependentDylib depInfo;
-        depInfo.path = loadPath;
+        depInfo.path = dylib->installName();
         depInfo.kind = llvm::MachO::LC_LOAD_DYLIB;
-        depInfo.currentVersion = _ctx.dylibCurrentVersion(loadPath);
-        depInfo.compatVersion = _ctx.dylibCompatVersion(loadPath);
+        depInfo.currentVersion = _ctx.dylibCurrentVersion(dylib->path());
+        depInfo.compatVersion = _ctx.dylibCompatVersion(dylib->path());
         nFile.dependentDylibs.push_back(depInfo);
       }
     } else {
-      if ( slAtom->canBeNullAtRuntime() )
-        pos->second.hasWeak = true;
-      else
-        pos->second.hasNonWeak = true;
+      pos->second.hasWeak = false;
+      pos->second.hasNonWeak = !pos->second.hasWeak;
     }
   }
   // Automatically weak link dylib in which all symbols are weak (canBeNull).
diff --git a/lld/test/mach-o/use-dylib.yaml b/lld/test/mach-o/use-dylib.yaml
new file mode 100644
index 00000000000..ddde2a8fe8d
--- /dev/null
+++ b/lld/test/mach-o/use-dylib.yaml
@@ -0,0 +1,39 @@
+# RUN: lld -flavor darwin -arch x86_64 %s \
+# RUN: %p/Inputs/use-simple-dylib.yaml %p/Inputs/x86_64/libSystem.yaml -dylib -o %t.dylib
+# RUN: llvm-objdump -private-headers %t.dylib | FileCheck %s
+
+# This test ensures that we have a LC_LOAD_DYLIB for libspecial.dylib even though we don't 
+# use any atoms from it.  This matches the ld64 behaviour.
+--- !mach-o
+arch:            x86_64
+file-type:       MH_OBJECT
+flags:           [  ]
+has-UUID:        false
+OS:              unknown
+sections:
+  - segment:         __TEXT
+    section:         __text
+    type:            S_REGULAR
+    attributes:      [ S_ATTR_PURE_INSTRUCTIONS ]
+    address:         0x0000000000000000
+    content:         [ 0x55, 0x48, 0x89, 0xE5, 0xE8, 0x00, 0x00, 0x00,
+                       0x00, 0xE8, 0x00, 0x00, 0x00, 0x00, 0xE8, 0x00,
+                       0x00, 0x00, 0x00, 0xE8, 0x00, 0x00, 0x00, 0x00,
+                       0xE8, 0x00, 0x00, 0x00, 0x00, 0x5D, 0xE9, 0x00,
+                       0x00, 0x00, 0x00 ]
+global-symbols:
+  - name:            _foo
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            1
+    value:           0x0000000000000000
+
+
+# CHECK:           cmd LC_LOAD_DYLIB
+# CHECK:          name libspecial.dylib (offset 24)
+# CHECK:       current version 1.0.0
+# CHECK: compatibility version 1.0.0
+# CHECK:           cmd LC_LOAD_DYLIB
+# CHECK:          name /usr/lib/libSystem.B.dylib (offset 24)
+# CHECK:       current version 1.0.0
+# CHECK: compatibility version 1.0.0