[lld][MachO] Respect dylibs linked with -allowable_client

[carlocab]

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
.tbds.

Support for .tbds added in e7e6f15.

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

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-macho

Author: Carlo Cabrera (carlocab)

Changes

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
.tbds.

---

Full diff: https://github.com/llvm/llvm-project/pull/114638.diff

7 Files Affected:

• (modified) lld/MachO/Config.h (+1)
• (modified) lld/MachO/Driver.cpp (+38-1)
• (modified) lld/MachO/InputFiles.cpp (+7)
• (modified) lld/MachO/InputFiles.h (+1)
• (modified) lld/MachO/Options.td (+1-2)
• (added) lld/test/MachO/Inputs/liballowable_client.dylib ()
• (added) lld/test/MachO/allowable-client.s (+14)

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 ab4abb1fa97efc..934604cbd111d1 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -407,8 +407,33 @@ static InputFile *addFile(StringRef path, LoadType loadType,
   case file_magic::macho_dynamically_linked_shared_lib_stub:
   case file_magic::tapi_file:
     if (DylibFile *dylibFile =
-            loadDylib(mbref, nullptr, /*isBundleLoader=*/false, isExplicit))
+            loadDylib(mbref, nullptr, /*isBundleLoader=*/false, isExplicit)) {
+      if (isExplicit && !dylibFile->allowedClients.empty()) {
+        bool allowed = false;
+
+        for (StringRef allowedClient : dylibFile->allowedClients) {
+          if (allowedClient == "!") {
+            allowed = false;
+            break;
+          }
+
+          if (allowedClient == config->clientName) {
+            allowed = true;
+          }
+        }
+
+        // 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(dylibFile->installName) +
+                "' because " + config->clientName + " is not an allowed client");
+      }
       newFile = dylibFile;
+    }
     break;
   case file_magic::bitcode:
     newFile = make<BitcodeFile>(mbref, "", 0, isLazy);
@@ -1863,6 +1888,18 @@ 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 firstDot = cn.find_first_of('.');
+    cn = cn.take_front(firstDot);
+    auto firstUnderscore = cn.find_first_of('_');
+    cn = cn.take_front(firstUnderscore);
+    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/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 3086c9cc4729dd..a71030618c0d2e 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -1730,6 +1730,13 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
                       ? this
                       : this->umbrella;
 
+  if (!canBeImplicitlyLinked) {
+    for (auto *cmd: findCommands<sub_client_command>(hdr, LC_SUB_CLIENT)) {
+      StringRef allowedClient{reinterpret_cast<const char *>(cmd) + cmd->client};
+      allowedClients.push_back(allowedClient);
+    }
+  }
+
   const auto *dyldInfo = findCommand<dyld_info_command>(hdr, LC_DYLD_INFO_ONLY);
   const auto *exportsTrie =
       findCommand<linkedit_data_command>(hdr, LC_DYLD_EXPORTS_TRIE);
diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 5e550c167c232e..63f60fe715d563 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> allowedClients;
   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 differ
diff --git a/lld/test/MachO/allowable-client.s b/lld/test/MachO/allowable-client.s
new file mode 100644
index 00000000000000..7a1501bc3cee5a
--- /dev/null
+++ b/lld/test/MachO/allowable-client.s
@@ -0,0 +1,14 @@
+# 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
+
+# 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

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[carlocab]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off Keep my email addresses private setting in your account. See LLVM Discourse for more information.

Yep, fixed.

[carlocab]

Ping?

[nico]

Thank you very much! This looks basically right to me, just some minor comments.

[nico]

Thanks, code looks good to me.

I patched this in and verified that Chromium Framework.framework still links fine with this, just to make sure :)

Do you have commit permissions, or do you want me to push the button?

[carlocab]

No commit permissions, please merge whenever you're ready. Thanks!