A Wednesday 28 May 2008 00:23:25, Paul Pluzhnikov wrote:
> On Tue, May 27, 2008 at 1:06 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> > Could you confirm that this hunk of my patch,
> >
> > Index: src/gdb/linux-thread-db.c
> > ===================================================================
> > --- src.orig/gdb/linux-thread-db.c      2008-05-06 12:22:31.000000000
> > +0100 +++ src/gdb/linux-thread-db.c   2008-05-06 12:53:18.000000000 +0100
> > @@ -840,7 +840,7 @@ thread_db_wait (ptid_t ptid, struct targ
> >       unpush_target (&thread_db_ops);
> >       using_thread_db = 0;
> >
> > -      return pid_to_ptid (GET_PID (ptid));
> > +      return ptid;
> >     }
> >
> >   /* If we do not know about the main thread yet, this would be a good
> > time to
> >
> > ... fixes the issue,
>
> Confirmed.
>
> > and that you were hitting that new_thread_event piece
> > in infrun.c:handle_inferior_event while handling a TARGET_WAITKIND_EXECD
> > ?
>
> I am not sure I understand that second question correctly.
>
> The 'ecs->new_thread_event' is set to 0 while handling
> TARGET_WAITKIND_EXECD, and add_thread() is not called for it.
>

Hmmmm, then, how is a thread getting to the thread list, with
a ptid{pid,0,0} format?  It should never happen in linux native.

Hmmmm2, I just tried it, and it does happen to me:

(top-gdb) bt
#0  add_thread (ptid={pid = 1140, lwp = 0, tid = 0}) 
at ../../src/gdb/thread.c:153
#1  0x0000000000501b8a in handle_inferior_event (ecs=0x7fff72d6f6f0) 
at ../../src/gdb/infrun.c:1767
#2  0x000000000050114f in wait_for_inferior (treat_exec_as_sigtrap=0) 
at ../../src/gdb/infrun.c:1475
#3  0x0000000000500eb1 in proceed (addr=18446744073709551615, 
siggnal=TARGET_SIGNAL_DEFAULT, step=1)
    at ../../src/gdb/infrun.c:1273
#4  0x00000000004fd0bb in step_1 (skip_subroutines=1, single_inst=0, 
count_string=0x0) at ../../src/gdb/infcmd.c:784
#5  0x00000000004fcd9a in next_command (count_string=0x0, from_tty=1) 
at ../../src/gdb/infcmd.c:678
(...)

void
handle_inferior_event (struct execution_control_state *ecs)
{
...
  /* If it's a new process, add it to the thread database */

  ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid)
			   && !ptid_equal (ecs->ptid, minus_one_ptid)
			   && !in_thread_list (ecs->ptid));

  if (ecs->ws.kind != TARGET_WAITKIND_EXITED
      && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event)
    add_thread (ecs->ptid);



(top-gdb) p ecs->new_thread_event
$1 = 1

(top-gdb) p ecs->ws.kind
$2 = TARGET_WAITKIND_EXECD

(top-gdb) p inferior_ptid
$3 = {pid = 1140, lwp = 1140, tid = 0}

(top-gdb) p ecs->ptid
$4 = {pid = 1140, lwp = 0, tid = 0}

That's the trimmed ptid that escaped thread_db_wait.

> > We may need some more interface cleanup to clear the current thread
> > list across an exec, if the original process had threads, but I don't
> > think your call is in the right place.
>
> Yes, it did feel out of place to me as well.
>

Right, what is happening if the original process loaded thread-db,
had more than 1 thread, and the new exec image doesn't load
thread-db?  Your call isn't reached then.

Hmmm, I having just tried it, the OS gives us thread exit events
for the other threads of the original process, before giving us
a PTRACE_EVENT_EXEC, so there's nothing else to do, we're safe there.

This happens before PTRACE_EVENT_EXEC:

#0  delete_thread (ptid={pid = 13458, lwp = 13463, tid = 0}) 
at ../../src/gdb/thread.c:161
#1  0x000000000046da5f in exit_lwp (lp=0xc18460) 
at ../../src/gdb/linux-nat.c:1029
#2  0x0000000000470f3f in linux_nat_filter_event (lwpid=13463, status=0, 
options=-2147483647)
    at ../../src/gdb/linux-nat.c:2382
#3  0x0000000000471aa2 in linux_nat_wait (ptid={pid = -1, lwp = 0, tid = 0}, 
ourstatus=0x7fff07d0f680)
    at ../../src/gdb/linux-nat.c:2646
#4  0x0000000000476f33 in thread_db_wait (ptid={pid = -1, lwp = 0, tid = 0}, 
ourstatus=0x7fff07d0f680)
    at ../../src/gdb/linux-thread-db.c:826
#5  0x00000000005010e1 in wait_for_inferior (treat_exec_as_sigtrap=0) 
at ../../src/gdb/infrun.c:1465
#6  0x0000000000500ea9 in proceed (addr=18446744073709551615, 
siggnal=TARGET_SIGNAL_DEFAULT, step=1)
    at ../../src/gdb/infrun.c:1273
...

(gdb) PASS: gdb.threads/execl.exp: info threads before exec
continue
Continuing.
[Thread 0x40f13950 (LWP 13377) exited]
[Thread 0x40d12950 (LWP 13376) exited]


Hope you don't mind, but I've extended your test to test that case; the 
original bug is still covered, as I still get FAILures if
linux-thread-db.c isn't fixed.

What do you think?

-- 
Pedro Alves