[PATCH v4 0/2] MR2618: ntdll: Increment offset by len in build_clr_surrogate_section.
Instead of multiplying it by 2. I don't know how this ever worked. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54808 -- v4: ntdll: Increment offset by len in build_clr_surrogate_section. kernel32/tests: Test internal consistency of activation context headers. https://gitlab.winehq.org/wine/wine/-/merge_requests/2618
From: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/kernel32/tests/actctx.c | 84 ++++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 9 deletions(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index da351e70466..ff1a6ac4109 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -1255,6 +1255,71 @@ struct progidredirect_data ULONG clsid_offset; }; +#define STRSECTION_MAGIC 0x64487353 /* dHsS */ +#define GUIDSECTION_MAGIC 0x64487347 /* dHsG */ + +static void validate_str_index(const ACTCTX_SECTION_KEYED_DATA *data, int line) +{ + struct strsection_header *header = (struct strsection_header*)data->lpSectionBase; + struct string_index *index; + ULONG min_offset, max_offset, i; + + ok_(__FILE__, line)(header != NULL, "header == NULL\n"); + ok_(__FILE__, line)(header->magic == STRSECTION_MAGIC, "got wrong magic 0x%08lx\n", header->magic); + ok_(__FILE__, line)(header->size == sizeof(*header), + "got size %ld, expected %Iu\n", header->size, sizeof(*header)); + ok_(__FILE__, line)(header->index_offset >= header->size, + "got index offset %lu, expected at least %lu\n", header->index_offset, header->size); + + index = (struct string_index*)((BYTE*)header + header->index_offset); + min_offset = header->index_offset + header->count * sizeof(*index); + for (i = 0; i < header->count; ++i) + { + ok_(__FILE__, line)(index[i].name_len <= data->ulSectionTotalLength - min_offset, + "i=%lu got name len %ld, expected no more than %ld\n", + i, index[i].name_len, data->ulSectionTotalLength - min_offset); + max_offset = data->ulSectionTotalLength - index[i].name_len; + ok_(__FILE__, line)(index[i].name_offset >= min_offset && index[i].name_offset <= max_offset, + "i=%lu got name offset %ld, expected from %ld to %ld\n", + i, index[i].name_offset, min_offset, max_offset); + + ok_(__FILE__, line)(index[i].data_len <= data->ulSectionTotalLength - min_offset, + "i=%lu got data len %ld, expected no more than %ld\n", + i, index[i].data_len, data->ulSectionTotalLength - min_offset); + max_offset = data->ulSectionTotalLength - index[i].data_len; + ok_(__FILE__, line)(index[i].data_offset >= min_offset && index[i].data_offset <= max_offset, + "i=%lu got data offset %ld, expected from %ld to %ld\n", + i, index[i].data_offset, min_offset, max_offset); + } +} + +static void validate_guid_index(const ACTCTX_SECTION_KEYED_DATA *data, int line) +{ + struct guidsection_header *header = (struct guidsection_header*)data->lpSectionBase; + struct guid_index *index; + ULONG min_offset, max_offset, i; + + ok_(__FILE__, line)(header != NULL, "header == NULL\n"); + ok_(__FILE__, line)(header->magic == GUIDSECTION_MAGIC, "got wrong magic 0x%08lx\n", header->magic); + ok_(__FILE__, line)(header->size == sizeof(*header), + "got size %ld, expected %Iu\n", header->size, sizeof(*header)); + ok_(__FILE__, line)(header->index_offset >= header->size, + "got index offset %lu, expected at least %lu\n", header->index_offset, header->size); + + index = (struct guid_index*)((BYTE*)header + header->index_offset); + min_offset = header->index_offset + header->count * sizeof(*index); + for (i = 0; i < header->count; ++i) + { + ok_(__FILE__, line)(index[i].data_len <= data->ulSectionTotalLength - min_offset, + "i=%lu got data len %ld, expected no more than %ld\n", + i, index[i].data_len, data->ulSectionTotalLength - min_offset); + max_offset = data->ulSectionTotalLength - index[i].data_len; + ok_(__FILE__, line)(index[i].data_offset >= min_offset && index[i].data_offset <= max_offset, + "i=%lu got data offset %ld, expected from %ld to %ld\n", + i, index[i].data_offset, min_offset, max_offset); + } +} + static void test_find_dll_redirection(HANDLE handle, LPCWSTR libname, ULONG exid, int line) { ACTCTX_SECTION_KEYED_DATA data; @@ -1337,7 +1402,7 @@ static void test_find_window_class(HANDLE handle, LPCWSTR clsname, ULONG exid, i header = (struct strsection_header*)data.lpSectionBase; wnddata = (struct wndclass_redirect_data*)data.lpData; - ok_(__FILE__, line)(header->magic == 0x64487353, "got wrong magic 0x%08lx\n", header->magic); + validate_str_index(&data, line); ok_(__FILE__, line)(header->count > 0, "got count %ld\n", header->count); ok_(__FILE__, line)(data.cbSize == sizeof(data), "data.cbSize=%lu\n", data.cbSize); ok_(__FILE__, line)(data.ulDataFormatVersion == 1, "data.ulDataFormatVersion=%lu\n", data.ulDataFormatVersion); @@ -1453,9 +1518,8 @@ static void test_find_activatable_class(HANDLE handle, const WCHAR *classid, enu ok_(__FILE__, line)(data.ulDataFormatVersion == 1, "got ulDataFormatVersion %lu\n", data.ulDataFormatVersion); ok_(__FILE__, line)(data.lpData != NULL, "got lpData %p\n", data.lpData); + validate_str_index(&data, line); header = (struct strsection_header *)data.lpSectionBase; - ok_(__FILE__, line)(header->magic == 0x64487353, "got wrong magic 0x%08lx\n", header->magic); - ok_(__FILE__, line)(data.lpSectionBase != NULL, "got lpSectionBase %p\n", data.lpSectionBase); ok_(__FILE__, line)(data.ulSectionTotalLength > 0, "got ulSectionTotalLength %lu\n", data.ulSectionTotalLength); ok_(__FILE__, line)(data.lpSectionGlobalData == (BYTE *)header + header->global_offset, "got lpSectionGlobalData %p\n", data.lpSectionGlobalData); @@ -1705,11 +1769,11 @@ static void test_find_com_redirection(HANDLE handle, const GUID *clsid, const GU } } + validate_guid_index(&data, line); header = (struct guidsection_header*)data.lpSectionBase; ok_(__FILE__, line)(data.lpSectionGlobalData == ((BYTE*)header + header->names_offset), "data.lpSectionGlobalData == NULL\n"); ok_(__FILE__, line)(data.ulSectionGlobalDataLength == header->names_len, "data.ulSectionGlobalDataLength=%lu\n", data.ulSectionGlobalDataLength); - ok_(__FILE__, line)(data.lpSectionBase != NULL, "data.lpSectionBase == NULL\n"); ok_(__FILE__, line)(data.ulSectionTotalLength > 0, "data.ulSectionTotalLength=%lu\n", data.ulSectionTotalLength); ok_(__FILE__, line)(data.hActCtx == NULL, "data.hActCtx=%p\n", data.hActCtx); @@ -1862,10 +1926,10 @@ static void test_find_surrogate(HANDLE handle, const GUID *clsid, const WCHAR *n ok(!lstrcmpW(ptrW, version), "got wrong name %s\n", wine_dbgstr_w(ptrW)); } + validate_guid_index(&data, line); ok_(__FILE__, line)(data.lpSectionGlobalData == NULL, "data.lpSectionGlobalData != NULL\n"); ok_(__FILE__, line)(data.ulSectionGlobalDataLength == 0, "data.ulSectionGlobalDataLength=%lu\n", data.ulSectionGlobalDataLength); - ok_(__FILE__, line)(data.lpSectionBase != NULL, "data.lpSectionBase == NULL\n"); ok_(__FILE__, line)(data.ulSectionTotalLength > 0, "data.ulSectionTotalLength=%lu\n", data.ulSectionTotalLength); ok_(__FILE__, line)(data.hActCtx == NULL, "data.hActCtx=%p\n", data.hActCtx); @@ -1913,10 +1977,10 @@ static void test_find_progid_redirection(HANDLE handle, const GUID *clsid, const ok_(__FILE__, line)(IsEqualGUID(clsid, &comclass->clsid), "got wrong class referenced from progid %s, %s\n", progid, wine_dbgstr_guid(clsid)); } + validate_str_index(&data, line); header = (struct strsection_header*)data.lpSectionBase; ok_(__FILE__, line)(data.lpSectionGlobalData == (BYTE*)header + header->global_offset, "data.lpSectionGlobalData == NULL\n"); ok_(__FILE__, line)(data.ulSectionGlobalDataLength == header->global_len, "data.ulSectionGlobalDataLength=%lu\n", data.ulSectionGlobalDataLength); - ok_(__FILE__, line)(data.lpSectionBase != NULL, "data.lpSectionBase == NULL\n"); ok_(__FILE__, line)(data.ulSectionTotalLength > 0, "data.ulSectionTotalLength=%lu\n", data.ulSectionTotalLength); ok_(__FILE__, line)(data.hActCtx == NULL, "data.hActCtx=%p\n", data.hActCtx); ok_(__FILE__, line)(data.ulAssemblyRosterIndex == exid, "data.ulAssemblyRosterIndex=%lu, expected %lu\n", @@ -1960,9 +2024,9 @@ static void test_wndclass_section(void) ret = FindActCtxSectionStringW(0, NULL, ACTIVATION_CONTEXT_SECTION_WINDOW_CLASS_REDIRECTION, wndClass3W, &data2); ok(ret, "got %d\n", ret); + validate_str_index(&data, __LINE__); section = (struct strsection_header*)data.lpSectionBase; ok(section->count == 4, "got %ld\n", section->count); - ok(section->size == sizeof(*section), "got %ld\n", section->size); /* For both string same section is returned, meaning it's one wndclass section per context */ ok(data.lpSectionBase == data2.lpSectionBase, "got %p, %p\n", data.lpSectionBase, data2.lpSectionBase); @@ -2020,9 +2084,9 @@ static void test_dllredirect_section(void) ret = FindActCtxSectionStringW(0, NULL, ACTIVATION_CONTEXT_SECTION_DLL_REDIRECTION, testlib2W, &data2); ok(ret, "got %d\n", ret); + validate_str_index(&data, __LINE__); section = (struct strsection_header*)data.lpSectionBase; ok(section->count == 4, "got %ld\n", section->count); - ok(section->size == sizeof(*section), "got %ld\n", section->size); /* For both string same section is returned, meaning it's one dll redirect section per context */ ok(data.lpSectionBase == data2.lpSectionBase, "got %p, %p\n", data.lpSectionBase, data2.lpSectionBase); @@ -2073,9 +2137,11 @@ static void test_typelib_section(void) &IID_TlibTest4, &data2); ok(ret, "got %d\n", ret); + validate_guid_index(&data, __LINE__); + validate_guid_index(&data2, __LINE__); + section = (struct guidsection_header*)data.lpSectionBase; ok(section->count == 4, "got %ld\n", section->count); - ok(section->size == sizeof(*section), "got %ld\n", section->size); /* For both GUIDs same section is returned */ ok(data.lpSectionBase == data2.lpSectionBase, "got %p, %p\n", data.lpSectionBase, data2.lpSectionBase); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2618
From: Alex Henrie <alexhenrie24(a)gmail.com> Instead of multiplying it by 2. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54808 --- dlls/kernel32/tests/actctx.c | 5 +++++ dlls/ntdll/actctx.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index ff1a6ac4109..6e026ec0d72 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -147,6 +147,11 @@ static const char manifest3[] = " name=\"testsurrogate\"" " runtimeVersion=\"v2.0.50727\"" " />" +" <clrSurrogate " +" clsid=\"{96666666-8888-7777-6666-555555555556}\"" +" name=\"testsurrogate\"" +" runtimeVersion=\"v2.0.50727\"" +" />" " <clrClass " " clsid=\"{22345678-1234-5678-1234-111122223333}\"" " name=\"clrclass\"" diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index 08a4893a85d..54167a2db67 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -4828,7 +4828,7 @@ static NTSTATUS build_clr_surrogate_section(ACTIVATION_CONTEXT* actctx, struct g ptrW[data->version_len/sizeof(WCHAR)] = 0; } - data_offset += index->data_offset; + data_offset += index->data_len; index++; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2618
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132042 Your paranoid android. === w7pro64 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w864 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064v1507 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064v1809 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_2qxl (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_adm (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_tsign (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_en_AE_u8 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_ar (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_ja (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_zh_CN (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w11pro64_amd (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105
Now there is no checks for underflow when you subtract offsets in tests. Where are the tests failure coming from? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2618#note_30621
On Thu Apr 20 14:35:40 2023 +0000, Nikolay Sivov wrote:
Now there is no checks for underflow when you subtract offsets in tests. Where are the tests failure coming from? The test failures are all pre-existing; I don't know why the testbot is complaining. If the tests are still not to your satisfaction, please send me another diff and I will apply it without making any other changes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2618#note_30739
On Thu Apr 20 14:35:40 2023 +0000, Alex Henrie wrote:
The test failures are all pre-existing; I don't know why the testbot is complaining. If the tests are still not to your satisfaction, please send me another diff and I will apply it without making any other changes. That's what I did in https://gitlab.winehq.org/wine/wine/-/merge_requests/2618#note_30547.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2618#note_30740
participants (4)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Marvin -
Nikolay Sivov (＠nsivov)