[lld] 76c36c1 - [lld-macho] Don't load dylibs more than once
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 10 15:58:42 PST 2020
Author: Jez Ng
Date: 2020-12-10T15:57:52-08:00
New Revision: 76c36c11a9c620a5eeced5750b844a1097ab7586
URL: https://github.com/llvm/llvm-project/commit/76c36c11a9c620a5eeced5750b844a1097ab7586
DIFF: https://github.com/llvm/llvm-project/commit/76c36c11a9c620a5eeced5750b844a1097ab7586.diff
LOG: [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
Added:
Modified:
lld/MachO/Driver.cpp
lld/MachO/Driver.h
lld/MachO/DriverUtils.cpp
lld/MachO/InputFiles.cpp
lld/MachO/InputFiles.h
lld/test/MachO/dylink.s
lld/test/MachO/implicit-dylibs.s
lld/test/MachO/lc-linker-option.ll
Removed:
################################################################################
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 469ab801bfe8..73846d420096 100644
--- a/lld/MachO/Driver.cpp
+++ b/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;
}
diff --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
index 9a6a05af0e0d..97b897f0271b 100644
--- a/lld/MachO/Driver.h
+++ b/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);
diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index 9f06901d3847..05677a9df78d 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/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))
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index e9e71e0c6a40..92fc7d10ebb0 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/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;
diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 2b492280c6f5..e5a1566dda20 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -134,7 +134,6 @@ class DylibFile : public InputFile {
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
diff --git a/lld/test/MachO/dylink.s b/lld/test/MachO/dylink.s
index 99d4ef8e081a..2a8e19d966d6 100644
--- a/lld/test/MachO/dylink.s
+++ b/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
diff --git a/lld/test/MachO/implicit-dylibs.s b/lld/test/MachO/implicit-dylibs.s
index 292fedb31dde..2030da423020 100644
--- a/lld/test/MachO/implicit-dylibs.s
+++ b/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
diff --git a/lld/test/MachO/lc-linker-option.ll b/lld/test/MachO/lc-linker-option.ll
index f3ad60d946f2..617de53d38f9 100644
--- a/lld/test/MachO/lc-linker-option.ll
+++ b/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
More information about the llvm-commits
mailing list