From 6a348f6158ecdb7a4bcac3f4cd1d3c5b6e80a550 Mon Sep 17 00:00:00 2001 From: Jez Ng Date: Wed, 9 Dec 2020 15:08:05 -0800 Subject: [PATCH] [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 --- lld/MachO/Config.h | 1 + lld/MachO/Driver.cpp | 1 + lld/MachO/InputFiles.cpp | 34 ++++++++++--- lld/MachO/Options.td | 1 - lld/test/MachO/implicit-dylibs.s | 87 ++++++++++++++++++++++++++++++++ lld/test/MachO/reexport-stub.s | 4 +- lld/test/MachO/stub-link.s | 2 +- 7 files changed, 119 insertions(+), 11 deletions(-) create mode 100644 lld/test/MachO/implicit-dylibs.s 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 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 loadReexport(StringRef path, DylibFile *umbrella) { +static Optional loadReexportHelper(StringRef path, + DylibFile *umbrella) { if (path::is_absolute(path, path::Style::posix)) for (StringRef root : config->systemLibraryRoots) if (Optional dylibPath = @@ -504,6 +505,26 @@ static Optional 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 loadReexport(StringRef path, DylibFile *umbrella) { + Optional 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(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; def no_implicit_dylibs : Flag<["-"], "no_implicit_dylibs">, HelpText<"Do not optimize public dylib transitive symbol references">, - Flags<[HelpHidden]>, Group; def exported_symbols_order : Separate<["-"], "exported_symbols_order">, MetaVarName<"">, 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