Changeset 3161

Timestamp:

Mar 19, 2018, 11:40:35 PM (7 years ago)

Author:

bird

Message:

kmk/win: More child process work, focusing on making kmk_builtin_redirect not requiring the shared lock.

Location:

trunk/src/kmk

Files:

• kmkbuiltin/redirect.c (modified) (20 diffs)
• main.c (modified) (2 diffs)
• w32/winchildren.c (modified) (14 diffs)

trunk/src/kmk/kmkbuiltin/redirect.c

@@ -39 +39 @@
 #if defined(KBUILD_OS_WINDOWS) || defined(KBUILD_OS_OS2)
 # include <process.h>
+#endif
+#if defined(KBUILD_OS_WINDOWS) || defined(KBUILD_OS_OS2)
+# include <Windows.h>
+# include <Winternl.h>
 #endif
 #if defined(_MSC_VER)
@@ -91 +95 @@
 
 
+/*********************************************************************************************************************************
+*   Defined Constants And Macros                                                                                                 *
+*********************************************************************************************************************************/
 /* String + strlen tuple. */
 #define TUPLE(a_sz)     a_sz, sizeof(a_sz) - 1
+
+/** Only standard handles on windows. */
+#ifdef KBUILD_OS_WINDOWS
+# define ONLY_TARGET_STANDARD_HANDLES
+#endif
+
 
 
@@ -156 +169 @@
             "\n"
             "The -d switch duplicate the right hand file descriptor (src-fd) to the left\n"
-            "hand side one (fd).\n"
+            "hand side one (fd). The latter is limited to standard handles on windows.\n"
             "\n"
-            "The -c switch will close the specified file descriptor.\n"
+            "The -c switch will close the specified file descriptor. Limited to standard\n"
+            "handles on windows.\n"
             "\n"
             "The -Z switch zaps the environment.\n"
@@ -165 +179 @@
             "fashion.\n"
             "\n"
-            "The -C switch is for changing the current directory. This takes immediate\n"
-            "effect, so be careful where you put it.\n"
+            "The -C switch is for changing the current directory.  Please specify an\n"
+            "absolute program path as it's platform dependent whether this takes effect\n"
+            "before or after the executable is located.\n"
             "\n"
             "The --wcc-brain-damage switch is to work around wcc and wcc386 (Open Watcom)\n"
@@ -439 +454 @@
         if (fdOpened >= 0)
         {
+#ifndef KBUILD_OS_WINDOWS
             if (   !kRedirectHasConflict(fdOpened, cOrders, paOrders)
-#ifdef _MSC_VER
+# ifdef _MSC_VER
                 && fdOpened != fdTarget
-#endif
+# endif
                 )
+#endif
             {
 #ifndef _MSC_VER
@@ -510 +527 @@
 
 
-#ifndef USE_POSIX_SPAWN
+#if !defined(USE_POSIX_SPAWN) && !defined(KBUILD_OS_WINDOWS)
 
 /**
@@ -778 +795 @@
 #endif /* !USE_POSIX_SPAWN */
 
+#ifdef KBUILD_OS_WINDOWS
+
+/**
+ * Tries to locate the executable image.
+ *
+ * This isn't quite perfect yet...
+ *
+ * @returns pszExecutable or pszBuf with valid string.
+ * @param   pszExecutable   The specified executable.
+ * @param   pszBuf          Buffer to return a modified path in.
+ * @param   cbBuf           Size of return buffer.
+ * @param   pszPath         The search path.
+ */
+static const char *kRedirectCreateProcessWindowsFindImage(const char *pszExecutable, char *pszBuf, size_t cbBuf,
+                                                          const char *pszPath)
+{
+    /*
+     * Analyze the name.
+     */
+    size_t const cchExecutable = strlen(pszExecutable);
+    BOOL         fHavePath = FALSE;
+    BOOL         fHaveSuffix = FALSE;
+    size_t       off = cchExecutable;
+    while (off > 0)
+    {
+        char ch = pszExecutable[--off];
+        if (ch == '.')
+        {
+            fHaveSuffix = TRUE;
+            break;
+        }
+        if (ch == '\\' || ch == '/' || ch == ':')
+        {
+            fHavePath = TRUE;
+            break;
+        }
+    }
+    if (!fHavePath)
+        while (off > 0)
+        {
+            char ch = pszExecutable[--off];
+            if (ch == '\\' || ch == '/' || ch == ':')
+            {
+                fHavePath = TRUE;
+                break;
+            }
+        }
+   /*
+    * If no path, search the path value.
+    */
+   if (!fHavePath)
+   {
+       char *pszFilename;
+       DWORD cchFound = SearchPathA(pszPath, pszExecutable, fHaveSuffix ? NULL : ".exe", cbBuf, pszBuf, &pszFilename);
+       if (cchFound)
+           return pszBuf;
+   }
+
+   /*
+    * If no suffix, try add .exe.
+    */
+   if (   !fHaveSuffix
+       && GetFileAttributesA(pszExecutable) == INVALID_FILE_ATTRIBUTES
+       && cchExecutable + 4 < cbBuf)
+   {
+       memcpy(pszBuf, pszExecutable, cchExecutable);
+       memcpy(&pszBuf[cchExecutable], ".exe", 5);
+       if (GetFileAttributesA(pszBuf) != INVALID_FILE_ATTRIBUTES)
+           return pszBuf;
+   }
+
+   return pszExecutable;
+}
+
+/**
+ * Alternative approach on windows that use CreateProcess and doesn't require
+ * any serialization wrt handles and CWD.
+ *
+ * @returns 0 on success, non-zero on failure to create.
+ * @param   pszExecutable       The child process executable.
+ * @param   cArgs               Number of arguments.
+ * @param   papszArgs           The child argument vector.
+ * @param   papszEnvVars        The child environment vector.
+ * @param   pszCwd              The current working directory of the child.
+ * @param   cOrders             Number of file operation orders.
+ * @param   paOrders            The file operation orders.
+ * @param   phProcess           Where to return process handle.
+ */
+static kRedirectCreateProcessWindows(const char *pszExecutable, int cArgs, char **papszArgs, char **papszEnvVars,
+                                     const char *pszCwd, unsigned cOrders, REDIRECTORDERS *paOrders, HANDLE *phProcess)
+{
+    static NTSTATUS (NTAPI *s_pfnNtQueryInformationProcess)(HANDLE, PROCESSINFOCLASS, PVOID, ULONG, PULONG) = NULL;
+    size_t cbArgs;
+    char  *pszCmdLine;
+    size_t cbEnv;
+    char  *pszzEnv;
+    char  *pch;
+    int    i;
+    int    rc;
+
+    /*
+     * Determin host bitness and APIs while we can still easily return.
+     */
+#if K_ARCH_BITS == 32
+    BOOL   f64BitHost = TRUE;
+    if (!IsWow64Process(GetCurrentProcess(), &f64BitHost))
+        return errx(9, "IsWow64Process failed: %u", GetLastError());
+#elif K_ARCH_BITS == 64
+    BOOL const f64BitHost = TRUE;
+#else
+# error "K_ARCH_BITS is bad/missing"
+#endif
+    if (cOrders > 0 && !s_pfnNtQueryInformationProcess)
+    {
+        *(FARPROC *)&s_pfnNtQueryInformationProcess = GetProcAddress(GetModuleHandleA("NTDLL.DLL"), "NtQueryInformationProcess");
+        if (!s_pfnNtQueryInformationProcess)
+            return errx(9, "NtQueryInformationProcess not found!");
+    }
+
+    /*
+     * Start by making the the command line.  We just need to put spaces
+     * between the arguments since quote_argv don't the quoting already.
+     */
+    cbArgs = 0;
+    for (i = 0; i < cArgs; i++)
+        cbArgs += strlen(papszArgs[i]) + 1;
+    pszCmdLine = pch = (char *)malloc(cbArgs);
+    if (!pszCmdLine)
+        return errx(9, "out of memory!");
+    for (i = 0; i < cArgs; i++)
+    {
+        size_t cch;
+        if (i != 0)
+            *pch++ = ' ';
+        cch = strlen(papszArgs[i]);
+        memcpy(pch, papszArgs[i], cch);
+        pch += cch;
+    }
+    *pch++ = '\0';
+    assert(pch - pszCmdLine == cbArgs);
+
+    /*
+     * The environment vector is also simple.
+     */
+    cbEnv = 0;
+    for (i = 0; papszEnvVars[i]; i++)
+        cbEnv += strlen(papszEnvVars[i]) + 1;
+    cbEnv++;
+    pszzEnv = pch = (char *)malloc(cbEnv);
+    if (pszzEnv)
+    {
+        char                szAbsExe[1024];
+        const char         *pszPathVal = NULL;
+        STARTUPINFOA        StartupInfo;
+        PROCESS_INFORMATION ProcInfo = { NULL, NULL, 0, 0 };
+
+        for (i = 0; papszEnvVars[i]; i++)
+        {
+            size_t cbSrc = strlen(papszEnvVars[i]) + 1;
+            memcpy(pch, papszEnvVars[i], cbSrc);
+            if (   !pszPathVal
+                && cbSrc >= 5
+                && pch[4] == '='
+                && (pch[0] == 'P' || pch[0] == 'p')
+                && (pch[1] == 'A' || pch[1] == 'a')
+                && (pch[2] == 'T' || pch[2] == 't')
+                && (pch[3] == 'H' || pch[3] == 'h'))
+                pszPathVal = &pch[5];
+            pch += cbSrc;
+        }
+        *pch++ = '\0';
+        assert(pch - pszzEnv == cbEnv);
+
+        /*
+         * Locate the executable.
+         */
+        pszExecutable = kRedirectCreateProcessWindowsFindImage(pszExecutable, szAbsExe, sizeof(szAbsExe), pszPathVal);
+
+        /*
+         * Do basic startup info preparation.
+         */
+        memset(&StartupInfo, 0, sizeof(StartupInfo));
+        StartupInfo.cb = sizeof(StartupInfo);
+        GetStartupInfoA(&StartupInfo);
+        StartupInfo.dwFlags &= ~STARTF_USESTDHANDLES;
+
+        /*
+         * If there are no redirection orders, we're good.
+         */
+        if (!cOrders)
+        {
+            if (CreateProcessA(pszExecutable, pszCmdLine, NULL /*pProcAttrs*/, NULL /*pThreadAttrs*/,
+                               FALSE /*fInheritHandles*/, 0 /*fFlags*/, pszzEnv, pszCwd, &StartupInfo, &ProcInfo))
+            {
+                CloseHandle(ProcInfo.hThread);
+                *phProcess = ProcInfo.hProcess;
+                rc = 0;
+            }
+            else
+                rc = errx(10, "CreateProcessA(%s) failed: %u", pszExecutable, GetLastError());
+        }
+        else
+        {
+            /*
+             * Execute the orders, ending up with three handles we need to
+             * implant into the guest process.
+             *
+             * This isn't 100% perfect wrt O_APPEND, but it'll have to do for now.
+             */
+            BOOL   afReplace[3] = { FALSE, FALSE, FALSE };
+            HANDLE ahChild[3]   = { NULL,  NULL,  NULL  };
+            rc = 0;
+            for (i = 0; i < (int)cOrders; i++)
+            {
+                int fdTarget = paOrders[i].fdTarget;
+                assert(fdTarget >= 0 && fdTarget < 3);
+                switch (paOrders[i].enmOrder)
+                {
+                    case kRedirectOrder_Open:
+                        if (   (paOrders[i].fOpen & O_APPEND)
+                            && lseek(paOrders[i].fdSource, 0, SEEK_END) < 0)
+                            rc = err(10, "lseek-to-end failed on %d (for %d)", paOrders[i].fdSource, fdTarget);
+                    case kRedirectOrder_Dup:
+                        ahChild[fdTarget] = (HANDLE)_get_osfhandle(paOrders[i].fdSource);
+                        if (ahChild[fdTarget] == NULL || ahChild[fdTarget] == INVALID_HANDLE_VALUE)
+                            rc = err(10, "_get_osfhandle failed on %d (for %d)", paOrders[i].fdSource, fdTarget);
+                        break;
+                    case kRedirectOrder_Close:
+                        ahChild[fdTarget] = NULL;
+                        break;
+                    default:
+                        assert(0);
+                }
+                afReplace[fdTarget] = TRUE;
+            }
+            if (rc == 0)
+            {
+                /*
+                 * Start the process in suspended animation so we can inject handles.
+                 *
+                 * We clear the reserved 2 pointer + size to avoid passing the wrong
+                 * filehandle info to the child.  We may later want to generate this.
+                 */
+                StartupInfo.cbReserved2 = 0;
+                StartupInfo.lpReserved2 = 0;
+
+                if (CreateProcessA(pszExecutable, pszCmdLine, NULL /*pProcAttrs*/, NULL /*pThreadAttrs*/,
+                                   FALSE /*fInheritHandles*/, CREATE_SUSPENDED, pszzEnv, pszCwd, &StartupInfo, &ProcInfo))
+                {
+                    /*
+                     * Figure out where we need to write the handles.
+                     */
+                    ULONG                     cbActual1 = 0;
+                    PROCESS_BASIC_INFORMATION BasicInfo = { 0, 0, };
+                    NTSTATUS rcNt = s_pfnNtQueryInformationProcess(ProcInfo.hProcess, ProcessBasicInformation,
+                                                                   &BasicInfo, sizeof(BasicInfo), &cbActual1);
+                    if (NT_SUCCESS(rcNt))
+                    {
+                        BOOL const   f32BitPeb  = !f64BitHost;
+                        ULONG  const cbChildPtr = f32BitPeb ? 4 : 8;
+                        PVOID        pvSrcInPeb = (char *)BasicInfo.PebBaseAddress + (f32BitPeb ? 0x10 : 0x20);
+                        char *       pbDst      = 0;
+                        SIZE_T       cbActual2  = 0;
+                        if (ReadProcessMemory(ProcInfo.hProcess, pvSrcInPeb, &pbDst, cbChildPtr, &cbActual2))
+                        {
+                            union
+                            {
+                                KU32 au32[3];
+                                KU64 au64[3];
+                            } uBuf;
+                            memset(&uBuf, 0, sizeof(uBuf));
+                            pbDst += f32BitPeb ? 0x18 : 0x20;
+                            if (   (afReplace[0] && afReplace[1] && afReplace[2])
+                                || ReadProcessMemory(ProcInfo.hProcess, pbDst, &uBuf, cbChildPtr * 3, &cbActual2))
+                            {
+                                for (i = 0; i < 3; i++)
+                                    if (afReplace[i])
+                                    {
+                                        HANDLE hInChild = INVALID_HANDLE_VALUE;
+                                        if (   ahChild[i] == NULL /* just closed*/
+                                            || DuplicateHandle(GetCurrentProcess(), ahChild[i], ProcInfo.hProcess, &hInChild,
+                                                               0, TRUE /*fInheriable*/, DUPLICATE_SAME_ACCESS))
+                                        {
+                                            if (f32BitPeb)
+                                                uBuf.au32[i] = (KU32)(uintptr_t)hInChild;
+                                            else
+                                                uBuf.au64[i] = (uintptr_t)hInChild;
+                                        }
+                                        else
+                                            rc = errx(10, "Error duplicating %p into the child: %u", ahChild[i], GetLastError());
+                                    }
+                                if (    rc == 0
+                                    && !WriteProcessMemory(ProcInfo.hProcess, pbDst, &uBuf, cbChildPtr * 3, &cbActual2))
+                                    rc = errx(10, "Error writing standard handles at %p LB %u in the child: %u",
+                                              pbDst, cbChildPtr * 3, GetLastError());
+                            }
+                            else
+                                rc = errx(10, "Error reading %p LB %u from the child: %u",
+                                          pbDst, cbChildPtr * 3, GetLastError());
+                        }
+                        else
+                            rc = errx(10, "Error reading %p LB %u from the child: %u", pvSrcInPeb, cbChildPtr);
+                    }
+                    else
+                        rc = errx(10, "NtQueryInformationProcess failed: %#x", rcNt);
+
+                    /* Resume the bugger... */
+                    if (   rc == 0
+                        && !ResumeThread(ProcInfo.hThread))
+                        rc = errx(10, "ResumeThread failed: %u", GetLastError());
+
+                    /* .. or kill it. */
+                    if (rc != 0)
+                        TerminateProcess(ProcInfo.hProcess, rc);
+
+                    CloseHandle(ProcInfo.hThread);
+                    *phProcess = ProcInfo.hProcess;
+                    rc = 0;
+                }
+                else
+                    rc = errx(10, "CreateProcessA(%s) failed: %u", pszExecutable, GetLastError());
+            }
+        }
+        free(pszzEnv);
+    }
+    else
+        rc = errx(9, "out of memory!");
+    free(pszCmdLine);
+    return rc;
+}
+#endif /* KBUILD_OS_WINDOWS */
 
 /**
@@ -802 +1150 @@
  *                              is from the child or from our setup efforts.
  */
-static int kRedirectDoSpawn(const char *pszExecutable, int cArgs, char **papszArgs, int fWatcomBrainDamage, char **papszEnvVars,
-                            const char *pszCwd, const char *pszSavedCwd, unsigned cOrders, REDIRECTORDERS *paOrders,
+static int kRedirectDoSpawn(const char *pszExecutable, int cArgs, char **papszArgs, int fWatcomBrainDamage,
+                            char **papszEnvVars, const char *pszCwd, const char *pszSavedCwd,
+                            unsigned cOrders, REDIRECTORDERS *paOrders,
 #ifdef USE_POSIX_SPAWN
                             posix_spawn_file_actions_t *pFileActions,
@@ -863 +1212 @@
         }
 
+#ifndef KBUILD_OS_WINDOWS
         /*
          * Change working directory if so requested.
@@ -871 +1221 @@
                 rcExit = errx(10, "Failed to change directory to '%s'", pszCwd);
         }
+#endif /* KBUILD_OS_WINDOWS */
         if (rcExit == 0)
         {
-#ifndef USE_POSIX_SPAWN
+# if !defined(USE_POSIX_SPAWN) && !defined(KBUILD_OS_WINDOWS)
             /*
              * Execute the file orders.
              */
             FILE *pWorkingStdErr = NULL;
-# if defined(CONFIG_NEW_WIN_CHILDREN) && defined(KMK)
+#  if defined(CONFIG_NEW_WIN_CHILDREN) && defined(KMK)
             if (cOrders > 0)
                 MkWinChildExclusiveAcquire();
-# endif
+#  endif
             rcExit = kRedirectExecFdOrders(cOrders, paOrders, &pWorkingStdErr);
             if (rcExit == 0)
-#endif
-            {
-#ifdef KMK
+# endif
+            {
+# ifdef KMK
                 /*
                  * We're spawning from within kmk.
                  */
-#if defined(KBUILD_OS_WINDOWS)
-                /* Windows is slightly complicated due to handles and sub_proc.c. */
+# if defined(KBUILD_OS_WINDOWS)
+                /* Windows is slightly complicated due to handles and winchildren.c. */
+#  if 1
+                HANDLE hProcess = INVALID_HANDLE_VALUE;
+                rcExit = kRedirectCreateProcessWindows(pszExecutable, cArgs, papszArgs, papszEnvVars, pszCwd,
+                                                       cOrders, paOrders, &hProcess);
+                if (rcExit == 0)
+#  else
                 HANDLE  hProcess = (HANDLE)_spawnvpe(_P_NOWAIT, pszExecutable, papszArgs, papszEnvVars);
                 kRedirectRestoreFdOrders(cOrders, paOrders, &pWorkingStdErr);
-# ifdef CONFIG_NEW_WIN_CHILDREN
+#  ifdef CONFIG_NEW_WIN_CHILDREN
                 if (cOrders > 0)
                     MkWinChildExclusiveRelease();
+#  endif
+                if ((intptr_t)hProcess != -1)
 # endif
-                if ((intptr_t)hProcess != -1)
-                {
-# ifndef CONFIG_NEW_WIN_CHILDREN
+                {
+#  ifndef CONFIG_NEW_WIN_CHILDREN
                     if (process_kmk_register_redirect(hProcess, pPidSpawned) == 0)
-# else
+#  else
                     if (MkWinChildCreateRedirect((intptr_t)hProcess, pPidSpawned) == 0)
-# endif
+#  endif
                     {
                         if (cVerbosity > 0)
@@ -912 +1270 @@
                     {
                         DWORD dwTmp;
-# ifndef CONFIG_NEW_WIN_CHILDREN
+#  ifndef CONFIG_NEW_WIN_CHILDREN
                         warn("sub_proc is out of slots, waiting for child...");
-# else
+#  else
                         warn("MkWinChildCreateRedirect failed...");
-# endif
+#  endif
                         dwTmp = WaitForSingleObject(hProcess, INFINITE);
                         if (dwTmp != WAIT_OBJECT_0)
@@ -935 +1293 @@
                     }
                 }
+#  if 0
                 else
                     rcExit = err(10, "_spawnvpe(%s) failed", pszExecutable);
+#  endif
 
 # elif defined(KBUILD_OS_OS2)
@@ -951 +1311 @@
                     *pPidSpawned = 0;
                 }
-#else
+# else
                 rcExit = posix_spawnp(pPidSpawned, pszExecutable, pFileActions, NULL /*pAttr*/, papszArgs, papszEnvVars);
                 if (rcExit == 0)
@@ -963 +1323 @@
                     *pPidSpawned = 0;
                 }
-#endif
+# endif
 
 #else  /* !KMK */
@@ -969 +1329 @@
                  * Spawning from inside the kmk_redirect executable.
                  */
-# if defined(KBUILD_OS_WINDOWS) || defined(KBUILD_OS_OS2)
+# ifdef KBUILD_OS_WINDOWS
+                HANDLE hProcess = INVALID_HANDLE_VALUE;
+                rcExit = kRedirectCreateProcessWindows(pszExecutable, cArgs, papszArgs, papszEnvVars, pszCwd,
+                                                       cOrders, paOrders, &hProcess);
+                if (rcExit == 0)
+                {
+                    DWORD dwWait;
+                    do
+                        dwWait = WaitForSingleObject(hProcess, INFINITE);
+                    while (dwWait == WAIT_IO_COMPLETION || dwWait == WAIT_TIMEOUT);
+
+                    dwWait = 11;
+                    if (GetExitCodeProcess(hProcess, &dwWait))
+                        rcExit = dwWait;
+                    else
+                        rcExit = errx(11, "GetExitCodeProcess(%s) failed: %u", pszExecutable, GetLastError());
+                }
+
+#elif defined(KBUILD_OS_WINDOWS) || defined(KBUILD_OS_OS2)
                 errno  = 0;
 #  if defined(KBUILD_OS_WINDOWS)
@@ -1023 +1401 @@
 #endif /* !KMK */
             }
-#if defined(CONFIG_NEW_WIN_CHILDREN) && defined(KBUILD_OS_WINDOWS) && defined(KMK)
+#if defined(CONFIG_NEW_WIN_CHILDREN) && defined(KBUILD_OS_WINDOWS) && defined(KMK) && 0
             else if (cOrders > 0)
                 MkWinChildExclusiveRelease();
@@ -1441 +1819 @@
                 else if (fd < 0)
                     rcExit = errx(2, "error: negative fd %d (%s)", fd, pszValue);
+#ifdef ONLY_TARGET_STANDARD_HANDLES
+                else if (fd > 2)
+                    rcExit = errx(2, "error: %d is not a standard descriptor number", fd);
+#endif
                 else
                 {
@@ -1464 +1846 @@
                 else if (fd < 0)
                     rcExit = errx(2, "error: negative target descriptor %d ('-d %s')", fd, pszValue);
+#ifdef ONLY_TARGET_STANDARD_HANDLES
+                else if (fd > 2)
+                    rcExit = errx(2, "error: target %d is not a standard descriptor number", fd);
+#endif
                 else if (*pszEqual != '=')
                     rcExit = errx(2, "syntax error: expected '=' to follow target descriptor: '-d %s'", pszValue);
@@ -1608 +1994 @@
                         else if (fd < 0)
                             rcExit = errx(2, "error: negative fd %d (%s)", fd, argv[iArg]);
+#ifdef ONLY_TARGET_STANDARD_HANDLES
+                        else if (fd > 2)
+                            rcExit = errx(2, "error: %d is not a standard descriptor number", fd);
+#endif
                         else
                             break;
@@ -1688 +2078 @@
          * Do the spawning in a separate function (main is far to large as it is by now).
          */
-        rcExit = kRedirectDoSpawn(pszExecutable, argc - iArg, &argv[iArg], fWatcomBrainDamage, papszEnvVars, szCwd, pszSavedCwd,
+        rcExit = kRedirectDoSpawn(pszExecutable, argc - iArg, &argv[iArg], fWatcomBrainDamage,
+                                  papszEnvVars,
+                                  szCwd, pszSavedCwd,
 #ifdef USE_POSIX_SPAWN
                                   cOrders, aOrders, &FileActions, cVerbosity,

trunk/src/kmk/main.c

@@ -1470 +1470 @@
     if (!cpus)
       cpus = 1;
+#  ifdef CONFIG_NEW_WIN_CHILDREN
     if (cpus > 64)
       cpus = 64; /* (wait for multiple objects limit) */
+#  endif
     return cpus;
 
@@ -3129 +3131 @@
             setrlimit (RLIMIT_STACK, &stack_limit);
 #endif
-fprintf(stderr, "respawning 1..\n");
-fprintf(stderr, "respawning 2..\n");
-fprintf(stderr, "respawning 3..!\n");
 # if !defined(WINDOWS32) || !defined(CONFIG_NEW_WIN_CHILDREN)
           exec_command ((char **)nargv, environ);

trunk/src/kmk/w32/winchildren.c

@@ -76 +76 @@
 
 #include <Windows.h>
+#include <Winternl.h>
 #include <assert.h>
 #include <process.h>
@@ -270 +271 @@
 /** Kernel32!SetThreadGroupAffinity */
 static BOOL (WINAPI        *g_pfnSetThreadGroupAffinity)(HANDLE, CONST GROUP_AFFINITY *, GROUP_AFFINITY *);
+/** NTDLL!NtQueryInformationProcess */
+static NTSTATUS (NTAPI     *g_pfnNtQueryInformationProcess)(HANDLE, PROCESSINFOCLASS, PVOID, ULONG, PULONG);
+/** Set if the windows host is 64-bit. */
+static BOOL                 g_f64BitHost = (K_ARCH_BITS == 64);
 /** Windows version info.
  * @note Putting this before the volatile stuff, hoping to keep it in a
@@ -289 +294 @@
 static unsigned volatile    g_idxLastChildcareWorker = 0;
 
-/** Temporary RW lock for serializing kmkbuiltin_redirect and CreateProcess. */
+/** RW lock for serializing kmkbuiltin_redirect and CreateProcess. */
 static SRWLOCK              g_RWLock;
 
@@ -301 +306 @@
 void MkWinChildInit(unsigned int cJobSlots)
 {
+    HMODULE hmod;
+
     /*
      * Figure out how many childcare workers first.
@@ -322 +329 @@
 
     /*
+     * NTDLL imports that we need.
+     */
+    hmod = GetModuleHandleA("NTDLL.DLL");
+    *(FARPROC *)&g_pfnNtQueryInformationProcess = GetProcAddress(hmod, "NtQueryInformationProcess");
+    if (!g_pfnNtQueryInformationProcess)
+        fatal(NILF, 0, _("MkWinChildInit: NtQueryInformationProcess not found"));
+
+#if K_ARCH_BITS == 32
+    /*
+     * Initialize g_f64BitHost.
+     */
+    if (!IsWow64Process(GetCurrentProcess(), &g_f64BitHost))
+        fatal(NILF, INTSTR_LENGTH, _("MkWinChildInit: IsWow64Process failed: %u"), GetLastError());
+#elif K_ARCH_BITS == 64
+    assert(g_f64BitHost);
+#else
+# error "K_ARCH_BITS is bad/missing"
+#endif
+
+    /*
      * Figure out how many processor groups there are.
      * For that we need to first figure the windows version.
@@ -334 +361 @@
     if (g_VersionInfo.dwMajorVersion >= 6)
     {
-        HMODULE hmod = GetModuleHandleA("KERNEL32.DLL");
+        hmod = GetModuleHandleA("KERNEL32.DLL");
         *(FARPROC *)&g_pfnGetActiveProcessorGroupCount = GetProcAddress(hmod, "GetActiveProcessorGroupCount");
         *(FARPROC *)&g_pfnGetActiveProcessorCount      = GetProcAddress(hmod, "GetActiveProcessorCount");
@@ -365 +392 @@
     }
 
-    /* Temporary: */
+    /*
+     * For serializing with standard file handle manipulation (kmkbuiltin_redirect).
+     */
     InitializeSRWLock(&g_RWLock);
 }
@@ -465 +494 @@
     }
 }
+
+
+/**
+ * Does the actual process creation given.
+ *
+ * @returns 0 if there is anything to wait on, otherwise non-zero windows error.
+ * @param   pWorker             The childcare worker.
+ * @param   pChild              The child.
+ * @param   pwszImageName       The image path.
+ * @param   pwszCommandLine     The command line.
+ * @param   pwszzEnvironment    The enviornment block.
+ */
+static int mkWinChildcareWorkerCreateProcess(PWINCHILDCAREWORKER pWorker, PWINCHILD pChild, WCHAR const *pwszImageName,
+                                             WCHAR const *pwszCommandLine, WCHAR const *pwszzEnvironment)
+{
+    PROCESS_INFORMATION ProcInfo;
+    STARTUPINFOW        StartupInfo;
+    DWORD               fFlags       = CREATE_UNICODE_ENVIRONMENT;
+    BOOL const          fHaveHandles = pChild->u.Process.hStdErr != INVALID_HANDLE_VALUE
+                                    || pChild->u.Process.hStdOut != INVALID_HANDLE_VALUE;
+    BOOL                fRet;
+    DWORD               dwErr;
+#ifdef KMK
+    extern int          process_priority;
+#endif
+
+    /*
+     * Populate startup info.
+     *
+     * Turns out we can get away without passing TRUE for the inherit handles
+     * parameter to CreateProcess when we're not using STARTF_USESTDHANDLES.
+     * At least on NT, which is all worth caring about at this point + context IMO.
+     *
+     * Not inherting the handles is a good thing because it means we won't
+     * accidentally end up with a pipe handle or such intended for a different
+     * child process, potentially causing the EOF/HUP event to be delayed.
+     *
+     * Since the present handle inhertiance requirements only involves standard
+     * output and error, we'll never set the inherit handles flag and instead
+     * do manual handle duplication and planting.
+     */
+    memset(&StartupInfo, 0, sizeof(StartupInfo));
+    StartupInfo.cb = sizeof(StartupInfo);
+    GetStartupInfoW(&StartupInfo);
+    if (!fHaveHandles)
+        StartupInfo.dwFlags &= ~STARTF_USESTDHANDLES;
+    else
+    {
+        fFlags |= CREATE_SUSPENDED;
+        StartupInfo.dwFlags &= ~STARTF_USESTDHANDLES;
+
+        /* Don't pass CRT inheritance info to the child (from our parent actually). */
+        StartupInfo.cbReserved2 = 0;
+        StartupInfo.lpReserved2 = 0;
+    }
+
+    /*
+     * Flags.
+     */
+#ifdef KMK
+    switch (process_priority)
+    {
+        case 1: fFlags |= CREATE_SUSPENDED | IDLE_PRIORITY_CLASS; break;
+        case 2: fFlags |= CREATE_SUSPENDED | BELOW_NORMAL_PRIORITY_CLASS; break;
+        case 3: fFlags |= CREATE_SUSPENDED | NORMAL_PRIORITY_CLASS; break;
+        case 4: fFlags |= CREATE_SUSPENDED | HIGH_PRIORITY_CLASS; break;
+        case 5: fFlags |= CREATE_SUSPENDED | REALTIME_PRIORITY_CLASS; break;
+    }
+#endif
+    if (g_cProcessorGroups > 1)
+        fFlags |= CREATE_SUSPENDED;
+
+    /*
+     * Try create the process.
+     */
+    DB(DB_JOBS, ("CreateProcessW(%ls, %ls,,, TRUE, %#x...)\n", pwszImageName, pwszCommandLine, fFlags));
+    memset(&ProcInfo, 0, sizeof(ProcInfo));
+    AcquireSRWLockShared(&g_RWLock);
+
+    fRet = CreateProcessW((WCHAR *)pwszImageName, (WCHAR *)pwszCommandLine, NULL /*pProcSecAttr*/, NULL /*pThreadSecAttr*/,
+                          FALSE /*fInheritHandles*/, fFlags, (WCHAR *)pwszzEnvironment, NULL /*pwsz*/, &StartupInfo, &ProcInfo);
+    dwErr = GetLastError();
+
+    ReleaseSRWLockShared(&g_RWLock);
+    if (fRet)
+        pChild->u.Process.hProcess = ProcInfo.hProcess;
+    else
+    {
+        fprintf(stderr, "CreateProcess(%ls) failed: %u\n", pwszImageName, dwErr);
+        return pChild->iExitCode = (int)dwErr;
+    }
+
+    /*
+     * If the child is suspended, we've got some adjustment work to be done.
+     */
+    dwErr = ERROR_SUCCESS;
+    if (fFlags & CREATE_SUSPENDED)
+    {
+        /*
+         * First do handle inhertiance as that's the most complicated.
+         */
+        if (fHaveHandles)
+        {
+            /*
+             * Get the PEB address and figure out the child process bit count.
+             */
+            ULONG                     cbActual1 = 0;
+            PROCESS_BASIC_INFORMATION BasicInfo = { 0, 0, };
+            NTSTATUS rcNt = g_pfnNtQueryInformationProcess(ProcInfo.hProcess, ProcessBasicInformation,
+                                                           &BasicInfo, sizeof(BasicInfo), &cbActual1);
+            if (NT_SUCCESS(rcNt))
+            {
+                /*
+                 * Read the user process parameter pointer from the PEB.
+                 *
+                 * Note! Seems WOW64 processes starts out with a 64-bit PEB and
+                 *       process parameter block.
+                 */
+                BOOL const   f32BitPeb  = !g_f64BitHost;
+                ULONG  const cbChildPtr = f32BitPeb ? 4 : 8;
+                PVOID        pvSrcInPeb = (char *)BasicInfo.PebBaseAddress + (f32BitPeb ? 0x10 : 0x20);
+                char *       pbDst      = 0;
+                SIZE_T       cbActual2  = 0;
+                if (ReadProcessMemory(ProcInfo.hProcess, pvSrcInPeb, &pbDst, cbChildPtr, &cbActual2))
+                {
+                    /*
+                     * Duplicate the handles into the child.
+                     */
+                    union
+                    {
+                        ULONGLONG   au64Bit[2];
+                        ULONG       au32Bit[2];
+                    } WriteBuf;
+                    ULONG  idx = 0;
+                    HANDLE hChildStdOut = INVALID_HANDLE_VALUE;
+                    HANDLE hChildStdErr = INVALID_HANDLE_VALUE;
+
+                    pbDst += (f32BitPeb ? 0x1c : 0x28);
+                    if (pChild->u.Process.hStdOut != INVALID_HANDLE_VALUE)
+                    {
+                        if (DuplicateHandle(GetCurrentProcess(), pChild->u.Process.hStdOut, ProcInfo.hProcess,
+                                            &hChildStdOut, 0, TRUE /*fInheritable*/, DUPLICATE_SAME_ACCESS))
+                        {
+                            if (f32BitPeb)
+                                WriteBuf.au32Bit[idx++] = (DWORD)(uintptr_t)hChildStdOut;
+                            else
+                                WriteBuf.au64Bit[idx++] = (uintptr_t)hChildStdOut;
+                        }
+                        else
+                        {
+                            dwErr = GetLastError();
+                            fprintf(stderr, "Failed to duplicate %p (stdout) into the child: %u\n",
+                                    pChild->u.Process.hStdOut, dwErr);
+                        }
+                    }
+                    else
+                        pbDst += cbChildPtr;
+
+                    if (pChild->u.Process.hStdErr != INVALID_HANDLE_VALUE)
+                    {
+                        if (DuplicateHandle(GetCurrentProcess(), pChild->u.Process.hStdErr, ProcInfo.hProcess,
+                                            &hChildStdErr, 0, TRUE /*fInheritable*/, DUPLICATE_SAME_ACCESS))
+                        {
+                            if (f32BitPeb)
+                                WriteBuf.au32Bit[idx++] = (DWORD)(uintptr_t)hChildStdOut;
+                            else
+                                WriteBuf.au64Bit[idx++] = (uintptr_t)hChildStdOut;
+                        }
+                        else
+                        {
+                            dwErr = GetLastError();
+                            fprintf(stderr, "Failed to duplicate %p (stderr) into the child: %u\n",
+                                    pChild->u.Process.hStdOut, dwErr);
+                        }
+                    }
+
+                    /*
+                     * Finally write the handle values into the child.
+                     */
+                    if (   idx > 0
+                        && !WriteProcessMemory(ProcInfo.hProcess, pbDst, &WriteBuf, idx * cbChildPtr, &cbActual2))
+                    {
+                        dwErr = GetLastError();
+                        fprintf(stderr, "Failed to write %p LB %u into child: %u\n", pbDst, idx * cbChildPtr, dwErr);
+                    }
+                }
+                else
+                {
+                    dwErr = GetLastError();
+                    fprintf(stderr, "Failed to read %p LB %u from the child: %u\n", pvSrcInPeb, cbChildPtr, dwErr);
+                }
+            }
+            else
+            {
+                fprintf(stderr, "NtQueryInformationProcess failed on child: %#x\n", rcNt);
+                dwErr = (DWORD)rcNt;
+            }
+        }
+
+        /*
+         * Assign processor group (ignore failure).
+         */
+        if (g_cProcessorGroups > 1)
+        {
+            GROUP_AFFINITY Affinity = { ~(ULONG_PTR)0, pWorker->iProcessorGroup, { 0, 0, 0 } };
+            fRet = g_pfnSetThreadGroupAffinity(ProcInfo.hThread, &Affinity, NULL);
+            assert(fRet);
+        }
+
+#ifdef KMK
+        /*
+         * Set priority (ignore failure).
+         */
+        switch (process_priority)
+        {
+            case 1: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_IDLE); break;
+            case 2: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_BELOW_NORMAL); break;
+            case 3: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_NORMAL); break;
+            case 4: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_HIGHEST); break;
+            case 5: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_TIME_CRITICAL); break;
+            default: fRet = TRUE;
+        }
+        assert(fRet);
+#endif
+
+        /*
+         * Resume the thread if the adjustments succeeded, otherwise kill it.
+         */
+        if (dwErr == ERROR_SUCCESS)
+        {
+            fRet = ResumeThread(ProcInfo.hThread);
+            assert(fRet);
+            if (!fRet)
+            {
+                dwErr = GetLastError();
+                fprintf(stderr, "ResumeThread failed on child process: %u\n", dwErr);
+            }
+        }
+        if (dwErr != ERROR_SUCCESS)
+            TerminateProcess(ProcInfo.hProcess, dwErr);
+    }
+
+    /*
+     * Close unnecessary handles.
+     */
+    if (   pChild->u.Process.fCloseStdOut
+        && pChild->u.Process.hStdOut != INVALID_HANDLE_VALUE)
+    {
+        CloseHandle(pChild->u.Process.hStdOut);
+        pChild->u.Process.hStdOut      = INVALID_HANDLE_VALUE;
+        pChild->u.Process.fCloseStdOut = FALSE;
+    }
+    if (   pChild->u.Process.fCloseStdErr
+        && pChild->u.Process.hStdErr != INVALID_HANDLE_VALUE)
+    {
+        CloseHandle(pChild->u.Process.hStdErr);
+        pChild->u.Process.hStdErr      = INVALID_HANDLE_VALUE;
+        pChild->u.Process.fCloseStdErr = FALSE;
+    }
+
+    CloseHandle(ProcInfo.hThread);
+    return 0;
+}
+
 
 #define MKWCCWCMD_F_CYGWIN_SHELL    1
@@ -1215 +1508 @@
 static void mkWinChildcareWorkerThreadHandleProcess(PWINCHILDCAREWORKER pWorker, PWINCHILD pChild)
 {
-    PROCESS_INFORMATION     ProcInfo;
-    STARTUPINFOW            StartupInfo;
     WCHAR const            *pwszPath         = NULL;
     WCHAR                  *pwszzEnvironment = NULL;
@@ -1222 +1513 @@
     WCHAR                  *pwszImageName    = NULL;
     BOOL                    fNeedShell       = FALSE;
-    DWORD                   fFlags = CREATE_UNICODE_ENVIRONMENT;
-    BOOL                    fRet;
     int                     rc;
-#ifdef KMK
-    extern int process_priority;
-#endif
 
     /*
@@ -1249 +1535 @@
         else
             rc = mkWinChildcareWorkerConvertCommandlineWithShell(pwszImageName, pChild->u.Process.papszArgs, &pwszCommandLine);
-    }
-    if (rc == 0)
-    {
-        AcquireSRWLockShared(&g_RWLock); /* temporary */
 
         /*
-         * Populate startup info.
+         * Create the child process.
          */
-        memset(&StartupInfo, 0, sizeof(StartupInfo));
-        StartupInfo.cb = sizeof(StartupInfo);
-        GetStartupInfoW(&StartupInfo);
-        if (   pChild->u.Process.hStdErr == INVALID_HANDLE_VALUE
-            && pChild->u.Process.hStdOut == INVALID_HANDLE_VALUE)
-            StartupInfo.dwFlags &= ~STARTF_USESTDHANDLES;
-        else
-        {
-            StartupInfo.dwFlags |= STARTF_USESTDHANDLES;
-            StartupInfo.hStdOutput = pChild->u.Process.hStdOut != INVALID_HANDLE_VALUE
-                                   ? pChild->u.Process.hStdOut : GetStdHandle(STD_OUTPUT_HANDLE);
-            StartupInfo.hStdError  = pChild->u.Process.hStdErr != INVALID_HANDLE_VALUE
-                                   ? pChild->u.Process.hStdErr : GetStdHandle(STD_ERROR_HANDLE);
-            StartupInfo.hStdInput  = GetStdHandle(STD_INPUT_HANDLE);
-        }
-
-        /*
-         * Flags.
-         */
-#ifdef KMK
-        switch (process_priority)
-        {
-            case 1: fFlags |= CREATE_SUSPENDED | IDLE_PRIORITY_CLASS; break;
-            case 2: fFlags |= CREATE_SUSPENDED | BELOW_NORMAL_PRIORITY_CLASS; break;
-            case 3: fFlags |= CREATE_SUSPENDED | NORMAL_PRIORITY_CLASS; break;
-            case 4: fFlags |= CREATE_SUSPENDED | HIGH_PRIORITY_CLASS; break;
-            case 5: fFlags |= CREATE_SUSPENDED | REALTIME_PRIORITY_CLASS; break;
-        }
-#endif
-        if (g_cProcessorGroups > 1)
-            fFlags |= CREATE_SUSPENDED;
-
-        /*
-         * Try create the process.
-         */
-        DB(DB_JOBS, ("CreateProcessW(%ls, %ls,,, TRUE, %#x...)\n", pwszImageName, pwszCommandLine, fFlags));
-        memset(&ProcInfo, 0, sizeof(ProcInfo));
-        fRet = CreateProcessW(pwszImageName, pwszCommandLine, NULL /*pProcSecAttr*/, NULL /*pThreadSecAttr*/,
-                              TRUE /*fInheritHandles*/, fFlags, pwszzEnvironment, NULL /*pwsz*/, &StartupInfo, &ProcInfo);
-        rc = GetLastError();
-        ReleaseSRWLockShared(&g_RWLock); /* temporary */
-        if (fRet)
-        {
-            /*
-             * Make thread priority and affinity changes if necessary.
-             */
-            if (fFlags & CREATE_SUSPENDED)
-            {
-                BOOL fRet;
-                if (g_cProcessorGroups > 1)
-                {
-                    GROUP_AFFINITY Affinity = { ~(ULONG_PTR)0, pWorker->iProcessorGroup, { 0, 0, 0 } };
-                    fRet = g_pfnSetThreadGroupAffinity(ProcInfo.hThread, &Affinity, NULL);
-                    assert(fRet);
-                }
-#ifdef KMK
-                switch (process_priority)
-                {
-                    case 1: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_IDLE); break;
-                    case 2: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_BELOW_NORMAL); break;
-                    case 3: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_NORMAL); break;
-                    case 4: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_HIGHEST); break;
-                    case 5: fRet = SetThreadPriority(ProcInfo.hThread, THREAD_PRIORITY_TIME_CRITICAL); break;
-                    default: fRet = TRUE;
-                }
-                assert(fRet);
-#endif
-                fRet = ResumeThread(ProcInfo.hThread);
-                assert(fRet);
-                (void)fRet;
-            }
-
-            /*
-             * Close unncessary handles.
-             */
-            CloseHandle(ProcInfo.hThread);
-            pChild->u.Process.hProcess = ProcInfo.hProcess;
-
-            if (   pChild->u.Process.fCloseStdOut
-                && pChild->u.Process.hStdOut != INVALID_HANDLE_VALUE)
-            {
-                CloseHandle(pChild->u.Process.hStdOut);
-                pChild->u.Process.hStdOut      = INVALID_HANDLE_VALUE;
-                pChild->u.Process.fCloseStdOut = FALSE;
-            }
-            if (   pChild->u.Process.fCloseStdErr
-                && pChild->u.Process.hStdErr != INVALID_HANDLE_VALUE)
-            {
-                CloseHandle(pChild->u.Process.hStdErr);
-                pChild->u.Process.hStdErr      = INVALID_HANDLE_VALUE;
-                pChild->u.Process.fCloseStdErr = FALSE;
-            }
-
-            /*
-             * Wait for the child to complete.
-             */
-            mkWinChildcareWorkerWaitForProcess(pWorker, pChild, ProcInfo.hProcess);
+        if (rc == 0)
+        {
+            rc = mkWinChildcareWorkerCreateProcess(pWorker, pChild, pwszImageName, pwszCommandLine, pwszzEnvironment);
+            if (rc == 0)
+            {
+                /*
+                 * Wait for the child to complete.
+                 */
+                mkWinChildcareWorkerWaitForProcess(pWorker, pChild, pChild->u.Process.hProcess);
+            }
+            else
+                pChild->iExitCode = rc;
         }
         else
@@ -1513 +1710 @@
                     pChild->pNext = pTailExpect;
                     pTailActual = _InterlockedCompareExchangePointer(&g_pTailCompletedChildren, pChild, pTailExpect);
-                    if (pTailActual == pTailExpect)
+                    if (pTailActual != pTailExpect)
+                        pTailExpect = pTailActual;
+                    else
                     {
                         _InterlockedDecrement(&g_cPendingChildren);
@@ -2205 +2404 @@
 }
 
-/** Temporary serialization with kmkbuiltin_redirect. */
+/** Serialization with kmkbuiltin_redirect. */
 void MkWinChildExclusiveAcquire(void)
 {
@@ -2211 +2410 @@
 }
 
-/** Temporary serialization with kmkbuiltin_redirect. */
+/** Serialization with kmkbuiltin_redirect. */
 void MkWinChildExclusiveRelease(void)
 {