From e206bfd4d58f577b648e6a30096d6ea80da4a71d Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 8 Jul 2024 15:30:59 +0100 Subject: [PATCH] Pass `-lld-allow-duplicate-weak` for coverage on Windows Profiling currently relies on the ability to emit duplicate weak symbols across translation units and having the linker coalesce them. Unfortunately `link.exe` does not support this, so require `lld-link` and pass `-lld-allow-duplicate-weak` when `-profile-generate` is passed. We ought to be able to remove this once coverage is changed to not rely on duplicate weak symbols, rdar://131295678 is tracking that. rdar://129337999 --- .../Jobs/WindowsToolchain+LinkerSupport.swift | 46 +++++++++++--- Tests/SwiftDriverTests/SwiftDriverTests.swift | 62 ++++++++++++++++++- 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift b/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift index c53aada02..86d75f9b0 100644 --- a/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift +++ b/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift @@ -39,18 +39,41 @@ extension WindowsToolchain { sanitizers: Set, targetInfo: FrontendTargetInfo) throws -> ResolvedTool { + // Check to see whether we need to use lld as the linker. + let requiresLLD = { + if let ld = parsedOptions.getLastArgument(.useLd)?.asSingle { + switch ld { + case "lld", "lld.exe", "lld-link", "lld-link.exe": + return true + default: + return false + } + } + if lto != nil { + return true + } + // Profiling currently relies on the ability to emit duplicate weak + // symbols across translation units and having the linker coalesce them. + // Unfortunately link.exe does not support this, so require lld-link + // for now, which supports the behavior via a flag. + // TODO: Once we've changed coverage to no longer rely on emitting + // duplicate weak symbols (rdar://131295678), we can remove this. + if parsedOptions.hasArgument(.profileGenerate) { + return true + } + return false + }() + // Special case static linking as clang cannot drive the operation. if linkerOutputType == .staticLibrary { let librarian: String - switch parsedOptions.getLastArgument(.useLd)?.asSingle { - case .none: - librarian = lto == nil ? "link" : "lld-link" - case .some("lld"), .some("lld.exe"), .some("lld-link"), .some("lld-link.exe"): + if requiresLLD { librarian = "lld-link" - case let .some(linker): - librarian = linker + } else if let ld = parsedOptions.getLastArgument(.useLd)?.asSingle { + librarian = ld + } else { + librarian = "link" } - commandLine.appendFlag("/LIB") commandLine.appendFlag("/NOLOGO") commandLine.appendFlag("/OUT:\(outputFile.name.spm_shellEscaped())") @@ -101,7 +124,7 @@ extension WindowsToolchain { // Select the linker to use. if let arg = parsedOptions.getLastArgument(.useLd)?.asSingle { commandLine.appendFlag("-fuse-ld=\(arg)") - } else if lto != nil { + } else if requiresLLD { commandLine.appendFlag("-fuse-ld=lld") } @@ -195,6 +218,13 @@ extension WindowsToolchain { // FIXME(compnerd) wrap llvm::getInstrProfRuntimeHookVarName() commandLine.appendFlag("-include:__llvm_profile_runtime") commandLine.appendFlag("-lclang_rt.profile") + + // FIXME(rdar://131295678): Currently profiling requires the ability to + // emit duplicate weak symbols. Assuming we're using lld, pass + // -lld-allow-duplicate-weak to enable this behavior. + if requiresLLD { + commandLine.appendFlags("-Xlinker", "-lld-allow-duplicate-weak") + } } try addExtraClangLinkerArgs(to: &commandLine, parsedOptions: &parsedOptions) diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index 6c6da824e..41343dba1 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -4401,7 +4401,67 @@ final class SwiftDriverTests: XCTestCase { } #endif - // TODO: Windows + for explicitUseLd in [true, false] { + var args = ["swiftc", "-profile-generate", "-target", "x86_64-unknown-windows-msvc", "test.swift"] + if explicitUseLd { + // Explicitly passing '-use-ld=lld' should still result in '-lld-allow-duplicate-weak'. + args.append("-use-ld=lld") + } + var driver = try Driver(args: args) + let plannedJobs = try driver.planBuild() + print(plannedJobs[1].commandLine) + + XCTAssertEqual(plannedJobs.count, 2) + XCTAssertEqual(plannedJobs[0].kind, .compile) + + XCTAssertEqual(plannedJobs[1].kind, .link) + + let linkCmds = plannedJobs[1].commandLine + XCTAssert(linkCmds.contains(.flag("-include:__llvm_profile_runtime"))) + XCTAssert(linkCmds.contains(.flag("-lclang_rt.profile"))) + + // rdar://131295678 - Make sure we force the use of lld and pass + // '-lld-allow-duplicate-weak'. + XCTAssert(linkCmds.contains(.flag("-fuse-ld=lld"))) + XCTAssert(linkCmds.contains([.flag("-Xlinker"), .flag("-lld-allow-duplicate-weak")])) + } + + do { + // If the user passes -use-ld for a non-lld linker, respect that and + // don't use '-lld-allow-duplicate-weak' + var driver = try Driver(args: ["swiftc", "-profile-generate", "-use-ld=link", "-target", "x86_64-unknown-windows-msvc", "test.swift"]) + let plannedJobs = try driver.planBuild() + print(plannedJobs[1].commandLine) + + XCTAssertEqual(plannedJobs.count, 2) + XCTAssertEqual(plannedJobs[0].kind, .compile) + + XCTAssertEqual(plannedJobs[1].kind, .link) + + let linkCmds = plannedJobs[1].commandLine + XCTAssert(linkCmds.contains(.flag("-include:__llvm_profile_runtime"))) + XCTAssert(linkCmds.contains(.flag("-lclang_rt.profile"))) + + XCTAssertTrue(linkCmds.contains(.flag("-fuse-ld=link"))) + XCTAssertFalse(linkCmds.contains(.flag("-fuse-ld=lld"))) + XCTAssertFalse(linkCmds.contains(.flag("-lld-allow-duplicate-weak"))) + } + + do { + // If we're not building for profiling, don't add '-lld-allow-duplicate-weak'. + var driver = try Driver(args: ["swiftc", "-use-ld=lld", "-target", "x86_64-unknown-windows-msvc", "test.swift"]) + let plannedJobs = try driver.planBuild() + print(plannedJobs[1].commandLine) + + XCTAssertEqual(plannedJobs.count, 2) + XCTAssertEqual(plannedJobs[0].kind, .compile) + + XCTAssertEqual(plannedJobs[1].kind, .link) + + let linkCmds = plannedJobs[1].commandLine + XCTAssertTrue(linkCmds.contains(.flag("-fuse-ld=lld"))) + XCTAssertFalse(linkCmds.contains(.flag("-lld-allow-duplicate-weak"))) + } } func testConditionalCompilationArgValidation() throws {