As described in bugzilla ticket, when calling CreateProcess: - with "default" console inheritance (no CREATE_NEW_CONSOLE nor DETACHED_PROCESS flags), - when parent is not attached to a console - and when the child's subsystem is CUI , native behaves as if CREATE_NEW_CONSOLE was passed.
builtin doesn't...
this serie: - adds a test case for showing the CUI/GUI discrepancies - fixes kernelbase accordingly - but also requires to fix all usage of CreateProcess in Wine Code that would now generate the creation of a console + one patch of wow64 redirection (only for CUI executables) + when looking to the other usage of CreateProcess, I only convinced myself to adapt one of them
V1 => V2: - don't mess up with the number of patches in the serie - no longer modify makedep for creating cui vs gui test executable, but copy current *_test.exe executable and adapt its subsystem to the need
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52048
A+ ---
Eric Pouech (3): dlls/kernel32/tests: add some console tests about creating cui vs gui processes programs: don't recreate a console when redirecting a program to wow64 dlls/kernelbase: handle corner case in CreateProcess
dlls/kernel32/tests/console.c | 85 +++++++++++++++++++++++++++++++++++ dlls/kernelbase/process.c | 6 ++- programs/services/services.c | 2 +- programs/uninstaller/main.c | 6 ++- programs/winedbg/winedbg.c | 3 +- programs/wusa/main.c | 3 +- 6 files changed, 99 insertions(+), 6 deletions(-)
adding helper to copy argv[0] into an executable with CUI subsystem
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/kernel32/tests/console.c | 85 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+)
diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 5443a611f80..7ab88957ae0 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4647,6 +4647,85 @@ static void test_pseudo_console(void) pClosePseudoConsole(pseudo_console); }
+/* copy an executable, but changing its subsystem */ +static 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 (%u)\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 (%u)\n", out, GetLastError()); + hMap = CreateFileMappingW(hFile, NULL, PAGE_READWRITE, 0, 0, NULL); + ok(hMap != NULL, "Couldn't create map (%u)\n", GetLastError()); + mapping = MapViewOfFile(hMap, FILE_MAP_ALL_ACCESS, 0, 0, 0); + ok(mapping != NULL, "Couldn't map (%u)\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 (%u)\n", GetLastError()); + CloseHandle(hMap); + CloseHandle(hFile); +} + +static BOOL check_whether_child_attached(const char* exec, DWORD flags) +{ + STARTUPINFOA si = { sizeof(si) }; + PROCESS_INFORMATION info; + char buf[MAX_PATH]; + DWORD exit_code; + BOOL res; + DWORD ret; + + sprintf(buf, ""%s" console check_console", exec); + res = CreateProcessA(NULL, buf, NULL, NULL, FALSE, flags, NULL, NULL, &si, &info); + ok(res, "CreateProcess failed: %u %s\n", GetLastError(), buf); + CloseHandle(info.hThread); + ret = WaitForSingleObject(info.hProcess, 30000); + ok(ret == WAIT_OBJECT_0, "Could not wait for the child process: %d le=%u\n", + (UINT)ret, (UINT)GetLastError()); + ret = GetExitCodeProcess(info.hProcess, &exit_code); + ok(ret && exit_code <= 255, "Couldn't get exit_code\n"); + CloseHandle(info.hProcess); + return exit_code != 0; +} + +static void test_CreateProcessCUI(void) +{ + char guiexec[MAX_PATH]; + char cuiexec[MAX_PATH]; + char **argv; + BOOL res; + + winetest_get_mainargs(&argv); + GetTempPathA(ARRAY_SIZE(guiexec), guiexec); + strcat(guiexec, "console_gui.exe"); + copy_change_subsystem(argv[0], guiexec, IMAGE_SUBSYSTEM_WINDOWS_GUI); + GetTempPathA(ARRAY_SIZE(cuiexec), cuiexec); + strcat(cuiexec, "console_cui.exe"); + copy_change_subsystem(argv[0], cuiexec, IMAGE_SUBSYSTEM_WINDOWS_CUI); + + FreeConsole(); + + res = check_whether_child_attached(guiexec, DETACHED_PROCESS); + ok(!res, "Don't expecting child to be attached to a console\n"); + res = check_whether_child_attached(guiexec, 0); + ok(!res, "Don't expecting child to be attached to a console\n"); + res = check_whether_child_attached(cuiexec, DETACHED_PROCESS); + ok(!res, "Don't expecting child to be attached to a console\n"); + res = check_whether_child_attached(cuiexec, 0); + todo_wine ok(res, "Expecting child to be attached to a console\n"); + + DeleteFileA(guiexec); + DeleteFileA(cuiexec); +} + START_TEST(console) { HANDLE hConIn, hConOut, revert_output = NULL, unbound_output; @@ -4675,6 +4754,11 @@ START_TEST(console) return; }
+ if (argc == 3 && !strcmp(argv[2], "check_console")) + { + ExitProcess(GetConsoleCP() != 0); + } + test_current = argc >= 3 && !strcmp(argv[2], "--current"); using_pseudo_console = argc >= 3 && !strcmp(argv[2], "--pseudo-console");
@@ -4865,6 +4949,7 @@ START_TEST(console) test_AttachConsole(hConOut); test_AllocConsole(); test_FreeConsole(); + test_CreateProcessCUI(); } else if (revert_output) SetConsoleActiveScreenBuffer(revert_output);
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- programs/uninstaller/main.c | 6 ++++-- programs/winedbg/winedbg.c | 3 ++- programs/wusa/main.c | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/programs/uninstaller/main.c b/programs/uninstaller/main.c index 68d0dfe49f9..fa88c92078f 100644 --- a/programs/uninstaller/main.c +++ b/programs/uninstaller/main.c @@ -166,7 +166,8 @@ int __cdecl wmain(int argc, WCHAR *argv[]) wcscat( filename, L"\uninstaller.exe" );
Wow64DisableWow64FsRedirection( &redir ); - if (CreateProcessW( filename, GetCommandLineW(), NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi )) + if (CreateProcessW( filename, GetCommandLineW(), NULL, NULL, FALSE, + GetConsoleCP() == 0 ? DETACHED_PROCESS : 0, NULL, NULL, &si, &pi )) { WINE_TRACE( "restarting %s\n", wine_dbgstr_w(filename) ); WaitForSingleObject( pi.hProcess, INFINITE ); @@ -342,7 +343,8 @@ static void UninstallProgram(void) memset(&si, 0, sizeof(STARTUPINFOW)); si.cb = sizeof(STARTUPINFOW); si.wShowWindow = SW_NORMAL; - res = CreateProcessW(NULL, entries[i].command, NULL, NULL, FALSE, 0, NULL, NULL, &si, &info); + res = CreateProcessW(NULL, entries[i].command, NULL, NULL, FALSE, + GetConsoleCP() == 0 ? DETACHED_PROCESS : 0, NULL, NULL, &si, &info); if (res) { /* wait for the process to exit */ WaitForSingleObject(info.hProcess, INFINITE); diff --git a/programs/winedbg/winedbg.c b/programs/winedbg/winedbg.c index 95318eeb2f1..6462b8e8b50 100644 --- a/programs/winedbg/winedbg.c +++ b/programs/winedbg/winedbg.c @@ -606,7 +606,8 @@ static void restart_if_wow64(void) lstrcatW( filename, L"\winedbg.exe" );
Wow64DisableWow64FsRedirection( &redir ); - if (CreateProcessW( filename, GetCommandLineW(), NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi )) + if (CreateProcessW( filename, GetCommandLineW(), NULL, NULL, FALSE, + GetConsoleCP() == 0 ? DETACHED_PROCESS : 0, NULL, NULL, &si, &pi )) { WINE_TRACE( "restarting %s\n", wine_dbgstr_w(filename) ); SetConsoleCtrlHandler( NULL, TRUE ); /* Ignore ^C */ diff --git a/programs/wusa/main.c b/programs/wusa/main.c index d1bb385cd06..390ac85b12b 100644 --- a/programs/wusa/main.c +++ b/programs/wusa/main.c @@ -1027,7 +1027,8 @@ static void restart_as_x86_64(void) wcscat( filename, L"\wusa.exe" );
Wow64DisableWow64FsRedirection(&redir); - if (CreateProcessW(filename, GetCommandLineW(), NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) + if (CreateProcessW(filename, GetCommandLineW(), NULL, NULL, FALSE, + GetConsoleCP() == 0 ? DETACHED_PROCESS : 0, NULL, NULL, &si, &pi)) { TRACE("Restarting %s\n", wine_dbgstr_w(filename)); WaitForSingleObject(pi.hProcess, INFINITE);
Hi Eric,
On 2/15/22 17:43, Eric Pouech wrote:
programs/uninstaller/main.c | 6 ++++-- programs/winedbg/winedbg.c | 3 ++- programs/wusa/main.c | 3 ++-
wusa should probably use GUI subsystem, which would prevent the problem. I think that uninstaller could do the same.
winedbg is a bit more problematic, since -mconsole seems right here. For auto mode, we can make sure that we launch the right winedbg in the first place. For interactive mode, it feels like it would make sense to create a console if we don't have one available. Maybe even we could make it a more general default by making initial console handling match what your patch 3 does? It would look something like the attached patch (attaching for sake of discussion; not really tested, maybe needs some better heuristics).
Thanks,
Jacek
in CreateProcess, if: - parent isn't attached to a console - CreateProcess's flag isn't set with DETACHED_PROCESS nor CREATE_NEW_CONSOLE - child is a CUI program then a console must be allocated for the child
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52048 Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/kernel32/tests/console.c | 2 +- dlls/kernelbase/process.c | 6 +++++- programs/services/services.c | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 7ab88957ae0..3eada958f34 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4720,7 +4720,7 @@ static void test_CreateProcessCUI(void) res = check_whether_child_attached(cuiexec, DETACHED_PROCESS); ok(!res, "Don't expecting child to be attached to a console\n"); res = check_whether_child_attached(cuiexec, 0); - todo_wine ok(res, "Expecting child to be attached to a console\n"); + ok(res, "Expecting child to be attached to a console\n");
DeleteFileA(guiexec); DeleteFileA(cuiexec); diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 35381f409e9..1cecbce9321 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -191,7 +191,11 @@ static RTL_USER_PROCESS_PARAMETERS *create_process_params( const WCHAR *filename
if (flags & CREATE_NEW_PROCESS_GROUP) params->ConsoleFlags = 1; if (flags & CREATE_NEW_CONSOLE) params->ConsoleHandle = CONSOLE_HANDLE_ALLOC; - else if (!(flags & DETACHED_PROCESS)) params->ConsoleHandle = NtCurrentTeb()->Peb->ProcessParameters->ConsoleHandle; + else if (!(flags & DETACHED_PROCESS)) + { + params->ConsoleHandle = NtCurrentTeb()->Peb->ProcessParameters->ConsoleHandle; + if (!params->ConsoleHandle) params->ConsoleHandle = CONSOLE_HANDLE_ALLOC; + }
if (startup->dwFlags & STARTF_USESTDHANDLES) { diff --git a/programs/services/services.c b/programs/services/services.c index 87ab319367b..2edc02d300c 100644 --- a/programs/services/services.c +++ b/programs/services/services.c @@ -1094,7 +1094,7 @@ found: process->use_count++; service_unlock(service_entry);
- r = CreateProcessW(NULL, path, NULL, NULL, FALSE, CREATE_UNICODE_ENVIRONMENT, environment, NULL, &si, &pi); + r = CreateProcessW(NULL, path, NULL, NULL, FALSE, CREATE_UNICODE_ENVIRONMENT | DETACHED_PROCESS, environment, NULL, &si, &pi); HeapFree(GetProcessHeap(), 0, path); if (!r) {
-
Eric Pouech -
Jacek Caban