[PATCH 0/3] MR9149: ntdll: Fix clearing nested exception flag on x64.
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 | 41 +++++++++++++++++++++++++++--------- dlls/ntdll/tests/exception.c | 6 +++--- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index ae16bb77dae..63ed4c6a096 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -169,13 +169,20 @@ __ASM_GLOBAL_FUNC( RtlCaptureContext, "ret" ); +struct call_seh_handler_frame +{ + ULONG64 unused[4]; + ULONG64 establisher_frame; +}; + /******************************************************************* * nested_exception_handler */ -EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, void *frame, - CONTEXT *context, void *dispatch ) +EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, struct call_seh_handler_frame *frame, + CONTEXT *context, DISPATCHER_CONTEXT *dispatch ) { if (rec->ExceptionFlags & (EXCEPTION_UNWINDING | EXCEPTION_EXIT_UNWIND)) return ExceptionContinueSearch; + dispatch->EstablisherFrame = frame->establisher_frame; return ExceptionNestedException; } @@ -187,8 +194,10 @@ EXCEPTION_DISPOSITION WINAPI nested_exception_handler( EXCEPTION_RECORD *rec, vo DWORD WINAPI call_seh_handler( EXCEPTION_RECORD *rec, ULONG_PTR frame, CONTEXT *context, void *dispatch, PEXCEPTION_ROUTINE handler ); __ASM_GLOBAL_FUNC( call_seh_handler, - "subq $0x28, %rsp\n\t" - ".seh_stackalloc 0x28\n\t" + "pushq %rdx\n\t" /* call_seh_handler_frame.establisher_frame <- frame */ + ".seh_pushreg %rdx\n\t" + "subq $0x20, %rsp\n\t" + ".seh_stackalloc 0x20\n\t" ".seh_endprologue\n\t" ".seh_handler nested_exception_handler, @except\n\t" "callq *0x50(%rsp)\n\t" /* handler */ @@ -199,13 +208,19 @@ __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; + union + { + struct call_seh_handler_frame seh_handler_frame; + EXCEPTION_REGISTRATION_RECORD 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.seh_handler_frame.establisher_frame = frame; + __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 +238,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 +247,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 +270,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 +283,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 +301,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 +314,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: 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
Nested exception flag is supposed to be cleared when the nested exception unwind goes up the handler which had an exception during its call. The actual crash could happen in inner frame with own exception handlers (which may return ExceptionContinueSearch), currently nested flag will be cleared after the innermost handler is called. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9149#note_118209
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)