Merge pull request #768 from owenv/linker-quoting

owenv · web-flow · commit 3a242d680c19 · 2025-09-04T09:10:43.000-07:00
Properly quote linker file list contents on Linux and Windows
diff --git a/Sources/SWBCore/Settings/BuiltinMacros.swift b/Sources/SWBCore/Settings/BuiltinMacros.swift
@@ -1181,6 +1181,7 @@ public final class BuiltinMacros {
     public static let _WRAPPER_PARENT_PATH = BuiltinMacros.declareStringMacro("_WRAPPER_PARENT_PATH")
     public static let _WRAPPER_RESOURCES_DIR = BuiltinMacros.declareStringMacro("_WRAPPER_RESOURCES_DIR")
     public static let __INPUT_FILE_LIST_PATH__ = BuiltinMacros.declarePathMacro("__INPUT_FILE_LIST_PATH__")
+    public static let LINKER_FILE_LIST_FORMAT = BuiltinMacros.declareEnumMacro("LINKER_FILE_LIST_FORMAT") as EnumMacroDeclaration<LinkerFileListFormat>
     public static let SWIFT_RESPONSE_FILE_PATH = BuiltinMacros.declarePathMacro("SWIFT_RESPONSE_FILE_PATH")
     public static let __ARCHS__ = BuiltinMacros.declareStringListMacro("__ARCHS__")
 
@@ -2425,6 +2426,7 @@ public final class BuiltinMacros {
         _WRAPPER_PARENT_PATH,
         _WRAPPER_RESOURCES_DIR,
         __INPUT_FILE_LIST_PATH__,
+        LINKER_FILE_LIST_FORMAT,
         __ARCHS__,
         __SWIFT_MODULE_ONLY_ARCHS__,
         arch,
@@ -2870,6 +2872,14 @@ public enum StripStyle: String, Equatable, Hashable, EnumerationMacroType {
     case debugging
 }
 
+public enum LinkerFileListFormat: String, Equatable, Hashable, EnumerationMacroType {
+    public static let defaultValue = Self.unescapedNewlineSeparated
+
+    case unescapedNewlineSeparated
+    case unixShellQuotedNewlineSeparated
+    case windowsShellQuotedNewlineSeparated
+}
+
 public enum MergedBinaryType: String, Equatable, Hashable, EnumerationMacroType {
     public static let defaultValue = Self.none
 
diff --git a/Sources/SWBCore/SpecImplementations/LinkerSpec.swift b/Sources/SWBCore/SpecImplementations/LinkerSpec.swift
@@ -138,6 +138,23 @@ open class LinkerSpec : CommandLineToolSpec, @unchecked Sendable {
         return ruleInfo
     }
 
+    public func inputFileListContents(_ cbc: CommandBuildContext) -> ByteString {
+        let contents = OutputByteStream()
+        for input in cbc.inputs {
+            switch cbc.scope.evaluate(BuiltinMacros.LINKER_FILE_LIST_FORMAT) {
+            case .unescapedNewlineSeparated:
+                contents <<< input.absolutePath.strWithPosixSlashes <<< "\n"
+            case .unixShellQuotedNewlineSeparated:
+                let escaper = UNIXShellCommandCodec(encodingStrategy: .singleQuotes, encodingBehavior: .argumentsOnly)
+                contents <<< escaper.encode([input.absolutePath.strWithPosixSlashes]) <<< "\n"
+            case .windowsShellQuotedNewlineSeparated:
+                let escaper = WindowsProcessArgumentsCodec()
+                contents <<< escaper.encode([input.absolutePath.strWithPosixSlashes]) <<< "\n"
+            }
+        }
+        return contents.bytes
+    }
+
     open override func constructTasks(_ cbc: CommandBuildContext, _ delegate: any TaskGenerationDelegate) async {
         // FIXME: We should ensure this cannot happen.
         fatalError("unexpected direct invocation")
diff --git a/Sources/SWBCore/SpecImplementations/PropertyDomainSpec.swift b/Sources/SWBCore/SpecImplementations/PropertyDomainSpec.swift
@@ -116,6 +116,8 @@ private final class EnumBuildOptionType : BuildOptionType {
             return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<LinkerDriverChoice>
         case "SWIFT_API_DIGESTER_MODE":
             return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<SwiftAPIDigesterMode>
+        case "LINKER_FILE_LIST_FORMAT":
+            return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<LinkerFileListFormat>
         default:
             return try namespace.declareStringMacro(name)
         }
diff --git a/Sources/SWBCore/SpecImplementations/Tools/LinkerTools.swift b/Sources/SWBCore/SpecImplementations/Tools/LinkerTools.swift
@@ -382,13 +382,8 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
         // Define the linker file list.
         let fileListPath = cbc.scope.evaluate(BuiltinMacros.__INPUT_FILE_LIST_PATH__, lookup: linkerDriverLookup)
         if !fileListPath.isEmpty {
-            let contents = OutputByteStream()
-            for input in cbc.inputs {
-                // ld64 reads lines from the file using fgets, without doing any other processing.
-                contents <<< input.absolutePath.strWithPosixSlashes <<< "\n"
-            }
             let fileListPath = fileListPath
-            cbc.producer.writeFileSpec.constructFileTasks(CommandBuildContext(producer: cbc.producer, scope: cbc.scope, inputs: [], output: fileListPath), delegate, contents: contents.bytes, permissions: nil, preparesForIndexing: false, additionalTaskOrderingOptions: [.immediate, .ignorePhaseOrdering])
+            cbc.producer.writeFileSpec.constructFileTasks(CommandBuildContext(producer: cbc.producer, scope: cbc.scope, inputs: [], output: fileListPath), delegate, contents: inputFileListContents(cbc), permissions: nil, preparesForIndexing: false, additionalTaskOrderingOptions: [.immediate, .ignorePhaseOrdering])
             inputPaths.append(fileListPath)
         }
 
@@ -1600,8 +1595,7 @@ public final class LibtoolLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @u
             // Define the linker file list.
             let fileListPath = cbc.scope.evaluate(BuiltinMacros.__INPUT_FILE_LIST_PATH__)
             if !fileListPath.isEmpty {
-                let contents = cbc.inputs.map({ return $0.absolutePath.strWithPosixSlashes + "\n" }).joined(separator: "")
-                cbc.producer.writeFileSpec.constructFileTasks(CommandBuildContext(producer: cbc.producer, scope: cbc.scope, inputs: [], output: fileListPath), delegate, contents: ByteString(encodingAsUTF8: contents), permissions: nil, preparesForIndexing: false, additionalTaskOrderingOptions: [.immediate, .ignorePhaseOrdering])
+                cbc.producer.writeFileSpec.constructFileTasks(CommandBuildContext(producer: cbc.producer, scope: cbc.scope, inputs: [], output: fileListPath), delegate, contents: inputFileListContents(cbc), permissions: nil, preparesForIndexing: false, additionalTaskOrderingOptions: [.immediate, .ignorePhaseOrdering])
                 inputPaths.append(fileListPath)
             }
         } else {
diff --git a/Sources/SWBGenericUnixPlatform/Specs/UnixLd.xcspec b/Sources/SWBGenericUnixPlatform/Specs/UnixLd.xcspec
@@ -180,6 +180,16 @@
                 Name = "SWIFTC__INPUT_FILE_LIST_PATH__";
                 Type = Path;
                 Condition = "NO";
+            },
+            {
+                Name = "LINKER_FILE_LIST_FORMAT";
+                Type = Enumeration;
+                Values = (
+                    unescapedNewlineSeparated,
+                    unixShellQuotedNewlineSeparated,
+                    windowsShellQuotedNewlineSeparated,
+                );
+                DefaultValue = unixShellQuotedNewlineSeparated;
             }
         );
     },
diff --git a/Sources/SWBGenericUnixPlatform/Specs/UnixLibtool.xcspec b/Sources/SWBGenericUnixPlatform/Specs/UnixLibtool.xcspec
@@ -68,6 +68,16 @@
                 Type = Boolean;
                 DefaultValue = YES;
             },
+            {
+                Name = "LINKER_FILE_LIST_FORMAT";
+                Type = Enumeration;
+                Values = (
+                    unescapedNewlineSeparated,
+                    unixShellQuotedNewlineSeparated,
+                    windowsShellQuotedNewlineSeparated,
+                );
+                DefaultValue = unixShellQuotedNewlineSeparated;
+            }
         );
     },
 )
diff --git a/Sources/SWBUtil/ArgumentSplitting.swift b/Sources/SWBUtil/ArgumentSplitting.swift
@@ -280,3 +280,39 @@ public final class LLVMStyleCommandCodec: CommandSequenceEncodable, CommandSeque
     private static let space = Character(" ")
     private static let dollarSign = Character("$")
 }
+
+/// Suitable for escaping Windows process arguments, but NOT command lines which will be interpreted by a shell
+public final class WindowsProcessArgumentsCodec: CommandSequenceEncodable, Sendable {
+    public init() {}
+
+    // Adapted from swift-testing's process spawning code.
+    public func encode(_ sequence: [String]) -> String {
+        return sequence.lazy
+            .map { arg in
+                if !arg.contains(where: {" \t\n\"".contains($0)}) {
+                    return arg
+                }
+
+                var quoted = "\""
+                var unquoted = arg.unicodeScalars
+                while !unquoted.isEmpty {
+                    guard let firstNonBackslash = unquoted.firstIndex(where: { $0 != "\\" }) else {
+                        let backslashCount = unquoted.count
+                        quoted.append(String(repeating: "\\", count: backslashCount * 2))
+                        break
+                    }
+                    let backslashCount = unquoted.distance(from: unquoted.startIndex, to: firstNonBackslash)
+                    if (unquoted[firstNonBackslash] == "\"") {
+                        quoted.append(String(repeating: "\\", count: backslashCount * 2 + 1))
+                        quoted.append(String(unquoted[firstNonBackslash]))
+                    } else {
+                        quoted.append(String(repeating: "\\", count: backslashCount))
+                        quoted.append(String(unquoted[firstNonBackslash]))
+                    }
+                    unquoted.removeFirst(backslashCount + 1)
+                }
+                quoted.append("\"")
+                return quoted
+            }.joined(separator: " ")
+    }
+}
diff --git a/Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec b/Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec
@@ -196,6 +196,16 @@
                 Type = Path;
                 Condition = "NO";
             },
+            {
+                Name = "LINKER_FILE_LIST_FORMAT";
+                Type = Enumeration;
+                Values = (
+                    unescapedNewlineSeparated,
+                    unixShellQuotedNewlineSeparated,
+                    windowsShellQuotedNewlineSeparated,
+                );
+                DefaultValue = windowsShellQuotedNewlineSeparated;
+            },
             {
                 Name = "ALTERNATE_LINKER";
                 Type = String;
diff --git a/Sources/SWBWindowsPlatform/Specs/WindowsLibtool.xcspec b/Sources/SWBWindowsPlatform/Specs/WindowsLibtool.xcspec
@@ -63,6 +63,16 @@
                 );
                 IsInputDependency = Yes;
             },
+            {
+                Name = "LINKER_FILE_LIST_FORMAT";
+                Type = Enumeration;
+                Values = (
+                    unescapedNewlineSeparated,
+                    unixShellQuotedNewlineSeparated,
+                    windowsShellQuotedNewlineSeparated,
+                );
+                DefaultValue = windowsShellQuotedNewlineSeparated;
+            },
         );
     },
 )
diff --git a/Tests/SWBBuildSystemTests/BuildOperationTests.swift b/Tests/SWBBuildSystemTests/BuildOperationTests.swift
@@ -382,6 +382,124 @@ fileprivate struct BuildOperationTests: CoreBasedTests {
         }
     }
 
+    @Test(.requireSDKs(.host))
+    func commandLineTool_whitespaceEscaping() async throws {
+        try await withTemporaryDirectory { (tmpDir: Path) in
+            let tmpDir = tmpDir.join("has whitespace")
+            let testProject = try await TestProject(
+                "TestProject",
+                sourceRoot: tmpDir,
+                groupTree: TestGroup(
+                    "SomeFiles",
+                    children: [
+                        TestFile("main.swift"),
+                        TestFile("dynamic.swift"),
+                        TestFile("static.swift"),
+                    ]),
+                buildConfigurations: [
+                    TestBuildConfiguration("Debug", buildSettings: [
+                        "ARCHS": "$(ARCHS_STANDARD)",
+                        "CODE_SIGNING_ALLOWED": ProcessInfo.processInfo.hostOperatingSystem() == .macOS ? "YES" : "NO",
+                        "CODE_SIGN_IDENTITY": "-",
+                        "CODE_SIGN_ENTITLEMENTS": "Entitlements.plist",
+                        "DEFINES_MODULE": "YES",
+                        "PRODUCT_NAME": "$(TARGET_NAME)",
+                        "SDKROOT": "$(HOST_PLATFORM)",
+                        "SUPPORTED_PLATFORMS": "$(HOST_PLATFORM)",
+                        "SWIFT_VERSION": swiftVersion,
+                        "GCC_GENERATE_DEBUGGING_SYMBOLS": "YES",
+                    ])
+                ],
+                targets: [
+                    TestStandardTarget(
+                        "tool",
+                        type: .commandLineTool,
+                        buildConfigurations: [
+                            TestBuildConfiguration("Debug", buildSettings: [
+                                "LD_RUNPATH_SEARCH_PATHS": "@loader_path/",
+                            ])
+                        ],
+                        buildPhases: [
+                            TestSourcesBuildPhase(["main.swift"]),
+                            TestFrameworksBuildPhase([
+                                TestBuildFile(.target("dynamiclib")),
+                                TestBuildFile(.target("staticlib")),
+                            ])
+                        ],
+                        dependencies: [
+                            "dynamiclib",
+                            "staticlib",
+                        ]
+                    ),
+                    TestStandardTarget(
+                        "dynamiclib",
+                        type: .dynamicLibrary,
+                        buildConfigurations: [
+                            TestBuildConfiguration("Debug", buildSettings: [
+                                "DYLIB_INSTALL_NAME_BASE": "$ORIGIN",
+                                "DYLIB_INSTALL_NAME_BASE[sdk=macosx*]": "@rpath",
+
+                                // FIXME: Find a way to make these default
+                                "EXECUTABLE_PREFIX": "lib",
+                                "EXECUTABLE_PREFIX[sdk=windows*]": "",
+                            ])
+                        ],
+                        buildPhases: [
+                            TestSourcesBuildPhase(["dynamic.swift"]),
+                        ]
+                    ),
+                    TestStandardTarget(
+                        "staticlib",
+                        type: .staticLibrary,
+                        buildConfigurations: [
+                            TestBuildConfiguration("Debug", buildSettings: [
+                                // FIXME: Find a way to make these default
+                                "EXECUTABLE_PREFIX": "lib",
+                                "EXECUTABLE_PREFIX[sdk=windows*]": "",
+                            ])
+                        ],
+                        buildPhases: [
+                            TestSourcesBuildPhase(["static.swift"]),
+                        ]
+                    ),
+                ])
+            let core = try await getCore()
+            let tester = try await BuildOperationTester(core, testProject, simulated: false)
+
+            let projectDir = tester.workspace.projects[0].sourceRoot
+
+            try await tester.fs.writeFileContents(projectDir.join("main.swift")) { stream in
+                stream <<< "import dynamiclib\n"
+                stream <<< "import staticlib\n"
+                stream <<< "dynamicLib()\n"
+                stream <<< "dynamicLib()\n"
+                stream <<< "staticLib()\n"
+                stream <<< "print(\"Hello world\")\n"
+            }
+
+            try await tester.fs.writeFileContents(projectDir.join("dynamic.swift")) { stream in
+                stream <<< "public func dynamicLib() { }"
+            }
+
+            try await tester.fs.writeFileContents(projectDir.join("static.swift")) { stream in
+                stream <<< "public func staticLib() { }"
+            }
+
+            try await tester.fs.writePlist(projectDir.join("Entitlements.plist"), .plDict([:]))
+
+            let provisioningInputs = [
+                "dynamiclib": ProvisioningTaskInputs(identityHash: "-", signedEntitlements: .plDict([:]), simulatedEntitlements: .plDict([:])),
+                "staticlib": ProvisioningTaskInputs(identityHash: "-", signedEntitlements: .plDict([:]), simulatedEntitlements: .plDict([:])),
+                "tool": ProvisioningTaskInputs(identityHash: "-", signedEntitlements: .plDict([:]), simulatedEntitlements: .plDict([:]))
+            ]
+
+            let destination: RunDestinationInfo = .host
+            try await tester.checkBuild(runDestination: destination, persistent: true, signableTargets: Set(provisioningInputs.keys), signableTargetInputs: provisioningInputs) { results in
+                results.checkNoErrors()
+            }
+        }
+    }
+
     @Test(.requireSDKs(.macOS))
     func unitTestWithGeneratedEntryPointViaMacOSOverride() async throws {
         try await withTemporaryDirectory(removeTreeOnDeinit: false) { (tmpDir: Path) in
diff --git a/Tests/SWBUtilTests/ArgumentSplittingTests.swift b/Tests/SWBUtilTests/ArgumentSplittingTests.swift
@@ -283,4 +283,13 @@ import Testing
         #expect(UNIXShellCommandCodec(encodingStrategy: .singleQuotes, joinSequence: " ", encodingBehavior: .fullCommandLine)?.encode(["This", "is", "typical"]) == "This is typical")
         #expect(UNIXShellCommandCodec(encodingStrategy: .singleQuotes, joinSequence: "   ", encodingBehavior: .fullCommandLine)?.encode(["This", "is", "apparently", "more", "readable"]) == "This   is   apparently   more   readable")
     }
+
+    @Test
+    func windowsProcessArgsEncoding() {
+        #expect(WindowsProcessArgumentsCodec().encode([#"normalarg"#]) == #"normalarg"#)
+        #expect(WindowsProcessArgumentsCodec().encode([#"argument with spaces"#]) == #""argument with spaces""#)
+        #expect(WindowsProcessArgumentsCodec().encode([#"\windows\path\ending\in\backslash\"#]) == #"\windows\path\ending\in\backslash\"#)
+        #expect(WindowsProcessArgumentsCodec().encode([#"\windows\path\with space\ending\in\backslash\"#]) == #""\windows\path\with space\ending\in\backslash\\""#)
+        #expect(WindowsProcessArgumentsCodec().encode([#"has"quote"#]) == #""has\"quote""#)
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -116,6 +116,8 @@ private final class EnumBuildOptionType : BuildOptionType {`
`116`	`116`	`return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<LinkerDriverChoice>`
`117`	`117`	`case "SWIFT_API_DIGESTER_MODE":`
`118`	`118`	`return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<SwiftAPIDigesterMode>`
	`119`	`+ case "LINKER_FILE_LIST_FORMAT":`
	`120`	`+ return try namespace.declareEnumMacro(name) as EnumMacroDeclaration<LinkerFileListFormat>`
`119`	`121`	`default:`
`120`	`122`	`return try namespace.declareStringMacro(name)`
`121`	`123`	`}`