[PATCH v12 0/2] MR8466: ntdll: Also restore rbp before calling user mode callback.
If wine dlls are built with frame pointers enabled, the frame pointer will be used during unwinding. If we don't restore frame pointer before calling the user mode callback, then later when the unwinder encounters the user mode callback frame, it will set the frame pointer to something unexpected (depends on what it was during `call_user_mode_callback`). Then for the subsequent frame it adjusts the stack pointer based on the frame pointer, thus derailing the unwinding process. -- v12: ntdll: Also restore rbp before calling user mode callback. ntdll/tests: Check the context of a user callback. https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
From: Yuxuan Shui <yshui(a)codeweavers.com> Check which non-volatile registers are set to their saved values before a user callback is entered. --- dlls/ntdll/tests/exception.c | 118 +++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 025923b7d8e..1afeb97389f 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -6074,6 +6074,123 @@ static void test_direct_syscalls(void) CloseHandle(event); } +static void *unwind_target = NULL; +static void *target_frame; + +static LRESULT unwinding_wnd_proc(HWND w, UINT msg, WPARAM p2, LPARAM p3) +{ + CONTEXT context; + int frames; + UNWIND_HISTORY_TABLE table; + RUNTIME_FUNCTION *func; + ULONG_PTR frame, base; + void *data; + BOOL found = FALSE; + + RtlCaptureContext(&context); + + switch (msg) + { + case WM_NCDESTROY: + for (frames = 0; frames < 16; frames++) + { + func = RtlLookupFunctionEntry(context.Rip, &base, &table); + if (RtlVirtualUnwind(UNW_FLAG_NHANDLER, base, context.Rip, func, &context, &data, &frame, NULL)) + break; + if (!context.Rip) break; + if (!frame) break; + if (context.Rip == (DWORD64)unwind_target || frame == (DWORD64)target_frame) + { + found = TRUE; + + /* check that non-volatile registers are set properly before entering user callback. */ + todo_wine { + ok(!context.Rbx, "unexpected register value, %%rbx = %#I64x\n", context.Rbx); + ok(!context.R12, "unexpected register value, %%r12 = %#I64x\n", context.R12); + ok(!context.R13, "unexpected register value, %%r13 = %#I64x\n", context.R13); + ok(!context.R14, "unexpected register value, %%r14 = %#I64x\n", context.R14); + ok(!context.R15, "unexpected register value, %%r15 = %#I64x\n", context.R15); + ok(context.Rbp == 0xdeadbeef, "unexpected register value, %%rbp = %#I64x\n", context.Rbp); + } + + /* native seems to zero these registers, unclear whether wine should do the same. */ + break; + } + } + ok(found, "couldn't find target frame in parent frames\n"); + break; + default: break; + } + return DefWindowProcA(w, msg, p2, p3); +} + +static void test_user_callback_context(void) +{ + WNDCLASSA cls = {0}; + HWND hwnd; + ATOM atom; + UINT64 patch; + BYTE *code_mem = VirtualAlloc( NULL, 65536, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE ); + void (*pdestroy_window_trampoline)(HWND) = (void (*)(HWND))code_mem; + + /* setup register context so later we can check what changed and what hasn't. */ + static const BYTE trampoline[] = + { + 0x55, /* 00: push %rbp */ + 0x41, 0x57, /* 01: push %r15 */ + 0x41, 0x56, /* 03: push %r14 */ + 0x41, 0x55, /* 05: push %r13 */ + 0x41, 0x54, /* 07: push %r12 */ + 0x53, /* 09: push %rbx */ + 0x48, 0x83, 0xec, 0x28, /* 0a: sub 0x28,%rsp */ + 0xbd, 0xef, 0xbe, 0xad, 0xde, /* 0e: mov $0xdeadbeef,%rbp */ + 0xbb, 0xef, 0xbe, 0xad, 0xde, /* 13: mov $0xdeadbeef,%rbx */ + 0x41, 0xbc, 0xef, 0xbe, 0xad, 0xde, /* 18: mov $0xdeadbeef,%r12 */ + 0x41, 0xbd, 0xef, 0xbe, 0xad, 0xde, /* 1e: mov $0xdeadbeef,%r13 */ + 0x41, 0xbe, 0xef, 0xbe, 0xad, 0xde, /* 24: mov $0xdeadbeef,%r14 */ + 0x41, 0xbf, 0xef, 0xbe, 0xad, 0xde, /* 2a: mov $0xdeadbeef,%r15 */ + 0x48, 0xb8, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, /* 30: mov ?,%rax # &target_frame */ + 0x48, 0x89, 0x20, /* 3a: mov %rsp,(%rax) */ + 0x48, 0xb8, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, /* 3d: mov ?,%rax # DestroyWindow */ + 0xff, 0xd0, /* 47: call *%rax */ + 0x90, /* 49: nop */ + 0x48, 0x83, 0xc4, 0x28, /* 4a: add $0x28,%rsp */ + 0x5b, /* 4e: pop %rbx */ + 0x41, 0x5c, /* 4f: pop %r12 */ + 0x41, 0x5d, /* 51: pop %r13 */ + 0x41, 0x5e, /* 53: pop %r14 */ + 0x41, 0x5f, /* 55: pop %r15 */ + 0x5d, /* 57: pop %rbp */ + 0xc3, /* 58: ret */ + }; + + memcpy(code_mem, trampoline, ARRAYSIZE(trampoline)); + + patch = (ULONG_PTR)&target_frame; + memcpy(code_mem + 0x30 + 2, &patch, 8); + patch = (ULONG_PTR)DestroyWindow; + memcpy(code_mem + 0x3d + 2, &patch, 8); + + unwind_target = code_mem + 0x49; + + cls.style = CS_HREDRAW | CS_VREDRAW; + cls.lpfnWndProc = unwinding_wnd_proc; + cls.hInstance = GetModuleHandleA(0); + cls.lpszClassName = "test_user_callback_registers_class"; + + atom = RegisterClassA(&cls); + ok(!!atom, "RegisterClassA failed, error %#lx.\n", GetLastError()); + + hwnd = CreateWindowExA(WS_EX_TOPMOST, cls.lpszClassName, "", WS_POPUP | WS_VISIBLE, 100, 100, + 100, 100, NULL, NULL, 0, NULL); + ok(!!hwnd, "CreateWindowA failed, error %#lx.\n", GetLastError()); + + pdestroy_window_trampoline(hwnd); + + VirtualFree(code_mem, 0, MEM_RELEASE); + UnregisterClassA(cls.lpszClassName, GetModuleHandleA(0)); +} + #elif defined(__arm__) static void test_thread_context(void) @@ -12298,6 +12415,7 @@ START_TEST(exception) test_restore_context(); test_prot_fault(); test_dpe_exceptions(); + test_user_callback_context(); test_wow64_context(); test_nested_exception(); test_collided_unwind(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
From: Yuxuan Shui <yshui(a)codeweavers.com> If wine dlls are built with frame pointers enabled, the frame pointer will be used during unwinding. If we don't restore frame pointer before calling the user mode callback, then later when the unwinder encounters the user mode callback frame, it will set the frame pointer to something unexpected (depends on what it was during `call_user_mode_callback`). Then for the subsequent frame it adjusts the stack pointer based on the frame pointer, thus derailing the unwinding process. --- dlls/ntdll/tests/exception.c | 2 -- dlls/ntdll/unix/signal_x86_64.c | 15 +++++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 1afeb97389f..aeea6d9f685 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -6104,14 +6104,12 @@ static LRESULT unwinding_wnd_proc(HWND w, UINT msg, WPARAM p2, LPARAM p3) found = TRUE; /* check that non-volatile registers are set properly before entering user callback. */ - todo_wine { ok(!context.Rbx, "unexpected register value, %%rbx = %#I64x\n", context.Rbx); ok(!context.R12, "unexpected register value, %%r12 = %#I64x\n", context.R12); ok(!context.R13, "unexpected register value, %%r13 = %#I64x\n", context.R13); ok(!context.R14, "unexpected register value, %%r14 = %#I64x\n", context.R14); ok(!context.R15, "unexpected register value, %%r15 = %#I64x\n", context.R15); ok(context.Rbp == 0xdeadbeef, "unexpected register value, %%rbp = %#I64x\n", context.Rbp); - } /* native seems to zero these registers, unclear whether wine should do the same. */ break; diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 6ec662b9850..5ed17e0d512 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1642,8 +1642,8 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "andq $~63,%rsp\n\t" "leaq 0x10(%rbp),%rax\n\t" "movq %rax,0xa8(%rsp)\n\t" /* frame->syscall_cfa */ - "movq 0x378(%r13),%r10\n\t" /* thread_data->syscall_frame */ - "movq %r10,0xa0(%rsp)\n\t" /* frame->prev_frame */ + "movq 0x378(%r13),%r14\n\t" /* thread_data->syscall_frame */ + "movq %r14,0xa0(%rsp)\n\t" /* frame->prev_frame */ "movq %rsp,0x378(%r13)\n\t" /* thread_data->syscall_frame */ "testl $1,0x380(%r13)\n\t" /* thread_data->syscall_trace */ "jz 1f\n\t" @@ -1658,6 +1658,7 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "movq %r15,%rcx\n" /* func */ /* switch to user stack */ "1:\tmovq %rdi,%rsp\n\t" /* user_rsp */ + "movq 0x98(%r14),%rbp\n\t" /* user_frame->Rbp */ #ifdef __linux__ "movw 0x338(%r13),%ax\n" /* amd64_thread_data()->fs */ "testw %ax,%ax\n\t" @@ -1676,8 +1677,14 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "movq (%r10),%r10\n\t" "test %r10,%r10\n\t" "jz 1f\n\t" - "xchgq %rcx,%r10\n\t" - "1\t:jmpq *%rcx" ) /* func */ + "xchgq %rcx,%r10\n" + "1:\n\t" + "xor %rbx,%rbx\n\t" + "xor %r12,%r12\n\t" + "xor %r13,%r13\n\t" + "xor %r14,%r14\n\t" + "xor %r15,%r15\n\t" + "jmpq *%rcx" ) /* func */ /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
participants (2)
-
Yuxuan Shui -
Yuxuan Shui (＠yshui)