[lld] Reapply "[lld] enable fixup chains by default (#79894)" (PR #99255)

Daniel Bertalan via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 22 10:50:26 PDT 2024


https://github.com/BertalanD updated https://github.com/llvm/llvm-project/pull/99255

>From 5370eec41dd4cccaf6d6c6b3d879706f9092fe3d Mon Sep 17 00:00:00 2001
From: Daniel Bertalan <dani at danielbertalan.dev>
Date: Sat, 29 Jun 2024 10:30:11 +0200
Subject: [PATCH 1/2] Reapply "[lld] enable fixup chains by default (#79894)"

This reverts commit f55b79f59a77b4be586d649e9ced9f8667265011.

The known issues with chained fixups have been addressed by #98913,
#98305, #97156 and #95171.

Compared to the original commit, support for xrOS (which postdates
chained fixups' introduction) was added and an unnecessary test change
was removed.

----------
Original commit message:

Enable chained fixups in lld when all platform and version criteria are
met. This is an attempt at simplifying the logic used in ld 907:

https://github.com/apple-oss-distributions/ld64/blob/93d74eafc37c0558b4ffb88a8bc15c17bed44a20/src/ld/Options.cpp#L5458-L5549

Some changes were made to simplify the logic:
- only enable chained fixups for macOS from 13.0 to avoid the arch check
- only enable chained fixups for iphonesimulator from 16.0 to avoid the
arch check
- don't enable chained fixups for not specifically listed platforms
- don't enable chained fixups for arm64_32
---
 lld/MachO/Driver.cpp                          | 64 +++++++++++--------
 lld/test/MachO/arm64-32-stubs.s               |  2 +-
 lld/test/MachO/arm64-stubs.s                  |  2 +-
 lld/test/MachO/dyld-stub-binder.s             | 14 ++--
 .../invalid/chained-fixups-incompatible.s     |  4 --
 lld/test/MachO/objc-selrefs.s                 |  7 +-
 6 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index a370d5734124a..7185f6c13a244 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -49,6 +49,7 @@
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/TargetParser/Host.h"
+#include "llvm/TextAPI/Architecture.h"
 #include "llvm/TextAPI/PackedVersion.h"
 
 #include <algorithm>
@@ -1048,42 +1049,55 @@ static bool shouldEmitChainedFixups(const InputArgList &args) {
   if (arg && arg->getOption().matches(OPT_no_fixup_chains))
     return false;
 
-  bool isRequested = arg != nullptr;
-
-  // Version numbers taken from the Xcode 13.3 release notes.
-  static const std::array<std::pair<PlatformType, VersionTuple>, 5> minVersion =
-      {{{PLATFORM_MACOS, VersionTuple(11, 0)},
-        {PLATFORM_IOS, VersionTuple(13, 4)},
-        {PLATFORM_TVOS, VersionTuple(14, 0)},
-        {PLATFORM_WATCHOS, VersionTuple(7, 0)},
-        {PLATFORM_XROS, VersionTuple(1, 0)}}};
-  PlatformType platform = removeSimulator(config->platformInfo.target.Platform);
-  auto it = llvm::find_if(minVersion,
-                          [&](const auto &p) { return p.first == platform; });
-  if (it != minVersion.end() &&
-      it->second > config->platformInfo.target.MinDeployment) {
-    if (!isRequested)
-      return false;
+  bool requested = arg && arg->getOption().matches(OPT_fixup_chains);
+  if (!config->isPic) {
+    if (requested)
+      error("-fixup_chains is incompatible with -no_pie");
 
-    warn("-fixup_chains requires " + getPlatformName(config->platform()) + " " +
-         it->second.getAsString() + ", which is newer than target minimum of " +
-         config->platformInfo.target.MinDeployment.getAsString());
+    return false;
   }
 
   if (!is_contained({AK_x86_64, AK_x86_64h, AK_arm64}, config->arch())) {
-    if (isRequested)
+    if (requested)
       error("-fixup_chains is only supported on x86_64 and arm64 targets");
+
     return false;
   }
 
-  if (!config->isPic) {
-    if (isRequested)
-      error("-fixup_chains is incompatible with -no_pie");
+  if (args.hasArg(OPT_preload)) {
+    if (requested)
+      error("-fixup_chains is incompatible with -preload");
+
     return false;
   }
 
-  // TODO: Enable by default once stable.
-  return isRequested;
+  if (requested)
+    return true;
+
+  static const std::array<std::pair<PlatformType, VersionTuple>, 9> minVersion =
+      {{
+          {PLATFORM_IOS, VersionTuple(13, 4)},
+          {PLATFORM_IOSSIMULATOR, VersionTuple(16, 0)},
+          {PLATFORM_MACOS, VersionTuple(13, 0)},
+          {PLATFORM_TVOS, VersionTuple(14, 0)},
+          {PLATFORM_TVOSSIMULATOR, VersionTuple(15, 0)},
+          {PLATFORM_WATCHOS, VersionTuple(7, 0)},
+          {PLATFORM_WATCHOSSIMULATOR, VersionTuple(8, 0)},
+          {PLATFORM_XROS, VersionTuple(1, 0)},
+          {PLATFORM_XROS_SIMULATOR, VersionTuple(1, 0)},
+      }};
+  PlatformType platform = config->platformInfo.target.Platform;
+  auto it = llvm::find_if(minVersion,
+                          [&](const auto &p) { return p.first == platform; });
+
+  // We don't know the versions for other platforms, so default to disabled.
+  if (it == minVersion.end())
+    return false;
+
+  if (it->second > config->platformInfo.target.MinDeployment)
+    return false;
+
+  return true;
 }
 
 static bool shouldEmitRelativeMethodLists(const InputArgList &args) {
diff --git a/lld/test/MachO/arm64-32-stubs.s b/lld/test/MachO/arm64-32-stubs.s
index 743d5c419bf1a..a5d48ff5baef3 100644
--- a/lld/test/MachO/arm64-32-stubs.s
+++ b/lld/test/MachO/arm64-32-stubs.s
@@ -10,7 +10,7 @@
 # RUN: llvm-mc -filetype=obj -triple=arm64_32-apple-watchos %t/test.s -o %t/test.o
 # RUN: %lld-watchos -dylib -install_name @executable_path/libfoo.dylib %t/foo.o -o %t/libfoo.dylib
 # RUN: %lld-watchos -dylib -install_name @executable_path/libbar.dylib %t/bar.o -o %t/libbar.dylib
-# RUN: %lld-watchos -lSystem %t/libfoo.dylib %t/libbar.dylib %t/test.o -o %t/test
+# RUN: %lld-watchos -lSystem %t/libfoo.dylib %t/libbar.dylib %t/test.o -o %t/test -no_fixup_chains
 
 # RUN: llvm-objdump --no-print-imm-hex --macho -d --no-show-raw-insn --section="__TEXT,__stubs" --section="__TEXT,__stub_helper" %t/test | FileCheck %s
 
diff --git a/lld/test/MachO/arm64-stubs.s b/lld/test/MachO/arm64-stubs.s
index f714e20084895..6fd94661d32e2 100644
--- a/lld/test/MachO/arm64-stubs.s
+++ b/lld/test/MachO/arm64-stubs.s
@@ -5,7 +5,7 @@
 # RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/test.s -o %t/test.o
 # RUN: %lld -arch arm64 -dylib -install_name @executable_path/libfoo.dylib %t/foo.o -o %t/libfoo.dylib
 # RUN: %lld -arch arm64 -dylib -install_name @executable_path/libbar.dylib %t/bar.o -o %t/libbar.dylib
-# RUN: %lld -arch arm64 -lSystem %t/libfoo.dylib %t/libbar.dylib %t/test.o -o %t/test
+# RUN: %lld -arch arm64 -lSystem %t/libfoo.dylib %t/libbar.dylib %t/test.o -o %t/test -no_fixup_chains
 
 # RUN: llvm-objdump --no-print-imm-hex --macho -d --no-show-raw-insn --section="__TEXT,__stubs" --section="__TEXT,__stub_helper" %t/test | FileCheck %s
 
diff --git a/lld/test/MachO/dyld-stub-binder.s b/lld/test/MachO/dyld-stub-binder.s
index 1d2e556607c79..170fe8abd89bd 100644
--- a/lld/test/MachO/dyld-stub-binder.s
+++ b/lld/test/MachO/dyld-stub-binder.s
@@ -14,27 +14,27 @@
 # RUN: %lld -arch arm64 -lSystem -dylib %t/foo.o -o %t/libfoo.dylib
 # RUN: llvm-nm -m %t/libfoo.dylib | FileCheck --check-prefix=STUB %s
 
-
 ## Dylibs that do lazy dynamic calls do need dyld_stub_binder.
 # RUN: not %no-lsystem-lld -arch arm64 -dylib %t/bar.o %t/libfoo.dylib \
-# RUN:     -o %t/libbar.dylib 2>&1 | FileCheck --check-prefix=MISSINGSTUB %s
+# RUN:     -o %t/libbar.dylib -no_fixup_chains 2>&1 | \
+# RUN:     FileCheck --check-prefix=MISSINGSTUB %s
 # RUN: %lld -arch arm64 -lSystem -dylib %t/bar.o  %t/libfoo.dylib \
-# RUN:     -o %t/libbar.dylib
+# RUN:     -o %t/libbar.dylib -no_fixup_chains
 # RUN: llvm-nm -m %t/libbar.dylib | FileCheck --check-prefix=STUB %s
 
 ## As do executables.
 # RUN: not %no-lsystem-lld -arch arm64 %t/libfoo.dylib %t/libbar.dylib %t/test.o \
-# RUN:     -o %t/test 2>&1 | FileCheck --check-prefix=MISSINGSTUB %s
+# RUN:     -o %t/test -no_fixup_chains 2>&1 | FileCheck --check-prefix=MISSINGSTUB %s
 # RUN: %lld -arch arm64 -lSystem %t/libfoo.dylib %t/libbar.dylib %t/test.o \
-# RUN:     -o %t/test
+# RUN:     -o %t/test -no_fixup_chains
 # RUN: llvm-nm -m %t/test | FileCheck --check-prefix=STUB %s
 
 ## Test dynamic lookup of dyld_stub_binder.
 # RUN: %no-lsystem-lld -arch arm64 %t/libfoo.dylib %t/libbar.dylib %t/test.o \
-# RUN:     -o %t/test -undefined dynamic_lookup
+# RUN:     -o %t/test -undefined dynamic_lookup -no_fixup_chains
 # RUN: llvm-nm -m %t/test | FileCheck --check-prefix=DYNSTUB %s
 # RUN: %no-lsystem-lld -arch arm64 %t/libfoo.dylib %t/libbar.dylib %t/test.o \
-# RUN:     -o %t/test -U dyld_stub_binder
+# RUN:     -o %t/test -U dyld_stub_binder -no_fixup_chains
 # RUN: llvm-nm -m %t/test | FileCheck --check-prefix=DYNSTUB %s
 
 # MISSINGSTUB:      error: undefined symbol: dyld_stub_binder
diff --git a/lld/test/MachO/invalid/chained-fixups-incompatible.s b/lld/test/MachO/invalid/chained-fixups-incompatible.s
index 83411472beb9f..0589204968fe3 100644
--- a/lld/test/MachO/invalid/chained-fixups-incompatible.s
+++ b/lld/test/MachO/invalid/chained-fixups-incompatible.s
@@ -2,9 +2,6 @@
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: not %lld -lSystem -no_pie -fixup_chains %t.o -o /dev/null 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=NO-PIE
-# RUN: %no-fatal-warnings-lld -fixup_chains %t.o -o /dev/null \
-# RUN:   -lSystem -platform_version macos 10.15 10.15 2>&1 | \
-# RUN:   FileCheck %s --check-prefix=VERSION
 # RUN: llvm-mc -filetype=obj -triple=arm64_32-apple-darwin %s -o %t-arm64_32.o
 # RUN: not %lld-watchos -lSystem -fixup_chains %t-arm64_32.o -o /dev/null 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=ARCH
@@ -12,7 +9,6 @@
 ## Check that we emit diagnostics when -fixup_chains is explicitly specified,
 ## but we don't support creating chained fixups for said configuration.
 # NO-PIE:  error: -fixup_chains is incompatible with -no_pie
-# VERSION: warning: -fixup_chains requires macOS 11.0, which is newer than target minimum of 10.15
 # ARCH:    error: -fixup_chains is only supported on x86_64 and arm64 targets
 
 .globl _main
diff --git a/lld/test/MachO/objc-selrefs.s b/lld/test/MachO/objc-selrefs.s
index 6d144f4938b45..eebe7c6476cd5 100644
--- a/lld/test/MachO/objc-selrefs.s
+++ b/lld/test/MachO/objc-selrefs.s
@@ -6,14 +6,14 @@
 # RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/implicit-selrefs.s -o %t/implicit-selrefs.o
 
 # RUN: %lld -dylib -arch arm64 -lSystem -o %t/explicit-only-no-icf \
-# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o
+# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o -no_fixup_chains
 # RUN: llvm-otool -vs __DATA __objc_selrefs %t/explicit-only-no-icf | \
 # RUN:   FileCheck %s --check-prefix=EXPLICIT-NO-ICF
 
 ## NOTE: ld64 always dedups the selrefs unconditionally, but we only do it when
 ## ICF is enabled.
 # RUN: %lld -dylib -arch arm64 -lSystem -o %t/explicit-only-with-icf \
-# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o
+# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o -no_fixup_chains
 # RUN: llvm-otool -vs __DATA __objc_selrefs %t/explicit-only-with-icf \
 # RUN:   | FileCheck %s --check-prefix=EXPLICIT-WITH-ICF
 
@@ -25,7 +25,8 @@
 # SELREFS-EMPTY:
 
 # RUN: %lld -dylib -arch arm64 -lSystem --icf=all -o %t/explicit-and-implicit \
-# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o %t/implicit-selrefs.o
+# RUN:   %t/explicit-selrefs-1.o %t/explicit-selrefs-2.o %t/implicit-selrefs.o \
+# RUN:   -no_fixup_chains
 # RUN: llvm-otool -vs __DATA __objc_selrefs %t/explicit-and-implicit \
 # RUN:   | FileCheck %s --check-prefix=EXPLICIT-AND-IMPLICIT
 

>From 6312552a77a2b60be965ace7a27e682497774c9b Mon Sep 17 00:00:00 2001
From: Daniel Bertalan <dani at danielbertalan.dev>
Date: Mon, 22 Jul 2024 19:50:01 +0200
Subject: [PATCH 2/2] add release notes blurb

---
 lld/docs/ReleaseNotes.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
index 05179bfdcb536..8c85ed38c8642 100644
--- a/lld/docs/ReleaseNotes.rst
+++ b/lld/docs/ReleaseNotes.rst
@@ -99,6 +99,11 @@ MinGW Improvements
 MachO Improvements
 ------------------
 
+* Chained fixups are now enabled by default when targeting macOS 13.0,
+  iOS 13.4, tvOS 14.0, watchOS 7.0, and visionOS 1.0 or later.
+  They can be disabled with the `-no_fixup_chains` flag.
+  (`#99255 <https://github.com/llvm/llvm-project/pull/99255>`_)
+
 WebAssembly Improvements
 ------------------------
 



More information about the llvm-commits mailing list