[PATCH v6 0/1] MR4720: Implement test_continue() for amd64

[Dāvis Mosāns (davispuh) (＠davispuh)]

Currently there is `NtContinue()` test only for `aarch64` so implement it for amd64 aswell.

This implementation is very similar to `aarch64` and it's very basic test.

It doesn't change/test `ContextFlags` so it won't catch https://bugs.winehq.org/show_bug.cgi?id=56050 but that can be implemented later on top of this.

--
  v6: ntdll/tests: Implement test_continue() for amd64

https://gitlab.winehq.org/wine/wine/-/merge_requests/4720

[PATCH v6 1/1] ntdll/tests: Implement test_continue() for amd64

[Dāvis Mosāns]

From: Dāvis Mosāns

---
 dlls/ntdll/tests/exception.c | 163 +++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c
index bac571b2733..31a7649110a 100644
--- a/dlls/ntdll/tests/exception.c
+++ b/dlls/ntdll/tests/exception.c
@@ -4413,6 +4413,168 @@ static void test_thread_context(void)
 #undef COMPARE
 }

+static void test_continue(void)
+{
+    struct context_pair {
+        CONTEXT before;
+        CONTEXT after;
+    } contexts;
+    NTSTATUS (*func_ptr)( struct context_pair *, BOOL alertable, void *continue_func, void *capture_func ) = code_mem;
+
+    static const BYTE call_func[] =
+    {
+        /* ret at 8*9(rsp) */
+
+        /* need to preserve these */
+        0x53,             /* push %rbx; 8*8(rsp) */
+        0x55,             /* push %rbp; 8*7(rsp) */
+        0x56,             /* push %rsi; 8*6(rsp) */
+        0x57,             /* push %rdi; 8*5(rsp) */
+        0x41, 0x54,       /* push %r12; 8*4(rsp) */
+        0x41, 0x55,       /* push %r13; 8*3(rsp) */
+        0x41, 0x56,       /* push %r14; 8*2(rsp) */
+        0x41, 0x57,       /* push %r15; 8*1(rsp) */
+
+        0x48, 0x83, 0xec, 0x08, /* sub $0x8, %rsp; reserve space for rsp */
+        0x48, 0x89, 0x24, 0x24, /* mov %rsp, (%rsp); for stack validation */
+
+        /* save args */
+        0x48, 0x89, 0x4c, 0x24, 0x50, /* mov %rcx, 8*10(%rsp) */
+        0x48, 0x89, 0x54, 0x24, 0x58, /* mov %rdx, 8*11(%rsp) */
+        0x4c, 0x89, 0x44, 0x24, 0x60, /* mov %r8,  8*12(%rsp) */
+        0x4c, 0x89, 0x4c, 0x24, 0x68, /* mov %r9,  8*13(%rsp) */
+
+        /* invoke capture context */
+        0x41, 0xff, 0xd1,       /* call *%r9 */
+
+        /* overwrite general registers */
+        0x48, 0xb8, 0xef, 0xbe, 0xad, 0xde, 0xef, 0xbe, 0xad, 0xde,  /* movabs $0xdeadbeefdeadbeef, %rax */
+        0x48, 0x89, 0xc1, /* mov %rax, %rcx */
+        0x48, 0x89, 0xc2, /* mov %rax, %rdx */
+        0x48, 0x89, 0xc3, /* mov %rax, %rbx */
+        0x48, 0x89, 0xc5, /* mov %rax, %rbp */
+        0x48, 0x89, 0xc6, /* mov %rax, %rsi */
+        0x48, 0x89, 0xc7, /* mov %rax, %rdi */
+        0x49, 0x89, 0xc0, /* mov %rax, %r8 */
+        0x49, 0x89, 0xc1, /* mov %rax, %r9 */
+        0x49, 0x89, 0xc2, /* mov %rax, %r10 */
+        0x49, 0x89, 0xc3, /* mov %rax, %r11 */
+        0x49, 0x89, 0xc4, /* mov %rax, %r12 */
+        0x49, 0x89, 0xc5, /* mov %rax, %r13 */
+        0x49, 0x89, 0xc6, /* mov %rax, %r14 */
+        0x49, 0x89, 0xc7, /* mov %rax, %r15 */
+
+        /* overwrite SSE registers */
+        0x66, 0x48, 0x0f, 0x6e, 0xc0, /* movq %rax, %xmm0 */
+        0x66, 0x0f, 0x6c, 0xc0,       /* punpcklqdq %xmm0, %xmm0; extend to high quadword */
+        0x0f, 0x28, 0xc8,             /* movaps %xmm0, %xmm1 */
+        0x0f, 0x28, 0xd0,             /* movaps %xmm0, %xmm2 */
+        0x0f, 0x28, 0xd8,             /* movaps %xmm0, %xmm3 */
+        0x0f, 0x28, 0xe0,             /* movaps %xmm0, %xmm4 */
+        0x0f, 0x28, 0xe8,             /* movaps %xmm0, %xmm5 */
+        0x0f, 0x28, 0xf0,             /* movaps %xmm0, %xmm6 */
+        0x0f, 0x28, 0xf8,             /* movaps %xmm0, %xmm7 */
+        0x44, 0x0f, 0x28, 0xc0,       /* movaps %xmm0, %xmm8 */
+        0x44, 0x0f, 0x28, 0xc8,       /* movaps %xmm0, %xmm9 */
+        0x44, 0x0f, 0x28, 0xd0,       /* movaps %xmm0, %xmm10 */
+        0x44, 0x0f, 0x28, 0xd8,       /* movaps %xmm0, %xmm11 */
+        0x44, 0x0f, 0x28, 0xe0,       /* movaps %xmm0, %xmm12 */
+        0x44, 0x0f, 0x28, 0xe8,       /* movaps %xmm0, %xmm13 */
+        0x44, 0x0f, 0x28, 0xf0,       /* movaps %xmm0, %xmm14 */
+        0x44, 0x0f, 0x28, 0xf8,       /* movaps %xmm0, %xmm15 */
+
+        /* FIXME: overwrite debug, x87 FPU and AVX registers to test those */
+
+        /* load args */
+        0x48, 0x8b, 0x4c, 0x24, 0x50, /* mov 8*10(%rsp), %rcx; context   */
+        0x48, 0x8b, 0x54, 0x24, 0x58, /* mov 8*11(%rsp), %rdx; alertable */
+        0x48, 0x83, 0xec, 0x70,       /* sub $0x70, %rsp; change stack   */
+
+        /* setup context to return to label 1 */
+        0x48, 0x8d, 0x05, 0x18, 0x00, 0x00, 0x00, /* lea 1f(%rip) */
+        0x48, 0x89, 0x81, 0xf8, 0x00, 0x00, 0x00, /* mov %rax, 0xf8(%rcx); context.Rip */
+
+        /* flip some EFLAGS */
+        0x9c,                                           /* pushf */
+        /*
+           0x0001 Carry flag
+           0x0004 Parity flag
+           0x0010 Auxiliary Carry flag
+           0x0040 Zero flag
+           0x0080 Sign flag
+           FIXME: 0x0400 Direction flag - not changing as it breaks Wine
+           0x0800 Overflow flag
+           ~0x4000~ Nested task flag - not changing - breaks Wine
+           = 0x8d5
+        */
+        0x48, 0x81, 0x34, 0x24, 0xd5, 0x08, 0x00, 0x00, /* xorq $0x8d5, (%rsp) */
+        0x9d,                                           /* popf */
+
+        /* invoke NtContinue... */
+        0xff, 0x94, 0x24, 0xd0, 0x00, 0x00, 0x00, /* call *8*12+0x70(%rsp) */
+
+        /* validate stack pointer */
+        0x48, 0x8b, 0x0c, 0x24, /* 1: mov (%rsp), %rcx */
+        0x48, 0x39, 0xe1,       /* cmp %rsp, %rcx  */
+        0x74, 0x02,             /* je 2f; jump over ud2 */
+        0x0f, 0x0b,             /* ud2; stack pointer invalid, let's crash */
+
+        /* invoke capture context */
+        0x48, 0x8b, 0x4c, 0x24, 0x50,             /* 2: mov 8*10(%rsp), %rcx; context */
+        0x48, 0x81, 0xc1, 0xd0, 0x04, 0x00, 0x00, /* add $0x4d0, %rcx; +sizeof(CONTEXT) to get context->after */
+        0xff, 0x54, 0x24, 0x68,                   /* call *8*13(%rsp) */
+
+        /* free stack */
+        0x48, 0x83, 0xc4, 0x08, /* add $0x8, %rsp */
+
+        /* restore back */
+        0x41, 0x5f,       /* pop %r15 */
+        0x41, 0x5e,       /* pop %r14 */
+        0x41, 0x5d,       /* pop %r13 */
+        0x41, 0x5c,       /* pop %r12 */
+        0x5f,             /* pop %rdi */
+        0x5e,             /* pop %rsi */
+        0x5d,             /* pop %rbp */
+        0x5b,             /* pop %rbx */
+        0xc3              /* ret      */
+    };
+
+    if (!pRtlCaptureContext)
+    {
+        win_skip("RtlCaptureContext is not available.\n");
+        return;
+    }
+
+    memcpy( func_ptr, call_func, sizeof(call_func) );
+    FlushInstructionCache( GetCurrentProcess(), func_ptr, sizeof(call_func) );
+
+    func_ptr( &contexts, FALSE, NtContinue, pRtlCaptureContext );
+
+#define COMPARE(reg) \
+    ok( contexts.before.reg == contexts.after.reg, "wrong " #reg " %p/%p\n", (void *)(ULONG64)contexts.before.reg, (void *)(ULONG64)contexts.after.reg )
+
+    COMPARE( Rax );
+    COMPARE( Rdx );
+    COMPARE( Rbx );
+    COMPARE( Rbp );
+    COMPARE( Rsi );
+    COMPARE( Rdi );
+    COMPARE( R8 );
+    COMPARE( R9 );
+    COMPARE( R10 );
+    COMPARE( R11 );
+    COMPARE( R12 );
+    COMPARE( R13 );
+    COMPARE( R14 );
+    COMPARE( R15 );
+
+    ok(memcmp(&contexts.before.Xmm0, &contexts.after.Xmm0, &contexts.before.Xmm15 - &contexts.before.Xmm0) == 0,
+       "wrong some SSE register, Xmm0 %lld/%lld Xmm15 %lld/%lld", contexts.before.Xmm0.Low, contexts.after.Xmm0.Low,
+       contexts.before.Xmm15.High, contexts.after.Xmm15.High);
+
+#undef COMPARE
+}
+
 static void test_wow64_context(void)
 {
     const char appname[] = "C:\windows\syswow64\cmd.exe";
@@ -12456,6 +12618,7 @@ START_TEST(exception)
     test_debug_registers_wow64();
     test_debug_service(1);
     test_simd_exceptions();
+    test_continue();
     test_virtual_unwind();
     test___C_specific_handler();
     test_restore_context();

[Dāvis Mosāns (davispuh) (＠davispuh)]

On Tue Jan  2 10:19:28 2024 +0000, Dāvis Mosāns (davispuh) wrote:
> changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/4720/diffs?diff_id=91677&start_sha=1a94e3d4b00b98de7634fe65fff1dd8ed8c6c58e#1104a825a741495a2a3c3093b4d85d95e4f8180f_4439_4439)

Opps, such silly mistakes :smile: fixed now

[Dāvis Mosāns (davispuh) (＠davispuh)]

On Tue Jan  2 10:19:29 2024 +0000, Dāvis Mosāns (davispuh) wrote:
> changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/4720/diffs?diff_id=91677&start_sha=1a94e3d4b00b98de7634fe65fff1dd8ed8c6c58e#1104a825a741495a2a3c3093b4d85d95e4f8180f_4494_4494)

Fixed

[Dāvis Mosāns (davispuh) (＠davispuh)]

On Tue Jan  2 10:19:31 2024 +0000, Dāvis Mosāns (davispuh) wrote:
> changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/4720/diffs?diff_id=91677&start_sha=1a94e3d4b00b98de7634fe65fff1dd8ed8c6c58e#1104a825a741495a2a3c3093b4d85d95e4f8180f_4517_4517)

Fixed

[Dāvis Mosāns (davispuh) (＠davispuh)]

On Tue Jan  2 10:19:31 2024 +0000, Dāvis Mosāns (davispuh) wrote:
> changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/4720/diffs?diff_id=91677&start_sha=1a94e3d4b00b98de7634fe65fff1dd8ed8c6c58e#1104a825a741495a2a3c3093b4d85d95e4f8180f_4519_4519)

Fixed

[Dāvis Mosāns (davispuh) (＠davispuh)]

On Tue Jan  2 10:19:32 2024 +0000, Dāvis Mosāns (davispuh) wrote:
> changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/4720/diffs?diff_id=91677&start_sha=1a94e3d4b00b98de7634fe65fff1dd8ed8c6c58e#1104a825a741495a2a3c3093b4d85d95e4f8180f_4524_4524)

Fixed

[Dāvis Mosāns (davispuh) (＠davispuh)]

On Tue Jan  2 10:19:33 2024 +0000, Dāvis Mosāns (davispuh) wrote:
> changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/4720/diffs?diff_id=91677&start_sha=1a94e3d4b00b98de7634fe65fff1dd8ed8c6c58e#1104a825a741495a2a3c3093b4d85d95e4f8180f_4528_4528)

Fixed

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntdll/tests/exception.c:
> -
>    0x48, 0x8b, 0x54, 0x24, 0x58, /* mov 8*11(%rsp), %rdx; alertable */
> -
>    0x48, 0x83, 0xec, 0x70,       /* sub $0x70, %rsp; change stack   */
> -
> -
>    /* setup context to return to label 1 */
> -
>    0x48, 0x8d, 0x05, 0x18, 0x00, 0x00, 0x00, /* lea 1f(%rip) */
> -
>    0x48, 0x89, 0x81, 0xf8, 0x00, 0x00, 0x00, /* mov %rax, 0xf8(%rcx); context.Rip */
> -
> -
>    /* flip some EFLAGS */
> -
>    0x9c,                                           /* pushf */
> -
>    /*
> -
>       0x0001 Carry flag
> -
>       0x0004 Parity flag
> -
>       0x0010 Auxiliary Carry flag
> -
>       0x0040 Zero flag
> -
>       0x0080 Sign flag
> -
>       FIXME: 0x0400 Direction flag - not changing as it breaks Wine

Nested Task surely breaks Wine; however, I cannot reproduce such breakage with Direction Flag.

https://testbot.winehq.org/JobDetails.pl?Key=141583

Therefore, I suggest that DF be included in the mask as well.

(Did you make sure to test each flag separately?)

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntdll/tests/exception.c:
> - COMPARE( Rbx );
> - COMPARE( Rbp );
> - COMPARE( Rsi );
> - COMPARE( Rdi );
> - COMPARE( R8 );
> - COMPARE( R9 );
> - COMPARE( R10 );
> - COMPARE( R11 );
> - COMPARE( R12 );
> - COMPARE( R13 );
> - COMPARE( R14 );
> - COMPARE( R15 );
> -
> - ok(memcmp(&contexts.before.Xmm0, &contexts.after.Xmm0, &contexts.before.Xmm15 - &contexts.before.Xmm0) == 0,
> -
>   "wrong some SSE register, Xmm0 %lld/%lld Xmm15 %lld/%lld", contexts.before.Xmm0.Low, contexts.after.Xmm0.Low,
> -
>   contexts.before.Xmm15.High, contexts.after.Xmm15.High);

We should test individual registers for consistency with `test_thread_context`.  You can just copy-paste code from it, with a few modifications.

Individual comparison in general leads to easier troubleshooting.  In this case, it helps with debugging potential memory corruption issues across batch winetest runs (https://test.winehq.org/data/patterns-tb-wine.html#ntdll:exception).

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntdll/tests/exception.c:
> - COMPARE( Rdx );
> - COMPARE( Rbx );
> - COMPARE( Rbp );
> - COMPARE( Rsi );
> - COMPARE( Rdi );
> - COMPARE( R8 );
> - COMPARE( R9 );
> - COMPARE( R10 );
> - COMPARE( R11 );
> - COMPARE( R12 );
> - COMPARE( R13 );
> - COMPARE( R14 );
> - COMPARE( R15 );
> -
> - ok(memcmp(&contexts.before.Xmm0, &contexts.after.Xmm0, &contexts.before.Xmm15 - &contexts.before.Xmm0) == 0,
> -
>   "wrong some SSE register, Xmm0 %lld/%lld Xmm15 %lld/%lld", contexts.before.Xmm0.Low, contexts.after.Xmm0.Low,

`%lld` is not recognized by MinGW. Use `%I64x`.

See also: https://learn.microsoft.com/en-us/cpp/c-runtime-library/format-specification-syntax-printf-and-wprintf-functions?view=msvc-170#size-prefixes-for-printf-and-wprintf-format-type-specifiers.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntdll/tests/exception.c:
> -
>    0x44, 0x0f, 0x28, 0xd0,       /* movaps %xmm0, %xmm10 */
> -
>    0x44, 0x0f, 0x28, 0xd8,       /* movaps %xmm0, %xmm11 */
> -
>    0x44, 0x0f, 0x28, 0xe0,       /* movaps %xmm0, %xmm12 */
> -
>    0x44, 0x0f, 0x28, 0xe8,       /* movaps %xmm0, %xmm13 */
> -
>    0x44, 0x0f, 0x28, 0xf0,       /* movaps %xmm0, %xmm14 */
> -
>    0x44, 0x0f, 0x28, 0xf8,       /* movaps %xmm0, %xmm15 */
> -
> -
>    /* FIXME: overwrite debug, x87 FPU and AVX registers to test those */
> -
> -
>    /* load args */
> -
>    0x48, 0x8b, 0x4c, 0x24, 0x50, /* mov 8*10(%rsp), %rcx; context   */
> -
>    0x48, 0x8b, 0x54, 0x24, 0x58, /* mov 8*11(%rsp), %rdx; alertable */
> -
>    0x48, 0x83, 0xec, 0x70,       /* sub $0x70, %rsp; change stack   */
> -
> -
>    /* setup context to return to label 1 */
> -
>    0x48, 0x8d, 0x05, 0x18, 0x00, 0x00, 0x00, /* lea 1f(%rip) */

```suggestion:-0+0
        0x48, 0x8d, 0x05, 0x18, 0x00, 0x00, 0x00, /* lea 1f(%rip), %rax */
```

Sorry, missed this one.