From 6a1021fb7d14604b4f66b2c992f47eadcc812989 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Fri, 16 Dec 2022 19:33:06 -0800 Subject: [PATCH 1/5] ChildProcess.spawnWindows: Fix PATH search when the ext is in the command For example, if the command is specified as `something.exe`, the retry will now try: ``` C:\some\path\something.exe C:\some\path\something.exe.COM C:\some\path\something.exe.EXE C:\some\path\something.exe.BAT ... etc ... ``` whereas before it would only try the versions with an added extension from `PATHEXT`, which would cause the retry to fail on things that it should find. --- lib/std/child_process.zig | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index b706f0aecb..a5e066c1d5 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -995,12 +995,22 @@ pub const ChildProcess = struct { const path_no_ext = try fs.path.join(self.allocator, &[_][]const u8{ search_path, app_name }); defer self.allocator.free(path_no_ext); + const path_no_ext_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, path_no_ext); + defer self.allocator.free(path_no_ext_w); + + if (windowsCreateProcess(path_no_ext_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { + break :retry; + } else |err| switch (err) { + error.FileNotFound, error.AccessDenied => {}, + else => return err, + } + var ext_it = mem.tokenize(u8, PATHEXT, ";"); while (ext_it.next()) |app_ext| { - const joined_path = try mem.concat(self.allocator, u8, &[_][]const u8{ path_no_ext, app_ext }); - defer self.allocator.free(joined_path); + const ext_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, app_ext); + defer self.allocator.free(ext_w); - const joined_path_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, joined_path); + const joined_path_w = try std.mem.concatWithSentinel(self.allocator, u16, &.{ path_no_ext_w, ext_w }, 0); defer self.allocator.free(joined_path_w); if (windowsCreateProcess(joined_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { From b362cbbc9fb4ab09c6a52948476d3d7dff27d795 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Fri, 16 Dec 2022 20:08:28 -0800 Subject: [PATCH 2/5] ChildProcess.spawnWindows: Drastically reduce the amount of allocation during FileNotFound recovery Avoid a lot of unnecessary utf8 -> utf16 conversion and use a single ArrayList buffer for all the joined paths instead of a separate allocation for each join --- lib/std/child_process.zig | 55 ++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index a5e066c1d5..be38b9ae45 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -968,52 +968,41 @@ pub const ChildProcess = struct { windowsCreateProcess(app_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo) catch |no_path_err| { if (no_path_err != error.FileNotFound) return no_path_err; - var free_path = true; - const PATH = process.getEnvVarOwned(self.allocator, "PATH") catch |err| switch (err) { - error.EnvironmentVariableNotFound => blk: { - free_path = false; - break :blk ""; - }, - else => |e| return e, - }; - defer if (free_path) self.allocator.free(PATH); + const PATH: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATH")) orelse &[_:0]u16{}; + const PATHEXT: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATHEXT")) orelse &[_:0]u16{}; - var free_path_ext = true; - const PATHEXT = process.getEnvVarOwned(self.allocator, "PATHEXT") catch |err| switch (err) { - error.EnvironmentVariableNotFound => blk: { - free_path_ext = false; - break :blk ""; - }, - else => |e| return e, - }; - defer if (free_path_ext) self.allocator.free(PATHEXT); + var path_buf = std.ArrayListUnmanaged(u16){}; + defer path_buf.deinit(self.allocator); const app_name = self.argv[0]; + const app_name_trimmed_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, mem.trimLeft(u8, app_name, "\\/")); + defer self.allocator.free(app_name_trimmed_w); - var it = mem.tokenize(u8, PATH, ";"); + var it = mem.tokenize(u16, PATH, &[_]u16{';'}); retry: while (it.next()) |search_path| { - const path_no_ext = try fs.path.join(self.allocator, &[_][]const u8{ search_path, app_name }); - defer self.allocator.free(path_no_ext); + path_buf.clearRetainingCapacity(); + const search_path_trimmed = mem.trimRight(u16, search_path, &[_]u16{ '\\', '/' }); + try path_buf.appendSlice(self.allocator, search_path_trimmed); + try path_buf.append(self.allocator, fs.path.sep); + try path_buf.appendSlice(self.allocator, app_name_trimmed_w); + try path_buf.append(self.allocator, 0); + const path_no_ext = path_buf.items[0 .. path_buf.items.len - 1 :0]; - const path_no_ext_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, path_no_ext); - defer self.allocator.free(path_no_ext_w); - - if (windowsCreateProcess(path_no_ext_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { + if (windowsCreateProcess(path_no_ext.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { break :retry; } else |err| switch (err) { error.FileNotFound, error.AccessDenied => {}, else => return err, } - var ext_it = mem.tokenize(u8, PATHEXT, ";"); - while (ext_it.next()) |app_ext| { - const ext_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, app_ext); - defer self.allocator.free(ext_w); + var ext_it = mem.tokenize(u16, PATHEXT, &[_]u16{';'}); + while (ext_it.next()) |ext| { + path_buf.shrinkRetainingCapacity(path_no_ext.len); + try path_buf.appendSlice(self.allocator, ext); + try path_buf.append(self.allocator, 0); + const joined_path = path_buf.items[0 .. path_buf.items.len - 1 :0]; - const joined_path_w = try std.mem.concatWithSentinel(self.allocator, u16, &.{ path_no_ext_w, ext_w }, 0); - defer self.allocator.free(joined_path_w); - - if (windowsCreateProcess(joined_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { + if (windowsCreateProcess(joined_path.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { break :retry; } else |err| switch (err) { error.FileNotFound => continue, From 5843b7987e705ec01d07cd98a7f7bb10aa579587 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Fri, 16 Dec 2022 22:14:33 -0800 Subject: [PATCH 3/5] Add error.InvalidExe to CreateProcessW error set and handle it in ChildProcess.spawnWindows --- lib/std/child_process.zig | 5 +++-- lib/std/os/windows.zig | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index be38b9ae45..1d1eab2a14 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -966,7 +966,7 @@ pub const ChildProcess = struct { defer self.allocator.free(cmd_line_w); windowsCreateProcess(app_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo) catch |no_path_err| { - if (no_path_err != error.FileNotFound) return no_path_err; + if (no_path_err != error.FileNotFound and no_path_err != error.InvalidExe) return no_path_err; const PATH: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATH")) orelse &[_:0]u16{}; const PATHEXT: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATHEXT")) orelse &[_:0]u16{}; @@ -991,7 +991,7 @@ pub const ChildProcess = struct { if (windowsCreateProcess(path_no_ext.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { break :retry; } else |err| switch (err) { - error.FileNotFound, error.AccessDenied => {}, + error.FileNotFound, error.AccessDenied, error.InvalidExe => {}, else => return err, } @@ -1007,6 +1007,7 @@ pub const ChildProcess = struct { } else |err| switch (err) { error.FileNotFound => continue, error.AccessDenied => continue, + error.InvalidExe => continue, else => return err, } } diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index 0bd695a029..d654a60bd9 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -1569,6 +1569,7 @@ pub const CreateProcessError = error{ AccessDenied, InvalidName, NameTooLong, + InvalidExe, Unexpected, }; @@ -1603,6 +1604,27 @@ pub fn CreateProcessW( .INVALID_PARAMETER => unreachable, .INVALID_NAME => return error.InvalidName, .FILENAME_EXCED_RANGE => return error.NameTooLong, + // These are all the system errors that are mapped to ENOEXEC by + // the undocumented _dosmaperr (old CRT) or __acrt_errno_map_os_error + // (newer CRT) functions. Their code can be found in crt/src/dosmap.c (old SDK) + // or urt/misc/errno.cpp (newer SDK) in the Windows SDK. + .BAD_FORMAT, + .INVALID_STARTING_CODESEG, // MIN_EXEC_ERROR in errno.cpp + .INVALID_STACKSEG, + .INVALID_MODULETYPE, + .INVALID_EXE_SIGNATURE, + .EXE_MARKED_INVALID, + .BAD_EXE_FORMAT, + .ITERATED_DATA_EXCEEDS_64k, + .INVALID_MINALLOCSIZE, + .DYNLINK_FROM_INVALID_RING, + .IOPL_NOT_ENABLED, + .INVALID_SEGDPL, + .AUTODATASEG_EXCEEDS_64k, + .RING2SEG_MUST_BE_MOVABLE, + .RELOC_CHAIN_XEEDS_SEGLIM, + .INFLOOP_IN_RELOC_CHAIN, // MAX_EXEC_ERROR in errno.cpp + => return error.InvalidExe, else => |err| return unexpectedError(err), } } From d3242408d4bed933c589a8143f4a06851795bdd1 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Fri, 16 Dec 2022 22:34:08 -0800 Subject: [PATCH 4/5] spawnWindows: If an exe is found but fails to exec, retry with PATHEXT values appended This matches `cmd.exe` behavior. For example, if there is only a file named `mycommand` in the cwd but it is a Linux executable, then running the command `mycommand` will result in: 'mycommand' is not recognized as an internal or external command, operable program or batch file. However, if there is *both* a `mycommand` (that is a Linux executable) and a `mycommand.exe` that is a valid Windows exe, then running the command `mycommand` will successfully run `mycommand.exe`. --- lib/std/child_process.zig | 124 ++++++++++++++++++++++++-------------- 1 file changed, 79 insertions(+), 45 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 1d1eab2a14..2c99f58b88 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -965,56 +965,90 @@ pub const ChildProcess = struct { const cmd_line_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, cmd_line); defer self.allocator.free(cmd_line_w); - windowsCreateProcess(app_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo) catch |no_path_err| { - if (no_path_err != error.FileNotFound and no_path_err != error.InvalidExe) return no_path_err; - - const PATH: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATH")) orelse &[_:0]u16{}; - const PATHEXT: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATHEXT")) orelse &[_:0]u16{}; - - var path_buf = std.ArrayListUnmanaged(u16){}; - defer path_buf.deinit(self.allocator); - - const app_name = self.argv[0]; - const app_name_trimmed_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, mem.trimLeft(u8, app_name, "\\/")); - defer self.allocator.free(app_name_trimmed_w); - - var it = mem.tokenize(u16, PATH, &[_]u16{';'}); - retry: while (it.next()) |search_path| { - path_buf.clearRetainingCapacity(); - const search_path_trimmed = mem.trimRight(u16, search_path, &[_]u16{ '\\', '/' }); - try path_buf.appendSlice(self.allocator, search_path_trimmed); - try path_buf.append(self.allocator, fs.path.sep); - try path_buf.appendSlice(self.allocator, app_name_trimmed_w); - try path_buf.append(self.allocator, 0); - const path_no_ext = path_buf.items[0 .. path_buf.items.len - 1 :0]; - - if (windowsCreateProcess(path_no_ext.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { - break :retry; - } else |err| switch (err) { - error.FileNotFound, error.AccessDenied, error.InvalidExe => {}, - else => return err, + exec: { + windowsCreateProcess(app_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo) catch |no_path_err| { + switch (no_path_err) { + error.FileNotFound, error.InvalidExe => {}, + else => |e| return e, } - var ext_it = mem.tokenize(u16, PATHEXT, &[_]u16{';'}); - while (ext_it.next()) |ext| { - path_buf.shrinkRetainingCapacity(path_no_ext.len); - try path_buf.appendSlice(self.allocator, ext); - try path_buf.append(self.allocator, 0); - const joined_path = path_buf.items[0 .. path_buf.items.len - 1 :0]; + const PATH: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATH")) orelse &[_:0]u16{}; + const PATHEXT: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATHEXT")) orelse &[_:0]u16{}; - if (windowsCreateProcess(joined_path.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { - break :retry; - } else |err| switch (err) { - error.FileNotFound => continue, - error.AccessDenied => continue, - error.InvalidExe => continue, - else => return err, + var path_buf = std.ArrayListUnmanaged(u16){}; + defer path_buf.deinit(self.allocator); + + // Try again with PATHEXT's extensions appended + { + try path_buf.appendSlice(self.allocator, app_path_w); + var ext_it = mem.tokenize(u16, PATHEXT, &[_]u16{';'}); + while (ext_it.next()) |ext| { + path_buf.shrinkRetainingCapacity(app_path_w.len); + try path_buf.appendSlice(self.allocator, ext); + try path_buf.append(self.allocator, 0); + const path_with_ext = path_buf.items[0 .. path_buf.items.len - 1 :0]; + + if (windowsCreateProcess(path_with_ext.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { + break :exec; + } else |err| switch (err) { + error.FileNotFound, error.AccessDenied, error.InvalidExe => {}, + else => return err, + } } } - } else { - return no_path_err; // return the original error - } - }; + + // app_path_w has the cwd prepended to it if cwd is non-null, so when + // searching the PATH we should make sure we use the app_name verbatim. + var app_name_w_needs_free = false; + const app_name_w = x: { + if (self.cwd) |_| { + app_name_w_needs_free = true; + break :x try unicode.utf8ToUtf16LeWithNull(self.allocator, self.argv[0]); + } else { + break :x app_path_w; + } + }; + defer if (app_name_w_needs_free) self.allocator.free(app_name_w); + + var it = mem.tokenize(u16, PATH, &[_]u16{';'}); + while (it.next()) |search_path| { + path_buf.clearRetainingCapacity(); + const search_path_trimmed = mem.trimRight(u16, search_path, &[_]u16{ '\\', '/' }); + try path_buf.appendSlice(self.allocator, search_path_trimmed); + try path_buf.append(self.allocator, fs.path.sep); + const app_name_trimmed = mem.trimLeft(u16, app_name_w, &[_]u16{ '\\', '/' }); + try path_buf.appendSlice(self.allocator, app_name_trimmed); + try path_buf.append(self.allocator, 0); + const path_no_ext = path_buf.items[0 .. path_buf.items.len - 1 :0]; + + if (windowsCreateProcess(path_no_ext.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { + break :exec; + } else |err| switch (err) { + error.FileNotFound, error.AccessDenied, error.InvalidExe => {}, + else => return err, + } + + var ext_it = mem.tokenize(u16, PATHEXT, &[_]u16{';'}); + while (ext_it.next()) |ext| { + path_buf.shrinkRetainingCapacity(path_no_ext.len); + try path_buf.appendSlice(self.allocator, ext); + try path_buf.append(self.allocator, 0); + const joined_path = path_buf.items[0 .. path_buf.items.len - 1 :0]; + + if (windowsCreateProcess(joined_path.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| { + break :exec; + } else |err| switch (err) { + error.FileNotFound => continue, + error.AccessDenied => continue, + error.InvalidExe => continue, + else => return err, + } + } + } else { + return no_path_err; // return the original error + } + }; + } if (g_hChildStd_IN_Wr) |h| { self.stdin = File{ .handle = h }; From 9e8ac2b6664b03116d7991d57ddb37c8108fffec Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Sat, 17 Dec 2022 03:34:59 -0800 Subject: [PATCH 5/5] spawnWindows: Don't search PATH if app path is absolute --- lib/std/child_process.zig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 2c99f58b88..956ee2adaf 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -997,6 +997,9 @@ pub const ChildProcess = struct { } } + // No need to search the PATH if the app path is absolute + if (fs.path.isAbsoluteWindowsWTF16(app_path_w)) return no_path_err; + // app_path_w has the cwd prepended to it if cwd is non-null, so when // searching the PATH we should make sure we use the app_name verbatim. var app_name_w_needs_free = false;