[llvm-branch-commits] [lld] 6a348f6 - [lld-macho] Implement `-no_implicit_dylibs`
Jez Ng via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Dec 10 16:03:02 PST 2020
Author: Jez Ng
Date: 2020-12-10T15:57:52-08:00
New Revision: 6a348f6158ecdb7a4bcac3f4cd1d3c5b6e80a550
URL: https://github.com/llvm/llvm-project/commit/6a348f6158ecdb7a4bcac3f4cd1d3c5b6e80a550
DIFF: https://github.com/llvm/llvm-project/commit/6a348f6158ecdb7a4bcac3f4cd1d3c5b6e80a550.diff
LOG: [lld-macho] Implement `-no_implicit_dylibs`
Dylibs that are "public" -- i.e. top-level system libraries -- are considered
implicitly linked when another library re-exports them. That is, we should load
them & bind directly to their symbols instead of via their re-exporting
umbrella library. This diff implements that behavior by default, as well as an
opt-out flag.
In theory, this is just a performance optimization, but in practice it seems
that it's needed for correctness.
Fixes llvm.org/PR48395.
Reviewed By: thakis
Differential Revision: https://reviews.llvm.org/D93000
Added:
lld/test/MachO/implicit-dylibs.s
Modified:
lld/MachO/Config.h
lld/MachO/Driver.cpp
lld/MachO/InputFiles.cpp
lld/MachO/Options.td
lld/test/MachO/reexport-stub.s
lld/test/MachO/stub-link.s
Removed:
################################################################################
diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index 46ce02fb25e0..2835b5c1546e 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -36,6 +36,7 @@ struct Configuration {
bool allLoad = false;
bool forceLoadObjC = false;
bool staticLink = false;
+ bool implicitDylibs = false;
bool isPic = false;
bool headerPadMaxInstallNames = false;
bool printEachFile = false;
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index b8a926279891..469ab801bfe8 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -719,6 +719,7 @@ bool macho::link(llvm::ArrayRef<const char *> argsArr, bool canExitEarly,
config->allLoad = args.hasArg(OPT_all_load);
config->forceLoadObjC = args.hasArg(OPT_ObjC);
config->demangle = args.hasArg(OPT_demangle);
+ config->implicitDylibs = !args.hasArg(OPT_no_implicit_dylibs);
if (const opt::Arg *arg = args.getLastArg(OPT_static, OPT_dynamic))
config->staticLink = (arg->getOption().getID() == OPT_static);
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 3e0ad220bff1..e9e71e0c6a40 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -479,7 +479,8 @@ const InterfaceFile *currentTopLevelTapi = nullptr;
// Re-exports can either refer to on-disk files, or to documents within .tbd
// files.
-static Optional<DylibFile *> loadReexport(StringRef path, DylibFile *umbrella) {
+static Optional<DylibFile *> loadReexportHelper(StringRef path,
+ DylibFile *umbrella) {
if (path::is_absolute(path, path::Style::posix))
for (StringRef root : config->systemLibraryRoots)
if (Optional<std::string> dylibPath =
@@ -504,6 +505,26 @@ static Optional<DylibFile *> loadReexport(StringRef path, DylibFile *umbrella) {
return {};
}
+// If a re-exported dylib is public (lives in /usr/lib or
+// /System/Library/Frameworks), then it is considered implicitly linked: we
+// should bind to its symbols directly instead of via the re-exporting umbrella
+// library.
+static bool isImplicitlyLinked(StringRef path) {
+ if (!config->implicitDylibs)
+ return false;
+
+ return path::parent_path(path) == "/usr/lib";
+ // TODO: check for public frameworks too. We'll need to implement
+ // -sub_umbrella first to write a test case.
+}
+
+static Optional<DylibFile *> 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)
: InputFile(DylibKind, mb) {
if (umbrella == nullptr)
@@ -522,17 +543,15 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella)
}
// Initialize symbols.
- // TODO: if a re-exported dylib is public (lives in /usr/lib or
- // /System/Library/Frameworks), we should bind to its symbols directly
- // instead of the re-exporting umbrella library.
+ DylibFile *exportingFile = isImplicitlyLinked(dylibName) ? this : umbrella;
if (const load_command *cmd = findCommand(hdr, LC_DYLD_INFO_ONLY)) {
auto *c = reinterpret_cast<const dyld_info_command *>(cmd);
parseTrie(buf + c->export_off, c->export_size,
[&](const Twine &name, uint64_t flags) {
bool isWeakDef = flags & EXPORT_SYMBOL_FLAGS_WEAK_DEFINITION;
bool isTlv = flags & EXPORT_SYMBOL_FLAGS_KIND_THREAD_LOCAL;
- symbols.push_back(symtab->addDylib(saver.save(name), umbrella,
- isWeakDef, isTlv));
+ symbols.push_back(symtab->addDylib(
+ saver.save(name), exportingFile, isWeakDef, isTlv));
});
} else {
error("LC_DYLD_INFO_ONLY not found in " + toString(this));
@@ -564,8 +583,9 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella)
umbrella = this;
dylibName = saver.save(interface.getInstallName());
+ DylibFile *exportingFile = isImplicitlyLinked(dylibName) ? this : umbrella;
auto addSymbol = [&](const Twine &name) -> void {
- symbols.push_back(symtab->addDylib(saver.save(name), umbrella,
+ symbols.push_back(symtab->addDylib(saver.save(name), exportingFile,
/*isWeakDef=*/false,
/*isTlv=*/false));
};
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index f00e23bfd791..d1d06879cafa 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -246,7 +246,6 @@ def seg1addr : Separate<["-"], "seg1addr">,
Group<grp_opts>;
def no_implicit_dylibs : Flag<["-"], "no_implicit_dylibs">,
HelpText<"Do not optimize public dylib transitive symbol references">,
- Flags<[HelpHidden]>,
Group<grp_opts>;
def exported_symbols_order : Separate<["-"], "exported_symbols_order">,
MetaVarName<"<file>">,
diff --git a/lld/test/MachO/implicit-dylibs.s b/lld/test/MachO/implicit-dylibs.s
new file mode 100644
index 000000000000..292fedb31dde
--- /dev/null
+++ b/lld/test/MachO/implicit-dylibs.s
@@ -0,0 +1,87 @@
+# REQUIRES: x86
+# RUN: rm -rf %t; split-file %s %t
+# RUN: mkdir -p %t/usr/lib/system
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libfoo.s -o %t/libfoo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libtoplevel.s -o %t/libtoplevel.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libsublevel.s -o %t/libsublevel.o
+## libunused will be used to verify that we load implicit dylibs even if we
+## don't use any symbols they contain.
+# RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-darwin -o %t/libunused.o
+# RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-darwin -o %t/reexporter.o
+
+# RUN: %lld -dylib -lSystem %t/libfoo.o -o %t/libfoo.dylib
+# RUN: %lld -dylib -lSystem %t/libtoplevel.o -o %t/usr/lib/libtoplevel.dylib -install_name /usr/lib/libtoplevel.dylib
+# RUN: %lld -dylib -lSystem %t/libsublevel.o -o %t/usr/lib/system/libsublevel.dylib -install_name /usr/lib/system/libsublevel.dylib
+# RUN: %lld -dylib -lSystem %t/libunused.o -o %t/usr/lib/libunused.dylib -install_name /usr/lib/libunused.dylib
+# RUN: %lld -dylib -syslibroot %t \
+# RUN: -lc++ -ltoplevel -lunused %t/usr/lib/system/libsublevel.dylib %t/libfoo.dylib \
+# RUN: -sub_library libc++ -sub_library libfoo -sub_library libtoplevel \
+# RUN: -sub_library libsublevel -sub_library libunused \
+# RUN: %t/reexporter.o -o %t/libreexporter.dylib
+
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %t/test.s -o %t/test.o
+# RUN: %lld -syslibroot %t -o %t/test -lSystem -L%t -lreexporter %t/test.o
+# RUN: llvm-objdump --bind --no-show-raw-insn -d %t/test | FileCheck %s
+# CHECK: Bind table:
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter _foo
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libtoplevel _toplevel
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter _sublevel
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libc++abi ___gxx_personality_v0
+
+# 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
+# LOAD: cmd LC_LOAD_DYLIB
+# LOAD-NEXT: cmdsize
+# LOAD-NEXT: name /usr/lib/libc++abi.dylib
+# LOAD: cmd LC_LOAD_DYLIB
+# LOAD-NEXT: cmdsize
+# LOAD-NEXT: name /usr/lib/libc++.dylib
+# LOAD: cmd LC_LOAD_DYLIB
+# LOAD-NEXT: cmdsize
+# LOAD-NEXT: name /usr/lib/libtoplevel.dylib
+# LOAD: cmd LC_LOAD_DYLIB
+# LOAD-NEXT: cmdsize
+# LOAD-NEXT: name /usr/lib/libunused.dylib
+# LOAD: cmd LC_LOAD_DYLIB
+# LOAD-NEXT: cmdsize
+# LOAD-NEXT: name [[DIR]]/libreexporter.dylib
+
+# RUN: %lld -no_implicit_dylibs -syslibroot %t -o %t/no-implicit -lSystem -L%t -lreexporter %t/test.o
+# RUN: llvm-objdump --bind --no-show-raw-insn -d %t/no-implicit | FileCheck %s --check-prefix=NO-IMPLICIT
+# NO-IMPLICIT: Bind table:
+# NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _foo
+# NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _toplevel
+# NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _sublevel
+# NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter ___gxx_personality_v0
+
+#--- libfoo.s
+.data
+.globl _foo
+_foo:
+
+#--- libtoplevel.s
+.data
+.globl _toplevel
+_toplevel:
+
+#--- libsublevel.s
+.data
+.globl _sublevel
+_sublevel:
+
+#--- test.s
+.text
+.globl _main
+
+_main:
+ ret
+
+.data
+ .quad _foo
+ .quad _toplevel
+ .quad _sublevel
+ .quad ___gxx_personality_v0
diff --git a/lld/test/MachO/reexport-stub.s b/lld/test/MachO/reexport-stub.s
index c63e49812e4d..a1706d19a14c 100644
--- a/lld/test/MachO/reexport-stub.s
+++ b/lld/test/MachO/reexport-stub.s
@@ -15,8 +15,8 @@
# RUN: %lld -o %t/test -lSystem -L%t -lreexporter %t/test.o
# RUN: llvm-objdump --bind --no-show-raw-insn -d %t/test | FileCheck %s
-# CHECK: Bind table:
-# CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter ___gxx_personality_v0
+# CHECK: Bind table:
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libc++abi ___gxx_personality_v0
.text
.globl _main
diff --git a/lld/test/MachO/stub-link.s b/lld/test/MachO/stub-link.s
index df8d94707d63..a0ee36d700ec 100644
--- a/lld/test/MachO/stub-link.s
+++ b/lld/test/MachO/stub-link.s
@@ -16,7 +16,7 @@
# CHECK-DAG: __DATA __data {{.*}} pointer 0 CoreFoundation _OBJC_METACLASS_$_NSObject
# CHECK-DAG: __DATA __data {{.*}} pointer 0 CoreFoundation _OBJC_IVAR_$_NSConstantArray._count
# CHECK-DAG: __DATA __data {{.*}} pointer 0 CoreFoundation _OBJC_EHTYPE_$_NSException
-# CHECK-DAG: __DATA __data {{.*}} pointer 0 libc++ ___gxx_personality_v0
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libc++abi ___gxx_personality_v0
.section __TEXT,__text
.global _main
More information about the llvm-branch-commits
mailing list