[lld-macho] Don't load dylibs more than once

Also remove `DylibFile::reexported` since it's unused.

Fixes llvm.org/PR48393.

Reviewed By: thakis

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

lld/MachO/Driver.cpp

@@ -309,10 +309,8 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive) {
     break;
   case file_magic::macho_dynamically_linked_shared_lib:
   case file_magic::macho_dynamically_linked_shared_lib_stub:
-    newFile = make<DylibFile>(mbref);
-    break;
   case file_magic::tapi_file: {
-    if (Optional<DylibFile *> dylibFile = makeDylibFromTAPI(mbref))
+    if (Optional<DylibFile *> dylibFile = loadDylib(mbref))
       newFile = *dylibFile;
     break;
   }

lld/MachO/Driver.h

@@ -43,8 +43,8 @@ std::string createResponseFile(const llvm::opt::InputArgList &args);
 // Check for both libfoo.dylib and libfoo.tbd (in that order).
 llvm::Optional<std::string> resolveDylibPath(llvm::StringRef path);
 
-llvm::Optional<DylibFile *> makeDylibFromTAPI(llvm::MemoryBufferRef mbref,
-                                              DylibFile *umbrella = nullptr);
+llvm::Optional<DylibFile *> loadDylib(llvm::MemoryBufferRef mbref,
+                                      DylibFile *umbrella = nullptr);
 
 uint32_t getModTime(llvm::StringRef path);
 

lld/MachO/DriverUtils.cpp

@@ -14,6 +14,8 @@
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
 #include "lld/Common/Reproduce.h"
+#include "llvm/ADT/CachedHashString.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
@@ -166,7 +168,7 @@ Optional<std::string> macho::resolveDylibPath(StringRef path) {
   return {};
 }
 
-Optional<DylibFile *> macho::makeDylibFromTAPI(MemoryBufferRef mbref,
+static Optional<DylibFile *> makeDylibFromTapi(MemoryBufferRef mbref,
                                                DylibFile *umbrella) {
   Expected<std::unique_ptr<InterfaceFile>> result = TextAPIReader::get(mbref);
   if (!result) {
@@ -177,6 +179,24 @@ Optional<DylibFile *> macho::makeDylibFromTAPI(MemoryBufferRef mbref,
   return make<DylibFile>(**result, umbrella);
 }
 
+static DenseSet<CachedHashStringRef> loadedDylibs;
+
+Optional<DylibFile *> macho::loadDylib(MemoryBufferRef mbref,
+                                       DylibFile *umbrella) {
+  StringRef path = mbref.getBufferIdentifier();
+  if (loadedDylibs.contains(CachedHashStringRef(path)))
+    return {};
+  loadedDylibs.insert(CachedHashStringRef(path));
+
+  file_magic magic = identify_magic(mbref.getBuffer());
+  if (magic == file_magic::tapi_file)
+    return makeDylibFromTapi(mbref, umbrella);
+
+  assert(magic == file_magic::macho_dynamically_linked_shared_lib ||
+         magic == file_magic::macho_dynamically_linked_shared_lib_stub);
+  return make<DylibFile>(mbref, umbrella);
+}
+
 uint32_t macho::getModTime(StringRef path) {
   fs::file_status stat;
   if (!fs::status(path, stat))

lld/MachO/InputFiles.cpp

@@ -457,12 +457,7 @@ static Optional<DylibFile *> loadDylib(StringRef path, DylibFile *umbrella) {
     error("could not read dylib file at " + path);
     return {};
   }
-
-  file_magic magic = identify_magic(mbref->getBuffer());
-  if (magic == file_magic::tapi_file)
-    return makeDylibFromTAPI(*mbref, umbrella);
-  assert(magic == file_magic::macho_dynamically_linked_shared_lib);
-  return make<DylibFile>(*mbref, umbrella);
+  return loadDylib(*mbref, umbrella);
 }
 
 // TBD files are parsed into a series of TAPI documents (InterfaceFiles), with
@@ -518,11 +513,10 @@ static bool isImplicitlyLinked(StringRef path) {
   // -sub_umbrella first to write a test case.
 }
 
-static Optional<DylibFile *> loadReexport(StringRef path, DylibFile *umbrella) {
+void loadReexport(StringRef path, DylibFile *umbrella) {
   Optional<DylibFile *> reexport = loadReexportHelper(path, umbrella);
   if (reexport && isImplicitlyLinked(path))
     inputFiles.push_back(*reexport);
-  return reexport;
 }
 
 DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella)
@@ -572,8 +566,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella)
     auto *c = reinterpret_cast<const dylib_command *>(cmd);
     StringRef reexportPath =
         reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
-    if (Optional<DylibFile *> reexport = loadReexport(reexportPath, umbrella))
-      reexported.push_back(*reexport);
+    loadReexport(reexportPath, umbrella);
   }
 }
 
@@ -621,9 +614,7 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella)
   }
 
   for (InterfaceFileRef intfRef : interface.reexportedLibraries())
-    if (Optional<DylibFile *> reexport =
-            loadReexport(intfRef.getInstallName(), umbrella))
-      reexported.push_back(*reexport);
+    loadReexport(intfRef.getInstallName(), umbrella);
 
   if (isTopLevelTapi)
     currentTopLevelTapi = nullptr;

lld/MachO/InputFiles.h

@@ -134,7 +134,6 @@ public:
   uint64_t ordinal = 0; // Ordinal numbering starts from 1, so 0 is a sentinel
   bool reexport = false;
   bool forceWeakImport = false;
-  std::vector<DylibFile *> reexported;
 };
 
 // .a file

lld/test/MachO/dylink.s

@@ -38,6 +38,20 @@
 # CHECK-DAG: __DATA       __data 0x{{0*}}[[#%x, DATA_ADDR + 8]]                       pointer 8   libhello   _hello_its_me
 # CHECK-DAG: __DATA       __data 0x{{0*}}[[#%x, DATA_ADDR + 16]]                      pointer -15 libgoodbye _goodbye_world
 
+# RUN: llvm-objdump --macho --all-headers %t/dylink | FileCheck %s \
+# RUN:   --check-prefix=LOAD --implicit-check-not LC_LOAD_DYLIB
+## Check that we don't create duplicate LC_LOAD_DYLIBs.
+# RUN: %lld -o %t/dylink -L%t -lhello -lhello -lgoodbye -lgoodbye %t/dylink.o
+# RUN: llvm-objdump --macho --all-headers %t/dylink | FileCheck %s \
+# RUN:   --check-prefix=LOAD --implicit-check-not LC_LOAD_DYLIB
+
+# LOAD:              cmd LC_LOAD_DYLIB
+# LOAD-NEXT:     cmdsize
+# LOAD-NEXT:        name @executable_path/libhello.dylib
+# LOAD:              cmd LC_LOAD_DYLIB
+# LOAD-NEXT:     cmdsize
+# LOAD-NEXT:        name @executable_path/libgoodbye.dylib
+
 .section __TEXT,__text
 .globl _main
 

lld/test/MachO/implicit-dylibs.s

@@ -31,6 +31,11 @@
 
 # RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s \
 # RUN:   --check-prefix=LOAD -DDIR=%t --implicit-check-not LC_LOAD_DYLIB
+## Check that we don't create duplicate LC_LOAD_DYLIBs.
+# RUN: %lld -syslibroot %t -o %t/test -lSystem -L%t -lreexporter -ltoplevel %t/test.o
+# RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s \
+# RUN:   --check-prefix=LOAD -DDIR=%t --implicit-check-not LC_LOAD_DYLIB
+
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
 # LOAD-NEXT:    name /usr/lib/libSystem.B.dylib

lld/test/MachO/lc-linker-option.ll

@@ -4,14 +4,21 @@
 
 # RUN: llvm-as %t/framework.ll -o %t/framework.o
 # RUN: %lld %t/framework.o -o %t/frame
-# RUN: llvm-objdump --macho --all-headers %t/frame | FileCheck --check-prefix=FRAME %s
+# RUN: llvm-objdump --macho --all-headers %t/frame | FileCheck --check-prefix=FRAME %s \
+# RUN:  --implicit-check-not LC_LOAD_DYLIB
 # FRAME:          cmd LC_LOAD_DYLIB
 # FRAME-NEXT: cmdsize
 # FRAME-NEXT:    name /System/Library/Frameworks/CoreFoundation.framework/CoreFoundation
 
 # RUN: llvm-as %t/l.ll -o %t/l.o
 # RUN: %lld %t/l.o -o %t/l
-# RUN: llvm-objdump --macho --all-headers %t/l | FileCheck --check-prefix=LIB %s
+# RUN: llvm-objdump --macho --all-headers %t/l | FileCheck --check-prefix=LIB %s \
+# RUN:  --implicit-check-not LC_LOAD_DYLIB
+
+## Check that we don't create duplicate LC_LOAD_DYLIBs.
+# RUN: %lld -lSystem %t/l.o -o %t/l
+# RUN: llvm-objdump --macho --all-headers %t/l | FileCheck --check-prefix=LIB %s \
+# RUN:  --implicit-check-not LC_LOAD_DYLIB
 # LIB:          cmd LC_LOAD_DYLIB
 # LIB-NEXT: cmdsize
 # LIB-NEXT:    name /usr/lib/libSystem.B.dylib
@@ -36,7 +43,7 @@ target triple = "x86_64-apple-macosx10.15.0"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 !0 = !{!"-lSystem"}
-!llvm.linker.options = !{!0}
+!llvm.linker.options = !{!0, !0}
 
 define void @main() {
   ret void