[PATCH 0/3] Fix for BZ52048
https://bugs.winehq.org/show_bug.cgi?id=52048 Basically, when calling CreateProcess with current console inheritance (no CREATE_NEW_CONSOLE nor DETACHED_PROCESS), and that the child is a CUI program, native behaves as if CREATE_NEW_CONSOLE was passed. builtin doesn't... this serie: - adds a test case for this case (which requires to extend makedep to support creation of exe sub-modules) - fixes kernelbase accordingly - but also requires to fix the usage of CreateProcess that will now force the creation of a console + one patch of wow64 redirection (only for CUI executables) + after quickly auditing all CreateProcess calls, I fixed a couple of them. --- Eric Pouech (4): tools/makedep: extend submodule in testdll to support exe as well as dll dlls/kernel32/tests: add tests about CreateProcess on CUI programs with console inheritance programs: don't recreate a console when redirecting a program to wow64 dlls/kernelbase: in CreateProcess, allocate a console for CUI process dlls/kernel32/tests/Makefile.in | 4 ++ dlls/kernel32/tests/console.c | 56 ++++++++++++++++++++++++++++ dlls/kernel32/tests/console_cui.c | 31 +++++++++++++++ dlls/kernel32/tests/console_cui.spec | 0 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 +- tools/makedep.c | 10 +++-- 10 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 dlls/kernel32/tests/console_cui.c create mode 100644 dlls/kernel32/tests/console_cui.spec
Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- tools/makedep.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/makedep.c b/tools/makedep.c index d5b30b606b8..6f94dc4da1d 100644 --- a/tools/makedep.c +++ b/tools/makedep.c @@ -2951,6 +2951,8 @@ static void output_source_spec( struct makefile *make, struct incl_file *source, { struct strarray imports = get_expanded_file_local_var( make, obj, "IMPORTS" ); struct strarray dll_flags = get_expanded_file_local_var( make, obj, "EXTRADLLFLAGS" ); + unsigned is_exe = strarray_exists( &dll_flags, "-mconsole" ) || + strarray_exists( &dll_flags, "-mwindows" ); struct strarray all_libs, dep_libs = empty_strarray; char *dll_name, *obj_name, *output_file; const char *debug_file; @@ -2960,7 +2962,7 @@ static void output_source_spec( struct makefile *make, struct incl_file *source, if (!strarray_exists( &dll_flags, "-nodefaultlibs" )) imports = add_default_imports( make, imports ); all_libs = add_import_libs( make, &dep_libs, imports, 0, make->is_cross ); - dll_name = strmake( "%s.dll%s", obj, make->is_cross ? "" : dll_ext ); + dll_name = strmake( "%s.%s%s", obj, is_exe ? "exe" : "dll", make->is_cross ? "" : dll_ext ); obj_name = strmake( "%s%s", obj_dir_path( make, obj ), make->is_cross ? ".cross.o" : ".o" ); output_file = obj_dir_path( make, dll_name ); @@ -2970,8 +2972,8 @@ static void output_source_spec( struct makefile *make, struct incl_file *source, output_filename( obj_dir_path( make, dll_name )); output_filename( tools_path( make, "wrc" )); output( "\n" ); - output( "\t%secho \"%s.dll TESTDLL \\\"%s\\\"\" | %s -u -o $@\n", cmd_prefix( "WRC" ), obj, output_file, - tools_path( make, "wrc" )); + output( "\t%secho \"%s.%s TESTDLL \\\"%s\\\"\" | %s -u -o $@\n", cmd_prefix( "WRC" ), obj, + is_exe ? "exe" : "dll", output_file, tools_path( make, "wrc" )); output( "%s:", output_file); output_filename( source->filename ); @@ -2983,7 +2985,7 @@ static void output_source_spec( struct makefile *make, struct incl_file *source, output_winegcc_command( make, make->is_cross ); output_filename( "-s" ); output_filenames( dll_flags ); - output_filename( "-shared" ); + if (!is_exe) output_filename( "-shared" ); output_filename( source->filename ); output_filename( obj_name ); if ((debug_file = get_debug_file( make, dll_name )))
adding console_cui as a CUI submodule Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/kernel32/tests/Makefile.in | 4 ++ dlls/kernel32/tests/console.c | 56 ++++++++++++++++++++++++++++++++++ dlls/kernel32/tests/console_cui.c | 31 +++++++++++++++++++ dlls/kernel32/tests/console_cui.spec | 0 4 files changed, 91 insertions(+) create mode 100644 dlls/kernel32/tests/console_cui.c create mode 100644 dlls/kernel32/tests/console_cui.spec diff --git a/dlls/kernel32/tests/Makefile.in b/dlls/kernel32/tests/Makefile.in index db106ab9fc9..772ca0fe7c3 100644 --- a/dlls/kernel32/tests/Makefile.in +++ b/dlls/kernel32/tests/Makefile.in @@ -2,6 +2,8 @@ EXTRADEFS = -DWINE_NO_LONG_TYPES TESTDLL = kernel32.dll IMPORTS = user32 advapi32 +console_cui_EXTRADLLFLAGS = -mconsole -municode + SOURCES = \ actctx.c \ atom.c \ @@ -9,6 +11,8 @@ SOURCES = \ codepage.c \ comm.c \ console.c \ + console_cui.c \ + console_cui.spec \ debugger.c \ directory.c \ drive.c \ diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index e4d9d43f4c4..16cbb234c21 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4647,6 +4647,61 @@ static void test_pseudo_console(void) pClosePseudoConsole(pseudo_console); } +static void extract_resource(const char *name, const char *where) +{ + DWORD written; + HANDLE file; + HRSRC res; + void *ptr; + + trace("foobar %s %s\n", name, where); + file = CreateFileA(where, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); + ok(file != INVALID_HANDLE_VALUE, "file creation failed, at %s, error %u\n", where, GetLastError()); + + res = FindResourceA(NULL, name, "TESTDLL"); + ok( res != 0, "couldn't find resource\n" ); + ptr = LockResource( LoadResource( GetModuleHandleA(NULL), res )); + WriteFile( file, ptr, SizeofResource( GetModuleHandleA(NULL), res ), &written, NULL ); + ok( written == SizeofResource( GetModuleHandleA(NULL), res ), "couldn't write resource\n" ); + CloseHandle( file ); +} + +static void test_CreateProcess_CUI(void) +{ + STARTUPINFOA si = { sizeof(si) }; + PROCESS_INFORMATION info; + char buf[MAX_PATH]; + BOOL res; + DWORD ec = 255; + + FreeConsole(); + + GetTempPathA(ARRAY_SIZE(buf), buf); + strcat(buf, "console_cui.exe"); + extract_resource("console_cui.exe", buf); + + res = CreateProcessA(NULL, buf, NULL, NULL, FALSE, DETACHED_PROCESS, NULL, NULL, &si, &info); + ok(res, "CreateProcess failed: %u %s\n", GetLastError(), buf); + CloseHandle(info.hThread); + + res = WaitForSingleObject(info.hProcess, 30000) == WAIT_OBJECT_0 && + GetExitCodeProcess(info.hProcess, &ec); + ok(res, "Couldn't grab exit code %u\n", GetLastError()); + ok(!res || ec == 1, "Got unexpected error code %u\n", ec); + CloseHandle(info.hProcess); + + res = CreateProcessA(NULL, buf, NULL, NULL, FALSE, 0, NULL, NULL, &si, &info); + ok(res, "CreateProcess failed: %u\n", GetLastError()); + CloseHandle(info.hThread); + + res = WaitForSingleObject(info.hProcess, 30000) == WAIT_OBJECT_0 && + GetExitCodeProcess(info.hProcess, &ec); + res = GetExitCodeProcess(info.hProcess, &ec); + ok(res, "Couldn't grab exit code %u\n", GetLastError()); + todo_wine ok(!res || ec == 0, "Got unexpected error code %u\n", ec); + CloseHandle(info.hProcess); +} + START_TEST(console) { HANDLE hConIn, hConOut, revert_output = NULL, unbound_output; @@ -4865,6 +4920,7 @@ START_TEST(console) test_AttachConsole(hConOut); test_AllocConsole(); test_FreeConsole(); + test_CreateProcess_CUI(); } else if (revert_output) SetConsoleActiveScreenBuffer(revert_output); diff --git a/dlls/kernel32/tests/console_cui.c b/dlls/kernel32/tests/console_cui.c new file mode 100644 index 00000000000..2b6f01d5b67 --- /dev/null +++ b/dlls/kernel32/tests/console_cui.c @@ -0,0 +1,31 @@ +/* + * a CUI application for testing + * + * Copyright 2022 Eric Pouech + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <stdarg.h> + +#include "windef.h" +#include "winbase.h" +#include "wincon.h" + +int WINAPI wWinMain( HINSTANCE inst, HINSTANCE prev, WCHAR *cmdline, INT show ) +{ + /* We assume GetConsoleCP() returns 0 only when not attached to a console */ + return GetConsoleCP() == 0; +} diff --git a/dlls/kernel32/tests/console_cui.spec b/dlls/kernel32/tests/console_cui.spec new file mode 100644 index 00000000000..e69de29bb2d
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=107587 Your paranoid android. === w7u_adm (32 bit report) === kernel32: comm.c:886: Test failed: WaitCommEvent used 1498 ms for waiting === w7u_2qxl (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w7u_adm (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w7u_el (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w8 (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w8adm (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w864 (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064v1507 (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064v1809 (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064 (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064_tsign (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w10pro64 (32 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w864 (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064v1507 (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064v1809 (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064 (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064_2qxl (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w1064_tsign (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w10pro64 (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w10pro64_ar (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w10pro64_he (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w10pro64_ja (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w10pro64_zh_CN (64 bit report) === kernel32: Fatal: test 'console_cui' does not exist. === w7u_2qxl (32 bit report) === kernel32: debugger.c:1027: Test failed: ole32.dll was not reported === w7u_adm (32 bit report) === kernel32: debugger.c:1027: Test failed: ole32.dll was not reported === w7u_el (32 bit report) === kernel32: debugger.c:1027: Test failed: ole32.dll was not reported === w10pro64 (32 bit report) === kernel32: debugger.c:681: Test failed: debugger reported 7 failures === w10pro64_zh_CN (64 bit report) === kernel32: loader.c:720: Test failed: 1219: wrong status c000011b/c0000130 loader.c:720: Test failed: 1224: wrong status c000011b/c000007b loader.c:720: Test failed: 1229: wrong status c000011b/c000007b loader.c:720: Test failed: 1234: wrong status c000011b/c000007b loader.c:720: Test failed: 1239: wrong status c000011b/c000007b loader.c:720: Test failed: 1244: wrong status c000011b/c000007b loader.c:720: Test failed: 1260: wrong status c000011b/0 loader.c:720: Test failed: 1264: wrong status c000011b/0 loader.c:720: Test failed: 1269: wrong status c000011b/0 loader.c:720: Test failed: 1273: wrong status c000011b/0 === w8adm (32 bit report) === kernel32: path: Timeout
Le 10/02/2022 à 19:35, Marvin a écrit :
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?*
failures come from: - either existing ones (like missing oleaut.dll... test doesn't work on non wow setup) - marvin calling tests derived from every file in the tests directory (instead of getting it from testlist.c) A+
Signed-off-by: Eric Pouech <eric.pouech(a)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);
in CreateProcess, if: - parent isn't attached to any 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(a)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 16cbb234c21..c62b78e26c1 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4698,7 +4698,7 @@ static void test_CreateProcess_CUI(void) GetExitCodeProcess(info.hProcess, &ec); res = GetExitCodeProcess(info.hProcess, &ec); ok(res, "Couldn't grab exit code %u\n", GetLastError()); - todo_wine ok(!res || ec == 0, "Got unexpected error code %u\n", ec); + ok(!res || ec == 0, "Got unexpected error code %u\n", ec); CloseHandle(info.hProcess); } diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 12187b92e5c..fe2f79e60fd 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) {
participants (4)
-
Eric Pouech -
Eric Pouech -
eric.pouech＠gmail.com
-
Marvin