[PATCH 0/1] MR4931: ntdll: Remove libunwind support for aarch64.
Libunwind hasn't been necessary for unwinding through the ELF bits since 03d4ba67f4a0b75548f27eb4bf1c4715888de07d. This reduces the number of potential build configurations to keep track of. Signed-off-by: Martin Storsjö <martin(a)martin.st> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931
From: Martin Storsjö <martin(a)martin.st> Libunwind hasn't been necessary for unwinding through the ELF bits since 03d4ba67f4a0b75548f27eb4bf1c4715888de07d. This reduces the number of potential build configurations to keep track of. Signed-off-by: Martin Storsjö <martin(a)martin.st> --- dlls/ntdll/unix/signal_arm64.c | 153 ++------------------------------- 1 file changed, 8 insertions(+), 145 deletions(-) diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index f96ec330796..62886c74515 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -50,10 +50,6 @@ #ifdef HAVE_SYS_UCONTEXT_H # include <sys/ucontext.h> #endif -#ifdef HAVE_LIBUNWIND -# define UNW_LOCAL_ONLY -# include <libunwind.h> -#endif #include "ntstatus.h" #define WIN32_NO_STATUS @@ -332,140 +328,6 @@ static NTSTATUS dwarf_virtual_unwind( ULONG64 ip, ULONG64 *frame, CONTEXT *conte } -#ifdef HAVE_LIBUNWIND -static NTSTATUS libunwind_virtual_unwind( ULONG_PTR ip, ULONG_PTR *frame, CONTEXT *context, - PEXCEPTION_ROUTINE *handler, void **handler_data ) -{ - unw_context_t unw_context; - unw_cursor_t cursor; - unw_proc_info_t info; - int rc; - -#ifdef __APPLE__ - rc = unw_getcontext( &unw_context ); - if (rc == UNW_ESUCCESS) - rc = unw_init_local( &cursor, &unw_context ); - if (rc == UNW_ESUCCESS) - { - int i; - for (i = 0; i <= 28; i++) - unw_set_reg( &cursor, UNW_ARM64_X0 + i, context->X[i] ); - unw_set_reg( &cursor, UNW_ARM64_FP, context->Fp ); - unw_set_reg( &cursor, UNW_ARM64_LR, context->Lr ); - unw_set_reg( &cursor, UNW_ARM64_SP, context->Sp ); - unw_set_reg( &cursor, UNW_REG_IP, context->Pc ); - } -#else - memcpy( unw_context.uc_mcontext.regs, context->X, sizeof(context->X) ); - unw_context.uc_mcontext.sp = context->Sp; - unw_context.uc_mcontext.pc = context->Pc; - - rc = unw_init_local( &cursor, &unw_context ); -#endif - if (rc != UNW_ESUCCESS) - { - WARN( "setup failed: %d\n", rc ); - return STATUS_INVALID_DISPOSITION; - } - rc = unw_get_proc_info( &cursor, &info ); - if (UNW_ENOINFO < 0) rc = -rc; /* LLVM libunwind has negative error codes */ - if (rc != UNW_ESUCCESS && rc != -UNW_ENOINFO) - { - WARN( "failed to get info: %d\n", rc ); - return STATUS_INVALID_DISPOSITION; - } - if (rc == -UNW_ENOINFO || ip < info.start_ip || ip > info.end_ip) - { - TRACE( "no info found for %lx ip %lx-%lx, assuming leaf function\n", - ip, info.start_ip, info.end_ip ); - *handler = NULL; - *frame = context->Sp; - context->Pc = context->Lr; - context->ContextFlags |= CONTEXT_UNWOUND_TO_CALL; - return STATUS_SUCCESS; - } - - TRACE( "ip %#lx function %#lx-%#lx personality %#lx lsda %#lx fde %#lx\n", - ip, (unsigned long)info.start_ip, (unsigned long)info.end_ip, (unsigned long)info.handler, - (unsigned long)info.lsda, (unsigned long)info.unwind_info ); - - rc = unw_step( &cursor ); - if (rc < 0) - { - WARN( "failed to unwind: %d %d\n", rc, UNW_ENOINFO ); - return STATUS_INVALID_DISPOSITION; - } - - *handler = (void *)info.handler; - *handler_data = (void *)info.lsda; - *frame = context->Sp; -#ifdef __APPLE__ - { - int i; - for (i = 0; i <= 28; i++) - unw_get_reg( &cursor, UNW_ARM64_X0 + i, (unw_word_t *)&context->X[i] ); - } - unw_get_reg( &cursor, UNW_ARM64_FP, (unw_word_t *)&context->Fp ); - unw_get_reg( &cursor, UNW_ARM64_X30, (unw_word_t *)&context->Lr ); - unw_get_reg( &cursor, UNW_ARM64_SP, (unw_word_t *)&context->Sp ); -#else - unw_get_reg( &cursor, UNW_AARCH64_X0, (unw_word_t *)&context->X0 ); - unw_get_reg( &cursor, UNW_AARCH64_X1, (unw_word_t *)&context->X1 ); - unw_get_reg( &cursor, UNW_AARCH64_X2, (unw_word_t *)&context->X2 ); - unw_get_reg( &cursor, UNW_AARCH64_X3, (unw_word_t *)&context->X3 ); - unw_get_reg( &cursor, UNW_AARCH64_X4, (unw_word_t *)&context->X4 ); - unw_get_reg( &cursor, UNW_AARCH64_X5, (unw_word_t *)&context->X5 ); - unw_get_reg( &cursor, UNW_AARCH64_X6, (unw_word_t *)&context->X6 ); - unw_get_reg( &cursor, UNW_AARCH64_X7, (unw_word_t *)&context->X7 ); - unw_get_reg( &cursor, UNW_AARCH64_X8, (unw_word_t *)&context->X8 ); - unw_get_reg( &cursor, UNW_AARCH64_X9, (unw_word_t *)&context->X9 ); - unw_get_reg( &cursor, UNW_AARCH64_X10, (unw_word_t *)&context->X10 ); - unw_get_reg( &cursor, UNW_AARCH64_X11, (unw_word_t *)&context->X11 ); - unw_get_reg( &cursor, UNW_AARCH64_X12, (unw_word_t *)&context->X12 ); - unw_get_reg( &cursor, UNW_AARCH64_X13, (unw_word_t *)&context->X13 ); - unw_get_reg( &cursor, UNW_AARCH64_X14, (unw_word_t *)&context->X14 ); - unw_get_reg( &cursor, UNW_AARCH64_X15, (unw_word_t *)&context->X15 ); - unw_get_reg( &cursor, UNW_AARCH64_X16, (unw_word_t *)&context->X16 ); - unw_get_reg( &cursor, UNW_AARCH64_X17, (unw_word_t *)&context->X17 ); - unw_get_reg( &cursor, UNW_AARCH64_X18, (unw_word_t *)&context->X18 ); - unw_get_reg( &cursor, UNW_AARCH64_X19, (unw_word_t *)&context->X19 ); - unw_get_reg( &cursor, UNW_AARCH64_X20, (unw_word_t *)&context->X20 ); - unw_get_reg( &cursor, UNW_AARCH64_X21, (unw_word_t *)&context->X21 ); - unw_get_reg( &cursor, UNW_AARCH64_X22, (unw_word_t *)&context->X22 ); - unw_get_reg( &cursor, UNW_AARCH64_X23, (unw_word_t *)&context->X23 ); - unw_get_reg( &cursor, UNW_AARCH64_X24, (unw_word_t *)&context->X24 ); - unw_get_reg( &cursor, UNW_AARCH64_X25, (unw_word_t *)&context->X25 ); - unw_get_reg( &cursor, UNW_AARCH64_X26, (unw_word_t *)&context->X26 ); - unw_get_reg( &cursor, UNW_AARCH64_X27, (unw_word_t *)&context->X27 ); - unw_get_reg( &cursor, UNW_AARCH64_X28, (unw_word_t *)&context->X28 ); - unw_get_reg( &cursor, UNW_AARCH64_X29, (unw_word_t *)&context->Fp ); - unw_get_reg( &cursor, UNW_AARCH64_X30, (unw_word_t *)&context->Lr ); - unw_get_reg( &cursor, UNW_AARCH64_SP, (unw_word_t *)&context->Sp ); -#endif - unw_get_reg( &cursor, UNW_REG_IP, (unw_word_t *)&context->Pc ); - context->ContextFlags |= CONTEXT_UNWOUND_TO_CALL; - - TRACE( "next function pc=%016lx%s\n", context->Pc, rc ? "" : " (last frame)" ); - TRACE(" x0=%016lx x1=%016lx x2=%016lx x3=%016lx\n", - context->X0, context->X1, context->X2, context->X3 ); - TRACE(" x4=%016lx x5=%016lx x6=%016lx x7=%016lx\n", - context->X4, context->X5, context->X6, context->X7 ); - TRACE(" x8=%016lx x9=%016lx x10=%016lx x11=%016lx\n", - context->X8, context->X9, context->X10, context->X11 ); - TRACE(" x12=%016lx x13=%016lx x14=%016lx x15=%016lx\n", - context->X12, context->X13, context->X14, context->X15 ); - TRACE(" x16=%016lx x17=%016lx x18=%016lx x19=%016lx\n", - context->X16, context->X17, context->X18, context->X19 ); - TRACE(" x20=%016lx x21=%016lx x22=%016lx x23=%016lx\n", - context->X20, context->X21, context->X22, context->X23 ); - TRACE(" x24=%016lx x25=%016lx x26=%016lx x27=%016lx\n", - context->X24, context->X25, context->X26, context->X27 ); - TRACE(" x28=%016lx fp=%016lx lr=%016lx sp=%016lx\n", - context->X28, context->Fp, context->Lr, context->Sp ); - return STATUS_SUCCESS; -} -#endif - /*********************************************************************** * unwind_builtin_dll * @@ -482,13 +344,14 @@ NTSTATUS unwind_builtin_dll( void *args ) if (fde) return dwarf_virtual_unwind( context->Pc, &dispatch->EstablisherFrame, context, fde, &bases, &dispatch->LanguageHandler, &dispatch->HandlerData ); -#ifdef HAVE_LIBUNWIND - return libunwind_virtual_unwind( context->Pc, &dispatch->EstablisherFrame, context, - &dispatch->LanguageHandler, &dispatch->HandlerData ); -#else - ERR("libunwind not available, unable to unwind\n"); - return STATUS_INVALID_DISPOSITION; -#endif + + TRACE( "no info found for %lx, assuming leaf function\n", + context->Pc ); + dispatch->LanguageHandler = NULL; + dispatch->EstablisherFrame = context->Sp; + context->Pc = context->Lr; + context->ContextFlags |= CONTEXT_UNWOUND_TO_CALL; + return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4931
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=142329 Your paranoid android. === debian11b (64 bit WoW report) === mf: Unhandled exception: divide by zero in 64-bit code (0x0000000042e22c).
Can we still support macOS libraries? Some sources claim that [DWARF unwinding information may be unavailable for functions that have unwinding information encoded in Apple compact unwinding format](<https://faultlore.com/blah/compact-unwinding/>):
Clang also started preferentially emitting that format over DWARF CFI on Apple’s platforms. So to generate good backtraces on Apple platforms, you need to be able to parse and interpret compact unwinding tables.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931#note_58849
On Wed Jan 24 18:20:38 2024 +0000, Jinoh Kang wrote:
Can we still support macOS libraries? Some sources claim that [DWARF unwinding information may be unavailable for functions that have unwinding information encoded in Apple compact unwinding format](<https://faultlore.com/blah/compact-unwinding/>):
Clang also started preferentially emitting that format over DWARF CFI on Apple’s platforms. So to generate good backtraces on Apple platforms, you need to be able to parse and interpret compact unwinding tables. Thanks; that’s probably quite true - in that case we probably shouldn’t get rid of libunwind here, until we have a complete enough unwinder for the compact unwinding format - or until we have better knowledge on the situation (e.g. if we could conclude with some certainty that we don’t need to unwind through functions with such data - or if we could disable the compact unwinding format for the unixlibs; the amount of non-PE code that the unwinder needs to handle is pretty small these days).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931#note_58862
On Wed Jan 24 18:20:38 2024 +0000, Martin Storsjö wrote:
Thanks; that’s probably quite true - in that case we probably shouldn’t get rid of libunwind here, until we have a complete enough unwinder for the compact unwinding format - or until we have better knowledge on the situation (e.g. if we could conclude with some certainty that we don’t need to unwind through functions with such data - or if we could disable the compact unwinding format for the unixlibs; the amount of non-PE code that the unwinder needs to handle is pretty small these days). We are never unwinding Unix libs. libunwind is only used for .dll.so files, which don't exist on aarch64.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931#note_58869
On Wed Jan 24 19:46:03 2024 +0000, Alexandre Julliard wrote:
We are never unwinding Unix libs. libunwind is only used for .dll.so files, which don't exist on aarch64. Hmm, so none of the DWARF unwinder, or EHABI unwinder on ARM, is needed except for in non-PE builds? I presume this is a (somewhat) recent development - because back when I moved ARM/AArch64 over to these internal unwinders, I'm pretty sure they were used for some bits in PE builds as well (although I can't say for certain).
I guess it's possible to hit unwinding in the unix libs still in unexpected cases (not sure if it works properly there or not though); on crashes/unexpected signals in the unix side? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931#note_58870
On Wed Jan 24 19:54:21 2024 +0000, Martin Storsjö wrote:
Hmm, so none of the DWARF unwinder, or EHABI unwinder on ARM, is needed except for in non-PE builds? I presume this is a (somewhat) recent development - because back when I moved ARM/AArch64 over to these internal unwinders, I'm pretty sure they were used for some bits in PE builds as well (although I can't say for certain). I guess it's possible to hit unwinding in the unix libs still in unexpected cases (not sure if it works properly there or not though); on crashes/unexpected signals in the unix side? I tested this myself now, for ARM and AArch64, and indeed, the dwarf/ehabi/libunwind unwinding isn't needed at all in PE builds. As AArch64 requires PE builds right now, we probably could update this to stop using the dwarf unwinder as well?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931#note_58873
On Wed Jan 24 20:22:40 2024 +0000, Martin Storsjö wrote:
I tested this myself now, for ARM and AArch64, and indeed, the dwarf/ehabi/libunwind unwinding isn't needed at all in PE builds. As AArch64 requires PE builds right now, we probably could update this to stop using the dwarf unwinder as well? It's fairly recent, basically since the PE/Unix separation was completed.
Crashes in Unix libs essentially longjmp() to the last syscall entry point, they don't report an exception to the PE side, so there's no unwinding. The only case where the unwinder could see an address in a Unix lib is if there's a .dll.so that calls Unix without going through the syscall interface. That can't be allowed on aarch64, because it wouldn't preserve x18. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931#note_58874
On Wed Jan 24 20:22:48 2024 +0000, Alexandre Julliard wrote:
It's fairly recent, basically since the PE/Unix separation was completed. Crashes in Unix libs essentially longjmp() to the last syscall entry point, they don't report an exception to the PE side, so there's no unwinding. The only case where the unwinder could see an address in a Unix lib is if there's a .dll.so that calls Unix without going through the syscall interface. That can't be allowed on aarch64, because it wouldn't preserve x18. Ok, that's great stuff indeed, and makes things clearer and nicer! Some of the custom opcodes for unwinding through unixlib trampolines were quite a mess.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4931#note_58875
participants (4)
-
Alexandre Julliard (＠julliard) -
Jinoh Kang (＠iamahuman)
-
Martin Storsjö
-
Marvin