Skip to content

Commit

Permalink
Auto merge of #80537 - ehuss:macos-posix-spawn-chdir, r=dtolnay
Browse files Browse the repository at this point in the history
Don't use posix_spawn_file_actions_addchdir_np on macOS.

There is a bug on macOS where using `posix_spawn_file_actions_addchdir_np` with a relative executable path will cause `posix_spawnp` to return ENOENT, even though it successfully spawned the process in the given directory.

`posix_spawn_file_actions_addchdir_np` was introduced in macOS 10.15 first released in Oct 2019.  I have tested macOS 10.15.7 and 11.0.1.

Example offending program:

```rust
use std::fs;
use std::os::unix::fs::PermissionsExt;
use std::process::*;

fn main() {
    fs::create_dir_all("bar").unwrap();
    fs::create_dir_all("foo").unwrap();
    fs::write("foo/foo.sh", "#!/bin/sh\necho hello ${PWD}\n").unwrap();
    let perms = fs::Permissions::from_mode(0o755);
    fs::set_permissions("foo/foo.sh", perms).unwrap();
    let c = Command::new("../foo/foo.sh").current_dir("bar").spawn();
    eprintln!("{:?}", c);
}
```

This prints:

```
Err(Os { code: 2, kind: NotFound, message: "No such file or directory" })
hello /Users/eric/Temp/bar
```

I wanted to open this PR to get some feedback on possible solutions.  Alternatives:
* Do nothing.
* Document the bug.
* Try to detect if the executable is a relative path on macOS, and avoid using `posix_spawn_file_actions_addchdir_np` only in that case.

I looked at the [XNU source code](https://opensource.apple.com/source/xnu/xnu-6153.141.1/bsd/kern/kern_exec.c.auto.html), but I didn't see anything obvious that would explain the behavior.  The actual chdir succeeds, it is something else further down that fails, but I couldn't see where.

EDIT: I forgot to mention, relative exe paths with `current_dir` in general are discouraged (see #37868).  I don't know if #37868 is fixable, since normalizing it would change the semantics for some platforms. Another option is to convert the executable to an absolute path with something like joining the cwd with the new cwd and the executable, but I'm uncertain about that.
  • Loading branch information
bors committed Jan 17, 2021
2 parents 4253153 + 6e467b7 commit c4df63f
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 4 deletions.
18 changes: 14 additions & 4 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,20 @@ impl Command {
) -> libc::c_int
}
let addchdir = match self.get_cwd() {
Some(cwd) => match posix_spawn_file_actions_addchdir_np.get() {
Some(f) => Some((f, cwd)),
None => return Ok(None),
},
Some(cwd) => {
if cfg!(target_os = "macos") {
// There is a bug in macOS where a relative executable
// path like "../myprogram" will cause `posix_spawn` to
// successfully launch the program, but erroneously return
// ENOENT when used with posix_spawn_file_actions_addchdir_np
// which was introduced in macOS 10.15.
return Ok(None);
}
match posix_spawn_file_actions_addchdir_np.get() {
Some(f) => Some((f, cwd)),
None => return Ok(None),
}
}
None => None,
};

Expand Down
49 changes: 49 additions & 0 deletions src/test/ui/command/command-current-dir.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// run-pass
// ignore-emscripten no processes
// ignore-sgx no processes

use std::env;
use std::fs;
use std::path::Path;
use std::process::Command;

fn main() {
// Checks the behavior of current_dir when used with a relative exe path.
let me = env::current_exe().unwrap();
if matches!(env::args().skip(1).next().as_deref(), Some("current-dir")) {
let cwd = env::current_dir().unwrap();
assert_eq!(cwd.file_name().unwrap(), "bar");
std::process::exit(0);
}
let exe = me.file_name().unwrap();
let cwd = me.parent().unwrap();
eprintln!("cwd={:?}", cwd);
// Change directory to where the exectuable is located, since this test
// fundamentally needs to use relative paths. In some cases (like
// remote-test-server), the current_dir can be somewhere else, so make
// sure it is something we can use. We assume we can write to this
// directory.
env::set_current_dir(&cwd).unwrap();
let foo = cwd.join("foo");
let bar = cwd.join("bar");
fs::create_dir_all(&foo).unwrap();
fs::create_dir_all(&bar).unwrap();
fs::copy(&me, foo.join(exe)).unwrap();

// Unfortunately this is inconsistent based on the platform, see
// https://github.com/rust-lang/rust/issues/37868. On Windows,
// it is relative *before* changing the directory, and on Unix
// it is *after* changing the directory.
let relative_exe = if cfg!(windows) {
Path::new("foo").join(exe)
} else {
Path::new("../foo").join(exe)
};

let status = Command::new(relative_exe)
.arg("current-dir")
.current_dir("bar")
.status()
.unwrap();
assert!(status.success());
}

0 comments on commit c4df63f

Please sign in to comment.