Re: wrong assumptions about pthread_t being numeric

[John Spencer <maillist-gdbpatches@barfooze.de>]

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

On 09/17/2011 04:28 AM, Joel Brobecker wrote:
>> i disagree. adding a proper solution once is superior to creating
>> dozens of special case hacks.  also it saves a lot of time in the long
>> term.
> Based on my experience so far, I have to agree with Pedro.
> But if you do have a superior solution, we'll take a look.
> I still have to tell you that I won't like it if your solution
> makes the code harder to read. Sometimes, that's OK because it
> helps achieving portability.  But when there is no such need for
> portability, it just gets in the way. Let's also be pragmatic,
> here and fix the problems that we know we have.
>
it would be nice to have an approach which would just work anywhere and 
respect the spec,
however i currently have not the resources to implement it.

thus, as you suggested, i came up with a small patch fixing the issue at 
hand. the solution should work on any sys/libc combination where 
sizeof(long) == sizeof(void*) and pthread_t is either a pointer or an 
int with sizeof equal or less than sizeof long.
this should cover all currently existing combinations.

i may find more spots in the future which need the application of the 
macro, and will eventually be sending more patches applying the 
conversion macro to it.

-- JS



[-- Attachment #2: commit-1 --]
[-- Type: text/plain, Size: 3348 bytes --]

commit 5254245d1e3dbbe43c23bbec2c67e10525f0155a
Author: John Spencer <maillist-gdbpatches@barfooze.de>
Date:   Sat Sep 17 17:21:58 2011 +0200

    use an explicit macro for pthread_t to number conversion.

diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index 529516e..200e9e7 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -29,6 +29,7 @@ static int thread_db_use_events;
 
 #include "gdb_proc_service.h"
 #include "../gdb_thread_db.h"
+#include "../threadtol.h"
 
 #ifndef USE_LIBTHREAD_DB_DIRECTLY
 #include <dlfcn.h>
@@ -290,7 +291,7 @@ find_one_thread (ptid_t ptid)
 
   if (debug_threads)
     fprintf (stderr, "Found thread %ld (LWP %d)\n",
-	     ti.ti_tid, ti.ti_lid);
+	     THREADTOL(ti.ti_tid), ti.ti_lid);
 
   if (lwpid != ti.ti_lid)
     {
@@ -309,7 +310,7 @@ find_one_thread (ptid_t ptid)
 
   /* If the new thread ID is zero, a final thread ID will be available
      later.  Do not enable thread debugging yet.  */
-  if (ti.ti_tid == 0)
+  if (THREADTOL(ti.ti_tid) == 0)
     return 0;
 
   lwp->thread_known = 1;
@@ -327,13 +328,13 @@ attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
 
   if (debug_threads)
     fprintf (stderr, "Attaching to thread %ld (LWP %d)\n",
-	     ti_p->ti_tid, ti_p->ti_lid);
+	     THREADTOL(ti_p->ti_tid), ti_p->ti_lid);
   linux_attach_lwp (ti_p->ti_lid);
   lwp = find_lwp_pid (pid_to_ptid (ti_p->ti_lid));
   if (lwp == NULL)
     {
       warning ("Could not attach to thread %ld (LWP %d)\n",
-	       ti_p->ti_tid, ti_p->ti_lid);
+	       THREADTOL(ti_p->ti_tid), ti_p->ti_lid);
       return 0;
     }
 
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 005a34a..b7d6671 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -24,6 +24,7 @@
 #include <dlfcn.h>
 #include "gdb_proc_service.h"
 #include "gdb_thread_db.h"
+#include "threadtol.h"
 
 #include "bfd.h"
 #include "command.h"
@@ -1059,7 +1060,7 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
      if we change GDB to always have at least one thread in the thread
      list this will have to go somewhere else; maybe private == NULL
      until the thread_db target claims it.  */
-  gdb_assert (ti_p->ti_tid != 0);
+  gdb_assert (THREADTOL(ti_p->ti_tid) != 0);
   private->th = *th_p;
   private->tid = ti_p->ti_tid;
 
@@ -1335,7 +1336,7 @@ find_new_threads_callback (const td_thrhandle_t *th_p, void *data)
   if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
     return 0;			/* A zombie -- ignore.  */
 
-  if (ti.ti_tid == 0 && target_has_execution)
+  if (THREADTOL(ti.ti_tid) == 0 && target_has_execution)
     {
       /* A thread ID of zero means that this is the main thread, but
 	 glibc has not yet initialized thread-local storage and the
diff --git a/gdb/threadtol.h b/gdb/threadtol.h
new file mode 100644
index 0000000..667b747
--- /dev/null
+++ b/gdb/threadtol.h
@@ -0,0 +1,13 @@
+#ifndef THREADTOL_H
+#define THREADTOL_H
+
+/* macro for conversion of a pthread_t to a long, for numerical representation.
+ * pthread_t is a opaque type and it is in fact illegal to use it in a numeric
+ * context according to POSIX, but all libc implementations in existence
+ * use either a pointer or a number, so its safe to cast it to long 
+ * for the moment. */
+
+#define THREADTOL(X) ((long) (X))
+
+#endif
+