[lld] 1de9bc1 - [lld][MachO] Respect dylibs linked with `-allowable_client` (#114638)

via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 20 17:02:21 PST 2024


Author: Carlo Cabrera
Date: 2024-11-20T20:02:17-05:00
New Revision: 1de9bc1a27137a7559a247b73c14cfab3be81b54

URL: https://github.com/llvm/llvm-project/commit/1de9bc1a27137a7559a247b73c14cfab3be81b54
DIFF: https://github.com/llvm/llvm-project/commit/1de9bc1a27137a7559a247b73c14cfab3be81b54.diff

LOG: [lld][MachO] Respect dylibs linked with `-allowable_client` (#114638)

ld64.lld would previously 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. 

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

Added: 
    lld/test/MachO/Inputs/liballowable_client.dylib
    lld/test/MachO/allowable-client.s

Modified: 
    lld/MachO/Config.h
    lld/MachO/Driver.cpp
    lld/MachO/DriverUtils.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/InputFiles.h
    lld/MachO/Options.td

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index 8f6da6330d7ad4..41bcd58acc27f7 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 be0ee7ad8dff9b..53b4372435ab54 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1881,6 +1881,15 @@ bool link(ArrayRef<const char *> 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 858a4bb34029cf..308c5eaf8c3178 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -268,6 +268,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 3086c9cc4729dd..c3f7c434ffca14 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<sub_client_command>(hdr, LC_SUB_CLIENT)) {
+      StringRef allowableClient{reinterpret_cast<const char *>(cmd) +
+                                cmd->client};
+      allowableClients.push_back(allowableClient);
+    }
+  }
+
   const auto *dyldInfo = findCommand<dyld_info_command>(hdr, LC_DYLD_INFO_ONLY);
   const auto *exportsTrie =
       findCommand<linkedit_data_command>(hdr, LC_DYLD_EXPORTS_TRIE);
@@ -1891,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<std::string>(allowableClient.getInstallName().data()));
+
   auto addSymbol = [&](const llvm::MachO::Symbol &symbol,
                        const Twine &name) -> void {
     StringRef savedName = saver().save(name);

diff  --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 5e550c167c232e..bc8c8038a39d18 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<StringRef, 2> rpaths;
+  SmallVector<StringRef> 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 70eb7c8b9e466b..739d1da15d4660 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -875,8 +875,7 @@ def allowable_client : Separate<["-"], "allowable_client">,
     Group<grp_rare>;
 def client_name : Separate<["-"], "client_name">,
     MetaVarName<"<name>">,
-    HelpText<"Specifies a <name> this client should match with the -allowable_client <name> in a dependent dylib">,
-    Flags<[HelpHidden]>,
+    HelpText<"Specifies a <name> this client should match with the -allowable_client <name> in an explicitly linked dylib">,
     Group<grp_rare>;
 def umbrella : Separate<["-"], "umbrella">,
     MetaVarName<"<name>">,

diff  --git a/lld/test/MachO/Inputs/liballowable_client.dylib b/lld/test/MachO/Inputs/liballowable_client.dylib
new file mode 100755
index 00000000000000..7c174a8a72a4c0
Binary files /dev/null and b/lld/test/MachO/Inputs/liballowable_client.dylib 
diff er

diff  --git a/lld/test/MachO/allowable-client.s b/lld/test/MachO/allowable-client.s
new file mode 100644
index 00000000000000..3341dc59c1d811
--- /dev/null
+++ b/lld/test/MachO/allowable-client.s
@@ -0,0 +1,74 @@
+# REQUIRES: x86
+# 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
+
+# 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
+.globl _main
+_main:
+  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
+}


        


More information about the llvm-commits mailing list