From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26963 invoked by alias); 17 Sep 2011 15:47:08 -0000 Received: (qmail 26955 invoked by uid 22791); 17 Sep 2011 15:47:06 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from barfooze.de (HELO barfooze.de) (78.46.117.212) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 17 Sep 2011 15:46:49 +0000 Received: from xdsl-188-155-204-78.adslplus.ch ([188.155.204.78] helo=[172.16.0.230]) by barfooze.de with esmtpsa (TLSv1:CAMELLIA256-SHA:256) (Exim 4.76) (envelope-from ) id 1R4x6D-0001F1-Qs; Sat, 17 Sep 2011 17:46:47 +0200 Message-ID: <4E74C031.8050603@barfooze.de> Date: Sun, 18 Sep 2011 00:11:00 -0000 From: John Spencer User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.14) Gecko/20110221 SUSE/3.1.8 Mail/1.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org CC: Joel Brobecker Subject: Re: wrong assumptions about pthread_t being numeric References: <4E73D06F.603@barfooze.de> <201109170000.43306.pedro@codesourcery.com> <4E73D806.2040302@barfooze.de> <201109170130.42276.pedro@codesourcery.com> <4E73F1A4.2020606@barfooze.de> <20110917022840.GD17681@adacore.com> In-Reply-To: <20110917022840.GD17681@adacore.com> Content-Type: multipart/mixed; boundary="------------040809010105090403090808" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-09/txt/msg00345.txt.bz2 This is a multi-part message in MIME format. --------------040809010105090403090808 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1270 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 --------------040809010105090403090808 Content-Type: text/plain; name="commit-1" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="commit-1" Content-length: 3348 commit 5254245d1e3dbbe43c23bbec2c67e10525f0155a Author: John Spencer 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 @@ -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 #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 + --------------040809010105090403090808--