From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20297 invoked by alias); 5 Oct 2011 08:02:37 -0000 Received: (qmail 20286 invoked by uid 22791); 5 Oct 2011 08:02:36 -0000 X-SWARE-Spam-Status: No, hits=-2.2 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; Wed, 05 Oct 2011 08:02:18 +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 1RBMQZ-00022Q-5v; Wed, 05 Oct 2011 10:02:17 +0200 Message-ID: <4E8C0E7C.2030406@barfooze.de> Date: Wed, 05 Oct 2011 08:02: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: Pedro Alves Subject: Re: wrong assumptions about pthread_t being numeric References: <4E73D06F.603@barfooze.de> <201109171629.09383.pedro@codesourcery.com> <4E8665ED.4070905@barfooze.de> <201110010300.00160.pedro@codesourcery.com> In-Reply-To: <201110010300.00160.pedro@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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-10/txt/msg00125.txt.bz2 On 10/01/2011 03:59 AM, Pedro Alves wrote: > On Saturday 01 October 2011 01:59:25, John Spencer wrote: >>>>> i disagree. adding a proper solution once is superior to creating dozens >>>>> of special case hacks. >>>> What's the "proper" solution then? The only possible way I can think >>>> of to handle some libc what uses a struct for pthread_t, is with some way at >>>> runtime to know which libc/thread_db gdb is talking to, failing that, >>>> #ifdefs and/or autoconfig'ury. Since a simple cast will >>>> work for glibc, android, musl, and whatever other libc's happen be >>>> be building gdbserver today, what's the point? >>> I'll even go a step further, and suggest you do expose pthread_t >>> as a typedef to long in public headers, and cast it to pointer internally >>> in musl, just like glibc and most other libc's do. Hiding the fact >>> that you implement things with a pointer could even be a good thing. >>> >>> It's just easier for everybody else that way. There's really no point in >>> being different for the sake of it. >>> >> that's definitely going to far, you can't dictate how libcs should >> handle an opaque type internally just to keep broken code working. > The fact that you're still calling gdb's libthread_db.so handling > code broken because of this only shows that you still missed the point > of the code you're talking about. Let me sum it up: > The debugger is coordinating with a libc-provided shared library, that > is used to peek at the libc's internal state. that's the disturbing part. there are cleaner ways to get a unique number without poking at libc internals. however, the error has been made and it is much effort to correct it, so let's continue patching for the time being. > pthread_t's type is > implementation defined. The debugger's code in question is interacting > with a specific libc implementation (glibc). For that fact, the > debugger's code in question can be seen as being part of "the > implementation" (as in the pthread_t's type being implementation > defined part). The fact that other libcs work with the same code > is a bonus. > > Again: > >> that's definitely going to far, you can't dictate how libcs should >> handle an opaque type internally just to keep broken code working. > I can at least try to persuade one libc author into sanity. :-) i'm not musl's author, just helping to get some important packages built ;) i doubt that the author will change his implementation, since the solution you proposed will induce a performance penalty on some compilers, and result in less strict compile-time type checks. > Ever heard of not inflicting unnecessary pain on others for no > good reason? You know people do sprinkle around printfs of > pthread_t's for debug purposes, don't you? Things like printing > printf ("%ld been here\n", pthread_self ()). I really don't see > the point of not being source compatible with glibc (and all other > linux libcs) when it's super easy to be so... > > Again: > >> that's definitely going to far, you can't dictate how libcs should >> handle an opaque type internally just to keep broken code working. > public header: > > typedef unsigned long pthread_t; > > musl implementation: > > struct pthread > { > whatever you have today; > }; > > int pthread_foo_public_function (pthread_t th, ...) > { > struct pthread *t = (pthread_t) th; > > // business as usual. > } > > There, how did that dictate how your libc handles its > opaque type _internally_ again? It shouldn't take more than > 15 minutes to change all of that in musl, and everyone lives > happy ever after. > >> there >> should be at least a explicit function/macro which takes a thread_t and >> converts it to long, since it is assumed in a couple of spots that it is >> of this type. >> that is exactly what my patch does. >> >> and as you wished, it fixes the current issue with minimal effort. > The patch has a number of problems (no biggie, just the usual for > someone not used to GNU code). I'll take a look if I still failed > to convince you to change musl instead. > yes, please take a look at the patch. it imposes no performance costs and makes the code more correct. also if a new platform shows up, one can simply add a single ifdef to the macro and all code accessing the thread_t in a numeric context will magically work. -- JS