[PATCH v8 0/5] MR2555: ntdll: Support ISOLATIONAWARE_MANIFEST_RESOURCE_ID (#18889)
From Microsoft documentation, [Using Side-by-Side Assemblies as a Resource][sxs-rsrc] outlines the significance of each reserved manifest resource ID.
Further investigation reveals the following: 1. `CREATEPROCESS_MANIFEST_RESOURCE_ID` (value: `1`): This is used for process-wide initial (`NULL`) activation context. - **Usage documentation**: [Enabling an Assembly in an Application Without Extensions](https://learn.microsoft.com/en-us/windows/win32/sbscs/enabling-an-assembly-i...) - **Intended container module type**: EXE only. - **Lifetime of the associated activation context**: Alive until the process is terminated. Owned by system (NTDLL). - **Isolation-aware**: No. 2. `ISOLATIONAWARE_MANIFEST_RESOURCE_ID` (value: `2`): This is used to specify the assembly manifest that the loader (Ldr) uses to resolve static imports. Also, this is used by isolation-aware wrappers defined in `*.inl` files included in the Windows SDK (`include\um`) when the `ISOLATION_AWARE_ENABLED` macro is defined as a nonzero value. - **Usage documentation**: [Enabling an Assembly in an Application Hosting a DLL, Extension, or Control Panel](https://learn.microsoft.com/en-us/windows/win32/sbscs/enabling-an-assembly-i...) - **Intended container module type**: EXE and DLL. - **Lifetime of the associated activation context**: Alive until the module is unloaded (e.g., via `FreeLibrary`). Owned by system (NTDLL). - **Isolation-aware**: Yes. 3. `ISOLATIONAWARE_NOSTATICIMPORT_MANIFEST_RESOURCE_ID` (value: `3`): Ignored by the loader when resolving static imports. This is used by isolation-aware API wrappers defined in `*.inl` files included in the Windows SDK (`include\um`) when the `ISOLATION_AWARE_ENABLED` macro is defined as a nonzero value. - **Usage documentation**: [IsolationAwareCleanup function][IsolationAwareCleanup] - **Intended container module type**: EXE and DLL. - **Lifetime of the associated activation context**: Alive until the application calls [`IsolationAwareCleanup`][IsolationAwareCleanup][^ia-cleanup-note]. Owned by the isolation-aware API wrapper library (part of the Windows SDK, rather than being a system DLL). - **Isolation-aware**: Yes. The `ISOLATION_AWARE_ENABLED` macro is documented in [Isolating Components][sxs-ic] as well as [Specifying a Default Activation Context][sxs-def-actctx]. When `ISOLATION_AWARE_ENABLED` macro is defined as a nonzero value, then the isolation-aware API wrapper library is enabled. The isolation-aware API wrapper library is responsible for: - Creating (lazily) and destroying activation context (in `IsolationAwareCleanup`), if the manifest resource ID is `ISOLATIONAWARE_NOSTATICIMPORT_MANIFEST_RESOURCE_ID`.[^actctx-lifetime-remark] - Note: the isolation-aware API wrapper library does not manage the activation context's lifetime and delegates the responsibility to the system loader if the manifest resource ID is `ISOLATIONAWARE_MANIFEST_RESOURCE_ID`. - Intercepting Win32 API calls for automatic activation context activation. Each API wrapper activates the "isolation-aware" activation context (obtaining one if it did not exist) before calling the original procedure, and deactivates it before returning. The isolation-aware wrappers may be either be defined inline, or compiled into static libraries that are linked into the final application executable.
From the information above as well as the tests included in this merge request, we can infer the following:
1. Although side-by-side awareness is a cross-cutting concern, the *activation* of activation contexts itself is *not* usually performed automatically by the system for the application (with the sole exception of process-wide activation context associated with `CREATEPROCESS_MANIFEST_RESOURCE_ID`, and static import resolution). 2. `ISOLATIONAWARE_NOSTATICIMPORT_MANIFEST_RESOURCE_ID` is treated like any other resource ID (from `0x0004` to `0xffff`) as far as the loader is concerned. 3. The resource ID of the manifest only concerns how the module containing the manifest intends to use the manifest. In fact, for DLLs that do not *statically* import symbols from side-by-side assemblies, the three resource IDs above are basically equivalent. A DLL can elect not to use manifest resources at all, and manage activation context and library loading by itself. This is why I believe that this merge request is sufficient in completing the isolation-aware component support in Wine; the heavy lifting is done in the application side, not the system. **EDIT**: Fix a few typos and unclear wording. --- [^ia-cleanup-note]: According to the documentation, the application is presumably responsible for calling `IsolationAwareCleanup` from `DllMain` on `fdwReason = DLL_PROCESS_DETACH`. [^actctx-lifetime-remark]: [IsolationAwareCleanup § Remarks](https://learn.microsoft.com/en-us/previous-versions/windows/desktop/legacy/a...), from Microsoft documentation. [sxs-rsrc]: https://learn.microsoft.com/en-us/windows/win32/sbscs/using-side-by-side-ass... [IsolationAwareCleanup]: https://learn.microsoft.com/en-us/previous-versions/windows/desktop/legacy/a...) [sxs-ic]: https://learn.microsoft.com/en-us/windows/win32/sbscs/isolating-components [sxs-def-actctx]: https://learn.microsoft.com/en-us/windows/win32/sbscs/specifying-a-default-a... Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=18889 -- v8: ntdll: Don't hard-code DLL manifest resource ID when looking up dependency assembly. kernel32/tests: Test loading assembly manifest resource inside dependencies. ntdll: Move ACTCTX lpResourceName validation to RtlCreateActivationContext. kernel32/tests: Test setting lpResourceName to NULL for CreateActCtxW. kernel32/tests: Remove test for ACTCTX_FLAG_HMODULE_VALID with hModule = NULL case. https://gitlab.winehq.org/wine/wine/-/merge_requests/2555
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 ACTCTXW 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. When the aforementioned test reports ERROR_SXS_CANT_GEN_ACTCTX on Windows, an event is recorded in the Windows Event Log as follows: - Log Name: Application - Source: SideBySide - Event ID: 59 - Level: Error - Description: Activation context generation failed for "<path..>\kernel32_test.exe".Error in manifest or policy file "<path..>\kernel32_test.exe" on line 0. Invalid Xml syntax. 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: - ERROR_SXS_CANT_GEN_ACTCTX: A valid PE executable that starts with the "MZ" signature is not a valid XML file. - ERROR_SXS_MANIFEST_TOO_BIG (or ERROR_NOT_ENOUGH_MEMORY): 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 removing the failing test entirely. --- dlls/kernel32/tests/actctx.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index 2225215be8a..d64bba5e87d 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -2828,17 +2828,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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2555
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/kernel32/tests/actctx.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index d64bba5e87d..98cb346fded 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -3779,6 +3779,59 @@ static void test_manifest_in_module(void) ReleaseActCtx(handle); } +static void test_manifest_resource_name_omitted(void) +{ + WCHAR pathbuf[MAX_PATH]; + HANDLE handle; + ACTCTXW ctx; + DWORD err, len; + + memset(&ctx, 0, sizeof(ctx)); + ctx.cbSize = sizeof(ctx); + ctx.dwFlags = ACTCTX_FLAG_HMODULE_VALID; + ctx.hModule = GetModuleHandleW(NULL); + handle = CreateActCtxW(&ctx); + err = GetLastError(); + ok(handle == INVALID_HANDLE_VALUE, "CreateActCtxW shall fail\n"); + todo_wine + ok(err == ERROR_RESOURCE_TYPE_NOT_FOUND, "got %lu\n", err); + + memset(&ctx, 0, sizeof(ctx)); + ctx.cbSize = sizeof(ctx); + ctx.dwFlags = ACTCTX_FLAG_HMODULE_VALID | ACTCTX_FLAG_RESOURCE_NAME_VALID; + ctx.hModule = GetModuleHandleW(NULL); + ctx.lpResourceName = NULL; + handle = CreateActCtxW(&ctx); + err = GetLastError(); + ok(handle == INVALID_HANDLE_VALUE, "CreateActCtxW shall fail\n"); + todo_wine + ok(err == ERROR_INVALID_PARAMETER, "got %lu\n", err); + + len = GetModuleFileNameW(NULL, pathbuf, ARRAY_SIZE(pathbuf)); + ok(len > 0 && len < ARRAY_SIZE(pathbuf), "GetModuleFileNameW returned error %lu\n", GetLastError()); + + memset(&ctx, 0, sizeof(ctx)); + ctx.cbSize = sizeof(ctx); + ctx.lpSource = pathbuf; + ctx.dwFlags = 0; + handle = CreateActCtxW(&ctx); + err = GetLastError(); + ok(handle == INVALID_HANDLE_VALUE, "CreateActCtxW shall fail\n"); + todo_wine + ok(err == ERROR_RESOURCE_TYPE_NOT_FOUND, "got %lu\n", err); + + memset(&ctx, 0, sizeof(ctx)); + ctx.cbSize = sizeof(ctx); + ctx.lpSource = pathbuf; + ctx.dwFlags = ACTCTX_FLAG_RESOURCE_NAME_VALID; + ctx.lpResourceName = NULL; + handle = CreateActCtxW(&ctx); + err = GetLastError(); + ok(handle == INVALID_HANDLE_VALUE, "CreateActCtxW shall fail\n"); + todo_wine + ok(err == ERROR_INVALID_PARAMETER, "got %lu\n", err); +} + START_TEST(actctx) { int argc; @@ -3806,6 +3859,7 @@ START_TEST(actctx) } test_manifest_in_module(); + test_manifest_resource_name_omitted(); test_actctx(); test_create_fail(); test_CreateActCtx(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2555
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This prevents passing NULL resource name to get_manifest_in_module(). --- dlls/kernel32/tests/actctx.c | 2 -- dlls/ntdll/actctx.c | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index 98cb346fded..040003c2430 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -3804,7 +3804,6 @@ static void test_manifest_resource_name_omitted(void) handle = CreateActCtxW(&ctx); err = GetLastError(); ok(handle == INVALID_HANDLE_VALUE, "CreateActCtxW shall fail\n"); - todo_wine ok(err == ERROR_INVALID_PARAMETER, "got %lu\n", err); len = GetModuleFileNameW(NULL, pathbuf, ARRAY_SIZE(pathbuf)); @@ -3828,7 +3827,6 @@ static void test_manifest_resource_name_omitted(void) handle = CreateActCtxW(&ctx); err = GetLastError(); ok(handle == INVALID_HANDLE_VALUE, "CreateActCtxW shall fail\n"); - todo_wine ok(err == ERROR_INVALID_PARAMETER, "got %lu\n", err); } diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index 54167a2db67..a2e159c76ae 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -2951,8 +2951,6 @@ static NTSTATUS get_manifest_in_module( struct actctx_loader* acl, struct assemb hModule, debugstr_w(filename) ); } - if (!resname) return STATUS_INVALID_PARAMETER; - info.Type = RT_MANIFEST; info.Language = lang; if (!((ULONG_PTR)resname >> 16)) @@ -5232,6 +5230,9 @@ NTSTATUS WINAPI RtlCreateActivationContext( HANDLE *handle, const void *ptr ) (pActCtx->dwFlags & ~ACTCTX_FLAGS_ALL)) return STATUS_INVALID_PARAMETER; + if ((pActCtx->dwFlags & ACTCTX_FLAG_RESOURCE_NAME_VALID) && !pActCtx->lpResourceName) + return STATUS_INVALID_PARAMETER; + if (!(actctx = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*actctx) ))) return STATUS_NO_MEMORY; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2555
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/kernel32/tests/actctx.c | 420 +++++++++++++++++++++++++++++++++++ 1 file changed, 420 insertions(+) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index 040003c2430..0b249e82b0e 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -3704,6 +3704,424 @@ cleanup: } } +struct manifest_res_spec +{ + const char *name; + LANGID lang; + const char *override_manifest; +}; + +struct manifest_res_spec_list +{ + size_t count; + const struct manifest_res_spec *items; +}; + +static void add_sxs_dll_manifest(const char *pathname, + const struct manifest_res_spec_list *res_lists, + size_t num_lists, + const char *manifest) +{ + HANDLE update_h; + BOOL ret; + size_t i, j; + + update_h = BeginUpdateResourceA(pathname, FALSE); + ok(update_h != NULL, "BeginUpdateResourceA returned error %lu.\n", GetLastError()); + + for (i = 0; i < num_lists; i++) + { + for (j = 0; j < res_lists[i].count; j++) + { + const struct manifest_res_spec *res_spec = &res_lists[i].items[j]; + const char *cur_manifest = res_spec->override_manifest ? res_spec->override_manifest : manifest; + ret = UpdateResourceA(update_h, + MAKEINTRESOURCEA(RT_MANIFEST), + res_spec->name, + res_spec->lang, + (void *)cur_manifest, + strlen(cur_manifest)); + ok(ret, "UpdateResourceA returned error %lu.\n", GetLastError()); + } + } + + ret = EndUpdateResourceA(update_h, FALSE); + ok(ret, "EndUpdateResourceA returned error %lu.\n", GetLastError()); +} + +struct multiple_manifest_test +{ + struct manifest_res_spec manifest_inline; /* optional */ + const struct manifest_res_spec *manifests; /* optional */ + DWORD expected_error; + BOOL is_todo_wine; +}; + +#define subtest_manifest_res(d,e,t,l) subtest_manifest_res_(__LINE__,d,e,t,l) +static DWORD subtest_manifest_res_(int line, const char *manifest_exe, const char *manifest_dll, + const struct multiple_manifest_test *test_data, LANGID lang) +{ + char path_tmp[MAX_PATH] = "", path_dll[MAX_PATH] = "", path_manifest_exe[MAX_PATH] = ""; + struct manifest_res_spec_list res_lists[2]; + static const char path_tmp_suffix[] = "winek32t\\"; + WCHAR locale_name[LOCALE_NAME_MAX_LENGTH] = {0}; + char path_tmp_lang[MAX_PATH] = ""; + DWORD err, prefix_len; + ACTCTXA actctx; + HANDLE handle; + BOOL ret; + int r; + + prefix_len = GetTempPathA(MAX_PATH - ARRAY_SIZE(path_tmp_suffix), path_tmp); + ok_(__FILE__, line)(prefix_len > 0, "GetTempPathA returned error %lu.\n", GetLastError()); + + memcpy(&path_tmp[prefix_len], path_tmp_suffix, sizeof(path_tmp_suffix) - sizeof(*path_tmp_suffix)); + ret = CreateDirectoryA(path_tmp, NULL); + ok_(__FILE__, line)(ret || GetLastError() == ERROR_ALREADY_EXISTS, + "CreateDirectoryA returned error %lu.\n", GetLastError()); + + if (lang) + { + r = LCIDToLocaleName(MAKELCID(lang, SORT_DEFAULT), + locale_name, ARRAY_SIZE(locale_name), LOCALE_ALLOW_NEUTRAL_NAMES); + ok(r > 0, "lang 0x%04x, error %lu.\n", lang, GetLastError()); + } + + if (locale_name[0]) + { + r = snprintf(path_tmp_lang, ARRAY_SIZE(path_tmp_lang), "%s%ls\\", path_tmp, locale_name); + ok_(__FILE__, line)(r > 0 && r < ARRAY_SIZE(path_tmp_lang), "got %d\n", r); + + ret = CreateDirectoryA(path_tmp_lang, NULL); + ok_(__FILE__, line)(ret || GetLastError() == ERROR_ALREADY_EXISTS, + "CreateDirectoryA returned error %lu.\n", GetLastError()); + } + else + { + r = snprintf(path_tmp_lang, ARRAY_SIZE(path_tmp_lang), "%s", path_tmp); + ok_(__FILE__, line)(r > 0 && r < ARRAY_SIZE(path_tmp_lang), "got %d\n", r); + } + + r = snprintf(path_dll, ARRAY_SIZE(path_dll), "%s%s", path_tmp_lang, "sxs_dll.dll"); + ok_(__FILE__, line)(r > 0 && r < ARRAY_SIZE(path_dll), "got %d\n", r); + + r = snprintf(path_manifest_exe, ARRAY_SIZE(path_manifest_exe), "%s%s", path_tmp, "exe.manifest"); + ok_(__FILE__, line)(r > 0 && r < ARRAY_SIZE(path_manifest_exe), "got %d\n", r); + create_manifest_file(path_manifest_exe, manifest_exe, -1, NULL, NULL); + + extract_resource("dummy.dll", "TESTDLL", path_dll); + + res_lists[0].count = test_data->manifest_inline.name ? 1 : 0; + res_lists[0].items = &test_data->manifest_inline; + + if (test_data->manifests) + { + size_t n = 0; + + while (test_data->manifests[n].name) + n++; + + res_lists[1].count = n; + res_lists[1].items = test_data->manifests; + } + else + { + res_lists[1].count = 0; + res_lists[1].items = NULL; + } + + add_sxs_dll_manifest(path_dll, res_lists, ARRAY_SIZE(res_lists), manifest_dll); + + memset(&actctx, 0, sizeof(actctx)); + actctx.cbSize = sizeof(actctx); + actctx.lpSource = path_manifest_exe; + actctx.lpAssemblyDirectory = path_tmp; + actctx.dwFlags = ACTCTX_FLAG_ASSEMBLY_DIRECTORY_VALID; + + SetLastError(0xccccccccUL); + handle = CreateActCtxA(&actctx); + if (handle == INVALID_HANDLE_VALUE) + { + err = GetLastError(); + ok_(__FILE__, line)(err != ERROR_SUCCESS, "got %#lx.\n", err); + } + else + { + err = ERROR_SUCCESS; + ok_(__FILE__, line)(handle != NULL, "CreateActCtxA returned %p (error %lu)\n", handle, err); + ReleaseActCtx(handle); + } + todo_wine_if(test_data->is_todo_wine) + ok_(__FILE__, line)(err == test_data->expected_error, + "expected error %lu, got %lu\n", test_data->expected_error, err); + + ret = DeleteFileA(path_manifest_exe); + ok_(__FILE__, line)(ret, "DeleteFileA(%s) returned error %lu\n.", + debugstr_a(path_manifest_exe), GetLastError()); + + ret = DeleteFileA(path_dll); + ok_(__FILE__, line)(ret, "DeleteFileA(%s) returned error %lu\n.", debugstr_a(path_dll), GetLastError()); + + if (locale_name[0]) + { + ret = RemoveDirectoryA(path_tmp_lang); + ok_(__FILE__, line)(ret, "RemoveDirectoryA(%s) returned error %lu\n.", + debugstr_a(path_tmp_lang), GetLastError()); + } + + ret = RemoveDirectoryA(path_tmp); + ok_(__FILE__, line)(ret, "RemoveDirectoryA(%s) returned error %lu %s\n.", + debugstr_a(path_tmp), GetLastError(), debugstr_a(path_tmp_lang)); + + return err; +} + +/* Test loading DLL with dependency assembly in manifest resource */ +static void test_manifest_resources(void) +{ + static const struct manifest_res_spec wrong_manifest_resources_numbered[] = { + { (char *)2, MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT), wrong_manifest1 }, + { (char *)2, MAKELANGID(LANG_FRENCH,SUBLANG_DEFAULT), wrong_manifest1 }, + { (char *)3, 0, wrong_manifest1 }, + { (char *)0x1234, 0, wrong_manifest1 }, + { NULL }, + }; + static const struct manifest_res_spec correct_multiple_resources_numbered[] = { + { (char *)2, MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT) }, + { (char *)2, MAKELANGID(LANG_FRENCH,SUBLANG_DEFAULT) }, + { (char *)3, 0 }, + { (char *)0x1234, 0 }, + { NULL }, + }; + static const struct manifest_res_spec wrong_manifest_resources_gte_3[] = { + { (char *)3, 0, wrong_manifest1 }, + { (char *)3, MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT), wrong_manifest1 }, + { NULL }, + }; + static const struct manifest_res_spec correct_manifest_resources_gte_3[] = { + { (char *)3, 0 }, + { (char *)3, MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT) }, + { NULL }, + }; + static const struct manifest_res_spec wrong_manifest_resources_named[] = { + { "foo", 0, wrong_manifest1 }, + { "bar", MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT), wrong_manifest1 }, + { NULL }, + }; + static const struct manifest_res_spec correct_manifest_resources_named[] = { + { "foo", 0 }, + { "bar", MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT) }, + { NULL }, + }; + struct multiple_manifest_test tests[] = { + /* Test well-known manifest resource IDs */ + { { (char *)CREATEPROCESS_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)ISOLATIONAWARE_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)ISOLATIONAWARE_NOSTATICIMPORT_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, TRUE }, + + /* Test remaining reserved manifest resource IDs */ + { { (char *)4 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)5 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)6 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)7 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)8 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)9 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)10 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)11 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)12 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)13 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)14 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)15 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)MAXIMUM_RESERVED_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, TRUE }, + + /* Test arbitrary resource IDs */ + { { (char *)0x1234 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)0x89ab }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)0xffff }, NULL, ERROR_SUCCESS, TRUE }, + + /* Test arbitrary LANGID */ + { { (char *)2, MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT) }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)2, MAKELANGID(LANG_FRENCH,SUBLANG_DEFAULT) }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)2, 0x1234 }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)2, 0xffff }, NULL, ERROR_SUCCESS, TRUE }, + + /* Test multiple manifest resources coexisting inside a module */ + { { (char *)2, 0 }, wrong_manifest_resources_numbered, ERROR_SUCCESS, TRUE }, + { { (char *)2, 0, wrong_manifest1 }, correct_multiple_resources_numbered, + ERROR_SXS_CANT_GEN_ACTCTX, FALSE }, + + /* Test that smaller resource ID takes precedence regardless of language ID */ + { { (char *)2, MAKELANGID(LANG_INVARIANT,SUBLANG_NEUTRAL) }, + wrong_manifest_resources_gte_3, ERROR_SUCCESS, TRUE }, + { { (char *)2, MAKELANGID(LANG_INVARIANT,SUBLANG_NEUTRAL), wrong_manifest1 }, + correct_manifest_resources_gte_3, ERROR_SXS_CANT_GEN_ACTCTX, FALSE }, + + /* Test multiple manifest resources (ID / name) coexisting inside a module */ + { { (char *)2, 0 }, wrong_manifest_resources_named, ERROR_SUCCESS, TRUE }, + { { (char *)2, 0, wrong_manifest1 }, + correct_manifest_resources_named, ERROR_SXS_CANT_GEN_ACTCTX, FALSE }, + + /* Test name-only RT_MANIFEST resources */ + { { NULL }, correct_manifest_resources_named, ERROR_SXS_CANT_GEN_ACTCTX, FALSE }, + }; + size_t i; + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + winetest_push_context("tests[%Iu] (%p 0x%04x)", i, + tests[i].manifest_inline.name, tests[i].manifest_inline.lang); + subtest_manifest_res(two_dll_manifest_exe, two_dll_manifest_dll, &tests[i], 0); + winetest_pop_context(); + } +} + +#define LANGID_PREC_MAX_COUNT 5 + +static void get_langid_precedence(LANGID *langs_arr, size_t *lang_count) +{ + LANGID src_langs[LANGID_PREC_MAX_COUNT]; + LANGID user_ui_lang; + size_t i, j, n = 0; + + user_ui_lang = GetUserDefaultUILanguage(); + + src_langs[0] = MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL); + src_langs[1] = user_ui_lang; + src_langs[2] = MAKELANGID(PRIMARYLANGID(user_ui_lang), SUBLANG_NEUTRAL); + src_langs[3] = MAKELANGID(LANG_ENGLISH, SUBLANG_DEFAULT); + src_langs[4] = 0x1; /* least number that is a valid LANGID */ + + for (i = 0; i < ARRAY_SIZE(src_langs); i++) + { + LANGID item = src_langs[i]; + BOOL is_item_duplicate = FALSE; + + for (j = 0; j < n; j++) + { + if (langs_arr[j] == item) + { + is_item_duplicate = TRUE; + break; + } + } + + if (!is_item_duplicate) + { + langs_arr[n++] = item; + } + } + + *lang_count = n; +} + +static void subtest_valid_manifest_resources_locale(LANGID actctx_lang) +{ + static const char manifest_exe_fmt[] = + "<assembly xmlns=\"urn:schemas-microsoft-com:asm.v1\" manifestVersion=\"1.0\">" + "<dependency>" + "<dependentAssembly>" + "<assemblyIdentity type=\"win32\" name=\"sxs_dll\" version=\"1.0.0.0\"" + " processorArchitecture=\"" ARCH "\" publicKeyToken=\"0000000000000000\" language=\"%ls\"/>" + "</dependentAssembly>" + "</dependency>" + "</assembly>"; + static const char manifest_dll_fmt[] = + "<assembly xmlns=\"urn:schemas-microsoft-com:asm.v3\" manifestVersion=\"1.0\">" + "<assemblyIdentity type=\"win32\" name=\"sxs_dll\" version=\"1.0.0.0\"" + " processorArchitecture=\"" ARCH "\" publicKeyToken=\"0000000000000000\" language=\"%ls\"/>" + "</assembly>"; + static const char manifest_dll_nofmt[] = + "<assembly xmlns=\"urn:schemas-microsoft-com:asm.v3\" manifestVersion=\"1.0\">" + "<assemblyIdentity type=\"win32\" name=\"sxs_dll\" version=\"1.0.0.0\"" + " processorArchitecture=\"" ARCH "\" publicKeyToken=\"0000000000000000\"/>" + "</assembly>"; + char manifest_exe[1024], manifest_dll[1024]; + WCHAR locale_name[LOCALE_NAME_MAX_LENGTH]; + UINT16 langs_arr[LANGID_PREC_MAX_COUNT]; + size_t lang_count = 0, i, j; + int ret; + + if (actctx_lang == MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL)) + { + wcscpy(locale_name, L"*"); + strcpy(manifest_dll, manifest_dll_nofmt); + } + else + { + actctx_lang = ConvertDefaultLocale(actctx_lang); + ok(actctx_lang != MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL), + "unexpected neutral locale\n"); + ret = LCIDToLocaleName(MAKELCID(actctx_lang, SORT_DEFAULT), + locale_name, ARRAY_SIZE(locale_name), LOCALE_ALLOW_NEUTRAL_NAMES); + ok(ret > 0, "error %lu.\n", GetLastError()); + + ret = snprintf(manifest_dll, ARRAY_SIZE(manifest_dll), manifest_dll_fmt, locale_name); + ok(ret > 0 && ret < ARRAY_SIZE(manifest_dll), "ret %d.\n", ret); + } + + ret = snprintf(manifest_exe, ARRAY_SIZE(manifest_exe), manifest_exe_fmt, locale_name); + ok(ret > 0 && ret < ARRAY_SIZE(manifest_exe), "ret %d.\n", ret); + + get_langid_precedence(langs_arr, &lang_count); + + for (i = 0; i < lang_count; i++) + { + struct manifest_res_spec specs[ARRAY_SIZE(langs_arr) + 1]; + struct multiple_manifest_test test = { { NULL } }; + size_t num_specs; + DWORD err; + + winetest_push_context("langs[%Id:]", i); + + /* Generate manifest spec list from language IDs. + * + * All manifest spec items point to the wrong manifest, expect for the + * current language ID. + */ + num_specs = 0; + for (j = i; j < lang_count; j++) + { + struct manifest_res_spec spec = {(char *)2}; + spec.lang = langs_arr[j]; + if (j != i) spec.override_manifest = wrong_manifest1; + ok(num_specs < ARRAY_SIZE(specs), "overrun\n"); + specs[num_specs++] = spec; + } + memset(&specs[num_specs++], 0, sizeof(*specs)); + + test.manifests = specs; + test.expected_error = ERROR_SUCCESS; + test.is_todo_wine = TRUE; + err = subtest_manifest_res(manifest_exe, manifest_dll, &test, actctx_lang); + + if (winetest_debug > 1 && err != ERROR_SUCCESS) + { + for (j = 0; j < lang_count; j++) + { + trace("langs[%Id] = 0x%04x %c\n", j, langs_arr[j], j == i ? '<' : ' '); + } + } + + winetest_pop_context(); + } +} + +static void test_valid_manifest_resources_locale(void) +{ + static const LANGID langs[] = { + MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL), + MAKELANGID(LANG_INVARIANT, SUBLANG_NEUTRAL), + }; + size_t i; + + for (i = 0; i < ARRAY_SIZE(langs); i++) + { + winetest_push_context("[%Iu]lang=0x%04x", i, langs[i]); + subtest_valid_manifest_resources_locale(langs[i]); + winetest_pop_context(); + } +} + static void run_sxs_test(int run) { switch(run) @@ -3858,6 +4276,8 @@ START_TEST(actctx) test_manifest_in_module(); test_manifest_resource_name_omitted(); + test_manifest_resources(); + test_valid_manifest_resources_locale(); test_actctx(); test_create_fail(); test_CreateActCtx(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2555
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This allows any manifest resource IDs (e.g., ISOLATIONAWARE_MANIFEST_RESOURCE_ID) to be recognized when looking up the assembly manifest of a dependency. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=18889 --- dlls/kernel32/tests/actctx.c | 52 ++++++++++++++++++------------------ dlls/ntdll/actctx.c | 27 +++++++++++++++++-- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index 0b249e82b0e..62d121e83d1 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -3916,48 +3916,48 @@ static void test_manifest_resources(void) struct multiple_manifest_test tests[] = { /* Test well-known manifest resource IDs */ { { (char *)CREATEPROCESS_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, FALSE }, - { { (char *)ISOLATIONAWARE_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)ISOLATIONAWARE_NOSTATICIMPORT_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)ISOLATIONAWARE_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)ISOLATIONAWARE_NOSTATICIMPORT_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, FALSE }, /* Test remaining reserved manifest resource IDs */ - { { (char *)4 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)5 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)6 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)7 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)8 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)9 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)10 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)11 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)12 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)13 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)14 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)15 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)MAXIMUM_RESERVED_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)4 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)5 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)6 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)7 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)8 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)9 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)10 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)11 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)12 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)13 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)14 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)15 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)MAXIMUM_RESERVED_MANIFEST_RESOURCE_ID }, NULL, ERROR_SUCCESS, FALSE }, /* Test arbitrary resource IDs */ - { { (char *)0x1234 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)0x89ab }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)0xffff }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)0x1234 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)0x89ab }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)0xffff }, NULL, ERROR_SUCCESS, FALSE }, /* Test arbitrary LANGID */ - { { (char *)2, MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT) }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)2, MAKELANGID(LANG_FRENCH,SUBLANG_DEFAULT) }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)2, 0x1234 }, NULL, ERROR_SUCCESS, TRUE }, - { { (char *)2, 0xffff }, NULL, ERROR_SUCCESS, TRUE }, + { { (char *)2, MAKELANGID(LANG_ENGLISH,SUBLANG_DEFAULT) }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)2, MAKELANGID(LANG_FRENCH,SUBLANG_DEFAULT) }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)2, 0x1234 }, NULL, ERROR_SUCCESS, FALSE }, + { { (char *)2, 0xffff }, NULL, ERROR_SUCCESS, FALSE }, /* Test multiple manifest resources coexisting inside a module */ - { { (char *)2, 0 }, wrong_manifest_resources_numbered, ERROR_SUCCESS, TRUE }, + { { (char *)2, 0 }, wrong_manifest_resources_numbered, ERROR_SUCCESS, FALSE }, { { (char *)2, 0, wrong_manifest1 }, correct_multiple_resources_numbered, ERROR_SXS_CANT_GEN_ACTCTX, FALSE }, /* Test that smaller resource ID takes precedence regardless of language ID */ { { (char *)2, MAKELANGID(LANG_INVARIANT,SUBLANG_NEUTRAL) }, - wrong_manifest_resources_gte_3, ERROR_SUCCESS, TRUE }, + wrong_manifest_resources_gte_3, ERROR_SUCCESS, FALSE }, { { (char *)2, MAKELANGID(LANG_INVARIANT,SUBLANG_NEUTRAL), wrong_manifest1 }, correct_manifest_resources_gte_3, ERROR_SXS_CANT_GEN_ACTCTX, FALSE }, /* Test multiple manifest resources (ID / name) coexisting inside a module */ - { { (char *)2, 0 }, wrong_manifest_resources_named, ERROR_SUCCESS, TRUE }, + { { (char *)2, 0 }, wrong_manifest_resources_named, ERROR_SUCCESS, FALSE }, { { (char *)2, 0, wrong_manifest1 }, correct_manifest_resources_named, ERROR_SXS_CANT_GEN_ACTCTX, FALSE }, @@ -4091,7 +4091,7 @@ static void subtest_valid_manifest_resources_locale(LANGID actctx_lang) test.manifests = specs; test.expected_error = ERROR_SUCCESS; - test.is_todo_wine = TRUE; + test.is_todo_wine = FALSE; err = subtest_manifest_res(manifest_exe, manifest_dll, &test, actctx_lang); if (winetest_debug > 1 && err != ERROR_SUCCESS) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index a2e159c76ae..e2adbd3394c 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -2929,6 +2929,24 @@ static NTSTATUS open_nt_file( HANDLE *handle, UNICODE_STRING *name ) FILE_SHARE_READ | FILE_SHARE_DELETE, FILE_SYNCHRONOUS_IO_ALERT ); } +static NTSTATUS find_first_manifest_resource_in_module( HANDLE hModule, const WCHAR **resname ) +{ + static const LDR_RESOURCE_INFO manifest_res_info = { RT_MANIFEST }; + const IMAGE_RESOURCE_DIRECTORY_ENTRY *entry_base, *entry; + const IMAGE_RESOURCE_DIRECTORY *resdir; + NTSTATUS status; + + status = LdrFindResourceDirectory_U( hModule, &manifest_res_info, 1, &resdir ); + if (status != STATUS_SUCCESS) return status; + + if (!resdir->NumberOfIdEntries) return STATUS_RESOURCE_NAME_NOT_FOUND; + entry_base = (const IMAGE_RESOURCE_DIRECTORY_ENTRY *)(resdir + 1); + entry = entry_base + resdir->NumberOfNamedEntries; + *resname = (const WCHAR *)(ULONG_PTR)entry->u.Id; + + return STATUS_SUCCESS; +} + static NTSTATUS get_manifest_in_module( struct actctx_loader* acl, struct assembly_identity* ai, LPCWSTR filename, LPCWSTR directory, BOOL shared, HANDLE hModule, LPCWSTR resname, ULONG lang ) @@ -2951,6 +2969,12 @@ static NTSTATUS get_manifest_in_module( struct actctx_loader* acl, struct assemb hModule, debugstr_w(filename) ); } + if (!resname) + { + status = find_first_manifest_resource_in_module( hModule, &resname ); + if (status != STATUS_SUCCESS) return status; + } + info.Type = RT_MANIFEST; info.Language = lang; if (!((ULONG_PTR)resname >> 16)) @@ -3330,8 +3354,7 @@ static NTSTATUS lookup_assembly(struct actctx_loader* acl, status = open_nt_file( &file, &nameW ); if (!status) { - status = get_manifest_in_pe_file( acl, ai, nameW.Buffer, directory, FALSE, file, - (LPCWSTR)CREATEPROCESS_MANIFEST_RESOURCE_ID, 0 ); + status = get_manifest_in_pe_file( acl, ai, nameW.Buffer, directory, FALSE, file, NULL, 0 ); NtClose( file ); if (status == STATUS_SUCCESS) break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2555
- v7: rebase - v8: fix commit message typo (ACTCTX -> ACTCTW); simplify tests -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2555#note_33544
Rémi Bernon (@rbernon) commented about dlls/kernel32/tests/actctx.c:
test.manifests = specs; test.expected_error = ERROR_SUCCESS; - test.is_todo_wine = TRUE; + test.is_todo_wine = FALSE;
I think it's best to completely remove the `is_todo_wine` field now, as it's not needed anymore. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2555#note_33651
On Wed May 24 09:00:14 2023 +0000, Rémi Bernon wrote:
I think it's best to completely remove the `is_todo_wine` field now, as it's not needed anymore. I'll still need to add it back in the future when I go on to implement manifest locales, and that adds noise to the diff.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2555#note_33662
On Wed May 24 10:01:45 2023 +0000, Jinoh Kang wrote:
I'll still need to add it back in the future when I go on to implement manifest locales, and that adds noise to the diff. I think removing the todo now, and eventually add one again later if and when you need it, makes things clearer. You can also rely on implicit zero initialization to reduce the diff.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2555#note_33663
On Wed May 24 10:09:34 2023 +0000, Rémi Bernon wrote:
I think removing the todo now, and eventually add one again later if and when you need it, makes things clearer. You can also rely on implicit zero initialization to reduce the diff. (I'll approve nonetheless as it's probably not such a big deal but I think it's good practice).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2555#note_33664
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2555
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Rémi Bernon