[PATCH v2 0/4] MR1088: ntdll: Avoid calling pthread_exit on thread exit.
Using a dedicated exit jmpbuf and removing the need for assembly routines. When Wine handles an exception in unix code, we return to user mode by jumping to the last syscall frame. This can leave some pthread cancel cleanups registered, in the pthread internal linked list, and at the same time later overwrite the stack frame they were registered for. In the same way, jumping to the exit frame on thread exit or abort, can also leave some cleanup handlers registered for invalid stack frames. Depending on the implementation, calling pthread_exit will cause all the registered pthread cleanup handlers to be called, possibly jumping back to now overwritten stack frames and causing segmentation faults. Exiting a pthread normally, by returning from its procedure, or calling exit(0) for the main thread doesn't run pthread_exit and doesn't call cleanup handlers, avoiding that situation. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 ### Additional note: For robustness, we should probably try to execute these cleanup handlers when unwinding the stack frames, as we would otherwise leave pthread objects in a potential problematic state (like a mutex locked, etc). It is however hard to do so when the handlers are registered from some C code: pthread C implementation is done by calling some internal pthread functions to register the handlers, and they aren't registered as standard unwind handlers. Only pthread_cancel and pthread_exit can unwind and call / unregister the C handlers, but interrupting that procedure, for instance calling setjmp / longjmp from withing our own handler isn't supported.
From C++ code, pthread cleanup handlers are registered through C++ class constructors / destructors, and it would then be possible to partially unwind and call them at the same time.
-- v2: ntdll: Remove unnecessary signal_start_thread register spilling. ntdll: Remove unnecessary arch specific exit frame pointer. ntdll: Avoid calling pthread_exit on thread exit. loader: Allow __wine_main return without emitting a warning. https://gitlab.winehq.org/wine/wine/-/merge_requests/1088
From: Rémi Bernon <rbernon(a)codeweavers.com> --- loader/main.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/loader/main.c b/loader/main.c index 242ff15accd..d1919c987da 100644 --- a/loader/main.c +++ b/loader/main.c @@ -175,17 +175,21 @@ static void *load_ntdll( char *argv0 ) */ int main( int argc, char *argv[] ) { + void (*__wine_main)(int, char **, char **); void *handle; - if ((handle = load_ntdll( argv[0] ))) + if (!(handle = load_ntdll( argv[0] ))) + { + fprintf( stderr, "wine: could not load ntdll.so: %s\n", dlerror() ); + pthread_detach( pthread_self() ); /* force importing libpthread for OpenGL */ + exit(1); + } + + if (!(__wine_main = dlsym( handle, "__wine_main" ))) { - void (*init_func)(int, char **, char **) = dlsym( handle, "__wine_main" ); - if (init_func) init_func( argc, argv, environ ); fprintf( stderr, "wine: __wine_main function not found in ntdll.so\n" ); exit(1); } - fprintf( stderr, "wine: could not load ntdll.so: %s\n", dlerror() ); - pthread_detach( pthread_self() ); /* force importing libpthread for OpenGL */ - exit(1); + __wine_main( argc, argv, environ ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1088
From: Rémi Bernon <rbernon(a)codeweavers.com> Using a dedicated exit jmpbuf and removing the need for assembly routines. When Wine handles an exception in unix code, we return to user mode by jumping to the last syscall frame. This can leave some pthread cancel cleanups registered, in the pthread internal linked list, and at the same time later overwrite the stack frame they were registered for. In the same way, jumping to the exit frame on thread exit or abort, can also leave some cleanup handlers registered for invalid stack frames. Depending on the implementation, calling pthread_exit will cause all the registered pthread cleanup handlers to be called, possibly jumping back to now overwritten stack frames and causing segmentation faults. Exiting a pthread normally, by returning from its procedure doesn't run pthread_exit and doesn't call cleanup handlers, avoiding that situation. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 --- dlls/ntdll/unix/server.c | 2 +- dlls/ntdll/unix/signal_arm.c | 13 ------------ dlls/ntdll/unix/signal_arm64.c | 12 ----------- dlls/ntdll/unix/signal_i386.c | 25 ----------------------- dlls/ntdll/unix/signal_x86_64.c | 21 -------------------- dlls/ntdll/unix/thread.c | 35 +++++++++++++++++++++++++-------- dlls/ntdll/unix/unix_private.h | 4 +++- 7 files changed, 31 insertions(+), 81 deletions(-) diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 77e8d5c7566..98afc42ad27 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -1581,7 +1581,7 @@ void server_init_process_done(void) SERVER_END_REQ; assert( !status ); - signal_start_thread( entry, peb, suspend, NtCurrentTeb() ); + thread_start( entry, peb, suspend, NtCurrentTeb() ); } diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index 5d1478a1ff4..ab00a67d436 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -1183,19 +1183,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "bl " __ASM_NAME("call_init_thunk") ) -/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - "ldr r3, [r2, #0x1d4]\n\t" /* arm_thread_data()->exit_frame */ - "mov ip, #0\n\t" - "str ip, [r2, #0x1d4]\n\t" - "cmp r3, ip\n\t" - "it ne\n\t" - "movne sp, r3\n\t" - "blx r1" ) - - /*********************************************************************** * __wine_syscall_dispatcher */ diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index 4e552e0f10a..b24dbc2d8a4 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -1253,18 +1253,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "1:\tmov sp, x8\n\t" "bl " __ASM_NAME("call_init_thunk") ) -/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - "stp x29, x30, [sp,#-16]!\n\t" - "ldr x3, [x2, #0x2f0]\n\t" /* arm64_thread_data()->exit_frame */ - "str xzr, [x2, #0x2f0]\n\t" - "cbz x3, 1f\n\t" - "mov sp, x3\n" - "1:\tldp x29, x30, [sp], #16\n\t" - "br x1" ) - /*********************************************************************** * __wine_syscall_dispatcher diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 2dfce706394..0eecb5fb57e 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2468,31 +2468,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "call " __ASM_NAME("call_init_thunk") ) -/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - "movl 8(%esp),%ecx\n\t" - "movl 12(%esp),%esi\n\t" - "xorl %edx,%edx\n\t" - /* fetch exit frame */ - "xchgl %edx,0x1f4(%esi)\n\t" /* x86_thread_data()->exit_frame */ - "testl %edx,%edx\n\t" - "jnz 1f\n\t" - "jmp *%ecx\n\t" - /* switch to exit frame stack */ - "1:\tmovl 4(%esp),%eax\n\t" - "movl %edx,%ebp\n\t" - __ASM_CFI(".cfi_def_cfa %ebp,4\n\t") - __ASM_CFI(".cfi_rel_offset %ebp,0\n\t") - __ASM_CFI(".cfi_rel_offset %ebx,-4\n\t") - __ASM_CFI(".cfi_rel_offset %esi,-8\n\t") - __ASM_CFI(".cfi_rel_offset %edi,-12\n\t") - "leal -20(%ebp),%esp\n\t" - "pushl %eax\n\t" - "call *%ecx" ) - - /*********************************************************************** * __wine_syscall_dispatcher */ diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 88c517daecf..fc73925e819 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -3324,27 +3324,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "call " __ASM_NAME("call_init_thunk")) -/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - /* fetch exit frame */ - "xorl %ecx,%ecx\n\t" - "xchgq %rcx,0x320(%rdx)\n\t" /* amd64_thread_data()->exit_frame */ - "testq %rcx,%rcx\n\t" - "jnz 1f\n\t" - "jmp *%rsi\n" - /* switch to exit frame stack */ - "1:\tmovq %rcx,%rsp\n\t" - __ASM_CFI(".cfi_adjust_cfa_offset 56\n\t") - __ASM_CFI(".cfi_rel_offset %rbp,48\n\t") - __ASM_CFI(".cfi_rel_offset %rbx,40\n\t") - __ASM_CFI(".cfi_rel_offset %r12,32\n\t") - __ASM_CFI(".cfi_rel_offset %r13,24\n\t") - __ASM_CFI(".cfi_rel_offset %r14,16\n\t") - __ASM_CFI(".cfi_rel_offset %r15,8\n\t") - "call *%rsi" ) - /*********************************************************************** * __wine_syscall_dispatcher */ diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c index bb8c1c2936b..ff994ea1503 100644 --- a/dlls/ntdll/unix/thread.c +++ b/dlls/ntdll/unix/thread.c @@ -1050,24 +1050,43 @@ static void pthread_exit_wrapper( int status ) close( ntdll_get_thread_data()->wait_fd[1] ); close( ntdll_get_thread_data()->reply_fd ); close( ntdll_get_thread_data()->request_fd ); - pthread_exit( UIntToPtr(status) ); +} + +void DECLSPEC_NORETURN thread_exit( int status, void (*func)(int), TEB *teb ) +{ + struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch; + + func(status); + __wine_longjmp( thread_data->exit_buf, 1 ); } /*********************************************************************** - * start_thread + * pthread_start_wrapper * * Startup routine for a newly created thread. */ -static void start_thread( TEB *teb ) +static void *pthread_start_wrapper( void *arg ) { + TEB *teb = arg; struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch; BOOL suspend; thread_data->pthread_id = pthread_self(); signal_init_thread( teb ); server_init_thread( thread_data->start, &suspend ); - signal_start_thread( thread_data->start, thread_data->param, suspend, teb ); + thread_start( thread_data->start, thread_data->param, suspend, teb ); + return NULL; +} + +void thread_start( PRTL_THREAD_START_ROUTINE entry, void *arg, + BOOL suspend, TEB *teb ) +{ + struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch; + __wine_jmp_buf exit_buf; + + if (!__wine_setjmpex( (thread_data->exit_buf = &exit_buf), NULL )) + signal_start_thread( entry, arg, suspend, teb ); } @@ -1345,7 +1364,7 @@ NTSTATUS WINAPI NtCreateThreadEx( HANDLE *handle, ACCESS_MASK access, OBJECT_ATT pthread_attr_setguardsize( &pthread_attr, 0 ); pthread_attr_setscope( &pthread_attr, PTHREAD_SCOPE_SYSTEM ); /* force creating a kernel thread */ InterlockedIncrement( &nb_threads ); - if (pthread_create( &pthread_id, &pthread_attr, (void * (*)(void *))start_thread, teb )) + if (pthread_create( &pthread_id, &pthread_attr, pthread_start_wrapper, teb )) { InterlockedDecrement( &nb_threads ); virtual_free_teb( teb ); @@ -1373,7 +1392,7 @@ void abort_thread( int status ) { pthread_sigmask( SIG_BLOCK, &server_block_set, NULL ); if (InterlockedDecrement( &nb_threads ) <= 0) abort_process( status ); - signal_exit_thread( status, pthread_exit_wrapper, NtCurrentTeb() ); + thread_exit( status, pthread_exit_wrapper, NtCurrentTeb() ); } @@ -1406,7 +1425,7 @@ static DECLSPEC_NORETURN void exit_thread( int status ) virtual_free_teb( teb ); } } - signal_exit_thread( status, pthread_exit_wrapper, NtCurrentTeb() ); + thread_exit( status, pthread_exit_wrapper, NtCurrentTeb() ); } @@ -1416,7 +1435,7 @@ static DECLSPEC_NORETURN void exit_thread( int status ) void exit_process( int status ) { pthread_sigmask( SIG_BLOCK, &server_block_set, NULL ); - signal_exit_thread( get_unix_exit_code( status ), process_exit_wrapper, NtCurrentTeb() ); + thread_exit( get_unix_exit_code( status ), process_exit_wrapper, NtCurrentTeb() ); } diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 47f0f9c56a9..77d5feb7661 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -61,6 +61,7 @@ struct ntdll_thread_data PRTL_THREAD_START_ROUTINE start; /* thread entry point */ void *param; /* thread entry point parameter */ void *jmp_buf; /* setjmp buffer for exception handling */ + void *exit_buf; /* setjmp buffer for thread exit */ }; C_ASSERT( sizeof(struct ntdll_thread_data) <= sizeof(((TEB *)0)->GdiTebBatch) ); @@ -237,7 +238,8 @@ extern void signal_init_thread( TEB *teb ) DECLSPEC_HIDDEN; extern void signal_init_process(void) DECLSPEC_HIDDEN; extern void DECLSPEC_NORETURN signal_start_thread( PRTL_THREAD_START_ROUTINE entry, void *arg, BOOL suspend, TEB *teb ) DECLSPEC_HIDDEN; -extern void DECLSPEC_NORETURN signal_exit_thread( int status, void (*func)(int), TEB *teb ) DECLSPEC_HIDDEN; +extern void thread_start( PRTL_THREAD_START_ROUTINE entry, void *arg, + BOOL suspend, TEB *teb ) DECLSPEC_HIDDEN; extern SYSTEM_SERVICE_TABLE KeServiceDescriptorTable[4] DECLSPEC_HIDDEN; extern void __wine_syscall_dispatcher(void) DECLSPEC_HIDDEN; extern void WINAPI DECLSPEC_NORETURN __wine_syscall_dispatcher_return( void *frame, ULONG_PTR retval ) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1088
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/unix/signal_arm.c | 14 +++++--------- dlls/ntdll/unix/signal_arm64.c | 15 +++++---------- dlls/ntdll/unix/signal_i386.c | 14 +++++--------- dlls/ntdll/unix/signal_x86_64.c | 20 ++++++++------------ 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index ab00a67d436..d5905622d1c 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -202,13 +202,11 @@ C_ASSERT( sizeof( struct syscall_frame ) == 0x160); struct arm_thread_data { - void *exit_frame; /* 1d4 exit frame pointer */ - struct syscall_frame *syscall_frame; /* 1d8 frame pointer on syscall entry */ + struct syscall_frame *syscall_frame; /* 1d4 frame pointer on syscall entry */ }; C_ASSERT( sizeof(struct arm_thread_data) <= sizeof(((struct ntdll_thread_data *)0)->cpu_data) ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct arm_thread_data, exit_frame ) == 0x1d4 ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct arm_thread_data, syscall_frame ) == 0x1d8 ); +C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct arm_thread_data, syscall_frame ) == 0x1d4 ); static inline struct arm_thread_data *arm_thread_data(void) { @@ -1172,13 +1170,11 @@ void DECLSPEC_HIDDEN call_init_thunk( LPTHREAD_START_ROUTINE entry, void *arg, B */ __ASM_GLOBAL_FUNC( signal_start_thread, "push {r4-r12,lr}\n\t" - /* store exit frame */ - "str sp, [r3, #0x1d4]\n\t" /* arm_thread_data()->exit_frame */ /* set syscall frame */ - "ldr r6, [r3, #0x1d8]\n\t" /* arm_thread_data()->syscall_frame */ + "ldr r6, [r3, #0x1d4]\n\t" /* arm_thread_data()->syscall_frame */ "cbnz r6, 1f\n\t" "sub r6, sp, #0x160\n\t" /* sizeof(struct syscall_frame) */ - "str r6, [r3, #0x1d8]\n\t" /* arm_thread_data()->syscall_frame */ + "str r6, [r3, #0x1d4]\n\t" /* arm_thread_data()->syscall_frame */ "1:\tmov sp, r6\n\t" "bl " __ASM_NAME("call_init_thunk") ) @@ -1188,7 +1184,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, */ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "mrc p15, 0, r1, c13, c0, 2\n\t" /* NtCurrentTeb() */ - "ldr r1, [r1, #0x1d8]\n\t" /* arm_thread_data()->syscall_frame */ + "ldr r1, [r1, #0x1d4]\n\t" /* arm_thread_data()->syscall_frame */ "add r0, r1, #0x10\n\t" "stm r0, {r4-r12,lr}\n\t" "add r2, sp, #0x10\n\t" diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index b24dbc2d8a4..a9c3a1b3844 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -150,13 +150,11 @@ C_ASSERT( sizeof( struct syscall_frame ) == 0x330 ); struct arm64_thread_data { - void *exit_frame; /* 02f0 exit frame pointer */ - struct syscall_frame *syscall_frame; /* 02f8 frame pointer on syscall entry */ + struct syscall_frame *syscall_frame; /* 02f0 frame pointer on syscall entry */ }; C_ASSERT( sizeof(struct arm64_thread_data) <= sizeof(((struct ntdll_thread_data *)0)->cpu_data) ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct arm64_thread_data, exit_frame ) == 0x2f0 ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct arm64_thread_data, syscall_frame ) == 0x2f8 ); +C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct arm64_thread_data, syscall_frame ) == 0x2f0 ); static inline struct arm64_thread_data *arm64_thread_data(void) { @@ -1242,14 +1240,11 @@ void DECLSPEC_HIDDEN call_init_thunk( LPTHREAD_START_ROUTINE entry, void *arg, B */ __ASM_GLOBAL_FUNC( signal_start_thread, "stp x29, x30, [sp,#-16]!\n\t" - /* store exit frame */ - "mov x29, sp\n\t" - "str x29, [x3, #0x2f0]\n\t" /* arm64_thread_data()->exit_frame */ /* set syscall frame */ - "ldr x8, [x3, #0x2f8]\n\t" /* arm64_thread_data()->syscall_frame */ + "ldr x8, [x3, #0x2f0]\n\t" /* arm64_thread_data()->syscall_frame */ "cbnz x8, 1f\n\t" "sub x8, sp, #0x330\n\t" /* sizeof(struct syscall_frame) */ - "str x8, [x3, #0x2f8]\n\t" /* arm64_thread_data()->syscall_frame */ + "str x8, [x3, #0x2f0]\n\t" /* arm64_thread_data()->syscall_frame */ "1:\tmov sp, x8\n\t" "bl " __ASM_NAME("call_init_thunk") ) @@ -1274,7 +1269,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "ldr x30, [sp, #80]\n\t" "ldp x0, x1, [sp], #96\n\t" - "ldr x10, [x18, #0x2f8]\n\t" /* arm64_thread_data()->syscall_frame */ + "ldr x10, [x18, #0x2f0]\n\t" /* arm64_thread_data()->syscall_frame */ "stp x18, x19, [x10, #0x90]\n\t" "stp x20, x21, [x10, #0xa0]\n\t" "stp x22, x23, [x10, #0xb0]\n\t" diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 0eecb5fb57e..c9e2684bde6 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -477,14 +477,12 @@ struct x86_thread_data DWORD dr3; /* 1e8 */ DWORD dr6; /* 1ec */ DWORD dr7; /* 1f0 */ - void *exit_frame; /* 1f4 exit frame pointer */ - struct syscall_frame *syscall_frame; /* 1f8 frame pointer on syscall entry */ + struct syscall_frame *syscall_frame; /* 1f4 frame pointer on syscall entry */ }; C_ASSERT( sizeof(struct x86_thread_data) <= sizeof(((struct ntdll_thread_data *)0)->cpu_data) ); C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct x86_thread_data, gs ) == 0x1d8 ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct x86_thread_data, exit_frame ) == 0x1f4 ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct x86_thread_data, syscall_frame ) == 0x1f8 ); +C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct x86_thread_data, syscall_frame ) == 0x1f4 ); /* flags to control the behavior of the syscall dispatcher */ #define SYSCALL_HAVE_XSAVE 1 @@ -2450,16 +2448,14 @@ __ASM_GLOBAL_FUNC( signal_start_thread, __ASM_CFI(".cfi_rel_offset %esi,-8\n\t") "pushl %edi\n\t" __ASM_CFI(".cfi_rel_offset %edi,-12\n\t") - /* store exit frame */ "movl 20(%ebp),%ecx\n\t" /* teb */ - "movl %ebp,0x1f4(%ecx)\n\t" /* x86_thread_data()->exit_frame */ /* set syscall frame */ - "movl 0x1f8(%ecx),%eax\n\t" /* x86_thread_data()->syscall_frame */ + "movl 0x1f4(%ecx),%eax\n\t" /* x86_thread_data()->syscall_frame */ "orl %eax,%eax\n\t" "jnz 1f\n\t" "leal -0x380(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */ "andl $~63,%eax\n\t" - "movl %eax,0x1f8(%ecx)\n" /* x86_thread_data()->syscall_frame */ + "movl %eax,0x1f4(%ecx)\n" /* x86_thread_data()->syscall_frame */ "1:\tmovl %eax,%esp\n\t" "pushl %ecx\n\t" /* teb */ "pushl 16(%ebp)\n\t" /* suspend */ @@ -2472,7 +2468,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, * __wine_syscall_dispatcher */ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, - "movl %fs:0x1f8,%ecx\n\t" /* x86_thread_data()->syscall_frame */ + "movl %fs:0x1f4,%ecx\n\t" /* x86_thread_data()->syscall_frame */ "movw $0,0x02(%ecx)\n\t" /* frame->restore_flags */ "popl 0x08(%ecx)\n\t" /* frame->eip */ "pushfl\n\t" diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index fc73925e819..48091d45a8f 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -418,16 +418,14 @@ struct amd64_thread_data DWORD_PTR dr3; /* 0308 */ DWORD_PTR dr6; /* 0310 */ DWORD_PTR dr7; /* 0318 */ - void *exit_frame; /* 0320 exit frame pointer */ - struct syscall_frame *syscall_frame; /* 0328 syscall frame pointer */ - void *pthread_teb; /* 0330 thread data for pthread */ - DWORD fs; /* 0338 WOW TEB selector */ + struct syscall_frame *syscall_frame; /* 0320 syscall frame pointer */ + void *pthread_teb; /* 0328 thread data for pthread */ + DWORD fs; /* 0330 WOW TEB selector */ }; C_ASSERT( sizeof(struct amd64_thread_data) <= sizeof(((struct ntdll_thread_data *)0)->cpu_data) ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct amd64_thread_data, exit_frame ) == 0x320 ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct amd64_thread_data, syscall_frame ) == 0x328 ); -C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct amd64_thread_data, pthread_teb ) == 0x330 ); +C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct amd64_thread_data, syscall_frame ) == 0x320 ); +C_ASSERT( offsetof( TEB, GdiTebBatch ) + offsetof( struct amd64_thread_data, pthread_teb ) == 0x328 ); static inline struct amd64_thread_data *amd64_thread_data(void) { @@ -3311,15 +3309,13 @@ __ASM_GLOBAL_FUNC( signal_start_thread, __ASM_CFI(".cfi_rel_offset %r14,16\n\t") "movq %r15,8(%rsp)\n\t" __ASM_CFI(".cfi_rel_offset %r15,8\n\t") - /* store exit frame */ - "movq %rsp,0x320(%rcx)\n\t" /* amd64_thread_data()->exit_frame */ /* set syscall frame */ - "movq 0x328(%rcx),%rax\n\t" /* amd64_thread_data()->syscall_frame */ + "movq 0x320(%rcx),%rax\n\t" /* amd64_thread_data()->syscall_frame */ "orq %rax,%rax\n\t" "jnz 1f\n\t" "leaq -0x400(%rsp),%rax\n\t" /* sizeof(struct syscall_frame) */ "andq $~63,%rax\n\t" - "movq %rax,0x328(%rcx)\n" /* amd64_thread_data()->syscall_frame */ + "movq %rax,0x320(%rcx)\n" /* amd64_thread_data()->syscall_frame */ "1:\tmovq %rax,%rsp\n\t" "call " __ASM_NAME("call_init_thunk")) @@ -3329,7 +3325,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, */ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq %gs:0x30,%rcx\n\t" - "movq 0x328(%rcx),%rcx\n\t" /* amd64_thread_data()->syscall_frame */ + "movq 0x320(%rcx),%rcx\n\t" /* amd64_thread_data()->syscall_frame */ "popq 0x70(%rcx)\n\t" /* frame->rip */ "pushfq\n\t" "popq 0x80(%rcx)\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1088
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/unix/signal_i386.c | 6 ------ dlls/ntdll/unix/signal_x86_64.c | 16 ---------------- 2 files changed, 22 deletions(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index c9e2684bde6..07f90a853d2 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2442,12 +2442,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, __ASM_CFI(".cfi_rel_offset %ebp,0\n\t") "movl %esp,%ebp\n\t" __ASM_CFI(".cfi_def_cfa_register %ebp\n\t") - "pushl %ebx\n\t" - __ASM_CFI(".cfi_rel_offset %ebx,-4\n\t") - "pushl %esi\n\t" - __ASM_CFI(".cfi_rel_offset %esi,-8\n\t") - "pushl %edi\n\t" - __ASM_CFI(".cfi_rel_offset %edi,-12\n\t") "movl 20(%ebp),%ecx\n\t" /* teb */ /* set syscall frame */ "movl 0x1f4(%ecx),%eax\n\t" /* x86_thread_data()->syscall_frame */ diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 48091d45a8f..f14b4f96886 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -3293,22 +3293,6 @@ void DECLSPEC_HIDDEN call_init_thunk( LPTHREAD_START_ROUTINE entry, void *arg, B * signal_start_thread */ __ASM_GLOBAL_FUNC( signal_start_thread, - "subq $56,%rsp\n\t" - __ASM_SEH(".seh_stackalloc 56\n\t") - __ASM_SEH(".seh_endprologue\n\t") - __ASM_CFI(".cfi_adjust_cfa_offset 56\n\t") - "movq %rbp,48(%rsp)\n\t" - __ASM_CFI(".cfi_rel_offset %rbp,48\n\t") - "movq %rbx,40(%rsp)\n\t" - __ASM_CFI(".cfi_rel_offset %rbx,40\n\t") - "movq %r12,32(%rsp)\n\t" - __ASM_CFI(".cfi_rel_offset %r12,32\n\t") - "movq %r13,24(%rsp)\n\t" - __ASM_CFI(".cfi_rel_offset %r13,24\n\t") - "movq %r14,16(%rsp)\n\t" - __ASM_CFI(".cfi_rel_offset %r14,16\n\t") - "movq %r15,8(%rsp)\n\t" - __ASM_CFI(".cfi_rel_offset %r15,8\n\t") /* set syscall frame */ "movq 0x320(%rcx),%rax\n\t" /* amd64_thread_data()->syscall_frame */ "orq %rax,%rax\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1088
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=125088 Your paranoid android. === debian11 (32 bit report) === d3d9: d3d9ex.c:3196: Test failed: Expected message 0x5 for window 0, but didn't receive it, i=0. d3d9ex.c:3202: Test failed: Got unexpected WINDOWPOS hwnd=00000000, x=0, y=0, cx=0, cy=0, flags=0 ddraw: ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x1000. ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x3000. mmdevapi: render.c:1116: Test marked flaky: Position 54184 too far after playing 100ms Report validation errors: kernel32:sync has no test summary line (early exit of the main process?) === debian11 (build log) === Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24745. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24745. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24745.
Ah yeah, I added that because we don't normally expect to return from `__wine_main`. I guess I should change that instead.
That won't help, it will still call exit(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1088#note_11119
On Tue Oct 18 12:33:27 2022 +0000, Alexandre Julliard wrote:
Ah yeah, I added that because we don't normally expect to return from `__wine_main`. I guess I should change that instead. That won't help, it will still call exit(). I'm not sure to understand then. Is it different from calling `pthread_exit` on the main thread like we are currently doing? Does pthread_exit wait for the other threads in that case?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1088#note_11124
On Tue Oct 18 12:33:27 2022 +0000, Rémi Bernon wrote:
I'm not sure to understand then. Is it different from calling `pthread_exit` on the main thread like we are currently doing? Does pthread_exit wait for the other threads in that case? As we're already tracking the remaining threads and calling `_exit` in `abort_process`, would some infinite sleep at the end of main to prevent it from returning early be right?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1088#note_11127
On Tue Oct 18 12:39:02 2022 +0000, Rémi Bernon wrote:
As we're already tracking the remaining threads and calling `_exit` in `abort_process`, would some infinite sleep at the end of main to prevent it from returning early be right? I guess another way would be to not use the main unix thread, and spawn a new one to be the main win32 thread. We could then safely call `pthread_exit` from the main unix thread, which would wait for all pthreads to exit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1088#note_11136
participants (3)
-
Alexandre Julliard (＠julliard) -
Marvin -
Rémi Bernon