[rfa] Include the LWP in thread-db's PTIDs

[rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

At one time, I believe that thread-db.c was planned to support the full
range of features supported by the libthread_db interface, presumably as
defined by Sun's implementation.  That never panned out, and while non-1:1
support did work at one point, I don't think it has in a long while.  If it
was wanted, I wouldn't re-implement it the same way.  So this patch begins
the process of removing unneeded generality from thread-db.  In particular,
while thread-db will still compute the TID, the mapping of threads to LWPs
will be considered fixed.

My goal is to have a GNU/Linux target vector, whose entry points call into
thread-db when necessary, instead of having a thread-db wrapper around all
the GNU/Linux methods.  One of the things this will fix is the need for two
separate versions of the GNU/Linux native wait() code - we will always use
the multi-threaded-aware version.  Another thing it will fix is a bug in the
fork-following code which tries to find the LWP from a thread ID.

This patch tested on i686-pc-linux-gnu using NPTL; no regressions. OK?

-- 
Daniel Jacobowitz

2004-10-10  Daniel Jacobowitz  <dan@debian.org>

	* thread-db.c (BUILD_THREAD): Delete.
	(thread_get_info_callback): Include LWP ID in thread ID.
	(thread_from_lwp, check_event, find_new_threads_callback): Likewise.
	(lwp_from_thread): Use the LWP ID from the thread ID.

Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.46
diff -u -p -r1.46 thread-db.c
--- thread-db.c	8 Oct 2004 20:29:56 -0000	1.46
+++ thread-db.c	10 Oct 2004 21:23:44 -0000
@@ -143,7 +143,6 @@ static void detach_thread (ptid_t ptid, 
 #define is_thread(ptid)		(GET_THREAD (ptid) != 0)
 
 #define BUILD_LWP(lwp, pid)	ptid_build (pid, lwp, 0)
-#define BUILD_THREAD(tid, pid)	ptid_build (pid, 0, tid)
 \f
 
 /* Use "struct private_thread_info" to cache thread state.  This is
@@ -267,7 +266,7 @@ thread_get_info_callback (const td_thrha
 	   thread_db_err_str (err));
 
   /* Fill the cache.  */
-  thread_ptid = BUILD_THREAD (ti.ti_tid, GET_PID (inferior_ptid));
+  thread_ptid = ptid_build (GET_PID (inferior_ptid), ti.ti_lid, ti.ti_tid);
   thread_info = find_thread_pid (thread_ptid);
 
   /* In the case of a zombie thread, don't continue.  We don't want to
@@ -385,22 +384,14 @@ thread_from_lwp (ptid_t ptid)
 
   gdb_assert (thread_info && thread_info->private->ti_valid);
 
-  return BUILD_THREAD (thread_info->private->ti.ti_tid, GET_PID (ptid));
+  return ptid_build (GET_PID (ptid), GET_LWP (ptid),
+		     thread_info->private->ti.ti_tid);
 }
 
 static ptid_t
 lwp_from_thread (ptid_t ptid)
 {
-  struct thread_info *thread_info;
-  ptid_t thread_ptid;
-
-  if (!is_thread (ptid))
-    return ptid;
-
-  thread_info = find_thread_pid (ptid);
-  thread_db_get_info (thread_info);
-
-  return BUILD_LWP (thread_info->private->ti.ti_lid, GET_PID (ptid));
+  return BUILD_LWP (GET_LWP (ptid), GET_PID (ptid));
 }
 \f
 
@@ -903,7 +894,7 @@ check_event (ptid_t ptid)
       if (err != TD_OK)
 	error ("Cannot get thread info: %s", thread_db_err_str (err));
 
-      ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
+      ptid = ptid_build (GET_PID (ptid), ti.ti_lid, ti.ti_tid);
 
       switch (msg.event)
 	{
@@ -1175,7 +1166,7 @@ find_new_threads_callback (const td_thrh
   if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
     return 0;			/* A zombie -- ignore.  */
 
-  ptid = BUILD_THREAD (ti.ti_tid, GET_PID (inferior_ptid));
+  ptid = ptid_build (GET_PID (inferior_ptid), ti.ti_lid, ti.ti_tid);
 
   if (!in_thread_list (ptid))
     attach_thread (ptid, th_p, &ti, 1);

Re: [rfa] Include the LWP in thread-db's PTIDs

[Andrew Cagney]

> At one time, I believe that thread-db.c was planned to support the full
> range of features supported by the libthread_db interface, presumably as
> defined by Sun's implementation.  That never panned out, and while non-1:1
> support did work at one point, I don't think it has in a long while.  If it
> was wanted, I wouldn't re-implement it the same way.  So this patch begins
> the process of removing unneeded generality from thread-db.  In particular,
> while thread-db will still compute the TID, the mapping of threads to LWPs
> will be considered fixed.

JeffJ's been in a constant fight with that one.

> My goal is to have a GNU/Linux target vector, whose entry points call into
> thread-db when necessary, instead of having a thread-db wrapper around all
> the GNU/Linux methods.  One of the things this will fix is the need for two
> separate versions of the GNU/Linux native wait() code - we will always use
> the multi-threaded-aware version.  Another thing it will fix is a bug in the
> fork-following code which tries to find the LWP from a thread ID.

Per the changes I've been making, yes, there needs to be a single 
inf-linux inferior (derived from inf-ptrace?) that always has the LWP 
code enabled(1).

thread-db is more interesting.  As a user-level thread model, yes it is 
GNU/Linux specific and should be consolidated - linux-nptl say?

However, as with many systems, GNU/Linux needs to be able to support 
multiple user-level thread models (e.g., Ada's tasks), and be able to 
layer each of those user-level thread models over more than just 
inf-linux (esp corefiles).  Consequently, linux-nptl can't be folded 
into inf-linux, and the indirection provided by the thread-stratum needs 
to be retained.

Andrew

(1) Have you noticed now the lin_lwp inferior uses /proc for memory 
accesses yet the default vector does not?

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Mon, Oct 11, 2004 at 11:26:27AM -0400, Andrew Cagney wrote:
> >At one time, I believe that thread-db.c was planned to support the full
> >range of features supported by the libthread_db interface, presumably as
> >defined by Sun's implementation.  That never panned out, and while non-1:1
> >support did work at one point, I don't think it has in a long while.  If it
> >was wanted, I wouldn't re-implement it the same way.  So this patch begins
> >the process of removing unneeded generality from thread-db.  In particular,
> >while thread-db will still compute the TID, the mapping of threads to LWPs
> >will be considered fixed.
> 
> JeffJ's been in a constant fight with that one.

Yes, so have I.  I have at least two pending patches (not posted yet)
that depend on fixing this.

> >My goal is to have a GNU/Linux target vector, whose entry points call into
> >thread-db when necessary, instead of having a thread-db wrapper around all
> >the GNU/Linux methods.  One of the things this will fix is the need for two
> >separate versions of the GNU/Linux native wait() code - we will always use
> >the multi-threaded-aware version.  Another thing it will fix is a bug in 
> >the
> >fork-following code which tries to find the LWP from a thread ID.
> 
> Per the changes I've been making, yes, there needs to be a single 
> inf-linux inferior (derived from inf-ptrace?) that always has the LWP 
> code enabled(1).

Yes, I know you've been working on this.  Therefore I'm working on
something related but approaching from the opposite side - fixing the
LWP vector.  Hopefully we'll help each other instead of tripping over
each other.

> thread-db is more interesting.  As a user-level thread model, yes it is 
> GNU/Linux specific and should be consolidated - linux-nptl say?

It's not NPTL specific; the bits of thread_db we need to be using
work for both LinuxThreads and NPTL.

> However, as with many systems, GNU/Linux needs to be able to support 
> multiple user-level thread models (e.g., Ada's tasks), and be able to 
> layer each of those user-level thread models over more than just 
> inf-linux (esp corefiles).  Consequently, linux-nptl can't be folded 
> into inf-linux, and the indirection provided by the thread-stratum needs 
> to be retained.

I hadn't thought about the core issue; I'll do some pondering. However,
I don't think your comment is quite right.  Thread_db can not be
layered over core files, we've already decided that - it's too iffy to
find the right thread_db, not to mention cross-debugging issues.  And
similarly we can't use it for remote thread debugging.  Thread_db only
makes any sense on top of local, running, native threads.

If any of the higher level features of thread_db, like mutex
information, were usable on GNU/Linux, then a mode in which we
associated a local thread_db with a core file might be interesting.  But
this isn't the case today; none of that is implemented in glibc.

Ada tasks are actually a very interesting example, since IIUC they are
layered on top of POSIX threads.  Joel, have I got that right?  So if
we use a thread strata for libthread_db, we'd need two thread strata.

> (1) Have you noticed now the lin_lwp inferior uses /proc for memory 
> accesses yet the default vector does not?

Yes, I wrote that code.  I've never had time to consolidate it and this
seems like a good chance.  It was for a very specific problem which
only mattered when thread_db was in use - thread_db triggers single
reads of excessively large size.

-- 
Daniel Jacobowitz

Re: [rfa] Include the LWP in thread-db's PTIDs

[Joel Brobecker]

> Ada tasks are actually a very interesting example, since IIUC they are
> layered on top of POSIX threads.  Joel, have I got that right?

Yes. Or maybe more precisely, the threads library does not have to
be POSIX, AFAIK. But basically, Ada tasking is usually implemented
using the system threads.

-- 
Joel

Re: [rfa] Include the LWP in thread-db's PTIDs

[Andrew Cagney]

> I hadn't thought about the core issue; I'll do some pondering. However,
> I don't think your comment is quite right.  Thread_db can not be
> layered over core files, we've already decided that - it's too iffy to
> find the right thread_db, not to mention cross-debugging issues.  And
> similarly we can't use it for remote thread debugging.  Thread_db only
> makes any sense on top of local, running, native threads.

"we"'ve definitly not decided this.

Long ago you committed a hack to stop GDB layering thread-db over core 
files.  It was to stop GDB barfing on native GNU/Linux core files.  It 
had the side effect of breaking threads on all other systems, namely 
solaris.  What keeps being pointing out is that thread-db should be 
loaded over a core file, and not doing it is broken.

If we try it and it barfs, we've a bug.  But what we've not got is an 
excuse for hobble native support (just because embedeed debuging is "iffy").

Andrew

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Mon, Oct 11, 2004 at 12:16:46PM -0400, Andrew Cagney wrote:
> >I hadn't thought about the core issue; I'll do some pondering. However,
> >I don't think your comment is quite right.  Thread_db can not be
> >layered over core files, we've already decided that - it's too iffy to
> >find the right thread_db, not to mention cross-debugging issues.  And
> >similarly we can't use it for remote thread debugging.  Thread_db only
> >makes any sense on top of local, running, native threads.
> 
> "we"'ve definitly not decided this.
> 
> Long ago you committed a hack to stop GDB layering thread-db over core 
> files.  It was to stop GDB barfing on native GNU/Linux core files.  It 
> had the side effect of breaking threads on all other systems, namely 
> solaris.  What keeps being pointing out is that thread-db should be 
> loaded over a core file, and not doing it is broken.
> 
> If we try it and it barfs, we've a bug.  But what we've not got is an 
> excuse for hobble native support (just because embedeed debuging is "iffy").

Huh?  It was a change to thread-db.c which has never been used for
Solaris, so I haven't got any idea what you are talking about.  I did
not break Solaris threads.

Also, it was an approved patch.  Michael responded at the end of the
thread saying that he agreed it was the right thing not to use
thread_db on core files.  Yes, there was a lot of disagreement before
that; but before the patch was committed the thread-db.c maintainer
agreed that we should not to use thread_db in this case.  I think I'm
justified in saying that "we" have decided this.

-- 
Daniel Jacobowitz

Re: [rfa] Include the LWP in thread-db's PTIDs

[Andrew Cagney]

> On Mon, Oct 11, 2004 at 12:16:46PM -0400, Andrew Cagney wrote:
> 
>>>> >I hadn't thought about the core issue; I'll do some pondering. However,
>>>> >I don't think your comment is quite right.  Thread_db can not be
>>>> >layered over core files, we've already decided that - it's too iffy to
>>>> >find the right thread_db, not to mention cross-debugging issues.  And
>>>> >similarly we can't use it for remote thread debugging.  Thread_db only
>>>> >makes any sense on top of local, running, native threads.
>>
>>> 
>>> "we"'ve definitly not decided this.
>>> 
>>> Long ago you committed a hack to stop GDB layering thread-db over core 
>>> files.  It was to stop GDB barfing on native GNU/Linux core files.  It 
>>> had the side effect of breaking threads on all other systems, namely 
>>> solaris.  What keeps being pointing out is that thread-db should be 
>>> loaded over a core file, and not doing it is broken.
>>> 
>>> If we try it and it barfs, we've a bug.  But what we've not got is an 
>>> excuse for hobble native support (just because embedeed debuging is "iffy").
> 
> 
> Huh?  It was a change to thread-db.c which has never been used for
> Solaris, so I haven't got any idea what you are talking about.  I did
> not break Solaris threads.

Then on that front, then I'm now wrong (and not shy in admitting it ;-) 
("but" in my defence at the time the expectation was that thread-db.c 
would be used by solaris :-).

> Also, it was an approved patch.  Michael responded at the end of the
> thread saying that he agreed it was the right thing not to use
> thread_db on core files.  Yes, there was a lot of disagreement before
> that; but before the patch was committed the thread-db.c maintainer
> agreed that we should not to use thread_db in this case.  I think I'm
> justified in saying that "we" have decided this.

But here on this technical matter I'm not.

The discussion starts here (there's a bit in the next month):
http://sources.redhat.com/ml/gdb-patches/2001-12/msg00345.html
please read it.

Your position was that:

> On Thu, Dec 13, 2001 at 06:56:36PM -0500, Daniel Jacobowitz wrote:
>> Thread_db, as things stand, does not work on core files.  Is preventing
>> it from trying, and thus crashing GDB, really such a disruptive
>> suggestion?
> 
> OK, that came out a little harsher than I really wanted it.  Sorry.
> 
> I'd like to apply this patch and then add an entry to TODO about
> how it "should be done".  Is that better?

because:

>> Sounds like that is the bug to fix.  Enabling event reporting probably 
>> doesn't make much sense when the target is lifeless.
> 
> I was about to try a patch for this when I realized that my primary
> objection still holds.
> 
> This only works if you're debugging on a very similar host to the one
> the core was dumped on.  If you've got, say, a glibc 2.1.3 host and are
> looking at a glibc 2.2.3 core... well, you can provide target libraries
> and make GDB use those, but there's no way to provide a cross
> libthread_db.

Given that this was "hard" Michael approved the change (a sound 
technical decision).

Just like on Solaris and consistent with the original thread, the 
user-level thread library should be loaded over the core file as only by 
doing that can user-level thread information be displayed.

If this makes cross debugging "iffy", fix the bugs, don't cripple GDB 
for it's native users.

Andrew

Re: [rfa] Include the LWP in thread-db's PTIDs

[Mark Kettenis]

   Date: Mon, 11 Oct 2004 11:26:27 -0400
   From: Andrew Cagney <cagney@gnu.org>

   > At one time, I believe that thread-db.c was planned to support
   > the full range of features supported by the libthread_db
   > interface, presumably as defined by Sun's implementation.  That
   > never panned out, and while non-1:1 support did work at one
   > point, I don't think it has in a long while.  If it was wanted, I
   > wouldn't re-implement it the same way.  So this patch begins the
   > process of removing unneeded generality from thread-db.  In
   > particular, while thread-db will still compute the TID, the
   > mapping of threads to LWPs will be considered fixed.

I think assuming 1:1 threads is fair enough.  But Daniel, if you're
going to do that, I'd like to ask you to rename thread-db.c into
linux-thread.c or something like that.

Anyway, in that case I think linux-thread.c should be a fairly
lightweight layer around the normal Linux LWP-aware
inf-ptrace.c-derived target vector.  Only use it to provide an LWP <->
TID mapping, and perhaps an initial list of LWPs to attach, but
nothing else.  The normal Linux target vector should in no way be
dependent on it, such that it allows alternative threads strata on top
of it.  Something else just would be a bad design.

Hmm, I guess I'm just re-stating Andrew's points here.

   JeffJ's been in a constant fight with that one.

That's because we try to support every combination of a broken glibc
and a broken kernel that we can find out there.  Well, perhaps not
every possible combination, but certainly some broken combinations.
Reliable threads-debugging requires reliable reporting of thread/LWP
exits.  Otherwise we keep having problems with threads disappearing
from beneath us, leaving us poking at nonexistent memory and
registers.

I think we should ditch most of the buggy combinations and state that
Linux threads debugging will only work reliably on Linux x.y.z or
later with glibc a.b.c or later.

Mark

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Mon, Oct 11, 2004 at 02:28:32PM -0400, Andrew Cagney wrote:
> >Also, it was an approved patch.  Michael responded at the end of the
> >thread saying that he agreed it was the right thing not to use
> >thread_db on core files.  Yes, there was a lot of disagreement before
> >that; but before the patch was committed the thread-db.c maintainer
> >agreed that we should not to use thread_db in this case.  I think I'm
> >justified in saying that "we" have decided this.
> 
> But here on this technical matter I'm not.
> 
> The discussion starts here (there's a bit in the next month):
> http://sources.redhat.com/ml/gdb-patches/2001-12/msg00345.html
> please read it.

Yes, I read that.

> Given that this was "hard" Michael approved the change (a sound 
> technical decision).
> 
> Just like on Solaris and consistent with the original thread, the 
> user-level thread library should be loaded over the core file as only by 
> doing that can user-level thread information be displayed.

Actually, Michael didn't explain why he changed his mind - he just
eventually agreed.  Since he's still right here we could ask him :-)
but let's just move on.

> If this makes cross debugging "iffy", fix the bugs, don't cripple GDB 
> for it's native users.

Some thinking about your messages has persuaded me that you're right
that we should support thread-db as a separate stratum and support
using it for core files (although I will remain opposed to doing it by
default unless someone adds a robust do-this-image-and-library-match
check - hmm, maybe it's OK if libc is found from the system location
instead of via solib-search-path and solib-absolute-prefix?).

However, please stop calling this a "crippling" issue.  Can you even
list the visible changes that using thread-db would cause over just
using corelow's LWP support?  As far as I know there are two:

  - Thread IDs would not be displayed.  Since these are pointer-sized
    numbers internal to the NPTL implementation, that's not much of a
    loss.

  - TLS data may not work, because we currently use thread_db to find
    the thread's TLS base.  This could be pretty easily fixed by using
    a gdbarch mechanism to return the architecture's TLS base and an
    xfer_partial gdbarch hook to query native or remote targets for the
    value.

-- 
Daniel Jacobowitz

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Mon, Oct 11, 2004 at 09:40:26PM +0200, Mark Kettenis wrote:
>    Date: Mon, 11 Oct 2004 11:26:27 -0400
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    > At one time, I believe that thread-db.c was planned to support
>    > the full range of features supported by the libthread_db
>    > interface, presumably as defined by Sun's implementation.  That
>    > never panned out, and while non-1:1 support did work at one
>    > point, I don't think it has in a long while.  If it was wanted, I
>    > wouldn't re-implement it the same way.  So this patch begins the
>    > process of removing unneeded generality from thread-db.  In
>    > particular, while thread-db will still compute the TID, the
>    > mapping of threads to LWPs will be considered fixed.
> 
> I think assuming 1:1 threads is fair enough.  But Daniel, if you're
> going to do that, I'd like to ask you to rename thread-db.c into
> linux-thread.c or something like that.

How about glibc-thread-db.c?

> Anyway, in that case I think linux-thread.c should be a fairly
> lightweight layer around the normal Linux LWP-aware
> inf-ptrace.c-derived target vector.  Only use it to provide an LWP <->
> TID mapping, and perhaps an initial list of LWPs to attach, but
> nothing else.  The normal Linux target vector should in no way be
> dependent on it, such that it allows alternative threads strata on top
> of it.  Something else just would be a bad design.

What I'm wondering is how to handle new threads.  There are two
options: have the kernel report them and lin-lwp report them to
thread-db, and have thread-db discover them and tell lin-lwp to attach
to them.  The former is much nicer but see below...

Anyway, I'm wondering if there should be some other method of
communication between the native and thread strata (as opposed to
between lin-lwp and thread_db!).  I don't much want to use to_wait for
this, because the assumption in the rest of GDB is that to_wait returns
when all threads are stopped and I don't want to gratuitously stop all
threads when we receive a creation event.

> That's because we try to support every combination of a broken glibc
> and a broken kernel that we can find out there.  Well, perhaps not
> every possible combination, but certainly some broken combinations.
> Reliable threads-debugging requires reliable reporting of thread/LWP
> exits.  Otherwise we keep having problems with threads disappearing
> from beneath us, leaving us poking at nonexistent memory and
> registers.
> 
> I think we should ditch most of the buggy combinations and state that
> Linux threads debugging will only work reliably on Linux x.y.z or
> later with glibc a.b.c or later.

Unfortunately, thread creation events only work reliably in 2.6.x
kernels.  2.4.x is still in wide enough use that I think there's value
in holding on to it.  Thread exit, on the other hand, works pretty
reliably - there were a series of RH NPTL kernels which couldn't report
it properly, but it worked fine for LinuxThreads and now it works fine
for NPTL too.

-- 
Daniel Jacobowitz

Re: [rfa] Include the LWP in thread-db's PTIDs

[Mark Kettenis]

   Date: Tue, 12 Oct 2004 09:31:40 -0400
   From: Daniel Jacobowitz <drow@false.org>

   On Mon, Oct 11, 2004 at 09:40:26PM +0200, Mark Kettenis wrote:
   > 
   > I think assuming 1:1 threads is fair enough.  But Daniel, if you're
   > going to do that, I'd like to ask you to rename thread-db.c into
   > linux-thread.c or something like that.

   How about glibc-thread-db.c?

I'm not sure whether that's appropriate.  NPTL is defenitely
Linux-specific.  The Hurd doesn't have a thread_db yet.  I suppose
that if you're going to make some Linux-specific assumptions
glibc-thread-db.c isn't right.

   > Anyway, in that case I think linux-thread.c should be a fairly
   > lightweight layer around the normal Linux LWP-aware
   > inf-ptrace.c-derived target vector.  Only use it to provide an LWP <->
   > TID mapping, and perhaps an initial list of LWPs to attach, but
   > nothing else.  The normal Linux target vector should in no way be
   > dependent on it, such that it allows alternative threads strata on top
   > of it.  Something else just would be a bad design.

   What I'm wondering is how to handle new threads.  There are two
   options: have the kernel report them and lin-lwp report them to
   thread-db, and have thread-db discover them and tell lin-lwp to attach
   to them.  The former is much nicer but see below...

Agreed.  If the LWP-layer could discover new LWP's without the help of
a threads-layer, all possible threads layers would benefit.

   Anyway, I'm wondering if there should be some other method of
   communication between the native and thread strata (as opposed to
   between lin-lwp and thread_db!).  I don't much want to use to_wait for
   this, because the assumption in the rest of GDB is that to_wait returns
   when all threads are stopped and I don't want to gratuitously stop all
   threads when we receive a creation event.

But you don't have to return from to_wait upon a creation event.  But
we probably should if the user asks us to report new threads.  The
current way of printing [new thread] messages is a bit broken, since
GDB doesn't always have control over the terminal.

   > That's because we try to support every combination of a broken glibc
   > and a broken kernel that we can find out there.  Well, perhaps not
   > every possible combination, but certainly some broken combinations.
   > Reliable threads-debugging requires reliable reporting of thread/LWP
   > exits.  Otherwise we keep having problems with threads disappearing
   > from beneath us, leaving us poking at nonexistent memory and
   > registers.
   > 
   > I think we should ditch most of the buggy combinations and state that
   > Linux threads debugging will only work reliably on Linux x.y.z or
   > later with glibc a.b.c or later.

   Unfortunately, thread creation events only work reliably in 2.6.x
   kernels.  2.4.x is still in wide enough use that I think there's value
   in holding on to it.  Thread exit, on the other hand, works pretty
   reliably - there were a series of RH NPTL kernels which couldn't report
   it properly, but it worked fine for LinuxThreads and now it works fine
   for NPTL too.

Thread creation is less critical than thread exit.  It doesn't matter
if you don't access something that's already there. It does matter if
you try to access something that's no longer there.  Akthough hitting
a breakpoint in a thread that we don't know about yet can be a little
tricky.

It might be a good idea though to clearly distinguish between kernels
where thread creation is reported correctly and kernels where it
isn't.

Mark

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Wed, Oct 13, 2004 at 11:16:05PM +0200, Mark Kettenis wrote:
>    Date: Tue, 12 Oct 2004 09:31:40 -0400
>    From: Daniel Jacobowitz <drow@false.org>
> 
>    On Mon, Oct 11, 2004 at 09:40:26PM +0200, Mark Kettenis wrote:
>    > 
>    > I think assuming 1:1 threads is fair enough.  But Daniel, if you're
>    > going to do that, I'd like to ask you to rename thread-db.c into
>    > linux-thread.c or something like that.
> 
>    How about glibc-thread-db.c?
> 
> I'm not sure whether that's appropriate.  NPTL is defenitely
> Linux-specific.  The Hurd doesn't have a thread_db yet.  I suppose
> that if you're going to make some Linux-specific assumptions
> glibc-thread-db.c isn't right.

I was not planning on making any Linux-specific assumptions in the
thread-db code - just in lin-lwp.  I doubt glibc's thread_db
implementation is going to support non-1:1 threading, and that's the
only assumption I am planning on using in what is now thread-db.c.

But I don't know much about threading for the only active non-Linux
users of glibc (the kBSD projects and the Hurd).  Do you think that the
1:1 assumption is Linux-specific rather than glibc-specific?  If so,
linux-thread-db.c would be a better choice indeed, and I can use that.

>    > Anyway, in that case I think linux-thread.c should be a fairly
>    > lightweight layer around the normal Linux LWP-aware
>    > inf-ptrace.c-derived target vector.  Only use it to provide an LWP <->
>    > TID mapping, and perhaps an initial list of LWPs to attach, but
>    > nothing else.  The normal Linux target vector should in no way be
>    > dependent on it, such that it allows alternative threads strata on top
>    > of it.  Something else just would be a bad design.
> 
>    What I'm wondering is how to handle new threads.  There are two
>    options: have the kernel report them and lin-lwp report them to
>    thread-db, and have thread-db discover them and tell lin-lwp to attach
>    to them.  The former is much nicer but see below...
> 
> Agreed.  If the LWP-layer could discover new LWP's without the help of
> a threads-layer, all possible threads layers would benefit.
> 
>    Anyway, I'm wondering if there should be some other method of
>    communication between the native and thread strata (as opposed to
>    between lin-lwp and thread_db!).  I don't much want to use to_wait for
>    this, because the assumption in the rest of GDB is that to_wait returns
>    when all threads are stopped and I don't want to gratuitously stop all
>    threads when we receive a creation event.
> 
> But you don't have to return from to_wait upon a creation event.  But
> we probably should if the user asks us to report new threads.  The
> current way of printing [new thread] messages is a bit broken, since
> GDB doesn't always have control over the terminal.

Indeed, we don't have to return from to_wait after thread creation. 
However, if there are thread strata above us, we want to report the new
thread to them promptly.  For instance, the thread-db layer should
always (when in use) sit between lin-lwp and GDB for thread reporting,
so that threads are reported with the correct TID.

[It's a little more complicated than that, in this case.  The TID is
not available quite as early as the LWP is, so we have the LWP without
its corresponding thread.  So GDB might have to handle the thread IDs
changing during its lifetime; ew.]

> Thread creation is less critical than thread exit.  It doesn't matter
> if you don't access something that's already there. It does matter if
> you try to access something that's no longer there.  Akthough hitting
> a breakpoint in a thread that we don't know about yet can be a little
> tricky.

More than just tricky - in NPTL it is fatal.  This is one of the few
points where I agree that the threading model is clinically broken.
If a thread that the debugger is not attached to takes a fatal signal,
the thread group will exit.

> It might be a good idea though to clearly distinguish between kernels
> where thread creation is reported correctly and kernels where it
> isn't.

Distinguish in what sense?  Use a different implementation?  We already
test at run-time whether we can expect thread creation events; it's in
linux-nat.c.

-- 
Daniel Jacobowitz

Re: [rfa] Include the LWP in thread-db's PTIDs

[Paul Gilliam]

On Monday 11 October 2004 12:40, Mark Kettenis wrote:

> I think assuming 1:1 threads is fair enough.  But Daniel, if you're
> going to do that, I'd like to ask you to rename thread-db.c into
> linux-thread.c or something like that.
>

How about one2one-thread.c?

-=# Paul Gilliam #=-

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Wed, Oct 13, 2004 at 05:27:15PM -0400, Daniel Jacobowitz wrote:
> On Wed, Oct 13, 2004 at 11:16:05PM +0200, Mark Kettenis wrote:
> >    Date: Tue, 12 Oct 2004 09:31:40 -0400
> >    From: Daniel Jacobowitz <drow@false.org>
> > 
> >    On Mon, Oct 11, 2004 at 09:40:26PM +0200, Mark Kettenis wrote:
> >    > 
> >    > I think assuming 1:1 threads is fair enough.  But Daniel, if you're
> >    > going to do that, I'd like to ask you to rename thread-db.c into
> >    > linux-thread.c or something like that.
> > 
> >    How about glibc-thread-db.c?
> > 
> > I'm not sure whether that's appropriate.  NPTL is defenitely
> > Linux-specific.  The Hurd doesn't have a thread_db yet.  I suppose
> > that if you're going to make some Linux-specific assumptions
> > glibc-thread-db.c isn't right.
> 
> I was not planning on making any Linux-specific assumptions in the
> thread-db code - just in lin-lwp.  I doubt glibc's thread_db
> implementation is going to support non-1:1 threading, and that's the
> only assumption I am planning on using in what is now thread-db.c.
> 
> But I don't know much about threading for the only active non-Linux
> users of glibc (the kBSD projects and the Hurd).  Do you think that the
> 1:1 assumption is Linux-specific rather than glibc-specific?  If so,
> linux-thread-db.c would be a better choice indeed, and I can use that.

Ping on the renaming question.

I think that I should take care of the renaming of this file before I
start adding assumptions to it, so I withdraw the patch at the
beginning of this thread until that has been taken care of.

> >    Anyway, I'm wondering if there should be some other method of
> >    communication between the native and thread strata (as opposed to
> >    between lin-lwp and thread_db!).  I don't much want to use to_wait for
> >    this, because the assumption in the rest of GDB is that to_wait returns
> >    when all threads are stopped and I don't want to gratuitously stop all
> >    threads when we receive a creation event.
> > 
> > But you don't have to return from to_wait upon a creation event.  But
> > we probably should if the user asks us to report new threads.  The
> > current way of printing [new thread] messages is a bit broken, since
> > GDB doesn't always have control over the terminal.
> 
> Indeed, we don't have to return from to_wait after thread creation. 
> However, if there are thread strata above us, we want to report the new
> thread to them promptly.  For instance, the thread-db layer should
> always (when in use) sit between lin-lwp and GDB for thread reporting,
> so that threads are reported with the correct TID.
> 
> [It's a little more complicated than that, in this case.  The TID is
> not available quite as early as the LWP is, so we have the LWP without
> its corresponding thread.  So GDB might have to handle the thread IDs
> changing during its lifetime; ew.]

I'm still thinking about this bit...

-- 
Daniel Jacobowitz

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Sun, Oct 10, 2004 at 05:36:30PM -0400, Daniel Jacobowitz wrote:
> At one time, I believe that thread-db.c was planned to support the full
> range of features supported by the libthread_db interface, presumably as
> defined by Sun's implementation.  That never panned out, and while non-1:1
> support did work at one point, I don't think it has in a long while.  If it
> was wanted, I wouldn't re-implement it the same way.  So this patch begins
> the process of removing unneeded generality from thread-db.  In particular,
> while thread-db will still compute the TID, the mapping of threads to LWPs
> will be considered fixed.
> 
> My goal is to have a GNU/Linux target vector, whose entry points call into
> thread-db when necessary, instead of having a thread-db wrapper around all
> the GNU/Linux methods.  One of the things this will fix is the need for two
> separate versions of the GNU/Linux native wait() code - we will always use
> the multi-threaded-aware version.  Another thing it will fix is a bug in the
> fork-following code which tries to find the LWP from a thread ID.
> 
> This patch tested on i686-pc-linux-gnu using NPTL; no regressions. OK?

Here's the patch, updated to apply to linux-thread-db.c instead.  Still
no regressions; Mark indicated that the 1:1 assumption seemed reasonable
once the file was marked as Linux-specific.

Michael, OK to commit?

-- 
Daniel Jacobowitz

2004-11-14  Daniel Jacobowitz  <dan@debian.org>

	* linux-thread-db.c (BUILD_THREAD): Delete.
	(thread_get_info_callback): Include LWP ID in thread ID.
	(thread_from_lwp, check_event, find_new_threads_callback): Likewise.
	(lwp_from_thread): Use the LWP ID from the thread ID.

Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.46
diff -u -p -r1.46 thread-db.c
--- linux-thread-db.c	8 Oct 2004 20:29:56 -0000	1.46
+++ linux-thread-db.c	10 Oct 2004 21:23:44 -0000
@@ -143,7 +143,6 @@ static void detach_thread (ptid_t ptid, 
 #define is_thread(ptid)		(GET_THREAD (ptid) != 0)
 
 #define BUILD_LWP(lwp, pid)	ptid_build (pid, lwp, 0)
-#define BUILD_THREAD(tid, pid)	ptid_build (pid, 0, tid)
 \f
 
 /* Use "struct private_thread_info" to cache thread state.  This is
@@ -267,7 +266,7 @@ thread_get_info_callback (const td_thrha
 	   thread_db_err_str (err));
 
   /* Fill the cache.  */
-  thread_ptid = BUILD_THREAD (ti.ti_tid, GET_PID (inferior_ptid));
+  thread_ptid = ptid_build (GET_PID (inferior_ptid), ti.ti_lid, ti.ti_tid);
   thread_info = find_thread_pid (thread_ptid);
 
   /* In the case of a zombie thread, don't continue.  We don't want to
@@ -385,22 +384,14 @@ thread_from_lwp (ptid_t ptid)
 
   gdb_assert (thread_info && thread_info->private->ti_valid);
 
-  return BUILD_THREAD (thread_info->private->ti.ti_tid, GET_PID (ptid));
+  return ptid_build (GET_PID (ptid), GET_LWP (ptid),
+		     thread_info->private->ti.ti_tid);
 }
 
 static ptid_t
 lwp_from_thread (ptid_t ptid)
 {
-  struct thread_info *thread_info;
-  ptid_t thread_ptid;
-
-  if (!is_thread (ptid))
-    return ptid;
-
-  thread_info = find_thread_pid (ptid);
-  thread_db_get_info (thread_info);
-
-  return BUILD_LWP (thread_info->private->ti.ti_lid, GET_PID (ptid));
+  return BUILD_LWP (GET_LWP (ptid), GET_PID (ptid));
 }
 \f
 
@@ -903,7 +894,7 @@ check_event (ptid_t ptid)
       if (err != TD_OK)
 	error ("Cannot get thread info: %s", thread_db_err_str (err));
 
-      ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
+      ptid = ptid_build (GET_PID (ptid), ti.ti_lid, ti.ti_tid);
 
       switch (msg.event)
 	{
@@ -1175,7 +1166,7 @@ find_new_threads_callback (const td_thrh
   if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
     return 0;			/* A zombie -- ignore.  */
 
-  ptid = BUILD_THREAD (ti.ti_tid, GET_PID (inferior_ptid));
+  ptid = ptid_build (GET_PID (inferior_ptid), ti.ti_lid, ti.ti_tid);
 
   if (!in_thread_list (ptid))
     attach_thread (ptid, th_p, &ti, 1);

Re: [rfa] Include the LWP in thread-db's PTIDs

[Michael Snyder]

Daniel Jacobowitz wrote:
> On Sun, Oct 10, 2004 at 05:36:30PM -0400, Daniel Jacobowitz wrote:
> 
>>At one time, I believe that thread-db.c was planned to support the full
>>range of features supported by the libthread_db interface, presumably as
>>defined by Sun's implementation.  That never panned out, and while non-1:1
>>support did work at one point, I don't think it has in a long while.  If it
>>was wanted, I wouldn't re-implement it the same way.  So this patch begins
>>the process of removing unneeded generality from thread-db.  In particular,
>>while thread-db will still compute the TID, the mapping of threads to LWPs
>>will be considered fixed.
>>
>>My goal is to have a GNU/Linux target vector, whose entry points call into
>>thread-db when necessary, instead of having a thread-db wrapper around all
>>the GNU/Linux methods.  One of the things this will fix is the need for two
>>separate versions of the GNU/Linux native wait() code - we will always use
>>the multi-threaded-aware version.  Another thing it will fix is a bug in the
>>fork-following code which tries to find the LWP from a thread ID.
>>
>>This patch tested on i686-pc-linux-gnu using NPTL; no regressions. OK?
> 
> 
> Here's the patch, updated to apply to linux-thread-db.c instead.  Still
> no regressions; Mark indicated that the 1:1 assumption seemed reasonable
> once the file was marked as Linux-specific.
> 
> Michael, OK to commit?

Hey, sorry for losing this thread.  Yes, this is OK to commit.

Re: [rfa] Include the LWP in thread-db's PTIDs

[Daniel Jacobowitz]

On Thu, Dec 02, 2004 at 01:15:59PM -0800, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> >On Sun, Oct 10, 2004 at 05:36:30PM -0400, Daniel Jacobowitz wrote:
> >
> >>At one time, I believe that thread-db.c was planned to support the full
> >>range of features supported by the libthread_db interface, presumably as
> >>defined by Sun's implementation.  That never panned out, and while non-1:1
> >>support did work at one point, I don't think it has in a long while.  If 
> >>it
> >>was wanted, I wouldn't re-implement it the same way.  So this patch begins
> >>the process of removing unneeded generality from thread-db.  In 
> >>particular,
> >>while thread-db will still compute the TID, the mapping of threads to LWPs
> >>will be considered fixed.
> >>
> >>My goal is to have a GNU/Linux target vector, whose entry points call into
> >>thread-db when necessary, instead of having a thread-db wrapper around all
> >>the GNU/Linux methods.  One of the things this will fix is the need for 
> >>two
> >>separate versions of the GNU/Linux native wait() code - we will always use
> >>the multi-threaded-aware version.  Another thing it will fix is a bug in 
> >>the
> >>fork-following code which tries to find the LWP from a thread ID.
> >>
> >>This patch tested on i686-pc-linux-gnu using NPTL; no regressions. OK?
> >
> >
> >Here's the patch, updated to apply to linux-thread-db.c instead.  Still
> >no regressions; Mark indicated that the 1:1 assumption seemed reasonable
> >once the file was marked as Linux-specific.
> >
> >Michael, OK to commit?
> 
> Hey, sorry for losing this thread.  Yes, this is OK to commit.

Thanks!  Checked in.

-- 
Daniel Jacobowitz