[PATCH v4 0/2] MR2618: ntdll: Increment offset by len in build_clr_surrogate_section.

[Alex Henrie (＠alexhenrie)]

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

[PATCH v4 1/2] kernel32/tests: Test internal consistency of activation context headers.

[Alex Henrie]

From: Alex Henrie

---
 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);

[PATCH v4 2/2] ntdll: Increment offset by len in build_clr_surrogate_section.

[Alex Henrie]

From: Alex Henrie

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++;
             }
         }

[PATCH v4 2/2] ntdll: Increment offset by len in build_clr_surrogate_section.

[Marvin]

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

[Nikolay Sivov (＠nsivov)]

Now there is no checks for underflow when you subtract offsets in tests. Where are the tests failure coming from?

[Alex Henrie (＠alexhenrie)]

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.

[Nikolay Sivov (＠nsivov)]

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.