[lld] aeae3e0 - [lld/mac] Emit only one LC_LOAD_DYLIB per dylib

[Nico Weber via llvm-commits]

Author: Nico Weber
Date: 2021-06-01T18:15:35-04:00
New Revision: aeae3e0ba9061a40209987d6256e489146c2bffb

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

LOG: [lld/mac] Emit only one LC_LOAD_DYLIB per dylib

In some cases, we end up with several distinct DylibFiles that
have the same install name. Only emit a single LC_LOAD_DYLIB in
those cases.

This happens in 3 cases I know of:

1. Some tbd files are symlinks. libpthread.tbd is a symlink against
   libSystem.tbd for example, so `-lSystem -lpthread` loads
   libSystem.tbd twice. We could (and maybe should) cache loaded
   dylibs by realpath() to catch this.

2. Some tbd files are copies of each other. For example,
   CFNetwork.framework/CFNetwork.tbd and
   CFNetwork.framework/Versions/A/CFNetwork.tbd are two distinct
   copies of the same file. The former is found by
   `-framework CFNetwork` and the latter by the reexport in
   CoreServices.tbd. We could conceivably catch this by
   making `-framework` search look in `Versions/Current` instead
   of in the root, and/or by using a content hash to cache
   tbd files, but that's starting to sound complicated.

3. Magic $ld$ symbol processing can change the install name of
   a dylib based on the target platform_version. Here, two
   truly distinct dylibs can have the same install name.

So we need this code to deal with (3) anyways. Might as well use
it for 1 and 2, at least for now :)

With this (and D103430), clang-format links in the same dylibs
when linked with lld and ld64.

Differential Revision: https://reviews.llvm.org/D103488

Added: 
    lld/test/MachO/dylink-ordinal.s

Modified: 
    lld/MachO/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 05cb10d16df5..b9686ce0fd86 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -678,6 +678,7 @@ template <class LP> void Writer::createLoadCommands() {
     in.header->addLoadCommand(make<LCMinVersion>(config->platformInfo));
 
   int64_t dylibOrdinal = 1;
+  DenseMap<StringRef, int64_t> ordinalForInstallName;
   for (InputFile *file : inputFiles) {
     if (auto *dylibFile = dyn_cast<DylibFile>(file)) {
       if (dylibFile->isBundleLoader) {
@@ -701,7 +702,29 @@ template <class LP> void Writer::createLoadCommands() {
            config->deadStripDylibs))
         continue;
 
-      dylibFile->ordinal = dylibOrdinal++;
+      // Several DylibFiles can have the same installName. Only emit a single
+      // load command for that installName and give all these DylibFiles the
+      // same ordinal.
+      // This can happen if:
+      // - a new framework could change its installName to an older
+      //   framework name via an $ld$ symbol depending on platform_version
+      // - symlink (eg libpthread.tbd is a symlink to libSystem.tbd)
+      // - a framework can be linked both explicitly on the linker
+      //   command line and implicitly as a reexport from a 
diff erent
+      //   framework. The re-export will usually point to the tbd file
+      //   in Foo.framework/Versions/A/Foo.tbd, while the explicit link will
+      //   usually find Foo.framwork/Foo.tbd. These are usually two identical
+      //   but distinct files (concrete example: CFNetwork.framework, reexported
+      //   from CoreServices.framework).
+      // In the first case, *semantically distinct* DylibFiles will have the
+      // same installName.
+      int64_t &ordinal = ordinalForInstallName[dylibFile->dylibName];
+      if (ordinal) {
+        dylibFile->ordinal = ordinal;
+        continue;
+      }
+
+      ordinal = dylibFile->ordinal = dylibOrdinal++;
       LoadCommandType lcType =
           dylibFile->forceWeakImport || dylibFile->refState == RefState::Weak
               ? LC_LOAD_WEAK_DYLIB

diff  --git a/lld/test/MachO/dylink-ordinal.s b/lld/test/MachO/dylink-ordinal.s
new file mode 100644
index 000000000000..26a988f31266
--- /dev/null
+++ b/lld/test/MachO/dylink-ordinal.s
@@ -0,0 +1,65 @@
+# REQUIRES: x86
+
+## --no-leading-lines needed for .tbd files.
+# RUN: rm -rf %t; split-file --no-leading-lines %s %t
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/main.s -o %t/main.o
+# RUN: %lld -o %t/main -L%t -lFoo -lBar -lSystem %t/main.o
+# RUN: llvm-objdump --lazy-bind -d --no-show-raw-insn %t/main | FileCheck %s
+
+# CHECK: callq 0x[[#%x,FOO_OFF:]]
+# CHECK-NEXT: callq 0x[[#%x,BAR_OFF:]]
+
+# CHECK: [[#%x,BAR_OFF]]: jmpq {{.*}} # [[#%x,BAR_BIND:]]
+# CHECK: [[#%x,FOO_OFF]]: jmpq {{.*}} # [[#%x,FOO_BIND:]]
+
+# CHECK-LABEL: Lazy bind table:
+# CHECK-DAG: __DATA __la_symbol_ptr 0x[[#%x,FOO_BIND]] Foo _foo
+# CHECK-DAG: __DATA __la_symbol_ptr 0x[[#%x,BAR_BIND]] Foo _bar
+
+# RUN: llvm-nm -m %t/main | FileCheck --check-prefix=NM %s
+
+# NM-DAG: _bar (from Foo)
+# NM-DAG: _foo (from Foo)
+
+# RUN: llvm-otool -L %t/main | FileCheck %s --check-prefix=LOAD
+
+# LOAD: Foo.dylib
+# LOAD-NOT: Foo.dylib
+
+#--- libFoo.tbd
+--- !tapi-tbd
+tbd-version:      4
+targets:          [ x86_64-macos ]
+uuids:
+  - target:       x86_64-macos
+    value:        00000000-0000-0000-0000-000000000000
+install-name:     'Foo.dylib'
+current-version:  0001.001.1
+exports:
+  - targets:      [ x86_64-macos ]
+    symbols:      [ _foo ]
+
+#--- libBar.tbd
+--- !tapi-tbd
+tbd-version:      4
+targets:          [ x86_64-macos ]
+uuids:
+  - target:       x86_64-macos
+    value:        00000000-0000-0000-0000-000000000000
+## Also uses Foo.dylib as install-name!
+## Normally, this would happen conditionally via an $ld$ symbol.
+install-name:     'Foo.dylib'
+current-version:  0001.001.1
+exports:
+  - targets:      [ x86_64-macos ]
+    symbols:      [ _bar ]
+
+#--- main.s
+.section __TEXT,__text
+.globl _main, _foo, _bar
+
+_main:
+  callq _foo
+  callq _bar
+  ret