Skip to content
Permalink
Browse files
Merge pull request #58935 from xymus/swiftmodule-per-sdk-soft-reject
[Serialization] Soft-reject swiftmodules built against a different SDK on tagged compilers
  • Loading branch information
xymus committed May 18, 2022
2 parents 5693331 + c341010 commit 6c287b2b41ef258baa3151d5ccca0e6bb7ce742a
Showing 15 changed files with 88 additions and 53 deletions.
@@ -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;

@@ -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

@@ -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);
@@ -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) {
@@ -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");

@@ -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
@@ -2538,7 +2538,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;
@@ -2574,7 +2574,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) {
@@ -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;
}

@@ -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) {
@@ -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.
@@ -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)) {
@@ -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();
@@ -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
@@ -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: {
@@ -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) {
@@ -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;
@@ -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;
@@ -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.
@@ -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.
@@ -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)),
@@ -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);
@@ -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.
@@ -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);
@@ -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);

@@ -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;
@@ -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;
}

@@ -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() {}

@@ -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";
@@ -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());
}

0 comments on commit 6c287b2

Please sign in to comment.