[RFC][Patch] Fix gdb failure to access tls data for parent thread

[RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

Hello,

While using gdb to access tls data for a parent thread of a multi-threaded program, we get the following error :

"Cannot find thread-local storage for LWP 11884,  executable file /home/test/test
TLS not supported on this target"

This can be recreated as follows :

$ cat test.c
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <omp.h>

__thread int thread;

void initTlsData() {
    printf("Initialising %d\n",thread);
}
int main(int argc, char *argv[]) {
    #pragma omp parallel
    {
        thread = omp_get_thread_num();
        initTlsData();
    }
    return(0);
}

0. export "OMP_NUM_THREADS=5" 	// 5 for example
1. gdb ./test
2. break initTlsData
3. run
4. thread 1		// Switch to the parent thread
5. print thread

The above stated error occurs.

The reason for this is gdb does not fill in the private field of the thread_info structure of the
parent thread. The below patch sets up this private field before the child
threads are added to gdb's list.

I'm not sure if the patch below breaks any existing behaviour. I'm also not sure if this is the right way to go about this. 
This is purely RFC.
If there's a better way of doing this please let me know.

--- linux-thread-db.c.old	2008-12-11 14:19:12.000000000 -0500
+++ linux-thread-db.c	2008-12-30 00:26:52.000000000 -0500
@@ -879,9 +879,33 @@ check_event (ptid_t ptid)
   while (loop);
 }
 
+void set_private_data (ptid_t ptid)
+{
+  td_thrhandle_t th;
+  struct thread_info *thread_info;
+  td_thrinfo_t ti;
+  td_err_e err;
+  struct private_thread_info *private;
+
+  private = xmalloc (sizeof (struct private_thread_info));
+  memset (private, 0, sizeof (struct private_thread_info));
+
+  err = td_ta_map_lwp2thr_p (thread_agent, GET_LWP (ptid), &th);
+  thread_get_info_callback (&th, &thread_info);
+  err = td_thr_get_info_p (&th, &ti);
+
+  if (ti.ti_tid == 0)
+    err = td_thr_event_enable_p (&th, 1);
+
+  private->th = th;
+  private->tid = ti.ti_tid;
+  thread_info->private = private;
+}
+
 static ptid_t
 thread_db_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
 {
+  struct thread_info *tp;
   ptid = target_beneath->to_wait (ptid, ourstatus);
 
   if (ourstatus->kind == TARGET_WAITKIND_IGNORE)
@@ -900,6 +924,13 @@ thread_db_wait (ptid_t ptid, struct targ
       return ptid;
     }
 
+    tp = find_thread_pid (ptid);
+
+    if (!tp->private)
+      set_private_data(ptid);
+    else if (tp->private->tid == 0)
+      set_private_data(ptid);
+
   /* If we do not know about the main thread yet, this would be a good time to
      find it.  */
   if (ourstatus->kind == TARGET_WAITKIND_STOPPED && !have_threads ())



Regards,
VInay

---
Vinay Sridhar,
Linux Technology Center,
IBM ISTL,
Bangalore, India

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Fri, Jan 09, 2009 at 02:16:09PM +0530, Vinay Sridhar wrote:
> The reason for this is gdb does not fill in the private field of the
> thread_info structure of the parent thread. The below patch sets up
> this private field before the child threads are added to gdb's list.

The private info is set in attach_thread.  That is supposed to be
called for every thread; how did you get a thread on the thread list
that did not have this routine called?  A breakpoint on add_thread
and add_thread_with_info will probably figure this out.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Wed, 2009-02-04 at 08:28 -0500, Daniel Jacobowitz wrote:
> On Fri, Jan 09, 2009 at 02:16:09PM +0530, Vinay Sridhar wrote:
> > The reason for this is gdb does not fill in the private field of the
> > thread_info structure of the parent thread. The below patch sets up
> > this private field before the child threads are added to gdb's list.
> 
> The private info is set in attach_thread.  That is supposed to be
> called for every thread; how did you get a thread on the thread list
> that did not have this routine called?  A breakpoint on add_thread
> and add_thread_with_info will probably figure this out.
> 

Daniel,

Sorry for the late response. I can confirm that attach_thread is not
called on the parent thread. The test case I had listed in my earlier
mail uses OMP for thread creation. Could that have something to do with
this?

Thanks,
Vinay

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Wed, Feb 11, 2009 at 02:01:08PM +0530, Vinay Sridhar wrote:
> Daniel,
> 
> Sorry for the late response. I can confirm that attach_thread is not
> called on the parent thread. The test case I had listed in my earlier
> mail uses OMP for thread creation. Could that have something to do with
> this?

Well, it shouldn't; libgomp uses the normal POSIX thread interfaces.
I assume you're using GCC -fopenmp, right?  Or is this another OpenMP
implementation?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Wed, 2009-02-11 at 10:53 -0500, Daniel Jacobowitz wrote:
> On Wed, Feb 11, 2009 at 02:01:08PM +0530, Vinay Sridhar wrote:
> > Daniel,
> > 
> > Sorry for the late response. I can confirm that attach_thread is not
> > called on the parent thread. The test case I had listed in my earlier
> > mail uses OMP for thread creation. Could that have something to do with
> > this?
> 
> Well, it shouldn't; libgomp uses the normal POSIX thread interfaces.
> I assume you're using GCC -fopenmp, right?  Or is this another OpenMP
> implementation?
> 

I used IBM's XLC compiler which has an inbuilt OpenMP implementation.

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Thu, Feb 12, 2009 at 11:13:26AM +0530, Vinay Sridhar wrote:
> On Wed, 2009-02-11 at 10:53 -0500, Daniel Jacobowitz wrote:
> > On Wed, Feb 11, 2009 at 02:01:08PM +0530, Vinay Sridhar wrote:
> > > Daniel,
> > > 
> > > Sorry for the late response. I can confirm that attach_thread is not
> > > called on the parent thread. The test case I had listed in my earlier
> > > mail uses OMP for thread creation. Could that have something to do with
> > > this?
> > 
> > Well, it shouldn't; libgomp uses the normal POSIX thread interfaces.
> > I assume you're using GCC -fopenmp, right?  Or is this another OpenMP
> > implementation?
> > 
> 
> I used IBM's XLC compiler which has an inbuilt OpenMP implementation.

Sorry for the delay, I missed your message.

In that case, you need to determine whether it is using POSIX threads
or whether it is using clone directly and ignoring the thread library.
If it is not using the POSIX threads library, how is it handling TLS
inside the threads?  If it is using pthread_create, why didn't we get
a notification about it?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Sun, 2009-02-15 at 13:22 -0500, Daniel Jacobowitz wrote:
> On Thu, Feb 12, 2009 at 11:13:26AM +0530, Vinay Sridhar wrote:
> > On Wed, 2009-02-11 at 10:53 -0500, Daniel Jacobowitz wrote:
> > > On Wed, Feb 11, 2009 at 02:01:08PM +0530, Vinay Sridhar wrote:
> > > > Daniel,
> > > > 
> > > > Sorry for the late response. I can confirm that attach_thread is not
> > > > called on the parent thread. The test case I had listed in my earlier
> > > > mail uses OMP for thread creation. Could that have something to do with
> > > > this?
> > > 
> > > Well, it shouldn't; libgomp uses the normal POSIX thread interfaces.
> > > I assume you're using GCC -fopenmp, right?  Or is this another OpenMP
> > > implementation?
> > > 
> > 
> > I used IBM's XLC compiler which has an inbuilt OpenMP implementation.
> 
> Sorry for the delay, I missed your message.
> 
> In that case, you need to determine whether it is using POSIX threads
> or whether it is using clone directly and ignoring the thread library.
> If it is not using the POSIX threads library, how is it handling TLS
> inside the threads?  If it is using pthread_create, why didn't we get
> a notification about it?
> 

I was able to recreate this against a gcc compiled binary, so I guess
the omp implementation isnt an issue here.

Just to recap what I've been trying:

$ cat tlsdbg01.c 

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <omp.h>

__thread int thr;

void initTlsData() {
	printf("Initialising thread %d\n",thread);
}

int main(int argc, char *argv[]) {
    #pragma omp parallel
    {
        thr = omp_get_thread_num();
        initTlsData();
    }
    return(0);
}


1. gcc -g tlsdbg01.c -fopenmp -o tlsdbg
2. gdb ./tlsdbg
3. break initTlsData
4. thread 1
5. print thr

*Cannot find thread-local storage for LWP 29455, executable
*file /home/tmp/tlsdbg:
*TLS not supported on this target

Regards,
Vinay

--
Vinay Sridhar,
IBM Linux Technology Center

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Mon, Feb 16, 2009 at 03:54:28PM +0530, Vinay Sridhar wrote:
> I was able to recreate this against a gcc compiled binary, so I guess
> the omp implementation isnt an issue here.

OK, then we're back to my earlier question:

> The private info is set in attach_thread.  That is supposed to be
> called for every thread; how did you get a thread on the thread list
> that did not have this routine called?  A breakpoint on add_thread
> and add_thread_with_info will probably figure this out.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Mon, 2009-02-16 at 10:04 -0500, Daniel Jacobowitz wrote:
> On Mon, Feb 16, 2009 at 03:54:28PM +0530, Vinay Sridhar wrote:
> > I was able to recreate this against a gcc compiled binary, so I guess
> > the omp implementation isnt an issue here.
> 
> OK, then we're back to my earlier question:
> 
> > The private info is set in attach_thread.  That is supposed to be
> > called for every thread; how did you get a thread on the thread list
> > that did not have this routine called?  A breakpoint on add_thread
> > and add_thread_with_info will probably figure this out.
> 

Same response. add_thread_with_info () is not called for parent thread. 
Could this be related? :
http://sourceware.org/ml/gdb-patches/2008-03/msg00266.html

On applying the patch I had posted earlier this appears to be fixed.
However I'm not sure if it has other repercussions.

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Tue, Feb 17, 2009 at 11:21:47AM +0530, Vinay Sridhar wrote:
> On Mon, 2009-02-16 at 10:04 -0500, Daniel Jacobowitz wrote:
> > On Mon, Feb 16, 2009 at 03:54:28PM +0530, Vinay Sridhar wrote:
> > > I was able to recreate this against a gcc compiled binary, so I guess
> > > the omp implementation isnt an issue here.
> > 
> > OK, then we're back to my earlier question:
> > 
> > > The private info is set in attach_thread.  That is supposed to be
> > > called for every thread; how did you get a thread on the thread list
> > > that did not have this routine called?  A breakpoint on add_thread
> > > and add_thread_with_info will probably figure this out.
> > 
> 
> Same response. add_thread_with_info () is not called for parent thread. 

That doesn't answer the question, sorry.  At some point, add_thread
(not add_thread_with_info) must have been called.  Where did that
happen?

> Could this be related? :
> http://sourceware.org/ml/gdb-patches/2008-03/msg00266.html

I don't think so.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Tue, 2009-02-17 at 08:29 -0500, Daniel Jacobowitz wrote:
> On Tue, Feb 17, 2009 at 11:21:47AM +0530, Vinay Sridhar wrote:
> > On Mon, 2009-02-16 at 10:04 -0500, Daniel Jacobowitz wrote:
> > > On Mon, Feb 16, 2009 at 03:54:28PM +0530, Vinay Sridhar wrote:
> > > > I was able to recreate this against a gcc compiled binary, so I guess
> > > > the omp implementation isnt an issue here.
> > > 
> > > OK, then we're back to my earlier question:
> > > 
> > > > The private info is set in attach_thread.  That is supposed to be
> > > > called for every thread; how did you get a thread on the thread list
> > > > that did not have this routine called?  A breakpoint on add_thread
> > > > and add_thread_with_info will probably figure this out.
> > > 
> > 
> > Same response. add_thread_with_info () is not called for parent thread. 
> 
> That doesn't answer the question, sorry.  At some point, add_thread
> (not add_thread_with_info) must have been called.  Where did that
> happen?
> 
> > Could this be related? :
> > http://sourceware.org/ml/gdb-patches/2008-03/msg00266.html
> 
> I don't think so.
> 

I placed a break on "add_thread" as you suggested. It doesn't seem to be
hit at all, for any of the threads. 
BTW, I'm using GNU gdb (GDB) 6.8.50.20090216.

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Fri, Feb 20, 2009 at 09:29:19AM +0530, Vinay Sridhar wrote:
> I placed a break on "add_thread" as you suggested. It doesn't seem to be
> hit at all, for any of the threads. 
> BTW, I'm using GNU gdb (GDB) 6.8.50.20090216.

At the time of the original error, are all your threads in the thread
list?  GDB's internal thread_list variable, not the result of "info
threads", which will check for new threads.

If they are, and they do not have private info set, what added them to
the thread list?  I missed one function; maybe they were added by
add_thread_silent?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Sun, 2009-02-22 at 14:43 -0500, Daniel Jacobowitz wrote:
> On Fri, Feb 20, 2009 at 09:29:19AM +0530, Vinay Sridhar wrote:
> > I placed a break on "add_thread" as you suggested. It doesn't seem to be
> > hit at all, for any of the threads. 
> > BTW, I'm using GNU gdb (GDB) 6.8.50.20090216.
> 
> At the time of the original error, are all your threads in the thread
> list?  GDB's internal thread_list variable, not the result of "info
> threads", which will check for new threads.
> 

At the time of the error, all the threads are in the thread_list. All
threads but the parent thread have private field filled.

(gdb) p thread_list->private                  {pid = 12022, lwp = 12027}
$7 = (struct private_thread_info *) 0x10870530
(gdb) p thread_list->next->private            {pid = 12022, lwp = 12026}
$8 = (struct private_thread_info *) 0x108a7b80
(gdb) p thread_list->next->next->private      {pid = 12022, lwp = 12025}
$9 = (struct private_thread_info *) 0x10844e70
(gdb) p thread_list->next->next->next->private {pid = 12022,lwp = 12022}
$10 = (struct private_thread_info *) 0x0


> If they are, and they do not have private info set, what added them to
> the thread list?  I missed one function; maybe they were added by
> add_thread_silent?
> 

yes, add_thread_silent () is called on the threads. Its called for the
parent from fork_inferior () and for the child threads from
add_thread_with_info ()

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Mon, Feb 23, 2009 at 02:20:59PM +0530, Vinay Sridhar wrote:
> > If they are, and they do not have private info set, what added them to
> > the thread list?  I missed one function; maybe they were added by
> > add_thread_silent?
> > 
> 
> yes, add_thread_silent () is called on the threads. Its called for the
> parent from fork_inferior () and for the child threads from
> add_thread_with_info ()

Thanks, now we're making progress.

Pedro, copying you because this is related to always-a-thread.

What's happening here is that we go to look up a TLS variable.
We have some threads in the thread list with thread->private set, but
the main thread does not have it set - thread_db never added it,
fork_inferior did.  So we don't really know about it.

Vinay, thread_db_find_new_threads should have been called when the
program started up.  Was find_new_threads_callback called for
the main thread during that process?  If so, was ti_tid == 0?
That shouldn't happen unless the program is staticly linked.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Pedro Alves]

On Monday 23 February 2009 14:08:12, Daniel Jacobowitz wrote:
> On Mon, Feb 23, 2009 at 02:20:59PM +0530, Vinay Sridhar wrote:
> > > If they are, and they do not have private info set, what added them to
> > > the thread list?  I missed one function; maybe they were added by
> > > add_thread_silent?
> > > 
> > 
> > yes, add_thread_silent () is called on the threads. Its called for the
> > parent from fork_inferior () and for the child threads from
> > add_thread_with_info ()
> 
> Thanks, now we're making progress.
> 
> Pedro, copying you because this is related to always-a-thread.
> 
> What's happening here is that we go to look up a TLS variable.
> We have some threads in the thread list with thread->private set, but
> the main thread does not have it set - thread_db never added it,
> fork_inferior did.  So we don't really know about it.
> 
> Vinay, thread_db_find_new_threads should have been called when the
> program started up.  Was find_new_threads_callback called for
> the main thread during that process?  If so, was ti_tid == 0?
> That shouldn't happen unless the program is staticly linked.
> 

I haven't had a chance to investigate this yet, other than building
Vinay's testcase (which doesn't build BTW.  Please confirm if there
isn't anything important missing), and noticing I can't reproduce.
Sounds like there's a race somewhere that one of the calls that
look like this:

 if (!have_threads ())
   thread_db_find_new_threads_1 ();

... is finding that we already have a ->private field set for
some thread not the main, and hence, we're not filling it
for the main thread.

Vinay, could you tell us some more details about your system?

-- 
Pedro Alves

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Mon, 2009-02-23 at 20:09 +0000, Pedro Alves wrote:
> On Monday 23 February 2009 14:08:12, Daniel Jacobowitz wrote:
> > On Mon, Feb 23, 2009 at 02:20:59PM +0530, Vinay Sridhar wrote:
> > > > If they are, and they do not have private info set, what added them to
> > > > the thread list?  I missed one function; maybe they were added by
> > > > add_thread_silent?
> > > > 
> > > 
> > > yes, add_thread_silent () is called on the threads. Its called for the
> > > parent from fork_inferior () and for the child threads from
> > > add_thread_with_info ()
> > 
> > Thanks, now we're making progress.
> > 
> > Pedro, copying you because this is related to always-a-thread.
> > 
> > What's happening here is that we go to look up a TLS variable.
> > We have some threads in the thread list with thread->private set, but
> > the main thread does not have it set - thread_db never added it,
> > fork_inferior did.  So we don't really know about it.
> > 
> > Vinay, thread_db_find_new_threads should have been called when the
> > program started up.  Was find_new_threads_callback called for
> > the main thread during that process?  If so, was ti_tid == 0?
> > That shouldn't happen unless the program is staticly linked.
> > 
> 
> I haven't had a chance to investigate this yet, other than building
> Vinay's testcase (which doesn't build BTW.  Please confirm if there
> isn't anything important missing), and noticing I can't reproduce.

Pedro,

Please try this test and let me know if you're able to get a recreate:

$ cat test.c 

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <omp.h>

__thread int thr;

void initTlsData() {
        printf("Initialising thread %d\n",thr);
}

int main(int argc, char *argv[]) {
    #pragma omp parallel
    {
        thr = omp_get_thread_num();
        initTlsData();
    }
    return(0);
}

0. export OMP_NUM_THREADS = <X>
1. gcc -g test.c -fopenmp -o test
2. gdb ./test
3. break initTlsData
4. thread 1   (Switch to main thread)
5. print thr  (You should see the error after this)



> Sounds like there's a race somewhere that one of the calls that
> look like this:
> 
>  if (!have_threads ())
>    thread_db_find_new_threads_1 ();
> 
> ... is finding that we already have a ->private field set for
> some thread not the main, and hence, we're not filling it
> for the main thread.

A race? I'm able to get this error each time I run the test...

> 
> Vinay, could you tell us some more details about your system?

I'm running SLES11 on a ppc64 machine using gdb-6.8.50.20090216.
Standard OMP, gcc etc..

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

> Pedro,
> 
> Please try this test and let me know if you're able to get a recreate:
> 
> $ cat test.c 
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <omp.h>
> 
> __thread int thr;
> 
> void initTlsData() {
>         printf("Initialising thread %d\n",thr);
> }
> 
> int main(int argc, char *argv[]) {
>     #pragma omp parallel
>     {
>         thr = omp_get_thread_num();
>         initTlsData();
>     }
>     return(0);
> }
> 
> 0. export OMP_NUM_THREADS = <X>
> 1. gcc -g test.c -fopenmp -o test
> 2. gdb ./test
> 3. break initTlsData
==========================

  4. run	(extremely sorry for missing this in the previous mail)

==========================

> 4. thread 1   (Switch to main thread)
> 5. print thr  (You should see the error after this)
> 

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Daniel Jacobowitz]

On Tue, Feb 24, 2009 at 04:20:10PM +0530, Vinay Sridhar wrote:
> Please try this test and let me know if you're able to get a recreate:

I can reproduce this.  Here's where we ought to discover the thread:

1012      /* Iterate over all user-space threads to discover new threads.  */
1013      err = td_ta_thr_iter_p (thread_agent, find_new_threads_callback, NULL,
1014                              TD_THR_ANY_STATE, TD_THR_LOWEST_PRIORITY,
1015                              TD_SIGNO_MASK, TD_THR_ANY_USER_FLAGS);

The first two times this is called, ti_tid is 0 for the main thread,
so we do nothing.  The third time, find_new_threads_callback
is never called!  The thread library is claiming we have no threads.
Repeating "info threads" again has the same effect:

  5 Thread 0x7ffff5c5f950 (LWP 7440)  0x00007ffff7bde9c7 in ?? () from
  /usr/lib/libgomp.so.1
  4 Thread 0x7ffff6460950 (LWP 7439)  initTlsData () at omp-test.c:9
  3 Thread 0x7ffff6c61950 (LWP 7438)  0x00007ffff7bdea1a in ?? () from
  /usr/lib/libgomp.so.1
  2 Thread 0x7ffff7462950 (LWP 7418)  0x00007ffff7bdea1a in ?? () from
  /usr/lib/libgomp.so.1
* 1 LWP 7410  0x00007ffff7bdea1a in ?? () from /usr/lib/libgomp.so.1

Pedro, unfortunately this bug is related to non-stop :-(

81            /* Verify that this thread's pid field matches the child PID.
82               If its pid field is negative, it's about to do a fork or it
83               is the sole thread in a fork child.  */

It's checking that the PID (not TID) matches proc_handle.pid.  We need
to find another way to read from a stopped thread, since if we put any
other PID there, we get no threads.  I would suggest expanding
ps_prochandle to include a memory thread as ptid_t.  We always try to
read from the most recently added stopped thread, since the lwp_list
gets additions at the front.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Pedro Alves]

On Tuesday 24 February 2009 15:31:53, Daniel Jacobowitz wrote:
> 81            /* Verify that this thread's pid field matches the child PID.
> 82               If its pid field is negative, it's about to do a fork or it
> 83               is the sole thread in a fork child.  */

Boo, I'm on glibc 2.7, which doesn't have this check, but I see it
in glibc-2.9 sources...

> It's checking that the PID (not TID) matches proc_handle.pid.  We need
> to find another way to read from a stopped thread, since if we put any
> other PID there, we get no threads.  I would suggest expanding
> ps_prochandle to include a memory thread as ptid_t.  

Right, I'll take this.  Funny, I was actually noticing the other day
that the proc service implementation in sol-threads.c passes a ptid
already, and I was considering making sol-threads.c use proc-service.c:

 /* This struct is defined by us, but mainly used for the proc_service
    interface.  We don't have much use for it, except as a handy place
    to get a real PID for memory accesses.  */

 struct ps_prochandle
 {
   ptid_t ptid;
 };

> We always try to 
> read from the most recently added stopped thread, since the lwp_list
> gets additions at the front.
> 


-- 
Pedro Alves

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Pedro Alves]

On Tuesday 24 February 2009 15:58:31, Pedro Alves wrote:
> On Tuesday 24 February 2009 15:31:53, Daniel Jacobowitz wrote:
> > 81            /* Verify that this thread's pid field matches the child PID.
> > 82               If its pid field is negative, it's about to do a fork or it
> > 83               is the sole thread in a fork child.  */
> 
> Boo, I'm on glibc 2.7, which doesn't have this check, but I see it
> in glibc-2.9 sources...
> 
> > It's checking that the PID (not TID) matches proc_handle.pid.  We need
> > to find another way to read from a stopped thread, since if we put any
> > other PID there, we get no threads.  I would suggest expanding
> > ps_prochandle to include a memory thread as ptid_t.  
> 
> Right, I'll take this.

Here it is, how does it look?  This works on glibc-2.7, but then again, that
version isn't affected by this.  Could one of you please give it a shot and
see if it fixes the reported issue?

-- 
Pedro Alves

2009-02-24  Pedro Alves  <pedro@codesourcery.com>

	* gdb_proc_service.h (struct ps_prochandle): Replace pid_t field
	with a ptid_t field.
	* linux-thread-db.c (thread_get_info_callback): Build the ptid
	using the pid stored in proc_handle.ptid.
	
	(thread_from_lwp, thread_db_attach_lwp, enable_thread_event)
	(check_for_thread_db, thread_db_detach, check_event)
	(thread_db_mourn_inferior, find_new_threads_callback)
	(thread_db_find_new_threads_1): Adjust.

	* proc-service.c (ps_xfer_memory, ps_lgetregs, ps_lsetregs)
	(ps_lgetfpregs, ps_lsetfpregs, ps_getpid): Adjust.

---
 gdb/gdb_proc_service.h |    4 ++--
 gdb/linux-thread-db.c  |   20 ++++++++++----------
 gdb/proc-service.c     |   12 ++++++------
 3 files changed, 18 insertions(+), 18 deletions(-)

Index: src/gdb/gdb_proc_service.h
===================================================================
--- src.orig/gdb/gdb_proc_service.h	2009-02-24 16:15:32.000000000 +0000
+++ src/gdb/gdb_proc_service.h	2009-02-24 16:51:55.000000000 +0000
@@ -75,8 +75,8 @@ typedef prfpregset_t gdb_prfpregset_t;
 /* Structure that identifies the target process.  */
 struct ps_prochandle
 {
-  /* The process id is all we need.  */
-  pid_t pid;
+  /* The LWP we use for memory reads.  */
+  ptid_t ptid;
 };
 
 #endif /* gdb_proc_service.h */
Index: src/gdb/linux-thread-db.c
===================================================================
--- src.orig/gdb/linux-thread-db.c	2009-02-24 16:18:53.000000000 +0000
+++ src/gdb/linux-thread-db.c	2009-02-24 16:33:51.000000000 +0000
@@ -265,7 +265,7 @@ thread_get_info_callback (const td_thrha
 	   thread_db_err_str (err));
 
   /* Fill the cache.  */
-  thread_ptid = ptid_build (GET_PID (inferior_ptid), ti.ti_lid, 0);
+  thread_ptid = ptid_build (GET_PID (proc_handle.ptid), ti.ti_lid, 0);
   thread_info = find_thread_pid (thread_ptid);
 
   /* In the case of a zombie thread, don't continue.  We don't want to
@@ -309,7 +309,7 @@ thread_from_lwp (ptid_t ptid)
   gdb_assert (GET_LWP (ptid) != 0);
 
   /* Access an lwp we know is stopped.  */
-  proc_handle.pid = GET_LWP (ptid);
+  proc_handle.ptid = ptid;
   err = td_ta_map_lwp2thr_p (thread_agent, GET_LWP (ptid), &th);
   if (err != TD_OK)
     error (_("Cannot find user-level thread for LWP %ld: %s"),
@@ -352,7 +352,7 @@ thread_db_attach_lwp (ptid_t ptid)
   gdb_assert (GET_LWP (ptid) != 0);
 
   /* Access an lwp we know is stopped.  */
-  proc_handle.pid = GET_LWP (ptid);
+  proc_handle.ptid = ptid;
 
   /* If we have only looked at the first thread before libpthread was
      initialized, we may not know its thread ID yet.  Make sure we do
@@ -457,7 +457,7 @@ enable_thread_event (td_thragent_t *thre
   td_err_e err;
 
   /* Access an lwp we know is stopped.  */
-  proc_handle.pid = GET_LWP (inferior_ptid);
+  proc_handle.ptid = inferior_ptid;
 
   /* Get the breakpoint address for thread EVENT.  */
   err = td_ta_event_addr_p (thread_agent, event, &notify);
@@ -637,7 +637,7 @@ check_for_thread_db (void)
     return;
 
   /* Initialize the structure that identifies the child process.  */
-  proc_handle.pid = GET_PID (inferior_ptid);
+  proc_handle.ptid = inferior_ptid;
 
   /* Now attempt to open a connection to the thread library.  */
   err = td_ta_new_p (&proc_handle, &thread_agent);
@@ -779,7 +779,7 @@ thread_db_detach (struct target_ops *ops
 
   /* Forget about the child's process ID.  We shouldn't need it
      anymore.  */
-  proc_handle.pid = 0;
+  proc_handle.ptid = null_ptid;
 
   /* Detach thread_db target ops.  */
   unpush_target (&thread_db_ops);
@@ -810,7 +810,7 @@ check_event (ptid_t ptid)
     return;
 
   /* Access an lwp we know is stopped.  */
-  proc_handle.pid = GET_LWP (ptid);
+  proc_handle.ptid = ptid;
 
   /* If we have only looked at the first thread before libpthread was
      initialized, we may not know its thread ID yet.  Make sure we do
@@ -934,7 +934,7 @@ thread_db_mourn_inferior (struct target_
 
   /* Forget about the child's process ID.  We shouldn't need it
      anymore.  */
-  proc_handle.pid = 0;
+  proc_handle.ptid = null_ptid;
 
   target_beneath->to_mourn_inferior (target_beneath);
 
@@ -963,7 +963,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 = ptid_build (GET_PID (inferior_ptid), ti.ti_lid, 0);
+  ptid = ptid_build (GET_PID (proc_handle.ptid), ti.ti_lid, 0);
 
   if (ti.ti_tid == 0)
     {
@@ -1008,7 +1008,7 @@ thread_db_find_new_threads_1 (void)
     return;
 
   /* Access an lwp we know is stopped.  */
-  proc_handle.pid = GET_LWP (ptid);
+  proc_handle.ptid = ptid;
   /* Iterate over all user-space threads to discover new threads.  */
   err = td_ta_thr_iter_p (thread_agent, find_new_threads_callback, NULL,
 			  TD_THR_ANY_STATE, TD_THR_LOWEST_PRIORITY,
Index: src/gdb/proc-service.c
===================================================================
--- src.orig/gdb/proc-service.c	2009-02-24 16:15:38.000000000 +0000
+++ src/gdb/proc-service.c	2009-02-24 16:33:04.000000000 +0000
@@ -97,7 +97,7 @@ ps_xfer_memory (const struct ps_prochand
   int ret;
   CORE_ADDR core_addr = ps_addr_to_core_addr (addr);
 
-  inferior_ptid = pid_to_ptid (ph->pid);
+  inferior_ptid = ph->ptid;
 
   if (write)
     ret = target_write_memory (core_addr, buf, len);
@@ -257,7 +257,7 @@ ps_lgetregs (gdb_ps_prochandle_t ph, lwp
   struct cleanup *old_chain = save_inferior_ptid ();
   struct regcache *regcache;
 
-  inferior_ptid = BUILD_LWP (lwpid, ph->pid);
+  inferior_ptid = BUILD_LWP (lwpid, ptid_get_pid (ph->ptid));
   regcache = get_thread_regcache (inferior_ptid);
 
   target_fetch_registers (regcache, -1);
@@ -276,7 +276,7 @@ ps_lsetregs (gdb_ps_prochandle_t ph, lwp
   struct cleanup *old_chain = save_inferior_ptid ();
   struct regcache *regcache;
 
-  inferior_ptid = BUILD_LWP (lwpid, ph->pid);
+  inferior_ptid = BUILD_LWP (lwpid, ptid_get_pid (ph->ptid));
   regcache = get_thread_regcache (inferior_ptid);
 
   supply_gregset (regcache, (const gdb_gregset_t *) gregset);
@@ -296,7 +296,7 @@ ps_lgetfpregs (gdb_ps_prochandle_t ph, l
   struct cleanup *old_chain = save_inferior_ptid ();
   struct regcache *regcache;
 
-  inferior_ptid = BUILD_LWP (lwpid, ph->pid);
+  inferior_ptid = BUILD_LWP (lwpid, ptid_get_pid (ph->ptid));
   regcache = get_thread_regcache (inferior_ptid);
 
   target_fetch_registers (regcache, -1);
@@ -316,7 +316,7 @@ ps_lsetfpregs (gdb_ps_prochandle_t ph, l
   struct cleanup *old_chain = save_inferior_ptid ();
   struct regcache *regcache;
 
-  inferior_ptid = BUILD_LWP (lwpid, ph->pid);
+  inferior_ptid = BUILD_LWP (lwpid, ptid_get_pid (ph->ptid));
   regcache = get_thread_regcache (inferior_ptid);
 
   supply_fpregset (regcache, (const gdb_fpregset_t *) fpregset);
@@ -332,7 +332,7 @@ ps_lsetfpregs (gdb_ps_prochandle_t ph, l
 pid_t
 ps_getpid (gdb_ps_prochandle_t ph)
 {
-  return ph->pid;
+  return ptid_get_pid (ph->ptid);
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Tue, 2009-02-24 at 17:16 +0000, Pedro Alves wrote:
> On Tuesday 24 February 2009 15:58:31, Pedro Alves wrote:
> > On Tuesday 24 February 2009 15:31:53, Daniel Jacobowitz wrote:
> > > 81            /* Verify that this thread's pid field matches the child PID.
> > > 82               If its pid field is negative, it's about to do a fork or it
> > > 83               is the sole thread in a fork child.  */
> > 
> > Boo, I'm on glibc 2.7, which doesn't have this check, but I see it
> > in glibc-2.9 sources...
> > 
> > > It's checking that the PID (not TID) matches proc_handle.pid.  We need
> > > to find another way to read from a stopped thread, since if we put any
> > > other PID there, we get no threads.  I would suggest expanding
> > > ps_prochandle to include a memory thread as ptid_t.  
> > 
> > Right, I'll take this.
> 
> Here it is, how does it look?  This works on glibc-2.7, but then again, that
> version isn't affected by this.  Could one of you please give it a shot and
> see if it fixes the reported issue?
> 

Tested this with the Feb snapshot. The patch fixes the issue. I'm using
glibc-2.9..

Thanks,
Vinay

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Pedro Alves]

On Wednesday 25 February 2009 07:16:14, Vinay Sridhar wrote:
> > 
> > Here it is, how does it look?  This works on glibc-2.7, but then again, that
> > version isn't affected by this.  Could one of you please give it a shot and
> > see if it fixes the reported issue?
> > 
> 
> Tested this with the Feb snapshot. The patch fixes the issue. I'm using
> glibc-2.9..

Ok, great!  I went ahead and checked the patch in.

Thanks,

-- 
Pedro Alves

Re: [RFC][Patch] Fix gdb failure to access tls data for parent thread

[Vinay Sridhar]

On Mon, 2009-02-23 at 09:08 -0500, Daniel Jacobowitz wrote:
> On Mon, Feb 23, 2009 at 02:20:59PM +0530, Vinay Sridhar wrote:
> > > If they are, and they do not have private info set, what added them to
> > > the thread list?  I missed one function; maybe they were added by
> > > add_thread_silent?
> > > 
> > 
> > yes, add_thread_silent () is called on the threads. Its called for the
> > parent from fork_inferior () and for the child threads from
> > add_thread_with_info ()
> 
> Thanks, now we're making progress.
> 
> Pedro, copying you because this is related to always-a-thread.
> 
> What's happening here is that we go to look up a TLS variable.
> We have some threads in the thread list with thread->private set, but
> the main thread does not have it set - thread_db never added it,
> fork_inferior did.  So we don't really know about it.
> 
> Vinay, thread_db_find_new_threads should have been called when the
> program started up. 

Yes, I did see thread_db_find_new_threads being called.

>  Was find_new_threads_callback called for
> the main thread during that process?  If so, was ti_tid == 0?
> That shouldn't happen unless the program is staticly linked.
> 

find_new_thread_callback was called on the parent thread, before the
other threads were initialised. (I was able to trace 2 of these calls on
the parent thread though, not sure why). And yes, "ti_tid" was set to 0
for the parent thread.