Skip to content

Commit 3f0eb8c

Browse files
tshortlitshortli
committed
Frontend: Fix -target-variant subarch normalization.
In swiftlang#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.
1 parent 0b6ab9a commit 3f0eb8c

File tree

10 files changed

+203
-42
lines changed

10 files changed

+203
-42
lines changed

‎include/swift/Serialization/SerializedModuleLoader.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,8 +573,7 @@ class SerializedASTFile final : public LoadedFile {
573573
bool extractCompilerFlagsFromInterface(
574574
StringRef interfacePath, StringRef buffer, llvm::StringSaver &ArgSaver,
575575
SmallVectorImpl<const char *> &SubArgs,
576-
std::optional<llvm::Triple> PreferredTarget = std::nullopt,
577-
std::optional<llvm::Triple> PreferredTargetVariant = std::nullopt);
576+
std::optional<llvm::Triple> PreferredTarget = std::nullopt);
578577

579578
/// Extract the user module version number from an interface file.
580579
llvm::VersionTuple extractUserModuleVersionFromInterface(StringRef moduleInterfacePath);

‎lib/Frontend/ModuleInterfaceLoader.cpp‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,8 +1513,7 @@ bool ModuleInterfaceLoader::buildSwiftModuleFromSwiftInterface(
15131513
static bool readSwiftInterfaceVersionAndArgs(
15141514
SourceManager &SM, DiagnosticEngine &Diags, llvm::StringSaver &ArgSaver,
15151515
SwiftInterfaceInfo &interfaceInfo, StringRef interfacePath,
1516-
SourceLoc diagnosticLoc, llvm::Triple preferredTarget,
1517-
std::optional<llvm::Triple> preferredTargetVariant) {
1516+
SourceLoc diagnosticLoc, llvm::Triple preferredTarget) {
15181517
llvm::vfs::FileSystem &fs = *SM.getFileSystem();
15191518
auto FileOrError = swift::vfs::getFileOrSTDIN(fs, interfacePath);
15201519
if (!FileOrError) {
@@ -1538,8 +1537,7 @@ static bool readSwiftInterfaceVersionAndArgs(
15381537

15391538
if (extractCompilerFlagsFromInterface(interfacePath, SB, ArgSaver,
15401539
interfaceInfo.Arguments,
1541-
preferredTarget,
1542-
preferredTargetVariant)) {
1540+
preferredTarget)) {
15431541
InterfaceSubContextDelegateImpl::diagnose(
15441542
interfacePath, diagnosticLoc, SM, &Diags,
15451543
diag::error_extracting_version_from_module_interface);
@@ -1623,8 +1621,7 @@ bool ModuleInterfaceLoader::buildExplicitSwiftModuleFromSwiftInterface(
16231621
readSwiftInterfaceVersionAndArgs(
16241622
Instance.getSourceMgr(), Instance.getDiags(), ArgSaver, InterfaceInfo,
16251623
interfacePath, SourceLoc(),
1626-
Instance.getInvocation().getLangOptions().Target,
1627-
Instance.getInvocation().getLangOptions().TargetVariant);
1624+
Instance.getInvocation().getLangOptions().Target);
16281625

16291626
auto Builder = ExplicitModuleInterfaceBuilder(
16301627
Instance, &Instance.getDiags(), Instance.getSourceMgr(),
@@ -1673,15 +1670,6 @@ void InterfaceSubContextDelegateImpl::inheritOptionsForBuildingInterface(
16731670
GenericArgs.push_back(triple);
16741671
}
16751672

1676-
if (LangOpts.TargetVariant.has_value()) {
1677-
genericSubInvocation.getLangOptions().TargetVariant = LangOpts.TargetVariant;
1678-
auto variantTriple = ArgSaver.save(genericSubInvocation.getLangOptions().TargetVariant->str());
1679-
if (!variantTriple.empty()) {
1680-
GenericArgs.push_back("-target-variant");
1681-
GenericArgs.push_back(variantTriple);
1682-
}
1683-
}
1684-
16851673
// Inherit the target SDK name and version
16861674
if (!LangOpts.SDKName.empty()) {
16871675
genericSubInvocation.getLangOptions().SDKName = LangOpts.SDKName;
@@ -1821,8 +1809,7 @@ bool InterfaceSubContextDelegateImpl::extractSwiftInterfaceVersionAndArgs(
18211809
StringRef interfacePath, SourceLoc diagnosticLoc) {
18221810
if (readSwiftInterfaceVersionAndArgs(SM, *Diags, ArgSaver, interfaceInfo,
18231811
interfacePath, diagnosticLoc,
1824-
subInvocation.getLangOptions().Target,
1825-
subInvocation.getLangOptions().TargetVariant))
1812+
subInvocation.getLangOptions().Target))
18261813
return true;
18271814

18281815
// Prior to Swift 5.9, swiftinterfaces were always built (accidentally) with

‎lib/Serialization/SerializedModuleLoader.cpp‎

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,14 +1390,6 @@ void swift::serialization::diagnoseSerializedASTLoadFailureTransitive(
13901390
}
13911391
}
13921392

1393-
static bool tripleNeedsSubarchitectureAdjustment(const llvm::Triple &lhs, const llvm::Triple &rhs) {
1394-
return (lhs.getSubArch() != rhs.getSubArch() &&
1395-
lhs.getArch() == rhs.getArch() &&
1396-
lhs.getVendor() == rhs.getVendor() &&
1397-
lhs.getOS() == rhs.getOS() &&
1398-
lhs.getEnvironment() == rhs.getEnvironment());
1399-
}
1400-
14011393
static std::optional<StringRef> getFlagsFromInterfaceFile(StringRef &file,
14021394
StringRef prefix) {
14031395
StringRef line, buffer = file;
@@ -1419,8 +1411,7 @@ static std::optional<StringRef> getFlagsFromInterfaceFile(StringRef &file,
14191411
bool swift::extractCompilerFlagsFromInterface(
14201412
StringRef interfacePath, StringRef buffer, llvm::StringSaver &ArgSaver,
14211413
SmallVectorImpl<const char *> &SubArgs,
1422-
std::optional<llvm::Triple> PreferredTarget,
1423-
std::optional<llvm::Triple> PreferredTargetVariant) {
1414+
std::optional<llvm::Triple> PreferredTarget) {
14241415
auto FlagMatch = getFlagsFromInterfaceFile(buffer, SWIFT_MODULE_FLAGS_KEY);
14251416
if (!FlagMatch)
14261417
return true;
@@ -1430,19 +1421,19 @@ bool swift::extractCompilerFlagsFromInterface(
14301421
// only in subarchitecture from the compatible target triple, then
14311422
// we have loaded a Swift interface from a different-but-compatible
14321423
// architecture slice. Use the compatible subarchitecture.
1433-
for (unsigned I = 1; I < SubArgs.size(); ++I) {
1434-
if (strcmp(SubArgs[I - 1], "-target") == 0) {
1435-
llvm::Triple target(SubArgs[I]);
1436-
if (PreferredTarget &&
1437-
tripleNeedsSubarchitectureAdjustment(target, *PreferredTarget))
1438-
target.setArch(PreferredTarget->getArch(), PreferredTarget->getSubArch());
1439-
SubArgs[I] = ArgSaver.save(target.str()).data();
1440-
} else if (strcmp(SubArgs[I - 1], "-target-variant") == 0) {
1441-
llvm::Triple targetVariant(SubArgs[I]);
1442-
if (PreferredTargetVariant &&
1443-
tripleNeedsSubarchitectureAdjustment(targetVariant, *PreferredTargetVariant))
1444-
targetVariant.setArch(PreferredTargetVariant->getArch(), PreferredTargetVariant->getSubArch());
1445-
SubArgs[I] = ArgSaver.save(targetVariant.str()).data();
1424+
if (PreferredTarget) {
1425+
for (unsigned I = 1; I < SubArgs.size(); ++I) {
1426+
if (strcmp(SubArgs[I - 1], "-target") != 0 &&
1427+
strcmp(SubArgs[I - 1], "-target-variant") != 0)
1428+
continue;
1429+
1430+
llvm::Triple triple(SubArgs[I]);
1431+
if (triple.getArch() != PreferredTarget->getArch())
1432+
continue;
1433+
if (triple.getSubArch() == PreferredTarget->getSubArch())
1434+
continue;
1435+
triple.setArch(PreferredTarget->getArch(), PreferredTarget->getSubArch());
1436+
SubArgs[I] = ArgSaver.save(triple.str()).data();
14461437
}
14471438
}
14481439

‎test/ScanDependencies/Inputs/target-normalization/iOSSupport/Unzippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name Unzippered -target arm64e-apple-ios15.0-macabi
3+
import Swift
4+
public func unzipperedFunc() { }

‎test/ScanDependencies/Inputs/target-normalization/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name Unzippered -target arm64e-apple-macosx12.0
3+
import Swift
4+
public func unzipperedFunc() { }

‎test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name Zippered -target arm64e-apple-ios15.0-macabi
3+
import Swift
4+
public func zipperedFunc() { }

‎test/ScanDependencies/Inputs/target-normalization/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name Zippered -target arm64e-apple-macosx12.0 -target-variant arm64e-apple-ios15.0-macabi
3+
import Swift
4+
public func zipperedFunc() { }

‎test/ScanDependencies/target_normalization.swift‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// REQUIRES: OS=macosx
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: %empty-directory(%t/module-cache)
5+
6+
// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \
7+
// RUN: -module-cache-path %t/module-cache \
8+
// RUN: -I %S/Inputs/target-normalization/iOSSupport \
9+
// RUN: -I %S/Inputs/target-normalization/macOS \
10+
// RUN: -target arm64-apple-macosx14.0 \
11+
// RUN: -o %t/deps-arm64-apple-macosx.json
12+
13+
// RUN: %validate-json %t/deps-arm64-apple-macosx.json
14+
// RUN: %FileCheck %s --input-file %t/deps-arm64-apple-macosx.json \
15+
// RUN: -DORIG_ARCH=arm64 \
16+
// RUN: -DNORM_ARCH=aarch64
17+
18+
// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \
19+
// RUN: -module-cache-path %t/module-cache \
20+
// RUN: -I %S/Inputs/target-normalization/iOSSupport \
21+
// RUN: -I %S/Inputs/target-normalization/macOS \
22+
// RUN: -target arm64e-apple-macosx14.0 \
23+
// RUN: -o %t/deps-arm64e-apple-macosx.json
24+
25+
// RUN: %validate-json %t/deps-arm64e-apple-macosx.json
26+
// RUN: %FileCheck %s --input-file %t/deps-arm64e-apple-macosx.json \
27+
// RUN: -DORIG_ARCH=arm64e \
28+
// RUN: -DNORM_ARCH=arm64e
29+
30+
import Zippered
31+
import Unzippered
32+
33+
// CHECK: "modulePath": "{{.*}}Unzippered-[[UNZIPPERED_HASH:[A-Z0-9]+]].swiftmodule",
34+
// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface",
35+
// CHECK: "commandLine"
36+
// CHECK: "-compile-module-from-interface",
37+
// CHECK: "-target",
38+
// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0"
39+
// CHECK-NOT: "-target-variant"
40+
// CHECK: "-target",
41+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0"
42+
// CHECK-NOT: "-target-variant"
43+
// CHECK: "contextHash": "[[UNZIPPERED_HASH]]",
44+
45+
// CHECK: "modulePath": "{{.*}}Zippered-[[ZIPPERED_HASH:[A-Z0-9]+]].swiftmodule",
46+
// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface",
47+
// CHECK: "commandLine"
48+
// CHECK: "-compile-module-from-interface",
49+
// CHECK: "-target",
50+
// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0"
51+
// CHECK-NOT: "-target-variant"
52+
// CHECK: "-target",
53+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0"
54+
// CHECK: "-target-variant"
55+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi",
56+
// CHECK: "contextHash": "[[ZIPPERED_HASH]]",

‎test/ScanDependencies/target_normalization_maccatalyst.swift‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// REQUIRES: OS=macosx
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: %empty-directory(%t/module-cache)
5+
6+
// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \
7+
// RUN: -module-cache-path %t/module-cache \
8+
// RUN: -I %S/Inputs/target-normalization/iOSSupport \
9+
// RUN: -I %S/Inputs/target-normalization/macOS \
10+
// RUN: -target arm64-apple-ios15.0-macabi \
11+
// RUN: -o %t/deps-arm64-apple-ios-macabi.json
12+
13+
// RUN: %validate-json %t/deps-arm64-apple-ios-macabi.json
14+
// RUN: %FileCheck %s --input-file %t/deps-arm64-apple-ios-macabi.json \
15+
// RUN: -DORIG_ARCH=arm64 \
16+
// RUN: -DNORM_ARCH=aarch64
17+
18+
// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \
19+
// RUN: -module-cache-path %t/module-cache \
20+
// RUN: -I %S/Inputs/target-normalization/iOSSupport \
21+
// RUN: -I %S/Inputs/target-normalization/macOS \
22+
// RUN: -target arm64e-apple-ios15.0-macabi \
23+
// RUN: -o %t/deps-arm64e-apple-ios-macabi.json
24+
25+
// RUN: %validate-json %t/deps-arm64e-apple-ios-macabi.json
26+
// RUN: %FileCheck %s --input-file %t/deps-arm64e-apple-ios-macabi.json \
27+
// RUN: -DORIG_ARCH=arm64e \
28+
// RUN: -DNORM_ARCH=arm64e
29+
30+
import Zippered
31+
import Unzippered
32+
33+
// CHECK: "modulePath": "{{.*}}Unzippered-[[UNZIPPERED_HASH:[A-Z0-9]+]].swiftmodule",
34+
// CHECK: "moduleInterfacePath": "{{.*}}/iOSSupport/Unzippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface",
35+
// CHECK: "commandLine"
36+
// CHECK: "-compile-module-from-interface",
37+
// CHECK: "-target",
38+
// CHECK-NEXT: "[[ORIG_ARCH]]-apple-ios15.0-macabi"
39+
// CHECK-NOT: "-target-variant"
40+
// CHECK: "-target",
41+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi"
42+
// CHECK-NOT: "-target-variant"
43+
// CHECK: "contextHash": "[[UNZIPPERED_HASH]]",
44+
45+
// CHECK: "modulePath": "{{.*}}Zippered-[[ZIPPERED_HASH:[A-Z0-9]+]].swiftmodule",
46+
// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Zippered.swiftmodule/arm64e-apple-ios-macabi.swiftinterface",
47+
// CHECK: "commandLine"
48+
// CHECK: "-compile-module-from-interface",
49+
// CHECK: "-target",
50+
// CHECK-NEXT: "[[ORIG_ARCH]]-apple-ios15.0-macabi"
51+
// CHECK-NOT: "-target-variant"
52+
// CHECK: "-target",
53+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi"
54+
// CHECK-NOT: "-target-variant"
55+
// CHECK: "contextHash": "[[ZIPPERED_HASH]]",

‎test/ScanDependencies/target_normalization_maccatalyst_zippered.swift‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// REQUIRES: OS=macosx
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: %empty-directory(%t/module-cache)
5+
6+
// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \
7+
// RUN: -module-cache-path %t/module-cache \
8+
// RUN: -I %S/Inputs/target-normalization/iOSSupport \
9+
// RUN: -I %S/Inputs/target-normalization/macOS \
10+
// RUN: -target arm64-apple-macosx14.0 \
11+
// RUN: -target-variant arm64-apple-ios15.0-macabi \
12+
// RUN: -o %t/deps-arm64-apple-macosx.json
13+
14+
// RUN: %validate-json %t/deps-arm64-apple-macosx.json
15+
// RUN: %FileCheck %s --input-file %t/deps-arm64-apple-macosx.json \
16+
// RUN: -DORIG_ARCH=arm64 \
17+
// RUN: -DNORM_ARCH=aarch64
18+
19+
// RUN: %target-swift-frontend -parse-stdlib -scan-dependencies %s \
20+
// RUN: -module-cache-path %t/module-cache \
21+
// RUN: -I %S/Inputs/target-normalization/iOSSupport \
22+
// RUN: -I %S/Inputs/target-normalization/macOS \
23+
// RUN: -target arm64e-apple-macosx14.0 \
24+
// RUN: -target-variant arm64e-apple-ios15.0-macabi \
25+
// RUN: -o %t/deps-arm64e-apple-macosx.json
26+
27+
// RUN: %validate-json %t/deps-arm64e-apple-macosx.json
28+
// RUN: %FileCheck %s --input-file %t/deps-arm64e-apple-macosx.json \
29+
// RUN: -DORIG_ARCH=arm64e \
30+
// RUN: -DNORM_ARCH=arm64e
31+
32+
import Zippered
33+
import Unzippered
34+
35+
// CHECK: "modulePath": "{{.*}}Unzippered-[[UNZIPPERED_HASH:[A-Z0-9]+]].swiftmodule",
36+
// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Unzippered.swiftmodule/arm64e-apple-macos.swiftinterface",
37+
// CHECK: "commandLine"
38+
// CHECK: "-compile-module-from-interface",
39+
// CHECK: "-target",
40+
// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0"
41+
// CHECK-NOT: "-target-variant"
42+
// CHECK: "-target",
43+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0"
44+
// CHECK-NOT: "-target-variant"
45+
// CHECK: "contextHash": "[[UNZIPPERED_HASH]]",
46+
47+
// CHECK: "modulePath": "{{.*}}Zippered-[[ZIPPERED_HASH:[A-Z0-9]+]].swiftmodule",
48+
// CHECK: "moduleInterfacePath": "{{.*}}/macOS/Zippered.swiftmodule/arm64e-apple-macos.swiftinterface",
49+
// CHECK: "commandLine"
50+
// CHECK: "-compile-module-from-interface",
51+
// CHECK: "-target",
52+
// CHECK-NEXT: "[[ORIG_ARCH]]-apple-macosx14.0"
53+
// CHECK: "-target",
54+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-macosx12.0"
55+
// CHECK: "-target-variant",
56+
// CHECK-NEXT: "[[NORM_ARCH]]-apple-ios15.0-macabi",
57+
// CHECK: "contextHash": "[[ZIPPERED_HASH]]",

0 commit comments

Comments
 (0)