The previous implementation might have given the impression of working, as long in some cases where the PE code actually used frame pointers, but turned out to be subly wrong.
This essentially reverts the functional aspects of 1c9fdaab0f4.
Use the new value of the Lr register, after fetching the registers from unw_step, as the return value.
To make single-stepping unwinding work properly, treat the registers consistently:
- Make RtlCaptureContext store the current values of x29/Fp and x30/Lr from within the function, not the ones backed up from the stack.
- After unwinding one step, first fetch the new values of all registers, including the new value of Lr - then use this value of Lr to set the new value of Pc (the address to actually return to).
This makes the unwinding actually coherent in reading unwind opcodes and return addresses from one single function; previously these were out of sync where the return address ended up being read from the function one step further up in the call stack.
This fixes unwinding for setjmp for binaries compiled with clang (in mingw mode).
Signed-off-by: Martin Storsjo martin@martin.st --- dlls/ntdll/signal_arm64.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/dlls/ntdll/signal_arm64.c b/dlls/ntdll/signal_arm64.c index 9a7fce930d..390315bf93 100644 --- a/dlls/ntdll/signal_arm64.c +++ b/dlls/ntdll/signal_arm64.c @@ -267,8 +267,7 @@ __ASM_STDCALL_FUNC( RtlCaptureContext, 8, "stp x23, x24, [x0, #0xc0]\n\t" /* context->X23,X24 */ "stp x25, x26, [x0, #0xd0]\n\t" /* context->X25,X26 */ "stp x27, x28, [x0, #0xe0]\n\t" /* context->X27,X28 */ - "ldp x1, x2, [x29]\n\t" - "stp x1, x2, [x0, #0xf0]\n\t" /* context->Fp,Lr */ + "stp x29, x30, [x0, #0xf0]\n\t" /* context->Fp,Lr */ "add x1, x29, #0x10\n\t" "stp x1, x30, [x0, #0x100]\n\t" /* context->Sp,Pc */ "mov w1, #0x400000\n\t" /* CONTEXT_ARM64 */ @@ -547,7 +546,6 @@ static NTSTATUS libunwind_virtual_unwind( ULONG_PTR ip, ULONG_PTR *frame, CONTEX *handler = (void *)info.handler; *handler_data = (void *)info.lsda; *frame = context->Sp; - context->Pc = context->u.s.Lr; unw_get_reg( &cursor, UNW_AARCH64_X0, (unw_word_t *)&context->u.s.X0 ); unw_get_reg( &cursor, UNW_AARCH64_X1, (unw_word_t *)&context->u.s.X1 ); unw_get_reg( &cursor, UNW_AARCH64_X2, (unw_word_t *)&context->u.s.X2 ); @@ -580,6 +578,7 @@ static NTSTATUS libunwind_virtual_unwind( ULONG_PTR ip, ULONG_PTR *frame, CONTEX unw_get_reg( &cursor, UNW_AARCH64_X29, (unw_word_t *)&context->u.s.Fp ); unw_get_reg( &cursor, UNW_AARCH64_X30, (unw_word_t *)&context->u.s.Lr ); unw_get_reg( &cursor, UNW_AARCH64_SP, (unw_word_t *)&context->Sp ); + context->Pc = context->u.s.Lr;
TRACE( "next function pc=%016lx%s\n", context->Pc, rc ? "" : " (last frame)" ); TRACE(" x0=%016lx x1=%016lx x2=%016lx x3=%016lx\n",
This fixes crashes when handling GNU/mingw style SEH based C++ exceptions on arm64; in these cases unwind_full_data ended up where it tries to write handler_rva + 1 to *handler_data.
Signed-off-by: Martin Storsjo martin@martin.st --- dlls/ntdll/signal_arm64.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/signal_arm64.c b/dlls/ntdll/signal_arm64.c index 390315bf93..8227d10fbb 100644 --- a/dlls/ntdll/signal_arm64.c +++ b/dlls/ntdll/signal_arm64.c @@ -1870,7 +1870,8 @@ void WINAPI RtlUnwindEx( PVOID end_frame, PVOID target_ip, EXCEPTION_RECORD *rec dispatch.ContextRecord = context; RtlVirtualUnwind( UNW_FLAG_NHANDLER, dispatch.ImageBase, dispatch.ControlPc, dispatch.FunctionEntry, - &new_context, NULL, &frame, NULL ); + &new_context, &dispatch.HandlerData, &frame, + NULL ); rec->ExceptionFlags |= EH_COLLIDED_UNWIND; goto unwind_done; } @@ -1893,7 +1894,8 @@ void WINAPI RtlUnwindEx( PVOID end_frame, PVOID target_ip, EXCEPTION_RECORD *rec dispatch.ContextRecord = context; RtlVirtualUnwind( UNW_FLAG_NHANDLER, dispatch.ImageBase, dispatch.ControlPc, dispatch.FunctionEntry, - &new_context, NULL, &frame, NULL ); + &new_context, &dispatch.HandlerData, + &frame, NULL ); rec->ExceptionFlags |= EH_COLLIDED_UNWIND; goto unwind_done; }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=71929
Your paranoid android.
=== debiant (32 bit report) ===
ntdll: threadpool: Timeout
=== debiant (32 bit WoW report) ===
ntdll: threadpool.c:1992: Test failed: TpWaitForIoCompletion() should not return threadpool.c:2008: Test failed: wait timed out
For the CR == 3 case, x29/x30 should be restored from x29, not from sp, which may have been decremented further for local stack storage.
This fixes uwinding the stack for C++ exceptions in code generated by MSVC.
Signed-off-by: Martin Storsjo martin@martin.st --- dlls/ntdll/signal_arm64.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/signal_arm64.c b/dlls/ntdll/signal_arm64.c index 8227d10fbb..8fb67f42d5 100644 --- a/dlls/ntdll/signal_arm64.c +++ b/dlls/ntdll/signal_arm64.c @@ -1585,7 +1585,12 @@ static void *unwind_packed_data( ULONG_PTR base, ULONG_PTR pc, RUNTIME_FUNCTION
if (!skip) { - if (func->u.s.CR == 3) restore_regs( 29, 2, 0, context, ptrs ); + if (func->u.s.CR == 3) + { + DWORD64 *fp = (DWORD64 *) context->u.s.Fp; /* u.X[29] */ + context->u.X[29] = fp[0]; + context->u.X[30] = fp[1]; + } context->Sp += local_size; if (fp_size) restore_fpregs( 8, fp_size / 8, int_size, context, ptrs ); if (func->u.s.CR == 1) restore_regs( 30, 1, int_size - 8, context, ptrs );
-
Martin Storsjo -
Marvin