[PATCH v2 0/1] MR5047: server, ntdll: Use 64 bit for unix thread id.
This is in preparation for the mach thread id rework, which requires 64 bit there (and incidentally also stops truncating the FreeBSD one). There wasn't any direct usage of `__int64` in the sever protocol before, so I added a new typedef and a new `dump_int64()` function (not sure if needed per se though). -- v2: server, ntdll: Use 64 bit for unix thread id. https://gitlab.winehq.org/wine/wine/-/merge_requests/5047
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/ntdll/unix/server.c | 8 +++----- dlls/ntdll/unix/thread.c | 12 +++++++----- dlls/ntdll/unix/unix_private.h | 2 +- server/mach.c | 6 +++--- server/procfs.c | 2 +- server/protocol.def | 9 +++++---- server/thread.c | 8 ++++---- server/thread.h | 4 ++-- server/trace.c | 23 +++++++++++++++++++---- tools/make_requests | 3 ++- 10 files changed, 47 insertions(+), 30 deletions(-) diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 69e6567c911..2d3133f6705 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -1476,9 +1476,9 @@ 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) +static unix_tid_t get_unix_tid(void) { - int ret = -1; + unix_tid_t ret = -1; #ifdef HAVE_PTHREAD_GETTHREADID_NP ret = pthread_getthreadid_np(); #elif defined(linux) @@ -1491,9 +1491,7 @@ static int get_unix_tid(void) #elif defined(__NetBSD__) ret = _lwp_self(); #elif defined(__FreeBSD__) - long lwpid; - thr_self( &lwpid ); - ret = lwpid; + thr_self( &ret ); #elif defined(__DragonFly__) ret = lwp_gettid(); #endif diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c index 52cb0f26e97..dbd53d0547e 100644 --- a/dlls/ntdll/unix/thread.c +++ b/dlls/ntdll/unix/thread.c @@ -1788,7 +1788,7 @@ void ntdll_set_exception_jmp_buf( jmp_buf jmp ) } -BOOL get_thread_times(int unix_pid, int unix_tid, LARGE_INTEGER *kernel_time, LARGE_INTEGER *user_time) +BOOL get_thread_times( int unix_pid, unix_tid_t unix_tid, LARGE_INTEGER *kernel_time, LARGE_INTEGER *user_time ) { #ifdef linux unsigned long clocks_per_sec = sysconf( _SC_CLK_TCK ); @@ -1801,7 +1801,7 @@ BOOL get_thread_times(int unix_pid, int unix_tid, LARGE_INTEGER *kernel_time, LA if (unix_tid == -1) snprintf( buf, sizeof(buf), "/proc/%u/stat", unix_pid ); else - snprintf( buf, sizeof(buf), "/proc/%u/task/%u/stat", unix_pid, unix_tid ); + snprintf( buf, sizeof(buf), "/proc/%u/task/%llu/stat", unix_pid, unix_tid ); if (!(f = fopen( buf, "r" ))) { WARN("Failed to open %s: %s\n", buf, strerror(errno)); @@ -1879,7 +1879,8 @@ static void set_native_thread_name( HANDLE handle, const UNICODE_STRING *name ) #ifdef linux unsigned int status; char path[64], nameA[64]; - int unix_pid, unix_tid, len, fd; + int unix_pid, len, fd; + unix_tid_t unix_tid; SERVER_START_REQ( get_thread_times ) { @@ -1904,7 +1905,7 @@ static void set_native_thread_name( HANDLE handle, const UNICODE_STRING *name ) } len = ntdll_wcstoumbs( name->Buffer, name->Length / sizeof(WCHAR), nameA, sizeof(nameA), FALSE ); - snprintf(path, sizeof(path), "/proc/%u/task/%u/comm", unix_pid, unix_tid); + snprintf( path, sizeof(path), "/proc/%u/task/%llu/comm", unix_pid, unix_tid ); if ((fd = open( path, O_WRONLY )) != -1) { write( fd, nameA, len ); @@ -2027,7 +2028,8 @@ NTSTATUS WINAPI NtQueryInformationThread( HANDLE handle, THREADINFOCLASS class, case ThreadTimes: { KERNEL_USER_TIMES kusrt; - int unix_pid, unix_tid; + int unix_pid; + unix_tid_t unix_tid; SERVER_START_REQ( get_thread_times ) { diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 07f2724eac7..982f758e217 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -269,7 +269,7 @@ extern NTSTATUS unwind_builtin_dll( void *args ); extern NTSTATUS get_thread_ldt_entry( HANDLE handle, void *data, ULONG len, ULONG *ret_len ); extern void *get_native_context( CONTEXT *context ); extern void *get_wow_context( CONTEXT *context ); -extern BOOL get_thread_times( int unix_pid, int unix_tid, LARGE_INTEGER *kernel_time, +extern BOOL get_thread_times( int unix_pid, unix_tid_t unix_tid, LARGE_INTEGER *kernel_time, LARGE_INTEGER *user_time ); extern void signal_init_threading(void); extern NTSTATUS signal_alloc_thread( TEB *teb ); diff --git a/server/mach.c b/server/mach.c index a619c1cd2a8..df36930135b 100644 --- a/server/mach.c +++ b/server/mach.c @@ -196,7 +196,7 @@ void get_thread_context( struct thread *thread, context_t *context, unsigned int } if (thread->unix_pid == -1 || !process_port || - mach_port_extract_right( process_port, thread->unix_tid, + mach_port_extract_right( process_port, (mach_port_name_t)thread->unix_tid, MACH_MSG_TYPE_COPY_SEND, &port, &type )) { set_error( STATUS_ACCESS_DENIED ); @@ -283,7 +283,7 @@ void set_thread_context( struct thread *thread, const context_t *context, unsign } if (thread->unix_pid == -1 || !process_port || - mach_port_extract_right( process_port, thread->unix_tid, + mach_port_extract_right( process_port, (mach_port_name_t)thread->unix_tid, MACH_MSG_TYPE_COPY_SEND, &port, &type )) { set_error( STATUS_ACCESS_DENIED ); @@ -368,7 +368,7 @@ int send_thread_signal( struct thread *thread, int sig ) mach_msg_type_name_t type; mach_port_t port; - if (!mach_port_extract_right( process_port, thread->unix_tid, + if (!mach_port_extract_right( process_port, (mach_port_name_t)thread->unix_tid, MACH_MSG_TYPE_COPY_SEND, &port, &type )) { ret = syscall( SYS___pthread_kill, port, sig ); diff --git a/server/procfs.c b/server/procfs.c index 7b61b2abb46..3636ea23369 100644 --- a/server/procfs.c +++ b/server/procfs.c @@ -75,7 +75,7 @@ static int open_proc_lwpctl( struct thread *thread ) if (thread->unix_pid == -1) return -1; - sprintf( buffer, "/proc/%u/lwp/%u/lwpctl", thread->unix_pid, thread->unix_tid ); + sprintf( buffer, "/proc/%u/lwp/%llu/lwpctl", thread->unix_pid, thread->unix_tid ); if ((fd = open( buffer, O_WRONLY )) == -1) { if (errno == ENOENT) /* probably got killed */ diff --git a/server/protocol.def b/server/protocol.def index 5d60e7fcda3..a37a3cc9cf2 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -45,6 +45,7 @@ typedef unsigned __int64 file_pos_t; typedef unsigned __int64 client_ptr_t; typedef unsigned __int64 affinity_t; typedef client_ptr_t mod_handle_t; +typedef __int64 unix_tid_t; struct request_header { @@ -966,7 +967,7 @@ typedef struct /* Initialize the first thread of a new process */ @REQ(init_first_thread) int unix_pid; /* Unix pid of new process */ - int unix_tid; /* Unix tid of new thread */ + unix_tid_t unix_tid; /* Unix tid of new thread */ int debug_level; /* new debug level */ int reply_fd; /* fd for reply pipe */ int wait_fd; /* fd for blocking calls pipe */ @@ -982,7 +983,7 @@ typedef struct /* Initialize a new thread; called from the child after pthread_create() */ @REQ(init_thread) - int unix_tid; /* Unix tid of new thread */ + unix_tid_t unix_tid; /* Unix tid of new thread */ int reply_fd; /* fd for reply pipe */ int wait_fd; /* fd for blocking calls pipe */ client_ptr_t teb; /* TEB of new thread (in thread address space) */ @@ -1101,7 +1102,7 @@ typedef struct timeout_t creation_time; /* thread creation time */ timeout_t exit_time; /* thread exit time */ int unix_pid; /* thread native pid */ - int unix_tid; /* thread native pid */ + unix_tid_t unix_tid; /* thread native pid */ @END @@ -1708,7 +1709,7 @@ struct thread_info thread_id_t tid; int base_priority; int current_priority; - int unix_tid; + unix_tid_t unix_tid; client_ptr_t teb; client_ptr_t entry_point; }; diff --git a/server/thread.c b/server/thread.c index 56f57cefd8f..2808a1cb30f 100644 --- a/server/thread.c +++ b/server/thread.c @@ -458,7 +458,7 @@ static void dump_thread( struct object *obj, int verbose ) struct thread *thread = (struct thread *)obj; assert( obj->ops == &thread_ops ); - fprintf( stderr, "Thread id=%04x unix pid=%d unix tid=%d state=%d\n", + fprintf( stderr, "Thread id=%04x unix pid=%d unix tid=%lld state=%d\n", thread->id, thread->unix_pid, thread->unix_tid, thread->state ); } @@ -541,7 +541,7 @@ struct thread *get_thread_from_handle( obj_handle_t handle, unsigned int access } /* find a thread from a Unix tid */ -struct thread *get_thread_from_tid( int tid ) +struct thread *get_thread_from_tid( unix_tid_t tid ) { struct thread *thread; @@ -578,7 +578,7 @@ int set_thread_affinity( struct thread *thread, affinity_t affinity ) for (i = 0, mask = 1; mask; i++, mask <<= 1) if (affinity & mask) CPU_SET( i, &set ); - ret = sched_setaffinity( thread->unix_tid, sizeof(set), &set ); + ret = sched_setaffinity( (int)thread->unix_tid, sizeof(set), &set ); } #endif if (!ret) thread->affinity = affinity; @@ -594,7 +594,7 @@ affinity_t get_thread_affinity( struct thread *thread ) cpu_set_t set; unsigned int i; - if (!sched_getaffinity( thread->unix_tid, sizeof(set), &set )) + if (!sched_getaffinity( (int)thread->unix_tid, sizeof(set), &set )) for (i = 0; i < 8 * sizeof(mask); i++) if (CPU_ISSET( i, &set )) mask |= (affinity_t)1 << i; } diff --git a/server/thread.h b/server/thread.h index 8dcf966a90a..e0d902d7839 100644 --- a/server/thread.h +++ b/server/thread.h @@ -73,7 +73,7 @@ struct thread enum run_state state; /* running state */ int exit_code; /* thread exit code */ int unix_pid; /* Unix pid of client */ - int unix_tid; /* Unix tid of client */ + unix_tid_t unix_tid; /* Unix tid of client */ struct context *context; /* current context */ client_ptr_t suspend_cookie;/* wait cookie of suspending select */ client_ptr_t teb; /* TEB address (in client address space) */ @@ -100,7 +100,7 @@ extern struct thread *create_thread( int fd, struct process *process, const struct security_descriptor *sd ); extern struct thread *get_thread_from_id( thread_id_t id ); extern struct thread *get_thread_from_handle( obj_handle_t handle, unsigned int access ); -extern struct thread *get_thread_from_tid( int tid ); +extern struct thread *get_thread_from_tid( unix_tid_t tid ); extern struct thread *get_thread_from_pid( int pid ); extern struct thread *get_wait_queue_thread( struct wait_queue_entry *entry ); extern enum select_op get_wait_queue_select_op( struct wait_queue_entry *entry ); diff --git a/server/trace.c b/server/trace.c index 1b65d2b977e..299519d3e21 100644 --- a/server/trace.c +++ b/server/trace.c @@ -103,6 +103,21 @@ static void dump_uint64( const char *prefix, const unsigned __int64 *val ) fprintf( stderr, "%s%08x", prefix, (unsigned int)*val ); } +static void dump_int64( const char *prefix, const __int64 *val ) +{ + unsigned __int64 abs_val; + + fprintf( stderr, "%s", prefix ); + if (*val < 0) + { + fprintf( stderr, "-" ); + abs_val = (unsigned __int64)(*val ^ (__int64)-1) + 1; + } + else + abs_val = (unsigned __int64)*val; + dump_uint64( "", &abs_val ); +} + static void dump_uint128( const char *prefix, const unsigned __int64 val[2] ) { unsigned __int64 low = val[0], high = val[1]; @@ -1217,7 +1232,7 @@ static void dump_varargs_process_info( const char *prefix, data_size_t size ) if (size - pos < sizeof(*thread)) break; if (i) fputc( ',', stderr ); dump_timeout( "{start_time=", &thread->start_time ); - fprintf( stderr, ",tid=%04x,base_priority=%d,current_priority=%d,unix_tid=%d}", + fprintf( stderr, ",tid=%04x,base_priority=%d,current_priority=%d,unix_tid=%lld}", thread->tid, thread->base_priority, thread->current_priority, thread->unix_tid ); pos += sizeof(*thread); } @@ -1468,7 +1483,7 @@ static void dump_init_process_done_reply( const struct init_process_done_reply * static void dump_init_first_thread_request( const struct init_first_thread_request *req ) { fprintf( stderr, " unix_pid=%d", req->unix_pid ); - fprintf( stderr, ", unix_tid=%d", req->unix_tid ); + dump_int64( ", unix_tid=", &req->unix_tid ); fprintf( stderr, ", debug_level=%d", req->debug_level ); fprintf( stderr, ", reply_fd=%d", req->reply_fd ); fprintf( stderr, ", wait_fd=%d", req->wait_fd ); @@ -1486,7 +1501,7 @@ static void dump_init_first_thread_reply( const struct init_first_thread_reply * static void dump_init_thread_request( const struct init_thread_request *req ) { - fprintf( stderr, " unix_tid=%d", req->unix_tid ); + dump_int64( " unix_tid=", &req->unix_tid ); fprintf( stderr, ", reply_fd=%d", req->reply_fd ); fprintf( stderr, ", wait_fd=%d", req->wait_fd ); dump_uint64( ", teb=", &req->teb ); @@ -1619,7 +1634,7 @@ static void dump_get_thread_times_reply( const struct get_thread_times_reply *re dump_timeout( " creation_time=", &req->creation_time ); dump_timeout( ", exit_time=", &req->exit_time ); fprintf( stderr, ", unix_pid=%d", req->unix_pid ); - fprintf( stderr, ", unix_tid=%d", req->unix_tid ); + dump_int64( ", unix_tid=", &req->unix_tid ); } static void dump_set_thread_info_request( const struct set_thread_info_request *req ) diff --git a/tools/make_requests b/tools/make_requests index 77b5ab331f4..04cbbc5a650 100755 --- a/tools/make_requests +++ b/tools/make_requests @@ -35,6 +35,7 @@ my %formats = "user_handle_t" => [ 4, 4, "%08x" ], "process_id_t" => [ 4, 4, "%04x" ], "thread_id_t" => [ 4, 4, "%04x" ], + "unix_tid_t" => [ 8, 8, "&dump_int64" ], "client_ptr_t" => [ 8, 8, "&dump_uint64" ], "mod_handle_t" => [ 8, 8, "&dump_uint64" ], "lparam_t" => [ 8, 8, "&dump_uint64" ], @@ -70,7 +71,7 @@ my %formats = "struct object_type_info" => [ 44, 4 ], "struct process_info" => [ 40, 8 ], "struct rawinput_device" => [ 12, 4 ], - "struct thread_info" => [ 40, 8 ], + "struct thread_info" => [ 48, 8 ], ); my @requests = (); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5047
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=142893 Your paranoid android. === debian11 (build log) === ../wine/include/winnt.h:424:21: error: static assertion failed: "sizeof(struct thread_info) == 48" Task: The win32 Wine build failed === debian11b (build log) === ../wine/include/winnt.h:424:21: error: static assertion failed: "sizeof(struct thread_info) == 48" Task: The wow32 Wine build failed
Now with `%llu` instead of `%lu` to get rid of the formatting warnings. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5047#note_60513
Brendan Shanks (@bshanks) commented about dlls/ntdll/unix/server.c:
#elif defined(__NetBSD__) ret = _lwp_self(); #elif defined(__FreeBSD__) - long lwpid; - thr_self( &lwpid ); - ret = lwpid; + thr_self( &ret ); I think this FreeBSD codepath can be removed (maybe in a separate commit), FreeBSD has supported `pthread_getthreadid_np()` since 9.0.
(Or if it stays in, I don't think this change is correct, `long` on 32-bit FreeBSD is still 32-bit.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5047#note_60521
participants (4)
-
Brendan Shanks (＠bshanks) -
Marc-Aurel Zent
-
Marc-Aurel Zent (＠mzent)
-
Marvin