[lld] 6578e0d - [lld-macho] Remove duplicate minimum version info

Keith Smiley via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 3 13:47:19 PST 2023


Author: Keith Smiley
Date: 2023-03-03T13:47:01-08:00
New Revision: 6578e0d1d0e435ab91b44bb2d40aea913044a65b

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

LOG: [lld-macho] Remove duplicate minimum version info

At some point PlatformInfo's Target changed types to a type that also
has minimum deployment target info. This caused ambiguity if you tried
to get the target triple from the Target, as the actual minimum version
info was being stored separately. This bulk of this change is changing
the parsing of these values to support this.

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

Added: 
    

Modified: 
    lld/MachO/Config.h
    lld/MachO/Driver.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/SyntheticSections.cpp
    lld/MachO/Writer.cpp
    lld/test/MachO/invalid/incompatible-target-tapi.test
    lld/test/MachO/tapi-link.s
    lld/test/MachO/zippered.yaml

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index af46303f1ae23..328aea49d77c0 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -42,7 +42,6 @@ using SegmentRenameMap = llvm::DenseMap<llvm::StringRef, llvm::StringRef>;
 
 struct PlatformInfo {
   llvm::MachO::Target target;
-  llvm::VersionTuple minimum;
   llvm::VersionTuple sdk;
 };
 

diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 121fa851041ce..0f2326b305b13 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -682,8 +682,9 @@ static PlatformVersion parsePlatformVersion(const Arg *arg) {
   return platformVersion;
 }
 
-// Has the side-effect of setting Config::platformInfo.
-static PlatformType parsePlatformVersions(const ArgList &args) {
+// Has the side-effect of setting Config::platformInfo and
+// potentially Config::secondaryPlatformInfo.
+static void setPlatformVersions(StringRef archName, const ArgList &args) {
   std::map<PlatformType, PlatformVersion> platformVersions;
   const PlatformVersion *lastVersionInfo = nullptr;
   for (const Arg *arg : args.filtered(OPT_platform_version)) {
@@ -699,11 +700,11 @@ static PlatformType parsePlatformVersions(const ArgList &args) {
 
   if (platformVersions.empty()) {
     error("must specify -platform_version");
-    return PLATFORM_UNKNOWN;
+    return;
   }
   if (platformVersions.size() > 2) {
     error("must specify -platform_version at most twice");
-    return PLATFORM_UNKNOWN;
+    return;
   }
   if (platformVersions.size() == 2) {
     bool isZipperedCatalyst = platformVersions.count(PLATFORM_MACOS) &&
@@ -715,21 +716,23 @@ static PlatformType parsePlatformVersions(const ArgList &args) {
     } else if (config->outputType != MH_DYLIB &&
                config->outputType != MH_BUNDLE) {
       error("writing zippered outputs only valid for -dylib and -bundle");
-    } else {
-      config->platformInfo.minimum = platformVersions[PLATFORM_MACOS].minimum;
-      config->platformInfo.sdk = platformVersions[PLATFORM_MACOS].sdk;
-      config->secondaryPlatformInfo = PlatformInfo{};
-      config->secondaryPlatformInfo->minimum =
-          platformVersions[PLATFORM_MACCATALYST].minimum;
-      config->secondaryPlatformInfo->sdk =
-          platformVersions[PLATFORM_MACCATALYST].sdk;
     }
-    return PLATFORM_MACOS;
+
+    config->platformInfo = {
+        MachO::Target(getArchitectureFromName(archName), PLATFORM_MACOS,
+                      platformVersions[PLATFORM_MACOS].minimum),
+        platformVersions[PLATFORM_MACOS].sdk};
+    config->secondaryPlatformInfo = {
+        MachO::Target(getArchitectureFromName(archName), PLATFORM_MACCATALYST,
+                      platformVersions[PLATFORM_MACCATALYST].minimum),
+        platformVersions[PLATFORM_MACCATALYST].sdk};
+    return;
   }
 
-  config->platformInfo.minimum = lastVersionInfo->minimum;
-  config->platformInfo.sdk = lastVersionInfo->sdk;
-  return lastVersionInfo->platform;
+  config->platformInfo = {MachO::Target(getArchitectureFromName(archName),
+                                        lastVersionInfo->platform,
+                                        lastVersionInfo->minimum),
+                          lastVersionInfo->sdk};
 }
 
 // Has the side-effect of setting Config::target.
@@ -740,14 +743,7 @@ static TargetInfo *createTargetInfo(InputArgList &args) {
     return nullptr;
   }
 
-  PlatformType platform = parsePlatformVersions(args);
-  config->platformInfo.target =
-      MachO::Target(getArchitectureFromName(archName), platform);
-  if (config->secondaryPlatformInfo) {
-    config->secondaryPlatformInfo->target =
-        MachO::Target(getArchitectureFromName(archName), PLATFORM_MACCATALYST);
-  }
-
+  setPlatformVersions(archName, args);
   auto [cpuType, cpuSubtype] = getCPUTypeFromArchitecture(config->arch());
   switch (cpuType) {
   case CPU_TYPE_X86_64:
@@ -987,7 +983,7 @@ static bool dataConstDefault(const InputArgList &args) {
   auto it = llvm::find_if(minVersion,
                           [&](const auto &p) { return p.first == platform; });
   if (it != minVersion.end())
-    if (config->platformInfo.minimum < it->second)
+    if (config->platformInfo.target.MinDeployment < it->second)
       return false;
 
   switch (config->outputType) {
@@ -1025,13 +1021,14 @@ static bool shouldEmitChainedFixups(const InputArgList &args) {
   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.minimum) {
+  if (it != minVersion.end() &&
+      it->second > config->platformInfo.target.MinDeployment) {
     if (!isRequested)
       return false;
 
     warn("-fixup_chains requires " + getPlatformName(config->platform()) + " " +
          it->second.getAsString() + ", which is newer than target minimum of " +
-         config->platformInfo.minimum.getAsString());
+         config->platformInfo.target.MinDeployment.getAsString());
   }
 
   if (!is_contained({AK_x86_64, AK_x86_64h, AK_arm64}, config->arch())) {

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index e1289103faf7a..ed0d98a2ececd 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -124,7 +124,7 @@ static std::vector<PlatformInfo> getPlatformInfos(const InputFile *input) {
   for (auto *cmd : findCommands<build_version_command>(hdr, LC_BUILD_VERSION)) {
     PlatformInfo info;
     info.target.Platform = static_cast<PlatformType>(cmd->platform);
-    info.minimum = decodeVersion(cmd->minos);
+    info.target.MinDeployment = decodeVersion(cmd->minos);
     platformInfos.emplace_back(std::move(info));
   }
   for (auto *cmd : findCommands<version_min_command>(
@@ -145,7 +145,7 @@ static std::vector<PlatformInfo> getPlatformInfos(const InputFile *input) {
       info.target.Platform = PLATFORM_WATCHOS;
       break;
     }
-    info.minimum = decodeVersion(cmd->version);
+    info.target.MinDeployment = decodeVersion(cmd->version);
     platformInfos.emplace_back(std::move(info));
   }
 
@@ -176,10 +176,11 @@ static bool checkCompatibility(const InputFile *input) {
     return false;
   }
 
-  if (it->minimum > config->platformInfo.minimum)
-    warn(toString(input) + " has version " + it->minimum.getAsString() +
+  if (it->target.MinDeployment > config->platformInfo.target.MinDeployment)
+    warn(toString(input) + " has version " +
+         it->target.MinDeployment.getAsString() +
          ", which is newer than target minimum of " +
-         config->platformInfo.minimum.getAsString());
+         config->platformInfo.target.MinDeployment.getAsString());
 
   return true;
 }
@@ -2017,8 +2018,8 @@ void DylibFile::handleLDPreviousSymbol(StringRef name, StringRef originalName) {
          originalName + "' ignored");
     return;
   }
-  if (config->platformInfo.minimum < start ||
-      config->platformInfo.minimum >= end)
+  if (config->platformInfo.target.MinDeployment < start ||
+      config->platformInfo.target.MinDeployment >= end)
     return;
 
   // Initialized to compatibilityVersion for the symbolName branch below.
@@ -2069,7 +2070,7 @@ void DylibFile::handleLDInstallNameSymbol(StringRef name,
   if (!condition.consume_front("os") || version.tryParse(condition))
     warn(toString(this) + ": failed to parse os version, symbol '" +
          originalName + "' ignored");
-  else if (version == config->platformInfo.minimum)
+  else if (version == config->platformInfo.target.MinDeployment)
     this->installName = saver().save(installName);
 }
 
@@ -2087,7 +2088,7 @@ void DylibFile::handleLDHideSymbol(StringRef name, StringRef originalName) {
            "` ignored.");
       return;
     }
-    shouldHide = versionTup == config->platformInfo.minimum;
+    shouldHide = versionTup == config->platformInfo.target.MinDeployment;
   } else {
     symbolName = name;
   }

diff  --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 8cde77d8968be..ca681eb9c90a4 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -105,7 +105,7 @@ static uint32_t cpuSubtype() {
   if (config->outputType == MH_EXECUTE && !config->staticLink &&
       target->cpuSubtype == CPU_SUBTYPE_X86_64_ALL &&
       config->platform() == PLATFORM_MACOS &&
-      config->platformInfo.minimum >= VersionTuple(10, 5))
+      config->platformInfo.target.MinDeployment >= VersionTuple(10, 5))
     subtype |= CPU_SUBTYPE_LIB64;
 
   return subtype;

diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index a09920687c552..28c01d39764a9 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -465,7 +465,7 @@ class LCMinVersion final : public LoadCommand {
       break;
     }
     c->cmdsize = getSize();
-    c->version = encodeVersion(platformInfo.minimum);
+    c->version = encodeVersion(platformInfo.target.MinDeployment);
     c->sdk = encodeVersion(platformInfo.sdk);
   }
 
@@ -490,7 +490,7 @@ class LCBuildVersion final : public LoadCommand {
     c->cmdsize = getSize();
 
     c->platform = static_cast<uint32_t>(platformInfo.target.Platform);
-    c->minos = encodeVersion(platformInfo.minimum);
+    c->minos = encodeVersion(platformInfo.target.MinDeployment);
     c->sdk = encodeVersion(platformInfo.sdk);
 
     c->ntools = ntools;
@@ -764,7 +764,9 @@ static bool useLCBuildVersion(const PlatformInfo &platformInfo) {
   auto it = llvm::find_if(minVersion, [&](const auto &p) {
     return p.first == platformInfo.target.Platform;
   });
-  return it == minVersion.end() ? true : platformInfo.minimum >= it->second;
+  return it == minVersion.end()
+             ? true
+             : platformInfo.target.MinDeployment >= it->second;
 }
 
 template <class LP> void Writer::createLoadCommands() {

diff  --git a/lld/test/MachO/invalid/incompatible-target-tapi.test b/lld/test/MachO/invalid/incompatible-target-tapi.test
index 052e854d91e55..e3c7dd6b633f4 100644
--- a/lld/test/MachO/invalid/incompatible-target-tapi.test
+++ b/lld/test/MachO/invalid/incompatible-target-tapi.test
@@ -6,5 +6,5 @@ RUN: not %lld -dylib -arch x86_64 %S/Inputs/libincompatible.tbd %t/x86_64-test.o
 RUN:   -o /dev/null 2>&1 | FileCheck %s --check-prefix=ARCH
 RUN: not %no-arg-lld -dylib -arch arm64 -platform_version ios-simulator 14.0 15.0 %t/arm64-test.o \
 RUN:   %S/Inputs/libincompatible.tbd -o /dev/null 2>&1 | FileCheck %s --check-prefix=PLATFORM
-ARCH:     error: {{.*}}libincompatible.tbd(/usr/lib/libincompatible.dylib) is incompatible with x86_64 (macOS)
-PLATFORM: error: {{.*}}libincompatible.tbd(/usr/lib/libincompatible.dylib) is incompatible with arm64 (iOS Simulator)
+ARCH:     error: {{.*}}libincompatible.tbd(/usr/lib/libincompatible.dylib) is incompatible with x86_64 (macOS11.0)
+PLATFORM: error: {{.*}}libincompatible.tbd(/usr/lib/libincompatible.dylib) is incompatible with arm64 (iOS Simulator14.0)

diff  --git a/lld/test/MachO/tapi-link.s b/lld/test/MachO/tapi-link.s
index af205b6c24e99..afe856f20cca4 100644
--- a/lld/test/MachO/tapi-link.s
+++ b/lld/test/MachO/tapi-link.s
@@ -15,9 +15,9 @@
 # RUN: env LD_DYLIB_CPU_SUBTYPES_MUST_MATCH=1 not %lld -arch x86_64h -o /dev/null -lSystem -lc++ -framework \
 # RUN:   CoreFoundation %t/libNested.tbd %t/libTlvWeak.tbd %t/test.o 2>&1 | FileCheck %s -check-prefix=INCOMPATIBLE
 
-# INCOMPATIBLE:      error: {{.*}}libSystem.tbd(/usr/lib/libSystem.dylib) is incompatible with x86_64h (macOS)
-# INCOMPATIBLE-NEXT: error: {{.*}}libc++.tbd(/usr/lib/libc++.dylib) is incompatible with x86_64h (macOS)
-# INCOMPATIBLE-NEXT: error: {{.*}}CoreFoundation.tbd(/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation) is incompatible with x86_64h (macOS)
+# INCOMPATIBLE:      error: {{.*}}libSystem.tbd(/usr/lib/libSystem.dylib) is incompatible with x86_64h (macOS11.0)
+# INCOMPATIBLE-NEXT: error: {{.*}}libc++.tbd(/usr/lib/libc++.dylib) is incompatible with x86_64h (macOS11.0)
+# INCOMPATIBLE-NEXT: error: {{.*}}CoreFoundation.tbd(/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation) is incompatible with x86_64h (macOS11.0)
 
 ## libReexportSystem.tbd tests that we can reference symbols from a 2nd-level
 ## tapi document, re-exported by a top-level tapi document, which itself is

diff  --git a/lld/test/MachO/zippered.yaml b/lld/test/MachO/zippered.yaml
index 75a43c41a750d..ae71f3a9538f6 100644
--- a/lld/test/MachO/zippered.yaml
+++ b/lld/test/MachO/zippered.yaml
@@ -12,7 +12,7 @@
 # RUN: %no-arg-lld -syslibroot %S/Inputs/MacOSX.sdk -lSystem -dylib -arch x86_64 -platform_version mac-catalyst 13.15.0 14.0 %t/test_maccatalyst.o -o /dev/null -framework MacOnly-Indirect
 
 # RUN: not %no-arg-lld -syslibroot %S/Inputs/MacOSX.sdk -lSystem -dylib -arch x86_64 -platform_version mac-catalyst 13.15.0 14.0 %t/test_maccatalyst.o -o /dev/null -framework MacOnly 2>&1 | FileCheck --check-prefix=INCOMPATIBLE %s
-# INCOMPATIBLE: System/Library/Frameworks{{[\\/]}}MacOnly.framework{{[\\/]}}MacOnly.tbd(MacOnly.dylib) is incompatible with x86_64 (macCatalyst)
+# INCOMPATIBLE: System/Library/Frameworks{{[\\/]}}MacOnly.framework{{[\\/]}}MacOnly.tbd(MacOnly.dylib) is incompatible with x86_64 (macCatalyst13.15.0)
 
 # RUN: not %no-arg-lld -syslibroot %S/Inputs/MacOSX.sdk -lSystem -dylib -arch x86_64 -platform_version ios 13.15.0 14.0 %t/test.dylib %t/test_ios.o -o /dev/null 2>&1 | FileCheck %s
 # CHECK: test.dylib has platform macOS/macCatalyst, which is 
diff erent from target platform iOS


        


More information about the llvm-commits mailing list