[PATCH 0/1] MR4928: ntdll: Fix building for ARM with libunwind enabled.
The references to raise_func_trampoline were left behind in 89f3c59739e6a879b6f362dfd29d41347590449d. In Linux builds, libunwind isn't practically needed since a27b202a4ddc314e3e856c10f2e5d010c4a88ee0 (which implemented an internal EHABI unwinder). However if the internal EHABI support is skipped (as it would be if building on an OS different than Linux), libunwind doesn't seem to be able to unwind properly through all cases after the changes in commits 5b068472141, 75d0d466ec9 and 89f3c59739e any longer, in a non-PE build. Signed-off-by: Martin Storsjö <martin(a)martin.st> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4928
From: Martin Storsjö <martin(a)martin.st> The references to raise_func_trampoline were left behind in 89f3c59739e6a879b6f362dfd29d41347590449d. In Linux builds, libunwind isn't practically needed since a27b202a4ddc314e3e856c10f2e5d010c4a88ee0 (which implemented an internal EHABI unwinder). However if the internal EHABI support is skipped (as it would be if building on an OS different than Linux), libunwind doesn't seem to be able to unwind properly through all cases after the changes in commits 5b068472141, 75d0d466ec9 and 89f3c59739e any longer, in a non-PE build. Signed-off-by: Martin Storsjö <martin(a)martin.st> --- dlls/ntdll/unix/signal_arm.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index 5115fa7ec3a..c0f715b7e36 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -260,8 +260,6 @@ static BOOL is_inside_syscall( ucontext_t *sigcontext ) (char *)SP_sig(sigcontext) <= (char *)arm_thread_data()->syscall_frame); } -extern void raise_func_trampoline( EXCEPTION_RECORD *rec, CONTEXT *context, void *dispatcher ); - struct exidx_entry { uint32_t addr; @@ -559,11 +557,6 @@ static NTSTATUS ehabi_virtual_unwind( UINT ip, DWORD *frame, CONTEXT *context, if (!set_pc) context->Pc = context->Lr; - /* There's no need to check for raise_func_trampoline and manually restore - * Lr separately from Pc like with libunwind; the EHABI unwind info - * describes how both of them are restored separately, and as long as - * the unwind info restored Pc, it doesn't have to be set from Lr. */ - TRACE( "next function pc=%08lx\n", context->Pc ); TRACE(" r0=%08lx r1=%08lx r2=%08lx r3=%08lx\n", context->R0, context->R1, context->R2, context->R3 ); @@ -736,17 +729,6 @@ static NTSTATUS libunwind_virtual_unwind( DWORD ip, DWORD *frame, CONTEXT *conte unw_get_reg( &cursor, UNW_REG_IP, (unw_word_t *)&context->Pc ); context->ContextFlags |= CONTEXT_UNWOUND_TO_CALL; - if ((info.start_ip & ~(unw_word_t)1) == - ((unw_word_t)raise_func_trampoline & ~(unw_word_t)1)) { - /* raise_func_trampoline stores the original Lr at the bottom of the - * stack. The unwinder normally can't restore both Pc and Lr to - * individual values, thus do that manually here. - * (The function we unwind to might be a leaf function that hasn't - * backed up its own original Lr value on the stack.) */ - const DWORD *orig_lr = (const DWORD *) *frame; - context->Lr = *orig_lr; - } - TRACE( "next function pc=%08lx%s\n", context->Pc, rc ? "" : " (last frame)" ); TRACE(" r0=%08lx r1=%08lx r2=%08lx r3=%08lx\n", context->R0, context->R1, context->R2, context->R3 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4928
I'm not sure we care about ARM libunwind anymore, especially since it's already broken. It could probably be removed altogether. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4928#note_58815
On Wed Jan 24 12:50:06 2024 +0000, Alexandre Julliard wrote:
I'm not sure we care about ARM libunwind anymore, especially since it's already broken. It could probably be removed altogether. That's probably quite reasonable (I guess we should probably do the same for aarch64 as well then?).
Although the EHABI unwinding only is implemented for Linux (it needs `dl_iterate_phdr`; I guess something similar is possible to find for other reasonable platforms if they would care), so it would leave other platforms broken. But this is probably such a fringe architecture anyway so that's probably fine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4928#note_58818
On Wed Jan 24 12:50:06 2024 +0000, Martin Storsjö wrote:
That's probably quite reasonable (I guess we should probably do the same for aarch64 as well then?). Although the EHABI unwinding only is implemented for Linux (it needs `dl_iterate_phdr`; I guess something similar is possible to find for other reasonable platforms if they would care), so it would leave other platforms broken. But this is probably such a fringe architecture anyway so that's probably fine. Yes, it could be removed from aarch64 as well.
A further question is whether we need to support non-PE builds at all on ARM... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4928#note_58825
On Wed Jan 24 13:06:21 2024 +0000, Alexandre Julliard wrote:
Yes, it could be removed from aarch64 as well. A further question is whether we need to support non-PE builds at all on ARM... Yep - I guess that non-PE builds for ARM won't be necessary in the long run.
As long as there's no huge extra burden to keep that going (I guess that you'll still support non-PE builds for x86 for some time still, due to downstream/packager/user inertia?), I think we probably should keep it alive for ARM as well for another little while (another major version?). If something crops up sooner, where it takes unreasonable amounts of work to keep that config working for ARM, I don't mind axing it sooner though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4928#note_58826
participants (2)
-
Alexandre Julliard (＠julliard) -
Martin Storsjö