[PATCH v3 0/1] MR2617: kernel32/tests: Fix test for ACTCTX_FLAG_HMODULE_VALID with hModule = NULL case.
Today, the test scenario "ACTCTX_FLAG_HMODULE_VALID but hModule if not set" is broken and unreliable. This problem is not evident in WineHQ batch test runs; rather, the test failure seems to only be triggered when the kernel32:actctx test is run in isolation. When the flag ACTCTX_FLAG_HMODULE_VALID is specified in ACTCTX but hModule is set to NULL, CreateActCtxW() may encounter different failure modes depending on the environment and/or the test executable file. Error codes observed so far include ERROR_SXS_CANT_GEN_ACTCTX and ERROR_SXS_MANIFEST_TOO_BIG. It appears that the inconsistent failure was caused by Windows trying to interpret the main executable file of the current process as an XML manifest file. This fails for the following reasons: - A valid PE executable that starts with the "MZ" signature is not a valid XML file. - The test executable's size exceeds the limit imposed by the manifest parser. This is much more likely for binaries with debugging symbols. Meanwhile, winetest bundles a stripped version of the test executable (kernel32_test-stripped.exe), which may end up masking the problem. Fix this by changing the FullDllName of the main executable module's LDR_DATA_TABLE_ENTRY to the pathname of a temporary manifest file (valid or invalid) before testing. The testing is performed in a child process, since it deliberately "corrupts" the process state. -- v3: kernel32/tests: Fix test for ACTCTX_FLAG_HMODULE_VALID with hModule = NULL case. https://gitlab.winehq.org/wine/wine/-/merge_requests/2617
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Today, the test scenario "ACTCTX_FLAG_HMODULE_VALID but hModule if not set" is broken and unreliable. This problem is not evident in WineHQ batch test runs; rather, the test failure seems to only be triggered when the kernel32:actctx test is run in isolation. When the flag ACTCTX_FLAG_HMODULE_VALID is specified in ACTCTX but hModule is set to NULL, CreateActCtxW() may encounter different failure modes depending on the environment and/or the test executable file. Error codes observed so far include ERROR_SXS_CANT_GEN_ACTCTX and ERROR_SXS_MANIFEST_TOO_BIG. It appears that the inconsistent failure was caused by Windows trying to interpret the main executable file of the current process as an XML manifest file. This fails for the following reasons: - A valid PE executable that starts with the "MZ" signature is not a valid XML file. - The test executable's size exceeds the limit imposed by the manifest parser. This is much more likely for binaries with debugging symbols. Meanwhile, winetest bundles a stripped version of the test executable (kernel32_test-stripped.exe), which may end up masking the problem. Fix this by changing the FullDllName of the main executable module's LDR_DATA_TABLE_ENTRY to the pathname of a temporary manifest file (valid or invalid) before testing. The testing is performed in a child process, since it deliberately "corrupts" the process state. --- dlls/kernel32/tests/actctx.c | 125 ++++++++++++++++++++++++++++++++--- 1 file changed, 114 insertions(+), 11 deletions(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index da351e70466..c8e549f7429 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -2792,17 +2792,6 @@ todo_wine { delete_manifest_file("testdep1.manifest"); delete_manifest_file("testdep2.manifest"); - /* ACTCTX_FLAG_HMODULE_VALID but hModule is not set */ - memset(&actctx, 0, sizeof(ACTCTXA)); - actctx.cbSize = sizeof(ACTCTXA); - actctx.dwFlags = ACTCTX_FLAG_HMODULE_VALID; - SetLastError(0xdeadbeef); - handle = CreateActCtxA(&actctx); - ok(handle == INVALID_HANDLE_VALUE, "got handle %p\n", handle); - todo_wine - ok(GetLastError() == ERROR_SXS_CANT_GEN_ACTCTX || broken(GetLastError() == ERROR_NOT_ENOUGH_MEMORY) /* XP, win2k3 */, - "got error %ld\n", GetLastError()); - /* create from HMODULE - resource doesn't exist, lpSource is set */ memset(&actctx, 0, sizeof(ACTCTXA)); actctx.cbSize = sizeof(ACTCTXA); @@ -3754,6 +3743,113 @@ static void test_manifest_in_module(void) ReleaseActCtx(handle); } +static LDR_DATA_TABLE_ENTRY *lookup_module_entry(HMODULE dll_base) +{ + LDR_DATA_TABLE_ENTRY *mod; + LIST_ENTRY *head, *entry; + + entry = head = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; + while ((entry = entry->Flink) != head) + { + mod = CONTAINING_RECORD(entry, LDR_DATA_TABLE_ENTRY, InMemoryOrderLinks); + if (mod->DllBase == dll_base) + return mod; + } + + return NULL; +} + +/* ACTCTX_FLAG_HMODULE_VALID but hModule is not set */ +static void test_actctx_flag_hmodule_valid_but_hmodule_not_set(void) +{ + static const struct test_entry { + const char *manifest; + DWORD expect_error; + } tests[] = { + { manifest1, ERROR_SUCCESS }, + { wrong_manifest1, ERROR_SXS_CANT_GEN_ACTCTX }, + }; + UNICODE_STRING *full_exe_name, orig_name; + HMODULE main_module; + size_t i; + + main_module = GetModuleHandleW(NULL); + full_exe_name = &lookup_module_entry(main_module)->FullDllName; + orig_name = *full_exe_name; + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + WCHAR tmp_manifest_pathname[MAX_PATH], tmp_buf[MAX_PATH]; + DWORD err, tmp_buf_len, tmp_manifest_pathname_len; + const struct test_entry *cur_test = &tests[i]; + HANDLE handle, manifest_file; + ACTCTXW actctx; + + winetest_push_context("test #%Iu", i); + + manifest_file = create_temp_manifest_file(cur_test->manifest, tmp_manifest_pathname); + tmp_manifest_pathname_len = wcslen(tmp_manifest_pathname); + full_exe_name->Length = tmp_manifest_pathname_len * sizeof(WCHAR); + full_exe_name->MaximumLength = full_exe_name->Length; + full_exe_name->Buffer = tmp_manifest_pathname; + + tmp_buf_len = GetModuleFileNameW(main_module, tmp_buf, ARRAY_SIZE(tmp_buf)); + ok(tmp_buf_len == tmp_manifest_pathname_len, + "expected %lu, got %lu.\n", tmp_manifest_pathname_len, tmp_buf_len); + ok(memcmp(tmp_manifest_pathname, tmp_buf, tmp_manifest_pathname_len * sizeof(WCHAR)) == 0, + "expected %s, got %s.\n", + wine_dbgstr_wn(tmp_manifest_pathname, tmp_manifest_pathname_len), + wine_dbgstr_wn(tmp_buf, tmp_buf_len)); + + memset(&actctx, 0, sizeof(ACTCTXW)); + actctx.cbSize = sizeof(ACTCTXW); + actctx.dwFlags = ACTCTX_FLAG_HMODULE_VALID; + SetLastError(0xdeadbeef); + handle = CreateActCtxW(&actctx); + err = GetLastError(); + ok(handle != NULL, "got handle %p\n", handle); + ok((handle == INVALID_HANDLE_VALUE) == (err != ERROR_SUCCESS), "got handle %p\n", handle); + todo_wine + ok(err == cur_test->expect_error, "got error %ld\n", err); + + if (manifest_file != INVALID_HANDLE_VALUE) + { + CloseHandle(manifest_file); + } + + winetest_pop_context(); + } + + *full_exe_name = orig_name; +} + +static void run_child_process_overwrite_module_path(void) +{ + char cmdline[MAX_PATH]; + char exe[MAX_PATH]; + char **argv; + PROCESS_INFORMATION pi; + STARTUPINFOA si = { 0 }; + BOOL ret; + + winetest_get_mainargs(&argv); + + if (strstr(argv[0], ".exe")) + sprintf(exe, "%s", argv[0]); + else + sprintf(exe, "%s.exe", argv[0]); + sprintf(cmdline, "\"%s\" %s overwrite_module_path", argv[0], argv[1]); + + si.cb = sizeof(si); + ret = CreateProcessA(exe, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret, "Could not create process: %lu\n", GetLastError()); + + wait_child_process(pi.hProcess); + + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); +} + START_TEST(actctx) { int argc; @@ -3780,6 +3876,12 @@ START_TEST(actctx) return; } + if (argc > 2 && !strcmp(argv[2], "overwrite_module_path")) + { + test_actctx_flag_hmodule_valid_but_hmodule_not_set(); + return; + } + test_manifest_in_module(); test_actctx(); test_create_fail(); @@ -3795,4 +3897,5 @@ START_TEST(actctx) run_child_process_two_dll(3); run_child_process_two_dll(4); run_child_process_two_dll(5); + run_child_process_overwrite_module_path(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2617
v1 -> v3: - Edit subject and description -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2617#note_29396
participants (2)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)