Fixes Issue #16114: The subprocess module no longer provides a

misleading error message stating that args[0] did not exist when
either the cwd or executable keyword arguments specified a path that
did not exist.

It now keeps track of if the child got as far as preexec and reports it if
not back to the parent via a special "noexec" error message value in
the error pipe so that the cwd can be blamed for a failed chdir
instead of the exec of the executable being blamed instead.

The executable is also always reported accurately when exec fails.

Unittests enhanced to cover these cases.

4 files changed, 63 insertions, 9 deletions

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 93262df..83c79ef 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1169,6 +1169,7 @@ class Popen(object):
 
             if executable is None:
                 executable = args[0]
+            orig_executable = executable
 
             # For transferring possible exec failure from child to parent.
             # Data format: "exception name:hex errno:description"
@@ -1224,6 +1225,7 @@ class Popen(object):
                         self._child_created = True
                         if self.pid == 0:
                             # Child
+                            reached_preexec = False
                             try:
                                 # Close parent's pipe ends
                                 if p2cwrite != -1:
@@ -1288,6 +1290,7 @@ class Popen(object):
                                 if start_new_session and hasattr(os, 'setsid'):
                                     os.setsid()
 
+                                reached_preexec = True
                                 if preexec_fn:
                                     preexec_fn()
 
@@ -1303,6 +1306,8 @@ class Popen(object):
                                         errno_num = exc_value.errno
                                     else:
                                         errno_num = 0
+                                    if not reached_preexec:
+                                        exc_value = "noexec"
                                     message = '%s:%x:%s' % (exc_type.__name__,
                                                             errno_num, exc_value)
                                     message = message.encode(errors="surrogatepass")
@@ -1364,10 +1369,17 @@ class Popen(object):
                 err_msg = err_msg.decode(errors="surrogatepass")
                 if issubclass(child_exception_type, OSError) and hex_errno:
                     errno_num = int(hex_errno, 16)
+                    child_exec_never_called = (err_msg == "noexec")
+                    if child_exec_never_called:
+                        err_msg = ""
                     if errno_num != 0:
                         err_msg = os.strerror(errno_num)
                         if errno_num == errno.ENOENT:
-                            err_msg += ': ' + repr(args[0])
+                            if child_exec_never_called:
+                                # The error must be from chdir(cwd).
+                                err_msg += ': ' + repr(cwd)
+                            else:
+                                err_msg += ': ' + repr(orig_executable)
                     raise child_exception_type(errno_num, err_msg)
                 raise child_exception_type(err_msg)
 
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 2879641..15fb498 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -884,24 +884,30 @@ class _SuppressCoreFiles(object):
 @unittest.skipIf(mswindows, "POSIX specific tests")
 class POSIXProcessTestCase(BaseTestCase):
 
-    def test_exceptions(self):
-        nonexistent_dir = "/_this/pa.th/does/not/exist"
+    def setUp(self):
+        super().setUp()
+        self._nonexistent_dir = "/_this/pa.th/does/not/exist"
+
+    def _get_chdir_exception(self):
         try:
-            os.chdir(nonexistent_dir)
+            os.chdir(self._nonexistent_dir)
         except OSError as e:
             # This avoids hard coding the errno value or the OS perror()
             # string and instead capture the exception that we want to see
             # below for comparison.
             desired_exception = e
-            desired_exception.strerror += ': ' + repr(sys.executable)
+            desired_exception.strerror += ': ' + repr(self._nonexistent_dir)
         else:
             self.fail("chdir to nonexistant directory %s succeeded." %
-                      nonexistent_dir)
+                      self._nonexistent_dir)
+        return desired_exception
 
-        # Error in the child re-raised in the parent.
+    def test_exception_cwd(self):
+        """Test error in the child raised in the parent for a bad cwd."""
+        desired_exception = self._get_chdir_exception()
         try:
             p = subprocess.Popen([sys.executable, "-c", ""],
-                                 cwd=nonexistent_dir)
+                                 cwd=self._nonexistent_dir)
         except OSError as e:
             # Test that the child process chdir failure actually makes
             # it up to the parent process as the correct exception.
@@ -910,6 +916,33 @@ class POSIXProcessTestCase(BaseTestCase):
         else:
             self.fail("Expected OSError: %s" % desired_exception)
 
+    def test_exception_bad_executable(self):
+        """Test error in the child raised in the parent for a bad executable."""
+        desired_exception = self._get_chdir_exception()
+        try:
+            p = subprocess.Popen([sys.executable, "-c", ""],
+                                 executable=self._nonexistent_dir)
+        except OSError as e:
+            # Test that the child process exec failure actually makes
+            # it up to the parent process as the correct exception.
+            self.assertEqual(desired_exception.errno, e.errno)
+            self.assertEqual(desired_exception.strerror, e.strerror)
+        else:
+            self.fail("Expected OSError: %s" % desired_exception)
+
+    def test_exception_bad_args_0(self):
+        """Test error in the child raised in the parent for a bad args[0]."""
+        desired_exception = self._get_chdir_exception()
+        try:
+            p = subprocess.Popen([self._nonexistent_dir, "-c", ""])
+        except OSError as e:
+            # Test that the child process exec failure actually makes
+            # it up to the parent process as the correct exception.
+            self.assertEqual(desired_exception.errno, e.errno)
+            self.assertEqual(desired_exception.strerror, e.strerror)
+        else:
+            self.fail("Expected OSError: %s" % desired_exception)
+
     def test_restore_signals(self):
         # Code coverage for both values of restore_signals to make sure it
         # at least does not blow up.
diff --git a/Misc/NEWS b/Misc/NEWS
index 759a12c..9e1e96b 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -129,6 +129,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #16114: The subprocess module no longer provides a misleading
+  error message stating that args[0] did not exist when either the cwd or
+  executable keyword arguments specified a path that did not exist.
+
 - Issue #15756: subprocess.poll() now properly handles errno.ECHILD to
   return a returncode of 0 when the child has already exited or cannot
   be waited on.
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 59673f4..19ca31f 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -354,7 +354,7 @@ child_exec(char *const exec_array[],
            PyObject *preexec_fn,
            PyObject *preexec_fn_args_tuple)
 {
-    int i, saved_errno, unused;
+    int i, saved_errno, unused, reached_preexec = 0;
     PyObject *result;
     const char* err_msg = "";
     /* Buffer large enough to hold a hex integer.  We can't malloc. */
@@ -438,6 +438,7 @@ child_exec(char *const exec_array[],
         POSIX_CALL(setsid());
 #endif
 
+    reached_preexec = 1;
     if (preexec_fn != Py_None && preexec_fn_args_tuple) {
         /* This is where the user has asked us to deadlock their program. */
         result = PyObject_Call(preexec_fn, preexec_fn_args_tuple, NULL);
@@ -487,6 +488,10 @@ error:
         }
         unused = write(errpipe_write, cur, hex_errno + sizeof(hex_errno) - cur);
         unused = write(errpipe_write, ":", 1);
+        if (!reached_preexec) {
+            /* Indicate to the parent that the error happened before exec(). */
+            unused = write(errpipe_write, "noexec", 6);
+        }
         /* We can't call strerror(saved_errno).  It is not async signal safe.
          * The parent process will look the error message up. */
     } else {