[PATCH v2 0/3] MR9149: ntdll: Fix clearing nested exception flag on x64.
-- v2: ntdll: Clear nested exception flag at correct frame on x64. https://gitlab.winehq.org/wine/wine/-/merge_requests/9149
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/exception.c | 61 ++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 122cd592238..1057cda292a 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -5482,22 +5482,29 @@ static void test_KiUserCallbackDispatcher(void) static BOOL got_nested_exception, got_prev_frame_exception; static void *nested_exception_initial_frame; +static void *nested_exception_crash_frame; static DWORD nested_exception_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRATION_RECORD *frame, CONTEXT *context, EXCEPTION_REGISTRATION_RECORD **dispatcher) { - trace("nested_exception_handler Rip %p, Rsp %p, code %#lx, flags %#lx, ExceptionAddress %p.\n", - (void *)context->Rip, (void *)context->Rsp, rec->ExceptionCode, rec->ExceptionFlags, rec->ExceptionAddress); + trace("nested_exception_handler Rip %p, Rsp %p, code %#lx, flags %#lx, ExceptionAddress %p, frame %p.\n", + (void *)context->Rip, (void *)context->Rsp, rec->ExceptionCode, rec->ExceptionFlags, + rec->ExceptionAddress, frame); + + if (!nested_exception_initial_frame) + nested_exception_initial_frame = frame; + if (frame == nested_exception_initial_frame) + return ExceptionContinueSearch; if (rec->ExceptionCode == 0x80000003 && !(rec->ExceptionFlags & EXCEPTION_NESTED_CALL)) { ok(rec->NumberParameters == 1, "Got unexpected rec->NumberParameters %lu.\n", rec->NumberParameters); - ok((void *)context->Rsp == frame, "Got unexpected frame %p.\n", frame); + ok((char *)context->Rsp + 8 == (char *)frame, "Got unexpected frame %p.\n", frame); ok(*(void **)frame == (char *)code_mem + 5, "Got unexpected *frame %p.\n", *(void **)frame); - ok(context->Rip == (ULONG_PTR)((char *)code_mem + 7), "Got unexpected Rip %#Ix.\n", context->Rip); + ok(context->Rip == (ULONG_PTR)((char *)code_mem + 14), "Got unexpected Rip %#Ix.\n", context->Rip); - nested_exception_initial_frame = frame; + nested_exception_crash_frame = frame; RaiseException(0xdeadbeef, 0, 0, 0); ++context->Rip; return ExceptionContinueExecution; @@ -5508,25 +5515,57 @@ static DWORD nested_exception_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRAT { ok(!rec->NumberParameters, "Got unexpected rec->NumberParameters %lu.\n", rec->NumberParameters); got_nested_exception = TRUE; - ok(frame == nested_exception_initial_frame, "Got unexpected frame %p.\n", frame); + ok(frame == nested_exception_crash_frame, "Got unexpected frame %p.\n", frame); return ExceptionContinueSearch; } ok(rec->ExceptionCode == 0xdeadbeef && (!rec->ExceptionFlags || rec->ExceptionFlags == EXCEPTION_SOFTWARE_ORIGINATE), "Got unexpected exception code %#lx, flags %#lx.\n", rec->ExceptionCode, rec->ExceptionFlags); ok(!rec->NumberParameters, "Got unexpected rec->NumberParameters %lu.\n", rec->NumberParameters); - ok(frame == (void *)((BYTE *)nested_exception_initial_frame + 8), + todo_wine ok(frame == (void *)((BYTE *)nested_exception_crash_frame + 8), "Got unexpected frame %p.\n", frame); got_prev_frame_exception = TRUE; return ExceptionContinueExecution; } +static DWORD nested_exception_handler2(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRATION_RECORD *frame, + CONTEXT *context, EXCEPTION_REGISTRATION_RECORD **dispatcher) +{ + trace("nested_exception_handler2 Rip %p, Rsp %p, code %#lx, flags %#lx, ExceptionAddress %p, frame %p.\n", + (void *)context->Rip, (void *)context->Rsp, rec->ExceptionCode, rec->ExceptionFlags, + rec->ExceptionAddress, frame); + + ok(rec->ExceptionCode == 0x80000003, "got %#lx.\n", rec->ExceptionCode); + if (!nested_exception_initial_frame) + nested_exception_initial_frame = frame; + if (nested_exception_initial_frame == frame) + { + ((DISPATCHER_CONTEXT *)dispatcher)->EstablisherFrame = (ULONG_PTR)nested_exception_initial_frame + 0x10; + ok(!(rec->ExceptionFlags & EXCEPTION_NESTED_CALL), "got %#lx.\n", rec->ExceptionFlags); + return ExceptionNestedException; + } + if ((char *)frame == (char *)nested_exception_initial_frame + 0x8) + { + ok((rec->ExceptionFlags & EXCEPTION_NESTED_CALL), "got %#lx.\n", rec->ExceptionFlags); + return ExceptionContinueSearch; + } + ok((char *)frame == (char *)nested_exception_initial_frame + 0x10, "got frame %p, nested_exception_initial_frame %p.\n", + frame, nested_exception_initial_frame); + todo_wine ok((rec->ExceptionFlags & EXCEPTION_NESTED_CALL), "got %#lx.\n", rec->ExceptionFlags); + ++context->Rip; + return ExceptionContinueExecution; +} + static const BYTE nested_except_code[] = { 0xe8, 0x02, 0x00, 0x00, 0x00, /* call nest */ 0x90, /* nop */ 0xc3, /* ret */ /* nest: */ + 0xe8, 0x02, 0x00, 0x00, 0x00, /* call nest2 */ + 0x90, /* nop */ + 0xc3, /* ret */ + /* nest2: */ 0xcc, /* int3 */ 0x90, /* nop */ 0xc3, /* ret */ @@ -5535,9 +5574,13 @@ static const BYTE nested_except_code[] = static void test_nested_exception(void) { got_nested_exception = got_prev_frame_exception = FALSE; + nested_exception_initial_frame = NULL; run_exception_test(nested_exception_handler, NULL, nested_except_code, sizeof(nested_except_code), PAGE_EXECUTE_READ); - ok(got_nested_exception, "Did not get nested exception.\n"); + todo_wine ok(got_nested_exception, "Did not get nested exception.\n"); ok(got_prev_frame_exception, "Did not get nested exception in the previous frame.\n"); + + nested_exception_initial_frame = NULL; + run_exception_test(nested_exception_handler2, NULL, nested_except_code, sizeof(nested_except_code), PAGE_EXECUTE_READ); } static unsigned int collided_unwind_exception_count; @@ -5563,7 +5606,7 @@ static DWORD collided_exception_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGISTR case 1: ok(rec->ExceptionCode == STATUS_UNWIND, "got %#lx.\n", rec->ExceptionCode); ok(rec->ExceptionFlags == EXCEPTION_UNWINDING, "got %#lx.\n", rec->ExceptionFlags); - ok((char *)context->Rip == (char *)code_mem + 7, "got %p.\n", rec->ExceptionAddress); + ok((char *)context->Rip == (char *)code_mem + 14, "got %p, expected %p.\n", (char *)context->Rip, (char *)code_mem + 14); /* generate exception in unwind handler. */ RaiseException(0xdeadbeef, 0, 0, 0); ok(0, "shouldn't be reached\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9149
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/exception.c | 10 ---------- dlls/ntdll/ntdll_misc.h | 2 -- dlls/ntdll/signal_arm.c | 11 +++++++++++ dlls/ntdll/signal_arm64.c | 13 +++++++++++++ dlls/ntdll/signal_arm64ec.c | 11 +++++++++++ dlls/ntdll/signal_x86_64.c | 11 +++++++++++ 6 files changed, 46 insertions(+), 12 deletions(-) diff --git a/dlls/ntdll/exception.c b/dlls/ntdll/exception.c index 2284333668a..691781017a2 100644 --- a/dlls/ntdll/exception.c +++ b/dlls/ntdll/exception.c @@ -302,16 +302,6 @@ NTSTATUS WINAPI dispatch_user_callback( void *args, ULONG len, ULONG id ) #endif -/******************************************************************* - * nested_exception_handler - */ -EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, - CONTEXT *context, void *dispatch ) -{ - if (rec->ExceptionFlags & (EXCEPTION_UNWINDING | EXCEPTION_EXIT_UNWIND)) return ExceptionContinueSearch; - return ExceptionNestedException; -} - /******************************************************************* * raise_status diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index 6fcc61149f4..1ac221bf156 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -57,8 +57,6 @@ extern NTSTATUS WINAPI dispatch_exception( EXCEPTION_RECORD *rec, CONTEXT *conte extern NTSTATUS WINAPI dispatch_user_callback( void *args, ULONG len, ULONG id ); extern EXCEPTION_DISPOSITION WINAPI user_callback_handler( EXCEPTION_RECORD *record, void *frame, CONTEXT *context, void *dispatch ); -extern EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, - CONTEXT *context, void *dispatch ); extern void DECLSPEC_NORETURN raise_status( NTSTATUS status, EXCEPTION_RECORD *rec ); extern LONG WINAPI call_unhandled_exception_filter( PEXCEPTION_POINTERS eptr ); extern void WINAPI process_breakpoint(void); diff --git a/dlls/ntdll/signal_arm.c b/dlls/ntdll/signal_arm.c index aedb20222c4..9f4cc940375 100644 --- a/dlls/ntdll/signal_arm.c +++ b/dlls/ntdll/signal_arm.c @@ -148,6 +148,17 @@ EXCEPTION_DISPOSITION WINAPI unwind_exception_handler( EXCEPTION_RECORD *record, } +/******************************************************************* + * nested_exception_handler + */ +EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, + CONTEXT *context, void *dispatch ) +{ + if (rec->ExceptionFlags & (EXCEPTION_UNWINDING | EXCEPTION_EXIT_UNWIND)) return ExceptionContinueSearch; + return ExceptionNestedException; +} + + /********************************************************************** * call_unwind_handler */ diff --git a/dlls/ntdll/signal_arm64.c b/dlls/ntdll/signal_arm64.c index bc981289ea5..9096f331dbc 100644 --- a/dlls/ntdll/signal_arm64.c +++ b/dlls/ntdll/signal_arm64.c @@ -182,6 +182,19 @@ __ASM_GLOBAL_FUNC( call_unwind_handler, "ret" ) + + +/******************************************************************* + * nested_exception_handler + */ +EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, + CONTEXT *context, void *dispatch ) +{ + if (rec->ExceptionFlags & (EXCEPTION_UNWINDING | EXCEPTION_EXIT_UNWIND)) return ExceptionContinueSearch; + return ExceptionNestedException; +} + + /*********************************************************************** * call_seh_handler */ diff --git a/dlls/ntdll/signal_arm64ec.c b/dlls/ntdll/signal_arm64ec.c index a93aae89554..9cbc6b48a2b 100644 --- a/dlls/ntdll/signal_arm64ec.c +++ b/dlls/ntdll/signal_arm64ec.c @@ -1099,6 +1099,17 @@ static DWORD __attribute__((naked)) call_unwind_handler( EXCEPTION_RECORD *rec, } +/******************************************************************* + * nested_exception_handler + */ +EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, + CONTEXT *context, void *dispatch ) +{ + if (rec->ExceptionFlags & (EXCEPTION_UNWINDING | EXCEPTION_EXIT_UNWIND)) return ExceptionContinueSearch; + return ExceptionNestedException; +} + + /*********************************************************************** * call_seh_handler */ diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 1f184f4ea58..ae16bb77dae 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -169,6 +169,17 @@ __ASM_GLOBAL_FUNC( RtlCaptureContext, "ret" ); +/******************************************************************* + * nested_exception_handler + */ +EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, + CONTEXT *context, void *dispatch ) +{ + if (rec->ExceptionFlags & (EXCEPTION_UNWINDING | EXCEPTION_EXIT_UNWIND)) return ExceptionContinueSearch; + return ExceptionNestedException; +} + + /*********************************************************************** * call_seh_handler */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9149
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/signal_x86_64.c | 47 ++++++++++++++++++++++-------------- dlls/ntdll/tests/exception.c | 6 ++--- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index ae16bb77dae..3d4bd23e8ff 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -169,13 +169,27 @@ __ASM_GLOBAL_FUNC( RtlCaptureContext, "ret" ); +struct unwind_exception_frame +{ + EXCEPTION_REGISTRATION_RECORD frame; + char dummy[0x10]; /* Layout 'dispatch' accessed from unwind_exception_handler() so it is above register + * save space when .seh handler is used. */ + DISPATCHER_CONTEXT *dispatch; +}; +C_ASSERT( sizeof(struct unwind_exception_frame) == 0x28 ); +C_ASSERT( offsetof(struct unwind_exception_frame, dispatch) == 0x20 ); + + /******************************************************************* * nested_exception_handler */ EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, - CONTEXT *context, void *dispatch ) + CONTEXT *context, DISPATCHER_CONTEXT *dispatch ) { + struct unwind_exception_frame *unwind_frame = (struct unwind_exception_frame *)frame; + if (rec->ExceptionFlags & (EXCEPTION_UNWINDING | EXCEPTION_EXIT_UNWIND)) return ExceptionContinueSearch; + dispatch->EstablisherFrame = unwind_frame->dispatch->EstablisherFrame; return ExceptionNestedException; } @@ -191,6 +205,7 @@ __ASM_GLOBAL_FUNC( call_seh_handler, ".seh_stackalloc 0x28\n\t" ".seh_endprologue\n\t" ".seh_handler nested_exception_handler, @except\n\t" + "movq %r9,0x20(%rsp)\n\t" /* unwind_exception_frame->dispatch */ "callq *0x50(%rsp)\n\t" /* handler */ "nop\n\t" /* avoid epilogue so handler is called */ "addq $0x28, %rsp\n\t" @@ -199,13 +214,14 @@ __ASM_GLOBAL_FUNC( call_seh_handler, static DWORD call_seh_handler( EXCEPTION_RECORD *rec, ULONG_PTR frame, CONTEXT *context, void *dispatch, PEXCEPTION_ROUTINE handler ) { - EXCEPTION_REGISTRATION_RECORD wrapper_frame; + struct unwind_exception_frame wrapper_frame; DWORD res; - wrapper_frame.Handler = (PEXCEPTION_HANDLER)nested_exception_handler; - __wine_push_frame( &wrapper_frame ); + wrapper_frame.frame.Handler = (PEXCEPTION_HANDLER)nested_exception_handler; + wrapper_frame.dispatch = dispatch; + __wine_push_frame( &wrapper_frame.frame ); res = handler( rec, (void *)frame, context, dispatch ); - __wine_pop_frame( &wrapper_frame ); + __wine_pop_frame( &wrapper_frame.frame ); return res; } #endif @@ -223,7 +239,7 @@ NTSTATUS call_seh_handlers( EXCEPTION_RECORD *rec, CONTEXT *orig_context ) DISPATCHER_CONTEXT dispatch; CONTEXT context; NTSTATUS status; - ULONG_PTR frame; + ULONG_PTR frame, nested_frame; DWORD res; context = *orig_context; @@ -232,6 +248,7 @@ NTSTATUS call_seh_handlers( EXCEPTION_RECORD *rec, CONTEXT *orig_context ) dispatch.TargetIp = 0; dispatch.ContextRecord = &context; dispatch.HistoryTable = &table; + nested_frame = 0; for (;;) { status = virtual_unwind( UNW_FLAG_EHANDLER, &dispatch, &context ); @@ -254,7 +271,8 @@ NTSTATUS call_seh_handlers( EXCEPTION_RECORD *rec, CONTEXT *orig_context ) dispatch.LanguageHandler, rec, dispatch.EstablisherFrame, orig_context, &dispatch ); res = call_seh_handler( rec, dispatch.EstablisherFrame, orig_context, &dispatch, dispatch.LanguageHandler ); - rec->ExceptionFlags &= EXCEPTION_NONCONTINUABLE; + if (dispatch.EstablisherFrame == nested_frame) + rec->ExceptionFlags &= EXCEPTION_NONCONTINUABLE; TRACE( "handler at %p returned %lu\n", dispatch.LanguageHandler, res ); switch (res) @@ -266,6 +284,7 @@ NTSTATUS call_seh_handlers( EXCEPTION_RECORD *rec, CONTEXT *orig_context ) break; case ExceptionNestedException: rec->ExceptionFlags |= EXCEPTION_NESTED_CALL; + nested_frame = dispatch.EstablisherFrame; TRACE( "nested exception\n" ); break; case ExceptionCollidedUnwind: @@ -283,6 +302,8 @@ NTSTATUS call_seh_handlers( EXCEPTION_RECORD *rec, CONTEXT *orig_context ) teb_frame->Handler, rec, teb_frame, orig_context, &dispatch, context.Rsp ); res = call_seh_handler( rec, (ULONG_PTR)teb_frame, orig_context, &dispatch, (PEXCEPTION_ROUTINE)teb_frame->Handler ); + if (dispatch.EstablisherFrame == nested_frame) + rec->ExceptionFlags &= EXCEPTION_NONCONTINUABLE; TRACE( "TEB handler at %p returned %lu\n", teb_frame->Handler, res ); switch (res) @@ -294,6 +315,7 @@ NTSTATUS call_seh_handlers( EXCEPTION_RECORD *rec, CONTEXT *orig_context ) break; case ExceptionNestedException: rec->ExceptionFlags |= EXCEPTION_NESTED_CALL; + nested_frame = dispatch.EstablisherFrame; TRACE( "nested exception\n" ); break; case ExceptionCollidedUnwind: @@ -435,17 +457,6 @@ BOOLEAN WINAPI RtlIsEcCode( ULONG_PTR ptr ) } -struct unwind_exception_frame -{ - EXCEPTION_REGISTRATION_RECORD frame; - char dummy[0x10]; /* Layout 'dispatch' accessed from unwind_exception_handler() so it is above register - * save space when .seh handler is used. */ - DISPATCHER_CONTEXT *dispatch; -}; -C_ASSERT( sizeof(struct unwind_exception_frame) == 0x28 ); -C_ASSERT( offsetof(struct unwind_exception_frame, dispatch) == 0x20 ); - - /********************************************************************** * unwind_exception_handler * diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index 1057cda292a..c071aa36f65 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -5522,7 +5522,7 @@ static DWORD nested_exception_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRAT ok(rec->ExceptionCode == 0xdeadbeef && (!rec->ExceptionFlags || rec->ExceptionFlags == EXCEPTION_SOFTWARE_ORIGINATE), "Got unexpected exception code %#lx, flags %#lx.\n", rec->ExceptionCode, rec->ExceptionFlags); ok(!rec->NumberParameters, "Got unexpected rec->NumberParameters %lu.\n", rec->NumberParameters); - todo_wine ok(frame == (void *)((BYTE *)nested_exception_crash_frame + 8), + ok(frame == (void *)((BYTE *)nested_exception_crash_frame + 8), "Got unexpected frame %p.\n", frame); got_prev_frame_exception = TRUE; return ExceptionContinueExecution; @@ -5551,7 +5551,7 @@ static DWORD nested_exception_handler2(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRA } ok((char *)frame == (char *)nested_exception_initial_frame + 0x10, "got frame %p, nested_exception_initial_frame %p.\n", frame, nested_exception_initial_frame); - todo_wine ok((rec->ExceptionFlags & EXCEPTION_NESTED_CALL), "got %#lx.\n", rec->ExceptionFlags); + ok((rec->ExceptionFlags & EXCEPTION_NESTED_CALL), "got %#lx.\n", rec->ExceptionFlags); ++context->Rip; return ExceptionContinueExecution; } @@ -5576,7 +5576,7 @@ static void test_nested_exception(void) got_nested_exception = got_prev_frame_exception = FALSE; nested_exception_initial_frame = NULL; run_exception_test(nested_exception_handler, NULL, nested_except_code, sizeof(nested_except_code), PAGE_EXECUTE_READ); - todo_wine ok(got_nested_exception, "Did not get nested exception.\n"); + ok(got_nested_exception, "Did not get nested exception.\n"); ok(got_prev_frame_exception, "Did not get nested exception in the previous frame.\n"); nested_exception_initial_frame = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9149
v2: - use the same frame layout as in unwind handler. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9149#note_118213
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)