[PATCH 0/4] MR4792: ntdll/tests: Fix xstate tests failing on Windows 11 and CPU with more XSAVE features.
Follow-up of MR !1939. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4792
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> The penultimate element of `ranges_x86` array has an incorrect value: it should be *at least* 0x2f0, which is the minimum size of an extended context. Fix this by setting it to 0x440, which is the minimum size of an extended context *with* CONTEXT_I386_XSTATE. This is consistent with `ranges_amd64`, the penultimate element of which has the minimum size of an extended context *with* CONTEXT_AMD64_XSTATE. Note that the incorrect value does not always lead to a test failure, since check_changes_in_range_() effectively ignores range `start`s that are not in order. Reproducing the failure requires a system with a sufficiently large XSAVE area; specifically, the following condition is necessary for check_changes_in_range_() to pick up the wrong value: 0x2cc < 0x294 + src_ex->XState.Length - sizeof(XSTATE). --- dlls/ntdll/tests/exception.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index bac571b2733..79a65dadab9 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -11848,7 +11848,7 @@ static void test_copy_context(void) static struct modified_range ranges_x86[] = { {0x0, ~0}, {0x4, 0x10}, {0x1c, 0x8}, {0x8c, 0x4}, {0x9c, 0x2}, {0xb4, 0x1}, {0xcc, 0x20}, {0x1ec, 0x80000020}, - {0x2cc, ~0}, {0x294, 0}, {0x1000, 0}, + {0x2cc, ~0}, {0x440, 0}, {0x1000, 0}, }; static const struct modified_range single_range[] = { @@ -11963,7 +11963,7 @@ static void test_copy_context(void) if (flags & CONTEXT_AMD64) ranges_amd64[ARRAY_SIZE(ranges_amd64) - 2].start = 0x640 + src_ex->XState.Length - sizeof(XSTATE); else - ranges_x86[ARRAY_SIZE(ranges_x86) - 2].start = 0x294 + src_ex->XState.Length - sizeof(XSTATE); + ranges_x86[ARRAY_SIZE(ranges_x86) - 2].start = 0x440 + src_ex->XState.Length - sizeof(XSTATE); } status = pRtlCopyExtendedContext(dst_ex, flags, src_ex); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4792
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> `(BYTE *)dst_ex - (BYTE *)dst` is the size of the legacy context, but `dst_ex->All` already contains the legacy context. Therefore, `context_length` has the legacy context size added *twice*. This becomes a problem when `context_length` exceeds `sizeof(src_context_buffer)`. This confuses `check_changes_in_range()`, causing out-of-bounds read and unpredictable test results. --- dlls/ntdll/tests/exception.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 79a65dadab9..9f6bfbd2008 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -11952,7 +11952,7 @@ static void test_copy_context(void) *(DWORD *)((BYTE *)dst + flags_offset) = 0; *(DWORD *)((BYTE *)src + flags_offset) = 0; - context_length = (BYTE *)dst_ex - (BYTE *)dst + dst_ex->All.Length; + context_length = dst_ex->All.Length; if (flags & 0x40) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4792
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This is required to support systems with a larger XSAVE area. --- dlls/ntdll/tests/exception.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 9f6bfbd2008..d98bacd2f16 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -10884,7 +10884,7 @@ static void test_extended_context(void) const struct context_parameters *context_arch; const ULONG64 supported_features = 7, supported_compaction_mask = supported_features | ((ULONG64)1 << 63); - ULONG expected_length, expected_length_xstate, context_flags, expected_offset; + ULONG expected_length, expected_length_xstate, context_flags, expected_offset, max_xstate_length; ULONG64 enabled_features, expected_compaction; DECLSPEC_ALIGN(64) BYTE context_buffer2[2048]; DECLSPEC_ALIGN(64) BYTE context_buffer[2048]; @@ -11511,6 +11511,10 @@ static void test_extended_context(void) context_ex = (CONTEXT_EX *)(context + 1); xs = (XSTATE *)((BYTE *)context_ex + context_ex->XState.Offset); + max_xstate_length = context_ex->XState.Length; + ok(max_xstate_length >= sizeof(XSTATE), "XSTATE size: %#lx; min: %#Ix.\n", max_xstate_length, sizeof(XSTATE)); + ok(!(max_xstate_length % 64), "XSTATE size: %#lx; should be multiple of 64.\n", max_xstate_length); + *(void **)(call_func_code_set_ymm0 + call_func_offsets.func_addr) = RtlCaptureContext; *(void **)(call_func_code_set_ymm0 + call_func_offsets.func_param1) = context; *(void **)(call_func_code_set_ymm0 + call_func_offsets.func_param2) = (void *)0xdeadbeef; @@ -11571,7 +11575,7 @@ static void test_extended_context(void) xs->CompactionMask = 4; xs->Mask = compaction_enabled ? 0 : 4; - context_ex->XState.Length = sizeof(XSTATE) + 64; + context_ex->XState.Length = max_xstate_length + 64; bret = func(); ok(!bret && GetLastError() == ERROR_INVALID_PARAMETER, "Got unexpected bret %#x, GetLastError() %lu.\n", bret, GetLastError()); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4792
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/tests/exception.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index d98bacd2f16..d659672b48f 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -228,7 +228,7 @@ static enum debugger_stages test_stage; #if defined(__i386__) || defined(__x86_64__) static void test_debugger_xstate(HANDLE thread, CONTEXT *ctx, enum debugger_stages stage) { - char context_buffer[sizeof(CONTEXT) + sizeof(CONTEXT_EX) + sizeof(XSTATE) + 1024]; + char context_buffer[sizeof(CONTEXT) + sizeof(CONTEXT_EX) + sizeof(XSTATE) + 3072]; CONTEXT_EX *c_ex; NTSTATUS status; YMMCONTEXT *ymm; @@ -10886,8 +10886,8 @@ static void test_extended_context(void) const ULONG64 supported_features = 7, supported_compaction_mask = supported_features | ((ULONG64)1 << 63); ULONG expected_length, expected_length_xstate, context_flags, expected_offset, max_xstate_length; ULONG64 enabled_features, expected_compaction; - DECLSPEC_ALIGN(64) BYTE context_buffer2[2048]; - DECLSPEC_ALIGN(64) BYTE context_buffer[2048]; + DECLSPEC_ALIGN(64) BYTE context_buffer2[4096]; + DECLSPEC_ALIGN(64) BYTE context_buffer[4096]; unsigned int i, j, address_offset, test; ULONG ret, ret2, length, length2, align; ULONG flags, flags_fpx, expected_flags; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4792
I run the tests with this MR on one AMD machine here and I am getting this new test failure:
exception.c:11516: Test failed: XSTATE size: 0x150; should be multiple of 64.
That's both on 64 and 32 bit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4792#note_56922
I guess it is just the test on this line which is wrong and can be dropped (the XSTATE length is not necessarily a multiple of 64 for a generic set of features). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4792#note_56923
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Paul Gofman (＠gofman)