Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: John Spencer <maillist-gdbpatches@barfooze.de>
To: gdb-patches@sourceware.org
Cc: Pedro Alves <pedro@codesourcery.com>
Subject: Re: wrong assumptions about pthread_t being numeric
Date: Sat, 17 Sep 2011 01:23:00 -0000	[thread overview]
Message-ID: <4E73F1A4.2020606@barfooze.de> (raw)
In-Reply-To: <201109170130.42276.pedro@codesourcery.com>

On 09/17/2011 02:30 AM, Pedro Alves wrote:
> On Saturday 17 September 2011 00:13:10, John Spencer wrote:
>> On 09/17/2011 01:00 AM, Pedro Alves wrote:
>>> These are only built natively on solaris and aix respectively, so
>>> let's just leave them alone.
>>>
>> I expected it to be desirable for a product in industrial use to be
>> standard-compliant and not invoking undefined behavior.
> Those files are tied to those platforms' thread_db/libc implementations.
> There's absolutely no need to handle some other hipotetical libc that
> defines pthread_t diferently there.  If it appears, we'll handle it.

> Chances are, some other changes would be necessary to make it really
> work, not just build.
>

exactly. for example in musl's case it is wrong to compare the 
underlying type (which is a struct pointer) with 0.
in the implementation specific part it would practically be valid to 
compare it with
NULL, but that's still invalid regarding the definition in POSIX.
so the few places that use a thread id had to be changed everywhere, to 
not have dozens of special case
sections for any libc/OS combination...

also there are a few spots where the thread_t is numerically compared to 
some arbitrary int value
(i have not yet evaluated where those originate from). this also can't 
work when thread_t is no number.
this happens for example in the if statement leading to the posted 
printf format string errors.

i expect that the number of actual fixes would be small, but it might 
require another way to get to a thread id.

if the id is only used for textual representation, it would be 
sufficient if gdb would assign a new unique number
on each successful pthread_create.
if it is also used to find a thread internally, it had to have a list of 
tid numbers -> pthread_t.


>>>> thread-db.c: In function 'find_one_thread':
>>>> thread-db.c:295:7: error: format '%ld' expects type 'long int', but
>>>> argument 3 has type 'thread_t'
>>>> thread-db.c: In function 'attach_thread':
>>>> thread-db.c:335:7: error: format '%ld' expects type 'long int', but
>>>> argument 3 has type 'thread_t'
>>>> thread-db.c:341:9: error: format '%ld' expects type 'long int', but
>>>> argument 2 has type 'thread_t'
>>> So just cast it to long, and you're done.
>>>
>> pthread_t could legally be a struct, which you can't just cast to a long.
> No need to complicate things for an hipotetical scenario.  The set of
> libc's in existence is finite.  If we were to handle a struct pthread_t,
> we'd need to be able to print it, and so we'd need some libc specific
> way to do it, something autoconf'ed.  There's no need to invent work.
>
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.

-- JS


  reply	other threads:[~2011-09-17  1:05 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-16 23:01 John Spencer
2011-09-16 23:16 ` Pedro Alves
2011-09-17  0:31   ` John Spencer
2011-09-17  1:05     ` Pedro Alves
2011-09-17  1:23       ` John Spencer [this message]
2011-09-17  2:29         ` Matt Rice
2011-09-17  6:47         ` Joel Brobecker
2011-09-18  0:11           ` John Spencer
2011-09-29  2:31             ` John Spencer
2011-09-29  8:39               ` Kai Tietz
2011-10-01  1:08                 ` John Spencer
2011-09-29 11:10               ` Pedro Alves
2011-09-17 15:29         ` Pedro Alves
2011-09-17 15:47           ` Pedro Alves
2011-10-01  1:02             ` John Spencer
2011-10-01  2:00               ` Pedro Alves
2011-10-01  9:00                 ` Mark Kettenis
2011-10-01  9:14                   ` Kevin Pouget
2011-10-05  8:02                 ` John Spencer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E73F1A4.2020606@barfooze.de \
    --to=maillist-gdbpatches@barfooze.de \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox