From 3ac5414b57c4248da3ca728047069f2202faac7e Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 21 Sep 2024 17:20:49 -0400 Subject: [PATCH 1/3] Avoid truncating the path to the test executable on Windows. On Windows, we get the path to the (current) test executable by calling `GetModuleFileNameW()`. This function has odd behaviour when the input buffer is too short. Rather than returning `false` or `-1` or some such, as you might expect, it returns successfully but truncates the path (and null-terminates while doing so.) The function _does_ set the last error in this case to `ERROR_INSUFFICIENT_BUFFER`, indicating we need a larger buffer. (The error behaviour on Windows XP is different, but we don't support Windows XP. If you decide to add support for Windows XP to Swift Testing, keep this in mind.) So this PR checks for `ERROR_INSUFFICIENT_BUFFER` and loops with a larger buffer, similar to what we do on Darwin. --- Sources/Testing/ExitTests/SpawnProcess.swift | 4 +-- .../Additions/CommandLineAdditions.swift | 30 +++++++++++++------ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index 50d78010a..c5d87e6f7 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -46,9 +46,9 @@ func spawnExecutable( ) throws -> ProcessID { // Darwin and Linux differ in their optionality for the posix_spawn types we // use, so use this typealias to paper over the differences. -#if SWT_TARGET_OS_APPLE +#if SWT_TARGET_OS_APPLE || os(FreeBSD) typealias P = T? -#elseif os(Linux) || os(FreeBSD) +#elseif os(Linux) typealias P = T #endif diff --git a/Sources/Testing/Support/Additions/CommandLineAdditions.swift b/Sources/Testing/Support/Additions/CommandLineAdditions.swift index c354e386a..f8a2eb691 100644 --- a/Sources/Testing/Support/Additions/CommandLineAdditions.swift +++ b/Sources/Testing/Support/Additions/CommandLineAdditions.swift @@ -16,7 +16,7 @@ extension CommandLine { get throws { #if os(macOS) var result: String? - var bufferCount = UInt32(1024) + var bufferCount = UInt32(PATH_MAX) while result == nil { result = withUnsafeTemporaryAllocation(of: CChar.self, capacity: Int(bufferCount)) { buffer in // _NSGetExecutablePath returns 0 on success and -1 if bufferCount is @@ -40,7 +40,7 @@ extension CommandLine { } #elseif os(FreeBSD) var mib = [CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1] - try mib.withUnsafeMutableBufferPointer { mib in + return try mib.withUnsafeMutableBufferPointer { mib in var bufferCount = 0 guard 0 == sysctl(mib.baseAddress!, .init(mib.count), nil, &bufferCount, nil, 0) else { throw CError(rawValue: swt_errno()) @@ -53,15 +53,27 @@ extension CommandLine { } } #elseif os(Windows) - return try withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: Int(MAX_PATH) * 2) { buffer in - guard 0 != GetModuleFileNameW(nil, buffer.baseAddress!, DWORD(buffer.count)) else { - throw Win32Error(rawValue: GetLastError()) - } - guard let path = String.decodeCString(buffer.baseAddress!, as: UTF16.self)?.result else { - throw Win32Error(rawValue: DWORD(ERROR_ILLEGAL_CHARACTER)) + var result: String? + var bufferCount = Int(MAX_PATH) + while result == nil { + result = try withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: bufferCount) { buffer in + SetLastError(DWORD(ERROR_SUCCESS)) + _ = GetModuleFileNameW(nil, buffer.baseAddress!, DWORD(buffer.count)) + switch GetLastError() { + case DWORD(ERROR_SUCCESS): + result = String.decodeCString(buffer.baseAddress!, as: UTF16.self)?.result + if result == nil { + throw Win32Error(rawValue: DWORD(ERROR_ILLEGAL_CHARACTER)) + } + case DWORD(ERROR_INSUFFICIENT_BUFFER): + bufferCount += Int(MAX_PATH) + return nil + case let errorCode: + throw Win32Error(rawValue: errorCode) + } } - return path } + return result! #elseif os(WASI) // WASI does not really have the concept of a file system path to the main // executable, so simply return the first argument--presumably the program From ae7683a0074c238a8e8cb65e150acaa02f5e20bd Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 21 Sep 2024 17:42:10 -0400 Subject: [PATCH 2/3] Consistently just set result rather than returning it --- .../Testing/Support/Additions/CommandLineAdditions.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Sources/Testing/Support/Additions/CommandLineAdditions.swift b/Sources/Testing/Support/Additions/CommandLineAdditions.swift index f8a2eb691..3a8c34bb9 100644 --- a/Sources/Testing/Support/Additions/CommandLineAdditions.swift +++ b/Sources/Testing/Support/Additions/CommandLineAdditions.swift @@ -18,14 +18,13 @@ extension CommandLine { var result: String? var bufferCount = UInt32(PATH_MAX) while result == nil { - result = withUnsafeTemporaryAllocation(of: CChar.self, capacity: Int(bufferCount)) { buffer in + withUnsafeTemporaryAllocation(of: CChar.self, capacity: Int(bufferCount)) { buffer in // _NSGetExecutablePath returns 0 on success and -1 if bufferCount is // too small. If that occurs, we'll return nil here and loop with the // new value of bufferCount. if 0 == _NSGetExecutablePath(buffer.baseAddress, &bufferCount) { - return String(cString: buffer.baseAddress!) + result = String(cString: buffer.baseAddress!) } - return nil } } return result! @@ -56,7 +55,7 @@ extension CommandLine { var result: String? var bufferCount = Int(MAX_PATH) while result == nil { - result = try withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: bufferCount) { buffer in + try withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: bufferCount) { buffer in SetLastError(DWORD(ERROR_SUCCESS)) _ = GetModuleFileNameW(nil, buffer.baseAddress!, DWORD(buffer.count)) switch GetLastError() { @@ -67,7 +66,6 @@ extension CommandLine { } case DWORD(ERROR_INSUFFICIENT_BUFFER): bufferCount += Int(MAX_PATH) - return nil case let errorCode: throw Win32Error(rawValue: errorCode) } From 83be4e77b3f643176a583dff59dcf9f443bdb32f Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 21 Sep 2024 17:51:59 -0400 Subject: [PATCH 3/3] Force looping in debug mode --- .../Testing/Support/Additions/CommandLineAdditions.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Sources/Testing/Support/Additions/CommandLineAdditions.swift b/Sources/Testing/Support/Additions/CommandLineAdditions.swift index 3a8c34bb9..762ab7290 100644 --- a/Sources/Testing/Support/Additions/CommandLineAdditions.swift +++ b/Sources/Testing/Support/Additions/CommandLineAdditions.swift @@ -16,7 +16,11 @@ extension CommandLine { get throws { #if os(macOS) var result: String? +#if DEBUG + var bufferCount = UInt32(1) // force looping +#else var bufferCount = UInt32(PATH_MAX) +#endif while result == nil { withUnsafeTemporaryAllocation(of: CChar.self, capacity: Int(bufferCount)) { buffer in // _NSGetExecutablePath returns 0 on success and -1 if bufferCount is @@ -53,7 +57,11 @@ extension CommandLine { } #elseif os(Windows) var result: String? +#if DEBUG + var bufferCount = Int(1) // force looping +#else var bufferCount = Int(MAX_PATH) +#endif while result == nil { try withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: bufferCount) { buffer in SetLastError(DWORD(ERROR_SUCCESS))