From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23188 invoked by alias); 28 May 2008 15:27:22 -0000 Received: (qmail 23180 invoked by uid 22791); 28 May 2008 15:27:21 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 28 May 2008 15:26:54 +0000 Received: (qmail 5443 invoked from network); 28 May 2008 15:26:48 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 May 2008 15:26:48 -0000 From: Pedro Alves To: "Paul Pluzhnikov" Subject: [new patch] Re: [RFC] Fix for gdb crash in "info thread" after exec(). Date: Wed, 28 May 2008 21:25:00 -0000 User-Agent: KMail/1.9.9 Cc: gdb-patches@sourceware.org References: <20080527190702.6956D3A6952@localhost> <200805272106.16082.pedro@codesourcery.com> <8ac60eac0805271623g492ade49j4da414d3c757d846@mail.gmail.com> In-Reply-To: <8ac60eac0805271623g492ade49j4da414d3c757d846@mail.gmail.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_6mXPIfe/fKlvL6Z" Message-Id: <200805281626.50992.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-05/txt/msg00732.txt.bz2 --Boundary-00=_6mXPIfe/fKlvL6Z Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 4553 A Wednesday 28 May 2008 00:23:25, Paul Pluzhnikov wrote: > On Tue, May 27, 2008 at 1:06 PM, Pedro Alves 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 --Boundary-00=_6mXPIfe/fKlvL6Z Content-Type: text/x-diff; charset="iso-8859-1"; name="execl.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="execl.diff" Content-length: 5197 gdb/ 2008-05-28 Pedro Alves * linux-thread-db.c (thread_db_wait): Don't trim event ptid. testsuite/ 2008-05-28 Paul Pluzhnikov Pedro Alves * 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 +#include +#include +#include +#include + +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 . + +# 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; +} --Boundary-00=_6mXPIfe/fKlvL6Z--