Skip to content

Commit 06f3c11

Browse files
authored
[Serialization] Use full target architectures for swiftmodule files (swiftlang#21053)
Previously these used the same "major architecture" names that the `#if os(...)` query accepts, but that can be a problem when building for, say, both armv7 and armv7s, even if the content is "the same". For a transition period where external build tools are involved, the compiler will look for "arm.swiftmodule" if it fails to find "armv7.swiftmodule" or any other 32-bit ARM architecture. No other Apple platform architectures are affected, and AFAIK no one's using the architecture-based layout on Linux or any other platforms. rdar://problem/45174692
1 parent 9596624 commit 06f3c11

File tree

4 files changed

+87
-33
lines changed

4 files changed

+87
-33
lines changed

lib/Serialization/SerializedModuleLoader.cpp

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,23 @@ bool SerializedModuleLoader::maybeDiagnoseArchitectureMismatch(
142142
return true;
143143
}
144144

145+
static std::pair<llvm::SmallString<16>, llvm::SmallString<16>>
146+
getArchSpecificModuleFileNames(StringRef archName) {
147+
llvm::SmallString<16> archFile, archDocFile;
148+
149+
if (!archName.empty()) {
150+
archFile += archName;
151+
archFile += '.';
152+
archFile += file_types::getExtension(file_types::TY_SwiftModuleFile);
153+
154+
archDocFile += archName;
155+
archDocFile += '.';
156+
archDocFile += file_types::getExtension(file_types::TY_SwiftModuleDocFile);
157+
}
158+
159+
return {archFile, archDocFile};
160+
}
161+
145162
bool
146163
SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
147164
std::unique_ptr<llvm::MemoryBuffer> *moduleBuffer,
@@ -157,19 +174,19 @@ SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
157174
moduleDocFilename +=
158175
file_types::getExtension(file_types::TY_SwiftModuleDocFile);
159176

160-
// FIXME: Which name should we be using here? Do we care about CPU subtypes?
161-
// FIXME: At the very least, don't hardcode "arch".
162-
llvm::SmallString<16> archName{
163-
Ctx.LangOpts.getPlatformConditionValue(PlatformConditionKind::Arch)};
164-
llvm::SmallString<16> archFile{archName};
165-
llvm::SmallString<16> archDocFile{archName};
166-
if (!archFile.empty()) {
167-
archFile += '.';
168-
archFile += file_types::getExtension(file_types::TY_SwiftModuleFile);
177+
StringRef archName = Ctx.LangOpts.Target.getArchName();
178+
auto archFileNames = getArchSpecificModuleFileNames(archName);
169179

170-
archDocFile += '.';
171-
archDocFile += file_types::getExtension(file_types::TY_SwiftModuleDocFile);
172-
}
180+
// FIXME: We used to use "major architecture" names for these files---the
181+
// names checked in "#if arch(...)". Fall back to that name in the one case
182+
// where it's different from what Swift 4.2 supported: 32-bit ARM platforms.
183+
// We should be able to drop this once there's an Xcode that supports the
184+
// new names.
185+
StringRef alternateArchName;
186+
if (Ctx.LangOpts.Target.getArch() == llvm::Triple::ArchType::arm)
187+
alternateArchName = "arm";
188+
auto alternateArchFileNames =
189+
getArchSpecificModuleFileNames(alternateArchName);
173190

174191
auto &fs = *Ctx.SourceMgr.getFileSystem();
175192
isFramework = false;
@@ -188,10 +205,19 @@ SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
188205
if (statResult && statResult->isDirectory()) {
189206
// A .swiftmodule directory contains architecture-specific files.
190207
result = openModuleFiles(currPath,
191-
archFile.str(), archDocFile.str(),
208+
archFileNames.first, archFileNames.second,
192209
moduleBuffer, moduleDocBuffer,
193210
scratch);
194211

212+
if (result == std::errc::no_such_file_or_directory &&
213+
!alternateArchName.empty()) {
214+
result = openModuleFiles(currPath,
215+
alternateArchFileNames.first,
216+
alternateArchFileNames.second,
217+
moduleBuffer, moduleDocBuffer,
218+
scratch);
219+
}
220+
195221
if (result == std::errc::no_such_file_or_directory) {
196222
if (maybeDiagnoseArchitectureMismatch(moduleID.second, moduleName,
197223
archName, currPath)) {
@@ -228,9 +254,18 @@ SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
228254
// Frameworks always use architecture-specific files within a .swiftmodule
229255
// directory.
230256
llvm::sys::path::append(currPath, "Modules", moduleFilename.str());
231-
auto err = openModuleFiles(currPath, archFile.str(), archDocFile.str(),
257+
auto err = openModuleFiles(currPath,
258+
archFileNames.first, archFileNames.second,
232259
moduleBuffer, moduleDocBuffer, scratch);
233260

261+
if (err == std::errc::no_such_file_or_directory &&
262+
!alternateArchName.empty()) {
263+
err = openModuleFiles(currPath,
264+
alternateArchFileNames.first,
265+
alternateArchFileNames.second,
266+
moduleBuffer, moduleDocBuffer, scratch);
267+
}
268+
234269
if (err == std::errc::no_such_file_or_directory) {
235270
if (maybeDiagnoseArchitectureMismatch(moduleID.second, moduleName,
236271
archName, currPath)) {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Test the fallback for 32-bit ARM platforms.
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: mkdir -p %t/empty.framework/Modules/empty.swiftmodule
5+
// RUN: %target-swift-frontend -emit-module -o %t/empty.framework/Modules/empty.swiftmodule/arm.swiftmodule %S/../Inputs/empty.swift -module-name empty
6+
// RUN: %target-swift-frontend -typecheck %s -F %t
7+
8+
// RUN: mv %t/empty.framework/Modules/empty.swiftmodule/arm.swiftmodule %t/empty.framework/Modules/empty.swiftmodule/%target-swiftmodule-name
9+
// RUN: touch %t/empty.framework/Modules/empty.swiftmodule/arm.swiftmodule
10+
// RUN: %target-swift-frontend -typecheck %s -F %t
11+
12+
// RUN: rm %t/empty.framework/Modules/empty.swiftmodule/%target-swiftmodule-name
13+
// RUN: not %target-swift-frontend -typecheck %s -F %t 2>&1 | %FileCheck %s
14+
15+
// REQUIRES: CPU=armv7 || CPU=armv7k || CPU=armv7s
16+
17+
import empty
18+
// CHECK: :[[@LINE-1]]:8: error: malformed module file: {{.*}}arm.swiftmodule
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Test the fallback for 32-bit ARM platforms.
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: mkdir %t/empty.swiftmodule
5+
// RUN: %target-swift-frontend -emit-module -o %t/empty.swiftmodule/arm.swiftmodule %S/../Inputs/empty.swift -module-name empty
6+
// RUN: %target-swift-frontend -typecheck %s -I %t
7+
8+
// RUN: mv %t/empty.swiftmodule/arm.swiftmodule %t/empty.swiftmodule/%target-swiftmodule-name
9+
// RUN: touch %t/empty.swiftmodule/arm.swiftmodule
10+
// RUN: %target-swift-frontend -typecheck %s -I %t
11+
12+
// RUN: rm %t/empty.swiftmodule/%target-swiftmodule-name
13+
// RUN: not %target-swift-frontend -typecheck %s -I %t 2>&1 | %FileCheck %s
14+
15+
// REQUIRES: CPU=armv7 || CPU=armv7k || CPU=armv7s
16+
17+
import empty
18+
// CHECK: :[[@LINE-1]]:8: error: malformed module file: {{.*}}arm.swiftmodule

test/lit.cfg

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -584,8 +584,6 @@ elif platform.system() == 'Linux':
584584
if 'swift_interpreter' in config.available_features:
585585
config.available_features.add('swift-remoteast-test')
586586

587-
config.target_swiftmodule_name = "unknown.swiftmodule"
588-
config.target_swiftdoc_name = "unknown.swiftdoc"
589587
config.target_runtime = "unknown"
590588

591589
swift_reflection_test_name = 'swift-reflection-test' + config.variant_suffix
@@ -623,8 +621,6 @@ def use_interpreter_for_simple_runs():
623621

624622
if run_vendor == 'apple':
625623
config.available_features.add('objc_interop')
626-
config.target_swiftmodule_name = run_cpu + ".swiftmodule"
627-
config.target_swiftdoc_name = run_cpu + ".swiftdoc"
628624
config.target_object_format = "macho"
629625
config.target_dylib_extension = "dylib"
630626
config.target_codesign = "codesign -f -s -"
@@ -660,15 +656,6 @@ if run_vendor == 'apple':
660656
lit_config.note('Testing watchOS ' + config.variant_triple)
661657
xcrun_sdk_name = "watchos"
662658

663-
if run_cpu == "armv7" or run_cpu == "armv7s" or run_cpu == "armv7k":
664-
config.target_swiftmodule_name = "arm.swiftmodule"
665-
config.target_swiftdoc_name = "arm.swiftdoc"
666-
elif run_cpu == "arm64":
667-
config.target_swiftmodule_name = "arm64.swiftmodule"
668-
config.target_swiftdoc_name = "arm64.swiftdoc"
669-
else:
670-
lit_config.fatal("Unknown CPU '%s'" % run_cpu)
671-
672659
config.target_cc_options = (
673660
"-arch %s -m%s-version-min=%s %s" %
674661
(run_cpu, run_os, run_vers, clang_mcp_opt))
@@ -807,8 +794,6 @@ elif run_os in ['windows-msvc']:
807794
config.target_dylib_extension = 'dll'
808795
config.target_sdk_name = 'windows'
809796
config.target_runtime = 'native'
810-
config.target_swiftmodule_name = run_cpu + '.swiftmodule'
811-
config.target_swiftdoc_name = run_cpu + '.swiftdoc'
812797

813798
config.target_build_swift = \
814799
('%r -target %s %s %s %s %s %s' % (config.swiftc, \
@@ -880,8 +865,6 @@ elif run_os in ['linux-gnu', 'linux-gnueabihf', 'freebsd', 'windows-cygnus', 'wi
880865
config.target_object_format = "elf"
881866
config.target_dylib_extension = "so"
882867
config.target_sdk_name = "linux"
883-
config.target_swiftmodule_name = run_cpu + ".swiftmodule"
884-
config.target_swiftdoc_name = run_cpu + ".swiftdoc"
885868
config.target_runtime = "native"
886869
config.target_swift_autolink_extract = inferSwiftBinary("swift-autolink-extract")
887870
config.target_build_swift = (
@@ -1333,8 +1316,8 @@ else:
13331316
config.substitutions.insert(0, ('%platform-module-dir', platform_module_dir))
13341317
config.substitutions.insert(0, ('%platform-sdk-overlay-dir', platform_sdk_overlay_dir))
13351318

1336-
config.substitutions.append(('%target-swiftmodule-name', config.target_swiftmodule_name))
1337-
config.substitutions.append(('%target-swiftdoc-name', config.target_swiftdoc_name))
1319+
config.substitutions.append(('%target-swiftmodule-name', run_cpu + '.swiftmodule'))
1320+
config.substitutions.append(('%target-swiftdoc-name', run_cpu + '.swiftdoc'))
13381321

13391322
config.substitutions.append(('%target-object-format', config.target_object_format))
13401323
config.substitutions.append(('%target-dylib-extension', config.target_dylib_extension))

0 commit comments

Comments
 (0)