[PATCH 0/4] MR7564: ntdll: Fix first trap instruction when setting eflags with SetThreadContext or returning from syscall.
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/signal_x86_64.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 52c907f1a32..054dfdcf006 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2200,6 +2200,7 @@ static void usr1_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { struct syscall_frame *frame = amd64_thread_data()->syscall_frame; ULONG64 saved_compaction = 0; + I386_CONTEXT *wow_context; struct xcontext *context; context = (struct xcontext *)(((ULONG_PTR)RSP_sig(ucontext) - 128 /* red zone */ - sizeof(*context)) & ~15); @@ -2224,6 +2225,13 @@ static void usr1_handler( int signal, siginfo_t *siginfo, void *sigcontext ) context->c.ContextFlags &= ~0x40; frame->restore_flags |= 0x40; } + if ((wow_context = get_cpu_area( IMAGE_FILE_MACHINE_I386 )) + && (wow_context->ContextFlags & CONTEXT_I386_CONTROL) == CONTEXT_I386_CONTROL) + { + WOW64_CPURESERVED *cpu = NtCurrentTeb()->TlsSlots[WOW64_TLS_CPURESERVED]; + + cpu->Flags |= WOW64_CPURESERVED_FLAG_RESET_STATE; + } NtSetContextThread( GetCurrentThread(), &context->c ); } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7564
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/exception.c | 143 +++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 9b5ca2ca508..b98d040a718 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -304,6 +304,147 @@ static void test_hwbpt_in_syscall(void) RemoveVectoredExceptionHandler(handler); TlsFree(ind); } + +static void *test_single_step_exc_address; +static int test_single_step_address_test; +static volatile LONG test_single_step_address_thread_wait; +static HANDLE test_single_step_address_signal, test_single_step_address_wait; + +static LONG CALLBACK test_single_step_address_handler(EXCEPTION_POINTERS *info) +{ + EXCEPTION_RECORD *rec = info->ExceptionRecord; + + test_single_step_exc_address = rec->ExceptionAddress; + ok(rec->ExceptionCode == EXCEPTION_SINGLE_STEP, "got %#lx, address %p.\n", rec->ExceptionCode, rec->ExceptionAddress); + ExitThread(0); + ok(0, "got here.\n"); + return EXCEPTION_CONTINUE_SEARCH; +} + +static DWORD WINAPI test_single_step_address_thread(void *dummy) +{ + CONTEXT c; + + memset( code_mem, 0x90 /* nop */, 3 ); + + if (test_single_step_address_test == 1) do + { + InterlockedIncrement(&test_single_step_address_thread_wait); + } while (1); + + if (test_single_step_address_test == 3) + { + SignalObjectAndWait(test_single_step_address_signal, test_single_step_address_wait, INFINITE, FALSE); + ok(0, "got here.\n"); + } + + c.ContextFlags = CONTEXT_FULL; + GetThreadContext(GetCurrentThread(), &c); +#ifdef __x86_64__ + c.Rip = (ULONG_PTR)code_mem; +#else + c.Eip = (ULONG_PTR)code_mem; +#endif + c.EFlags |= 0x100; + if (test_single_step_address_test == 2) + NtContinue( &c, FALSE ); + else + SetThreadContext(GetCurrentThread(), &c); + ok(0, "got here.\n"); + return 0; +} + +static void test_single_step_address_run(void *handler, int test) +{ + HANDLE thread; + CONTEXT c; + + winetest_push_context("test %d", test); + + test_single_step_address_signal = CreateEventW(NULL, FALSE, FALSE, NULL); + test_single_step_address_wait = CreateEventW(NULL, FALSE, FALSE, NULL); + + test_single_step_exc_address = NULL; + test_single_step_address_test = test; + test_single_step_address_thread_wait = 0; + thread = CreateThread(NULL, 0, test_single_step_address_thread, NULL, 0, NULL); + + switch (test) + { + case 1: + case 3: + if (test == 1) + { + while (!test_single_step_address_thread_wait) + Sleep(10); + } + else + { + WaitForSingleObject(test_single_step_address_signal, INFINITE); + } + SuspendThread(thread); + c.ContextFlags = CONTEXT_FULL; + GetThreadContext(thread, &c); +#ifdef __x86_64__ + c.Rip = (ULONG_PTR)code_mem; +#else + c.Eip = (ULONG_PTR)code_mem; +#endif + c.EFlags |= 0x100; + SetThreadContext(thread, &c); + ResumeThread(thread); + if (test == 3) + SetEvent(test_single_step_address_wait); + break; + } + + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + CloseHandle(test_single_step_address_signal); + CloseHandle(test_single_step_address_wait); + + /* Vectored handler stays referenced because it execution ends with TerminateThread() and it never returns, + * decrement its reference count each time. */ + pRtlRemoveVectoredExceptionHandler(handler); + winetest_pop_context(); +} + +static void test_single_step_address(void) +{ + void *handler; + + handler = pRtlAddVectoredExceptionHandler(TRUE, test_single_step_address_handler); + ok(!!handler, "got NULL.\n"); + memset( code_mem, 0x90 /* nop */, 3 ); + + test_single_step_address_run(handler, 0); + + if (is_wow64) + ok(test_single_step_exc_address == (char *)code_mem + 1, "got %p, expected %p.\n", + test_single_step_exc_address, (char *)code_mem + 1); + else + todo_wine ok(test_single_step_exc_address == code_mem, "got %p, expected %p.\n", test_single_step_exc_address, code_mem); + + + test_single_step_address_run(handler, 1); + ok(test_single_step_exc_address == (char *)code_mem + 1, "got %p, expected %p.\n", test_single_step_exc_address, + (char *)code_mem + 1); + + test_single_step_address_run(handler, 2); + + ok(test_single_step_exc_address == (char *)code_mem + 1, "got %p, expected %p.\n", test_single_step_exc_address, + (char *)code_mem + 1); + + test_single_step_address_run(handler, 3); + + if (is_wow64) + ok(test_single_step_exc_address == (char *)code_mem + 1, "got %p, expected %p.\n", + test_single_step_exc_address, (char *)code_mem + 1); + else + todo_wine ok(test_single_step_exc_address == code_mem, "got %p, expected %p.\n", test_single_step_exc_address, code_mem); + + pRtlRemoveVectoredExceptionHandler(handler); +} #endif #ifdef __i386__ @@ -12003,6 +12144,7 @@ START_TEST(exception) test_set_live_context(); test_hwbpt_in_syscall(); test_instrumentation_callback(); + test_single_step_address(); #elif defined(__x86_64__) @@ -12035,6 +12177,7 @@ START_TEST(exception) test_hwbpt_in_syscall(); test_instrumentation_callback(); test_direct_syscalls(); + test_single_step_address(); #elif defined(__aarch64__) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7564
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/exception.c | 4 ++-- dlls/ntdll/unix/signal_x86_64.c | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index b98d040a718..44343a6927b 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -423,7 +423,7 @@ static void test_single_step_address(void) ok(test_single_step_exc_address == (char *)code_mem + 1, "got %p, expected %p.\n", test_single_step_exc_address, (char *)code_mem + 1); else - todo_wine ok(test_single_step_exc_address == code_mem, "got %p, expected %p.\n", test_single_step_exc_address, code_mem); + ok(test_single_step_exc_address == code_mem, "got %p, expected %p.\n", test_single_step_exc_address, code_mem); test_single_step_address_run(handler, 1); @@ -441,7 +441,7 @@ static void test_single_step_address(void) ok(test_single_step_exc_address == (char *)code_mem + 1, "got %p, expected %p.\n", test_single_step_exc_address, (char *)code_mem + 1); else - todo_wine ok(test_single_step_exc_address == code_mem, "got %p, expected %p.\n", test_single_step_exc_address, code_mem); + ok(test_single_step_exc_address == code_mem, "got %p, expected %p.\n", test_single_step_exc_address, code_mem); pRtlRemoveVectoredExceptionHandler(handler); } diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 054dfdcf006..ea3122bbb03 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2948,6 +2948,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "jnz 1f\n\t" /* CONTEXT_CONTROL */ "movq (%rsp),%rcx\n\t" /* frame->rip */ + "pushq %r11\n\t" + /* make sure that if trap flag is set the trap happens on the first instruction after iret */ + "popfq\n\t" "iretq\n" /* CONTEXT_INTEGER */ "1:\tmovq 0x00(%rcx),%rax\n\t" @@ -2971,6 +2974,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "testl $0x2,%edx\n\t" /* CONTEXT_INTEGER */ "jnz 1b\n\t" "xchgq %r10,(%rsp)\n\t" + "pushq %r11\n\t" + /* make sure that if trap flag is set the trap happens on the first instruction after iret */ + "popfq\n\t" "iretq\n\t" /* pop rbp-based kernel stack cfi */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7564
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/signal_x86_64.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index ea3122bbb03..26b540bd629 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2269,7 +2269,11 @@ static void sigsys_handler( int signal, siginfo_t *siginfo, void *sigcontext ) if (instrumentation_callback) frame->restore_flags |= RESTORE_FLAGS_INSTRUMENTATION; RCX_sig(ucontext) = (ULONG_PTR)frame; R11_sig(ucontext) = frame->eflags; - EFL_sig(ucontext) &= ~0x100; /* clear single-step flag */ + if (EFL_sig(ucontext) & 0x100) + { + EFL_sig(ucontext) &= ~0x100; /* clear single-step flag */ + frame->restore_flags |= CONTEXT_CONTROL; + } RIP_sig(ucontext) = (ULONG64)__wine_syscall_dispatcher_prolog_end_ptr; } #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7564
This is primarily motivated by The Finals which now won't tolerate wrong single step exception address in some specific cases. But probably it is also potentially beneficial for debuggers to have SetContextThread() to result in the correct first trap when setting the trap flag for remote thread while that thread is in a syscall (case 3 in the test). The problem here is that 'iret' intentionally results in trap not happening at instruction it returns to, only on the one following that. It serves a good purpose for NtContinue (CONTEXT_INTEGER case in syscall dispatcher) but is wrong for syscall return with trap flag set on syscall, for NtSetContextThread setting trap flag and for return to instrumentation callback. The same is for 'popf': if it sets TF it will skip next instruction after popf. But 'popf' setting TF followed by 'iret' results in trap in happening at iret return address. Patch "ntdll: Properly set context control registers from the other thread on wow64." is not exactly related to the purpose of this patchset but without that the added tests in wow64 mode fail in an unexpected way: trap flag is currently not set at all with NtSetContextThread for different thread. Patch "ntdll: Set CONTEXT_CONTROL frame restore flag in sigsys_handler()." is partially related. Currently stepping through emulated syscall will result in the trap happening inside syscall dispatcher (cf. handle_syscall_trap() which sets CONTEXT_CONTROL to avoid that). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7564#note_97639
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)