From 3f0eb8ce2c310933f8667580cdfe7fcf76dc3c59 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Fri, 20 Dec 2024 16:10:16 -0800 Subject: [PATCH] Frontend: Fix -target-variant subarch normalization. In https://github.com/swiftlang/swift/pull/77156, normalization was introduced for -target-variant triples. That PR also caused -target-variant arguments to be inherited from the main compilation options whenever building dependency modules from their interfaces, which is incorrect. The -target-variant option must only be specified when compiling a "zippered" module, but the dependencies of zippered modules are not necessarily zippered themselves and indiscriminantly propagating the option can cause miscompilation. The new, more targeted approach to normalizing arm64e triples simply uses the arch and subarch of the -target argument of the main compile to decide whether the subarch of both the -target and -target-variant arguments of a dependency need adjustment. Resolves rdar://135322077 and rdar://141640919. --- .../Serialization/SerializedModuleLoader.h | 3 +- lib/Frontend/ModuleInterfaceLoader.cpp | 21 ++----- lib/Serialization/SerializedModuleLoader.cpp | 37 +++++------- .../arm64e-apple-ios-macabi.swiftinterface | 4 ++ .../arm64e-apple-macos.swiftinterface | 4 ++ .../arm64e-apple-ios-macabi.swiftinterface | 4 ++ .../arm64e-apple-macos.swiftinterface | 4 ++ .../target_normalization.swift | 56 ++++++++++++++++++ .../target_normalization_maccatalyst.swift | 55 ++++++++++++++++++ ...t_normalization_maccatalyst_zippered.swift | 57 +++++++++++++++++++ 10 files changed, 203 insertions(+), 42 deletions(-) create mode 100644 test/ScanDependencies/Inputs/target-normalization/iOSSupport/Unzippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface create mode 100644 test/ScanDependencies/Inputs/target-normalization/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface create mode 100644 test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface create mode 100644 test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface create mode 100644 test/ScanDependencies/target_normalization.swift create mode 100644 test/ScanDependencies/target_normalization_maccatalyst.swift create mode 100644 test/ScanDependencies/target_normalization_maccatalyst_zippered.swift diff --git a/include/swift/Serialization/SerializedModuleLoader.h b/include/swift/Serialization/SerializedModuleLoader.h index f2ecb65968dc..a80479f9589b 100644 --- a/include/swift/Serialization/SerializedModuleLoader.h +++ b/include/swift/Serialization/SerializedModuleLoader.h @@ -573,8 +573,7 @@ class SerializedASTFile final : public LoadedFile { bool extractCompilerFlagsFromInterface( StringRef interfacePath, StringRef buffer, llvm::StringSaver &ArgSaver, SmallVectorImpl &SubArgs, - std::optional PreferredTarget = std::nullopt, - std::optional PreferredTargetVariant = std::nullopt); + std::optional PreferredTarget = std::nullopt); /// Extract the user module version number from an interface file. llvm::VersionTuple extractUserModuleVersionFromInterface(StringRef moduleInterfacePath); diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp index 21ca9b2a4fce..9516a6e027e1 100644 --- a/lib/Frontend/ModuleInterfaceLoader.cpp +++ b/lib/Frontend/ModuleInterfaceLoader.cpp @@ -1513,8 +1513,7 @@ bool ModuleInterfaceLoader::buildSwiftModuleFromSwiftInterface( static bool readSwiftInterfaceVersionAndArgs( SourceManager &SM, DiagnosticEngine &Diags, llvm::StringSaver &ArgSaver, SwiftInterfaceInfo &interfaceInfo, StringRef interfacePath, - SourceLoc diagnosticLoc, llvm::Triple preferredTarget, - std::optional preferredTargetVariant) { + SourceLoc diagnosticLoc, llvm::Triple preferredTarget) { llvm::vfs::FileSystem &fs = *SM.getFileSystem(); auto FileOrError = swift::vfs::getFileOrSTDIN(fs, interfacePath); if (!FileOrError) { @@ -1538,8 +1537,7 @@ static bool readSwiftInterfaceVersionAndArgs( if (extractCompilerFlagsFromInterface(interfacePath, SB, ArgSaver, interfaceInfo.Arguments, - preferredTarget, - preferredTargetVariant)) { + preferredTarget)) { InterfaceSubContextDelegateImpl::diagnose( interfacePath, diagnosticLoc, SM, &Diags, diag::error_extracting_version_from_module_interface); @@ -1623,8 +1621,7 @@ bool ModuleInterfaceLoader::buildExplicitSwiftModuleFromSwiftInterface( readSwiftInterfaceVersionAndArgs( Instance.getSourceMgr(), Instance.getDiags(), ArgSaver, InterfaceInfo, interfacePath, SourceLoc(), - Instance.getInvocation().getLangOptions().Target, - Instance.getInvocation().getLangOptions().TargetVariant); + Instance.getInvocation().getLangOptions().Target); auto Builder = ExplicitModuleInterfaceBuilder( Instance, &Instance.getDiags(), Instance.getSourceMgr(), @@ -1673,15 +1670,6 @@ void InterfaceSubContextDelegateImpl::inheritOptionsForBuildingInterface( GenericArgs.push_back(triple); } - if (LangOpts.TargetVariant.has_value()) { - genericSubInvocation.getLangOptions().TargetVariant = LangOpts.TargetVariant; - auto variantTriple = ArgSaver.save(genericSubInvocation.getLangOptions().TargetVariant->str()); - if (!variantTriple.empty()) { - GenericArgs.push_back("-target-variant"); - GenericArgs.push_back(variantTriple); - } - } - // Inherit the target SDK name and version if (!LangOpts.SDKName.empty()) { genericSubInvocation.getLangOptions().SDKName = LangOpts.SDKName; @@ -1821,8 +1809,7 @@ bool InterfaceSubContextDelegateImpl::extractSwiftInterfaceVersionAndArgs( StringRef interfacePath, SourceLoc diagnosticLoc) { if (readSwiftInterfaceVersionAndArgs(SM, *Diags, ArgSaver, interfaceInfo, interfacePath, diagnosticLoc, - subInvocation.getLangOptions().Target, - subInvocation.getLangOptions().TargetVariant)) + subInvocation.getLangOptions().Target)) return true; // Prior to Swift 5.9, swiftinterfaces were always built (accidentally) with diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index 13ad96e339e8..b72df8d30ef5 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -1390,14 +1390,6 @@ void swift::serialization::diagnoseSerializedASTLoadFailureTransitive( } } -static bool tripleNeedsSubarchitectureAdjustment(const llvm::Triple &lhs, const llvm::Triple &rhs) { - return (lhs.getSubArch() != rhs.getSubArch() && - lhs.getArch() == rhs.getArch() && - lhs.getVendor() == rhs.getVendor() && - lhs.getOS() == rhs.getOS() && - lhs.getEnvironment() == rhs.getEnvironment()); -} - static std::optional getFlagsFromInterfaceFile(StringRef &file, StringRef prefix) { StringRef line, buffer = file; @@ -1419,8 +1411,7 @@ static std::optional getFlagsFromInterfaceFile(StringRef &file, bool swift::extractCompilerFlagsFromInterface( StringRef interfacePath, StringRef buffer, llvm::StringSaver &ArgSaver, SmallVectorImpl &SubArgs, - std::optional PreferredTarget, - std::optional PreferredTargetVariant) { + std::optional PreferredTarget) { auto FlagMatch = getFlagsFromInterfaceFile(buffer, SWIFT_MODULE_FLAGS_KEY); if (!FlagMatch) return true; @@ -1430,19 +1421,19 @@ bool swift::extractCompilerFlagsFromInterface( // only in subarchitecture from the compatible target triple, then // we have loaded a Swift interface from a different-but-compatible // architecture slice. Use the compatible subarchitecture. - for (unsigned I = 1; I < SubArgs.size(); ++I) { - if (strcmp(SubArgs[I - 1], "-target") == 0) { - llvm::Triple target(SubArgs[I]); - if (PreferredTarget && - tripleNeedsSubarchitectureAdjustment(target, *PreferredTarget)) - target.setArch(PreferredTarget->getArch(), PreferredTarget->getSubArch()); - SubArgs[I] = ArgSaver.save(target.str()).data(); - } else if (strcmp(SubArgs[I - 1], "-target-variant") == 0) { - llvm::Triple targetVariant(SubArgs[I]); - if (PreferredTargetVariant && - tripleNeedsSubarchitectureAdjustment(targetVariant, *PreferredTargetVariant)) - targetVariant.setArch(PreferredTargetVariant->getArch(), PreferredTargetVariant->getSubArch()); - SubArgs[I] = ArgSaver.save(targetVariant.str()).data(); + if (PreferredTarget) { + for (unsigned I = 1; I < SubArgs.size(); ++I) { + if (strcmp(SubArgs[I - 1], "-target") != 0 && + strcmp(SubArgs[I - 1], "-target-variant") != 0) + continue; + + llvm::Triple triple(SubArgs[I]); + if (triple.getArch() != PreferredTarget->getArch()) + continue; + if (triple.getSubArch() == PreferredTarget->getSubArch()) + continue; + triple.setArch(PreferredTarget->getArch(), PreferredTarget->getSubArch()); + SubArgs[I] = ArgSaver.save(triple.str()).data(); } } diff --git a/test/ScanDependencies/Inputs/target-normalization/iOSSupport/Unzippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface b/test/ScanDependencies/Inputs/target-normalization/iOSSupport/Unzippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface new file mode 100644 index 000000000000..0da3ed775351 --- /dev/null +++ b/test/ScanDependencies/Inputs/target-normalization/iOSSupport/Unzippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface @@ -0,0 +1,4 @@ +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name Unzippered -target arm64e-apple-ios15.0-macabi +import Swift +public func unzipperedFunc() { } diff --git a/test/ScanDependencies/Inputs/target-normalization/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface b/test/ScanDependencies/Inputs/target-normalization/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface new file mode 100644 index 000000000000..27ef27ba7dc7 --- /dev/null +++ b/test/ScanDependencies/Inputs/target-normalization/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface @@ -0,0 +1,4 @@ +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name Unzippered -target arm64e-apple-macosx12.0 +import Swift +public func unzipperedFunc() { } diff --git a/test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface b/test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface new file mode 100644 index 000000000000..1ff03e9cfb03 --- /dev/null +++ b/test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface @@ -0,0 +1,4 @@ +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name Zippered -target arm64e-apple-ios15.0-macabi +import Swift +public func zipperedFunc() { } diff --git a/test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface b/test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface new file mode 100644 index 000000000000..1d01553aa4fd --- /dev/null +++ b/test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface @@ -0,0 +1,4 @@ +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name Zippered -target arm64e-apple-macosx12.0 -target-variant arm64e-apple-ios15.0-macabi +import Swift +public func zipperedFunc() { } diff --git a/test/ScanDependencies/target_normalization.swift b/test/ScanDependencies/target_normalization.swift new file mode 100644 index 000000000000..2e0245d1a259 --- /dev/null +++ b/test/ScanDependencies/target_normalization.swift @@ -0,0 +1,56 @@ +// REQUIRES: OS=macosx + +// RUN: %empty-directory(%t) +// RUN: %empty-directory(%t/module-cache) + +// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \ +// RUN: -module-cache-path %t/module-cache \ +// RUN: -I %S/Inputs/target-normalization/iOSSupport \ +// RUN: -I %S/Inputs/target-normalization/macOS \ +// RUN: -target arm64-apple-macosx14.0 \ +// RUN: -o %t/deps-arm64-apple-macosx.json + +// RUN: %validate-json %t/deps-arm64-apple-macosx.json +// RUN: %FileCheck %s --input-file %t/deps-arm64-apple-macosx.json \ +// RUN: -DORIG_ARCH=arm64 \ +// RUN: -DNORM_ARCH=aarch64 + +// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \ +// RUN: -module-cache-path %t/module-cache \ +// RUN: -I %S/Inputs/target-normalization/iOSSupport \ +// RUN: -I %S/Inputs/target-normalization/macOS \ +// RUN: -target arm64e-apple-macosx14.0 \ +// RUN: -o %t/deps-arm64e-apple-macosx.json + +// RUN: %validate-json %t/deps-arm64e-apple-macosx.json +// RUN: %FileCheck %s --input-file %t/deps-arm64e-apple-macosx.json \ +// RUN: -DORIG_ARCH=arm64e \ +// RUN: -DNORM_ARCH=arm64e + +import Zippered +import Unzippered + +// CHECK: "modulePath": "{{.*}}Unzippered-[[UNZIPPERED_HASH:[A-Z0-9]+]].swiftmodule", +// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface", +// CHECK: "commandLine" +// CHECK: "-compile-module-from-interface", +// CHECK: "-target", +// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0" +// CHECK-NOT: "-target-variant" +// CHECK: "-target", +// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0" +// CHECK-NOT: "-target-variant" +// CHECK: "contextHash": "[[UNZIPPERED_HASH]]", + +// CHECK: "modulePath": "{{.*}}Zippered-[[ZIPPERED_HASH:[A-Z0-9]+]].swiftmodule", +// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface", +// CHECK: "commandLine" +// CHECK: "-compile-module-from-interface", +// CHECK: "-target", +// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0" +// CHECK-NOT: "-target-variant" +// CHECK: "-target", +// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0" +// CHECK: "-target-variant" +// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi", +// CHECK: "contextHash": "[[ZIPPERED_HASH]]", diff --git a/test/ScanDependencies/target_normalization_maccatalyst.swift b/test/ScanDependencies/target_normalization_maccatalyst.swift new file mode 100644 index 000000000000..ae76f6fadbec --- /dev/null +++ b/test/ScanDependencies/target_normalization_maccatalyst.swift @@ -0,0 +1,55 @@ +// REQUIRES: OS=macosx + +// RUN: %empty-directory(%t) +// RUN: %empty-directory(%t/module-cache) + +// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \ +// RUN: -module-cache-path %t/module-cache \ +// RUN: -I %S/Inputs/target-normalization/iOSSupport \ +// RUN: -I %S/Inputs/target-normalization/macOS \ +// RUN: -target arm64-apple-ios15.0-macabi \ +// RUN: -o %t/deps-arm64-apple-ios-macabi.json + +// RUN: %validate-json %t/deps-arm64-apple-ios-macabi.json +// RUN: %FileCheck %s --input-file %t/deps-arm64-apple-ios-macabi.json \ +// RUN: -DORIG_ARCH=arm64 \ +// RUN: -DNORM_ARCH=aarch64 + +// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \ +// RUN: -module-cache-path %t/module-cache \ +// RUN: -I %S/Inputs/target-normalization/iOSSupport \ +// RUN: -I %S/Inputs/target-normalization/macOS \ +// RUN: -target arm64e-apple-ios15.0-macabi \ +// RUN: -o %t/deps-arm64e-apple-ios-macabi.json + +// RUN: %validate-json %t/deps-arm64e-apple-ios-macabi.json +// RUN: %FileCheck %s --input-file %t/deps-arm64e-apple-ios-macabi.json \ +// RUN: -DORIG_ARCH=arm64e \ +// RUN: -DNORM_ARCH=arm64e + +import Zippered +import Unzippered + +// CHECK: "modulePath": "{{.*}}Unzippered-[[UNZIPPERED_HASH:[A-Z0-9]+]].swiftmodule", +// CHECK: "moduleInterfacePath": "{{.*}}/iOSSupport/Unzippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface", +// CHECK: "commandLine" +// CHECK: "-compile-module-from-interface", +// CHECK: "-target", +// CHECK-NEXT: "[[ORIG_ARCH]]-apple-ios15.0-macabi" +// CHECK-NOT: "-target-variant" +// CHECK: "-target", +// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi" +// CHECK-NOT: "-target-variant" +// CHECK: "contextHash": "[[UNZIPPERED_HASH]]", + +// CHECK: "modulePath": "{{.*}}Zippered-[[ZIPPERED_HASH:[A-Z0-9]+]].swiftmodule", +// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Zippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface", +// CHECK: "commandLine" +// CHECK: "-compile-module-from-interface", +// CHECK: "-target", +// CHECK-NEXT: "[[ORIG_ARCH]]-apple-ios15.0-macabi" +// CHECK-NOT: "-target-variant" +// CHECK: "-target", +// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi" +// CHECK-NOT: "-target-variant" +// CHECK: "contextHash": "[[ZIPPERED_HASH]]", diff --git a/test/ScanDependencies/target_normalization_maccatalyst_zippered.swift b/test/ScanDependencies/target_normalization_maccatalyst_zippered.swift new file mode 100644 index 000000000000..e3fbf6423a4f --- /dev/null +++ b/test/ScanDependencies/target_normalization_maccatalyst_zippered.swift @@ -0,0 +1,57 @@ +// REQUIRES: OS=macosx + +// RUN: %empty-directory(%t) +// RUN: %empty-directory(%t/module-cache) + +// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \ +// RUN: -module-cache-path %t/module-cache \ +// RUN: -I %S/Inputs/target-normalization/iOSSupport \ +// RUN: -I %S/Inputs/target-normalization/macOS \ +// RUN: -target arm64-apple-macosx14.0 \ +// RUN: -target-variant arm64-apple-ios15.0-macabi \ +// RUN: -o %t/deps-arm64-apple-macosx.json + +// RUN: %validate-json %t/deps-arm64-apple-macosx.json +// RUN: %FileCheck %s --input-file %t/deps-arm64-apple-macosx.json \ +// RUN: -DORIG_ARCH=arm64 \ +// RUN: -DNORM_ARCH=aarch64 + +// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \ +// RUN: -module-cache-path %t/module-cache \ +// RUN: -I %S/Inputs/target-normalization/iOSSupport \ +// RUN: -I %S/Inputs/target-normalization/macOS \ +// RUN: -target arm64e-apple-macosx14.0 \ +// RUN: -target-variant arm64e-apple-ios15.0-macabi \ +// RUN: -o %t/deps-arm64e-apple-macosx.json + +// RUN: %validate-json %t/deps-arm64e-apple-macosx.json +// RUN: %FileCheck %s --input-file %t/deps-arm64e-apple-macosx.json \ +// RUN: -DORIG_ARCH=arm64e \ +// RUN: -DNORM_ARCH=arm64e + +import Zippered +import Unzippered + +// CHECK: "modulePath": "{{.*}}Unzippered-[[UNZIPPERED_HASH:[A-Z0-9]+]].swiftmodule", +// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface", +// CHECK: "commandLine" +// CHECK: "-compile-module-from-interface", +// CHECK: "-target", +// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0" +// CHECK-NOT: "-target-variant" +// CHECK: "-target", +// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0" +// CHECK-NOT: "-target-variant" +// CHECK: "contextHash": "[[UNZIPPERED_HASH]]", + +// CHECK: "modulePath": "{{.*}}Zippered-[[ZIPPERED_HASH:[A-Z0-9]+]].swiftmodule", +// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface", +// CHECK: "commandLine" +// CHECK: "-compile-module-from-interface", +// CHECK: "-target", +// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0" +// CHECK: "-target", +// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0" +// CHECK: "-target-variant", +// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi", +// CHECK: "contextHash": "[[ZIPPERED_HASH]]",