Changeset 3162 in kBuild

Timestamp:

Mar 20, 2018 3:13:12 AM (7 years ago)

Author:

bird

Message:

kmk/win: Pretty sure I made this lpReserved2 mistake before. Duh.

Location:

trunk/src/kmk

Files:

• kmkbuiltin/redirect.c (modified) (2 diffs)
• makeint.h (modified) (1 diff)
• w32/winchildren.c (modified) (5 diffs)

trunk/src/kmk/kmkbuiltin/redirect.c

@@ -979 +979 @@
         StartupInfo.cb = sizeof(StartupInfo);
         GetStartupInfoA(&StartupInfo);
+        StartupInfo.lpReserved2 = 0; /* No CRT file handle + descriptor info possible, sorry. */
+        StartupInfo.cbReserved2 = 0;
         StartupInfo.dwFlags &= ~STARTF_USESTDHANDLES;
 
@@ -1034 +1036 @@
                 /*
                  * 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))

trunk/src/kmk/makeint.h

@@ -653 +653 @@
 #define OSSNS(_t,_a,_f,_s1,_s2,_n,_s3) _t((_a), strlen (_s1) + strlen (_s2) + strlen (_s3) + INTSTR_LENGTH, \
                                           (_f), (_s1), (_s2), (_n), (_s3))          /* bird */
+#define ONNS(_t,_a,_f,_n1,_n2,_s1)     _t((_a), INTSTR_LENGTH * 2 + strlen (_s1), \
+                                          (_f), (_n1), (_n2), (_s1))                /* bird */
+#define ONNNS(_t,_a,_f,_n1,_n2,_n3,_s1) _t((_a), INTSTR_LENGTH * 3 + strlen (_s1), \
+                                          (_f), (_n1), (_n2), (_n3), (_s1))         /* bird */
 
 #define OUT_OF_MEM() O (fatal, NILF, _("virtual memory exhausted"))

trunk/src/kmk/w32/winchildren.c

@@ -396 +396 @@
      */
     InitializeSRWLock(&g_RWLock);
+
+    /*
+     * This is dead code that was thought to fix a problem observed doing
+     * `tcc.exe /c "kmk |& tee bld.log"` and leading to a crash in cl.exe
+     * when spawned with fInheritHandles = FALSE, see hStdErr=NULL in the
+     * child.  However, it turns out this was probably caused by not clearing
+     * the CRT file descriptor and handle table in the startup info.
+     * Leaving the code here in case it comes in handy after all.
+     */
+#if 0
+    {
+        struct
+        {
+            DWORD  uStdHandle;
+            HANDLE hHandle;
+        } aHandles[3] = { { STD_INPUT_HANDLE, NULL }, { STD_OUTPUT_HANDLE, NULL }, { STD_ERROR_HANDLE, NULL } };
+        int i;
+
+        for (i = 0; i < 3; i++)
+            aHandles[i].hHandle = GetStdHandle(aHandles[i].uStdHandle);
+
+        for (i = 0; i < 3; i++)
+            if (   aHandles[i].hHandle == NULL
+                || aHandles[i].hHandle == INVALID_HANDLE_VALUE)
+            {
+                int fd = open("nul", _O_RDWR);
+                if (fd >= 0)
+                {
+                    if (_dup2(fd, i) >= 0)
+                    {
+                        assert((HANDLE)_get_osfhandle(i) != aHandles[i].hHandle);
+                        assert((HANDLE)_get_osfhandle(i) == GetStdHandle(aHandles[i].uStdHandle));
+                    }
+                    else
+                        ONNNS(fatal, NILF, "_dup2(%d('nul'), %d) failed: %u (%s)", fd, i, errno, strerror(errno));
+                    if (fd != i)
+                        close(fd);
+                }
+                else
+                    ONNS(fatal, NILF, "open(nul,RW) failed: %u (%s)", i, errno, strerror(errno));
+            }
+            else
+            {
+                int j;
+                for (j = i + 1; j < 3; j++)
+                    if (aHandles[j].hHandle == aHandles[i].hHandle)
+                    {
+                        int fd = _dup(j);
+                        if (fd >= 0)
+                        {
+                            if (_dup2(fd, j) >= 0)
+                            {
+                                aHandles[j].hHandle = (HANDLE)_get_osfhandle(j);
+                                assert(aHandles[j].hHandle != aHandles[i].hHandle);
+                                assert(aHandles[j].hHandle == GetStdHandle(aHandles[j].uStdHandle));
+                            }
+                            else
+                                ONNNS(fatal, NILF, "_dup2(%d, %d) failed: %u (%s)", fd, j, errno, strerror(errno));
+                            if (fd != j)
+                                close(fd);
+                        }
+                        else
+                            ONNS(fatal, NILF, "_dup(%d) failed: %u (%s)", j, errno, strerror(errno));
+                    }
+            }
+    }
+#endif
 }
 
@@ -538 +605 @@
     StartupInfo.cb = sizeof(StartupInfo);
     GetStartupInfoW(&StartupInfo);
+    StartupInfo.lpReserved2 = 0; /* No CRT file handle + descriptor info possible, sorry. */
+    StartupInfo.cbReserved2 = 0;
     if (!fHaveHandles)
         StartupInfo.dwFlags &= ~STARTF_USESTDHANDLES;
@@ -544 +613 @@
         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;
     }
 
@@ -2404 +2469 @@
 }
 
+#if 0  /* no longer needed */
 /** Serialization with kmkbuiltin_redirect. */
 void MkWinChildExclusiveAcquire(void)
@@ -2415 +2481 @@
     ReleaseSRWLockExclusive(&g_RWLock);
 }
+#endif
 
 /**