* [PATCH v2 0/2] PR threads/18600: Threads left stopped after fork+thread spawn
@ 2015-07-23 17:24 Pedro Alves
2015-07-23 17:25 ` [PATCH v2 1/2] " Pedro Alves
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Pedro Alves @ 2015-07-23 17:24 UTC (permalink / raw)
To: gdb-patches
This is intended for both master and 7.10.
The discussions on PR threads/18600 at:
https://sourceware.org/ml/gdb-patches/2015-07/msg00153.html
identified two problems. These patches are another revision of the
patches original posted at:
[1] https://sourceware.org/ml/gdb-patches/2015-07/msg00186.html
[2] https://sourceware.org/ml/gdb-patches/2015-07/msg00190.html
Which later Simon cleaned up a bit and wrote a test for:
https://sourceware.org/ml/gdb-patches/2015-07/msg00595.html
This revision addresses the comments I made to Simon's version.
Tested on x86_64 Fedora 20, native, remote and extended-remote
gdbserver.
Pedro Alves (2):
PR threads/18600: Threads left stopped after fork+thread spawn
PR threads/18600: Inferiors left around after fork+thread spawn
gdb/linux-nat.c | 111 ++++++++++++-----------
gdb/testsuite/gdb.threads/fork-plus-threads.c | 115 ++++++++++++++++++++++++
gdb/testsuite/gdb.threads/fork-plus-threads.exp | 65 ++++++++++++++
3 files changed, 235 insertions(+), 56 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads.c
create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads.exp
--
1.9.3
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 1/2] PR threads/18600: Threads left stopped after fork+thread spawn 2015-07-23 17:24 [PATCH v2 0/2] PR threads/18600: Threads left stopped after fork+thread spawn Pedro Alves @ 2015-07-23 17:25 ` Pedro Alves 2015-07-30 18:08 ` [pushed] " Pedro Alves 2015-07-23 17:25 ` [PATCH v2 2/2] PR threads/18600: Inferiors left around " Pedro Alves 2015-07-23 18:21 ` [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode Pedro Alves 2 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2015-07-23 17:25 UTC (permalink / raw) To: gdb-patches When a program forks and another process start threads while gdb is handling the fork event, newly created threads are left stuck stopped by gdb, even though gdb presents them as "running", to the user. This can be seen with the test added by this patch. The test has the inferior fork a certain number of times and waits for all children to exit. Each fork child spawns a number of threads that do nothing and joins them immediately. Normally, the program should run unimpeded (from the point of view of the user) and exit very quickly. Without this fix, it doesn't because of some threads left stopped by gdb, so inferior 1 never exits. The program triggers when a new clone thread is found while inside the linux_stop_and_wait_all_lwps call in linux-thread-db.c: linux_stop_and_wait_all_lwps (); ALL_LWPS (lp) if (ptid_get_pid (lp->ptid) == pid) thread_from_lwp (lp->ptid); linux_unstop_all_lwps (); Within linux_stop_and_wait_all_lwps, we reach linux_handle_extended_wait with the "stopping" parameter set to 1, and because of that we don't mark the new lwp as resumed. As consequence, the subsequent resume_stopped_resumed_lwps, called from linux_unstop_all_lwps, never resumes the new LWP. There's lots of cruft in linux_handle_extended_wait that no longer makes sense. On systems with CLONE events support, we don't rely on libthread_db for thread listing anymore, so the code that preserves stop_requested and the handling of last_resume_kind is all dead. So the fix is to remove all that, and simply always mark the new LWP as resumed, so that resume_stopped_resumed_lwps re-resumes it. gdb/ChangeLog: 2015-07-23 Pedro Alves <palves@redhat.com> Simon Marchi <simon.marchi@ericsson.com> PR threads/18600 * linux-nat.c (linux_handle_extended_wait): On CLONE event, always mark the new thread as resumed. Remove STOPPING parameter. (wait_lwp): Adjust call to linux_handle_extended_wait. (linux_nat_filter_event): Adjust call to linux_handle_extended_wait. (resume_stopped_resumed_lwps): Add debug output. gdb/testsuite/ChangeLog: 2015-07-23 Simon Marchi <simon.marchi@ericsson.com> Pedro Alves <palves@redhat.com> PR threads/18600 * gdb.threads/fork-plus-threads.c: New file. * gdb.threads/fork-plus-threads.exp: New file. --- gdb/linux-nat.c | 97 +++++++++----------- gdb/testsuite/gdb.threads/fork-plus-threads.c | 115 ++++++++++++++++++++++++ gdb/testsuite/gdb.threads/fork-plus-threads.exp | 61 +++++++++++++ 3 files changed, 217 insertions(+), 56 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads.c create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads.exp diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index be429f8..272b919 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2000,8 +2000,7 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping) true, the new LWP remains stopped, otherwise it is continued. */ static int -linux_handle_extended_wait (struct lwp_info *lp, int status, - int stopping) +linux_handle_extended_wait (struct lwp_info *lp, int status) { int pid = ptid_get_lwp (lp->ptid); struct target_waitstatus *ourstatus = &lp->waitstatus; @@ -2071,7 +2070,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, ourstatus->kind = TARGET_WAITKIND_FORKED; else if (event == PTRACE_EVENT_VFORK) ourstatus->kind = TARGET_WAITKIND_VFORKED; - else + else if (event == PTRACE_EVENT_CLONE) { struct lwp_info *new_lp; @@ -2086,43 +2085,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, new_lp = add_lwp (ptid_build (ptid_get_pid (lp->ptid), new_pid, 0)); new_lp->cloned = 1; new_lp->stopped = 1; - - if (WSTOPSIG (status) != SIGSTOP) - { - /* This can happen if someone starts sending signals to - the new thread before it gets a chance to run, which - have a lower number than SIGSTOP (e.g. SIGUSR1). - This is an unlikely case, and harder to handle for - fork / vfork than for clone, so we do not try - but - we handle it for clone events here. We'll send - the other signal on to the thread below. */ - - new_lp->signalled = 1; - } - else - { - struct thread_info *tp; - - /* When we stop for an event in some other thread, and - pull the thread list just as this thread has cloned, - we'll have seen the new thread in the thread_db list - before handling the CLONE event (glibc's - pthread_create adds the new thread to the thread list - before clone'ing, and has the kernel fill in the - thread's tid on the clone call with - CLONE_PARENT_SETTID). If that happened, and the core - had requested the new thread to stop, we'll have - killed it with SIGSTOP. But since SIGSTOP is not an - RT signal, it can only be queued once. We need to be - careful to not resume the LWP if we wanted it to - stop. In that case, we'll leave the SIGSTOP pending. - It will later be reported as GDB_SIGNAL_0. */ - tp = find_thread_ptid (new_lp->ptid); - if (tp != NULL && tp->stop_requested) - new_lp->last_resume_kind = resume_stop; - else - status = 0; - } + new_lp->resumed = 1; /* If the thread_db layer is active, let it record the user level thread id and status, and add the thread to GDB's @@ -2136,19 +2099,23 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, } /* Even if we're stopping the thread for some reason - internal to this module, from the user/frontend's - perspective, this new thread is running. */ + internal to this module, from the perspective of infrun + and the user/frontend, this new thread is running until + it next reports a stop. */ set_running (new_lp->ptid, 1); - if (!stopping) - { - set_executing (new_lp->ptid, 1); - /* thread_db_attach_lwp -> lin_lwp_attach_lwp forced - resume_stop. */ - new_lp->last_resume_kind = resume_continue; - } + set_executing (new_lp->ptid, 1); - if (status != 0) + if (WSTOPSIG (status) != SIGSTOP) { + /* This can happen if someone starts sending signals to + the new thread before it gets a chance to run, which + have a lower number than SIGSTOP (e.g. SIGUSR1). + This is an unlikely case, and harder to handle for + fork / vfork than for clone, so we do not try - but + we handle it for clone events here. */ + + new_lp->signalled = 1; + /* We created NEW_LP so it cannot yet contain STATUS. */ gdb_assert (new_lp->status == 0); @@ -2162,7 +2129,6 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, new_lp->status = status; } - new_lp->resumed = !stopping; return 1; } @@ -2353,7 +2319,7 @@ wait_lwp (struct lwp_info *lp) fprintf_unfiltered (gdb_stdlog, "WL: Handling extended status 0x%06x\n", status); - linux_handle_extended_wait (lp, status, 1); + linux_handle_extended_wait (lp, status); return 0; } @@ -3155,7 +3121,7 @@ linux_nat_filter_event (int lwpid, int status) fprintf_unfiltered (gdb_stdlog, "LLW: Handling extended status 0x%06x\n", status); - if (linux_handle_extended_wait (lp, status, 0)) + if (linux_handle_extended_wait (lp, status)) return NULL; } @@ -3673,9 +3639,28 @@ resume_stopped_resumed_lwps (struct lwp_info *lp, void *data) { ptid_t *wait_ptid_p = data; - if (lp->stopped - && lp->resumed - && !lwp_status_pending_p (lp)) + if (!lp->stopped) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "RSRL: NOT resuming LWP %s, not stopped\n", + target_pid_to_str (lp->ptid)); + } + else if (!lp->resumed) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "RSRL: NOT resuming LWP %s, not resumed\n", + target_pid_to_str (lp->ptid)); + } + else if (lwp_status_pending_p (lp)) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "RSRL: NOT resuming LWP %s, has pending status\n", + target_pid_to_str (lp->ptid)); + } + else { struct regcache *regcache = get_thread_regcache (lp->ptid); struct gdbarch *gdbarch = get_regcache_arch (regcache); diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.c b/gdb/testsuite/gdb.threads/fork-plus-threads.c new file mode 100644 index 0000000..780a4b8 --- /dev/null +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.c @@ -0,0 +1,115 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +#include <assert.h> +#include <pthread.h> +#include <unistd.h> +#include <stdio.h> +#include <sys/types.h> +#include <sys/wait.h> + + +/* Number of times the main process forks. */ +#define NFORKS 10 + +/* Number of threads by each fork child. */ +#define NTHREADS 10 + +static void * +thread_func (void *arg) +{ + /* Empty. */ +} + +static void +fork_child (void) +{ + pthread_t threads[NTHREADS]; + int i; + int ret; + + for (i = 0; i < NTHREADS; i++) + { + ret = pthread_create (&threads[i], NULL, thread_func, NULL); + assert (ret == 0); + } + + for (i = 0; i < NTHREADS; i++) + { + ret = pthread_join (threads[i], NULL); + assert (ret == 0); + } +} + +int +main (void) +{ + pid_t childs[NFORKS]; + int i; + int status; + int num_exited = 0; + + /* Don't run forever if the wait loop below gets stuck. */ + alarm (180); + + for (i = 0; i < NFORKS; i++) + { + pid_t pid; + + pid = fork (); + + if (pid > 0) + { + /* Parent. */ + childs[i] = pid; + } + else if (pid == 0) + { + /* Child. */ + fork_child (); + return 0; + } + else + { + perror ("fork"); + return 1; + } + } + + while (num_exited != NFORKS) + { + pid_t pid = wait (&status); + + if (pid == -1) + { + perror ("wait"); + return 1; + } + + if (WIFEXITED (status)) + { + num_exited++; + } + else + { + printf ("Hmm, unexpected wait status 0x%x from child %d\n", status, + pid); + } + } + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp new file mode 100644 index 0000000..9989346 --- /dev/null +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp @@ -0,0 +1,61 @@ +# Copyright (C) 2015 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/>. + +# This test verifies that threads created by the child fork are +# properly handled. Specifically, GDB used to have a bug where it +# would leave child fork threads stuck stopped, even though "info +# threads" would show them running. +# +# See https://sourceware.org/bugzilla/show_bug.cgi?id=18600 + +# gdbserver's fork support is broken in non-stop mode. +#if [gdb_is_target_remote] { +# return +#} + +standard_testfile +set saved_gdbflags $GDBFLAGS +set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""] + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} { + set GDBFLAGS $saved_gdbflags + return -1 +} + +if ![runto_main] then { + fail "Can't run to main" + set GDBFLAGS $saved_gdbflags + return 0 +} + +gdb_test_no_output "set detach-on-fork off" +set test "continue &" +gdb_test_multiple $test $test { + -re "$gdb_prompt " { + pass $test + } +} + +set test "reached breakpoint" +gdb_test_multiple "" $test { + -re "Inferior 1 \(\[^\r\n\]+\) exited normally" { + pass $test + } +} + +gdb_test "info threads" "No threads\." \ + "no threads left" + +set GDBFLAGS $saved_gdbflags -- 1.9.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [pushed] Re: [PATCH v2 1/2] PR threads/18600: Threads left stopped after fork+thread spawn 2015-07-23 17:25 ` [PATCH v2 1/2] " Pedro Alves @ 2015-07-30 18:08 ` Pedro Alves 0 siblings, 0 replies; 16+ messages in thread From: Pedro Alves @ 2015-07-30 18:08 UTC (permalink / raw) To: gdb-patches On 07/23/2015 06:24 PM, Pedro Alves wrote: > When a program forks and another process start threads while gdb is > handling the fork event, newly created threads are left stuck stopped > by gdb, even though gdb presents them as "running", to the user. > > This can be seen with the test added by this patch. The test has the > inferior fork a certain number of times and waits for all children to > exit. Each fork child spawns a number of threads that do nothing and > joins them immediately. Normally, the program should run unimpeded > (from the point of view of the user) and exit very quickly. Without > this fix, it doesn't because of some threads left stopped by gdb, so > inferior 1 never exits. > > The program triggers when a new clone thread is found while inside the > linux_stop_and_wait_all_lwps call in linux-thread-db.c: > > linux_stop_and_wait_all_lwps (); > > ALL_LWPS (lp) > if (ptid_get_pid (lp->ptid) == pid) > thread_from_lwp (lp->ptid); > > linux_unstop_all_lwps (); > > Within linux_stop_and_wait_all_lwps, we reach > linux_handle_extended_wait with the "stopping" parameter set to 1, and > because of that we don't mark the new lwp as resumed. As consequence, > the subsequent resume_stopped_resumed_lwps, called from > linux_unstop_all_lwps, never resumes the new LWP. > > There's lots of cruft in linux_handle_extended_wait that no longer > makes sense. On systems with CLONE events support, we don't rely on > libthread_db for thread listing anymore, so the code that preserves > stop_requested and the handling of last_resume_kind is all dead. > > So the fix is to remove all that, and simply always mark the new LWP > as resumed, so that resume_stopped_resumed_lwps re-resumes it. > > gdb/ChangeLog: > 2015-07-23 Pedro Alves <palves@redhat.com> > Simon Marchi <simon.marchi@ericsson.com> > > PR threads/18600 > * linux-nat.c (linux_handle_extended_wait): On CLONE event, always > mark the new thread as resumed. Remove STOPPING parameter. > (wait_lwp): Adjust call to linux_handle_extended_wait. > (linux_nat_filter_event): Adjust call to > linux_handle_extended_wait. > (resume_stopped_resumed_lwps): Add debug output. > > gdb/testsuite/ChangeLog: > 2015-07-23 Simon Marchi <simon.marchi@ericsson.com> > Pedro Alves <palves@redhat.com> > > PR threads/18600 > * gdb.threads/fork-plus-threads.c: New file. > * gdb.threads/fork-plus-threads.exp: New file. Here's what I pushed, to both master and 7.10 branch. Mostly the same as before, but I made the test check both set detach-on-fork on/off. From 4dd63d488a76482543517c4c4cde699ee6fa33ef Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Thu, 30 Jul 2015 18:50:29 +0100 Subject: [PATCH 1/3] PR threads/18600: Threads left stopped after fork+thread spawn When a program forks and another process start threads while gdb is handling the fork event, newly created threads are left stuck stopped by gdb, even though gdb presents them as "running", to the user. This can be seen with the test added by this patch. The test has the inferior fork a certain number of times and waits for all children to exit. Each fork child spawns a number of threads that do nothing and joins them immediately. Normally, the program should run unimpeded (from the point of view of the user) and exit very quickly. Without this fix, it doesn't because of some threads left stopped by gdb, so inferior 1 never exits. The program triggers when a new clone thread is found while inside the linux_stop_and_wait_all_lwps call in linux-thread-db.c: linux_stop_and_wait_all_lwps (); ALL_LWPS (lp) if (ptid_get_pid (lp->ptid) == pid) thread_from_lwp (lp->ptid); linux_unstop_all_lwps (); Within linux_stop_and_wait_all_lwps, we reach linux_handle_extended_wait with the "stopping" parameter set to 1, and because of that we don't mark the new lwp as resumed. As consequence, the subsequent resume_stopped_resumed_lwps, called from linux_unstop_all_lwps, never resumes the new LWP. There's lots of cruft in linux_handle_extended_wait that no longer makes sense. On systems with CLONE events support, we don't rely on libthread_db for thread listing anymore, so the code that preserves stop_requested and the handling of last_resume_kind is all dead. So the fix is to remove all that, and simply always mark the new LWP as resumed, so that resume_stopped_resumed_lwps re-resumes it. gdb/ChangeLog: 2015-07-30 Pedro Alves <palves@redhat.com> Simon Marchi <simon.marchi@ericsson.com> PR threads/18600 * linux-nat.c (linux_handle_extended_wait): On CLONE event, always mark the new thread as resumed. Remove STOPPING parameter. (wait_lwp): Adjust call to linux_handle_extended_wait. (linux_nat_filter_event): Adjust call to linux_handle_extended_wait. (resume_stopped_resumed_lwps): Add debug output. gdb/testsuite/ChangeLog: 2015-07-30 Simon Marchi <simon.marchi@ericsson.com> Pedro Alves <palves@redhat.com> PR threads/18600 * gdb.threads/fork-plus-threads.c: New file. * gdb.threads/fork-plus-threads.exp: New file. --- gdb/ChangeLog | 11 +++ gdb/testsuite/ChangeLog | 7 ++ gdb/linux-nat.c | 97 +++++++++----------- gdb/testsuite/gdb.threads/fork-plus-threads.c | 115 ++++++++++++++++++++++++ gdb/testsuite/gdb.threads/fork-plus-threads.exp | 69 ++++++++++++++ 5 files changed, 243 insertions(+), 56 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads.c create mode 100644 gdb/testsuite/gdb.threads/fork-plus-threads.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4d604de..40403f9 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2015-07-30 Pedro Alves <palves@redhat.com> + Simon Marchi <simon.marchi@ericsson.com> + + PR threads/18600 + * linux-nat.c (linux_handle_extended_wait): On CLONE event, always + mark the new thread as resumed. Remove STOPPING parameter. + (wait_lwp): Adjust call to linux_handle_extended_wait. + (linux_nat_filter_event): Adjust call to + linux_handle_extended_wait. + (resume_stopped_resumed_lwps): Add debug output. + 2015-07-30 Pierre Langlois <pierre.langlois@arm.com> * arch-utils.c (default_fast_tracepoint_valid_at): Remove unused diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 171784e..06ca987 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2015-07-30 Simon Marchi <simon.marchi@ericsson.com> + Pedro Alves <palves@redhat.com> + + PR threads/18600 + * gdb.threads/fork-plus-threads.c: New file. + * gdb.threads/fork-plus-threads.exp: New file. + 2015-07-29 Patrick Palka <patrick@parcs.ath.cx> * gdb.base/batch-preserve-term-settings.exp diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index b33abb0..966c6a8 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2000,8 +2000,7 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping) true, the new LWP remains stopped, otherwise it is continued. */ static int -linux_handle_extended_wait (struct lwp_info *lp, int status, - int stopping) +linux_handle_extended_wait (struct lwp_info *lp, int status) { int pid = ptid_get_lwp (lp->ptid); struct target_waitstatus *ourstatus = &lp->waitstatus; @@ -2071,7 +2070,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, ourstatus->kind = TARGET_WAITKIND_FORKED; else if (event == PTRACE_EVENT_VFORK) ourstatus->kind = TARGET_WAITKIND_VFORKED; - else + else if (event == PTRACE_EVENT_CLONE) { struct lwp_info *new_lp; @@ -2086,43 +2085,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, new_lp = add_lwp (ptid_build (ptid_get_pid (lp->ptid), new_pid, 0)); new_lp->cloned = 1; new_lp->stopped = 1; - - if (WSTOPSIG (status) != SIGSTOP) - { - /* This can happen if someone starts sending signals to - the new thread before it gets a chance to run, which - have a lower number than SIGSTOP (e.g. SIGUSR1). - This is an unlikely case, and harder to handle for - fork / vfork than for clone, so we do not try - but - we handle it for clone events here. We'll send - the other signal on to the thread below. */ - - new_lp->signalled = 1; - } - else - { - struct thread_info *tp; - - /* When we stop for an event in some other thread, and - pull the thread list just as this thread has cloned, - we'll have seen the new thread in the thread_db list - before handling the CLONE event (glibc's - pthread_create adds the new thread to the thread list - before clone'ing, and has the kernel fill in the - thread's tid on the clone call with - CLONE_PARENT_SETTID). If that happened, and the core - had requested the new thread to stop, we'll have - killed it with SIGSTOP. But since SIGSTOP is not an - RT signal, it can only be queued once. We need to be - careful to not resume the LWP if we wanted it to - stop. In that case, we'll leave the SIGSTOP pending. - It will later be reported as GDB_SIGNAL_0. */ - tp = find_thread_ptid (new_lp->ptid); - if (tp != NULL && tp->stop_requested) - new_lp->last_resume_kind = resume_stop; - else - status = 0; - } + new_lp->resumed = 1; /* If the thread_db layer is active, let it record the user level thread id and status, and add the thread to GDB's @@ -2136,19 +2099,23 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, } /* Even if we're stopping the thread for some reason - internal to this module, from the user/frontend's - perspective, this new thread is running. */ + internal to this module, from the perspective of infrun + and the user/frontend, this new thread is running until + it next reports a stop. */ set_running (new_lp->ptid, 1); - if (!stopping) - { - set_executing (new_lp->ptid, 1); - /* thread_db_attach_lwp -> lin_lwp_attach_lwp forced - resume_stop. */ - new_lp->last_resume_kind = resume_continue; - } + set_executing (new_lp->ptid, 1); - if (status != 0) + if (WSTOPSIG (status) != SIGSTOP) { + /* This can happen if someone starts sending signals to + the new thread before it gets a chance to run, which + have a lower number than SIGSTOP (e.g. SIGUSR1). + This is an unlikely case, and harder to handle for + fork / vfork than for clone, so we do not try - but + we handle it for clone events here. */ + + new_lp->signalled = 1; + /* We created NEW_LP so it cannot yet contain STATUS. */ gdb_assert (new_lp->status == 0); @@ -2162,7 +2129,6 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, new_lp->status = status; } - new_lp->resumed = !stopping; return 1; } @@ -2353,7 +2319,7 @@ wait_lwp (struct lwp_info *lp) fprintf_unfiltered (gdb_stdlog, "WL: Handling extended status 0x%06x\n", status); - linux_handle_extended_wait (lp, status, 1); + linux_handle_extended_wait (lp, status); return 0; } @@ -3155,7 +3121,7 @@ linux_nat_filter_event (int lwpid, int status) fprintf_unfiltered (gdb_stdlog, "LLW: Handling extended status 0x%06x\n", status); - if (linux_handle_extended_wait (lp, status, 0)) + if (linux_handle_extended_wait (lp, status)) return NULL; } @@ -3675,9 +3641,28 @@ resume_stopped_resumed_lwps (struct lwp_info *lp, void *data) { ptid_t *wait_ptid_p = data; - if (lp->stopped - && lp->resumed - && !lwp_status_pending_p (lp)) + if (!lp->stopped) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "RSRL: NOT resuming LWP %s, not stopped\n", + target_pid_to_str (lp->ptid)); + } + else if (!lp->resumed) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "RSRL: NOT resuming LWP %s, not resumed\n", + target_pid_to_str (lp->ptid)); + } + else if (lwp_status_pending_p (lp)) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "RSRL: NOT resuming LWP %s, has pending status\n", + target_pid_to_str (lp->ptid)); + } + else { struct regcache *regcache = get_thread_regcache (lp->ptid); struct gdbarch *gdbarch = get_regcache_arch (regcache); diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.c b/gdb/testsuite/gdb.threads/fork-plus-threads.c new file mode 100644 index 0000000..780a4b8 --- /dev/null +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.c @@ -0,0 +1,115 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +#include <assert.h> +#include <pthread.h> +#include <unistd.h> +#include <stdio.h> +#include <sys/types.h> +#include <sys/wait.h> + + +/* Number of times the main process forks. */ +#define NFORKS 10 + +/* Number of threads by each fork child. */ +#define NTHREADS 10 + +static void * +thread_func (void *arg) +{ + /* Empty. */ +} + +static void +fork_child (void) +{ + pthread_t threads[NTHREADS]; + int i; + int ret; + + for (i = 0; i < NTHREADS; i++) + { + ret = pthread_create (&threads[i], NULL, thread_func, NULL); + assert (ret == 0); + } + + for (i = 0; i < NTHREADS; i++) + { + ret = pthread_join (threads[i], NULL); + assert (ret == 0); + } +} + +int +main (void) +{ + pid_t childs[NFORKS]; + int i; + int status; + int num_exited = 0; + + /* Don't run forever if the wait loop below gets stuck. */ + alarm (180); + + for (i = 0; i < NFORKS; i++) + { + pid_t pid; + + pid = fork (); + + if (pid > 0) + { + /* Parent. */ + childs[i] = pid; + } + else if (pid == 0) + { + /* Child. */ + fork_child (); + return 0; + } + else + { + perror ("fork"); + return 1; + } + } + + while (num_exited != NFORKS) + { + pid_t pid = wait (&status); + + if (pid == -1) + { + perror ("wait"); + return 1; + } + + if (WIFEXITED (status)) + { + num_exited++; + } + else + { + printf ("Hmm, unexpected wait status 0x%x from child %d\n", status, + pid); + } + } + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp new file mode 100644 index 0000000..53d1102 --- /dev/null +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp @@ -0,0 +1,69 @@ +# Copyright (C) 2015 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/>. + +# This test verifies that threads created by the child fork are +# properly handled. Specifically, GDB used to have a bug where it +# would leave child fork threads stuck stopped, even though "info +# threads" would show them running. +# +# See https://sourceware.org/bugzilla/show_bug.cgi?id=18600 + +standard_testfile + +proc do_test { detach_on_fork } { + global GDBFLAGS + global srcfile testfile + global gdb_prompt + + set saved_gdbflags $GDBFLAGS + set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""] + + if {[prepare_for_testing "failed to prepare" \ + $testfile $srcfile {debug pthreads}] == -1} { + set GDBFLAGS $saved_gdbflags + return -1 + } + + set GDBFLAGS $saved_gdbflags + + if ![runto_main] then { + fail "Can't run to main" + return 0 + } + + gdb_test_no_output "set detach-on-fork $detach_on_fork" + set test "continue &" + gdb_test_multiple $test $test { + -re "$gdb_prompt " { + pass $test + } + } + + set test "inferior 1 exited" + gdb_test_multiple "" $test { + -re "Inferior 1 \(\[^\r\n\]+\) exited normally" { + pass $test + } + } + + gdb_test "info threads" "No threads\." \ + "no threads left" +} + +foreach detach_on_fork {"on" "off"} { + with_test_prefix "detach-on-fork=$detach_on_fork" { + do_test $detach_on_fork + } +} -- 1.9.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] PR threads/18600: Inferiors left around after fork+thread spawn 2015-07-23 17:24 [PATCH v2 0/2] PR threads/18600: Threads left stopped after fork+thread spawn Pedro Alves 2015-07-23 17:25 ` [PATCH v2 1/2] " Pedro Alves @ 2015-07-23 17:25 ` Pedro Alves 2015-07-23 18:21 ` [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode Pedro Alves 2 siblings, 0 replies; 16+ messages in thread From: Pedro Alves @ 2015-07-23 17:25 UTC (permalink / raw) To: gdb-patches The new gdb.threads/fork-plus-threads.exp test exposes one more problem. When one types "info inferiors" after running the program, one see's a couple inferior left still, while there should only be inferior #1 left. E.g.: (gdb) info inferiors Num Description Executable 4 process 8393 /home/pedro/bugs/src/test 2 process 8388 /home/pedro/bugs/src/test * 1 <null> /home/pedro/bugs/src/test (gdb) info threads Calling prune_inferiors() manually at this point (from a top gdb) does not remove them, because they still have inf->pid != 0 (while they shouldn't). This suggests that we never mourned those inferiors. Enabling logs (master + previous patch) we see: ... WL: waitpid Thread 0x7ffff7fc2740 (LWP 9513) received Trace/breakpoint trap (stopped) WL: Handling extended status 0x03057f LHEW: Got clone event from LWP 9513, new child is LWP 9579 [New Thread 0x7ffff37b8700 (LWP 9579)] WL: waitpid Thread 0x7ffff7fc2740 (LWP 9508) received 0 (exited) WL: Thread 0x7ffff7fc2740 (LWP 9508) exited. ^^^^^^^^ [Thread 0x7ffff7fc2740 (LWP 9508) exited] WL: waitpid Thread 0x7ffff7fc2740 (LWP 9499) received 0 (exited) WL: Thread 0x7ffff7fc2740 (LWP 9499) exited. [Thread 0x7ffff7fc2740 (LWP 9499) exited] RSRL: resuming stopped-resumed LWP Thread 0x7ffff37b8700 (LWP 9579) at 0x3615ef4ce1: step=0 ... (gdb) info inferiors Num Description Executable 5 process 9508 /home/pedro/bugs/src/test ^^^^ 4 process 9503 /home/pedro/bugs/src/test 3 process 9500 /home/pedro/bugs/src/test 2 process 9499 /home/pedro/bugs/src/test * 1 <null> /home/pedro/bugs/src/test (gdb) ... Note the "Thread 0x7ffff7fc2740 (LWP 9508) exited." line. That's this in wait_lwp: /* Check if the thread has exited. */ if (WIFEXITED (status) || WIFSIGNALED (status)) { thread_dead = 1; if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "WL: %s exited.\n", target_pid_to_str (lp->ptid)); } } That was the leader thread reporting an exit, meaning the whole process is gone. So the problem is that this code doesn't understand that an WIFEXITED status of the leader LWP should be reported to infrun as process exit. gdb/ChangeLog: 2015-07-23 Pedro Alves <palves@redhat.com> PR threads/18600 * linux-nat.c (wait_lwp): Report to the core when thread group leader exits. gdb/testsuite/ChangeLog: 2015-07-23 Pedro Alves <palves@redhat.com> PR threads/18600 * gdb.threads/fork-plus-threads.exp: Test that "info inferiors" only shows inferior 1. --- gdb/linux-nat.c | 14 ++++++++++++++ gdb/testsuite/gdb.threads/fork-plus-threads.exp | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 272b919..74d5997 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2274,6 +2274,20 @@ wait_lwp (struct lwp_info *lp) /* Check if the thread has exited. */ if (WIFEXITED (status) || WIFSIGNALED (status)) { + if (ptid_get_pid (lp->ptid) == ptid_get_lwp (lp->ptid)) + { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, "WL: Process %d exited.\n", + ptid_get_pid (lp->ptid)); + + /* This is the leader exiting, it means the whole + process is gone. Store the status to report to the + core. Store it in lp->waitstatus, because lp->status + would be ambiguous (W_EXITCODE(0,0) == 0). */ + store_waitstatus (&lp->waitstatus, status); + return 0; + } + thread_dead = 1; if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "WL: %s exited.\n", diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp index 9989346..f44dd76 100644 --- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp @@ -58,4 +58,8 @@ gdb_test_multiple "" $test { gdb_test "info threads" "No threads\." \ "no threads left" +gdb_test "info inferiors" \ + "Num\[ \t\]+Description\[ \t\]+Executable\[ \t\]+\r\n\\* 1 \[^\r\n\]+" \ + "only inferior 1 left" + set GDBFLAGS $saved_gdbflags -- 1.9.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-23 17:24 [PATCH v2 0/2] PR threads/18600: Threads left stopped after fork+thread spawn Pedro Alves 2015-07-23 17:25 ` [PATCH v2 1/2] " Pedro Alves 2015-07-23 17:25 ` [PATCH v2 2/2] PR threads/18600: Inferiors left around " Pedro Alves @ 2015-07-23 18:21 ` Pedro Alves 2015-07-24 18:05 ` Simon Marchi 2015-07-24 18:43 ` Don Breazeal 2 siblings, 2 replies; 16+ messages in thread From: Pedro Alves @ 2015-07-23 18:21 UTC (permalink / raw) To: Breazeal, Don, Simon Marchi; +Cc: GDB Patches So I managed to extract out this smaller patch from the gdbserver fixes I mentioned. I think this one looks safe enough for 7.10. WDYT? ----------- From 98d41152bff2a21f7fda864d87ee5dd0cffa2d17 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Thu, 23 Jul 2015 18:49:51 +0100 Subject: [PATCH] remote follow fork and spurious child stops in non-stop mode Running gdb.threads/fork-plus-threads.exp against gdbserver in extended-remote mode, even though the test passes, we still see broken behavior: Running gdb.threads/fork-plus-threads.exp against gdbserver in extended-remote mode, even though the test passes, we still see broken behavior: (gdb) PASS: gdb.threads/fork-plus-threads.exp: set detach-on-fork off continue & Continuing. (gdb) PASS: gdb.threads/fork-plus-threads.exp: continue & [New Thread 28092.28092] [Thread 28092.28092] #2 stopped. [New Thread 28094.28094] [Inferior 2 (process 28092) exited normally] [New Thread 28094.28105] [New Thread 28094.28109] ... [Thread 28174.28174] #18 stopped. [New Thread 28185.28185] [Inferior 10 (process 28174) exited normally] [New Thread 28185.28196] [Thread 28185.28185] #20 stopped. Cannot remove breakpoints because program is no longer writable. Further execution is probably impossible. [Inferior 11 (process 28185) exited normally] [Inferior 1 (process 28091) exited normally] PASS: gdb.threads/fork-plus-threads.exp: reached breakpoint info threads No threads. (gdb) PASS: gdb.threads/fork-plus-threads.exp: no threads left info inferiors Num Description Executable * 1 <null> /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.threads/fork-plus-threads (gdb) PASS: gdb.threads/fork-plus-threads.exp: only inferior 1 left All the "[Thread FOO] #NN stopped." above are bogus, as well as the "Cannot remove breakpoints because program is no longer writable.", which is a consequence. The problem is that when we intercept a fork event, we should report the event for the parent, only, and leave the child stopped, but not report its stop event. GDB later decides whether to follow the parent or the child. But because handle_extended_wait does not set the child's last_status.kind to TARGET_WAITKIND_STOPPED, a stop_all_threads/unstop_all_lwps sequence (e.g., from trying to access memory) by mistake ends up queueing a SIGSTOP on the child, resuming it, and then when that SIGSTOP is intercepted, because the LWP has last_resume_kind set to resume_stop, gdbserver reports the stop to GDB, as GDB_SIGNAL_0: ... >>>> entering unstop_all_lwps unstopping all lwps proceed_one_lwp: lwp 1600 client wants LWP to remain 1600 stopped proceed_one_lwp: lwp 1828 Client wants LWP 1828 to stop. Making sure it has a SIGSTOP pending ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sending sigstop to lwp 1828 pc is 0x3615ebc7cc Resuming lwp 1828 (continue, signal 0, stop expected) continue from pc 0x3615ebc7cc unstop_all_lwps done sigchld_handler <<<< exiting unstop_all_lwps handling possible target event >>>> entering linux_wait_1 linux_wait_1: [<all threads>] my_waitpid (-1, 0x40000001) my_waitpid (-1, 0x1): status(137f), 1828 LWFE: waitpid(-1, ...) returned 1828, ERRNO-OK LLW: waitpid 1828 received Stopped (signal) (stopped) pc is 0x3615ebc7cc Expected stop. LLW: resume_stop SIGSTOP caught for LWP 1828.1828. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... linux_wait_1 ret = LWP 1828.1828, 1, 0 <<<< exiting linux_wait_1 Writing resume reply for LWP 1828.1828:1 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ By inspection, I also noticed that we miss leaving the child with the suspend count incremented if stopping threads, like we do for clone threads. Tested on x86_64 Fedora 20, extended-remote. gdb/gdbserver/ChangeLog: 2015-07-23 Pedro Alves <palves@redhat.com> * linux-low.c (handle_extended_wait): Set the child's last reported status to TARGET_WAITKIND_STOPPED. --- gdb/gdbserver/linux-low.c | 7 ++++++ gdb/testsuite/gdb.threads/fork-plus-threads.exp | 30 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 17b2a51..56a33ff 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -488,6 +488,13 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) child_lwp->status_pending_p = 0; child_thr = get_lwp_thread (child_lwp); child_thr->last_resume_kind = resume_stop; + child_thr->last_status.kind = TARGET_WAITKIND_STOPPED; + + /* If we're suspending all threads, leave this one suspended + too. */ + if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS) + child_lwp->suspended = 1; + parent_proc = get_thread_process (event_thr); child_proc->attached = parent_proc->attached; clone_all_breakpoints (&child_proc->breakpoints, diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp index f44dd76..80d2464 100644 --- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp @@ -48,13 +48,43 @@ gdb_test_multiple $test $test { } } +# gdbserver had a bug that resulted in reporting the fork child's +# initial stop to gdb, which gdb does not expect, in turn resulting in +# a broken session, like: +# +# [Thread 31536.31536] #16 stopped. <== BAD +# [New Thread 31547.31547] +# [Inferior 10 (process 31536) exited normally] +# [New Thread 31547.31560] +# +# [Thread 31547.31547] #18 stopped. <== BAD +# Cannot remove breakpoints because program is no longer writable. <== BAD +# Further execution is probably impossible. <== BAD +# [Inferior 11 (process 31547) exited normally] +# [Inferior 1 (process 31454) exited normally] +# +# These variables track whether we see such broken behavior. +set saw_cannot_remove_breakpoints 0 +set saw_thread_stopped 0 + set test "reached breakpoint" gdb_test_multiple "" $test { + -re "Cannot remove breakpoints" { + set saw_cannot_remove_breakpoints 1 + exp_continue + } + -re "Thread \[^\r\n\]+ stopped\\." { + set saw_thread_stopped 1 + exp_continue + } -re "Inferior 1 \(\[^\r\n\]+\) exited normally" { pass $test } } +gdb_assert !$saw_cannot_remove_breakpoints "no failure to remove breakpoints" +gdb_assert !$saw_thread_stopped "no spurious thread stop" + gdb_test "info threads" "No threads\." \ "no threads left" -- 1.9.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-23 18:21 ` [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode Pedro Alves @ 2015-07-24 18:05 ` Simon Marchi 2015-07-24 18:17 ` Pedro Alves 2015-07-24 18:43 ` Don Breazeal 1 sibling, 1 reply; 16+ messages in thread From: Simon Marchi @ 2015-07-24 18:05 UTC (permalink / raw) To: Pedro Alves, Breazeal, Don; +Cc: GDB Patches On 15-07-23 02:21 PM, Pedro Alves wrote: > So I managed to extract out this smaller patch from the > gdbserver fixes I mentioned. I think this one looks safe enough > for 7.10. WDYT? > > ----------- > From 98d41152bff2a21f7fda864d87ee5dd0cffa2d17 Mon Sep 17 00:00:00 2001 > From: Pedro Alves <palves@redhat.com> > Date: Thu, 23 Jul 2015 18:49:51 +0100 > Subject: [PATCH] remote follow fork and spurious child stops in non-stop mode > > Running gdb.threads/fork-plus-threads.exp against gdbserver in > extended-remote mode, even though the test passes, we still see broken > behavior: > > Running gdb.threads/fork-plus-threads.exp against gdbserver in > extended-remote mode, even though the test passes, we still see broken > behavior: > > (gdb) PASS: gdb.threads/fork-plus-threads.exp: set detach-on-fork off > continue & > Continuing. > (gdb) PASS: gdb.threads/fork-plus-threads.exp: continue & > [New Thread 28092.28092] > > [Thread 28092.28092] #2 stopped. > [New Thread 28094.28094] > [Inferior 2 (process 28092) exited normally] > [New Thread 28094.28105] > [New Thread 28094.28109] > > ... > > [Thread 28174.28174] #18 stopped. > [New Thread 28185.28185] > [Inferior 10 (process 28174) exited normally] > [New Thread 28185.28196] > > [Thread 28185.28185] #20 stopped. > Cannot remove breakpoints because program is no longer writable. > Further execution is probably impossible. > [Inferior 11 (process 28185) exited normally] > [Inferior 1 (process 28091) exited normally] > PASS: gdb.threads/fork-plus-threads.exp: reached breakpoint > info threads > No threads. > (gdb) PASS: gdb.threads/fork-plus-threads.exp: no threads left > info inferiors > Num Description Executable > * 1 <null> /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.threads/fork-plus-threads > (gdb) PASS: gdb.threads/fork-plus-threads.exp: only inferior 1 left > > All the "[Thread FOO] #NN stopped." above are bogus, as well as the > "Cannot remove breakpoints because program is no longer writable.", > which is a consequence. > > The problem is that when we intercept a fork event, we should report > the event for the parent, only, and leave the child stopped, but not > report its stop event. GDB later decides whether to follow the parent > or the child. But because handle_extended_wait does not set the > child's last_status.kind to TARGET_WAITKIND_STOPPED, a > stop_all_threads/unstop_all_lwps sequence (e.g., from trying to access > memory) by mistake ends up queueing a SIGSTOP on the child, resuming > it, and then when that SIGSTOP is intercepted, because the LWP has > last_resume_kind set to resume_stop, gdbserver reports the stop to > GDB, as GDB_SIGNAL_0: > > ... > >>>> entering unstop_all_lwps > unstopping all lwps > proceed_one_lwp: lwp 1600 > client wants LWP to remain 1600 stopped > proceed_one_lwp: lwp 1828 > Client wants LWP 1828 to stop. Making sure it has a SIGSTOP pending > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Sending sigstop to lwp 1828 > pc is 0x3615ebc7cc > Resuming lwp 1828 (continue, signal 0, stop expected) > continue from pc 0x3615ebc7cc > unstop_all_lwps done > sigchld_handler > <<<< exiting unstop_all_lwps > handling possible target event > >>>> entering linux_wait_1 > linux_wait_1: [<all threads>] > my_waitpid (-1, 0x40000001) > my_waitpid (-1, 0x1): status(137f), 1828 > LWFE: waitpid(-1, ...) returned 1828, ERRNO-OK > LLW: waitpid 1828 received Stopped (signal) (stopped) > pc is 0x3615ebc7cc > Expected stop. > LLW: resume_stop SIGSTOP caught for LWP 1828.1828. > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > ... > linux_wait_1 ret = LWP 1828.1828, 1, 0 > <<<< exiting linux_wait_1 > Writing resume reply for LWP 1828.1828:1 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > By inspection, I also noticed that we miss leaving the child with the > suspend count incremented if stopping threads, like we do for clone > threads. > > Tested on x86_64 Fedora 20, extended-remote. > > gdb/gdbserver/ChangeLog: > 2015-07-23 Pedro Alves <palves@redhat.com> > > * linux-low.c (handle_extended_wait): Set the child's last > reported status to TARGET_WAITKIND_STOPPED. > --- > gdb/gdbserver/linux-low.c | 7 ++++++ > gdb/testsuite/gdb.threads/fork-plus-threads.exp | 30 +++++++++++++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 17b2a51..56a33ff 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -488,6 +488,13 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) > child_lwp->status_pending_p = 0; > child_thr = get_lwp_thread (child_lwp); > child_thr->last_resume_kind = resume_stop; > + child_thr->last_status.kind = TARGET_WAITKIND_STOPPED; > + > + /* If we're suspending all threads, leave this one suspended > + too. */ > + if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS) > + child_lwp->suspended = 1; > + > parent_proc = get_thread_process (event_thr); > child_proc->attached = parent_proc->attached; > clone_all_breakpoints (&child_proc->breakpoints, > diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp > index f44dd76..80d2464 100644 > --- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp > +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp > @@ -48,13 +48,43 @@ gdb_test_multiple $test $test { > } > } > > +# gdbserver had a bug that resulted in reporting the fork child's > +# initial stop to gdb, which gdb does not expect, in turn resulting in > +# a broken session, like: > +# > +# [Thread 31536.31536] #16 stopped. <== BAD > +# [New Thread 31547.31547] > +# [Inferior 10 (process 31536) exited normally] > +# [New Thread 31547.31560] > +# > +# [Thread 31547.31547] #18 stopped. <== BAD > +# Cannot remove breakpoints because program is no longer writable. <== BAD > +# Further execution is probably impossible. <== BAD > +# [Inferior 11 (process 31547) exited normally] > +# [Inferior 1 (process 31454) exited normally] > +# > +# These variables track whether we see such broken behavior. > +set saw_cannot_remove_breakpoints 0 > +set saw_thread_stopped 0 > + > set test "reached breakpoint" > gdb_test_multiple "" $test { > + -re "Cannot remove breakpoints" { > + set saw_cannot_remove_breakpoints 1 > + exp_continue > + } > + -re "Thread \[^\r\n\]+ stopped\\." { > + set saw_thread_stopped 1 > + exp_continue > + } > -re "Inferior 1 \(\[^\r\n\]+\) exited normally" { > pass $test > } > } > > +gdb_assert !$saw_cannot_remove_breakpoints "no failure to remove breakpoints" > +gdb_assert !$saw_thread_stopped "no spurious thread stop" > + > gdb_test "info threads" "No threads\." \ > "no threads left" I tried it and it works as expected. If you try the same test program in all-stop though, fork childs are left stopped. Is it expected? I am not sure how forking interacts with all-stop. ----------------------- $ ./gdb -q -nx -ex "set detach-on-fork off" testsuite/gdb.threads/fork-plus-threads Reading symbols from testsuite/gdb.threads/fork-plus-threads...done. (gdb) r & Starting program: /home/emaisin/src/binutils-gdb/gdb/testsuite/gdb.threads/fork-plus-threads (gdb) [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5304] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5305] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5306] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5307] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5308] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5309] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5310] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5311] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5312] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New process 5313] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". i th Id Target Id Frame 11 process 5313 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 10 process 5312 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 9 process 5311 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 8 process 5310 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 7 process 5309 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 6 process 5308 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 5 process 5307 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 4 process 5306 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 3 process 5305 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 2 process 5304 "fork-plus-threa" 0x00007ffff78b8025 in fork () from /lib/x86_64-linux-gnu/libc.so.6 * 1 Thread 0x7ffff7fc9740 (LWP 5300) "fork-plus-threa" (running) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-24 18:05 ` Simon Marchi @ 2015-07-24 18:17 ` Pedro Alves 0 siblings, 0 replies; 16+ messages in thread From: Pedro Alves @ 2015-07-24 18:17 UTC (permalink / raw) To: Simon Marchi, Breazeal, Don; +Cc: GDB Patches On 07/24/2015 07:05 PM, Simon Marchi wrote: > I tried it and it works as expected. If you try the same test program in all-stop > though, fork childs are left stopped. Is it expected? I am not sure how forking > interacts with all-stop. Yeah, in all-stop, you need "set schedule-multiple on" to let all processes run. That seems to trip on more breakage: ... [Thread 0x7ffff57bc700 (LWP 11703) exited] [Thread 0x7ffff7fc1700 (LWP 11700) exited] [New Thread 0x7ffff77c0700 (LWP 11710)] [New Thread 0x7ffff67be700 (LWP 11709)] [New Thread 0x7ffff57bc700 (LWP 11711)] [New Thread 0x7ffff4fbb700 (LWP 11712)] [New Thread 0x7ffff3fb9700 (LWP 11713)] [New process 11702] [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". [Thread 0x7ffff3fb9700 (LWP 11713) exited] [Thread 0x7ffff4fbb700 (LWP 11712) exited] [Thread 0x7ffff57bc700 (LWP 11711) exited] [Thread 0x7ffff77c0700 (LWP 11710) exited] [Thread 0x7ffff67be700 (LWP 11709) exited] [New Thread 0x7ffff7fc1700 (LWP 11714)] [New Thread 0x7ffff6fbf700 (LWP 11716)] [New Thread 0x7ffff5fbd700 (LWP 11715)] [New Thread 0x7ffff37b8700 (LWP 11717)] [Inferior 3 (process 11634) exited normally] Cannot find new threads: generic error (gdb) info threads Cannot find new threads: generic error (gdb) info threads Cannot find new threads: generic error (gdb) q ISTR that's not a new bug, but I haven't tried older releases. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-23 18:21 ` [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode Pedro Alves 2015-07-24 18:05 ` Simon Marchi @ 2015-07-24 18:43 ` Don Breazeal 2015-07-29 13:21 ` Pedro Alves 1 sibling, 1 reply; 16+ messages in thread From: Don Breazeal @ 2015-07-24 18:43 UTC (permalink / raw) To: Pedro Alves, Breazeal, Don, Simon Marchi; +Cc: GDB Patches On 7/23/2015 11:21 AM, Pedro Alves wrote: > So I managed to extract out this smaller patch from the > gdbserver fixes I mentioned. I think this one looks safe enough > for 7.10. WDYT? > > ----------- > From 98d41152bff2a21f7fda864d87ee5dd0cffa2d17 Mon Sep 17 00:00:00 2001 > From: Pedro Alves <palves@redhat.com> > Date: Thu, 23 Jul 2015 18:49:51 +0100 > Subject: [PATCH] remote follow fork and spurious child stops in non-stop mode > > Running gdb.threads/fork-plus-threads.exp against gdbserver in > extended-remote mode, even though the test passes, we still see broken > behavior: > > Running gdb.threads/fork-plus-threads.exp against gdbserver in > extended-remote mode, even though the test passes, we still see broken > behavior: > > (gdb) PASS: gdb.threads/fork-plus-threads.exp: set detach-on-fork off > continue & > Continuing. > (gdb) PASS: gdb.threads/fork-plus-threads.exp: continue & > [New Thread 28092.28092] > > [Thread 28092.28092] #2 stopped. > [New Thread 28094.28094] > [Inferior 2 (process 28092) exited normally] > [New Thread 28094.28105] > [New Thread 28094.28109] > > ... > > [Thread 28174.28174] #18 stopped. > [New Thread 28185.28185] > [Inferior 10 (process 28174) exited normally] > [New Thread 28185.28196] > > [Thread 28185.28185] #20 stopped. > Cannot remove breakpoints because program is no longer writable. > Further execution is probably impossible. > [Inferior 11 (process 28185) exited normally] > [Inferior 1 (process 28091) exited normally] > PASS: gdb.threads/fork-plus-threads.exp: reached breakpoint > info threads > No threads. > (gdb) PASS: gdb.threads/fork-plus-threads.exp: no threads left > info inferiors > Num Description Executable > * 1 <null> /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.threads/fork-plus-threads > (gdb) PASS: gdb.threads/fork-plus-threads.exp: only inferior 1 left > > All the "[Thread FOO] #NN stopped." above are bogus, as well as the > "Cannot remove breakpoints because program is no longer writable.", > which is a consequence. > > The problem is that when we intercept a fork event, we should report > the event for the parent, only, and leave the child stopped, but not > report its stop event. GDB later decides whether to follow the parent > or the child. But because handle_extended_wait does not set the > child's last_status.kind to TARGET_WAITKIND_STOPPED, a > stop_all_threads/unstop_all_lwps sequence (e.g., from trying to access > memory) by mistake ends up queueing a SIGSTOP on the child, resuming > it, and then when that SIGSTOP is intercepted, because the LWP has > last_resume_kind set to resume_stop, gdbserver reports the stop to > GDB, as GDB_SIGNAL_0: > > ... > >>>> entering unstop_all_lwps > unstopping all lwps > proceed_one_lwp: lwp 1600 > client wants LWP to remain 1600 stopped > proceed_one_lwp: lwp 1828 > Client wants LWP 1828 to stop. Making sure it has a SIGSTOP pending > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Sending sigstop to lwp 1828 > pc is 0x3615ebc7cc > Resuming lwp 1828 (continue, signal 0, stop expected) > continue from pc 0x3615ebc7cc > unstop_all_lwps done > sigchld_handler > <<<< exiting unstop_all_lwps > handling possible target event > >>>> entering linux_wait_1 > linux_wait_1: [<all threads>] > my_waitpid (-1, 0x40000001) > my_waitpid (-1, 0x1): status(137f), 1828 > LWFE: waitpid(-1, ...) returned 1828, ERRNO-OK > LLW: waitpid 1828 received Stopped (signal) (stopped) > pc is 0x3615ebc7cc > Expected stop. > LLW: resume_stop SIGSTOP caught for LWP 1828.1828. > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > ... > linux_wait_1 ret = LWP 1828.1828, 1, 0 > <<<< exiting linux_wait_1 > Writing resume reply for LWP 1828.1828:1 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > By inspection, I also noticed that we miss leaving the child with the > suspend count incremented if stopping threads, like we do for clone > threads. > > Tested on x86_64 Fedora 20, extended-remote. > > gdb/gdbserver/ChangeLog: > 2015-07-23 Pedro Alves <palves@redhat.com> > > * linux-low.c (handle_extended_wait): Set the child's last > reported status to TARGET_WAITKIND_STOPPED. > --- > gdb/gdbserver/linux-low.c | 7 ++++++ > gdb/testsuite/gdb.threads/fork-plus-threads.exp | 30 +++++++++++++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 17b2a51..56a33ff 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -488,6 +488,13 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) > child_lwp->status_pending_p = 0; > child_thr = get_lwp_thread (child_lwp); > child_thr->last_resume_kind = resume_stop; > + child_thr->last_status.kind = TARGET_WAITKIND_STOPPED; This makes perfect sense to me. > + > + /* If we're suspending all threads, leave this one suspended > + too. */ > + if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS) > + child_lwp->suspended = 1; I have a question about this. In the definition of struct lwp_info in linux-low.h, it has this comment: /* When this is true, we shall not try to resume this thread, even if last_resume_kind isn't resume_stop. */ int suspended; Since we are setting last_resume_kind to resume_stop here, is this unnecessary? Thanks, --Don > + > parent_proc = get_thread_process (event_thr); > child_proc->attached = parent_proc->attached; > clone_all_breakpoints (&child_proc->breakpoints, > diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp > index f44dd76..80d2464 100644 > --- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp > +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp > @@ -48,13 +48,43 @@ gdb_test_multiple $test $test { > } > } > > +# gdbserver had a bug that resulted in reporting the fork child's > +# initial stop to gdb, which gdb does not expect, in turn resulting in > +# a broken session, like: > +# > +# [Thread 31536.31536] #16 stopped. <== BAD > +# [New Thread 31547.31547] > +# [Inferior 10 (process 31536) exited normally] > +# [New Thread 31547.31560] > +# > +# [Thread 31547.31547] #18 stopped. <== BAD > +# Cannot remove breakpoints because program is no longer writable. <== BAD > +# Further execution is probably impossible. <== BAD > +# [Inferior 11 (process 31547) exited normally] > +# [Inferior 1 (process 31454) exited normally] > +# > +# These variables track whether we see such broken behavior. > +set saw_cannot_remove_breakpoints 0 > +set saw_thread_stopped 0 > + > set test "reached breakpoint" > gdb_test_multiple "" $test { > + -re "Cannot remove breakpoints" { > + set saw_cannot_remove_breakpoints 1 > + exp_continue > + } > + -re "Thread \[^\r\n\]+ stopped\\." { > + set saw_thread_stopped 1 > + exp_continue > + } > -re "Inferior 1 \(\[^\r\n\]+\) exited normally" { > pass $test > } > } > > +gdb_assert !$saw_cannot_remove_breakpoints "no failure to remove breakpoints" > +gdb_assert !$saw_thread_stopped "no spurious thread stop" > + > gdb_test "info threads" "No threads\." \ > "no threads left" > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-24 18:43 ` Don Breazeal @ 2015-07-29 13:21 ` Pedro Alves 2015-07-29 13:38 ` Pedro Alves 2015-07-30 18:13 ` Pedro Alves 0 siblings, 2 replies; 16+ messages in thread From: Pedro Alves @ 2015-07-29 13:21 UTC (permalink / raw) To: Don Breazeal, Breazeal, Don, Simon Marchi; +Cc: GDB Patches [-- Attachment #1: Type: text/plain, Size: 4061 bytes --] Hi Don, Sorry for the delay. On 07/24/2015 07:43 PM, Don Breazeal wrote: >> index 17b2a51..56a33ff 100644 >> --- a/gdb/gdbserver/linux-low.c >> +++ b/gdb/gdbserver/linux-low.c >> @@ -488,6 +488,13 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) >> child_lwp->status_pending_p = 0; >> child_thr = get_lwp_thread (child_lwp); >> child_thr->last_resume_kind = resume_stop; >> + child_thr->last_status.kind = TARGET_WAITKIND_STOPPED; > > This makes perfect sense to me. > Great. >> + >> + /* If we're suspending all threads, leave this one suspended >> + too. */ >> + if (stopping_threads == STOPPING_AND_SUSPENDING_THREADS) >> + child_lwp->suspended = 1; > > I have a question about this. In the definition of struct lwp_info in > linux-low.h, it has this comment: > > /* When this is true, we shall not try to resume this thread, even > if last_resume_kind isn't resume_stop. */ > int suspended; > > Since we are setting last_resume_kind to resume_stop here, is this > unnecessary? We still need it, because otherwise we'd decrement the suspend count below 0: static int unsuspend_and_proceed_one_lwp (struct inferior_list_entry *entry, void *except) { struct thread_info *thread = (struct thread_info *) entry; struct lwp_info *lwp = get_thread_lwp (thread); if (lwp == except) return 0; lwp->suspended--; gdb_assert (lwp->suspended >= 0); return proceed_one_lwp (entry, except); } It's proceed_one_lwp that skips resuming if the client wants the lwp stopped: static int proceed_one_lwp (struct inferior_list_entry *entry, void *except) { ... if (thread->last_resume_kind == resume_stop && thread->last_status.kind != TARGET_WAITKIND_IGNORE) { if (debug_threads) debug_printf (" client wants LWP to remain %ld stopped\n", lwpid_of (thread)); return 0; } I tried writing a test for this, by making a multithreaded program have all its threads but the main continuously fork (see attached), while the main thread continuously steps over a breakpoint (a conditional breakpoint with condition "0" should do it, as gdbserver handles that breakpoint itself), but that stumbles on yet more problems... :-/ $ ./gdb ./testsuite/gdb.threads/fork-plus-threads-2 -ex "set non-stop on" -ex "set detach-on-fork off" -ex "tar extended-rem :9999" ... Remote debugging using :9999 (gdb) [Thread 24971.24971] #1 stopped. 0x0000003615a011f0 in ?? () c& Continuing. (gdb) [New Thread 24971.24981] [New Thread 24983.24983] [New Thread 24971.24982] [Thread 24983.24983] #3 stopped. 0x0000003615ebc7cc in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:130 130 pid = ARCH_FORK (); [New Thread 24984.24984] Error in re-setting breakpoint -16: PC register is not available Error in re-setting breakpoint -17: PC register is not available Error in re-setting breakpoint -18: PC register is not available Error in re-setting breakpoint -19: PC register is not available Error in re-setting breakpoint -24: PC register is not available Error in re-setting breakpoint -25: PC register is not available Error in re-setting breakpoint -26: PC register is not available Error in re-setting breakpoint -27: PC register is not available Error in re-setting breakpoint -28: PC register is not available Error in re-setting breakpoint -29: PC register is not available Error in re-setting breakpoint -30: PC register is not available PC register is not available (gdb) >> set test "reached breakpoint" BTW, I noticed that this test message is stale from my previous attempt at running to a breakpoint instead of to exit. I changed it to: set test "inferior 1 exited" in patch 1/2. >> gdb_test_multiple "" $test { >> + -re "Cannot remove breakpoints" { >> + set saw_cannot_remove_breakpoints 1 >> + exp_continue >> + } >> + -re "Thread \[^\r\n\]+ stopped\\." { >> + set saw_thread_stopped 1 >> + exp_continue >> + } >> -re "Inferior 1 \(\[^\r\n\]+\) exited normally" { >> pass $test >> } >> } Thanks, Pedro Alves [-- Attachment #2: fork-plus-threads-2.c --] [-- Type: text/plain, Size: 2029 bytes --] /* This testcase is part of GDB, the GNU debugger. Copyright 2015 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/>. */ #include <assert.h> #include <pthread.h> #include <unistd.h> #include <stdio.h> #include <sys/types.h> #include <sys/wait.h> #include <stdlib.h> /* Number of threads. Each thread continuously spawns a fork and wait for it. If we have another thread continuously start a step over, gdbserver should end up finding new forks while suspending threads. */ #define NTHREADS 10 pthread_t threads[NTHREADS]; static void * thread_func (void *arg) { while (1) { pid_t pid; pid = fork (); if (pid > 0) { int status; /* Parent. */ pid = waitpid (pid, &status, 0); if (pid == -1) { perror ("wait"); exit (1); } if (!WIFEXITED (status)) { printf ("Unexpected wait status 0x%x from child %d\n", status, pid); } } else if (pid == 0) { /* Child. */ exit (0); } else { perror ("fork"); exit (1); } } } int main (void) { int i; int ret; for (i = 0; i < NTHREADS; i++) { ret = pthread_create (&threads[i], NULL, thread_func, NULL); assert (ret == 0); } for (i = 0; i < NTHREADS; i++) { ret = pthread_join (threads[i], NULL); assert (ret == 0); } /* Don't run forever. */ sleep (180); return 0; } ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-29 13:21 ` Pedro Alves @ 2015-07-29 13:38 ` Pedro Alves 2015-07-29 14:23 ` Pedro Alves 2015-07-30 18:13 ` Pedro Alves 1 sibling, 1 reply; 16+ messages in thread From: Pedro Alves @ 2015-07-29 13:38 UTC (permalink / raw) To: Don Breazeal, Breazeal, Don, Simon Marchi; +Cc: GDB Patches On 07/29/2015 02:21 PM, Pedro Alves wrote: > > I tried writing a test for this, by making a multithreaded program > have all its threads but the main continuously fork (see attached), while > the main thread continuously steps over a breakpoint (a conditional > breakpoint with condition "0" should do it, as gdbserver handles > that breakpoint itself), but that stumbles on yet more problems... :-/ > > $ ./gdb ./testsuite/gdb.threads/fork-plus-threads-2 -ex "set non-stop on" -ex "set detach-on-fork off" -ex "tar extended-rem :9999" > ... > Remote debugging using :9999 > (gdb) > [Thread 24971.24971] #1 stopped. > 0x0000003615a011f0 in ?? () > c& > Continuing. > (gdb) [New Thread 24971.24981] > [New Thread 24983.24983] > [New Thread 24971.24982] > > [Thread 24983.24983] #3 stopped. > 0x0000003615ebc7cc in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:130 > 130 pid = ARCH_FORK (); > [New Thread 24984.24984] > Error in re-setting breakpoint -16: PC register is not available > Error in re-setting breakpoint -17: PC register is not available > Error in re-setting breakpoint -18: PC register is not available > Error in re-setting breakpoint -19: PC register is not available > Error in re-setting breakpoint -24: PC register is not available > Error in re-setting breakpoint -25: PC register is not available > Error in re-setting breakpoint -26: PC register is not available > Error in re-setting breakpoint -27: PC register is not available > Error in re-setting breakpoint -28: PC register is not available > Error in re-setting breakpoint -29: PC register is not available > Error in re-setting breakpoint -30: PC register is not available > PC register is not available > (gdb) > Hmm, gdbserver's logs (for a different run) show: ... HEW: Got clone event from LWP 25962, new child is LWP 25989 my_waitpid (-1, 0x40000001) my_waitpid (-1, 0x1): status(137f), 25990 LWFE: waitpid(-1, ...) returned 25990, ERRNO-OK LLW: waitpid 25990 received Stopped (signal) (stopped) my_waitpid (-1, 0x40000001) my_waitpid (-1, 0x1): status(0), 25988 LWFE: waitpid(-1, ...) returned 25988, ERRNO-OK LLW: waitpid 25988 received 0 (exited) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LLFE: 25988 exited. ^^^^^^^^^^^^^^^^^^^ my_waitpid (-1, 0x40000001) my_waitpid (-1, 0x80000001): status(1057f), 25973 LWFE: waitpid(-1, ...) returned 25973, ERRNO-OK LLW: waitpid 25973 received Trace/breakpoint trap (stopped) pc is 0x3615ebc7cc HEW: Got fork event from LWP 25973, new child is 25990 pc is 0x3615ebc7cc pc is 0x3615ebc7cc my_waitpid (-1, 0x40000001) my_waitpid (-1, 0x80000001): status(117f), 25972 LWFE: waitpid(-1, ...) returned 25972, ERRNO-OK LLW: waitpid 25972 received Child exited (stopped) pc is 0x3616a0f279 my_waitpid (-1, 0x40000001) my_waitpid (-1, 0x80000001): status(117f), 0 LWFE: waitpid(-1, ...) returned 0, ERRNO-OK RSRL: resuming stopped-resumed LWP LWP 25962.25962 at 3615ef4ce1: step=0 pc is 0x3615ef4ce1 Resuming lwp 25962 (continue, signal 0, stop not expected) continue from pc 0x3615ef4ce1 RSRL: resuming stopped-resumed LWP LWP 25962.25989 at 0: step=0 pc is 0x3615ef4ce1 Resuming lwp 25989 (continue, signal 0, stop not expected) continue from pc 0x3615ef4ce1 sigchld_handler Ignored signal 17 for LWP 25972. pc is 0x3616a0f279 Resuming lwp 25972 (continue, signal 17, stop not expected) continue from pc 0x3616a0f279 handling possible target event >>>> entering linux_wait_1 linux_wait_1: [<all threads>] Got a pending child 25973 Got an event from pending child 25973 (1057f) Hit a non-gdbserver trap event. SEL: Found 2 SIGTRAP events, selecting #1 linux_wait_1 ret = LWP 25988.25988, 1, 0 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ("1" is TARGET_WAITKIND_STOPPED) <<<< exiting linux_wait_1 Writing resume reply for LWP 25988.25988:1 ptrace(regsets_fetch_inferior_registers) PID=25988: No such process ptrace(regsets_fetch_inferior_registers) PID=25988: No such process ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ignore the "SIGTRAP" mention in "SEL: Found 2 SIGTRAP events", it's "two events". And the one that was picked was a process exit. But the tail end of linux_wait_1 isn't expecting that can happen. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-29 13:38 ` Pedro Alves @ 2015-07-29 14:23 ` Pedro Alves 2015-07-29 15:40 ` Pedro Alves 0 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2015-07-29 14:23 UTC (permalink / raw) To: Don Breazeal, Breazeal, Don, Simon Marchi; +Cc: GDB Patches On 07/29/2015 02:38 PM, Pedro Alves wrote: > Ignore the "SIGTRAP" mention in "SEL: Found 2 SIGTRAP events", > it's "two events". And the one that was picked was a process > exit. But the tail end of linux_wait_1 isn't expecting that > can happen. > This seems to fix it (and ends up making the code a little more like linux-nat.c). I'm running it through the testsuite. -------- From: Pedro Alves <palves@redhat.com> Date: 2015-07-29 14:40:04 +0100 fix --- gdb/gdbserver/linux-low.c | 67 ++++++++++++++++++++++++++------------------- gdb/gdbserver/linux-low.h | 11 +++---- 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 9eaa912..99a44f9 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -264,6 +264,7 @@ static int linux_wait_for_event (ptid_t ptid, int *wstat, int options); static struct lwp_info *add_lwp (ptid_t ptid); static int linux_stopped_by_watchpoint (void); static void mark_lwp_dead (struct lwp_info *lwp, int wstat); +static int lwp_is_marked_dead (struct lwp_info *lwp); static void proceed_all_lwps (void); static int finish_step_over (struct lwp_info *lwp); static int kill_lwp (unsigned long lwpid, int signo); @@ -755,9 +756,9 @@ add_lwp (ptid_t ptid) { struct lwp_info *lwp; - lwp = (struct lwp_info *) xmalloc (sizeof (*lwp)); - memset (lwp, 0, sizeof (*lwp)); + lwp = (struct lwp_info *) xcalloc (1, sizeof (*lwp)); + lwp->waitstatus.kind = TARGET_WAITKIND_IGNORE; if (the_low_target.new_thread != NULL) the_low_target.new_thread (lwp); @@ -1397,7 +1398,7 @@ linux_thread_alive (ptid_t ptid) exited but we still haven't been able to report it to GDB, we'll hold on to the last lwp of the dead process. */ if (lwp != NULL) - return !lwp->dead; + return !lwp_is_marked_dead (lwp); else return 0; } @@ -2741,20 +2742,6 @@ ignore_event (struct target_waitstatus *ourstatus) return null_ptid; } -/* Return non-zero if WAITSTATUS reflects an extended linux - event. Otherwise, return zero. */ - -static int -extended_event_reported (const struct target_waitstatus *waitstatus) -{ - if (waitstatus == NULL) - return 0; - - return (waitstatus->kind == TARGET_WAITKIND_FORKED - || waitstatus->kind == TARGET_WAITKIND_VFORKED - || waitstatus->kind == TARGET_WAITKIND_VFORK_DONE); -} - /* Wait for process, returns status. */ static ptid_t @@ -3122,7 +3109,7 @@ linux_wait_1 (ptid_t ptid, || (gdb_breakpoint_here (event_child->stop_pc) && gdb_condition_true_at_breakpoint (event_child->stop_pc) && gdb_no_commands_at_breakpoint (event_child->stop_pc)) - || extended_event_reported (&event_child->waitstatus)); + || event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE); run_breakpoint_commands (event_child->stop_pc); @@ -3144,9 +3131,11 @@ linux_wait_1 (ptid_t ptid, paddress (event_child->stop_pc), paddress (event_child->step_range_start), paddress (event_child->step_range_end)); - if (extended_event_reported (&event_child->waitstatus)) + if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE) { - char *str = target_waitstatus_to_string (ourstatus); + char *str; + + str = target_waitstatus_to_string (&event_child->waitstatus); debug_printf ("LWP %ld: extended event with waitstatus %s\n", lwpid_of (get_lwp_thread (event_child)), str); xfree (str); @@ -3260,12 +3249,11 @@ linux_wait_1 (ptid_t ptid, unstop_all_lwps (1, event_child); } - if (extended_event_reported (&event_child->waitstatus)) + if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE) { - /* If the reported event is a fork, vfork or exec, let GDB know. */ - ourstatus->kind = event_child->waitstatus.kind; - ourstatus->value = event_child->waitstatus.value; - + /* If the reported event is an exit, fork, vfork or exec, let + GDB know. */ + *ourstatus = event_child->waitstatus; /* Clear the event lwp's waitstatus since we handled it already. */ event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE; } @@ -3473,13 +3461,23 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry, static void mark_lwp_dead (struct lwp_info *lwp, int wstat) { - /* It's dead, really. */ - lwp->dead = 1; - /* Store the exit status for later. */ lwp->status_pending_p = 1; lwp->status_pending = wstat; + /* Store in waitstatus as well, as there's nothing else to process + for this event. */ + if (WIFEXITED (wstat)) + { + lwp->waitstatus.kind = TARGET_WAITKIND_EXITED; + lwp->waitstatus.value.integer = WEXITSTATUS (wstat); + } + else if (WIFSIGNALED (wstat)) + { + lwp->waitstatus.kind = TARGET_WAITKIND_SIGNALLED; + lwp->waitstatus.value.sig = gdb_signal_from_host (WTERMSIG (wstat)); + } + /* Prevent trying to stop it. */ lwp->stopped = 1; @@ -3487,6 +3485,17 @@ mark_lwp_dead (struct lwp_info *lwp, int wstat) lwp->stop_expected = 0; } +/* Return true if LWP has exited already, and has a pending exit event + to report to GDB. */ + +static int +lwp_is_marked_dead (struct lwp_info *lwp) +{ + return (lwp->status_pending_p + && (WIFEXITED (lwp->status_pending) + || WIFSIGNALED (lwp->status_pending))); +} + /* Wait for all children to stop for the SIGSTOPs we just queued. */ static void @@ -3603,7 +3612,7 @@ lwp_running (struct inferior_list_entry *entry, void *data) struct thread_info *thread = (struct thread_info *) entry; struct lwp_info *lwp = get_thread_lwp (thread); - if (lwp->dead) + if (lwp_is_marked_dead (lwp)) return 0; if (lwp->stopped) return 0; diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h index 3300da9..0f2421b 100644 --- a/gdb/gdbserver/linux-low.h +++ b/gdb/gdbserver/linux-low.h @@ -266,16 +266,13 @@ struct lwp_info event already received in a wait()). */ int stopped; - /* If this flag is set, the lwp is known to be dead already (exit - event already received in a wait(), and is cached in - status_pending). */ - int dead; - /* When stopped is set, the last wait status recorded for this lwp. */ int last_status; - /* This is used to store extended ptrace event information until - it is reported to GDB. */ + /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for + this LWP's last event, to pass to GDB without any further + processing. This is used to store extended ptrace event + information or exit status until it can be reported to GDB. */ struct target_waitstatus waitstatus; /* When stopped is set, this is where the lwp last stopped, with ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-29 14:23 ` Pedro Alves @ 2015-07-29 15:40 ` Pedro Alves 2015-07-29 16:40 ` Pedro Alves 0 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2015-07-29 15:40 UTC (permalink / raw) To: Don Breazeal, Breazeal, Don, Simon Marchi; +Cc: GDB Patches On 07/29/2015 03:23 PM, Pedro Alves wrote: > On 07/29/2015 02:38 PM, Pedro Alves wrote: > >> Ignore the "SIGTRAP" mention in "SEL: Found 2 SIGTRAP events", >> it's "two events". And the one that was picked was a process >> exit. But the tail end of linux_wait_1 isn't expecting that >> can happen. >> > > This seems to fix it (and ends up making the code a little > more like linux-nat.c). I'm running it through the testsuite. It passes cleanly. And I confirmed that that test idea triggers the suspend count assertion I suspected: Child exited with status 0 /home/pedro/gdb/mygit/build/../src/gdb/gdbserver/linux-low.c:2619: A problem internal to GDBserver has been detected. unsuspend_one_lwp: Assertion `lwp->suspended >= 0' failed. [Inferior 1 (process 32473) exited with code 01] (gdb) I'll convert that to a proper test. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-29 15:40 ` Pedro Alves @ 2015-07-29 16:40 ` Pedro Alves 0 siblings, 0 replies; 16+ messages in thread From: Pedro Alves @ 2015-07-29 16:40 UTC (permalink / raw) To: Don Breazeal, Breazeal, Don, Simon Marchi; +Cc: GDB Patches On 07/29/2015 04:40 PM, Pedro Alves wrote: > On 07/29/2015 03:23 PM, Pedro Alves wrote: >> On 07/29/2015 02:38 PM, Pedro Alves wrote: >> >>> Ignore the "SIGTRAP" mention in "SEL: Found 2 SIGTRAP events", >>> it's "two events". And the one that was picked was a process >>> exit. But the tail end of linux_wait_1 isn't expecting that >>> can happen. >>> >> >> This seems to fix it (and ends up making the code a little >> more like linux-nat.c). I'm running it through the testsuite. > > It passes cleanly. > > And I confirmed that that test idea triggers the suspend count > assertion I suspected: > > Child exited with status 0 > /home/pedro/gdb/mygit/build/../src/gdb/gdbserver/linux-low.c:2619: A problem internal to GDBserver has been detected. > unsuspend_one_lwp: Assertion `lwp->suspended >= 0' failed. > [Inferior 1 (process 32473) exited with code 01] > (gdb) > > I'll convert that to a proper test. > And that that exposes more issues, like: [New Thread 27183.27382] [New Thread 27183.27684] /home/pedro/gdb/mygit/build/../src/gdb/thread.c:936: internal-error: finish_thread_state: Assertion `tp' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) FAIL: gdb.threads/fork-plus-threads-2.exp: inferior 1 exited (GDB internal error) Resyncing due to internal error. n ... Detaching from process 28486 Detaching from process 28487 Detaching from process 28488 /home/pedro/gdb/mygit/build/../src/gdb/gdbserver/linux-low.c:3569: A problem internal to GDBserver has been detected. stuck_in_jump_pad_callback: Assertion `lwp->suspended == 0' failed. testcase /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.threads/fork-plus-threads-2.exp completed in 3 seconds === gdb Summary === Looks like I found myself a nice deep rat hole... Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-29 13:21 ` Pedro Alves 2015-07-29 13:38 ` Pedro Alves @ 2015-07-30 18:13 ` Pedro Alves 2015-07-30 18:15 ` Simon Marchi 1 sibling, 1 reply; 16+ messages in thread From: Pedro Alves @ 2015-07-30 18:13 UTC (permalink / raw) To: Don Breazeal, Breazeal, Don, Simon Marchi; +Cc: GDB Patches On 07/29/2015 02:21 PM, Pedro Alves wrote: >> > I have a question about this. In the definition of struct lwp_info in >> > linux-low.h, it has this comment: >> > >> > /* When this is true, we shall not try to resume this thread, even >> > if last_resume_kind isn't resume_stop. */ >> > int suspended; >> > >> > Since we are setting last_resume_kind to resume_stop here, is this >> > unnecessary? > We still need it, because otherwise we'd decrement the suspend count > below 0: I think that today I fixed most of the issues this uncovered that I showed yesterday, but there's still more to do, so I removed that bit from the patch, and pushed it, to both master and 7.10, in order to close PR18600, and remove it from the 7.10 blockers list. Not sure yet whether the other new fixes will make it into 7.10. Below's what I pushed. ----------- From 998d452ac81bc240996c967dd27f7b747240cd66 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Thu, 30 Jul 2015 18:41:44 +0100 Subject: [PATCH 3/3] remote follow fork and spurious child stops in non-stop mode Running gdb.threads/fork-plus-threads.exp against gdbserver in extended-remote mode, even though the test passes, we still see broken behavior: (gdb) PASS: gdb.threads/fork-plus-threads.exp: set detach-on-fork off continue & Continuing. (gdb) PASS: gdb.threads/fork-plus-threads.exp: continue & [New Thread 28092.28092] [Thread 28092.28092] #2 stopped. [New Thread 28094.28094] [Inferior 2 (process 28092) exited normally] [New Thread 28094.28105] [New Thread 28094.28109] ... [Thread 28174.28174] #18 stopped. [New Thread 28185.28185] [Inferior 10 (process 28174) exited normally] [New Thread 28185.28196] [Thread 28185.28185] #20 stopped. Cannot remove breakpoints because program is no longer writable. Further execution is probably impossible. [Inferior 11 (process 28185) exited normally] [Inferior 1 (process 28091) exited normally] PASS: gdb.threads/fork-plus-threads.exp: reached breakpoint info threads No threads. (gdb) PASS: gdb.threads/fork-plus-threads.exp: no threads left info inferiors Num Description Executable * 1 <null> /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.threads/fork-plus-threads (gdb) PASS: gdb.threads/fork-plus-threads.exp: only inferior 1 left All the "[Thread FOO] #NN stopped." above are bogus, as well as the "Cannot remove breakpoints because program is no longer writable.", which is a consequence. The problem is that when we intercept a fork event, we should report the event for the parent, only, and leave the child stopped, but not report its stop event. GDB later decides whether to follow the parent or the child. But because handle_extended_wait does not set the child's last_status.kind to TARGET_WAITKIND_STOPPED, a stop_all_threads/unstop_all_lwps sequence (e.g., from trying to access memory) by mistake ends up queueing a SIGSTOP on the child, resuming it, and then when that SIGSTOP is intercepted, because the LWP has last_resume_kind set to resume_stop, gdbserver reports the stop to GDB, as GDB_SIGNAL_0: ... >>>> entering unstop_all_lwps unstopping all lwps proceed_one_lwp: lwp 1600 client wants LWP to remain 1600 stopped proceed_one_lwp: lwp 1828 Client wants LWP 1828 to stop. Making sure it has a SIGSTOP pending ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sending sigstop to lwp 1828 pc is 0x3615ebc7cc Resuming lwp 1828 (continue, signal 0, stop expected) continue from pc 0x3615ebc7cc unstop_all_lwps done sigchld_handler <<<< exiting unstop_all_lwps handling possible target event >>>> entering linux_wait_1 linux_wait_1: [<all threads>] my_waitpid (-1, 0x40000001) my_waitpid (-1, 0x1): status(137f), 1828 LWFE: waitpid(-1, ...) returned 1828, ERRNO-OK LLW: waitpid 1828 received Stopped (signal) (stopped) pc is 0x3615ebc7cc Expected stop. LLW: resume_stop SIGSTOP caught for LWP 1828.1828. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... linux_wait_1 ret = LWP 1828.1828, 1, 0 <<<< exiting linux_wait_1 Writing resume reply for LWP 1828.1828:1 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Tested on x86_64 Fedora 20, extended-remote. gdb/gdbserver/ChangeLog: 2015-07-30 Pedro Alves <palves@redhat.com> * linux-low.c (handle_extended_wait): Set the child's last reported status to TARGET_WAITKIND_STOPPED. --- gdb/testsuite/ChangeLog | 5 ++++ gdb/gdbserver/linux-low.c | 2 ++ gdb/testsuite/gdb.threads/fork-plus-threads.exp | 32 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index e3126ed..eda6625 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2015-07-30 Pedro Alves <palves@redhat.com> + * linux-low.c (handle_extended_wait): Set the child's last + reported status to TARGET_WAITKIND_STOPPED. + +2015-07-30 Pedro Alves <palves@redhat.com> + PR threads/18600 * gdb.threads/fork-plus-threads.exp: Test that "info inferiors" only shows inferior 1. diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 9bc9fa3..82fb7f9 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -483,6 +483,8 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) child_lwp->status_pending_p = 0; child_thr = get_lwp_thread (child_lwp); child_thr->last_resume_kind = resume_stop; + child_thr->last_status.kind = TARGET_WAITKIND_STOPPED; + parent_proc = get_thread_process (event_thr); child_proc->attached = parent_proc->attached; clone_all_breakpoints (&child_proc->breakpoints, diff --git a/gdb/testsuite/gdb.threads/fork-plus-threads.exp b/gdb/testsuite/gdb.threads/fork-plus-threads.exp index 8a503ec..2b34b6c 100644 --- a/gdb/testsuite/gdb.threads/fork-plus-threads.exp +++ b/gdb/testsuite/gdb.threads/fork-plus-threads.exp @@ -51,13 +51,45 @@ proc do_test { detach_on_fork } { } } + # gdbserver had a bug that resulted in reporting the fork child's + # initial stop to gdb, which gdb does not expect, in turn + # resulting in a broken session, like: + # + # [Thread 31536.31536] #16 stopped. <== BAD + # [New Thread 31547.31547] + # [Inferior 10 (process 31536) exited normally] + # [New Thread 31547.31560] + # + # [Thread 31547.31547] #18 stopped. <== BAD + # Cannot remove breakpoints because program is no longer writable. <== BAD + # Further execution is probably impossible. <== BAD + # [Inferior 11 (process 31547) exited normally] + # [Inferior 1 (process 31454) exited normally] + # + # These variables track whether we see such broken behavior. + set saw_cannot_remove_breakpoints 0 + set saw_thread_stopped 0 + set test "inferior 1 exited" gdb_test_multiple "" $test { + -re "Cannot remove breakpoints" { + set saw_cannot_remove_breakpoints 1 + exp_continue + } + -re "Thread \[^\r\n\]+ stopped\\." { + set saw_thread_stopped 1 + exp_continue + } -re "Inferior 1 \(\[^\r\n\]+\) exited normally" { pass $test } } + gdb_assert !$saw_cannot_remove_breakpoints \ + "no failure to remove breakpoints" + gdb_assert !$saw_thread_stopped \ + "no spurious thread stop" + gdb_test "info threads" "No threads\." \ "no threads left" -- 1.9.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-30 18:13 ` Pedro Alves @ 2015-07-30 18:15 ` Simon Marchi 2015-07-30 21:06 ` Don Breazeal 0 siblings, 1 reply; 16+ messages in thread From: Simon Marchi @ 2015-07-30 18:15 UTC (permalink / raw) To: Pedro Alves, Don Breazeal, Breazeal, Don; +Cc: GDB Patches On 15-07-30 02:13 PM, Pedro Alves wrote: > I think that today I fixed most of the issues this uncovered that I showed > yesterday, but there's still more to do, so I removed that bit from the > patch, and pushed it, to both master and 7.10, in order to close PR18600, > and remove it from the 7.10 blockers list. Not sure yet whether the > other new fixes will make it into 7.10. Thanks a lot for looking into this! ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode 2015-07-30 18:15 ` Simon Marchi @ 2015-07-30 21:06 ` Don Breazeal 0 siblings, 0 replies; 16+ messages in thread From: Don Breazeal @ 2015-07-30 21:06 UTC (permalink / raw) To: Simon Marchi, Pedro Alves; +Cc: GDB Patches On 7/30/2015 11:15 AM, Simon Marchi wrote: > On 15-07-30 02:13 PM, Pedro Alves wrote: >> I think that today I fixed most of the issues this uncovered that I showed >> yesterday, but there's still more to do, so I removed that bit from the >> patch, and pushed it, to both master and 7.10, in order to close PR18600, >> and remove it from the 7.10 blockers list. Not sure yet whether the >> other new fixes will make it into 7.10. > > Thanks a lot for looking into this! > Yes, thank you for cleaning this up, Pedro! --Don ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-07-30 21:06 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-07-23 17:24 [PATCH v2 0/2] PR threads/18600: Threads left stopped after fork+thread spawn Pedro Alves 2015-07-23 17:25 ` [PATCH v2 1/2] " Pedro Alves 2015-07-30 18:08 ` [pushed] " Pedro Alves 2015-07-23 17:25 ` [PATCH v2 2/2] PR threads/18600: Inferiors left around " Pedro Alves 2015-07-23 18:21 ` [PATCH 3/N] remote follow fork and spurious child stops in non-stop mode Pedro Alves 2015-07-24 18:05 ` Simon Marchi 2015-07-24 18:17 ` Pedro Alves 2015-07-24 18:43 ` Don Breazeal 2015-07-29 13:21 ` Pedro Alves 2015-07-29 13:38 ` Pedro Alves 2015-07-29 14:23 ` Pedro Alves 2015-07-29 15:40 ` Pedro Alves 2015-07-29 16:40 ` Pedro Alves 2015-07-30 18:13 ` Pedro Alves 2015-07-30 18:15 ` Simon Marchi 2015-07-30 21:06 ` Don Breazeal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).