From 6578e0d1d0e435ab91b44bb2d40aea913044a65b Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 3 Mar 2023 12:08:33 -0800 Subject: [PATCH] [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 --- lld/MachO/Config.h | 1 - lld/MachO/Driver.cpp | 51 ++++++++++------------ lld/MachO/InputFiles.cpp | 19 ++++---- lld/MachO/SyntheticSections.cpp | 2 +- lld/MachO/Writer.cpp | 8 ++-- .../MachO/invalid/incompatible-target-tapi.test | 4 +- lld/test/MachO/tapi-link.s | 6 +-- lld/test/MachO/zippered.yaml | 2 +- 8 files changed, 46 insertions(+), 47 deletions(-) diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h index af46303..328aea49 100644 --- a/lld/MachO/Config.h +++ b/lld/MachO/Config.h @@ -42,7 +42,6 @@ using SegmentRenameMap = llvm::DenseMap; 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 121fa85..0f2326b 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 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 e128910..ed0d98a 100644 --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -124,7 +124,7 @@ static std::vector getPlatformInfos(const InputFile *input) { for (auto *cmd : findCommands(hdr, LC_BUILD_VERSION)) { PlatformInfo info; info.target.Platform = static_cast(cmd->platform); - info.minimum = decodeVersion(cmd->minos); + info.target.MinDeployment = decodeVersion(cmd->minos); platformInfos.emplace_back(std::move(info)); } for (auto *cmd : findCommands( @@ -145,7 +145,7 @@ static std::vector 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 8cde77d..ca681eb 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 a099206..28c01d3 100644 --- a/lld/MachO/Writer.cpp +++ b/lld/MachO/Writer.cpp @@ -465,7 +465,7 @@ public: break; } c->cmdsize = getSize(); - c->version = encodeVersion(platformInfo.minimum); + c->version = encodeVersion(platformInfo.target.MinDeployment); c->sdk = encodeVersion(platformInfo.sdk); } @@ -490,7 +490,7 @@ public: c->cmdsize = getSize(); c->platform = static_cast(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 void Writer::createLoadCommands() { diff --git a/lld/test/MachO/invalid/incompatible-target-tapi.test b/lld/test/MachO/invalid/incompatible-target-tapi.test index 052e854..e3c7dd6 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 af205b6c..afe856f 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 75a43c4..ae71f3a 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 different from target platform iOS -- 2.7.4