Re: [RFC PATCH] loader: allow overriding pthread_create correctly on linux

[Austin English]

On Fri, Jun 28, 2013 at 3:53 AM, Maarten Lankhorst
<m.b.lankhorst(a)gmail.com> wrote:
> This should be enough to fix bug #30557 in the most thorough possible way.
> Only tested on ubuntu precise 32-bits atm, but I believe it should work on 64-bits too.
>
> TODO: pthread_create_2_0 is not handled correctly right now, so it will fail on ancient glibc's < 2.2.
> But because NPTL requires a slightly newer glibc than that I do not believe it will be a real issue.
>
> Comments welcome :-)
> ---
> diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c
> index 01a8026..e9775c3 100644
> --- a/dlls/ntdll/thread.c
> +++ b/dlls/ntdll/thread.c
> @@ -33,6 +33,7 @@
>  #ifdef HAVE_SYS_SYSCALL_H
>  #include <sys/syscall.h>
>  #endif
> +#include <errno.h>
>
> #define NONAMELESSUNION
>  #include "ntstatus.h"
> @@ -58,6 +59,7 @@ struct startup_info
>      TEB                            *teb;
>      PRTL_THREAD_START_ROUTINE       entry_point;
>      void                           *entry_arg;
> +    BOOL                            native_thread;
>  };
>
> static PEB *peb;
> @@ -186,6 +188,20 @@ done:
>      return status;
>  }
>
> +#ifdef __linux__
> +extern typeof(pthread_create) *__glob_pthread_create, *call_pthread_create;
> +static typeof(pthread_create) __hook_pthread_create;
> +
> +static void thread_wrap_init(void)
> +{
> +    call_pthread_create = __hook_pthread_create;
> +}
> +
> +#else
> +#define __glob_pthread_create pthread_create
> +#define thread_wrap_init()
> +#endif
> +
>  /***********************************************************************
>   *           thread_init
>   *
> @@ -204,6 +220,7 @@ HANDLE thread_init(void)
>      struct ntdll_thread_data *thread_data;
>      static struct debug_info debug_info;  /* debug info for initial thread */
>
> +    thread_wrap_init();
>      virtual_init();
>
> /* reserve space for shared user data */
> @@ -327,11 +344,7 @@ void terminate_thread( int status )
>      pthread_exit( UIntToPtr(status) );
>  }
>
> -
> -/***********************************************************************
> - *           exit_thread
> - */
> -void exit_thread( int status )
> +static void exit_thread_common( int status )
>  {
>      static void *prev_teb;
>      TEB *teb;
> @@ -377,9 +390,60 @@ void exit_thread( int status )
>      close( ntdll_get_thread_data()->wait_fd[1] );
>      close( ntdll_get_thread_data()->reply_fd );
>      close( ntdll_get_thread_data()->request_fd );
> +}
> +
> +void exit_thread( int status )
> +{
> +    exit_thread_common(status);
>      pthread_exit( UIntToPtr(status) );
>  }
>
> +#ifdef __linux__
> +
> +struct unix_arg {
> +    void *(*start)(void *);
> +    void *arg;
> +};
> +
> +/* dummy used for comparison */
> +static DWORD native_unix_start;
> +
> +static void call_native_cleanup(void *arg)
> +{
> +    RtlFreeThreadActivationContextStack();
> +    exit_thread_common(0);
> +}
> +
> +static int
> +__hook_pthread_create(pthread_t *thread, const pthread_attr_t *attr,
> +                     void *(*start_routine) (void *), void *parm)
> +{
> +    NTSTATUS ret;
> +    struct unix_arg arg;
> +    arg.start = start_routine;
> +    arg.arg = parm;
> +
> +    TRACE("Overriding thread creation!\n");
> +    if (attr)
> +        FIXME("thread attributes ignored!\n");
> +
> +    ret = RtlCreateUserThread( NtCurrentProcess(), NULL, FALSE, NULL, 0, 0, (void*)&native_unix_start, &arg, NULL, (void*)thread );
> +    if (ret != STATUS_SUCCESS)
> +        FIXME("ret: %08x\n", ret);
> +    switch (ret) {
> +    case STATUS_SUCCESS:
> +        return 0;
> +    case STATUS_NO_MEMORY:
> +        return ENOMEM;
> +    case STATUS_TOO_MANY_OPENED_FILES:
> +        return EMFILE;
> +    default:
> +        ERR("Unhandled ntstatus %08x\n", ret);
> +        return ENOMEM;
> +    }
> +}
> +
> +#endif
>
> /***********************************************************************
>   *           start_thread
> @@ -412,9 +476,18 @@ static void start_thread( struct startup_info *info )
>      if (TRACE_ON(relay))
>          DPRINTF( "%04x:Starting thread proc %p (arg=%p)\n", GetCurrentThreadId(), func, arg );
>
> -    call_thread_entry_point( (LPTHREAD_START_ROUTINE)func, arg );
> -}
> +#ifdef __linux__
> +    if (info->native_thread) {
> +        void *(*start)(void*) = (void*)func;
>
> +        FIXME("Started native thread %08x\n", GetCurrentThreadId());
> +        pthread_cleanup_push(call_native_cleanup, NULL);
> +        pthread_exit(start(arg));
> +        pthread_cleanup_pop(1);
> +    } else
> +#endif
> +        call_thread_entry_point( (LPTHREAD_START_ROUTINE)func, arg );
> +}
>
> /***********************************************************************
>   *              RtlCreateUserThread   (NTDLL.@)
> @@ -497,8 +570,18 @@ NTSTATUS WINAPI RtlCreateUserThread( HANDLE process, const SECURITY_DESCRIPTOR *
>
> info = (struct startup_info *)(teb + 1);
>      info->teb         = teb;
> -    info->entry_point = start;
> -    info->entry_arg   = param;
> +#ifdef __linux__
> +    info->native_thread = (void*)start == (void*)&native_unix_start;
> +    if (info->native_thread) {
> +        struct unix_arg *arg = param;
> +        info->entry_point = (void*)arg->start;
> +        info->entry_arg = arg->arg;
> +    } else
> +#endif
> +    {
> +        info->entry_point = start;
> +        info->entry_arg   = param;
> +    }
>
> thread_data = (struct ntdll_thread_data *)teb->SpareBytes1;
>      thread_data->request_fd  = request_pipe[1];
> @@ -513,7 +596,7 @@ NTSTATUS WINAPI RtlCreateUserThread( HANDLE process, const SECURITY_DESCRIPTOR *
>                             (char *)teb->Tib.StackBase - (char *)teb->DeallocationStack );
>      pthread_attr_setscope( &attr, PTHREAD_SCOPE_SYSTEM ); /* force creating a kernel thread */
>      interlocked_xchg_add( &nb_threads, 1 );
> -    if (pthread_create( &pthread_id, &attr, (void * (*)(void *))start_thread, info ))
> +    if (__glob_pthread_create( &pthread_id, &attr, (void * (*)(void *))start_thread, info ))
>      {
>          interlocked_xchg_add( &nb_threads, -1 );
>          pthread_attr_destroy( &attr );
> @@ -523,6 +606,11 @@ NTSTATUS WINAPI RtlCreateUserThread( HANDLE process, const SECURITY_DESCRIPTOR *
>      pthread_attr_destroy( &attr );
>      pthread_sigmask( SIG_SETMASK, &sigset, NULL );
>
> +#ifdef __linux__
> +    if (native_thread && id)
> +        *(pthread_t*)id = pthread_id;
> +    else
> +#endif
>      if (id) id->UniqueThread = ULongToHandle(tid);
>      if (handle_ptr) *handle_ptr = handle;
>      else NtClose( handle );
> diff --git a/dlls/winegstreamer/glibthread.c b/dlls/winegstreamer/glibthread.c
> index 25eceb8..7417941 100644
> --- a/dlls/winegstreamer/glibthread.c
> +++ b/dlls/winegstreamer/glibthread.c
> @@ -43,6 +43,7 @@
>  #include <stdlib.h>
>  #include <stdio.h>
>
> +#if 0
>  #include "windef.h"
>  #include "winbase.h"
>  #include "winnls.h"
> @@ -388,3 +389,15 @@ void g_thread_impl_init (void)
>    g_thread_self_tls = TlsAlloc ();
>    g_thread_init(&g_thread_functions_for_glib_use_default);
>  }
> +
> +#else
> +
> +void g_thread_impl_init (void)
> +{
> +  static gboolean beenhere = FALSE;
> +
> +  if (!beenhere++)
> +    g_thread_init(NULL);
> +}
> +
> +#endif
> diff --git a/libs/wine/loader.c b/libs/wine/loader.c
> index 094e5e1..69f4a38 100644
> --- a/libs/wine/loader.c
> +++ b/libs/wine/loader.c
> @@ -69,6 +69,11 @@ char **__wine_main_argv = NULL;
>  WCHAR **__wine_main_wargv = NULL;
>  char **__wine_main_environ = NULL;
>
> +#ifdef __linux__
> +#include <pthread.h>
> +typeof(pthread_create) *call_pthread_create, *__glob_pthread_create;
> +#endif
> +
>  struct dll_path_context
>  {
>      unsigned int index; /* current index in the dll path list */
> diff --git a/libs/wine/wine.map b/libs/wine/wine.map
> index 2159fac..694869a 100644
> --- a/libs/wine/wine.map
> +++ b/libs/wine/wine.map
> @@ -117,6 +117,8 @@ WINE_1.0
>      wine_utf8_mbstowcs;
>      wine_utf8_wcstombs;
>      wine_wctype_table;
> +    __glob_pthread_create;
> +    call_pthread_create;
>
> local: *;
>  };
> diff --git a/loader/main.c b/loader/main.c
> index ac67290..14b2428 100644
> --- a/loader/main.c
> +++ b/loader/main.c
> @@ -202,6 +202,27 @@ static int pre_exec(void)
>
> #endif
>
> +#ifdef __linux__
> +
> +extern typeof(pthread_create) *call_pthread_create, *__glob_pthread_create;
> +
> +int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
> +                  void *(*start_routine) (void *), void *arg)
> +{
> +    return call_pthread_create(thread, attr, start_routine, arg);
> +}
> +
> +static void init_thread_hook(void) {
> +    call_pthread_create = __glob_pthread_create = dlvsym(RTLD_NEXT, "pthread_create", "GLIBC_2.2");
> +    if (!__glob_pthread_create)
> +        call_pthread_create = __glob_pthread_create = dlvsym(RTLD_NEXT, "pthread_create", "GLIBC_2.1");
> +}
> +
> +#else
> +
> +#define init_thread_hook()
> +
> +#endif
>
> /**********************************************************************
>   *           main
> @@ -211,6 +232,8 @@ int main( int argc, char *argv[] )
>      char error[1024];
>      int i;
>
> +    init_thread_hook();
> +
>      if (!getenv( "WINELOADERNOEXEC" ))  /* first time around */
>      {
>          static char noexec[] = "WINELOADERNOEXEC=1";

Fails to compile here (as mentioned on bug 30557):
gcc -m32 -c -I. -I. -I../../include -I../../include  -D__WINESRC__
-D_NTSYSTEM_ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing
-Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers
-Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter
-Wwrite-strings -Wpointer-arith -Wlogical-op -gdwarf-2 -gstrict-dwarf
-fno-omit-frame-pointer  -g -O2  -o thread.o thread.c
thread.c: In function ‘RtlCreateUserThread’:
thread.c:610:9: error: ‘native_thread’ undeclared (first use in this function)
thread.c:610:9: note: each undeclared identifier is reported only once
for each function it appears in
make[1]: *** [thread.o] Error 1
make[1]: Leaving directory `/home/austin/wine-git/dlls/ntdll'
make: *** [dlls/ntdll] Error 2

I tried Ruslan's fixed patch, which failed as well:
gcc -m32 -c -I. -I. -I../include -I../include  -D__WINESRC__   -Wall
-pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body
-Wignored-qualifiers -Wstrict-prototypes -Wtype-limits
-Wunused-but-set-parameter -Wwrite-strings -Wpointer-arith
-Wlogical-op -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer  -g -O2
-fno-builtin -o main.o main.c
gcc -m32 -o wine -Wl,--export-dynamic
-Wl,--section-start,.interp=0x7bf00400 main.o -L../libs/wine -lwine
../libs/port/libwine_port.a -lpthread
-Wl,--rpath,\$ORIGIN/../libs/wine
/usr/bin/ld: main.o: undefined reference to symbol 'dlvsym@@GLIBC_2.1'
/usr/bin/ld: note: 'dlvsym@@GLIBC_2.1' is defined in DSO
/lib/libdl.so.2 so try adding it to the linker command line
/lib/libdl.so.2: could not read symbols: Invalid operation
collect2: error: ld returned 1 exit status
make[1]: *** [wine] Error 1
make[1]: Leaving directory `/home/austin/wine-git/loader'
make: *** [loader] Error 2

this is 32-bit wine on fedora 64 (against 1.6-rc4).

--
-Austin