Re: [RFA]corelow.c: Add tid to add_to_thread_list

[Hui Zhu <teawater@gmail.com>]

On Fri, Aug 6, 2010 at 17:57, Pedro Alves <pedro@codesourcery.com> wrote:
> On Friday 06 August 2010 03:55:51, Hui Zhu wrote:
>
>> 2010-08-06  Hui Zhu  <teawater@gmail.com>
>>
>>       * corelow.c(add_to_thread_list): Add tid.
>>
>> ---
>>  corelow.c |   10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> --- a/corelow.c
>> +++ b/corelow.c
>> @@ -244,7 +244,7 @@ add_to_thread_list (bfd *abfd, asection
>>  {
>>    ptid_t ptid;
>>    int core_tid;
>> -  int pid, lwpid;
>> +  int pid, lwpid, tid;
>>    asection *reg_sect = (asection *) reg_sect_arg;
>>
>>    if (strncmp (bfd_section_name (abfd, asect), ".reg/", 5) != 0)
>> @@ -278,7 +278,13 @@ add_to_thread_list (bfd *abfd, asection
>>    if (current_inferior ()->pid == 0)
>>      inferior_appeared (current_inferior (), pid);
>>
>> -  ptid = ptid_build (pid, lwpid, 0);
>> +  tid = 0;
>> +  do
>> +    {
>> +      ptid = ptid_build (pid, lwpid, tid);
>> +      tid ++;
>> +    }
>> +  while (find_thread_ptid (ptid));
>>
>>    add_thread (ptid);
>
> Sorry, no.  It's not a good idea to have the corelow target
> using both the lwpid and the tid fields.  It leaves no room for a
> thread_stratum target on top to use.  Going by what someone
> said on the other thread:
>
>> The goal was always that something could post process the output of
>> the kernel crashdump and create something that is gdb compatible.  It
>> looks to me like it would take just a moment to strip out all of the
>> idle threads.
>
> What are exactly these "threads" with no PID?  How useful is it for gdb
> to expose them as threads?  Are these the idle threads, one per core, not
> associated with any process?  Are we talking about a regular process core
> dump, or some other core dump?  From the comment quoted above, it appears
> that it would be expected that something just stripped out / ignored
> these "threads"/notes.  Say, a post processor tool, or bfd, or gdb.

The root cause about this issue is the idle thread's pid is 0.  If
more than one cpu is in idle and each cpu will be a thread in core
file, we got a core file that have some thread ptid is same.
For now, gdb cannot handle it:
struct thread_info *
add_thread_silent (ptid_t ptid)
{
  struct thread_info *tp;

  tp = find_thread_ptid (ptid);
  if (tp)
    /* Found an old thread with the same id.  It has to be dead,
       otherwise we wouldn't be adding a new thread with the same id.
       The OS is reusing this id --- delete it, and recreate a new
       one.  */

I make a patch that can ignore the same ptid thread and output a warning.

>
> BTW, with this particular patch, you've only gotten past this one
> problem, but, you'll be tripping on others.  For example, all register
> accesses for lwpid=XXX,tid=0, lwpid=XXX,tid=1, lwpid=XXX,tid=2, etc.,
> will be going to the same .reg/XXX section.

Yes, that is a trouble.

2010-08-07  Hui Zhu  <teawater@gmail.com>

	* corelow.c(add_to_thread_list): Ignore the thread if already
	a ptid is in thread list.

---
 corelow.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- a/corelow.c
+++ b/corelow.c
@@ -279,6 +279,12 @@ add_to_thread_list (bfd *abfd, asection
     inferior_appeared (current_inferior (), pid);

   ptid = ptid_build (pid, lwpid, 0);
+  if (find_thread_ptid (ptid))
+    {
+      warning (_("Get more than one thread ptid is %s.  Ignore it."),
+	       target_pid_to_str (ptid));
+      return;
+    }

   add_thread (ptid);