This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec().
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
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;
+}