[lld] [lld] Do not implicitly link non "public" libraries (PR #97639)

Wed Jul 3 15:19:32 PDT 2024

https://github.com/drodriguez updated https://github.com/llvm/llvm-project/pull/97639

>From 145dc3e1d426554571cc3858fd0b280157597f88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez=20Troiti=C3=B1o?=
 <danielrodriguez at meta.com>
Date: Mon, 24 Jun 2024 17:53:12 -0700
Subject: [PATCH 1/2] [lld] Do not implicitly link non "public" libraries

The LC_SUB_CLIENT Mach-O command and the `allowable-clients` TBD entry
specify that the given framework (or library?) can only be linked
directly from the specified names, even if it is sitting in `/usr/lib` or
`/System/Library/Frameworks`.

Add a check for those conditions before checking if a library should be
implicitly linked, and link against their umbrella if they have
allowable clients. The code needs to be in both the binary libraries and
the interface libraries.

Add a test that reproduces the scenario in which a framework reexports
a private framework that sits in `/System/Library/Frameworks`, and check
for the symbols of the reexported framework to be associated with the
public framework, and not the private one.
---
 lld/MachO/InputFiles.cpp                      | 10 +++-
 .../MachO/implicit-and-allowable-clients.test | 49 +++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 lld/test/MachO/implicit-and-allowable-clients.test

diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 0cee1a84d0b55a..b40a812f30bd3b 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -1725,7 +1725,10 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
   }
 
   // Initialize symbols.
-  exportingFile = isImplicitlyLinked(installName) ? this : this->umbrella;
+  bool canBeImplicitlyLinked = findCommand(hdr, LC_SUB_CLIENT) == nullptr;
+  exportingFile = (canBeImplicitlyLinked && isImplicitlyLinked(installName))
+                      ? this
+                      : this->umbrella;
 
   const auto *dyldInfo = findCommand<dyld_info_command>(hdr, LC_DYLD_INFO_ONLY);
   const auto *exportsTrie =
@@ -1884,7 +1887,10 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
 
   checkAppExtensionSafety(interface.isApplicationExtensionSafe());
 
-  exportingFile = isImplicitlyLinked(installName) ? this : umbrella;
+  bool canBeImplicitlyLinked = interface.allowableClients().size() == 0;
+  exportingFile = (canBeImplicitlyLinked && isImplicitlyLinked(installName))
+                      ? this
+                      : umbrella;
   auto addSymbol = [&](const llvm::MachO::Symbol &symbol,
                        const Twine &name) -> void {
     StringRef savedName = saver().save(name);
diff --git a/lld/test/MachO/implicit-and-allowable-clients.test b/lld/test/MachO/implicit-and-allowable-clients.test
new file mode 100644
index 00000000000000..97123ab9c5dce1
--- /dev/null
+++ b/lld/test/MachO/implicit-and-allowable-clients.test
@@ -0,0 +1,49 @@
+# REQUIRES: arm
+# RUN: rm -rf %t; split-file %s %t
+# RUN: ln -s Versions/A/Framework1.tbd %t/System/Library/Frameworks/Framework1.framework/
+# RUN: ln -s Versions/A/Framework11.tbd %t/System/Library/Frameworks/Framework11.framework/
+# RUN: llvm-mc -filetype obj -triple arm64-apple-macos11.0 %t/test.s -o %t/test.o
+# RUN: %lld -arch arm64 -platform_version macos 11.0 11.0 -o %t/test -syslibroot %t -framework Framework1 %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 Framework1   _func1
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 Framework1   _func11
+
+#--- System/Library/Frameworks/Framework1.framework/Versions/A/Framework1.tbd
+--- !tapi-tbd
+tbd-version: 4
+targets: [ arm64-macos ]
+install-name: '/System/Library/Frameworks/Framework1.framework/Versions/A/Framework1'
+current-version: 1.0.0
+reexported-libraries:
+  - targets: [ arm64-macos ]
+    libraries: [ '/System/Library/Frameworks/Framework11.framework/Versions/A/Framework11' ]
+exports:
+  - targets: [ arm64-macos ]
+    symbols: [ '_func1' ]
+...
+#--- System/Library/Frameworks/Framework11.framework/Versions/A/Framework11.tbd
+--- !tapi-tbd
+tbd-version: 4
+targets: [ arm64-macos ]
+install-name: '/System/Library/Frameworks/Framework11.framework/Versions/A/Framework11'
+current-version: 1.0.0
+allowable-clients:
+  - targets: [ arm64-macos ]
+    clients: [ Framework1 ]
+exports:
+  - targets: [ arm64-macos ]
+    symbols: [ '_func11' ]
+...
+#--- test.s
+.text
+.globl _main
+
+_main:
+  ret
+
+.data
+  .quad _func1
+  .quad _func11
+

>From b69bd2f98e75674185daf28cb94f55cf9907537c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Rodri=CC=81guez=20Troitin=CC=83o?=
 <danielrodriguez at meta.com>
Date: Wed, 3 Jul 2024 15:17:37 -0700
Subject: [PATCH 2/2] [lld][test] Avoid names that can be prefixes

The function and framework names could had matched incorrectly because
ones were prefixes of the others. Use more clear and non-prefix names
for the frameworks and the functions in the test.
---
 .../MachO/implicit-and-allowable-clients.test | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/lld/test/MachO/implicit-and-allowable-clients.test b/lld/test/MachO/implicit-and-allowable-clients.test
index 97123ab9c5dce1..e3f93aa7df0e83 100644
--- a/lld/test/MachO/implicit-and-allowable-clients.test
+++ b/lld/test/MachO/implicit-and-allowable-clients.test
@@ -1,40 +1,40 @@
 # REQUIRES: arm
 # RUN: rm -rf %t; split-file %s %t
-# RUN: ln -s Versions/A/Framework1.tbd %t/System/Library/Frameworks/Framework1.framework/
-# RUN: ln -s Versions/A/Framework11.tbd %t/System/Library/Frameworks/Framework11.framework/
+# RUN: ln -s Versions/A/FrameworkPublic.tbd %t/System/Library/Frameworks/FrameworkPublic.framework/
+# RUN: ln -s Versions/A/FrameworkPrivate.tbd %t/System/Library/Frameworks/FrameworkPrivate.framework/
 # RUN: llvm-mc -filetype obj -triple arm64-apple-macos11.0 %t/test.s -o %t/test.o
-# RUN: %lld -arch arm64 -platform_version macos 11.0 11.0 -o %t/test -syslibroot %t -framework Framework1 %t/test.o
+# RUN: %lld -arch arm64 -platform_version macos 11.0 11.0 -o %t/test -syslibroot %t -framework FrameworkPublic %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 Framework1   _func1
-# CHECK-DAG: __DATA __data {{.*}} pointer 0 Framework1   _func11
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 FrameworkPublic   _funcPublic
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 FrameworkPublic   _funcPrivate
 
-#--- System/Library/Frameworks/Framework1.framework/Versions/A/Framework1.tbd
+#--- System/Library/Frameworks/FrameworkPublic.framework/Versions/A/FrameworkPublic.tbd
 --- !tapi-tbd
 tbd-version: 4
 targets: [ arm64-macos ]
-install-name: '/System/Library/Frameworks/Framework1.framework/Versions/A/Framework1'
+install-name: '/System/Library/Frameworks/FrameworkPublic.framework/Versions/A/FrameworkPublic'
 current-version: 1.0.0
 reexported-libraries:
   - targets: [ arm64-macos ]
-    libraries: [ '/System/Library/Frameworks/Framework11.framework/Versions/A/Framework11' ]
+    libraries: [ '/System/Library/Frameworks/FrameworkPrivate.framework/Versions/A/FrameworkPrivate' ]
 exports:
   - targets: [ arm64-macos ]
-    symbols: [ '_func1' ]
+    symbols: [ '_funcPublic' ]
 ...
-#--- System/Library/Frameworks/Framework11.framework/Versions/A/Framework11.tbd
+#--- System/Library/Frameworks/FrameworkPrivate.framework/Versions/A/FrameworkPrivate.tbd
 --- !tapi-tbd
 tbd-version: 4
 targets: [ arm64-macos ]
-install-name: '/System/Library/Frameworks/Framework11.framework/Versions/A/Framework11'
+install-name: '/System/Library/Frameworks/FrameworkPrivate.framework/Versions/A/FrameworkPrivate'
 current-version: 1.0.0
 allowable-clients:
   - targets: [ arm64-macos ]
-    clients: [ Framework1 ]
+    clients: [ FrameworkPublic ]
 exports:
   - targets: [ arm64-macos ]
-    symbols: [ '_func11' ]
+    symbols: [ '_funcPrivate' ]
 ...
 #--- test.s
 .text
@@ -44,6 +44,6 @@ _main:
   ret
 
 .data
-  .quad _func1
-  .quad _func11
+  .quad _funcPublic
+  .quad _funcPrivate
 

