Skip to content

[Serialization] Soft-reject swiftmodules built against a different SDK on tagged compilers #58935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions include/swift/AST/SearchPathOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -327,10 +327,6 @@ class SearchPathOptions {
/// would for a non-system header.
bool DisableModulesValidateSystemDependencies = false;

/// Enforce loading only serialized modules built with the same SDK
/// as the context loading it.
bool EnableSameSDKCheck = true;

/// A set of compiled modules that may be ready to use.
std::vector<std::string> CandidateCompiledModules;

Expand Down
5 changes: 5 additions & 0 deletions include/swift/Basic/Version.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ std::string getSwiftFullVersion(Version effectiveLanguageVersion =
/// this Swift was built.
StringRef getSwiftRevision();

/// Is the running compiler built with a version tag for distribution?
/// When true, \c Version::getCurrentCompilerVersion returns a valid version
/// and \c getSwiftRevision returns the version tuple in string format.
bool isCurrentCompilerTagged();

} // end namespace version
} // end namespace swift

Expand Down
4 changes: 3 additions & 1 deletion include/swift/Serialization/Validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,15 @@ class ExtendedValidationInfo {
/// refers directly into this buffer.
/// \param requiresOSSAModules If true, necessitates the module to be
/// compiled with -enable-ossa-modules.
/// \param requiredSDK If not empty, only accept modules built with
/// a compatible SDK. The StringRef represents the canonical SDK name.
/// \param[out] extendedInfo If present, will be populated with additional
/// compilation options serialized into the AST at build time that may be
/// necessary to load it properly.
/// \param[out] dependencies If present, will be populated with list of
/// input files the module depends on, if present in INPUT_BLOCK.
ValidationInfo validateSerializedAST(
StringRef data, bool requiresOSSAModules,
StringRef data, bool requiresOSSAModules, StringRef requiredSDK,
ExtendedValidationInfo *extendedInfo = nullptr,
SmallVectorImpl<SerializationOptions::FileDependency> *dependencies =
nullptr);
Expand Down
4 changes: 0 additions & 4 deletions lib/APIDigester/ModuleAnalyzerNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2419,10 +2419,6 @@ swift::ide::api::getSDKNodeRoot(SDKContext &SDKCtx,

auto &Ctx = CI.getASTContext();

// Don't check if the stdlib was build with the same SDK as what is loaded
// here as some tests rely on using a different stdlib.
Ctx.SearchPathOpts.EnableSameSDKCheck = false;

// Load standard library so that Clang importer can use it.
auto *Stdlib = Ctx.getStdlibModule(/*loadIfAbsent=*/true);
if (!Stdlib) {
Expand Down
2 changes: 1 addition & 1 deletion lib/ASTSectionImporter/ASTSectionImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ bool swift::parseASTSection(MemoryBufferSerializedModuleLoader &Loader,
// headers. Iterate over all AST modules.
while (!buf.empty()) {
auto info = serialization::validateSerializedAST(
buf, Loader.isRequiredOSSAModules());
buf, Loader.isRequiredOSSAModules(), /*requiredSDK*/StringRef());

assert(info.name.size() < (2 << 10) && "name failed sanity check");

Expand Down
8 changes: 8 additions & 0 deletions lib/Basic/Version.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,5 +446,13 @@ StringRef getSwiftRevision() {
#endif
}

bool isCurrentCompilerTagged() {
#ifdef SWIFT_COMPILER_VERSION
return true;
#else
return false;
#endif
}

} // end namespace version
} // end namespace swift
4 changes: 2 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2522,7 +2522,7 @@ serialization::Status
CompilerInvocation::loadFromSerializedAST(StringRef data) {
serialization::ExtendedValidationInfo extendedInfo;
serialization::ValidationInfo info = serialization::validateSerializedAST(
data, getSILOptions().EnableOSSAModules, &extendedInfo);
data, getSILOptions().EnableOSSAModules, LangOpts.SDKName, &extendedInfo);

if (info.status != serialization::Status::Valid)
return info.status;
Expand Down Expand Up @@ -2558,7 +2558,7 @@ CompilerInvocation::setUpInputForSILTool(

auto result = serialization::validateSerializedAST(
fileBufOrErr.get()->getBuffer(), getSILOptions().EnableOSSAModules,
&extendedInfo);
LangOpts.SDKName, &extendedInfo);
bool hasSerializedAST = result.status == serialization::Status::Valid;

if (hasSerializedAST) {
Expand Down
11 changes: 7 additions & 4 deletions lib/Frontend/ModuleInterfaceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,11 @@ class DiscoveredModule {
namespace path = llvm::sys::path;

static bool serializedASTLooksValid(const llvm::MemoryBuffer &buf,
bool requiresOSSAModules) {
bool requiresOSSAModules,
StringRef requiredSDK) {
auto VI = serialization::validateSerializedAST(buf.getBuffer(),
requiresOSSAModules);
requiresOSSAModules,
requiredSDK);
return VI.status == serialization::Status::Valid;
}

Expand Down Expand Up @@ -500,7 +502,7 @@ class ModuleInterfaceLoaderImpl {

LLVM_DEBUG(llvm::dbgs() << "Validating deps of " << path << "\n");
auto validationInfo = serialization::validateSerializedAST(
buf.getBuffer(), requiresOSSAModules,
buf.getBuffer(), requiresOSSAModules, ctx.LangOpts.SDKName,
/*ExtendedValidationInfo=*/nullptr, &allDeps);

if (validationInfo.status != serialization::Status::Valid) {
Expand Down Expand Up @@ -542,7 +544,8 @@ class ModuleInterfaceLoaderImpl {

// First, make sure the underlying module path exists and is valid.
auto modBuf = fs.getBufferForFile(fwd.underlyingModulePath);
if (!modBuf || !serializedASTLooksValid(*modBuf.get(), requiresOSSAModules))
if (!modBuf || !serializedASTLooksValid(*modBuf.get(), requiresOSSAModules,
ctx.LangOpts.SDKName))
return false;

// Next, check the dependencies in the forwarding file.
Expand Down
18 changes: 1 addition & 17 deletions lib/Serialization/ModuleFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,22 +156,6 @@ Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
return error(status);
}

// The loaded module was built with a compatible SDK if either:
// * it was the same SDK
// * or one who's name is a prefix of the clients' SDK name. This expects
// that a module built with macOS11 can be used with the macOS11.secret SDK.
// This is generally the case as SDKs with suffixes are a superset of the
// short SDK name equivalent. While this is accepted, this is still not a
// recommended configuration and may lead to unreadable swiftmodules.
StringRef moduleSDK = Core->SDKName;
StringRef clientSDK = ctx.LangOpts.SDKName;
if (ctx.SearchPathOpts.EnableSameSDKCheck &&
!moduleSDK.empty() && !clientSDK.empty() &&
!clientSDK.startswith(moduleSDK)) {
status = Status::SDKMismatch;
return error(status);
}

StringRef SDKPath = ctx.SearchPathOpts.getSDKPath();
if (SDKPath.empty() ||
!Core->ModuleInputBuffer->getBufferIdentifier().startswith(SDKPath)) {
Expand Down Expand Up @@ -372,7 +356,7 @@ ModuleFile::getModuleName(ASTContext &Ctx, StringRef modulePath,
bool isFramework = false;
serialization::ValidationInfo loadInfo = ModuleFileSharedCore::load(
modulePath.str(), std::move(newBuf), nullptr, nullptr,
/*isFramework*/ isFramework, Ctx.SILOpts.EnableOSSAModules,
/*isFramework*/ isFramework, Ctx.SILOpts.EnableOSSAModules, Ctx.LangOpts.SDKName,
Ctx.SearchPathOpts.DeserializedPathRecoverer,
loadedModuleFile);
Name = loadedModuleFile->Name.str();
Expand Down
43 changes: 34 additions & 9 deletions lib/Serialization/ModuleFileSharedCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ static ValidationInfo validateControlBlock(
llvm::BitstreamCursor &cursor, SmallVectorImpl<uint64_t> &scratch,
std::pair<uint16_t, uint16_t> expectedVersion, bool requiresOSSAModules,
bool requiresRevisionMatch,
StringRef requiredSDK,
ExtendedValidationInfo *extendedInfo,
PathObfuscator &pathRecoverer) {
// The control block is malformed until we've at least read a major version
Expand Down Expand Up @@ -307,6 +308,30 @@ static ValidationInfo validateControlBlock(
break;
case control_block::SDK_NAME: {
result.sdkName = blobData;

// Enable this check for tagged compiler or when the
// env var is set (for testing).
static const char* forceDebugPreSDKRestriction =
::getenv("SWIFT_DEBUG_FORCE_SWIFTMODULE_PER_SDK");
if (!version::isCurrentCompilerTagged() &&
!forceDebugPreSDKRestriction) {
break;
}

// The loaded module was built with a compatible SDK if either:
// * it was the same SDK
// * or one who's name is a prefix of the clients' SDK name. This expects
// that a module built with macOS11 can be used with the macOS11.secret SDK.
// This is generally the case as SDKs with suffixes are a superset of the
// short SDK name equivalent. While this is accepted, this is still not a
// recommended configuration and may lead to unreadable swiftmodules.
StringRef moduleSDK = blobData;
if (!moduleSDK.empty() && !requiredSDK.empty() &&
!requiredSDK.startswith(moduleSDK)) {
result.status = Status::SDKMismatch;
return result;
}

break;
}
case control_block::REVISION: {
Expand All @@ -329,7 +354,7 @@ static ValidationInfo validateControlBlock(
::getenv("SWIFT_DEBUG_FORCE_SWIFTMODULE_REVISION");

bool isCompilerTagged = forcedDebugRevision ||
!version::Version::getCurrentCompilerVersion().empty();
version::isCurrentCompilerTagged();

StringRef moduleRevision = blobData;
if (isCompilerTagged) {
Expand Down Expand Up @@ -443,7 +468,7 @@ bool serialization::isSerializedAST(StringRef data) {
}

ValidationInfo serialization::validateSerializedAST(
StringRef data, bool requiresOSSAModules,
StringRef data, bool requiresOSSAModules, StringRef requiredSDK,
ExtendedValidationInfo *extendedInfo,
SmallVectorImpl<SerializationOptions::FileDependency> *dependencies) {
ValidationInfo result;
Expand Down Expand Up @@ -487,6 +512,7 @@ ValidationInfo serialization::validateSerializedAST(
cursor, scratch,
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
requiresOSSAModules, /*requiresRevisionMatch=*/true,
requiredSDK,
extendedInfo, localObfuscator);
if (result.status == Status::Malformed)
return result;
Expand Down Expand Up @@ -995,8 +1021,8 @@ bool ModuleFileSharedCore::readModuleDocIfPresent(PathObfuscator &pathRecoverer)

info = validateControlBlock(
docCursor, scratch, {SWIFTDOC_VERSION_MAJOR, SWIFTDOC_VERSION_MINOR},
RequiresOSSAModules, /*requiresRevisionMatch=*/false,
/*extendedInfo*/ nullptr, pathRecoverer);
RequiresOSSAModules, /*requiresRevisionMatch*/false,
/*requiredSDK*/StringRef(), /*extendedInfo*/nullptr, pathRecoverer);
if (info.status != Status::Valid)
return false;
// Check that the swiftdoc is actually for this module.
Expand Down Expand Up @@ -1139,9 +1165,8 @@ bool ModuleFileSharedCore::readModuleSourceInfoIfPresent(PathObfuscator &pathRec
info = validateControlBlock(
infoCursor, scratch,
{SWIFTSOURCEINFO_VERSION_MAJOR, SWIFTSOURCEINFO_VERSION_MINOR},
RequiresOSSAModules, /*requiresRevisionMatch=*/false,
/*extendedInfo*/ nullptr,
pathRecoverer);
RequiresOSSAModules, /*requiresRevisionMatch*/false,
/*requiredSDK*/StringRef(), /*extendedInfo*/nullptr, pathRecoverer);
if (info.status != Status::Valid)
return false;
// Check that the swiftsourceinfo is actually for this module.
Expand Down Expand Up @@ -1215,7 +1240,7 @@ ModuleFileSharedCore::ModuleFileSharedCore(
std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
bool isFramework, bool requiresOSSAModules,
bool isFramework, bool requiresOSSAModules, StringRef requiredSDK,
serialization::ValidationInfo &info, PathObfuscator &pathRecoverer)
: ModuleInputBuffer(std::move(moduleInputBuffer)),
ModuleDocInputBuffer(std::move(moduleDocInputBuffer)),
Expand Down Expand Up @@ -1267,7 +1292,7 @@ ModuleFileSharedCore::ModuleFileSharedCore(
info = validateControlBlock(
cursor, scratch,
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
RequiresOSSAModules, /*requiresRevisionMatch=*/true,
RequiresOSSAModules, /*requiresRevisionMatch=*/true, requiredSDK,
&extInfo, pathRecoverer);
if (info.status != Status::Valid) {
error(info.status);
Expand Down
6 changes: 3 additions & 3 deletions lib/Serialization/ModuleFileSharedCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ class ModuleFileSharedCore {
std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
bool isFramework, bool requiresOSSAModules,
bool isFramework, bool requiresOSSAModules, StringRef requiredSDK,
serialization::ValidationInfo &info, PathObfuscator &pathRecoverer);

/// Change the status of the current module.
Expand Down Expand Up @@ -510,13 +510,13 @@ class ModuleFileSharedCore {
std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
bool isFramework, bool requiresOSSAModules,
PathObfuscator &pathRecoverer,
StringRef requiredSDK, PathObfuscator &pathRecoverer,
std::shared_ptr<const ModuleFileSharedCore> &theModule) {
serialization::ValidationInfo info;
auto *core = new ModuleFileSharedCore(
std::move(moduleInputBuffer), std::move(moduleDocInputBuffer),
std::move(moduleSourceInfoInputBuffer), isFramework,
requiresOSSAModules, info, pathRecoverer);
requiresOSSAModules, requiredSDK, info, pathRecoverer);
if (!moduleInterfacePath.empty()) {
ArrayRef<char> path;
core->allocateBuffer(path, moduleInterfacePath);
Expand Down
7 changes: 4 additions & 3 deletions lib/Serialization/SerializedModuleLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ llvm::ErrorOr<ModuleDependencies> SerializedModuleLoaderBase::scanModuleFile(
bool isFramework = false;
serialization::ValidationInfo loadInfo = ModuleFileSharedCore::load(
modulePath.str(), std::move(moduleBuf.get()), nullptr, nullptr,
isFramework, isRequiredOSSAModules(),
isFramework, isRequiredOSSAModules(), Ctx.LangOpts.SDKName,
Ctx.SearchPathOpts.DeserializedPathRecoverer,
loadedModuleFile);

Expand Down Expand Up @@ -753,7 +753,7 @@ LoadedFile *SerializedModuleLoaderBase::loadAST(
serialization::ValidationInfo loadInfo = ModuleFileSharedCore::load(
moduleInterfacePath, std::move(moduleInputBuffer),
std::move(moduleDocInputBuffer), std::move(moduleSourceInfoInputBuffer),
isFramework, isRequiredOSSAModules(),
isFramework, isRequiredOSSAModules(), Ctx.LangOpts.SDKName,
Ctx.SearchPathOpts.DeserializedPathRecoverer,
loadedModuleFileCore);
SerializedASTFile *fileUnit = nullptr;
Expand Down Expand Up @@ -1176,7 +1176,8 @@ bool SerializedModuleLoaderBase::canImportModule(ImportPath::Module path,
// format, if present.
if (currentVersion.empty() && *unusedModuleBuffer) {
auto metaData = serialization::validateSerializedAST(
(*unusedModuleBuffer)->getBuffer(), Ctx.SILOpts.EnableOSSAModules);
(*unusedModuleBuffer)->getBuffer(), Ctx.SILOpts.EnableOSSAModules,
Ctx.LangOpts.SDKName);
currentVersion = metaData.userModuleVersion;
}

Expand Down
22 changes: 18 additions & 4 deletions test/Serialization/restrict-swiftmodule-to-sdk.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,34 @@
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -swift-version 5 -target-sdk-name A -o %t/build -parse-stdlib -module-cache-path %t/cache

/// Building Client against SDK A should work fine as expected.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name A -I %t/build -parse-stdlib -module-cache-path %t/cache
// RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_PER_SDK=true \
// RUN: %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name A -I %t/build -parse-stdlib -module-cache-path %t/cache

/// Build Client against SDK B, this should fail at loading Lib against a different SDK than A.
// RUN: not %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name B -I %t/build -parse-stdlib -module-cache-path %t/cache 2>&1 | %FileCheck %s -check-prefix=CHECK-AvsB
// RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_PER_SDK=true \
// RUN: not %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name B -I %t/build -parse-stdlib -module-cache-path %t/cache 2>&1 | %FileCheck %s -check-prefix=CHECK-AvsB
// CHECK-AvsB: cannot load module 'Lib' built with SDK 'A' when using SDK 'B': {{.*}}Lib.swiftmodule

/// Build Client against SDK A.Secret, this should accept the SDK as being a super set of A.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name A.Secret -I %t/build -parse-stdlib -module-cache-path %t/cache
// RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_PER_SDK=true \
// RUN: %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name A.Secret -I %t/build -parse-stdlib -module-cache-path %t/cache

/// Build Lib against SDK C.Secret and Client against SDK C, this should be rejected.
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -swift-version 5 -target-sdk-name C.Secret -o %t/build -parse-stdlib -module-cache-path %t/cache
// RUN: not %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name C -I %t/build -parse-stdlib -module-cache-path %t/cache 2>&1 | %FileCheck %s -check-prefix=CHECK-C
// RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_PER_SDK=true \
// RUN: not %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name C -I %t/build -parse-stdlib -module-cache-path %t/cache 2>&1 | %FileCheck %s -check-prefix=CHECK-C
// CHECK-C: cannot load module 'Lib' built with SDK 'C.Secret' when using SDK 'C': {{.*}}Lib.swiftmodule

/// Build a resilient Lib against SDK A, and a client against SDK B.
/// This should succeed after rebuilding from the swiftinterface.
// RUN: %empty-directory(%t/cache)
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -swift-version 5 -target-sdk-name A -o %t/build -parse-stdlib -module-cache-path %t/cache \
// RUN: -enable-library-evolution -emit-module-interface-path %t/build/Lib.swiftinterface
// RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_PER_SDK=true \
// RUN: %target-swift-frontend -typecheck %t/Client.swift -swift-version 5 -target-sdk-name B -I %t/build -parse-stdlib -module-cache-path %t/cache \
// RUN: -Rmodule-interface-rebuild 2>&1 | %FileCheck %s -check-prefix=CHECK-AvsB-REBUILD
// CHECK-AvsB-REBUILD: remark: rebuilding module 'Lib' from interface

// BEGIN Lib.swift
public func foo() {}

Expand Down
1 change: 1 addition & 0 deletions tools/lldb-moduleimport-test/lldb-moduleimport-test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ validateModule(llvm::StringRef data, bool Verbose, bool requiresOSSAModules,
swift::serialization::ValidationInfo &info,
swift::serialization::ExtendedValidationInfo &extendedInfo) {
info = swift::serialization::validateSerializedAST(data, requiresOSSAModules,
/*requiredSDK*/StringRef(),
&extendedInfo);
if (info.status != swift::serialization::Status::Valid) {
llvm::outs() << "error: validateSerializedAST() failed\n";
Expand Down
2 changes: 1 addition & 1 deletion unittests/FrontendTool/ModuleLoadingTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class ModuleInterfaceLoaderTest : public testing::Test {

auto bufData = (*bufOrErr)->getBuffer();
auto validationInfo = serialization::validateSerializedAST(
bufData, silOpts.EnableOSSAModules);
bufData, silOpts.EnableOSSAModules, /*requiredSDK*/StringRef());
ASSERT_EQ(serialization::Status::Valid, validationInfo.status);
ASSERT_EQ(bufData, moduleBuffer->getBuffer());
}
Expand Down