From 316ee58c2af2e0de2881b1d184db83fc0a46e31d Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Sat, 2 Nov 2024 12:13:11 +0800 Subject: [PATCH 1/2] [lld][MachO] Respect dylibs linked with `-allowable_client` ld64.lld will currently allow you to link against dylibs linked with `-allowable_client`, even if the client's name does not match any allowed client. This change fixes that. See #114146 for related discussion. It doesn't quite fix that issue yet, but this change should enable fixing that issue too, once lld learns how to parse the `allowable_clients` field in `.tbd`s. The test binary `liballowable_client.dylib` was created on macOS with: echo | clang -xc - -dynamiclib -mmacosx-version-min=10.11 -arch x86_64 -Wl,-allowable_client,allowed -o lib/liballowable_client.dylib --- lld/MachO/Config.h | 1 + lld/MachO/Driver.cpp | 9 ++++++++ lld/MachO/DriverUtils.cpp | 20 ++++++++++++++++++ lld/MachO/InputFiles.cpp | 8 +++++++ lld/MachO/InputFiles.h | 1 + lld/MachO/Options.td | 3 +-- .../MachO/Inputs/liballowable_client.dylib | Bin 0 -> 16416 bytes lld/test/MachO/allowable-client.s | 15 +++++++++++++ 8 files changed, 55 insertions(+), 2 deletions(-) create mode 100755 lld/test/MachO/Inputs/liballowable_client.dylib create mode 100644 lld/test/MachO/allowable-client.s diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h index 8f6da6330d7ad..41bcd58acc27f 100644 --- a/lld/MachO/Config.h +++ b/lld/MachO/Config.h @@ -164,6 +164,7 @@ struct Configuration { llvm::StringRef finalOutput; llvm::StringRef installName; + llvm::StringRef clientName; llvm::StringRef mapFile; llvm::StringRef ltoObjPath; llvm::StringRef thinLTOJobs; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index ab4abb1fa97ef..f11fcd94250ee 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1863,6 +1863,15 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, config->installName = config->finalOutput; } + auto getClientName = [&]() { + StringRef cn = path::filename(config->finalOutput); + cn.consume_front("lib"); + auto firstDotOrUnderscore = cn.find_first_of("._"); + cn = cn.take_front(firstDotOrUnderscore); + return cn; + }; + config->clientName = args.getLastArgValue(OPT_client_name, getClientName()); + if (args.hasArg(OPT_mark_dead_strippable_dylib)) { if (config->outputType != MH_DYLIB) warn("-mark_dead_strippable_dylib: ignored, only has effect with -dylib"); diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp index 077a639bf7ab1..c22e78f6e61a3 100644 --- a/lld/MachO/DriverUtils.cpp +++ b/lld/MachO/DriverUtils.cpp @@ -264,6 +264,26 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella, if (newFile->exportingFile) newFile->parseLoadCommands(mbref); } + + if (explicitlyLinked && !newFile->allowableClients.empty()) { + bool allowed = std::any_of( + newFile->allowableClients.begin(), newFile->allowableClients.end(), + [&](StringRef allowableClient) { + // We only do a prefix match to match LD64's behaviour. + return allowableClient.starts_with(config->clientName); + }); + + // TODO: This behaviour doesn't quite match the latest available source + // release of LD64 (ld64-951.9), which allows "parents" and "siblings" + // to link to libraries even when they're not explicitly named as + // allowable clients. However, behaviour around this seems to have + // changed in the latest release of Xcode (ld64-1115.7.3), so it's not + // clear what the correct thing to do is yet. + if (!allowed) + error("cannot link directly with '" + + sys::path::filename(newFile->installName) + "' because " + + config->clientName + " is not an allowed client"); + } return newFile; } diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp index 3086c9cc4729d..c715169927e7b 100644 --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -1730,6 +1730,14 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella, ? this : this->umbrella; + if (!canBeImplicitlyLinked) { + for (auto *cmd : findCommands(hdr, LC_SUB_CLIENT)) { + StringRef allowableClient{reinterpret_cast(cmd) + + cmd->client}; + allowableClients.push_back(allowableClient); + } + } + const auto *dyldInfo = findCommand(hdr, LC_DYLD_INFO_ONLY); const auto *exportsTrie = findCommand(hdr, LC_DYLD_EXPORTS_TRIE); diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h index 5e550c167c232..bc8c8038a39d1 100644 --- a/lld/MachO/InputFiles.h +++ b/lld/MachO/InputFiles.h @@ -241,6 +241,7 @@ class DylibFile final : public InputFile { DylibFile *exportingFile = nullptr; DylibFile *umbrella; SmallVector rpaths; + SmallVector allowableClients; uint32_t compatibilityVersion = 0; uint32_t currentVersion = 0; int64_t ordinal = 0; // Ordinal numbering starts from 1, so 0 is a sentinel diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td index 70eb7c8b9e466..739d1da15d466 100644 --- a/lld/MachO/Options.td +++ b/lld/MachO/Options.td @@ -875,8 +875,7 @@ def allowable_client : Separate<["-"], "allowable_client">, Group; def client_name : Separate<["-"], "client_name">, MetaVarName<"">, - HelpText<"Specifies a this client should match with the -allowable_client in a dependent dylib">, - Flags<[HelpHidden]>, + HelpText<"Specifies a this client should match with the -allowable_client in an explicitly linked dylib">, Group; def umbrella : Separate<["-"], "umbrella">, MetaVarName<"">, diff --git a/lld/test/MachO/Inputs/liballowable_client.dylib b/lld/test/MachO/Inputs/liballowable_client.dylib new file mode 100755 index 0000000000000000000000000000000000000000..7c174a8a72a4c0972033660b129a8739f2aa4e3c GIT binary patch literal 16416 zcmeI(F-yZh6bJAZtyOG=4h|I^L`2Y`6?Aa3sKtsRf)%>RQEe)PSiwfBlU*DfUHk$r zx^>gt58#IoK@q=zgZO_AO?1BYymw9CK||}9*Y$p5(h8Tx9Mzk( zdd1Z0)dO6J&Ufd}SMVRwI(V1xJV9UgsrMSQwz9sOUs?(Fn)B`$^%{{#ZDCw92=$vo zrjg-sr?!(tmL2DyS>ADMv+LCCx|^w-U=;U`iL|EC{u(*y{4=^2T_cTJL)$*I3FHRy zuVNSz={k$m=8HQzQ@XfGUOryE?u}!t^Mxe(v1q?c1vOfYBrrP3Q&Lo6rcbFC_n)UP=Epypa2CZ zKmiI+fC3bt00k&O0Sf%9z*gb>Y5U$gTR!5i9B%cjwVs`yMXP!1pM%%Fp7BrO40h`K VWV$7mDNDRt+NZdP2wJZa`2@@fOX&ar literal 0 HcmV?d00001 diff --git a/lld/test/MachO/allowable-client.s b/lld/test/MachO/allowable-client.s new file mode 100644 index 0000000000000..8d499d570fe80 --- /dev/null +++ b/lld/test/MachO/allowable-client.s @@ -0,0 +1,15 @@ +# REQUIRES: x86 +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o +# RUN: not %lld -o %t %t.o -L%S/Inputs -lallowable_client 2>&1 | FileCheck %s --check-prefix=NOTALLOWED1 +# RUN: not %lld -o %t %t.o -L%S/Inputs -lallowable_client -client_name notallowed 2>&1 | FileCheck %s --check-prefix=NOTALLOWED2 +# RUN: %lld -o %t %t.o -L%S/Inputs -lallowable_client -client_name allowed +# RUN: %lld -o %t %t.o -L%S/Inputs -lallowable_client -client_name all + +# NOTALLOWED1: error: cannot link directly with 'liballowable_client.dylib' because {{.*}} is not an allowed client +# NOTALLOWED2: error: cannot link directly with 'liballowable_client.dylib' because notallowed is not an allowed client + +.text +.global _main +_main: + mov $0, %rax + ret From e7e6f15d365054c90114a55bfe6f4144659f033d Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Sun, 3 Nov 2024 13:34:03 +0800 Subject: [PATCH 2/2] [lld][MachO] Respect `allowable-clients` field in `.tbd`s Closes #114146. --- lld/MachO/InputFiles.cpp | 6 +++ lld/test/MachO/allowable-client.s | 77 +++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp index c715169927e7b..c3f7c434ffca1 100644 --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -1899,6 +1899,12 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella, exportingFile = (canBeImplicitlyLinked && isImplicitlyLinked(installName)) ? this : umbrella; + + if (!canBeImplicitlyLinked) + for (const auto &allowableClient : interface.allowableClients()) + allowableClients.push_back( + *make(allowableClient.getInstallName().data())); + auto addSymbol = [&](const llvm::MachO::Symbol &symbol, const Twine &name) -> void { StringRef savedName = saver().save(name); diff --git a/lld/test/MachO/allowable-client.s b/lld/test/MachO/allowable-client.s index 8d499d570fe80..3341dc59c1d81 100644 --- a/lld/test/MachO/allowable-client.s +++ b/lld/test/MachO/allowable-client.s @@ -1,15 +1,74 @@ # REQUIRES: x86 -# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o -# RUN: not %lld -o %t %t.o -L%S/Inputs -lallowable_client 2>&1 | FileCheck %s --check-prefix=NOTALLOWED1 -# RUN: not %lld -o %t %t.o -L%S/Inputs -lallowable_client -client_name notallowed 2>&1 | FileCheck %s --check-prefix=NOTALLOWED2 -# RUN: %lld -o %t %t.o -L%S/Inputs -lallowable_client -client_name allowed -# RUN: %lld -o %t %t.o -L%S/Inputs -lallowable_client -client_name all +# RUN: rm -rf %t; split-file %s %t +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o -# NOTALLOWED1: error: cannot link directly with 'liballowable_client.dylib' because {{.*}} is not an allowed client -# NOTALLOWED2: error: cannot link directly with 'liballowable_client.dylib' because notallowed is not an allowed client +# Check linking against a .dylib +# RUN: not %lld -o %t/test %t/test.o -L%S/Inputs -lallowable_client 2>&1 | FileCheck %s --check-prefix=NOTALLOWED-IMPLICIT +# RUN: not %lld -o %t/libtest_debug.exe %t/test.o -L%S/Inputs -lallowable_client 2>&1 | FileCheck %s --check-prefix=NOTALLOWED-IMPLICIT +# RUN: not %lld -o %t/test %t/test.o -L%S/Inputs -lallowable_client -client_name notallowed 2>&1 | FileCheck %s --check-prefix=NOTALLOWED-EXPLICIT +# RUN: %lld -o %t/test %t/test.o -L%S/Inputs -lallowable_client -client_name allowed +# RUN: %lld -o %t/test %t/test.o -L%S/Inputs -lallowable_client -client_name all +# RUN: %lld -o %t/all %t/test.o -L%S/Inputs -lallowable_client +# RUN: %lld -o %t/allowed %t/test.o -L%S/Inputs -lallowable_client +# RUN: %lld -o %t/liballowed_debug.exe %t/test.o -L%S/Inputs -lallowable_client +# Check linking against a .tbd +# RUN: not %lld -o %t/test %t/test.o -L%t -lallowable_client 2>&1 | FileCheck %s --check-prefix=NOTALLOWED-IMPLICIT +# RUN: not %lld -o %t/libtest_debug.exe %t/test.o -L%t -lallowable_client 2>&1 | FileCheck %s --check-prefix=NOTALLOWED-IMPLICIT +# RUN: not %lld -o %t/test %t/test.o -L%t -lallowable_client -client_name notallowed 2>&1 | FileCheck %s --check-prefix=NOTALLOWED-EXPLICIT +# RUN: %lld -o %t/test %t/test.o -L%t -lallowable_client -client_name allowed +# RUN: %lld -o %t/test %t/test.o -L%t -lallowable_client -client_name all +# RUN: %lld -o %t/all %t/test.o -L%t -lallowable_client +# RUN: %lld -o %t/allowed %t/test.o -L%t -lallowable_client +# RUN: %lld -o %t/liballowed_debug.exe %t/test.o -L%t -lallowable_client + +# NOTALLOWED-IMPLICIT: error: cannot link directly with 'liballowable_client.dylib' because test is not an allowed client +# NOTALLOWED-EXPLICIT: error: cannot link directly with 'liballowable_client.dylib' because notallowed is not an allowed client + +#--- test.s .text -.global _main +.globl _main _main: - mov $0, %rax ret + +#--- liballowable_client.tbd +{ + "main_library": { + "allowable_clients": [ + { + "clients": [ + "allowed" + ] + } + ], + "compatibility_versions": [ + { + "version": "0" + } + ], + "current_versions": [ + { + "version": "0" + } + ], + "flags": [ + { + "attributes": [ + "not_app_extension_safe" + ] + } + ], + "install_names": [ + { + "name": "lib/liballowable_client.dylib" + } + ], + "target_info": [ + { + "min_deployment": "10.11", + "target": "x86_64-macos" + } + ] + }, + "tapi_tbd_version": 5 +}