[lld-macho] Remove duplicate minimum version info

[keith]

Summary

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.

[keith]

remove redundent return

[int3]

lld/MachO/Driver.cpp

686–687

can we set both platformInfo and secondaryPlatformInfo in this function, and make it return void? It doesn't seem like return them as values helps readability + I would like to avoid std::pair as much as possible (again for readability).

remember to update the "side-effect" comment above as well :)

688

while we're here, could we change this to a DenseMap or IndexedMap?

Doesn't really matter for perf but we try to avoid std::map in general. And this might save some binary size

lld/test/MachO/tapi-link.s

20

I think we should not put NFC in the commit title. Minor change but a change nonetheless...

[keith]

update function to mutate instead of return

lld/MachO/Driver.cpp

688

it looks like both of those have new requirements for the key type, I will do it in a separate change to avoid noise in this diff

[int3]

Thanks Keith!

lld/MachO/Driver.cpp

688

ah ok. No worries either way, it's not super important

lld/MachO/Config.h

 class Symbol;
 
 using NamePair = std::pair<llvm::StringRef, llvm::StringRef>;
 using SectionRenameMap = llvm::DenseMap<NamePair, NamePair>;
 using SegmentRenameMap = llvm::DenseMap<llvm::StringRef, llvm::StringRef>;
 
 struct PlatformInfo {
   llvm::MachO::Target target;
-  llvm::VersionTuple minimum;
   llvm::VersionTuple sdk;
 };
 
 inline uint32_t encodeVersion(const llvm::VersionTuple &version) {
   return ((version.getMajor() << 020) |
           (version.getMinor().value_or(0) << 010) |
           version.getSubminor().value_or(0));
 }

lld/MachO/Driver.cpp

static PlatformVersion parsePlatformVersion(const Arg *arg) {
   if (platformVersion.minimum.tryParse(minVersionStr))
     error(Twine("malformed minimum version: ") + minVersionStr);
   if (platformVersion.sdk.tryParse(sdkVersionStr))
     error(Twine("malformed sdk version: ") + sdkVersionStr);
   return platformVersion;
 }
 
 // Has the side-effect of setting Config::platformInfo.
-static PlatformType parsePlatformVersions(const ArgList &args) {
+static std::pair<PlatformVersion, std::optional<PlatformVersion>>
+parsePlatformVersions(const ArgList &args) {
   std::map<PlatformType, PlatformVersion> platformVersions;
   const PlatformVersion *lastVersionInfo = nullptr;
   for (const Arg *arg : args.filtered(OPT_platform_version)) {
     PlatformVersion version = parsePlatformVersion(arg);
 
     // For each platform, the last flag wins:
     // `-platform_version macos 2 3 -platform_version macos 4 5` has the same
     // effect as just passing `-platform_version macos 4 5`.
     // FIXME: ld64 warns on multiple flags for one platform. Should we?
     platformVersions[version.platform] = version;
     lastVersionInfo = &platformVersions[version.platform];
   }
 
   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) &&
                               platformVersions.count(PLATFORM_MACCATALYST);
 
     if (!isZipperedCatalyst) {
       error("lld supports writing zippered outputs only for "
             "macos and mac-catalyst");
     } 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.minimum = lastVersionInfo->minimum;
-  config->platformInfo.sdk = lastVersionInfo->sdk;
-  return lastVersionInfo->platform;
+    return {{PLATFORM_MACOS, platformVersions[PLATFORM_MACOS].minimum,
+             platformVersions[PLATFORM_MACOS].sdk},
+            std::optional<PlatformVersion>(
+                {PLATFORM_MACCATALYST,
+                 platformVersions[PLATFORM_MACCATALYST].minimum,
+                 platformVersions[PLATFORM_MACCATALYST].sdk})};
+  }
+
+  return {*lastVersionInfo, std::nullopt};
 }
 
 // Has the side-effect of setting Config::target.
 static TargetInfo *createTargetInfo(InputArgList &args) {
   StringRef archName = args.getLastArgValue(OPT_arch);
   if (archName.empty()) {
     error("must specify -arch");
     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);
+  auto versions = parsePlatformVersions(args);
+  config->platformInfo = {MachO::Target(getArchitectureFromName(archName),
+                                        versions.first.platform,
+                                        versions.first.minimum),
+                          versions.first.sdk};
+  if (versions.second) {
+    config->secondaryPlatformInfo = {
+        MachO::Target(getArchitectureFromName(archName), PLATFORM_MACCATALYST,
+                      versions.second->minimum),
+        versions.second->sdk};
   }
 
   auto [cpuType, cpuSubtype] = getCPUTypeFromArchitecture(config->arch());
   switch (cpuType) {
   case CPU_TYPE_X86_64:
     return createX86_64TargetInfo();
   case CPU_TYPE_ARM64:
     return createARM64TargetInfo();
static const std::array<std::pair<PlatformType, VersionTuple>, 5> minVersion =
         {PLATFORM_IOS, VersionTuple(13, 0)},
         {PLATFORM_TVOS, VersionTuple(13, 0)},
         {PLATFORM_WATCHOS, VersionTuple(6, 0)},
         {PLATFORM_BRIDGEOS, VersionTuple(4, 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())
-    if (config->platformInfo.minimum < it->second)
+    if (config->platformInfo.target.MinDeployment < it->second)
       return false;
 
   switch (config->outputType) {
   case MH_EXECUTE:
     return !(args.hasArg(OPT_no_pie) && supportsNoPie());
   case MH_BUNDLE:
     // FIXME: return false when -final_name ...
     // has prefix "/System/Library/UserEventPlugins/"
static bool shouldEmitChainedFixups(const InputArgList &args) {
   static const std::array<std::pair<PlatformType, VersionTuple>, 4> minVersion =
       {{{PLATFORM_MACOS, VersionTuple(11, 0)},
         {PLATFORM_IOS, VersionTuple(13, 4)},
         {PLATFORM_TVOS, VersionTuple(14, 0)},
         {PLATFORM_WATCHOS, VersionTuple(7, 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.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())) {
     if (isRequested)
       error("-fixup_chains is only supported on x86_64 and arm64 targets");
     return false;
   }
 

lld/MachO/InputFiles.cpp

static std::vector<PlatformInfo> getPlatformInfos(const InputFile *input) {
 
   const char *hdr = input->mb.getBufferStart();
 
   // "Zippered" object files can have multiple LC_BUILD_VERSION load commands.
   std::vector<PlatformInfo> platformInfos;
   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>(
            hdr, LC_VERSION_MIN_MACOSX, LC_VERSION_MIN_IPHONEOS,
            LC_VERSION_MIN_TVOS, LC_VERSION_MIN_WATCHOS)) {
     PlatformInfo info;
     switch (cmd->cmd) {
     case LC_VERSION_MIN_MACOSX:
       info.target.Platform = PLATFORM_MACOS;
       break;
     case LC_VERSION_MIN_IPHONEOS:
       info.target.Platform = PLATFORM_IOS;
       break;
     case LC_VERSION_MIN_TVOS:
       info.target.Platform = PLATFORM_TVOS;
       break;
     case LC_VERSION_MIN_WATCHOS:
       info.target.Platform = PLATFORM_WATCHOS;
       break;
     }
-    info.minimum = decodeVersion(cmd->version);
+    info.target.MinDeployment = decodeVersion(cmd->version);
     platformInfos.emplace_back(std::move(info));
   }
 
   return platformInfos;
 }
 
 static bool checkCompatibility(const InputFile *input) {
   std::vector<PlatformInfo> platformInfos = getPlatformInfos(input);
interleave(
         },
         "/");
     error(toString(input) + " has platform " + platformNames +
           Twine(", which is different from target platform ") +
           getPlatformName(config->platform()));
     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;
 }
 
 // This cache mostly exists to store system libraries (and .tbds) as they're
 // loaded, rather than the input archives, which are already cached at a higher
 // level, and other files like the filelist that are only read once.
 // Theoretically this caching could be more efficient by hoisting it, but that
if (start.tryParse(startVersion)) {
     return;
   }
   VersionTuple end;
   if (end.tryParse(endVersion)) {
     warn(toString(this) + ": failed to parse end version, symbol '" +
          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.
   uint32_t newCompatibilityVersion = compatibilityVersion;
   uint32_t newCurrentVersionForSymbol = currentVersion;
   if (!compatVersion.empty()) {
     VersionTuple cVersion;
     if (cVersion.tryParse(compatVersion)) {
void DylibFile::handleLDInstallNameSymbol(StringRef name,
                                           StringRef originalName) {
   // originalName: $ld$ install_name $ os<version> $ install_name
   StringRef condition, installName;
   std::tie(condition, installName) = name.split('$');
   VersionTuple version;
   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);
 }
 
 void DylibFile::handleLDHideSymbol(StringRef name, StringRef originalName) {
   StringRef symbolName;
   bool shouldHide = true;
   if (name.startswith("os")) {
     // If it's hidden based on versions.
     name = name.drop_front(2);
     StringRef minVersion;
     std::tie(minVersion, symbolName) = name.split('$');
     VersionTuple versionTup;
     if (versionTup.tryParse(minVersion)) {
       warn(toString(this) + ": failed to parse hidden version, symbol `" + originalName +
            "` ignored.");
       return;
     }
-    shouldHide = versionTup == config->platformInfo.minimum;
+    shouldHide = versionTup == config->platformInfo.target.MinDeployment;
   } else {
     symbolName = name;
   }
 
   if (shouldHide)
     exportingFile->hiddenSymbols.insert(CachedHashStringRef(symbolName));
 }
 

lld/MachO/SyntheticSections.cpp

 }
 
 static uint32_t cpuSubtype() {
   uint32_t subtype = target->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;
 }
 
 static bool hasWeakBinding() {
   return config->emitChainedFixups ? in.chainedFixups->hasWeakBinding()
                                    : in.weakBinding->hasEntry();

lld/MachO/Writer.cpp

void writeTo(uint8_t *buf) const override {
     case PLATFORM_WATCHOSSIMULATOR:
       c->cmd = LC_VERSION_MIN_WATCHOS;
       break;
     default:
       llvm_unreachable("invalid platform");
       break;
     }
     c->cmdsize = getSize();
-    c->version = encodeVersion(platformInfo.minimum);
+    c->version = encodeVersion(platformInfo.target.MinDeployment);
     c->sdk = encodeVersion(platformInfo.sdk);
   }
 
 private:
   const PlatformInfo &platformInfo;
 };
 
 class LCBuildVersion final : public LoadCommand {
 public:
   explicit LCBuildVersion(const PlatformInfo &platformInfo)
       : platformInfo(platformInfo) {}
 
   const int ntools = 1;
 
   uint32_t getSize() const override {
     return sizeof(build_version_command) + ntools * sizeof(build_tool_version);
   }
 
   void writeTo(uint8_t *buf) const override {
     auto *c = reinterpret_cast<build_version_command *>(buf);
     c->cmd = LC_BUILD_VERSION;
     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;
     auto *t = reinterpret_cast<build_tool_version *>(&c[1]);
     t->tool = TOOL_LD;
     t->version = encodeVersion(VersionTuple(
         LLVM_VERSION_MAJOR, LLVM_VERSION_MINOR, LLVM_VERSION_PATCH));
   }
static const std::array<std::pair<PlatformType, VersionTuple>, 7> minVersion =
         {PLATFORM_IOSSIMULATOR, VersionTuple(13, 0)},
         {PLATFORM_TVOS, VersionTuple(12, 0)},
         {PLATFORM_TVOSSIMULATOR, VersionTuple(13, 0)},
         {PLATFORM_WATCHOS, VersionTuple(5, 0)},
         {PLATFORM_WATCHOSSIMULATOR, VersionTuple(6, 0)}}};
   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() {
   uint8_t segIndex = 0;
   for (OutputSegment *seg : outputSegments) {
     in.header->addLoadCommand(make<LCSegment<LP>>(seg->name, seg));
     seg->index = segIndex++;
   }

lld/test/MachO/invalid/incompatible-target-tapi.test

 REQUIRES: x86, aarch64
 RUN: rm -rf %t; mkdir -p %t
 RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-macosx -o %t/x86_64-test.o
 RUN: echo "" | llvm-mc -filetype=obj -triple=arm64-apple-iossimulator -o %t/arm64-test.o
 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)

lld/test/MachO/tapi-link.s

 ## Targeting an arch not listed in the tbd should fallback to an ABI compatible arch
 # RUN: %lld -arch x86_64h -o %t/test-compat -lSystem -lc++ -framework CoreFoundation %t/libNested.tbd %t/libTlvWeak.tbd %t/test.o
 # RUN: llvm-objdump --bind --weak-bind --no-show-raw-insn -d -r %t/test-compat | FileCheck %s
 
 ## Setting LD_DYLIB_CPU_SUBTYPES_MUST_MATCH forces exact target arch match.
 # 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
 ## re-exported by another top-level tapi document.
 # RUN: %lld -o %t/with-reexport -lSystem -L%t %t/libReexportNested.tbd %t/libTlvWeak.tbd -lc++ -framework CoreFoundation %t/test.o
 # RUN: llvm-objdump --bind --weak-bind --no-show-raw-insn -d -r %t/with-reexport | FileCheck %s
 
 # CHECK: Bind table:

lld/test/MachO/zippered.yaml

 # REQUIRES: x86
 # RUN: rm -rf %t; mkdir %t
 # RUN: yaml2obj %s > %t/test.dylib
 # RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-macos10.15 -o %t/test_macos.o
 # RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-ios13.15.0-macabi -o %t/test_maccatalyst.o
 # RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-ios13.15.0 -o %t/test_ios.o
 
 ## Test linking against a zippered dylib.
 # RUN: %lld -lSystem -dylib %t/test.dylib %t/test_macos.o -o /dev/null
 # 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.dylib %t/test_maccatalyst.o -o /dev/null
 
 # 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 different from target platform iOS
 
 ## Test creating a zippered dylib.
 # RUN: %no-arg-lld -syslibroot %S/Inputs/MacOSX.sdk -lSystem -dylib -arch x86_64 -platform_version macos 12.0 13.0 -platform_version mac-catalyst 13.15.0 14.0 %t/test.dylib %t/test_macos.o -o %t/test_zippered.dylib
 # RUN: llvm-otool -l %t/test_zippered.dylib | FileCheck --check-prefix=ZIPPERED %s
 # ZIPPERED:           cmd LC_BUILD_VERSION