[PATCH 0/1] MR3496: ntdll: Log segmentation faults in native threads.
Currently if a native Unix thread (created by our Unix lib or a native library) is crashing the process just silently terminates without any log record. That happens because segv_handler gets segfault on the first attempt to access TEB (e. g., for logging anything but not just). I think having at least some note of the fact that happened is helpful. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/server.c | 2 +- dlls/ntdll/unix/signal_arm.c | 3 +++ dlls/ntdll/unix/signal_arm64.c | 3 +++ dlls/ntdll/unix/signal_i386.c | 7 ++++++- dlls/ntdll/unix/signal_x86_64.c | 6 +++++- dlls/ntdll/unix/unix_private.h | 1 + 6 files changed, 19 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index e5e234d05ae..4fb40e350ad 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -1480,7 +1480,7 @@ static void send_server_task_port(void) * * Retrieve the Unix tid to use on the server side for the current thread. */ -static int get_unix_tid(void) +int get_unix_tid(void) { int ret = -1; #ifdef HAVE_PTHREAD_GETTHREADID_NP diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index 28cb2222809..08999c69fd8 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -1286,6 +1286,9 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) EXCEPTION_RECORD rec = { 0 }; ucontext_t *context = sigcontext; + if (!NtCurrentTeb()) fprintf( stderr, "wine: Segfault in native thread %d, pc %p.\n", + get_unix_tid(), (void *)PC_sig(context) ); + switch (get_trap_code(signal, context)) { case TRAP_ARM_PRIVINFLT: /* Invalid opcode exception */ diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index 6dbc3fd5a59..5c8606dc474 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -1267,6 +1267,9 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) EXCEPTION_RECORD rec = { 0 }; ucontext_t *context = sigcontext; + if (!NtCurrentTeb()) fprintf( stderr, "wine: Segfault in native thread %d, pc %p.\n", + get_unix_tid(), (void *)PC_sig(context) ); + rec.NumberParameters = 2; rec.ExceptionInformation[0] = (get_fault_esr( context ) & 0x40) != 0; rec.ExceptionInformation[1] = (ULONG_PTR)siginfo->si_addr; diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 293b8689c06..5449647fc0f 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -1849,7 +1849,12 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) EXCEPTION_RECORD rec = { 0 }; struct xcontext xcontext; ucontext_t *ucontext = sigcontext; - void *stack = setup_exception_record( sigcontext, &rec, &xcontext ); + void *stack; + + if (!NtCurrentTeb()) fprintf( stderr, "wine: Segfault in native thread %d, pc %p.\n", + get_unix_tid(), (void *)EIP_sig(ucontext) ); + + stack = setup_exception_record( sigcontext, &rec, &xcontext ); switch (TRAP_sig(ucontext)) { diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 0204139631f..1247ddb9743 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1918,8 +1918,12 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) { EXCEPTION_RECORD rec = { 0 }; struct xcontext context; - ucontext_t *ucontext = init_handler( sigcontext ); + ucontext_t *ucontext = sigcontext; + + if (!NtCurrentTeb()) fprintf( stderr, "wine: Segfault in native thread %d, pc %p.\n", + get_unix_tid(), (void *)RIP_sig(ucontext) ); + init_handler( sigcontext ); rec.ExceptionAddress = (void *)RIP_sig(ucontext); save_context( &context, ucontext ); diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 6862d74b863..f15982cb7db 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -324,6 +324,7 @@ extern NTSTATUS wow64_wine_server_fd_to_handle( void *args ) DECLSPEC_HIDDEN; extern NTSTATUS wow64_wine_server_handle_to_fd( void *args ) DECLSPEC_HIDDEN; extern NTSTATUS wow64_wine_spawnvp( void *args ) DECLSPEC_HIDDEN; #endif +extern int get_unix_tid(void) DECLSPEC_HIDDEN; extern void dbg_init(void) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3496
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=135584 Your paranoid android. === debian11 (32 bit report) === Report validation errors: ntdll:exception has no test summary line (early exit of the main process?)
See 2126. I fear this is intractable, because you *cannot* make any calls into libc before restoring its TLS registers, and we cannot restore the TLS registers without the TEB. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496#note_41127
Yeah, indeed, already figured that much from looking into test failure. NtCurrentTeb() itself may crash without restoring fs. Looks more complicated than this patch, if even possible. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496#note_41128
This merge request was closed by Paul Gofman. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496
On Thu Aug 3 11:26:38 2023 +0000, Zebediah Figura wrote:
See 2126. I fear this is intractable, because you *cannot* make any calls into libc before restoring its TLS registers, and we cannot restore the TLS registers without the TEB. Do we mess up with TLS registers on threads that do not have a TEB?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496#note_41177
On Thu Aug 3 11:26:38 2023 +0000, Giovanni Mascellani wrote:
Do we mess up with TLS registers on threads that do not have a TEB? We don't. And the thing works for native threads. When it breaks is when there is a Wine thread which has TLS register altered. That is supposed to be restored first with init_handler(). With this patch which tries NtCurrentTeb() before init_handler() segv_handler crashes for Wine thread in pthread_getspecific. And for native thread init_handler can't work.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496#note_41247
On Thu Aug 3 15:56:21 2023 +0000, Paul Gofman wrote:
We don't. And the thing works for native threads. When it breaks is when there is a Wine thread which has TLS register altered. That is supposed to be restored first with init_handler(). With this patch which tries NtCurrentTeb() before init_handler() segv_handler crashes for Wine thread in pthread_getspecific. And for native thread init_handler can't work. In other words, we need a neater way to determine if thread is native or Wine, NtCurrentTeb() doesn't fully work for that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496#note_41248
On Thu Aug 3 15:59:23 2023 +0000, Paul Gofman wrote:
In other words, we need a neater way to determine if thread is native or Wine, NtCurrentTeb() doesn't fully work for that. Ah, that makes sense. And I guess that we don't have a reliable way to determine whether `%fs` or `%gs` belong to Wine or to pthreads, right?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3496#note_41328
participants (5)
-
Giovanni Mascellani (＠giomasce) -
Marvin -
Paul Gofman
-
Paul Gofman (＠gofman)
-
Zebediah Figura (＠zfigura)