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 + a couple of programs moved into the GUI subsystem (to avoid console creation on invocation) + 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
V2 => V3: - removed the hacks in wow64 redirect, and let programs/uninstaller and programs/wusa belong to the GUI subsystem instead - force console creation for wine initial process when a) none of the unix fd are attached to a TTY, b) the initial process is in CUI subsystem
V3 => V4: - included Jacek's comments: + removed unneeded casts + move CreateProcess change in programs/services into a separate patch + I didn't change programs/start as you suggested. Testing on W10 shows that 'start /b cui.exe' from a cmd prompt just reuses the current console.
And Marvin will generate errors for Chinese and Japanese VM in console testing that are independant of this serie.
Signed-off-by: Eric Pouech eric.pouech@gmail.com ---
Eric Pouech (5): dlls/kernel32/tests: add some console tests about creating cui vs gui processes programs/wusa: set subsystem to GUI programs/uninstaller: set subsystem to GUI programs/services: start services detached from console 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/Makefile.in | 2 +- programs/wusa/Makefile.in | 2 +- 5 files changed, 93 insertions(+), 4 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 5f5f0698040..6721a830064 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4662,6 +4662,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", + ret, 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; @@ -4690,6 +4769,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");
@@ -4880,6 +4964,7 @@ START_TEST(console) test_AttachConsole(hConOut); test_AllocConsole(); test_FreeConsole(); + test_CreateProcessCUI(); } else if (revert_output) SetConsoleActiveScreenBuffer(revert_output);
as it's on native
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- programs/wusa/Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/programs/wusa/Makefile.in b/programs/wusa/Makefile.in index 171377bc2f2..c512d3e63db 100644 --- a/programs/wusa/Makefile.in +++ b/programs/wusa/Makefile.in @@ -1,7 +1,7 @@ MODULE = wusa.exe IMPORTS = cabinet shlwapi ole32 oleaut32 advapi32
-EXTRADLLFLAGS = -mconsole -municode +EXTRADLLFLAGS = -mwindows -municode
C_SRCS = \ main.c \
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- programs/uninstaller/Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/programs/uninstaller/Makefile.in b/programs/uninstaller/Makefile.in index f7fef51a51c..5cc35b19dc3 100644 --- a/programs/uninstaller/Makefile.in +++ b/programs/uninstaller/Makefile.in @@ -2,7 +2,7 @@ MODULE = uninstaller.exe IMPORTS = advapi32 DELAYIMPORTS = shlwapi shell32 user32 gdi32 comctl32
-EXTRADLLFLAGS = -mconsole -municode +EXTRADLLFLAGS = -mwindows -municode
C_SRCS = \ main.c
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- programs/services/services.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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) {
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 +++++- 2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 6721a830064..6020da34b9b 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4735,7 +4735,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) {
Hi Eric,
On 2/21/22 08:34, Eric Pouech wrote:
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) {
I'm still a bit concerned about inconsistency this will bring due to how we conditionally use start.exe when Wine is launched by a Unix process. To give you an example, assume that we're not attached to Unix console and gcc.exe is a 32-bit console application accessible by Windows PATH:
# Wine will lunch gcc.exe directly, so it will not bring a new console window; that's fine:
wine path/to/gcc.exe
# start.exe will case a new console window, because gcc needs to be looked up:
wine gcc
# start.exe will cause a new window, because of 32/64 mismatch:
wine64 path/to/gcc.exe
Maybe we could set PEB console handle to some non-zero invalid value when load_start_exe() is used. This would make it consistent, but better ideas are welcomed ;)
Thanks,
Jacek
Le 15/03/2022 à 22:56, Jacek Caban a écrit :
Hi Eric,
On 2/21/22 08:34, Eric Pouech wrote:
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) {
I'm still a bit concerned about inconsistency this will bring due to how we conditionally use start.exe when Wine is launched by a Unix process. To give you an example, assume that we're not attached to Unix console and gcc.exe is a 32-bit console application accessible by Windows PATH:
# Wine will lunch gcc.exe directly, so it will not bring a new console window; that's fine:
wine path/to/gcc.exe
# start.exe will case a new console window, because gcc needs to be looked up:
wine gcc
# start.exe will cause a new window, because of 32/64 mismatch:
wine64 path/to/gcc.exe
Maybe we could set PEB console handle to some non-zero invalid value when load_start_exe() is used. This would make it consistent, but better ideas are welcomed ;)
one simple fix is to add DETACHED_PROCESS to creation_flags in start.exe when parsing /exec command line option and not being attached to a console
A+
On 3/16/22 08:13, Eric Pouech wrote:
Le 15/03/2022 à 22:56, Jacek Caban a écrit :
Hi Eric,
On 2/21/22 08:34, Eric Pouech wrote:
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) {
I'm still a bit concerned about inconsistency this will bring due to how we conditionally use start.exe when Wine is launched by a Unix process. To give you an example, assume that we're not attached to Unix console and gcc.exe is a 32-bit console application accessible by Windows PATH:
# Wine will lunch gcc.exe directly, so it will not bring a new console window; that's fine:
wine path/to/gcc.exe
# start.exe will case a new console window, because gcc needs to be looked up:
wine gcc
# start.exe will cause a new window, because of 32/64 mismatch:
wine64 path/to/gcc.exe
Maybe we could set PEB console handle to some non-zero invalid value when load_start_exe() is used. This would make it consistent, but better ideas are welcomed ;)
one simple fix is to add DETACHED_PROCESS to creation_flags in start.exe when parsing /exec command line option and not being attached to a console
Yes, that sounds good to me.
Thanks,
Jacek
-
Eric Pouech -
Eric Pouech -
Jacek Caban