[PATCH v4 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 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 due to one or more of the following reasons: - A valid PE executable that starts with the "MZ" signature is not a valid XML file. - The executable's size may exceed the limit imposed by the manifest parser. This is much more likely for binaries with debugging symbols. Meanwhile, winetest.exe bundles a stripped version of the test executable (kernel32_test-stripped.exe), which is often smaller than the original executable (not stripped). This probably explains why the problem was not visible in batch test runs. 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 "corrupting" the internal state of a main test process is not desirable for achieving deterministic and reliable tests. Blocks !2555. -- v4: 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 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 due to one or more of the following reasons: - A valid PE executable that starts with the "MZ" signature is not a valid XML file. - The executable's size may exceed the limit imposed by the manifest parser. This is much more likely for binaries with debugging symbols. Meanwhile, winetest.exe bundles a stripped version of the test executable (kernel32_test-stripped.exe), which is often smaller than the original executable (not stripped). This probably explains why the problem was not visible in batch test runs. 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 "corrupting" the internal state of a main test process is not desirable for achieving deterministic and reliable tests. --- 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
This merge request was closed by Jinoh Kang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2617
participants (2)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)