diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 6f75b01c87..eb6bf81146 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -19,6 +19,7 @@ const builtin = @import("builtin"); const Os = builtin.Os; const TailQueue = std.TailQueue; const maxInt = std.math.maxInt; +const assert = std.debug.assert; pub const ChildProcess = struct { pid: if (builtin.os.tag == .windows) void else i32, @@ -376,19 +377,44 @@ pub const ChildProcess = struct { if (any_ignore) os.close(dev_null_fd); } - var env_map_owned: BufMap = undefined; - var we_own_env_map: bool = undefined; - const env_map = if (self.env_map) |env_map| x: { - we_own_env_map = false; - break :x env_map; - } else x: { - we_own_env_map = true; - env_map_owned = try process.getEnvMap(self.allocator); - break :x &env_map_owned; - }; - defer { - if (we_own_env_map) env_map_owned.deinit(); + var arena_allocator = std.heap.ArenaAllocator.init(self.allocator); + defer arena_allocator.deinit(); + const arena = &arena_allocator.allocator; + + // The POSIX standard does not allow malloc() between fork() and execve(), + // and `self.allocator` may be a libc allocator. + // I have personally observed the child process deadlocking when it tries + // to call malloc() due to a heap allocation between fork() and execve(), + // in musl v1.1.24. + // Additionally, we want to reduce the number of possible ways things + // can fail between fork() and execve(). + // Therefore, we do all the allocation for the execve() before the fork(). + // This means we must do the null-termination of argv and env vars here. + const argv_buf = try arena.alloc(?[*:0]u8, self.argv.len + 1); + for (self.argv) |arg, i| { + const arg_buf = try arena.alloc(u8, arg.len + 1); + @memcpy(arg_buf.ptr, arg.ptr, arg.len); + arg_buf[arg.len] = 0; + argv_buf[i] = arg_buf[0..arg.len :0].ptr; } + argv_buf[self.argv.len] = null; + const argv_ptr = argv_buf[0..self.argv.len :null].ptr; + + const envp = m: { + if (self.env_map) |env_map| { + const envp_buf = try createNullDelimitedEnvMap(arena, env_map); + break :m envp_buf.ptr; + } else if (std.builtin.link_libc) { + break :m std.c.environ; + } else if (std.builtin.output_mode == .Exe) { + // Then we have Zig start code and this works. + // TODO type-safety for null-termination of `os.environ`. + break :m @ptrCast([*:null]?[*:0]u8, os.environ.ptr); + } else { + // TODO come up with a solution for this. + @compileError("missing std lib enhancement: ChildProcess implementation has no way to collect the environment variables to forward to the child process"); + } + }; // This pipe is used to communicate errors between the time of fork // and execve from the child process to the parent process. @@ -438,7 +464,10 @@ pub const ChildProcess = struct { os.setreuid(uid, uid) catch |err| forkChildErrReport(err_pipe[1], err); } - const err = os.execvpe_expandArg0(self.allocator, self.expand_arg0, self.argv, env_map); + const err = switch (self.expand_arg0) { + .expand => os.execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_ptr, envp), + .no_expand => os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp), + }; forkChildErrReport(err_pipe[1], err); } @@ -881,3 +910,24 @@ pub fn createWindowsEnvBlock(allocator: *mem.Allocator, env_map: *const BufMap) i += 1; return allocator.shrink(result, i); } + +pub fn createNullDelimitedEnvMap(arena: *mem.Allocator, env_map: *const std.BufMap) ![:null]?[*:0]u8 { + const envp_count = env_map.count(); + const envp_buf = try arena.alloc(?[*:0]u8, envp_count + 1); + mem.set(?[*:0]u8, envp_buf, null); + { + var it = env_map.iterator(); + var i: usize = 0; + while (it.next()) |pair| : (i += 1) { + const env_buf = try arena.alloc(u8, pair.key.len + pair.value.len + 2); + @memcpy(env_buf.ptr, pair.key.ptr, pair.key.len); + env_buf[pair.key.len] = '='; + @memcpy(env_buf.ptr + pair.key.len + 1, pair.value.ptr, pair.value.len); + const len = env_buf.len - 1; + env_buf[len] = 0; + envp_buf[i] = env_buf[0..len :0].ptr; + } + assert(i == envp_count); + } + return envp_buf[0..envp_count :null]; +} diff --git a/lib/std/os.zig b/lib/std/os.zig index 5a57fed5cf..28d4f6bb1a 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1348,89 +1348,10 @@ pub fn execvpeZ_expandArg0( /// If `file` is an absolute path, this is the same as `execveZ`. pub fn execvpeZ( file: [*:0]const u8, - argv: [*:null]const ?[*:0]const u8, + argv_ptr: [*:null]const ?[*:0]const u8, envp: [*:null]const ?[*:0]const u8, ) ExecveError { - return execvpeZ_expandArg0(.no_expand, file, argv, envp); -} - -/// This is the same as `execvpe` except if the `arg0_expand` parameter is set to `.expand`, -/// then argv[0] will be replaced with the expanded version of it, after resolving in accordance -/// with the PATH environment variable. -pub fn execvpe_expandArg0( - allocator: *mem.Allocator, - arg0_expand: Arg0Expand, - argv_slice: []const []const u8, - env_map: *const std.BufMap, -) (ExecveError || error{OutOfMemory}) { - const argv_buf = try allocator.alloc(?[*:0]u8, argv_slice.len + 1); - mem.set(?[*:0]u8, argv_buf, null); - defer { - for (argv_buf) |arg| { - const arg_buf = mem.spanZ(arg) orelse break; - allocator.free(arg_buf); - } - allocator.free(argv_buf); - } - for (argv_slice) |arg, i| { - const arg_buf = try allocator.alloc(u8, arg.len + 1); - @memcpy(arg_buf.ptr, arg.ptr, arg.len); - arg_buf[arg.len] = 0; - argv_buf[i] = arg_buf[0..arg.len :0].ptr; - } - argv_buf[argv_slice.len] = null; - const argv_ptr = argv_buf[0..argv_slice.len :null].ptr; - - const envp_buf = try createNullDelimitedEnvMap(allocator, env_map); - defer freeNullDelimitedEnvMap(allocator, envp_buf); - - switch (arg0_expand) { - .expand => return execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_ptr, envp_buf.ptr), - .no_expand => return execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp_buf.ptr), - } -} - -/// This function must allocate memory to add a null terminating bytes on path and each arg. -/// It must also convert to KEY=VALUE\0 format for environment variables, and include null -/// pointers after the args and after the environment variables. -/// `argv_slice[0]` is the executable path. -/// This function also uses the PATH environment variable to get the full path to the executable. -pub fn execvpe( - allocator: *mem.Allocator, - argv_slice: []const []const u8, - env_map: *const std.BufMap, -) (ExecveError || error{OutOfMemory}) { - return execvpe_expandArg0(allocator, .no_expand, argv_slice, env_map); -} - -pub fn createNullDelimitedEnvMap(allocator: *mem.Allocator, env_map: *const std.BufMap) ![:null]?[*:0]u8 { - const envp_count = env_map.count(); - const envp_buf = try allocator.alloc(?[*:0]u8, envp_count + 1); - mem.set(?[*:0]u8, envp_buf, null); - errdefer freeNullDelimitedEnvMap(allocator, envp_buf); - { - var it = env_map.iterator(); - var i: usize = 0; - while (it.next()) |pair| : (i += 1) { - const env_buf = try allocator.alloc(u8, pair.key.len + pair.value.len + 2); - @memcpy(env_buf.ptr, pair.key.ptr, pair.key.len); - env_buf[pair.key.len] = '='; - @memcpy(env_buf.ptr + pair.key.len + 1, pair.value.ptr, pair.value.len); - const len = env_buf.len - 1; - env_buf[len] = 0; - envp_buf[i] = env_buf[0..len :0].ptr; - } - assert(i == envp_count); - } - return envp_buf[0..envp_count :null]; -} - -pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) void { - for (envp_buf) |env| { - const env_buf = if (env) |ptr| ptr[0 .. mem.len(ptr) + 1] else break; - allocator.free(env_buf); - } - allocator.free(envp_buf); + return execvpeZ_expandArg0(.no_expand, file, argv_ptr, envp); } /// Get an environment variable. diff --git a/lib/std/process.zig b/lib/std/process.zig index e12bc28c0c..909b337f89 100644 --- a/lib/std/process.zig +++ b/lib/std/process.zig @@ -13,6 +13,7 @@ const math = std.math; const Allocator = mem.Allocator; const assert = std.debug.assert; const testing = std.testing; +const child_process = @import("child_process.zig"); pub const abort = os.abort; pub const exit = os.exit; @@ -778,3 +779,68 @@ pub fn getSelfExeSharedLibPaths(allocator: *Allocator) error{OutOfMemory}![][:0] else => @compileError("getSelfExeSharedLibPaths unimplemented for this target"), } } + +/// Tells whether calling the `execv` or `execve` functions will be a compile error. +pub const can_execv = std.builtin.os.tag != .windows; + +pub const ExecvError = std.os.ExecveError || error{OutOfMemory}; + +/// Replaces the current process image with the executed process. +/// This function must allocate memory to add a null terminating bytes on path and each arg. +/// It must also convert to KEY=VALUE\0 format for environment variables, and include null +/// pointers after the args and after the environment variables. +/// `argv[0]` is the executable path. +/// This function also uses the PATH environment variable to get the full path to the executable. +/// Due to the heap-allocation, it is illegal to call this function in a fork() child. +/// For that use case, use the `std.os` functions directly. +pub fn execv(allocator: *mem.Allocator, argv: []const []const u8) ExecvError { + return execve(allocator, argv, null); +} + +/// Replaces the current process image with the executed process. +/// This function must allocate memory to add a null terminating bytes on path and each arg. +/// It must also convert to KEY=VALUE\0 format for environment variables, and include null +/// pointers after the args and after the environment variables. +/// `argv[0]` is the executable path. +/// This function also uses the PATH environment variable to get the full path to the executable. +/// Due to the heap-allocation, it is illegal to call this function in a fork() child. +/// For that use case, use the `std.os` functions directly. +pub fn execve( + allocator: *mem.Allocator, + argv: []const []const u8, + env_map: ?*const std.BufMap, +) ExecvError { + if (!can_execv) @compileError("The target OS does not support execv"); + + var arena_allocator = std.heap.ArenaAllocator.init(allocator); + defer arena_allocator.deinit(); + const arena = &arena_allocator.allocator; + + const argv_buf = try arena.alloc(?[*:0]u8, argv.len + 1); + for (argv) |arg, i| { + const arg_buf = try arena.alloc(u8, arg.len + 1); + @memcpy(arg_buf.ptr, arg.ptr, arg.len); + arg_buf[arg.len] = 0; + argv_buf[i] = arg_buf[0..arg.len :0].ptr; + } + argv_buf[argv.len] = null; + const argv_ptr = argv_buf[0..argv.len :null].ptr; + + const envp = m: { + if (env_map) |m| { + const envp_buf = try child_process.createNullDelimitedEnvMap(arena, m); + break :m envp_buf.ptr; + } else if (std.builtin.link_libc) { + break :m std.c.environ; + } else if (std.builtin.output_mode == .Exe) { + // Then we have Zig start code and this works. + // TODO type-safety for null-termination of `os.environ`. + break :m @ptrCast([*:null]?[*:0]u8, os.environ.ptr); + } else { + // TODO come up with a solution for this. + @compileError("missing std lib enhancement: std.process.execv implementation has no way to collect the environment variables to forward to the child process"); + } + }; + + return os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp); +} diff --git a/src/main.zig b/src/main.zig index 99d0dcd5f9..c23cba98d8 100644 --- a/src/main.zig +++ b/src/main.zig @@ -116,15 +116,13 @@ pub fn main() anyerror!void { return mainArgs(gpa, arena, args); } -const os_can_execve = std.builtin.os.tag != .windows; - pub fn mainArgs(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !void { if (args.len <= 1) { std.log.info("{}", .{usage}); fatal("expected command argument", .{}); } - if (os_can_execve and std.os.getenvZ("ZIG_IS_DETECTING_LIBC_PATHS") != null) { + if (std.process.can_execv and std.os.getenvZ("ZIG_IS_DETECTING_LIBC_PATHS") != null) { // In this case we have accidentally invoked ourselves as "the system C compiler" // to figure out where libc is installed. This is essentially infinite recursion // via child process execution due to the CC environment variable pointing to Zig. @@ -147,11 +145,11 @@ pub fn mainArgs(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !v // CC environment variable. We detect and support this scenario here because of // the ZIG_IS_DETECTING_LIBC_PATHS environment variable. if (mem.eql(u8, args[1], "cc")) { - return std.os.execvpe(arena, args[1..], &env_map); + return std.process.execve(arena, args[1..], &env_map); } else { const modified_args = try arena.dupe([]const u8, args); modified_args[0] = "cc"; - return std.os.execvpe(arena, modified_args, &env_map); + return std.process.execve(arena, modified_args, &env_map); } } @@ -1841,10 +1839,8 @@ fn buildOutputType( } // We do not execve for tests because if the test fails we want to print the error message and // invocation below. - if (os_can_execve and arg_mode == .run and !watch) { - // TODO improve the std lib so that we don't need a call to getEnvMap here. - var env_vars = try process.getEnvMap(arena); - const err = std.os.execvpe(gpa, argv.items, &env_vars); + if (std.process.can_execv and arg_mode == .run and !watch) { + const err = std.process.execv(gpa, argv.items); const cmd = try argvCmd(arena, argv.items); fatal("the following command failed to execve with '{s}':\n{s}", .{ @errorName(err), cmd }); } else {