Merge pull request #1655 from hamishknight/windows-coverage

Pass `-lld-allow-duplicate-weak` for coverage on Windows

Author: hamishknight

Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift

@@ -39,18 +39,41 @@ extension WindowsToolchain {
                                             sanitizers: Set<Sanitizer>,
                                             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)

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 {