spawnWindows: Improve worst-case performance considerably + tests

lib/std/os.zig

@@ -1944,19 +1944,7 @@ pub fn getenvW(key: [*:0]const u16) ?[:0]const u16 {
         while (ptr[i] != 0) : (i += 1) {}
         const this_value = ptr[value_start..i :0];
 
-        const key_string_bytes = @intCast(u16, key_slice.len * 2);
-        const key_string = windows.UNICODE_STRING{
-            .Length = key_string_bytes,
-            .MaximumLength = key_string_bytes,
-            .Buffer = @intToPtr([*]u16, @ptrToInt(key)),
-        };
-        const this_key_string_bytes = @intCast(u16, this_key.len * 2);
-        const this_key_string = windows.UNICODE_STRING{
-            .Length = this_key_string_bytes,
-            .MaximumLength = this_key_string_bytes,
-            .Buffer = this_key.ptr,
-        };
-        if (windows.ntdll.RtlEqualUnicodeString(&key_string, &this_key_string, windows.TRUE) == windows.TRUE) {
+        if (windows.eqlIgnoreCaseWTF16(key_slice, this_key)) {
             return this_value;
         }
 

lib/std/os/windows.zig

@@ -1624,6 +1624,9 @@ pub fn CreateProcessW(
             .RING2SEG_MUST_BE_MOVABLE,
             .RELOC_CHAIN_XEEDS_SEGLIM,
             .INFLOOP_IN_RELOC_CHAIN, // MAX_EXEC_ERROR in errno.cpp
+            // This one is not mapped to ENOEXEC but it is possible, for example
+            // when calling CreateProcessW on a plain text file with a .exe extension
+            .EXE_MACHINE_TYPE_MISMATCH,
             => return error.InvalidExe,
             else => |err| return unexpectedError(err),
         }
@@ -1824,6 +1827,23 @@ pub fn nanoSecondsToFileTime(ns: i128) FILETIME {
     };
 }
 
+/// Compares two WTF16 strings using RtlEqualUnicodeString
+pub fn eqlIgnoreCaseWTF16(a: []const u16, b: []const u16) bool {
+    const a_bytes = @intCast(u16, a.len * 2);
+    const a_string = UNICODE_STRING{
+        .Length = a_bytes,
+        .MaximumLength = a_bytes,
+        .Buffer = @intToPtr([*]u16, @ptrToInt(a.ptr)),
+    };
+    const b_bytes = @intCast(u16, b.len * 2);
+    const b_string = UNICODE_STRING{
+        .Length = b_bytes,
+        .MaximumLength = b_bytes,
+        .Buffer = @intToPtr([*]u16, @ptrToInt(b.ptr)),
+    };
+    return ntdll.RtlEqualUnicodeString(&a_string, &b_string, TRUE) == TRUE;
+}
+
 pub const PathSpace = struct {
     data: [PATH_MAX_WIDE:0]u16,
     len: usize,
@@ -3682,6 +3702,20 @@ pub const RTL_DRIVE_LETTER_CURDIR = extern struct {
 
 pub const PPS_POST_PROCESS_INIT_ROUTINE = ?*const fn () callconv(.C) void;
 
+pub const FILE_DIRECTORY_INFORMATION = extern struct {
+    NextEntryOffset: ULONG,
+    FileIndex: ULONG,
+    CreationTime: LARGE_INTEGER,
+    LastAccessTime: LARGE_INTEGER,
+    LastWriteTime: LARGE_INTEGER,
+    ChangeTime: LARGE_INTEGER,
+    EndOfFile: LARGE_INTEGER,
+    AllocationSize: LARGE_INTEGER,
+    FileAttributes: ULONG,
+    FileNameLength: ULONG,
+    FileName: [1]WCHAR,
+};
+
 pub const FILE_BOTH_DIR_INFORMATION = extern struct {
     NextEntryOffset: ULONG,
     FileIndex: ULONG,

lib/std/os/windows/kernel32.zig

@@ -177,6 +177,8 @@ pub extern "kernel32" fn GetEnvironmentStringsW() callconv(WINAPI) ?[*:0]u16;
 
 pub extern "kernel32" fn GetEnvironmentVariableW(lpName: LPWSTR, lpBuffer: [*]u16, nSize: DWORD) callconv(WINAPI) DWORD;
 
+pub extern "kernel32" fn SetEnvironmentVariableW(lpName: LPCWSTR, lpValue: ?LPCWSTR) callconv(WINAPI) BOOL;
+
 pub extern "kernel32" fn GetExitCodeProcess(hProcess: HANDLE, lpExitCode: *DWORD) callconv(WINAPI) BOOL;
 
 pub extern "kernel32" fn GetFileSizeEx(hFile: HANDLE, lpFileSize: *LARGE_INTEGER) callconv(WINAPI) BOOL;

test/standalone.zig

@@ -63,6 +63,10 @@ pub fn addCases(cases: *tests.StandaloneContext) void {
         cases.addBuildFile("test/standalone/load_dynamic_library/build.zig", .{});
     }
 
+    if (builtin.os.tag == .windows) {
+        cases.addBuildFile("test/standalone/windows_spawn/build.zig", .{});
+    }
+
     cases.addBuildFile("test/standalone/c_compiler/build.zig", .{
         .build_modes = true,
         .cross_targets = true,

test/standalone/windows_spawn/build.zig

@@ -0,0 +1,16 @@
+const Builder = @import("std").build.Builder;
+
+pub fn build(b: *Builder) void {
+    const mode = b.standardReleaseOptions();
+
+    const hello = b.addExecutable("hello", "hello.zig");
+    hello.setBuildMode(mode);
+
+    const main = b.addExecutable("main", "main.zig");
+    main.setBuildMode(mode);
+    const run = main.run();
+    run.addArtifactArg(hello);
+
+    const test_step = b.step("test", "Test it");
+    test_step.dependOn(&run.step);
+}

test/standalone/windows_spawn/hello.zig

@@ -0,0 +1,6 @@
+const std = @import("std");
+
+pub fn main() !void {
+    const stdout = std.io.getStdOut().writer();
+    try stdout.writeAll("hello from exe\n");
+}

test/standalone/windows_spawn/main.zig

@@ -0,0 +1,161 @@
+const std = @import("std");
+const windows = std.os.windows;
+const utf16Literal = std.unicode.utf8ToUtf16LeStringLiteral;
+
+pub fn main() anyerror!void {
+    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
+    defer if (gpa.deinit()) @panic("found memory leaks");
+    const allocator = gpa.allocator();
+
+    var it = try std.process.argsWithAllocator(allocator);
+    defer it.deinit();
+    _ = it.next() orelse unreachable; // skip binary name
+    const hello_exe_cache_path = it.next() orelse unreachable;
+
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+
+    const tmp_absolute_path = try tmp.dir.realpathAlloc(allocator, ".");
+    defer allocator.free(tmp_absolute_path);
+    const tmp_absolute_path_w = try std.unicode.utf8ToUtf16LeWithNull(allocator, tmp_absolute_path);
+    defer allocator.free(tmp_absolute_path_w);
+    const cwd_absolute_path = try std.fs.cwd().realpathAlloc(allocator, ".");
+    defer allocator.free(cwd_absolute_path);
+    const tmp_relative_path = try std.fs.path.relative(allocator, cwd_absolute_path, tmp_absolute_path);
+    defer allocator.free(tmp_relative_path);
+
+    // Clear PATH
+    std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
+        utf16Literal("PATH"),
+        null,
+    ) == windows.TRUE);
+
+    // Set PATHEXT to something predictable
+    std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
+        utf16Literal("PATHEXT"),
+        utf16Literal(".COM;.EXE;.BAT;.CMD;.JS"),
+    ) == windows.TRUE);
+
+    // No PATH, so it should fail to find anything not in the cwd
+    try testExecError(error.FileNotFound, allocator, "something_missing");
+
+    std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
+        utf16Literal("PATH"),
+        tmp_absolute_path_w,
+    ) == windows.TRUE);
+
+    // Move hello.exe into the tmp dir which is now added to the path
+    try std.fs.cwd().copyFile(hello_exe_cache_path, tmp.dir, "hello.exe", .{});
+
+    // with extension should find the .exe (case insensitive)
+    try testExec(allocator, "HeLLo.exe", "hello from exe\n");
+    // without extension should find the .exe (case insensitive)
+    try testExec(allocator, "heLLo", "hello from exe\n");
+
+    // now add a .bat
+    try tmp.dir.writeFile("hello.bat", "@echo hello from bat");
+    // and a .cmd
+    try tmp.dir.writeFile("hello.cmd", "@echo hello from cmd");
+
+    // with extension should find the .bat (case insensitive)
+    try testExec(allocator, "heLLo.bat", "hello from bat\r\n");
+    // with extension should find the .cmd (case insensitive)
+    try testExec(allocator, "heLLo.cmd", "hello from cmd\r\n");
+    // without extension should find the .exe (since its first in PATHEXT)
+    try testExec(allocator, "heLLo", "hello from exe\n");
+
+    // now rename the exe to not have an extension
+    try tmp.dir.rename("hello.exe", "hello");
+
+    // with extension should now fail
+    try testExecError(error.FileNotFound, allocator, "hello.exe");
+    // without extension should succeed (case insensitive)
+    try testExec(allocator, "heLLo", "hello from exe\n");
+
+    try tmp.dir.makeDir("something");
+    try tmp.dir.rename("hello", "something/hello.exe");
+
+    const relative_path_no_ext = try std.fs.path.join(allocator, &.{ tmp_relative_path, "something/hello" });
+    defer allocator.free(relative_path_no_ext);
+
+    // Giving a full relative path to something/hello should work
+    try testExec(allocator, relative_path_no_ext, "hello from exe\n");
+    // But commands with path separators get excluded from PATH searching, so this will fail
+    try testExecError(error.FileNotFound, allocator, "something/hello");
+
+    // Now that .BAT is the first PATHEXT that should be found, this should succeed
+    try testExec(allocator, "heLLo", "hello from bat\r\n");
+
+    // Add a hello.exe that is not a valid executable
+    try tmp.dir.writeFile("hello.exe", "invalid");
+
+    // Trying to execute it with extension will give InvalidExe. This is a special
+    // case for .EXE extensions, where if they ever try to get executed but they are
+    // invalid, that gets treated as a fatal error wherever they are found and InvalidExe
+    // is returned immediately.
+    try testExecError(error.InvalidExe, allocator, "hello.exe");
+    // Same thing applies to the command with no extension--even though there is a
+    // hello.bat that could be executed, it should stop after it tries executing
+    // hello.exe and getting InvalidExe.
+    try testExecError(error.InvalidExe, allocator, "hello");
+
+    // If we now rename hello.exe to have no extension, it will behave differently
+    try tmp.dir.rename("hello.exe", "hello");
+
+    // Now, trying to execute it without an extension should treat InvalidExe as recoverable
+    // and skip over it and find hello.bat and execute that
+    try testExec(allocator, "hello", "hello from bat\r\n");
+
+    // If we rename the invalid exe to something else
+    try tmp.dir.rename("hello", "goodbye");
+    // Then we should now get FileNotFound when trying to execute 'goodbye',
+    // since that is what the original error will be after searching for 'goodbye'
+    // in the cwd. It will try to execute 'goodbye' from the PATH but the InvalidExe error
+    // should be ignored in this case.
+    try testExecError(error.FileNotFound, allocator, "goodbye");
+
+    // Now let's set the tmp dir as the cwd and set the path only include the "something" sub dir
+    try tmp.dir.setAsCwd();
+    const something_subdir_abs_path = try std.mem.concatWithSentinel(allocator, u16, &.{ tmp_absolute_path_w, utf16Literal("\\something") }, 0);
+    defer allocator.free(something_subdir_abs_path);
+
+    std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
+        utf16Literal("PATH"),
+        something_subdir_abs_path,
+    ) == windows.TRUE);
+
+    // Now trying to execute goodbye should give error.InvalidExe since it's the original
+    // error that we got when trying within the cwd
+    try testExecError(error.InvalidExe, allocator, "goodbye");
+
+    // hello should still find the .bat
+    try testExec(allocator, "hello", "hello from bat\r\n");
+
+    // If we rename something/hello.exe to something/goodbye.exe
+    try tmp.dir.rename("something/hello.exe", "something/goodbye.exe");
+    // And try to execute goodbye, then the one in something should be found
+    // since the one in cwd is an invalid executable
+    try testExec(allocator, "goodbye", "hello from exe\n");
+
+    // If we use an absolute path to execute the invalid goodbye
+    const goodbye_abs_path = try std.mem.join(allocator, "\\", &.{ tmp_absolute_path, "goodbye" });
+    defer allocator.free(goodbye_abs_path);
+    // then the PATH should not be searched and we should get InvalidExe
+    try testExecError(error.InvalidExe, allocator, goodbye_abs_path);
+}
+
+fn testExecError(err: anyerror, allocator: std.mem.Allocator, command: []const u8) !void {
+    return std.testing.expectError(err, testExec(allocator, command, ""));
+}
+
+fn testExec(allocator: std.mem.Allocator, command: []const u8, expected_stdout: []const u8) !void {
+    var result = try std.ChildProcess.exec(.{
+        .allocator = allocator,
+        .argv = &[_][]const u8{command},
+    });
+    defer allocator.free(result.stdout);
+    defer allocator.free(result.stderr);
+
+    try std.testing.expectEqualStrings("", result.stderr);
+    try std.testing.expectEqualStrings(expected_stdout, result.stdout);
+}