Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[musl] use posix_spawn if a directory change was requested #131851

Merged
merged 1 commit into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions library/std/src/process/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,23 @@ fn stdout_works() {
#[test]
#[cfg_attr(any(windows, target_os = "vxworks"), ignore)]
fn set_current_dir_works() {
// On many Unix platforms this will use the posix_spawn path.
workingjubilee marked this conversation as resolved.
Show resolved Hide resolved
let mut cmd = shell_cmd();
cmd.arg("-c").arg("pwd").current_dir("/").stdout(Stdio::piped());
assert_eq!(run_output(cmd), "/\n");

// Also test the fork/exec path by setting a pre_exec function.
#[cfg(unix)]
{
use crate::os::unix::process::CommandExt;

let mut cmd = shell_cmd();
cmd.arg("-c").arg("pwd").current_dir("/").stdout(Stdio::piped());
unsafe {
cmd.pre_exec(|| Ok(()));
}
assert_eq!(run_output(cmd), "/\n");
}
}

#[test]
Expand Down
54 changes: 43 additions & 11 deletions library/std/src/sys/pal/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,6 @@ impl Command {
use core::sync::atomic::{AtomicU8, Ordering};

use crate::mem::MaybeUninit;
use crate::sys::weak::weak;
use crate::sys::{self, cvt_nz, on_broken_pipe_flag_used};

if self.get_gid().is_some()
Expand All @@ -462,6 +461,8 @@ impl Command {

cfg_if::cfg_if! {
if #[cfg(target_os = "linux")] {
use crate::sys::weak::weak;

weak! {
fn pidfd_spawnp(
*mut libc::c_int,
Expand Down Expand Up @@ -575,16 +576,44 @@ impl Command {
}
}

// Solaris, glibc 2.29+, and musl 1.24+ can set a new working directory,
// and maybe others will gain this non-POSIX function too. We'll check
// for this weak symbol as soon as it's needed, so we can return early
// otherwise to do a manual chdir before exec.
weak! {
fn posix_spawn_file_actions_addchdir_np(
*mut libc::posix_spawn_file_actions_t,
*const libc::c_char
) -> libc::c_int
type PosixSpawnAddChdirFn = unsafe extern "C" fn(
*mut libc::posix_spawn_file_actions_t,
*const libc::c_char,
) -> libc::c_int;

/// Get the function pointer for adding a chdir action to a
/// `posix_spawn_file_actions_t`, if available, assuming a dynamic libc.
///
/// Some platforms can set a new working directory for a spawned process in the
/// `posix_spawn` path. This function looks up the function pointer for adding
/// such an action to a `posix_spawn_file_actions_t` struct.
#[cfg(not(all(target_os = "linux", target_env = "musl")))]
fn get_posix_spawn_addchdir() -> Option<PosixSpawnAddChdirFn> {
use crate::sys::weak::weak;

weak! {
fn posix_spawn_file_actions_addchdir_np(
*mut libc::posix_spawn_file_actions_t,
*const libc::c_char
) -> libc::c_int
}

posix_spawn_file_actions_addchdir_np.get()
}

/// Get the function pointer for adding a chdir action to a
/// `posix_spawn_file_actions_t`, if available, on platforms where the function
/// is known to exist.
///
/// Weak symbol lookup doesn't work with statically linked libcs, so in cases
/// where static linking is possible we need to either check for the presence
/// of the symbol at compile time or know about it upfront.
#[cfg(all(target_os = "linux", target_env = "musl"))]
fn get_posix_spawn_addchdir() -> Option<PosixSpawnAddChdirFn> {
// Our minimum required musl supports this function, so we can just use it.
Some(libc::posix_spawn_file_actions_addchdir_np)
}

let addchdir = match self.get_cwd() {
Some(cwd) => {
if cfg!(target_vendor = "apple") {
Expand All @@ -597,7 +626,10 @@ impl Command {
return Ok(None);
}
}
match posix_spawn_file_actions_addchdir_np.get() {
// Check for the availability of the posix_spawn addchdir
// function now. If it isn't available, bail and use the
// fork/exec path.
match get_posix_spawn_addchdir() {
Some(f) => Some((f, cwd)),
None => return Ok(None),
}
Expand Down
Loading