[PATCH v3 0/4] MR4441: Adding tests for CreateProcess and std handles.
Various new tests for CreateProcess showing a couple of issues in current implementation. -- v3: kernelbase: Reset std handles gotten by GetStartupInfo(). ntdll,server: Revisit std handles inheritance. kernel32/tests: Add more tests about CreateProcess. kernel32/tests: Introduce a new infrastructure for testing CreateProcess(). https://gitlab.winehq.org/wine/wine/-/merge_requests/4441
From: Eric Pouech <epouech(a)codeweavers.com> Move a couple of existing tests to this infrastructure. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 394 ++++++++++++++++++++++++---------- 1 file changed, 278 insertions(+), 116 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 8bb2748f8e1..028906660ca 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -317,6 +317,37 @@ static void WINAPIV __WINE_PRINTF_ATTR(2,3) childPrintf(HANDLE h, const char* fm WriteFile(h, buffer, strlen(buffer), &w, NULL); } +/* bits 0..1 contains FILE_TYPE_{UNKNOWN, CHAR, PIPE, DISK} */ +#define HATTR_NULL 0x08 /* NULL handle value */ +#define HATTR_INVALID 0x04 /* INVALID_HANDLE_VALUE */ +#define HATTR_TYPE 0x0c /* valid handle, with type set */ +#define HATTR_INHERIT 0x10 /* inheritance flag set */ + +static unsigned encode_handle_attributes(HANDLE h) +{ + DWORD dw; + unsigned result; + + if (h == NULL) + result = HATTR_NULL; + else if (h == INVALID_HANDLE_VALUE) + result = HATTR_INVALID; + else + { + result = HATTR_TYPE; + dw = GetFileType(h); + if (dw == FILE_TYPE_CHAR || dw == FILE_TYPE_DISK || dw == FILE_TYPE_PIPE) + { + DWORD info; + if (GetHandleInformation(h, &info) && (info & HANDLE_FLAG_INHERIT)) + result |= HATTR_INHERIT; + } + else + dw = FILE_TYPE_UNKNOWN; + result |= dw; + } + return result; +} /****************************************************************** * doChild @@ -347,17 +378,24 @@ static void doChild(const char* file, const char* option) "dwX=%lu\ndwY=%lu\ndwXSize=%lu\ndwYSize=%lu\n" "dwXCountChars=%lu\ndwYCountChars=%lu\ndwFillAttribute=%lu\n" "dwFlags=%lu\nwShowWindow=%u\n" - "hStdInput=%Iu\nhStdOutput=%Iu\nhStdError=%Iu\n\n", + "hStdInput=%Iu\nhStdOutput=%Iu\nhStdError=%Iu\n" + "hStdInputEncode=%u\nhStdOutputEncode=%u\nhStdErrorEncode=%u\n\n", siA.cb, encodeA(siA.lpDesktop), encodeA(siA.lpTitle), siA.dwX, siA.dwY, siA.dwXSize, siA.dwYSize, siA.dwXCountChars, siA.dwYCountChars, siA.dwFillAttribute, siA.dwFlags, siA.wShowWindow, - (DWORD_PTR)siA.hStdInput, (DWORD_PTR)siA.hStdOutput, (DWORD_PTR)siA.hStdError); + (DWORD_PTR)siA.hStdInput, (DWORD_PTR)siA.hStdOutput, (DWORD_PTR)siA.hStdError, + encode_handle_attributes(siA.hStdInput), encode_handle_attributes(siA.hStdOutput), + encode_handle_attributes(siA.hStdError)); /* check the console handles in the TEB */ - childPrintf(hFile, "[TEB]\nhStdInput=%Iu\nhStdOutput=%Iu\nhStdError=%Iu\n\n", + childPrintf(hFile, + "[TEB]\nhStdInput=%Iu\nhStdOutput=%Iu\nhStdError=%Iu\n" + "hStdInputEncode=%u\nhStdOutputEncode=%u\nhStdErrorEncode=%u\n\n", (DWORD_PTR)params->hStdInput, (DWORD_PTR)params->hStdOutput, - (DWORD_PTR)params->hStdError); + (DWORD_PTR)params->hStdError, + encode_handle_attributes(params->hStdInput), encode_handle_attributes(params->hStdOutput), + encode_handle_attributes(params->hStdError)); /* since GetStartupInfoW is only implemented in win2k, * zero out before calling so we can notice the difference @@ -369,12 +407,15 @@ static void doChild(const char* file, const char* option) "dwX=%lu\ndwY=%lu\ndwXSize=%lu\ndwYSize=%lu\n" "dwXCountChars=%lu\ndwYCountChars=%lu\ndwFillAttribute=%lu\n" "dwFlags=%lu\nwShowWindow=%u\n" - "hStdInput=%Iu\nhStdOutput=%Iu\nhStdError=%Iu\n\n", + "hStdInput=%Iu\nhStdOutput=%Iu\nhStdError=%Iu\n" + "hStdInputEncode=%u\nhStdOutputEncode=%u\nhStdErrorEncode=%u\n\n", siW.cb, encodeW(siW.lpDesktop), encodeW(siW.lpTitle), siW.dwX, siW.dwY, siW.dwXSize, siW.dwYSize, siW.dwXCountChars, siW.dwYCountChars, siW.dwFillAttribute, siW.dwFlags, siW.wShowWindow, - (DWORD_PTR)siW.hStdInput, (DWORD_PTR)siW.hStdOutput, (DWORD_PTR)siW.hStdError); + (DWORD_PTR)siW.hStdInput, (DWORD_PTR)siW.hStdOutput, (DWORD_PTR)siW.hStdError, + encode_handle_attributes(siW.hStdInput), encode_handle_attributes(siW.hStdOutput), + encode_handle_attributes(siW.hStdError)); /* Arguments */ childPrintf(hFile, "[Arguments]\nargcA=%d\n", myARGC); @@ -595,10 +636,17 @@ static void ok_child_int( int line, const char *sect, const char *key, UINT expe ok_(__FILE__, line)( result == expect, "%s:%s expected %u, but got %u\n", sect, key, expect, result ); } +static inline void ok_child_hexint( int line, const char *sect, const char *key, UINT expect, UINT is_broken ) +{ + UINT result = GetPrivateProfileIntA( sect, key, !expect, resfile ); + ok_(__FILE__, line)( result == expect || broken( is_broken && result == is_broken ), "%s:%s expected %#x, but got %#x\n", sect, key, expect, result ); +} + #define okChildString(sect, key, expect) ok_child_string(__LINE__, (sect), (key), (expect), 1 ) #define okChildIString(sect, key, expect) ok_child_string(__LINE__, (sect), (key), (expect), 0 ) #define okChildStringWA(sect, key, expect) ok_child_stringWA(__LINE__, (sect), (key), (expect), 1 ) #define okChildInt(sect, key, expect) ok_child_int(__LINE__, (sect), (key), (expect)) +#define okChildHexInt(sect, key, expect, is_broken) ok_child_hexint(__LINE__, (sect), (key), (expect), (is_broken)) static void test_Startup(void) { @@ -3057,72 +3105,241 @@ static void test_BreakawayOk(HANDLE parent_job) ok(ret, "SetInformationJobObject error %lu\n", GetLastError()); } -static void test_StartupNoConsole(void) +/* copy an executable, but changing its subsystem */ +static inline void copy_change_subsystem(const char* in, const char* out, DWORD subsyst) +{ + BOOL ret; + HANDLE hFile, hMap; + void* mapping; + IMAGE_NT_HEADERS *nthdr; + + ret = CopyFileA(in, out, FALSE); + ok(ret, "Failed to copy executable %s in %s (%lu)\n", in, out, GetLastError()); + + hFile = CreateFileA(out, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + ok(hFile != INVALID_HANDLE_VALUE, "Couldn't open file %s (%lu)\n", out, GetLastError()); + hMap = CreateFileMappingW(hFile, NULL, PAGE_READWRITE, 0, 0, NULL); + ok(hMap != NULL, "Couldn't create map (%lu)\n", GetLastError()); + mapping = MapViewOfFile(hMap, FILE_MAP_ALL_ACCESS, 0, 0, 0); + ok(mapping != NULL, "Couldn't map (%lu)\n", GetLastError()); + nthdr = RtlImageNtHeader(mapping); + ok(nthdr != NULL, "Cannot get NT headers out of %s\n", out); + if (nthdr) nthdr->OptionalHeader.Subsystem = subsyst; + ret = UnmapViewOfFile(mapping); + ok(ret, "Couldn't unmap (%lu)\n", GetLastError()); + CloseHandle(hMap); + CloseHandle(hFile); +} + +#define H_CONSOLE 0 +#define H_DISK 1 + +#define ARG_STD 0x80000000 +#define ARG_STARTUPINFO 0x00000000 +#define ARG_CP_INHERIT 0x40000000 +#define ARG_HANDLE_INHERIT 0x20000000 +#define ARG_HANDLE_MASK (~0xff000000) + +static inline BOOL check_run_child(const char *exec, DWORD flags, BOOL cp_inherit, + STARTUPINFOA *si) { -#ifndef _WIN64 - char buffer[2 * MAX_PATH + 25]; - STARTUPINFOA startup; PROCESS_INFORMATION info; + char buffer[2 * MAX_PATH + 64]; + DWORD exit_code; + BOOL res; + DWORD ret; - memset(&startup, 0, sizeof(startup)); - startup.cb = sizeof(startup); - startup.dwFlags = STARTF_USESHOWWINDOW; - startup.wShowWindow = SW_SHOWNORMAL; get_file_name(resfile); - sprintf(buffer, "\"%s\" process dump \"%s\"", selfname, resfile); - ok(CreateProcessA(NULL, buffer, NULL, NULL, TRUE, DETACHED_PROCESS, NULL, NULL, &startup, - &info), "CreateProcess\n"); - wait_and_close_child_process(&info); + sprintf(buffer, "\"%s\" process dump \"%s\"", exec, resfile); - reload_child_info(resfile); - okChildInt("StartupInfoA", "hStdInput", (UINT)INVALID_HANDLE_VALUE); - okChildInt("StartupInfoA", "hStdOutput", (UINT)INVALID_HANDLE_VALUE); - okChildInt("StartupInfoA", "hStdError", (UINT)INVALID_HANDLE_VALUE); - okChildInt("TEB", "hStdInput", 0); - okChildInt("TEB", "hStdOutput", 0); - okChildInt("TEB", "hStdError", 0); - release_memory(); - DeleteFileA(resfile); -#endif + res = CreateProcessA(NULL, buffer, NULL, NULL, cp_inherit, flags, NULL, NULL, si, &info); + ok(res, "CreateProcess failed: %lu %s\n", GetLastError(), buffer); + CloseHandle(info.hThread); + ret = WaitForSingleObject(info.hProcess, 30000); + ok(ret == WAIT_OBJECT_0, "Could not wait for the child process: %ld le=%lu\n", + ret, GetLastError()); + res = GetExitCodeProcess(info.hProcess, &exit_code); + ok(res && exit_code == 0, "Couldn't get exit_code\n"); + CloseHandle(info.hProcess); + return res; +} + +static char std_handle_file[MAX_PATH]; + +static inline BOOL build_startupinfo( STARTUPINFOA *startup, unsigned args, HANDLE hstd[2] ) +{ + SECURITY_ATTRIBUTES inherit_sa = { sizeof(inherit_sa), NULL, TRUE }; + SECURITY_ATTRIBUTES *psa; + BOOL needs_close = FALSE; + + psa = (args & ARG_HANDLE_INHERIT) ? &inherit_sa : NULL; + + memset(startup, 0, sizeof(*startup)); + startup->cb = sizeof(*startup); + + switch (args & ARG_HANDLE_MASK) + { + case H_CONSOLE: + hstd[0] = CreateFileA("CONIN$", GENERIC_READ, 0, psa, OPEN_EXISTING, 0, 0); + ok(hstd[0] != INVALID_HANDLE_VALUE, "Couldn't create input to console\n"); + hstd[1] = CreateFileA("CONOUT$", GENERIC_READ|GENERIC_WRITE, 0, psa, OPEN_EXISTING, 0, 0); + ok(hstd[1] != INVALID_HANDLE_VALUE, "Couldn't create input to console\n"); + needs_close = TRUE; + break; + case H_DISK: + hstd[0] = CreateFileA(std_handle_file, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, psa, OPEN_EXISTING, 0, 0); + ok(hstd[0] != INVALID_HANDLE_VALUE, "Couldn't create input to file %s\n", std_handle_file); + hstd[1] = CreateFileA(std_handle_file, GENERIC_READ|GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, psa, OPEN_EXISTING, 0, 0); + ok(hstd[1] != INVALID_HANDLE_VALUE, "Couldn't create input to file %s\n", std_handle_file); + needs_close = TRUE; + break; + default: + ok(0, "Unsupported handle type %x\n", args & ARG_HANDLE_MASK); + return FALSE; + } + + if (args & ARG_STD) + { + SetStdHandle(STD_INPUT_HANDLE, hstd[0]); + SetStdHandle(STD_OUTPUT_HANDLE, hstd[1]); + } + else /* through startup info */ + { + startup->dwFlags |= STARTF_USESTDHANDLES; + startup->hStdInput = hstd[0]; + startup->hStdOutput = hstd[1]; + } + return needs_close; } -static void test_DetachConsoleHandles(void) +struct std_handle_test +{ + /* input */ + unsigned args; + /* output */ + DWORD expected; + unsigned is_todo; /* bitmask: 1 on TEB values, 2 on StartupInfoA values, 4 on StartupInfoW values */ + DWORD is_broken; /* Win7 broken file types */ +}; + +static void test_StdHandleInheritance(void) { #ifndef _WIN64 - char buffer[2 * MAX_PATH + 25]; - STARTUPINFOA startup; - PROCESS_INFORMATION info; - UINT result; + HANDLE hsavestd[3]; + static char guiexec[MAX_PATH]; + static char cuiexec[MAX_PATH]; + char **argv; + BOOL ret; + int i, j; - memset(&startup, 0, sizeof(startup)); - startup.cb = sizeof(startup); - startup.dwFlags = STARTF_USESHOWWINDOW|STARTF_USESTDHANDLES; - startup.wShowWindow = SW_SHOWNORMAL; - startup.hStdInput = GetStdHandle(STD_INPUT_HANDLE); - startup.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); - startup.hStdError = GetStdHandle(STD_ERROR_HANDLE); - get_file_name(resfile); - sprintf(buffer, "\"%s\" process dump \"%s\"", selfname, resfile); - ok(CreateProcessA(NULL, buffer, NULL, NULL, TRUE, DETACHED_PROCESS, NULL, NULL, &startup, - &info), "CreateProcess\n"); - wait_and_close_child_process(&info); + static const struct std_handle_test + detached_cui[] = + { + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 4}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + /* all others handles type behave as H_DISK */ + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + }, + detached_gui[] = + { + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 4}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + /* all others handles type behave as H_DISK */ + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + }; + static const struct + { + DWORD cp_flags; + BOOL use_cui; + const struct std_handle_test* tests; + size_t count; + const char* descr; + } + tests[] = + { +#define X(d, cg, s) {(d), (cg), s, ARRAY_SIZE(s), #s} + X(DETACHED_PROCESS, TRUE, detached_cui), + X(DETACHED_PROCESS, FALSE, detached_gui), +#undef X + }; - reload_child_info(resfile); - result = GetPrivateProfileIntA("StartupInfoA", "hStdInput", 0, resfile); - ok(result != 0 && result != (UINT)INVALID_HANDLE_VALUE, "bad handle %x\n", result); - result = GetPrivateProfileIntA("StartupInfoA", "hStdOutput", 0, resfile); - ok(result != 0 && result != (UINT)INVALID_HANDLE_VALUE, "bad handle %x\n", result); - result = GetPrivateProfileIntA("StartupInfoA", "hStdError", 0, resfile); - ok(result != 0 && result != (UINT)INVALID_HANDLE_VALUE, "bad handle %x\n", result); - result = GetPrivateProfileIntA("TEB", "hStdInput", 0, resfile); - ok(result != 0 && result != (UINT)INVALID_HANDLE_VALUE, "bad handle %x\n", result); - result = GetPrivateProfileIntA("TEB", "hStdOutput", 0, resfile); - ok(result != 0 && result != (UINT)INVALID_HANDLE_VALUE, "bad handle %x\n", result); - result = GetPrivateProfileIntA("TEB", "hStdError", 0, resfile); - ok(result != 0 && result != (UINT)INVALID_HANDLE_VALUE, "bad handle %x\n", result); + hsavestd[0] = GetStdHandle(STD_INPUT_HANDLE); + hsavestd[1] = GetStdHandle(STD_OUTPUT_HANDLE); + hsavestd[2] = GetStdHandle(STD_ERROR_HANDLE); - release_memory(); - DeleteFileA(resfile); + winetest_get_mainargs(&argv); + + GetTempPathA(ARRAY_SIZE(guiexec), guiexec); + strcat(guiexec, "process_gui.exe"); + copy_change_subsystem(argv[0], guiexec, IMAGE_SUBSYSTEM_WINDOWS_GUI); + GetTempPathA(ARRAY_SIZE(cuiexec), cuiexec); + strcat(cuiexec, "process_cui.exe"); + copy_change_subsystem(argv[0], cuiexec, IMAGE_SUBSYSTEM_WINDOWS_CUI); + get_file_name(std_handle_file); + + for (j = 0; j < ARRAY_SIZE(tests); j++) + { + const struct std_handle_test* std_tests = tests[j].tests; + + for (i = 0; i < tests[j].count; i++) + { + STARTUPINFOA startup; + HANDLE hstd[2] = {}; + BOOL needs_close; + unsigned startup_expected; + + winetest_push_context("%s[%u] ", tests[j].descr, i); + needs_close = build_startupinfo( &startup, std_tests[i].args, hstd ); + + ret = check_run_child(tests[j].use_cui ? cuiexec : guiexec, + tests[j].cp_flags, !!(std_tests[i].args & ARG_CP_INHERIT), + &startup); + ok(ret, "Couldn't run child\n"); + reload_child_info(resfile); + + startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_INVALID : std_tests[i].expected; + + todo_wine_if(std_tests[i].is_todo & 2) + { + okChildHexInt("StartupInfoA", "hStdInputEncode", startup_expected, std_tests[i].is_broken); + okChildHexInt("StartupInfoA", "hStdOutputEncode", startup_expected, std_tests[i].is_broken); + } + + startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_NULL : std_tests[i].expected; + + todo_wine_if(std_tests[i].is_todo & 4) + { + okChildHexInt("StartupInfoW", "hStdInputEncode", startup_expected, std_tests[i].is_broken); + okChildHexInt("StartupInfoW", "hStdOutputEncode", startup_expected, std_tests[i].is_broken); + } + + todo_wine_if(std_tests[i].is_todo & 1) + { + okChildHexInt("TEB", "hStdInputEncode", std_tests[i].expected, std_tests[i].is_broken); + okChildHexInt("TEB", "hStdOutputEncode", std_tests[i].expected, std_tests[i].is_broken); + } + + release_memory(); + DeleteFileA(resfile); + if (needs_close) + { + CloseHandle(hstd[0]); + CloseHandle(hstd[1]); + } + winetest_pop_context(); + } + } + + DeleteFileA(guiexec); + DeleteFileA(cuiexec); + DeleteFileA(std_handle_file); + + SetStdHandle(STD_INPUT_HANDLE, hsavestd[0]); + SetStdHandle(STD_OUTPUT_HANDLE, hsavestd[1]); + SetStdHandle(STD_ERROR_HANDLE, hsavestd[2]); #endif } @@ -3568,59 +3785,6 @@ static void test_SuspendProcessState(void) } #endif -static void test_DetachStdHandles(void) -{ -#ifndef _WIN64 - char buffer[2 * MAX_PATH + 25], tempfile[MAX_PATH]; - STARTUPINFOA startup; - PROCESS_INFORMATION info; - HANDLE hstdin, hstdout, hstderr, htemp; - BOOL res; - - hstdin = GetStdHandle(STD_INPUT_HANDLE); - hstdout = GetStdHandle(STD_OUTPUT_HANDLE); - hstderr = GetStdHandle(STD_ERROR_HANDLE); - - get_file_name(tempfile); - htemp = CreateFileA(tempfile, GENERIC_READ|GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); - ok(htemp != INVALID_HANDLE_VALUE, "failed opening temporary file\n"); - - memset(&startup, 0, sizeof(startup)); - startup.cb = sizeof(startup); - startup.dwFlags = STARTF_USESHOWWINDOW; - startup.wShowWindow = SW_SHOWNORMAL; - get_file_name(resfile); - sprintf(buffer, "\"%s\" process dump \"%s\"", selfname, resfile); - - SetStdHandle(STD_INPUT_HANDLE, htemp); - SetStdHandle(STD_OUTPUT_HANDLE, htemp); - SetStdHandle(STD_ERROR_HANDLE, htemp); - - res = CreateProcessA(NULL, buffer, NULL, NULL, TRUE, DETACHED_PROCESS, NULL, NULL, &startup, - &info); - - SetStdHandle(STD_INPUT_HANDLE, hstdin); - SetStdHandle(STD_OUTPUT_HANDLE, hstdout); - SetStdHandle(STD_ERROR_HANDLE, hstderr); - - ok(res, "CreateProcess failed\n"); - wait_and_close_child_process(&info); - - reload_child_info(resfile); - okChildInt("StartupInfoA", "hStdInput", (UINT)INVALID_HANDLE_VALUE); - okChildInt("StartupInfoA", "hStdOutput", (UINT)INVALID_HANDLE_VALUE); - okChildInt("StartupInfoA", "hStdError", (UINT)INVALID_HANDLE_VALUE); - okChildInt("TEB", "hStdInput", 0); - okChildInt("TEB", "hStdOutput", 0); - okChildInt("TEB", "hStdError", 0); - release_memory(); - DeleteFileA(resfile); - - CloseHandle(htemp); - DeleteFileA(tempfile); -#endif -} - static void test_GetNumaProcessorNode(void) { SYSTEM_INFO si; @@ -5105,9 +5269,7 @@ START_TEST(process) test_ProcessorCount(); test_RegistryQuota(); test_DuplicateHandle(); - test_StartupNoConsole(); - test_DetachConsoleHandles(); - test_DetachStdHandles(); + test_StdHandleInheritance(); test_GetNumaProcessorNode(); test_session_info(); test_GetLogicalProcessorInformationEx(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4441
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 44 ++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 028906660ca..655d2013e31 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3134,6 +3134,8 @@ static inline void copy_change_subsystem(const char* in, const char* out, DWORD #define H_CONSOLE 0 #define H_DISK 1 +#define H_CHAR 2 +#define H_PIPE 3 #define ARG_STD 0x80000000 #define ARG_STARTUPINFO 0x00000000 @@ -3171,7 +3173,7 @@ static inline BOOL build_startupinfo( STARTUPINFOA *startup, unsigned args, HAND { SECURITY_ATTRIBUTES inherit_sa = { sizeof(inherit_sa), NULL, TRUE }; SECURITY_ATTRIBUTES *psa; - BOOL needs_close = FALSE; + BOOL ret, needs_close = FALSE; psa = (args & ARG_HANDLE_INHERIT) ? &inherit_sa : NULL; @@ -3194,6 +3196,18 @@ static inline BOOL build_startupinfo( STARTUPINFOA *startup, unsigned args, HAND ok(hstd[1] != INVALID_HANDLE_VALUE, "Couldn't create input to file %s\n", std_handle_file); needs_close = TRUE; break; + case H_CHAR: + hstd[0] = CreateFileA("NUL", GENERIC_READ, 0, psa, OPEN_EXISTING, 0, 0); + ok(hstd[0] != INVALID_HANDLE_VALUE, "Couldn't create input to NUL\n"); + hstd[1] = CreateFileA("NUL", GENERIC_READ|GENERIC_WRITE, 0, psa, OPEN_EXISTING, 0, 0); + ok(hstd[1] != INVALID_HANDLE_VALUE, "Couldn't create input to NUL\n"); + needs_close = TRUE; + break; + case H_PIPE: + ret = CreatePipe(&hstd[0], &hstd[1], psa, 0); + ok(ret, "Couldn't create anon pipe\n"); + needs_close = TRUE; + break; default: ok(0, "Unsupported handle type %x\n", args & ARG_HANDLE_MASK); return FALSE; @@ -3234,6 +3248,32 @@ static void test_StdHandleInheritance(void) int i, j; static const struct std_handle_test + nothing_cui[] = + { + /* all others handles type behave as H_DISK */ +/* 0*/ {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, + + /* all others handles type behave as H_DISK */ + {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, + }, + nothing_gui[] = + { + /* testing all types because of discrepancies */ +/* 0*/ {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_PIPE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_PIPE}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_PIPE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_PIPE, .is_todo = 6}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CHAR, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, +/* 5*/ {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CHAR, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR, .is_todo = 6}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + + /* all others handles type behave as H_DISK */ + {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 7}, + }, detached_cui[] = { {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 4}, @@ -3261,6 +3301,8 @@ static void test_StdHandleInheritance(void) tests[] = { #define X(d, cg, s) {(d), (cg), s, ARRAY_SIZE(s), #s} + X(0, TRUE, nothing_cui), + X(0, FALSE, nothing_gui), X(DETACHED_PROCESS, TRUE, detached_cui), X(DETACHED_PROCESS, FALSE, detached_gui), #undef X -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4441
From: Eric Pouech <epouech(a)codeweavers.com> Be more strict on the cases where the std handles are passed to child process. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 12 ++++++------ dlls/kernel32/tests/process.c | 6 +++--- dlls/ntdll/unix/env.c | 13 +++++++++---- dlls/ntdll/unix/process.c | 2 +- dlls/ntdll/unix/unix_private.h | 3 ++- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 54d1a80709b..74f8f467a9f 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5014,10 +5014,10 @@ static void test_CreateProcessCUI(void) {FALSE, DETACHED_PROCESS | CREATE_NO_WINDOW, NULL_STD, 0}, /* 5*/ {FALSE, CREATE_NEW_CONSOLE | CREATE_NO_WINDOW, NULL_STD, 0}, - {FALSE, 0, CONSOLE_STD, 0, TRUE}, + {FALSE, 0, CONSOLE_STD, 0}, {FALSE, DETACHED_PROCESS, CONSOLE_STD, 0}, {FALSE, CREATE_NEW_CONSOLE, CONSOLE_STD, 0}, - {FALSE, CREATE_NO_WINDOW, CONSOLE_STD, 0, TRUE}, + {FALSE, CREATE_NO_WINDOW, CONSOLE_STD, 0}, /*10*/ {FALSE, DETACHED_PROCESS | CREATE_NO_WINDOW, CONSOLE_STD, 0}, {FALSE, CREATE_NEW_CONSOLE | CREATE_NO_WINDOW, CONSOLE_STD, 0}, @@ -5070,10 +5070,10 @@ static void test_CreateProcessCUI(void) /* 5 */ {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER}, {TRUE, 0, STARTUPINFO_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_ENABLED_CTRLC}, {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER}, - {FALSE, 0, CONSOLE_STD, TRUE, 0, .is_todo = TRUE}, - {FALSE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, TRUE, CP_GROUP_LEADER, .is_todo = TRUE}, -/* 10 */ {FALSE, 0, CONSOLE_STD, FALSE, CP_ENABLED_CTRLC, .is_todo = TRUE}, - {FALSE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, FALSE, CP_GROUP_LEADER, .is_todo = TRUE}, + {FALSE, 0, CONSOLE_STD, TRUE, 0}, + {FALSE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, +/* 10 */ {FALSE, 0, CONSOLE_STD, FALSE, CP_ENABLED_CTRLC}, + {FALSE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, FALSE, CP_GROUP_LEADER}, {FALSE, 0, STARTUPINFO_STD, TRUE, 0, .is_todo = TRUE}, {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_GROUP_LEADER, .is_todo = TRUE}, {FALSE, 0, STARTUPINFO_STD, FALSE, CP_ENABLED_CTRLC, .is_todo = TRUE}, diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 655d2013e31..ebe9f890737 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3255,7 +3255,7 @@ static void test_StdHandleInheritance(void) {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, /* all others handles type behave as H_DISK */ - {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 6, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, }, nothing_gui[] = @@ -3271,8 +3271,8 @@ static void test_StdHandleInheritance(void) {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, /* all others handles type behave as H_DISK */ - {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, - {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 7}, + {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 6, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, }, detached_cui[] = { diff --git a/dlls/ntdll/unix/env.c b/dlls/ntdll/unix/env.c index 4fd3254b2d2..3cb86da8401 100644 --- a/dlls/ntdll/unix/env.c +++ b/dlls/ntdll/unix/env.c @@ -2146,7 +2146,8 @@ void init_startup_info(void) /*********************************************************************** * create_startup_info */ -void *create_startup_info( const UNICODE_STRING *nt_image, const RTL_USER_PROCESS_PARAMETERS *params, +void *create_startup_info( const UNICODE_STRING *nt_image, ULONG process_flags, + const RTL_USER_PROCESS_PARAMETERS *params, const pe_image_info_t *pe_info, DWORD *info_size ) { startup_info_t *info; @@ -2175,9 +2176,13 @@ void *create_startup_info( const UNICODE_STRING *nt_image, const RTL_USER_PROCES info->console_flags = params->ConsoleFlags; if (pe_info->subsystem == IMAGE_SUBSYSTEM_WINDOWS_CUI) info->console = wine_server_obj_handle( params->ConsoleHandle ); - info->hstdin = wine_server_obj_handle( params->hStdInput ); - info->hstdout = wine_server_obj_handle( params->hStdOutput ); - info->hstderr = wine_server_obj_handle( params->hStdError ); + if ((process_flags & PROCESS_CREATE_FLAGS_INHERIT_HANDLES) || + (pe_info->subsystem == IMAGE_SUBSYSTEM_WINDOWS_CUI && !(params->dwFlags & STARTF_USESTDHANDLES))) + { + info->hstdin = wine_server_obj_handle( params->hStdInput ); + info->hstdout = wine_server_obj_handle( params->hStdOutput ); + info->hstderr = wine_server_obj_handle( params->hStdError ); + } info->x = params->dwX; info->y = params->dwY; info->xsize = params->dwXSize; diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index db0d92a1b36..71eab55920d 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -802,7 +802,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ goto done; } if (!machine) machine = pe_info.machine; - if (!(startup_info = create_startup_info( attr.ObjectName, params, &pe_info, &startup_info_size ))) + if (!(startup_info = create_startup_info( attr.ObjectName, process_flags, params, &pe_info, &startup_info_size ))) goto done; env_size = get_env_size( params, &winedebug ); diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 9aeacc7b823..1362c3a04d8 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -178,7 +178,8 @@ extern struct ldt_copy __wine_ldt_copy; extern void init_environment( int argc, char *argv[], char *envp[] ); extern void init_startup_info(void); -extern void *create_startup_info( const UNICODE_STRING *nt_image, const RTL_USER_PROCESS_PARAMETERS *params, +extern void *create_startup_info( const UNICODE_STRING *nt_image, ULONG process_flags, + const RTL_USER_PROCESS_PARAMETERS *params, const pe_image_info_t *pe_info, DWORD *info_size ); extern char **build_envp( const WCHAR *envW ); extern char *get_alternate_wineloader( WORD machine ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4441
From: Eric Pouech <epouech(a)codeweavers.com> The std handles gotten from GetStartupInfo are only set when process was created with STARTF_USESTDHANDLES flag. And yes, Ansi and Unicode versions reset the std handles to a different value. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/kernel_main.c | 16 ++++++++++++---- dlls/kernel32/tests/process.c | 26 +++++++++++++------------- dlls/kernelbase/process.c | 15 ++++++++++++--- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/dlls/kernel32/kernel_main.c b/dlls/kernel32/kernel_main.c index 7b45db73257..da771df1100 100644 --- a/dlls/kernel32/kernel_main.c +++ b/dlls/kernel32/kernel_main.c @@ -108,10 +108,18 @@ static void copy_startup_info(void) startup_infoA.wShowWindow = rupp->wShowWindow; startup_infoA.cbReserved2 = rupp->RuntimeInfo.MaximumLength; startup_infoA.lpReserved2 = rupp->RuntimeInfo.MaximumLength ? (void*)rupp->RuntimeInfo.Buffer : NULL; - startup_infoA.hStdInput = rupp->hStdInput ? rupp->hStdInput : INVALID_HANDLE_VALUE; - startup_infoA.hStdOutput = rupp->hStdOutput ? rupp->hStdOutput : INVALID_HANDLE_VALUE; - startup_infoA.hStdError = rupp->hStdError ? rupp->hStdError : INVALID_HANDLE_VALUE; - + if (rupp->dwFlags & STARTF_USESTDHANDLES) + { + startup_infoA.hStdInput = rupp->hStdInput; + startup_infoA.hStdOutput = rupp->hStdOutput; + startup_infoA.hStdError = rupp->hStdError; + } + else + { + startup_infoA.hStdInput = INVALID_HANDLE_VALUE; + startup_infoA.hStdOutput = INVALID_HANDLE_VALUE; + startup_infoA.hStdError = INVALID_HANDLE_VALUE; + } RtlReleasePebLock(); } diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index ebe9f890737..6912ab8aef4 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3252,42 +3252,42 @@ static void test_StdHandleInheritance(void) { /* all others handles type behave as H_DISK */ /* 0*/ {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, /* all others handles type behave as H_DISK */ - {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 6, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, - {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, + {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, }, nothing_gui[] = { /* testing all types because of discrepancies */ /* 0*/ {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 6}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_PIPE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_PIPE}, - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_PIPE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_PIPE, .is_todo = 6}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_PIPE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_PIPE}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CHAR, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, -/* 5*/ {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CHAR, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR, .is_todo = 6}, +/* 5*/ {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CHAR, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 1, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, /* all others handles type behave as H_DISK */ - {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 6, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, - {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, + {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL}, }, detached_cui[] = { - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 4}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, /* all others handles type behave as H_DISK */ - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, }, detached_gui[] = { - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 4}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 7, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, /* all others handles type behave as H_DISK */ - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, }; static const struct diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 4807e84594d..860af7e7237 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -1280,9 +1280,18 @@ void init_startup_info( RTL_USER_PROCESS_PARAMETERS *params ) startup_infoW.wShowWindow = params->wShowWindow; startup_infoW.cbReserved2 = params->RuntimeInfo.MaximumLength; startup_infoW.lpReserved2 = params->RuntimeInfo.MaximumLength ? (void *)params->RuntimeInfo.Buffer : NULL; - startup_infoW.hStdInput = params->hStdInput ? params->hStdInput : INVALID_HANDLE_VALUE; - startup_infoW.hStdOutput = params->hStdOutput ? params->hStdOutput : INVALID_HANDLE_VALUE; - startup_infoW.hStdError = params->hStdError ? params->hStdError : INVALID_HANDLE_VALUE; + if (params->dwFlags & STARTF_USESTDHANDLES) + { + startup_infoW.hStdInput = params->hStdInput; + startup_infoW.hStdOutput = params->hStdOutput; + startup_infoW.hStdError = params->hStdError; + } + else + { + startup_infoW.hStdInput = NULL; + startup_infoW.hStdOutput = NULL; + startup_infoW.hStdError = NULL; + } command_lineW = params->CommandLine.Buffer; if (!RtlUnicodeStringToAnsiString( &ansi, ¶ms->CommandLine, TRUE )) command_lineA = ansi.Buffer; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4441
new version pushed; should fix Jacek's comment and win7 testbot failures -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4441#note_54372
Jacek Caban (@jacek) commented about dlls/kernel32/tests/process.c:
}
-static void test_DetachConsoleHandles(void) +struct std_handle_test +{ + /* input */ + unsigned args; + /* output */ + DWORD expected; + unsigned is_todo; /* bitmask: 1 on TEB values, 2 on StartupInfoA values, 4 on StartupInfoW values */ + DWORD is_broken; /* Win7 broken file types */ +}; + +static void test_StdHandleInheritance(void) { #ifndef _WIN64 This causes warnings on 64-bit clang about unused functions:
``` dlls/kernel32/tests/process.c:639:20: warning: unused function 'ok_child_hexint' [-Wunused-function] 639 | static inline void ok_child_hexint( int line, const char *sect, const char *key, UINT expect, UINT is_broken ) | ^~~~~~~~~~~~~~~ dlls/kernel32/tests/process.c:3109:20: warning: unused function 'copy_change_subsystem' [-Wunused-function] 3109 | static inline void copy_change_subsystem(const char* in, const char* out, DWORD subsyst) | ^~~~~~~~~~~~~~~~~~~~~ dlls/kernel32/tests/process.c:3146:20: warning: unused function 'check_run_child' [-Wunused-function] 3146 | static inline BOOL check_run_child(const char *exec, DWORD flags, BOOL cp_inherit, | ^~~~~~~~~~~~~~~ dlls/kernel32/tests/process.c:3172:20: warning: unused function 'build_startupinfo' [-Wunused-function] 3172 | static inline BOOL build_startupinfo( STARTUPINFOA *startup, unsigned args, HANDLE hstd[2] ) | ^~~~~~~~~~~~~~~~~ ``` I'm not sure why those tests are disabled on 64-bit, a quick test suggests that it should work. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4441#note_54393
You no longer change server, so please adjust the commit message. It could also be a bit more descriptive, perhaps something like "ntdll: Respect PROCESS_CREATE_FLAGS_INHERIT_HANDLES and STARTF_USESTDHANDLES flags in create_startup_info." -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4441#note_54394
On Thu Nov 30 17:24:54 2023 +0000, Jacek Caban wrote:
This causes warnings on 64-bit clang about unused functions: ``` dlls/kernel32/tests/process.c:639:20: warning: unused function 'ok_child_hexint' [-Wunused-function] 639 | static inline void ok_child_hexint( int line, const char *sect, const char *key, UINT expect, UINT is_broken ) | ^~~~~~~~~~~~~~~ dlls/kernel32/tests/process.c:3109:20: warning: unused function 'copy_change_subsystem' [-Wunused-function] 3109 | static inline void copy_change_subsystem(const char* in, const char* out, DWORD subsyst) | ^~~~~~~~~~~~~~~~~~~~~ dlls/kernel32/tests/process.c:3146:20: warning: unused function 'check_run_child' [-Wunused-function] 3146 | static inline BOOL check_run_child(const char *exec, DWORD flags, BOOL cp_inherit, | ^~~~~~~~~~~~~~~ dlls/kernel32/tests/process.c:3172:20: warning: unused function 'build_startupinfo' [-Wunused-function] 3172 | static inline BOOL build_startupinfo( STARTUPINFOA *startup, unsigned args, HANDLE hstd[2] ) | ^~~~~~~~~~~~~~~~~ ``` I'm not sure why those tests are disabled on 64-bit, a quick test suggests that it should work. existing (before this MR) 64bit code doesn't compile well (lots of warning for pointer to integer conversion in 64bit)
that maybe the cause ; don't know for sure so I wrapped in first commit these function inside a #ifndef block and enabled 64bit compilation after first commit (in separate patch) so that we don't have to first the warnings above and remove all of it afterwards -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4441#note_54415
will do (potentially amending a bit, it's likely your proposal doesn't fit inside the first line limit). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4441#note_54417
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)