[PATCH v2 0/6] MR4573: More CreateProcess work.
This series mainly: - tests and implement fixes for GetStartupInfoW: + it should transform PEB's RTL_USER_PROCESS_PARAMETERS on the fly (instead of caching the results as GetStartupInfoA) + it doesn't set all the fields of returned in STARTUPINFOW (unlike GetStartupInfoA) - no longer allow console and console handles for non CUI apps: + forbid inheritance of console handles for non CUI apps + don't create a unix console for initial (non CUI) app -- v2: ntdll,start: Don't create Unix console for GUI apps. ntdll: Don't inherit std console handles for non CUI child process. kernelbase: GetStartupInfoW: set std handle only when USESTDHANDLES is set. kernelbase: No longer cache GetStartupInfoW() results. kernel32/tests: Add tests for GetStartupInfo(A|W). kernel32/tests: Identify untouched fields returned from GetStartupInfo. https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
From: Eric Pouech <epouech(a)codeweavers.com> GetStartupInfoW() doesn't set all the fields. So in CreateProcess() tests, always use a marker for STARTUP_INFO initialization; make use of that marker to properly identify the std handles gotten from GetStartupInfo. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 43 +++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 8472da0bd70..5e46daac152 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -318,10 +318,13 @@ static void WINAPIV __WINE_PRINTF_ATTR(2,3) childPrintf(HANDLE h, const char* fm } /* 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 */ +#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 */ +#define HATTR_UNTOUCHED 0x20 /* Identify fields untouched by GetStartupInfoW */ + +#define HANDLE_UNTOUCHEDW (HANDLE)(DWORD_PTR)(0x5050505050505050ull) static unsigned encode_handle_attributes(HANDLE h) { @@ -332,6 +335,8 @@ static unsigned encode_handle_attributes(HANDLE h) result = HATTR_NULL; else if (h == INVALID_HANDLE_VALUE) result = HATTR_INVALID; + else if (h == HANDLE_UNTOUCHEDW) + result = HATTR_UNTOUCHED; else { result = HATTR_TYPE; @@ -372,6 +377,7 @@ static void doChild(const char* file, const char* option) if (hFile == INVALID_HANDLE_VALUE) return; /* output of startup info (Ansi) */ + memset(&siA, 0xA0, sizeof(siA)); GetStartupInfoA(&siA); childPrintf(hFile, "[StartupInfoA]\ncb=%08lu\nlpDesktop=%s\nlpTitle=%s\n" @@ -397,10 +403,7 @@ static void doChild(const char* file, const char* option) 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 - */ - memset(&siW, 0, sizeof(siW)); + memset(&siW, 0x50, sizeof(siW)); GetStartupInfoW(&siW); childPrintf(hFile, "[StartupInfoW]\ncb=%08lu\nlpDesktop=%s\nlpTitle=%s\n" @@ -3251,42 +3254,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}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 4}, /* all others handles type behave as H_DISK */ {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}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 4}, }, 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}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 4}, {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}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_PIPE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_PIPE, .is_todo = 4}, {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}, +/* 5*/ {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CHAR, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR, .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}, - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 1, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_todo = 5, .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_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, - {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, }, detached_cui[] = { - {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL}, + {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}, + {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}, + {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}, + {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 @@ -3349,7 +3352,7 @@ static void test_StdHandleInheritance(void) okChildHexInt("StartupInfoA", "hStdOutputEncode", startup_expected, std_tests[i].is_broken); } - startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_NULL : std_tests[i].expected; + startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_UNTOUCHED : std_tests[i].expected; todo_wine_if(std_tests[i].is_todo & 4) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 91 +++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 5e46daac152..dd2aecd227a 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -5197,6 +5197,96 @@ static void test_services_exe(void) ok(services_session_id == 0, "got services.exe SessionId %lu\n", services_session_id); } +static void test_startupinfo( void ) +{ + STARTUPINFOA startup_beforeA, startup_afterA; + STARTUPINFOW startup_beforeW, startup_afterW; + RTL_USER_PROCESS_PARAMETERS *params; + + params = RtlGetCurrentPeb()->ProcessParameters; + + startup_beforeA.hStdInput = (HANDLE)0x56780000; + GetStartupInfoA(&startup_beforeA); + + startup_beforeW.hStdInput = (HANDLE)0x12340000; + GetStartupInfoW(&startup_beforeW); + + /* change a couple of fields in PEB */ + params->dwX = ~params->dwX; + params->hStdInput = (HANDLE)~(DWORD_PTR)params->hStdInput; + + startup_afterA.hStdInput = (HANDLE)0x87650000; + GetStartupInfoA(&startup_afterA); + + /* wharf... ansi version is cached... */ + ok(startup_beforeA.dwX == startup_afterA.dwX, "Unexpected field value\n"); + ok(startup_beforeA.dwFlags == startup_afterA.dwFlags, "Unexpected field value\n"); + ok(startup_beforeA.hStdInput == startup_afterA.hStdInput, "Unexpected field value\n"); + + if (startup_beforeW.dwFlags & STARTF_USESTDHANDLES) + { + ok(startup_beforeA.hStdInput != NULL && startup_beforeA.hStdInput != INVALID_HANDLE_VALUE, + "Unexpected field value\n"); + ok(startup_afterA.hStdInput != NULL && startup_afterA.hStdInput != INVALID_HANDLE_VALUE, + "Unexpected field value\n"); + } + else + { + ok(startup_beforeA.hStdInput == INVALID_HANDLE_VALUE, "Unexpected field value %p\n", startup_beforeA.hStdInput); + ok(startup_afterA.hStdInput == INVALID_HANDLE_VALUE, "Unexpected field value %p\n", startup_afterA.hStdInput); + } + + /* ... while unicode is not */ + startup_afterW.hStdInput = (HANDLE)0x43210000; + GetStartupInfoW(&startup_afterW); + + todo_wine + ok(~startup_beforeW.dwX == startup_afterW.dwX, "Unexpected field value\n"); + if (startup_beforeW.dwFlags & STARTF_USESTDHANDLES) + { + todo_wine + ok(params->hStdInput == startup_afterW.hStdInput, "Unexpected field value\n"); + todo_wine + ok((HANDLE)~(DWORD_PTR)startup_beforeW.hStdInput == startup_afterW.hStdInput, "Unexpected field value\n"); + } + else + { + todo_wine + ok(startup_beforeW.hStdInput == (HANDLE)0x12340000, "Unexpected field value\n"); + todo_wine + ok(startup_afterW.hStdInput == (HANDLE)0x43210000, "Unexpected field value\n"); + } + + /* check impact of STARTF_USESTDHANDLES bit */ + params->dwFlags ^= STARTF_USESTDHANDLES; + + startup_afterW.hStdInput = (HANDLE)0x43210000; + GetStartupInfoW(&startup_afterW); + + todo_wine + ok((startup_beforeW.dwFlags ^ STARTF_USESTDHANDLES) == startup_afterW.dwFlags, "Unexpected field value\n"); + if (startup_afterW.dwFlags & STARTF_USESTDHANDLES) + { + todo_wine + ok(params->hStdInput == startup_afterW.hStdInput, "Unexpected field value\n"); + ok(startup_afterW.hStdInput != (HANDLE)0x43210000, "Unexpected field value\n"); + } + else + { + todo_wine + ok(startup_afterW.hStdInput == (HANDLE)0x43210000, "Unexpected field value\n"); + } + + /* FIXME add more tests to check whether the dwFlags controls the returned + * values (as done for STARTF_USESTDHANDLES) in unicode case. + */ + + /* reset the modified fields in PEB */ + params->dwX = ~params->dwX; + params->hStdInput = (HANDLE)~(DWORD_PTR)params->hStdInput; + params->dwFlags ^= STARTF_USESTDHANDLES; +} + START_TEST(process) { HANDLE job, hproc, h, h2; @@ -5325,6 +5415,7 @@ START_TEST(process) test_handle_list_attribute(FALSE, NULL, NULL); test_dead_process(); test_services_exe(); + test_startupinfo(); /* things that can be tested: * lookup: check the way program to be executed is searched -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 7 ---- dlls/kernelbase/process.c | 64 +++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index dd2aecd227a..70f46451e68 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -5240,20 +5240,15 @@ static void test_startupinfo( void ) startup_afterW.hStdInput = (HANDLE)0x43210000; GetStartupInfoW(&startup_afterW); - todo_wine ok(~startup_beforeW.dwX == startup_afterW.dwX, "Unexpected field value\n"); if (startup_beforeW.dwFlags & STARTF_USESTDHANDLES) { - todo_wine ok(params->hStdInput == startup_afterW.hStdInput, "Unexpected field value\n"); - todo_wine ok((HANDLE)~(DWORD_PTR)startup_beforeW.hStdInput == startup_afterW.hStdInput, "Unexpected field value\n"); } else { - todo_wine ok(startup_beforeW.hStdInput == (HANDLE)0x12340000, "Unexpected field value\n"); - todo_wine ok(startup_afterW.hStdInput == (HANDLE)0x43210000, "Unexpected field value\n"); } @@ -5263,11 +5258,9 @@ static void test_startupinfo( void ) startup_afterW.hStdInput = (HANDLE)0x43210000; GetStartupInfoW(&startup_afterW); - todo_wine ok((startup_beforeW.dwFlags ^ STARTF_USESTDHANDLES) == startup_afterW.dwFlags, "Unexpected field value\n"); if (startup_afterW.dwFlags & STARTF_USESTDHANDLES) { - todo_wine ok(params->hStdInput == startup_afterW.hStdInput, "Unexpected field value\n"); ok(startup_afterW.hStdInput != (HANDLE)0x43210000, "Unexpected field value\n"); } diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index bde50a08c70..1b08cad6401 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -1254,7 +1254,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH TerminateProcess( HANDLE handle, DWORD exit_code ) ***********************************************************************/ -static STARTUPINFOW startup_infoW; static char *command_lineA; static WCHAR *command_lineW; @@ -1265,34 +1264,6 @@ void init_startup_info( RTL_USER_PROCESS_PARAMETERS *params ) { ANSI_STRING ansi; - startup_infoW.cb = sizeof(startup_infoW); - startup_infoW.lpReserved = NULL; - startup_infoW.lpDesktop = params->Desktop.Buffer; - startup_infoW.lpTitle = params->WindowTitle.Buffer; - startup_infoW.dwX = params->dwX; - startup_infoW.dwY = params->dwY; - startup_infoW.dwXSize = params->dwXSize; - startup_infoW.dwYSize = params->dwYSize; - startup_infoW.dwXCountChars = params->dwXCountChars; - startup_infoW.dwYCountChars = params->dwYCountChars; - startup_infoW.dwFillAttribute = params->dwFillAttribute; - startup_infoW.dwFlags = params->dwFlags; - startup_infoW.wShowWindow = params->wShowWindow; - startup_infoW.cbReserved2 = params->RuntimeInfo.MaximumLength; - startup_infoW.lpReserved2 = params->RuntimeInfo.MaximumLength ? (void *)params->RuntimeInfo.Buffer : NULL; - 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; } @@ -1332,7 +1303,40 @@ LPWSTR WINAPI GetCommandLineW(void) */ void WINAPI DECLSPEC_HOTPATCH GetStartupInfoW( STARTUPINFOW *info ) { - *info = startup_infoW; + RTL_USER_PROCESS_PARAMETERS *params; + + RtlAcquirePebLock(); + + params = RtlGetCurrentPeb()->ProcessParameters; + + info->cb = sizeof(*info); + info->lpReserved = NULL; + info->lpDesktop = params->Desktop.Buffer; + info->lpTitle = params->WindowTitle.Buffer; + info->dwX = params->dwX; + info->dwY = params->dwY; + info->dwXSize = params->dwXSize; + info->dwYSize = params->dwYSize; + info->dwXCountChars = params->dwXCountChars; + info->dwYCountChars = params->dwYCountChars; + info->dwFillAttribute = params->dwFillAttribute; + info->dwFlags = params->dwFlags; + info->wShowWindow = params->wShowWindow; + info->cbReserved2 = params->RuntimeInfo.MaximumLength; + info->lpReserved2 = params->RuntimeInfo.MaximumLength ? (void *)params->RuntimeInfo.Buffer : NULL; + if (params->dwFlags & STARTF_USESTDHANDLES) + { + info->hStdInput = params->hStdInput; + info->hStdOutput = params->hStdOutput; + info->hStdError = params->hStdError; + } + else + { + info->hStdInput = NULL; + info->hStdOutput = NULL; + info->hStdError = NULL; + } + RtlReleasePebLock(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 23 +++++++++++------------ dlls/kernelbase/process.c | 6 ------ 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 70f46451e68..37012e48f63 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3254,42 +3254,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 = 4}, + {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_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, - {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK, .is_todo = 4}, + {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 = 4}, + {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 = 4}, + {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 = 4}, +/* 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 = 5, .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_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, - {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL, .is_todo = 4}, + {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 @@ -5266,7 +5266,6 @@ static void test_startupinfo( void ) } else { - todo_wine ok(startup_afterW.hStdInput == (HANDLE)0x43210000, "Unexpected field value\n"); } diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 1b08cad6401..0c5104e7452 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -1330,12 +1330,6 @@ void WINAPI DECLSPEC_HOTPATCH GetStartupInfoW( STARTUPINFOW *info ) info->hStdOutput = params->hStdOutput; info->hStdError = params->hStdError; } - else - { - info->hStdInput = NULL; - info->hStdOutput = NULL; - info->hStdError = NULL; - } RtlReleasePebLock(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 25 ++++++++++--------------- dlls/kernel32/tests/process.c | 6 +++--- dlls/ntdll/unix/env.c | 19 ++++++++++++++++--- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 74f8f467a9f..911c976128f 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4986,7 +4986,6 @@ static void test_CreateProcessCUI(void) DWORD cp_flags; enum inheritance_model inherit; DWORD expected; - BOOL is_todo; DWORD is_broken; } no_console_tests[] = @@ -5021,12 +5020,12 @@ static void test_CreateProcessCUI(void) /*10*/ {FALSE, DETACHED_PROCESS | CREATE_NO_WINDOW, CONSOLE_STD, 0}, {FALSE, CREATE_NEW_CONSOLE | CREATE_NO_WINDOW, CONSOLE_STD, 0}, - {FALSE, 0, STARTUPINFO_STD, 0, TRUE}, - {FALSE, DETACHED_PROCESS, STARTUPINFO_STD, 0, TRUE}, - {FALSE, CREATE_NEW_CONSOLE, STARTUPINFO_STD, 0, TRUE}, -/*15*/ {FALSE, CREATE_NO_WINDOW, STARTUPINFO_STD, 0, TRUE}, - {FALSE, DETACHED_PROCESS | CREATE_NO_WINDOW, STARTUPINFO_STD, 0, TRUE}, - {FALSE, CREATE_NEW_CONSOLE | CREATE_NO_WINDOW, STARTUPINFO_STD, 0, TRUE}, + {FALSE, 0, STARTUPINFO_STD, 0}, + {FALSE, DETACHED_PROCESS, STARTUPINFO_STD, 0}, + {FALSE, CREATE_NEW_CONSOLE, STARTUPINFO_STD, 0}, +/*15*/ {FALSE, CREATE_NO_WINDOW, STARTUPINFO_STD, 0}, + {FALSE, DETACHED_PROCESS | CREATE_NO_WINDOW, STARTUPINFO_STD, 0}, + {FALSE, CREATE_NEW_CONSOLE | CREATE_NO_WINDOW, STARTUPINFO_STD, 0}, {TRUE, 0, NULL_STD, CP_WITH_CONSOLE | CP_WITH_HANDLE | CP_WITH_WINDOW}, {TRUE, DETACHED_PROCESS, NULL_STD, 0}, @@ -5058,7 +5057,6 @@ static void test_CreateProcessCUI(void) BOOL noctrl_flag; /* output */ DWORD expected; - BOOL is_todo; } group_flags_tests[] = { @@ -5074,10 +5072,10 @@ static void test_CreateProcessCUI(void) {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}, -/* 15 */ {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_GROUP_LEADER, .is_todo = TRUE}, + {FALSE, 0, STARTUPINFO_STD, TRUE, 0}, + {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_GROUP_LEADER}, + {FALSE, 0, STARTUPINFO_STD, FALSE, CP_ENABLED_CTRLC}, +/* 15 */ {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_GROUP_LEADER}, {TRUE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER | CP_ALONE}, {FALSE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, {TRUE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER | CP_ALONE | CP_ENABLED_CTRLC}, @@ -5107,7 +5105,6 @@ static void test_CreateProcessCUI(void) res = check_child_console_bits(no_console_tests[i].use_cui ? cuiexec : guiexec, no_console_tests[i].cp_flags, no_console_tests[i].inherit); - todo_wine_if(no_console_tests[i].is_todo) ok(res == no_console_tests[i].expected, "[%d] Unexpected result %x (%lx)\n", i, res, no_console_tests[i].expected); } @@ -5119,7 +5116,6 @@ static void test_CreateProcessCUI(void) res = check_child_console_bits(with_console_tests[i].use_cui ? cuiexec : guiexec, with_console_tests[i].cp_flags, with_console_tests[i].inherit); - todo_wine_if(with_console_tests[i].is_todo) ok(res == with_console_tests[i].expected || broken(with_console_tests[i].is_broken && res == (with_console_tests[i].is_broken & 0xff)), "[%d] Unexpected result %x (%lx)\n", @@ -5135,7 +5131,6 @@ static void test_CreateProcessCUI(void) res = check_child_console_bits(group_flags_tests[i].use_cui ? cuiexec : guiexec, group_flags_tests[i].cp_flags, group_flags_tests[i].inherit); - todo_wine_if(group_flags_tests[i].is_todo) ok(res == group_flags_tests[i].expected || /* Win7 doesn't report group id */ broken(res == (group_flags_tests[i].expected & ~CP_GROUP_LEADER)), diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 37012e48f63..d59074b988c 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3269,8 +3269,8 @@ static void test_StdHandleInheritance(void) {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}, - {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 = 1, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .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_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, @@ -3287,7 +3287,7 @@ static void test_StdHandleInheritance(void) detached_gui[] = { {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}, + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_CONSOLE, HATTR_NULL, .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}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, diff --git a/dlls/ntdll/unix/env.c b/dlls/ntdll/unix/env.c index 3cb86da8401..092cadd9f15 100644 --- a/dlls/ntdll/unix/env.c +++ b/dlls/ntdll/unix/env.c @@ -2143,6 +2143,16 @@ void init_startup_info(void) } +/* helper for create_startup_info */ +static BOOL is_console_handle( HANDLE handle ) +{ + IO_STATUS_BLOCK io; + DWORD mode; + + return NtDeviceIoControlFile( handle, NULL, NULL, NULL, &io, IOCTL_CONDRV_GET_MODE, NULL, 0, + &mode, sizeof(mode) ) == STATUS_SUCCESS; +} + /*********************************************************************** * create_startup_info */ @@ -2179,9 +2189,12 @@ void *create_startup_info( const UNICODE_STRING *nt_image, ULONG process_flags, 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 ); + if (pe_info->subsystem == IMAGE_SUBSYSTEM_WINDOWS_CUI || !is_console_handle( params->hStdInput )) + info->hstdin = wine_server_obj_handle( params->hStdInput ); + if (pe_info->subsystem == IMAGE_SUBSYSTEM_WINDOWS_CUI || !is_console_handle( params->hStdOutput )) + info->hstdout = wine_server_obj_handle( params->hStdOutput ); + if (pe_info->subsystem == IMAGE_SUBSYSTEM_WINDOWS_CUI || !is_console_handle( params->hStdError )) + info->hstderr = wine_server_obj_handle( params->hStdError ); } info->x = params->dwX; info->y = params->dwY; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
From: Eric Pouech <epouech(a)codeweavers.com> Note: GUI apps using std I/O (this is not common) will no longer print on unix console. If such a behavior is needed, for an app started from Unix shell, one can either redirect output to a file, or pipe output: ./wine app | tee /dev/null Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/ntdll/unix/env.c | 5 ++++- programs/start/start.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/env.c b/dlls/ntdll/unix/env.c index 092cadd9f15..d84f5113b45 100644 --- a/dlls/ntdll/unix/env.c +++ b/dlls/ntdll/unix/env.c @@ -1400,6 +1400,9 @@ static void get_initial_console( RTL_USER_PROCESS_PARAMETERS *params ) wine_server_fd_to_handle( 1, GENERIC_WRITE|SYNCHRONIZE, OBJ_INHERIT, ¶ms->hStdOutput ); wine_server_fd_to_handle( 2, GENERIC_WRITE|SYNCHRONIZE, OBJ_INHERIT, ¶ms->hStdError ); + if (main_image_info.SubSystemType != IMAGE_SUBSYSTEM_WINDOWS_CUI) + return; + /* mark tty handles for kernelbase, see init_console */ if (params->hStdInput && isatty(0)) { @@ -1418,7 +1421,7 @@ static void get_initial_console( RTL_USER_PROCESS_PARAMETERS *params ) params->hStdOutput = (HANDLE)((UINT_PTR)params->hStdOutput | 1); output_fd = 1; } - if (!params->ConsoleHandle && main_image_info.SubSystemType == IMAGE_SUBSYSTEM_WINDOWS_CUI) + if (!params->ConsoleHandle) params->ConsoleHandle = CONSOLE_HANDLE_SHELL_NO_WINDOW; if (output_fd != -1) diff --git a/programs/start/start.c b/programs/start/start.c index 59fd72e07a4..c2e5850c291 100644 --- a/programs/start/start.c +++ b/programs/start/start.c @@ -401,6 +401,7 @@ static struct WCHAR *title; DWORD creation_flags; USHORT machine; + BOOL cp_inherit; } opts; static void parse_command_line( int argc, WCHAR *argv[] ) @@ -416,6 +417,7 @@ static void parse_command_line( int argc, WCHAR *argv[] ) /* Dunno what these mean, but it looks like winMe's start uses them */ opts.sei.fMask = SEE_MASK_FLAG_DDEWAIT | SEE_MASK_FLAG_NO_UI; opts.creation_flags = CREATE_NEW_CONSOLE; + opts.cp_inherit = FALSE; /* Canonical Microsoft commandline flag processing: * flags start with / and are case insensitive. @@ -490,6 +492,7 @@ static void parse_command_line( int argc, WCHAR *argv[] ) else if (is_option(argv[i], L"/exec")) { opts.creation_flags = 0; opts.sei.fMask = SEE_MASK_NOCLOSEPROCESS | SEE_MASK_NO_CONSOLE | SEE_MASK_FLAG_NO_UI; + opts.cp_inherit = TRUE; i++; break; } @@ -581,7 +584,7 @@ int __cdecl wmain (int argc, WCHAR *argv[]) si.StartupInfo.dwFlags |= STARTF_USESHOWWINDOW; si.StartupInfo.lpTitle = opts.title; - if (!CreateProcessW( opts.sei.lpFile, commandline, NULL, NULL, FALSE, + if (!CreateProcessW( opts.sei.lpFile, commandline, NULL, NULL, opts.cp_inherit, opts.creation_flags, NULL, opts.sei.lpDirectory, &si.StartupInfo, &process_information )) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
pushed new version. changes: * fixed indentation * fixed start.exe so that GUI can output to redirected unix fd @jacek: note that the last patch as it is, introduces a discrepancy whether start is used or not. when not using start, printing to fd 1 will end up on unix console; when using start, it won't. Not sure it matters that much (as I expect most of the usage being done through start thanks to $PATH search), but worth mentionning. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4573#note_54894
This looks good enough to me. I'm not sure if it's true that start is used in majority cases. PATH search is needed for builtin applications, but for regular ones I'd expect users to pass Unix paths most of the time. Still, it's needed for 32-bit/64-bit mismatch (although not in wow64 configurations). Still, I agree that if we can have mitigations working for more cases, it would be nice. I just don't see a good way of doing that, but I'd be more than happy if we can find a clean way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4573#note_55053
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4573
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)