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
From: Eric Pouech epouech@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@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) {
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@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
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@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(); }
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@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(); }
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@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..1f4bdf2b68d 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;
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@codeweavers.com --- dlls/ntdll/unix/env.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/unix/env.c b/dlls/ntdll/unix/env.c index 1f4bdf2b68d..c0ad3477289 100644 --- a/dlls/ntdll/unix/env.c +++ b/dlls/ntdll/unix/env.c @@ -1396,6 +1396,9 @@ static void get_initial_console( RTL_USER_PROCESS_PARAMETERS *params ) { int output_fd = -1;
+ if (main_image_info.SubSystemType != IMAGE_SUBSYSTEM_WINDOWS_CUI) + return; + wine_server_fd_to_handle( 0, GENERIC_READ|SYNCHRONIZE, OBJ_INHERIT, ¶ms->hStdInput ); 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 ); @@ -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)
@jacek: maybe we should keep std handles bound to fd 0,1,2 in last patch (that would match native cmd behavior when redirecting to a file) and we could have a workaround for MR 4566
Jacek Caban (@jacek) commented about dlls/ntdll/unix/env.c:
}
+/* 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;
Wrong indentation.
Yes, I think that it's worth trying to preserve more of current behavior, changes like that tend get noticed by users.
It's generally tricky to get `get_initial_console` right, when we're launched directly from Unix process, we don't have create process flags, so there is no single 'right' answer. In this case it comes down to a question if we want it to behave like `STARTF_USESTDHANDLES` or not. Making it behave like `STARTF_USESTDHANDLES` would match closer to Unix behavior and would not remove users' ability to do that. Moving CUI check below `wine_server_fd_to_handle` calls would achieve that in `get_initial_console`, but you're right that start.exe would also need a change.
And yes, it could at least partially mitigate !4566. Those would still not be true console handles, so depending on what apps are doing, it may or may not work, but it would at least be a valid state that would match what can be done on Windows - just instead of console handles, app would see an unknown pipe-alike device handle.
under Windows, from cmd, if you do "app > file", then <app> is started with STARTF_USESTDHANDLES and stdout set to file handle to <file>
native cmd doesn't do it for console handles
for example, extrac32 /D prints the list of files in .cab to stdout (our implem matches native)
so under windows: 'extrac32 /D <cab>' will print nothing onto the console, but 'extrac32 /D <cab> > <file>' get out the list written into file. so I'd expect GUI apps should not be "surprised" to get valid handles in std handles (it could be DISK or PIPE types, or even CHAR)
IMO, we need at least to support the 'extrac32 /D <cab> > <file>' from unix shell to mimic cmd's behavior ; so keeping output with redirection to the console (without the unicode translation to proper codepage) is a direct consequence of supporting point above
we have bugs in the case we get wrong handles (it's partly fixed in previous patches in this series)
so, I'll update get_initial_console() to return the unix fd in case of non GUI (with on element I still need to look at: I'm not sure that in the 'app > file' example above the handle to file has the inheritable attribute set)
that leaves way to fix to start open. I've been toying and testing various approaches, but couldn't convince myself of best option:
* when using start, depending if child is CUI or GUI, we have to use a different set of handles (current handles for console in case of CUI, the handles to unix fd for the GUI child) * in RTL_CURRENT_PROCESS_PARAMETERS, we can only pass one set * I've considered: * passing the unix handles in params won't work as kernelbase:init_console will erase them (in start's kernelbase.DllMain) * reopen the unix handles with wine_server_fd_to_handle() in start in case of gui child ; works, but not very clean - it has been frown upon in previous attempts (note that ntdll/unix/process.c already does something similar) * pass as command line options to start the unix handles values (so that start passes them in GUI child; and closes them if CUI child) (works, but needs a bit of rework of unix/env.c to change command line options on the fly) * change the logic of console allocation: * no longer do it in kernelbase.DllMain->init_console * force all CUI initial app to be launched with start * call start with the std handles set the unix handles * add a new magic pid to AttachConsole() (eg -2) to attach to underlying unix console, supposing the current process std handles are bound to the unix fd) (need also to mark tty handles... could use bits in ConsoleFlags) * in start, if cui child, create console with AttachConsole(-2), then create child; if gui child, create child (and then create console to keep attachconsole(parent) still available) * works as well, but it's a bit larger
what do you think?
Unless I'm missing something, if we use handles from PEB and pass them with `STARTF_USESTDHANDLES` in start.exe, we'd get the behavior that you described for extrac32: console handles would be skipped, but other handle types would be preserved.
(This wouldn't preserve console as pipe-alike handles, but maybe that's not too bad; there is always the possibility for users to just redirect the output.)
I can't say I like it, but this would mimic Windows cmd's behavior for not outputting to console for GUIs...
(note: in fact we don't even need the STARTF_ flag in start, just to set the inherit bool parameter in CreateProcess to TRUE)
so we need to tell users to either redirect their output of gui to a file (when they need it) or use | tee /dev/null to output to console
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)