Add tests for CreateProcess UseStdHandles !InheritHandles

misc/buffer-tests/HandleTests/CreateProcess_UseStdHandles_NoInherit.cc

@@ -0,0 +1,138 @@
+#include <TestCommon.h>
+
+template <typename T>
+void checkVariousInputs(T check) {
+    {
+        // Specify the original std values.  Starting with Windows 8, this code
+        // produces NULL child standard handles.  It used to produce valid
+        // child standard handles.  (i.e. It used to work, but no longer does.)
+        Worker p;
+        check(p, true, stdHandles(p));
+    }
+    {
+        // Completely invalid handles.
+        Worker p;
+        check(p, false, {
+            Handle::invent(nullptr, p),
+            Handle::invent(0x10000ull, p),
+            Handle::invent(0xdeadbeefull, p),
+        });
+        check(p, false, {
+            Handle::invent(INVALID_HANDLE_VALUE, p),
+            Handle::invent(nullptr, p),
+            Handle::invent(nullptr, p),
+        });
+    }
+    {
+        // Try a non-inheritable pipe.
+        Worker p;
+        auto pipe = newPipe(p, false);
+        check(p, true, {
+            std::get<0>(pipe),
+            std::get<1>(pipe),
+            std::get<1>(pipe),
+        });
+    }
+    {
+        // Try an inheritable pipe.
+        Worker p;
+        auto pipe = newPipe(p, true);
+        check(p, true, {
+            std::get<0>(pipe),
+            std::get<1>(pipe),
+            std::get<1>(pipe),
+        });
+    }
+}
+
+//
+// Test CreateProcess when called with these parameters:
+//  - bInheritHandles=FALSE
+//  - STARTF_USESTDHANDLES is specified
+//
+// Before Windows 8, the child process has the same standard handles as the
+// parent, without exception, and they might not be valid.
+//
+// As of Windows 8, the behavior depends upon the CreationConsole Mode:
+//  - If it is Inherit, then the new standard handles are NULL.
+//  - If it is NewConsole or NewConsoleWindow, then the child has three new
+//    console handles, which FreeConsole will close.
+//
+
+//
+// Part 1: CreationConsoleMode is Inherit
+//
+REGISTER(Test_CreateProcess_UseStdHandles_NoInherit_InheritConsoleMode, always);
+static void Test_CreateProcess_UseStdHandles_NoInherit_InheritConsoleMode() {
+    checkVariousInputs([](Worker &p,
+                    bool validHandles,
+                    std::vector<Handle> newHandles) {
+        ASSERT(newHandles.size() == 3);
+        auto c = p.child({false, 0, newHandles});
+        if (isTraditionalConio()) {
+            CHECK(handleValues(stdHandles(c)) == handleValues(newHandles));
+            checkInitConsoleHandleSet(c, p);
+            // The child handles have the same values as the parent.  If the
+            // parent handles are valid kernel handles, then the child handles
+            // are guaranteed not to reference the same object.
+            auto childHandles = stdHandles(c);
+            ObjectSnap snap;
+            for (int i = 0; i < 3; ++i) {
+                CHECK(!validHandles ||
+                      newHandles[i].isTraditionalConsole() ||
+                      !snap.eq(newHandles[i], childHandles[i]));
+            }
+        } else {
+            CHECK(handleInts(stdHandles(c)) == (std::vector<uint64_t> {0,0,0}));
+        }
+    });
+}
+
+//
+// Part 2: CreationConsoleMode is NewConsole
+//
+
+REGISTER(Test_CreateProcess_UseStdHandles_NoInherit_NewConsoleMode, always);
+static void Test_CreateProcess_UseStdHandles_NoInherit_NewConsoleMode() {
+    checkVariousInputs([](Worker &p,
+                          bool validHandles,
+                          std::vector<Handle> newHandles) {
+        { auto h = p.openConout(); h.setFirstChar('P'); h.close(); }
+        auto c = p.child({false, CREATE_NEW_CONSOLE, newHandles});
+        { auto h = c.openConout(); CHECK_EQ(h.firstChar(), ' '); h.close(); }
+        if (isTraditionalConio()) {
+            CHECK(handleValues(stdHandles(c)) == handleValues(newHandles));
+            checkInitConsoleHandleSet(c);
+            // The child handles have the same values as the parent.  If the
+            // parent handles are valid kernel handles, then the child handles
+            // are guaranteed not to reference the same object.
+            auto childHandles = stdHandles(c);
+            ObjectSnap snap;
+            for (int i = 0; i < 3; ++i) {
+                CHECK(!validHandles ||
+                      newHandles[i].isTraditionalConsole() ||
+                      !snap.eq(newHandles[i], childHandles[i]));
+            }
+        } else {
+            // Windows 8 acts exactly as if STARTF_USESTDHANDLES hadn't been
+            // specified.
+            //
+            // There are three new handles to two new Unbound console objects.
+            ObjectSnap snap;
+            CHECK(isUsableConsoleInputHandle(c.getStdin()));
+            CHECK(isUsableConsoleOutputHandle(c.getStdout()));
+            CHECK(isUsableConsoleOutputHandle(c.getStderr()));
+            CHECK(c.getStdout().value() != c.getStderr().value());
+            CHECK(snap.eq(c.getStdout(), c.getStderr()));
+            CHECK(isUnboundConsoleObject(c.getStdin()));
+            CHECK(isUnboundConsoleObject(c.getStdout()));
+            CHECK(isUnboundConsoleObject(c.getStderr()));
+
+            // Aside: calling FreeConsole closes the three handles it opened.
+            c.detach();
+            CHECK(!c.getStdin().tryFlags());
+            CHECK(!c.getStdout().tryFlags());
+            CHECK(!c.getStderr().tryFlags());
+        }
+    });
+}

misc/buffer-tests/HandleTests/Traditional.cc

@@ -1,23 +1,5 @@
 #include <TestCommon.h>
 
-// Verify that the child's open console handle set is as expected from having
-// just attached to or spawned from a source worker.
-//  * The set of child handles should exactly match the set of inheritable
-//    source handles.
-//  * Every open child handle should be inheritable.
-static void checkAttachHandleSet(Worker &child, Worker &source) {
-    auto cvec = child.scanForConsoleHandles();
-    auto cvecInherit = inheritableHandles(cvec);
-    auto svecInherit = inheritableHandles(source.scanForConsoleHandles());
-    auto hv = &handleValues;
-    if (hv(cvecInherit) == hv(svecInherit) && allInheritable(cvec)) {
-        return;
-    }
-    source.dumpConsoleHandles();
-    child.dumpConsoleHandles();
-    CHECK(false && "checkAttachHandleSet failed");
-}
-
 REGISTER(Test_HandleDuplication, isTraditionalConio);
 static void Test_HandleDuplication() {
     // A traditional console handle cannot be duplicated to another process,
@@ -209,8 +191,8 @@ static void Test_AttachConsole_And_CreateProcess_Inheritance() {
     auto conin = p.getStdin().dup(true);
     auto conout1 = p.getStdout().dup(true);
     auto conout2 = p.getStderr().dup(true);
-    p.openConout(false);    // an extra handle for checkAttachHandleSet testing
-    p.openConout(true);     // an extra handle for checkAttachHandleSet testing
+    p.openConout(false); // an extra handle for checkInitConsoleHandleSet testing
+    p.openConout(true);  // an extra handle for checkInitConsoleHandleSet testing
     p.getStdin().close();
     p.getStdout().close();
     p.getStderr().close();
@@ -243,9 +225,9 @@ static void Test_AttachConsole_And_CreateProcess_Inheritance() {
 
     // The set of inheritable console handles in these processes exactly match
     // that of the parent.
-    checkAttachHandleSet(c, p);
-    checkAttachHandleSet(c2, p);
-    checkAttachHandleSet(unrelated, p);
+    checkInitConsoleHandleSet(c, p);
+    checkInitConsoleHandleSet(c2, p);
+    checkInitConsoleHandleSet(unrelated, p);
 }
 
 REGISTER(Test_Detach_Implicitly_Closes_Handles, isTraditionalConio);

misc/buffer-tests/Makefile

@@ -45,6 +45,7 @@ HANDLETESTS_OBJECTS = \
     build/obj/HandleTests/CreateProcess_DefaultInherit.o \
     build/obj/HandleTests/CreateProcess_DefaultInherit_PseudoHandleBug.o \
     build/obj/HandleTests/CreateProcess_DefaultInherit_XPPipeBug.o \
+    build/obj/HandleTests/CreateProcess_UseStdHandles_NoInherit.o \
     build/obj/HandleTests/MiscTests.o \
     build/obj/HandleTests/Modern.o \
     build/obj/HandleTests/Traditional.o \

misc/buffer-tests/harness/RemoteHandle.h

@@ -65,6 +65,7 @@ public:
     bool tryNumberOfConsoleInputEvents(DWORD *ret=nullptr);
     HANDLE value() const { return m_value; }
     uint64_t uvalue() const { return reinterpret_cast<uint64_t>(m_value); }
+    bool isTraditionalConsole() const { return (uvalue() & 3) == 3; }
     RemoteWorker &worker() const { return *m_worker; }
 
 private:

misc/buffer-tests/harness/TestUtil.cc

@@ -5,6 +5,7 @@
 #include <string>
 #include <vector>
 
+#include "OsVersion.h"
 #include "NtHandleQuery.h"
 #include "RemoteHandle.h"
 #include "RemoteWorker.h"
@@ -16,20 +17,6 @@
 
 static RegistrationTable *g_testFunctions;
 
-std::tuple<RemoteHandle, RemoteHandle> newPipe(
-        RemoteWorker &w, BOOL inheritable) {
-    HANDLE readPipe, writePipe;
-    auto ret = CreatePipe(&readPipe, &writePipe, NULL, 0);
-    ASSERT(ret && "CreatePipe failed");
-    auto p1 = RemoteHandle::dup(readPipe, w, inheritable);
-    auto p2 = RemoteHandle::dup(writePipe, w, inheritable);
-    trace("Opened pipe in pid %u: rh=0x%I64x wh=0x%I64x",
-        w.pid(), p1.uvalue(), p2.uvalue());
-    CloseHandle(readPipe);
-    CloseHandle(writePipe);
-    return std::make_tuple(p1, p2);
-}
-
 void printTestName(const std::string &name) {
     trace("----------------------------------------------------------");
     trace("%s", name.c_str());
@@ -37,12 +24,19 @@ void printTestName(const std::string &name) {
     fflush(stdout);
 }
 
-std::string windowText(HWND hwnd) {
-    std::array<wchar_t, 256> buf;
-    DWORD ret = GetWindowTextW(hwnd, buf.data(), buf.size());
-    ASSERT(ret >= 0 && ret <= buf.size() - 1);
-    buf[ret] = L'\0';
-    return narrowString(std::wstring(buf.data()));
+void registerTest(const std::string &name, bool (&cond)(), void (&func)()) {
+    if (g_testFunctions == nullptr) {
+        g_testFunctions = new RegistrationTable {};
+    }
+    for (auto &entry : *g_testFunctions) {
+        // I think the compiler catches duplicates already, but just in case.
+        ASSERT(&cond != std::get<1>(entry) || &func != std::get<2>(entry));
+    }
+    g_testFunctions->push_back(std::make_tuple(name, &cond, &func));
+}
+
+RegistrationTable registeredTests() {
+    return *g_testFunctions;
 }
 
 // Get the ObjectPointer (underlying NT object) for the NT handle.
@@ -108,9 +102,17 @@ bool compareObjectHandles(RemoteHandle h1, RemoteHandle h2) {
 ObjectSnap::ObjectSnap() {
     if (!hasBuiltinCompareObjectHandles()) {
         m_table = queryNtHandles();
+        m_hasTable = true;
     }
 }
 
+void *ObjectSnap::object(RemoteHandle h) {
+    if (!m_hasTable) {
+        m_table = queryNtHandles();
+    }
+    return ntHandlePointer(m_table, h);
+}
+
 bool ObjectSnap::eq(std::initializer_list<RemoteHandle> handles) {
     if (handles.size() < 2) {
         return true;
@@ -132,17 +134,92 @@ bool ObjectSnap::eq(std::initializer_list<RemoteHandle> handles) {
     return true;
 }
 
-void registerTest(const std::string &name, bool (&cond)(), void (&func)()) {
-    if (g_testFunctions == nullptr) {
-        g_testFunctions = new RegistrationTable {};
-    }
-    for (auto &entry : *g_testFunctions) {
-        // I think the compiler catches duplicates already, but just in case.
-        ASSERT(&cond != std::get<1>(entry) || &func != std::get<2>(entry));
-    }
-    g_testFunctions->push_back(std::make_tuple(name, &cond, &func));
+std::tuple<RemoteHandle, RemoteHandle> newPipe(
+        RemoteWorker &w, BOOL inheritable) {
+    HANDLE readPipe, writePipe;
+    auto ret = CreatePipe(&readPipe, &writePipe, NULL, 0);
+    ASSERT(ret && "CreatePipe failed");
+    auto p1 = RemoteHandle::dup(readPipe, w, inheritable);
+    auto p2 = RemoteHandle::dup(writePipe, w, inheritable);
+    trace("Opened pipe in pid %u: rh=0x%I64x wh=0x%I64x",
+        w.pid(), p1.uvalue(), p2.uvalue());
+    CloseHandle(readPipe);
+    CloseHandle(writePipe);
+    return std::make_tuple(p1, p2);
 }
 
-RegistrationTable registeredTests() {
-    return *g_testFunctions;
+std::string windowText(HWND hwnd) {
+    std::array<wchar_t, 256> buf;
+    DWORD ret = GetWindowTextW(hwnd, buf.data(), buf.size());
+    ASSERT(ret >= 0 && ret <= buf.size() - 1);
+    buf[ret] = L'\0';
+    return narrowString(std::wstring(buf.data()));
+}
+
+// Verify that the process' open console handle set is as expected from
+// attaching to a new console.
+//  * The set of console handles is exactly (0x3, 0x7, 0xb).
+//  * The console handles are inheritable.
+void checkInitConsoleHandleSet(RemoteWorker &proc) {
+    CHECK(isTraditionalConio() && "checkInitConsoleHandleSet is not valid "
+                                  "with modern conio");
+    auto actualHandles = proc.scanForConsoleHandles();
+    auto correctHandles = std::vector<uint64_t> { 0x3, 0x7, 0xb };
+    if (handleInts(actualHandles) == correctHandles &&
+            allInheritable(actualHandles)) {
+        return;
+    }
+    proc.dumpConsoleHandles();
+    CHECK(false && "checkInitConsoleHandleSet failed");
+}
+
+// Verify that the child's open console handle set is as expected from having
+// just attached to or spawned from a source worker.
+//  * The set of child handles should exactly match the set of inheritable
+//    source handles.
+//  * Every open child handle should be inheritable.
+void checkInitConsoleHandleSet(RemoteWorker &child, RemoteWorker &source) {
+    ASSERT(isTraditionalConio() && "checkInitConsoleHandleSet is not valid "
+                                   "with modern conio");
+    auto cvec = child.scanForConsoleHandles();
+    auto cvecInherit = inheritableHandles(cvec);
+    auto svecInherit = inheritableHandles(source.scanForConsoleHandles());
+    auto hv = &handleValues;
+    if (hv(cvecInherit) == hv(svecInherit) && allInheritable(cvec)) {
+        return;
+    }
+    source.dumpConsoleHandles();
+    child.dumpConsoleHandles();
+    CHECK(false && "checkInitConsoleHandleSet failed");
+}
+
+// Returns true if the handle is a "usable console handle":
+//  * The handle must be open.
+//  * It must be a console handle.
+//  * The process must have an attached console.
+//  * With modern conio, the handle must be "unbound" or bound to the
+//    currently attached console.
+bool isUsableConsoleHandle(RemoteHandle h) {
+    // XXX: It would be more efficient/elegant to use GetConsoleMode instead.
+    return h.tryNumberOfConsoleInputEvents() || h.tryScreenBufferInfo();
+}
+
+bool isUsableConsoleInputHandle(RemoteHandle h) {
+    return h.tryNumberOfConsoleInputEvents();
+}
+
+bool isUsableConsoleOutputHandle(RemoteHandle h) {
+    return h.tryScreenBufferInfo();
+}
+
+bool isUnboundConsoleObject(RemoteHandle h) {
+    // XXX: Consider what happens here with NULL, INVALID_HANDLE_OBJECT, junk,
+    // etc.  I *think* it should work.
+    ASSERT(isModernConio() && "isUnboundConsoleObject is not valid with "
+                              "traditional conio");
+    static RemoteWorker other{ SpawnParams {false, CREATE_NO_WINDOW} };
+    auto dup = h.dup(other);
+    bool ret = isUsableConsoleHandle(dup);
+    dup.close();
+    return ret;
 }

misc/buffer-tests/harness/TestUtil.h

@@ -41,11 +41,11 @@ class RemoteWorker;
     int g_register_ ## cond ## _ ## name = (registerTest(#name, cond, name), 0)
 
 // Test registration
+void printTestName(const std::string &name);
 void registerTest(const std::string &name, bool(&cond)(), void(&func)());
 using RegistrationTable = std::vector<std::tuple<std::string, bool(*)(), void(*)()>>;
 RegistrationTable registeredTests();
 inline bool always() { return true; }
-void printTestName(const std::string &name);
 
 // NT kernel handle query
 void *ntHandlePointer(const std::vector<SYSTEM_HANDLE_ENTRY> &table,
@@ -57,9 +57,11 @@ bool compareObjectHandles(RemoteHandle h1, RemoteHandle h2);
 class ObjectSnap {
 public:
     ObjectSnap();
+    void *object(RemoteHandle h);
     bool eq(std::initializer_list<RemoteHandle> handles);
     bool eq(RemoteHandle h1, RemoteHandle h2) { return eq({h1, h2}); }
 private:
+    bool m_hasTable = false;
     std::vector<SYSTEM_HANDLE_ENTRY> m_table;
 };
 
@@ -67,3 +69,11 @@ private:
 std::tuple<RemoteHandle, RemoteHandle> newPipe(
         RemoteWorker &w, BOOL inheritable=FALSE);
 std::string windowText(HWND hwnd);
+
+// "domain-specific" routines: perhaps these belong outside the harness?
+void checkInitConsoleHandleSet(RemoteWorker &child);
+void checkInitConsoleHandleSet(RemoteWorker &child, RemoteWorker &source);
+bool isUsableConsoleHandle(RemoteHandle h);
+bool isUsableConsoleInputHandle(RemoteHandle h);
+bool isUsableConsoleOutputHandle(RemoteHandle h);
+bool isUnboundConsoleObject(RemoteHandle h);