Commit 3169f0529b

Andrew Kelley <andrew@ziglang.org>
2023-03-10 03:51:58
eliminate posix_spawn from the standard library
Today I found out that posix_spawn is trash. It's actually implemented on top of fork/exec inside of libc (or libSystem in the case of macOS). So, anything posix_spawn can do, we can do better. In particular, what we can do better is handle spawning of child processes that are potentially foreign binaries. If you try to spawn a wasm binary, for example, posix spawn does the following: * Goes ahead and creates a child process. * The child process writes "foo.wasm: foo.wasm: cannot execute binary file" to stderr (yes, it prints the filename twice). * The child process then exits with code 126. This behavior is indistinguishable from the binary being successfully spawned, and then printing to stderr, and exiting with a failure - something that is an extremely common occurrence. Meanwhile, using the lower level fork/exec will simply return ENOEXEC code from the execve syscall (which is mapped to zig error.InvalidExe). The posix_spawn behavior means the zig build runner can't tell the difference between a failure to run a foreign binary, and a binary that did run, but failed in some other fashion. This is unacceptable, because attempting to excecve is the proper way to support things like Rosetta.
1 parent 904f414
lib/std/Build/CompileStep.zig
@@ -742,7 +742,6 @@ pub fn runPkgConfig(self: *CompileStep, lib_name: []const u8) ![]const []const u
         error.ExecNotSupported => return error.PkgConfigFailed,
         error.ExitCodeFailure => return error.PkgConfigFailed,
         error.FileNotFound => return error.PkgConfigNotInstalled,
-        error.ChildExecFailed => return error.PkgConfigFailed,
         else => return err,
     };
 
@@ -2042,7 +2041,6 @@ fn getPkgConfigList(self: *std.Build) ![]const PkgConfigPkg {
             error.FileNotFound => error.PkgConfigNotInstalled,
             error.InvalidName => error.PkgConfigNotInstalled,
             error.PkgConfigInvalidOutput => error.PkgConfigInvalidOutput,
-            error.ChildExecFailed => error.PkgConfigFailed,
             else => return err,
         };
         self.pkg_config_pkg_list = result;
lib/std/os/posix_spawn.zig
@@ -1,290 +0,0 @@
-const std = @import("std");
-const builtin = @import("builtin");
-
-const os = @import("../os.zig");
-const system = os.system;
-const errno = system.getErrno;
-const fd_t = system.fd_t;
-const mode_t = system.mode_t;
-const pid_t = system.pid_t;
-const unexpectedErrno = os.unexpectedErrno;
-const UnexpectedError = os.UnexpectedError;
-const toPosixPath = os.toPosixPath;
-const WaitPidResult = os.WaitPidResult;
-
-pub usingnamespace posix_spawn;
-
-pub const Error = error{
-    SystemResources,
-    InvalidFileDescriptor,
-    NameTooLong,
-    TooBig,
-    PermissionDenied,
-    InputOutput,
-    FileSystem,
-    FileNotFound,
-    InvalidExe,
-    NotDir,
-    FileBusy,
-
-    /// Returned when the child fails to execute either in the pre-exec() initialization step, or
-    /// when exec(3) is invoked.
-    ChildExecFailed,
-} || UnexpectedError;
-
-const posix_spawn = if (builtin.target.isDarwin()) struct {
-    pub const Attr = struct {
-        attr: system.posix_spawnattr_t,
-
-        pub fn init() Error!Attr {
-            var attr: system.posix_spawnattr_t = undefined;
-            switch (errno(system.posix_spawnattr_init(&attr))) {
-                .SUCCESS => return Attr{ .attr = attr },
-                .NOMEM => return error.SystemResources,
-                .INVAL => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn deinit(self: *Attr) void {
-            defer self.* = undefined;
-            switch (errno(system.posix_spawnattr_destroy(&self.attr))) {
-                .SUCCESS => return,
-                .INVAL => unreachable, // Invalid parameters.
-                else => unreachable,
-            }
-        }
-
-        pub fn get(self: Attr) Error!u16 {
-            var flags: c_short = undefined;
-            switch (errno(system.posix_spawnattr_getflags(&self.attr, &flags))) {
-                .SUCCESS => return @bitCast(u16, flags),
-                .INVAL => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn set(self: *Attr, flags: u16) Error!void {
-            switch (errno(system.posix_spawnattr_setflags(&self.attr, @bitCast(c_short, flags)))) {
-                .SUCCESS => return,
-                .INVAL => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-    };
-
-    pub const Actions = struct {
-        actions: system.posix_spawn_file_actions_t,
-
-        pub fn init() Error!Actions {
-            var actions: system.posix_spawn_file_actions_t = undefined;
-            switch (errno(system.posix_spawn_file_actions_init(&actions))) {
-                .SUCCESS => return Actions{ .actions = actions },
-                .NOMEM => return error.SystemResources,
-                .INVAL => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn deinit(self: *Actions) void {
-            defer self.* = undefined;
-            switch (errno(system.posix_spawn_file_actions_destroy(&self.actions))) {
-                .SUCCESS => return,
-                .INVAL => unreachable, // Invalid parameters.
-                else => unreachable,
-            }
-        }
-
-        pub fn open(self: *Actions, fd: fd_t, path: []const u8, flags: u32, mode: mode_t) Error!void {
-            const posix_path = try toPosixPath(path);
-            return self.openZ(fd, &posix_path, flags, mode);
-        }
-
-        pub fn openZ(self: *Actions, fd: fd_t, path: [*:0]const u8, flags: u32, mode: mode_t) Error!void {
-            switch (errno(system.posix_spawn_file_actions_addopen(&self.actions, fd, path, @bitCast(c_int, flags), mode))) {
-                .SUCCESS => return,
-                .BADF => return error.InvalidFileDescriptor,
-                .NOMEM => return error.SystemResources,
-                .NAMETOOLONG => return error.NameTooLong,
-                .INVAL => unreachable, // the value of file actions is invalid
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn close(self: *Actions, fd: fd_t) Error!void {
-            switch (errno(system.posix_spawn_file_actions_addclose(&self.actions, fd))) {
-                .SUCCESS => return,
-                .BADF => return error.InvalidFileDescriptor,
-                .NOMEM => return error.SystemResources,
-                .INVAL => unreachable, // the value of file actions is invalid
-                .NAMETOOLONG => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn dup2(self: *Actions, fd: fd_t, newfd: fd_t) Error!void {
-            switch (errno(system.posix_spawn_file_actions_adddup2(&self.actions, fd, newfd))) {
-                .SUCCESS => return,
-                .BADF => return error.InvalidFileDescriptor,
-                .NOMEM => return error.SystemResources,
-                .INVAL => unreachable, // the value of file actions is invalid
-                .NAMETOOLONG => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn inherit(self: *Actions, fd: fd_t) Error!void {
-            switch (errno(system.posix_spawn_file_actions_addinherit_np(&self.actions, fd))) {
-                .SUCCESS => return,
-                .BADF => return error.InvalidFileDescriptor,
-                .NOMEM => return error.SystemResources,
-                .INVAL => unreachable, // the value of file actions is invalid
-                .NAMETOOLONG => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn chdir(self: *Actions, path: []const u8) Error!void {
-            const posix_path = try toPosixPath(path);
-            return self.chdirZ(&posix_path);
-        }
-
-        pub fn chdirZ(self: *Actions, path: [*:0]const u8) Error!void {
-            switch (errno(system.posix_spawn_file_actions_addchdir_np(&self.actions, path))) {
-                .SUCCESS => return,
-                .NOMEM => return error.SystemResources,
-                .NAMETOOLONG => return error.NameTooLong,
-                .BADF => unreachable,
-                .INVAL => unreachable, // the value of file actions is invalid
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-
-        pub fn fchdir(self: *Actions, fd: fd_t) Error!void {
-            switch (errno(system.posix_spawn_file_actions_addfchdir_np(&self.actions, fd))) {
-                .SUCCESS => return,
-                .BADF => return error.InvalidFileDescriptor,
-                .NOMEM => return error.SystemResources,
-                .INVAL => unreachable, // the value of file actions is invalid
-                .NAMETOOLONG => unreachable,
-                else => |err| return unexpectedErrno(err),
-            }
-        }
-    };
-
-    pub fn spawn(
-        path: []const u8,
-        actions: ?Actions,
-        attr: ?Attr,
-        argv: [*:null]?[*:0]const u8,
-        envp: [*:null]?[*:0]const u8,
-    ) Error!pid_t {
-        const posix_path = try toPosixPath(path);
-        return spawnZ(&posix_path, actions, attr, argv, envp);
-    }
-
-    pub fn spawnZ(
-        path: [*:0]const u8,
-        actions: ?Actions,
-        attr: ?Attr,
-        argv: [*:null]?[*:0]const u8,
-        envp: [*:null]?[*:0]const u8,
-    ) Error!pid_t {
-        var pid: pid_t = undefined;
-        switch (errno(system.posix_spawn(
-            &pid,
-            path,
-            if (actions) |a| &a.actions else null,
-            if (attr) |a| &a.attr else null,
-            argv,
-            envp,
-        ))) {
-            .SUCCESS => return pid,
-            .@"2BIG" => return error.TooBig,
-            .NOMEM => return error.SystemResources,
-            .BADF => return error.InvalidFileDescriptor,
-            .ACCES => return error.PermissionDenied,
-            .IO => return error.InputOutput,
-            .LOOP => return error.FileSystem,
-            .NAMETOOLONG => return error.NameTooLong,
-            .NOENT => return error.FileNotFound,
-            .NOEXEC => return error.InvalidExe,
-            .NOTDIR => return error.NotDir,
-            .TXTBSY => return error.FileBusy,
-            .BADARCH => return error.InvalidExe,
-            .BADEXEC => return error.InvalidExe,
-            .FAULT => unreachable,
-            .INVAL => unreachable,
-            else => |err| return unexpectedErrno(err),
-        }
-    }
-
-    pub fn spawnp(
-        file: []const u8,
-        actions: ?Actions,
-        attr: ?Attr,
-        argv: [*:null]?[*:0]const u8,
-        envp: [*:null]?[*:0]const u8,
-    ) Error!pid_t {
-        const posix_file = try toPosixPath(file);
-        return spawnpZ(&posix_file, actions, attr, argv, envp);
-    }
-
-    pub fn spawnpZ(
-        file: [*:0]const u8,
-        actions: ?Actions,
-        attr: ?Attr,
-        argv: [*:null]?[*:0]const u8,
-        envp: [*:null]?[*:0]const u8,
-    ) Error!pid_t {
-        var pid: pid_t = undefined;
-        switch (errno(system.posix_spawnp(
-            &pid,
-            file,
-            if (actions) |a| &a.actions else null,
-            if (attr) |a| &a.attr else null,
-            argv,
-            envp,
-        ))) {
-            .SUCCESS => return pid,
-            .@"2BIG" => return error.TooBig,
-            .NOMEM => return error.SystemResources,
-            .BADF => return error.InvalidFileDescriptor,
-            .ACCES => return error.PermissionDenied,
-            .IO => return error.InputOutput,
-            .LOOP => return error.FileSystem,
-            .NAMETOOLONG => return error.NameTooLong,
-            .NOENT => return error.FileNotFound,
-            .NOEXEC => return error.InvalidExe,
-            .NOTDIR => return error.NotDir,
-            .TXTBSY => return error.FileBusy,
-            .BADARCH => return error.InvalidExe,
-            .BADEXEC => return error.InvalidExe,
-            .FAULT => unreachable,
-            .INVAL => unreachable,
-            else => |err| return unexpectedErrno(err),
-        }
-    }
-
-    /// Use this version of the `waitpid` wrapper if you spawned your child process using `posix_spawn`
-    /// or `posix_spawnp` syscalls.
-    /// See also `std.os.waitpid` for an alternative if your child process was spawned via `fork` and
-    /// `execve` method.
-    pub fn waitpid(pid: pid_t, flags: u32) Error!WaitPidResult {
-        const Status = if (builtin.link_libc) c_int else u32;
-        var status: Status = undefined;
-        while (true) {
-            const rc = system.waitpid(pid, &status, if (builtin.link_libc) @intCast(c_int, flags) else flags);
-            switch (errno(rc)) {
-                .SUCCESS => return WaitPidResult{
-                    .pid = @intCast(pid_t, rc),
-                    .status = @bitCast(u32, status),
-                },
-                .INTR => continue,
-                .CHILD => return error.ChildExecFailed,
-                .INVAL => unreachable, // Invalid flags.
-                else => unreachable,
-            }
-        }
-    }
-} else struct {};
lib/std/child_process.zig
@@ -90,8 +90,7 @@ pub const ChildProcess = struct {
         os.SetIdError ||
         os.ChangeCurDirError ||
         windows.CreateProcessError ||
-        windows.WaitForSingleObjectError ||
-        os.posix_spawn.Error;
+        windows.WaitForSingleObjectError;
 
     pub const Term = union(enum) {
         Exited: u8,
@@ -143,10 +142,6 @@ pub const ChildProcess = struct {
             @compileError("the target operating system cannot spawn processes");
         }
 
-        if (comptime builtin.target.isDarwin()) {
-            return self.spawnMacos();
-        }
-
         if (builtin.os.tag == .windows) {
             return self.spawnWindows();
         } else {
@@ -337,10 +332,7 @@ pub const ChildProcess = struct {
     }
 
     fn waitUnwrapped(self: *ChildProcess) !void {
-        const res: os.WaitPidResult = if (comptime builtin.target.isDarwin())
-            try os.posix_spawn.waitpid(self.id, 0)
-        else
-            os.waitpid(self.id, 0);
+        const res: os.WaitPidResult = os.waitpid(self.id, 0);
         const status = res.status;
         self.cleanupStreams();
         self.handleWaitResult(status);
@@ -416,121 +408,6 @@ pub const ChildProcess = struct {
             Term{ .Unknown = status };
     }
 
-    fn spawnMacos(self: *ChildProcess) SpawnError!void {
-        const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0;
-        const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;
-        errdefer if (self.stdin_behavior == StdIo.Pipe) destroyPipe(stdin_pipe);
-
-        const stdout_pipe = if (self.stdout_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;
-        errdefer if (self.stdout_behavior == StdIo.Pipe) destroyPipe(stdout_pipe);
-
-        const stderr_pipe = if (self.stderr_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;
-        errdefer if (self.stderr_behavior == StdIo.Pipe) destroyPipe(stderr_pipe);
-
-        const any_ignore = (self.stdin_behavior == StdIo.Ignore or self.stdout_behavior == StdIo.Ignore or self.stderr_behavior == StdIo.Ignore);
-        const dev_null_fd = if (any_ignore)
-            os.openZ("/dev/null", os.O.RDWR, 0) catch |err| switch (err) {
-                error.PathAlreadyExists => unreachable,
-                error.NoSpaceLeft => unreachable,
-                error.FileTooBig => unreachable,
-                error.DeviceBusy => unreachable,
-                error.FileLocksNotSupported => unreachable,
-                error.BadPathName => unreachable, // Windows-only
-                error.InvalidHandle => unreachable, // WASI-only
-                error.WouldBlock => unreachable,
-                else => |e| return e,
-            }
-        else
-            undefined;
-        defer if (any_ignore) os.close(dev_null_fd);
-
-        var attr = try os.posix_spawn.Attr.init();
-        defer attr.deinit();
-        var flags: u16 = os.darwin.POSIX_SPAWN_SETSIGDEF | os.darwin.POSIX_SPAWN_SETSIGMASK;
-        if (self.disable_aslr) {
-            flags |= os.darwin._POSIX_SPAWN_DISABLE_ASLR;
-        }
-        if (self.start_suspended) {
-            flags |= os.darwin.POSIX_SPAWN_START_SUSPENDED;
-        }
-        try attr.set(flags);
-
-        var actions = try os.posix_spawn.Actions.init();
-        defer actions.deinit();
-
-        try setUpChildIoPosixSpawn(self.stdin_behavior, &actions, stdin_pipe, os.STDIN_FILENO, dev_null_fd);
-        try setUpChildIoPosixSpawn(self.stdout_behavior, &actions, stdout_pipe, os.STDOUT_FILENO, dev_null_fd);
-        try setUpChildIoPosixSpawn(self.stderr_behavior, &actions, stderr_pipe, os.STDERR_FILENO, dev_null_fd);
-
-        if (self.cwd_dir) |cwd| {
-            try actions.fchdir(cwd.fd);
-        } else if (self.cwd) |cwd| {
-            try actions.chdir(cwd);
-        }
-
-        var arena_allocator = std.heap.ArenaAllocator.init(self.allocator);
-        defer arena_allocator.deinit();
-        const arena = arena_allocator.allocator();
-
-        const argv_buf = try arena.allocSentinel(?[*:0]u8, self.argv.len, null);
-        for (self.argv, 0..) |arg, i| argv_buf[i] = (try arena.dupeZ(u8, arg)).ptr;
-
-        const envp = if (self.env_map) |env_map| m: {
-            const envp_buf = try createNullDelimitedEnvMap(arena, env_map);
-            break :m envp_buf.ptr;
-        } else std.c.environ;
-
-        const pid = try os.posix_spawn.spawnp(self.argv[0], actions, attr, argv_buf, envp);
-
-        if (self.stdin_behavior == StdIo.Pipe) {
-            self.stdin = File{ .handle = stdin_pipe[1] };
-        } else {
-            self.stdin = null;
-        }
-        if (self.stdout_behavior == StdIo.Pipe) {
-            self.stdout = File{ .handle = stdout_pipe[0] };
-        } else {
-            self.stdout = null;
-        }
-        if (self.stderr_behavior == StdIo.Pipe) {
-            self.stderr = File{ .handle = stderr_pipe[0] };
-        } else {
-            self.stderr = null;
-        }
-
-        self.id = pid;
-        self.term = null;
-
-        if (self.stdin_behavior == StdIo.Pipe) {
-            os.close(stdin_pipe[0]);
-        }
-        if (self.stdout_behavior == StdIo.Pipe) {
-            os.close(stdout_pipe[1]);
-        }
-        if (self.stderr_behavior == StdIo.Pipe) {
-            os.close(stderr_pipe[1]);
-        }
-    }
-
-    fn setUpChildIoPosixSpawn(
-        stdio: StdIo,
-        actions: *os.posix_spawn.Actions,
-        pipe_fd: [2]i32,
-        std_fileno: i32,
-        dev_null_fd: i32,
-    ) !void {
-        switch (stdio) {
-            .Pipe => {
-                const idx: usize = if (std_fileno == 0) 0 else 1;
-                try actions.dup2(pipe_fd[idx], std_fileno);
-                try actions.close(pipe_fd[1 - idx]);
-            },
-            .Close => try actions.close(std_fileno),
-            .Inherit => {},
-            .Ignore => try actions.dup2(dev_null_fd, std_fileno),
-        }
-    }
-
     fn spawnPosix(self: *ChildProcess) SpawnError!void {
         const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0;
         const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;
lib/std/os.zig
@@ -41,7 +41,6 @@ pub const plan9 = @import("os/plan9.zig");
 pub const uefi = @import("os/uefi.zig");
 pub const wasi = @import("os/wasi.zig");
 pub const windows = @import("os/windows.zig");
-pub const posix_spawn = @import("os/posix_spawn.zig");
 pub const ptrace = @import("os/ptrace.zig");
 
 comptime {
@@ -56,7 +55,6 @@ test {
     }
     _ = wasi;
     _ = windows;
-    _ = posix_spawn;
 
     _ = @import("os/test.zig");
 }
@@ -3998,8 +3996,7 @@ pub const WaitPidResult = struct {
 };
 
 /// Use this version of the `waitpid` wrapper if you spawned your child process using explicit
-/// `fork` and `execve` method. If you spawned your child process using `posix_spawn` method,
-/// use `std.os.posix_spawn.waitpid` instead.
+/// `fork` and `execve` method.
 pub fn waitpid(pid: pid_t, flags: u32) WaitPidResult {
     const Status = if (builtin.link_libc) c_int else u32;
     var status: Status = undefined;
CMakeLists.txt
@@ -303,7 +303,6 @@ set(ZIG_STAGE2_SOURCES
     "${CMAKE_SOURCE_DIR}/lib/std/os/linux.zig"
     "${CMAKE_SOURCE_DIR}/lib/std/os/linux/io_uring.zig"
     "${CMAKE_SOURCE_DIR}/lib/std/os/linux/x86_64.zig"
-    "${CMAKE_SOURCE_DIR}/lib/std/os/posix_spawn.zig"
     "${CMAKE_SOURCE_DIR}/lib/std/os/windows.zig"
     "${CMAKE_SOURCE_DIR}/lib/std/os/windows/ntstatus.zig"
     "${CMAKE_SOURCE_DIR}/lib/std/os/windows/win32error.zig"