Re: [RFC] Fix for gdb crash in "info thread" after exec().

Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Paul Pluzhnikov]

Ping.

http://sourceware.org/ml/gdb-patches/2008-05/msg00386.html

On Mon, May 12, 2008 at 6:38 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Greetings,
>
> Gdb (CVS Head) crashes when 'info thread' is executed after program exec()s:

-- 
Paul Pluzhnikov

Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Paul Pluzhnikov]

Ping.

http://sourceware.org/ml/gdb-patches/2008-05/msg00386.html

Also re-sending the patch as inline plain-text.

--
Paul Pluzhnikov

ChangeLog:
2008-05-12  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb/linux-thread-dl.c (check_for_thread_db): reinitialize
	thread list after exec().

testsuite/ChangeLog:
2008-05-12  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.threads/execl.c, gdb.threads/execl.exp: Add test for
	"info threads" after exec.


Index: gdb/linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.42
diff -u -p -u -r1.42 linux-thread-db.c
--- gdb/linux-thread-db.c	4 May 2008 14:12:34 -0000	1.42
+++ gdb/linux-thread-db.c	13 May 2008 01:28:48 -0000
@@ -613,6 +613,10 @@ check_for_thread_db (void)
       push_target (&thread_db_ops);
       using_thread_db = 1;
 
+      /* Existing entries in thread_list might not have thread_info.private
+	 set, so clear it out.  */
+      init_thread_list ();
+
       enable_thread_event_reporting ();
       thread_db_find_new_threads ();
       break;
Index: gdb/testsuite/gdb.threads/execl.c
===================================================================
RCS file: gdb/testsuite/gdb.threads/execl.c
diff -N gdb/testsuite/gdb.threads/execl.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.threads/execl.c	13 May 2008 01:28:48 -0000
@@ -0,0 +1,12 @@
+/* Test handling thread control across an execl.  */
+
+#include <unistd.h>
+
+int
+main (int argc, char* argv[])
+{
+  if (argc == 1)
+    execl (argv[0], argv[0], "second", NULL);
+
+  return 0;
+}
Index: gdb/testsuite/gdb.threads/execl.exp
===================================================================
RCS file: gdb/testsuite/gdb.threads/execl.exp
diff -N gdb/testsuite/gdb.threads/execl.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.threads/execl.exp	13 May 2008 01:28:48 -0000
@@ -0,0 +1,48 @@
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# Test handling of threads across an execl.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile "execl"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+runto_main
+
+# When continuing from this point we'll hit the breakpoint in main()
+# again, this time in the exec'd process.
+gdb_test "continue" ".*Breakpoint 1, main.*" \
+    "continue across exec"
+
+gdb_test "info threads" ".*" "info threads after exec"
+
+gdb_test "continue" ".*Program exited normally\\." \
+    "continue to end"

Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Pedro Alves]

A Tuesday 27 May 2008 20:07:02, Paul Pluzhnikov wrote:
> Ping.
>
> http://sourceware.org/ml/gdb-patches/2008-05/msg00386.html
>
> Also re-sending the patch as inline plain-text.
>

I agree that the threads should not survive across an exec, but
the crash you're reporting suggests that something is
trimming the ptid when it shouldn't, and that ptid is getting
into the thread list.

Here's a patch that handles this a bit more generically:
 http://sourceware.org/ml/gdb-patches/2008-05/msg00230.html

In your test case, what is probably happening, is that 
linux-nat.c/linux-thread-db.c is escaping an event ptid which
isn't in the thread list, which doesn't have a ptid.lwp member
set, so you're hitting this in handle_inferior_event:

  /* 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);

Which I've been claiming is bad...

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, and that you were hitting that new_thread_event piece
in infrun.c:handle_inferior_event while handling a TARGET_WAITKIND_EXECD ?

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.

-- 
Pedro Alves

Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Paul Pluzhnikov]

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.

> 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.

Thanks,
-- 
Paul Pluzhnikov

[new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 4553 bytes --]

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

[-- Attachment #2: execl.diff --]
[-- Type: text/x-diff, Size: 5197 bytes --]

gdb/
2008-05-28  Pedro Alves  <pedro@codesourcery.com>

	* linux-thread-db.c (thread_db_wait): Don't trim event ptid.

testsuite/
2008-05-28  Paul Pluzhnikov  <ppluzhnikov@google.com>
            Pedro Alves  <pedro@codesourcery.com>

	* gdb.threads/execl.c, gdb.threads/execl1.c,
	gdb.threads/execl.exp: New tests.

---
 gdb/linux-thread-db.c               |    2 
 gdb/testsuite/gdb.threads/execl.c   |   38 ++++++++++++++++++
 gdb/testsuite/gdb.threads/execl.exp |   75 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/execl1.c  |    9 ++++
 4 files changed, 123 insertions(+), 1 deletion(-)

Index: gdb/linux-thread-db.c
===================================================================
--- gdb/linux-thread-db.c.orig	2008-05-15 21:15:01.000000000 +0100
+++ gdb/linux-thread-db.c	2008-05-28 16:09:52.000000000 +0100
@@ -838,7 +838,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
Index: gdb/testsuite/gdb.threads/execl.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl.c	2008-05-28 15:48:17.000000000 +0100
@@ -0,0 +1,38 @@
+/* Test handling thread control across an execl.  */
+
+/* The original image loads thread-db and has several threads, while
+   the new image does not load thread-db.  */
+
+#include <unistd.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+void *
+thread_function (void *arg)
+{
+  while (1)
+    sleep (100);
+  return NULL;
+}
+
+int
+main (int argc, char* argv[])
+{
+  pthread_t thread1;
+  pthread_t thread2;
+  char *new_image;
+
+  pthread_create (&thread1, NULL, thread_function, NULL);
+  pthread_create (&thread2, NULL, thread_function, NULL);
+
+  new_image = malloc (strlen (argv[0] + 1));
+  strcpy (new_image, argv[0]);
+  strcat (new_image, "1");
+
+  if (execl (new_image, new_image, "second", NULL) == -1)  /* set breakpoint here */
+    return 1;
+
+  return 0;
+}
Index: gdb/testsuite/gdb.threads/execl.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl.exp	2008-05-28 16:25:07.000000000 +0100
@@ -0,0 +1,75 @@
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test handling of threads across an execl.
+
+
+# Original image, loads threaddb.
+set testfile "execl"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+
+# New image, that does not load threaddb.
+set testfile1 "execl1"
+set srcfile1 ${testfile1}.c
+set binfile1 ${objdir}/${subdir}/${testfile1}
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" executable {debug}] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+runto_main
+
+gdb_test "b [gdb_get_line_number "breakpoint here"]" \
+         ".*Breakpoint .*execl.*" "set breakpoint at execl"
+
+gdb_test "continue" ".*breakpoint here.*" "continue to exec"
+
+gdb_test "info threads" ".*3 Thread.*2 Thread.*1 Thread.*" "info threads before exec"
+
+# When continuing from this point we'll hit the breakpoint in main()
+# again, this time in the exec'd process.
+gdb_test "continue" ".*Breakpoint 1, main.*" \
+    "continue across exec"
+
+gdb_test "info threads" ".*" "info threads after exec"
+
+set test "info threads after exec"
+gdb_test_multiple "info threads" "$test" {
+    -re "2 Thread .*$gdb_prompt $" {
+	# Old threads left behind.
+	fail "$test"
+    }
+    -re "4 Thread .*$gdb_prompt $" {
+	# New threads registered.
+	fail "$test"
+    }
+    -re "$gdb_prompt $" {
+	# Target doesn't register the main thread, pass for now.
+	pass "$test"
+    }
+}
+
+gdb_test "continue" ".*Program exited normally\\." \
+    "continue to end"
Index: gdb/testsuite/gdb.threads/execl1.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl1.c	2008-05-28 16:14:02.000000000 +0100
@@ -0,0 +1,9 @@
+/* Test handling thread control across an execl.  */
+
+/* New exec image that doesn't load thread-db.  */
+
+int
+main (int argc, char* argv[])
+{
+  return 0;
+}

Re: [new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Paul Pluzhnikov]

On Wed, May 28, 2008 at 8:26 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> A Wednesday 28 May 2008 00:23:25, Paul Pluzhnikov wrote:

>> 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:

Sorry about it being dense. Yes, the exact same thing happens to
me once I revert your patch:

-      return pid_to_ptid (GET_PID (ptid));
+      return ptid;

> 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?

Looks good to me, except the "second" parameter in the test is no
longer necessary, and there is an off-by-three bug in the allocation
of new_image:

--- gdb/testsuite/gdb.threads/execl.c.orig      2008-05-28
09:55:29.000000000 -0700
+++ gdb/testsuite/gdb.threads/execl.c   2008-05-28 10:03:04.000000000 -0700
@@ -27,11 +27,11 @@ main (int argc, char* argv[])
   pthread_create (&thread1, NULL, thread_function, NULL);
   pthread_create (&thread2, NULL, thread_function, NULL);

-  new_image = malloc (strlen (argv[0] + 1));
+  new_image = malloc (strlen (argv[0]) + 2);
   strcpy (new_image, argv[0]);
   strcat (new_image, "1");

-  if (execl (new_image, new_image, "second", NULL) == -1)  /* set
breakpoint here */
+  if (execl (new_image, new_image, NULL) == -1)  /* set breakpoint here */
     return 1;

   return 0;

Thanks,
-- 
Paul Pluzhnikov

Re: [new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

A Wednesday 28 May 2008 18:09:40, Paul Pluzhnikov wrote:
> On Wed, May 28, 2008 at 8:26 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> > What do you think?
>
> Looks good to me, except the "second" parameter in the test is no
> longer necessary, and there is an off-by-three bug in the allocation
> of new_image:
>

Outch!  How careless am I ?   :-)

New patch attached.  Can anyone approve (or reject) this?

-- 
Pedro Alves

[-- Attachment #2: execl.diff --]
[-- Type: text/x-diff, Size: 5225 bytes --]

gdb/
2008-05-28  Pedro Alves  <pedro@codesourcery.com>

	* linux-thread-db.c (thread_db_wait): Don't trim event ptid.

testsuite/
2008-05-28  Paul Pluzhnikov  <ppluzhnikov@google.com>
            Pedro Alves  <pedro@codesourcery.com>

	* gdb.threads/execl.c, gdb.threads/execl1.c,
	gdb.threads/execl.exp: New tests.

---
 gdb/linux-thread-db.c               |    2 
 gdb/testsuite/gdb.threads/execl.c   |   38 ++++++++++++++++++
 gdb/testsuite/gdb.threads/execl.exp |   75 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/execl1.c  |    9 ++++
 4 files changed, 123 insertions(+), 1 deletion(-)

Index: gdb/linux-thread-db.c
===================================================================
--- gdb/linux-thread-db.c.orig	2008-05-28 18:49:15.000000000 +0100
+++ gdb/linux-thread-db.c	2008-05-28 18:49:37.000000000 +0100
@@ -838,7 +838,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
Index: gdb/testsuite/gdb.threads/execl.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl.c	2008-05-28 18:54:06.000000000 +0100
@@ -0,0 +1,38 @@
+/* Test handling thread control across an execl.  */
+
+/* The original image loads a thread library and has several threads,
+   while the new image does not load a thread library.  */
+
+#include <unistd.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+void *
+thread_function (void *arg)
+{
+  while (1)
+    sleep (100);
+  return NULL;
+}
+
+int
+main (int argc, char* argv[])
+{
+  pthread_t thread1;
+  pthread_t thread2;
+  char *new_image;
+
+  pthread_create (&thread1, NULL, thread_function, NULL);
+  pthread_create (&thread2, NULL, thread_function, NULL);
+
+  new_image = malloc (strlen (argv[0]) + 2);
+  strcpy (new_image, argv[0]);
+  strcat (new_image, "1");
+
+  if (execl (new_image, new_image, NULL) == -1) /* set breakpoint here */
+    return 1;
+
+  return 0;
+}
Index: gdb/testsuite/gdb.threads/execl.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl.exp	2008-05-28 18:53:39.000000000 +0100
@@ -0,0 +1,75 @@
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test handling of threads across an execl.
+
+
+# Original image, loads a thread library.
+set testfile "execl"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+
+# New image, that does not load a thread library.
+set testfile1 "execl1"
+set srcfile1 ${testfile1}.c
+set binfile1 ${objdir}/${subdir}/${testfile1}
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" executable {debug}] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+runto_main
+
+gdb_test "b [gdb_get_line_number "breakpoint here"]" \
+         ".*Breakpoint .*execl.*" "set breakpoint at execl"
+
+gdb_test "continue" ".*breakpoint here.*" "continue to exec"
+
+gdb_test "info threads" ".*3 Thread.*2 Thread.*1 Thread.*" "info threads before exec"
+
+# When continuing from this point we'll hit the breakpoint in main()
+# again, this time in the exec'd process.
+gdb_test "continue" ".*Breakpoint 1, main.*" \
+    "continue across exec"
+
+gdb_test "info threads" ".*" "info threads after exec"
+
+set test "info threads after exec"
+gdb_test_multiple "info threads" "$test" {
+    -re "2 Thread .*$gdb_prompt $" {
+	# Old threads left behind.
+	fail "$test"
+    }
+    -re "4 Thread .*$gdb_prompt $" {
+	# New threads registered.
+	fail "$test"
+    }
+    -re "$gdb_prompt $" {
+	# Target doesn't register the main thread, pass for now.
+	pass "$test"
+    }
+}
+
+gdb_test "continue" ".*Program exited normally\\." \
+    "continue to end"
Index: gdb/testsuite/gdb.threads/execl1.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl1.c	2008-05-28 18:58:00.000000000 +0100
@@ -0,0 +1,9 @@
+/* Test handling thread control across an execl.  */
+
+/* New exec image that doesn't load any thread library.  */
+
+int
+main (int argc, char* argv[])
+{
+  return 0;
+}

Re: [new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Paul Pluzhnikov]

Ping,

http://sourceware.org/ml/gdb-patches/2008-05/msg00727.html

On Wed, May 28, 2008 at 11:01 AM, Pedro Alves <pedro@codesourcery.com> wrote:
...
> New patch attached.  Can anyone approve (or reject) this?


-- 
Paul Pluzhnikov

Re: [new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Michael Snyder]

On Wed, 2008-06-04 at 18:16 -0700, Paul Pluzhnikov wrote:
> Ping,
> 
> http://sourceware.org/ml/gdb-patches/2008-05/msg00727.html
> 
> On Wed, May 28, 2008 at 11:01 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> ...
> > New patch attached.  Can anyone approve (or reject) this?

Well, I'll throw my two cents in and say that it looks
reasonable -- I'd give good odds that it's right, but it's
been too long since I worked closely on linux threads for
me to say positively.  

Other guys?

Great job on the test cases, by the way.

Michael

Re: [new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Daniel Jacobowitz]

On Thu, Jun 05, 2008 at 10:22:37AM -0700, Michael Snyder wrote:
> On Wed, 2008-06-04 at 18:16 -0700, Paul Pluzhnikov wrote:
> > Ping,
> > 
> > http://sourceware.org/ml/gdb-patches/2008-05/msg00727.html
> > 
> > On Wed, May 28, 2008 at 11:01 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> > ...
> > > New patch attached.  Can anyone approve (or reject) this?
> 
> Well, I'll throw my two cents in and say that it looks
> reasonable -- I'd give good odds that it's right, but it's
> been too long since I worked closely on linux threads for
> me to say positively.  

You're right - the patch is correct :-)

Pedro, could you add an FSF copyright notice to the new C file before
you check it in?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 209 bytes --]

A Thursday 05 June 2008 21:42:40, Daniel Jacobowitz wrote:

> Pedro, could you add an FSF copyright notice to the new C file before
> you check it in?

Done.  Checked in as attached.

Thanks,

-- 
Pedro Alves

[-- Attachment #2: execl.diff --]
[-- Type: text/x-diff, Size: 6774 bytes --]

gdb/
2008-06-05  Pedro Alves  <pedro@codesourcery.com>

	* linux-thread-db.c (thread_db_wait): Don't trim event ptid.

testsuite/
2008-06-05  Paul Pluzhnikov  <ppluzhnikov@google.com>
            Pedro Alves  <pedro@codesourcery.com>

	* gdb.threads/execl.c, gdb.threads/execl1.c,
	gdb.threads/execl.exp: New tests.

---
 gdb/linux-thread-db.c               |    2 
 gdb/testsuite/gdb.threads/execl.c   |   56 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/execl.exp |   75 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/execl1.c  |   27 ++++++++++++
 4 files changed, 159 insertions(+), 1 deletion(-)

Index: gdb/linux-thread-db.c
===================================================================
--- gdb/linux-thread-db.c.orig	2008-05-28 18:49:15.000000000 +0100
+++ gdb/linux-thread-db.c	2008-05-28 18:49:37.000000000 +0100
@@ -838,7 +838,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
Index: gdb/testsuite/gdb.threads/execl.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl.c	2008-06-05 21:53:34.000000000 +0100
@@ -0,0 +1,56 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/* Test handling thread control across an execl.  */
+
+/* The original image loads a thread library and has several threads,
+   while the new image does not load a thread library.  */
+
+#include <unistd.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+void *
+thread_function (void *arg)
+{
+  while (1)
+    sleep (100);
+  return NULL;
+}
+
+int
+main (int argc, char* argv[])
+{
+  pthread_t thread1;
+  pthread_t thread2;
+  char *new_image;
+
+  pthread_create (&thread1, NULL, thread_function, NULL);
+  pthread_create (&thread2, NULL, thread_function, NULL);
+
+  new_image = malloc (strlen (argv[0]) + 2);
+  strcpy (new_image, argv[0]);
+  strcat (new_image, "1");
+
+  if (execl (new_image, new_image, NULL) == -1) /* set breakpoint here */
+    return 1;
+
+  return 0;
+}
Index: gdb/testsuite/gdb.threads/execl.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl.exp	2008-05-28 18:53:39.000000000 +0100
@@ -0,0 +1,75 @@
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test handling of threads across an execl.
+
+
+# Original image, loads a thread library.
+set testfile "execl"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+
+# New image, that does not load a thread library.
+set testfile1 "execl1"
+set srcfile1 ${testfile1}.c
+set binfile1 ${objdir}/${subdir}/${testfile1}
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" executable {debug}] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+runto_main
+
+gdb_test "b [gdb_get_line_number "breakpoint here"]" \
+         ".*Breakpoint .*execl.*" "set breakpoint at execl"
+
+gdb_test "continue" ".*breakpoint here.*" "continue to exec"
+
+gdb_test "info threads" ".*3 Thread.*2 Thread.*1 Thread.*" "info threads before exec"
+
+# When continuing from this point we'll hit the breakpoint in main()
+# again, this time in the exec'd process.
+gdb_test "continue" ".*Breakpoint 1, main.*" \
+    "continue across exec"
+
+gdb_test "info threads" ".*" "info threads after exec"
+
+set test "info threads after exec"
+gdb_test_multiple "info threads" "$test" {
+    -re "2 Thread .*$gdb_prompt $" {
+	# Old threads left behind.
+	fail "$test"
+    }
+    -re "4 Thread .*$gdb_prompt $" {
+	# New threads registered.
+	fail "$test"
+    }
+    -re "$gdb_prompt $" {
+	# Target doesn't register the main thread, pass for now.
+	pass "$test"
+    }
+}
+
+gdb_test "continue" ".*Program exited normally\\." \
+    "continue to end"
Index: gdb/testsuite/gdb.threads/execl1.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.threads/execl1.c	2008-06-05 21:53:41.000000000 +0100
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/* Test handling thread control across an execl.  */
+
+/* New exec image that doesn't load any thread library.  */
+
+int
+main (int argc, char* argv[])
+{
+  return 0;
+}

Re: [RFC] Fix for gdb crash in "info thread" after exec().

[Paul Pluzhnikov]

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.

> 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.

Thanks,
-- 
Paul Pluzhnikov