Auto merge of rust-lang#80537 - ehuss:macos-posix-spawn-chdir, r=dtolnay

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 rust-lang#37868).  I don't know if rust-lang#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.

Author: bors

library/std/src/sys/unix/process/process_unix.rs

@@ -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,
         };
 

src/test/ui/command/command-current-dir.rs

@@ -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());
+}