From 0c73ed806c387985b5db938adae166c3cb1b22b8 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Fri, 6 Aug 2021 13:58:19 -0700 Subject: [PATCH] [Explicit Module Builds] Include source modulemap path into PCM filename hash. Not doing so can lead to dangerous collisions if a build system uses a cetral cache across different targets that have a common dependency that is found in different places in the filesystem. For example, when building targets `A` and `B`, both of which depend on Clang module `C`, but have different search paths, finding `C` in two different locations, we should produce two distinct PCMs. With today's hashing scheme, the second will overwrite the first. --- .../ExplicitDependencyBuildPlanner.swift | 21 ++++-- .../ExplicitModuleBuildTests.swift | 65 ++++++++++++------- 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift index ee8a6c84b..c3855cd17 100644 --- a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift +++ b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift @@ -219,10 +219,12 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT inputs: &inputs, commandLine: &commandLine) + let moduleMapPath = moduleDetails.moduleMapPath.path // Encode the target triple pcm args into the output `.pcm` filename let targetEncodedModulePath = try targetEncodedClangModuleFilePath(for: moduleInfo, - hashParts: getPCMHashParts(pcmArgs: pcmArgs)) + hashParts: getPCMHashParts(pcmArgs: pcmArgs, + moduleMapPath: moduleMapPath.description)) outputs.append(TypedVirtualPath(file: targetEncodedModulePath, type: .pcm)) commandLine.appendFlags("-emit-pcm", "-module-name", moduleId.moduleName, "-o", targetEncodedModulePath.description) @@ -230,7 +232,7 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT // The only required input is the .modulemap for this module. // Command line options in the dependency scanner output will include the // required modulemap, so here we must only add it to the list of inputs. - inputs.append(TypedVirtualPath(file: moduleDetails.moduleMapPath.path, + inputs.append(TypedVirtualPath(file: moduleMapPath, type: .clangModuleMap)) jobs.append(Job( @@ -322,9 +324,11 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT let dependencyInfo = try dependencyGraph.moduleInfo(of: dependencyId) let dependencyClangModuleDetails = try dependencyGraph.clangModuleDetails(of: dependencyId) + let moduleMapPath = dependencyClangModuleDetails.moduleMapPath.path let clangModulePath = try targetEncodedClangModuleFilePath(for: dependencyInfo, - hashParts: getPCMHashParts(pcmArgs: pcmArgs)) + hashParts: getPCMHashParts(pcmArgs: pcmArgs, + moduleMapPath: moduleMapPath.description)) // Accumulate the requried information about this dependency clangDependencyArtifacts.append( ClangModuleArtifactInfo(name: dependencyId.moduleName, @@ -389,11 +393,14 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT return VirtualPath.createUniqueTemporaryFileWithKnownContents(.init("\(moduleId.moduleName)-dependencies.json"), contents) } - private func getPCMHashParts(pcmArgs: [String]) -> [String] { + private func getPCMHashParts(pcmArgs: [String], moduleMapPath: String) -> [String] { + var results: [String] = [] + results.append(moduleMapPath) + results.append(contentsOf: pcmArgs) if integratedDriver { - return pcmArgs + return results } - var results = pcmArgs + // We need this to enable explict modules in the driver-as-executable mode. For instance, // we have two Swift targets A and B, where A depends on X.pcm which in turn depends on Y.pcm, // and B only depends on Y.pcm. In the driver-as-executable mode, the build system isn't aware @@ -445,7 +452,7 @@ extension ExplicitDependencyBuildPlanner { #else hashedArguments = SHA256().hash(hashInput).hexadecimalRepresentation #endif - let resultingName = moduleName + hashedArguments + let resultingName = moduleName + "-" + hashedArguments hashedModuleNameCache[cacheQuery] = resultingName return resultingName } diff --git a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift index b4d9739fb..9f808ce1e 100644 --- a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift +++ b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift @@ -174,14 +174,8 @@ final class ExplicitModuleBuildTests: XCTestCase { for job in modulePrebuildJobs { XCTAssertEqual(job.outputs.count, 1) XCTAssertFalse(driver.isExplicitMainModuleJob(job: job)) - let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in - try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo, - hashParts: hashParts) - } - let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in - try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName, - hashParts: hashParts) - } + + let (pcmFileEncoder, pcmModuleNameEncoder) = pcmEncoderProducer(dependencyGraph: moduleDependencyGraph, driver: driver) switch (job.outputs[0].file) { case .relative(pcmArgsEncodedRelativeModulePath(for: "SwiftShims", with: pcmArgs, pcmModuleNameEncoder: pcmModuleNameEncoder)): @@ -268,6 +262,39 @@ final class ExplicitModuleBuildTests: XCTestCase { path.extension! == FileType.swiftModule.rawValue } + private func pcmEncoderProducer(dependencyGraph: InterModuleDependencyGraph, + driver: Driver) + -> ((ModuleInfo, [String]) -> VirtualPath.Handle, (String, [String]) -> String) { + var driverCopy = driver + let moduleMapIncludedHashParts = { (_ moduleName: String, _ hashParts: [String]) -> [String] in + let moduleDetails = try? dependencyGraph.clangModuleDetails(of: .clang(moduleName)) + let lookupHashParts: [String] + if let details = moduleDetails { + let moduleMapPath = details.moduleMapPath.path.description + lookupHashParts = [moduleMapPath] + hashParts + } else { + // No such module found, no modulemap + lookupHashParts = hashParts + } + return lookupHashParts + } + + let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in + let plainModulePath = VirtualPath.lookup(moduleInfo.modulePath.path) + let moduleName = plainModulePath.basenameWithoutExt + let lookupHashParts = moduleMapIncludedHashParts(moduleName, hashParts) + return try! driverCopy.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo, + hashParts: lookupHashParts) + } + + let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in + let lookupHashParts = moduleMapIncludedHashParts(moduleName, hashParts) + return try! driverCopy.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName, + hashParts: lookupHashParts) + } + return (pcmFileEncoder, pcmModuleNameEncoder) + } + /// Test generation of explicit module build jobs for dependency modules when the driver /// is invoked with -experimental-explicit-module-build func testExplicitModuleBuildJobs() throws { @@ -308,14 +335,8 @@ final class ExplicitModuleBuildTests: XCTestCase { pcmArgs9.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"]) pcmArgs15.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"]) } - let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in - try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo, - hashParts: hashParts) - } - let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in - try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName, - hashParts: hashParts) - } + + let (pcmFileEncoder, pcmModuleNameEncoder) = pcmEncoderProducer(dependencyGraph: dependencyGraph, driver: driver) for job in jobs { XCTAssertEqual(job.outputs.count, 1) @@ -352,6 +373,7 @@ final class ExplicitModuleBuildTests: XCTestCase { // Clang Dependencies } else if outputFilePath.extension != nil, outputFilePath.extension! == FileType.pcm.rawValue { + switch (outputFilePath) { case .relative(pcmArgsEncodedRelativeModulePath(for: "A", with: pcmArgsCurrent, pcmModuleNameEncoder: pcmModuleNameEncoder)): @@ -470,14 +492,9 @@ final class ExplicitModuleBuildTests: XCTestCase { pcmArgs9.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"]) pcmArgs15.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"]) } - let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in - try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo, - hashParts: hashParts) - } - let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in - try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName, - hashParts: hashParts) - } + + let (pcmFileEncoder, pcmModuleNameEncoder) = pcmEncoderProducer(dependencyGraph: dependencyGraph, driver: driver) + for job in jobs { guard job.kind != .interpret else { continue } XCTAssertEqual(job.outputs.count, 1)