* [PATCH 0/3] Extended-remote follow-fork cleanups
@ 2015-05-22 18:55 Don Breazeal
2015-05-22 18:55 ` [PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Don Breazeal @ 2015-05-22 18:55 UTC (permalink / raw)
To: gdb-patches
This patchset addresses a few extended-remote test failures discovered
by the buildbot and noted by Sergio in this email thread:
https://sourceware.org/ml/gdb-patches/2015-05/msg00372.html
Several of the failures were expected. These were in
gdb.base/foll-vfork.exp and depended on exec events, which are not yet
supported for extended-remote.
I had mistakenly concluded that a couple of other failures were due
to the lack of exec event support. One of these was due to the
remote interface printing a thread identifier instead of a process
identifier in the follow_fork verbose mode "Detaching..." messages.
The other was more serious, where a vfork child could sometimes be
(incorrectly) resumed when handling the vfork event.
The patchset addresses these three issues with the following three
patches:
Patch 1: modifies the ptids used in the verbose-mode "Detaching..."
messages in infrun.c:follow_fork_inferior to ensure that they print
"process nnn" instead of possibly "Thread nnn.nnn", so that native
and remote follow fork have the same behavior and match what the
follow fork tests expect.
Patch 2: initializes the threadinfo.last_resume_kind field for new
fork children in gdbserver/linux-low.c:handle_extended_wait. This
prevents the event handler from incorrectly resuming the child
if the stop event that is generated when it starts is reported
before the vfork event that initiated it.
Patch 3: disables the tests in gdb.base/foll-vfork.exp that depend
on exec event support for gdbserver targets.
Tested native, native-gdbserver, and native-extended-gdbserver on
x86_64 GNU/Linux.
Thanks
--Don
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] Make remote follow fork 'Detaching' message match native 2015-05-22 18:55 [PATCH 0/3] Extended-remote follow-fork cleanups Don Breazeal @ 2015-05-22 18:55 ` Don Breazeal 2015-05-23 11:18 ` Pedro Alves 2015-05-22 18:55 ` [PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Don Breazeal @ 2015-05-22 18:55 UTC (permalink / raw) To: gdb-patches This patch fixes a couple of failures in gdb.base/foll-vfork.exp for extended-remote targets. The failures were the result of the verbose/debug "Detaching..." messages in infrun.c:follow_fork_inferior not matching what was expected in the extended-remote case. The path modifies the ptids used in the messages to ensure that they print "process nnn" instead of (possibly) "Thread nnn.nnn". The ptids for the native case are already in this form, so there the change has no effect. The ptids in the extended-remote case must be reported by gdbserver in the (pid,pid,0) form in order to later identify and remove new fork children that are reported prematurely by remote_update_thread_list. So here we generate process-style ptids to get identical messages in both native and extended-remote cases. OK? thanks --Don gdb/ 2015-05-22 Don Breazeal <donb@codesourcery.com> * infrun.c (follow_fork_inferior): Ensure the use of process-style ptids (pid,0,0) in verbose/debug "Detaching" messages. --- gdb/infrun.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 2f6bc41..d8eb0b0 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -445,11 +445,14 @@ holding the child stopped. Try \"set detach-on-fork\" or \ if (info_verbose || debug_infrun) { + /* Ensure that we have a process ptid. */ + ptid_t process_ptid = pid_to_ptid (ptid_get_pid (child_ptid)); + target_terminal_ours_for_output (); fprintf_filtered (gdb_stdlog, _("Detaching after %s from child %s.\n"), has_vforked ? "vfork" : "fork", - target_pid_to_str (child_ptid)); + target_pid_to_str (process_ptid)); } } else @@ -578,11 +581,14 @@ holding the child stopped. Try \"set detach-on-fork\" or \ { if (info_verbose || debug_infrun) { + /* Ensure that we have a process ptid. */ + ptid_t process_ptid = pid_to_ptid (ptid_get_pid (child_ptid)); + target_terminal_ours_for_output (); fprintf_filtered (gdb_stdlog, _("Detaching after fork from " "child %s.\n"), - target_pid_to_str (child_ptid)); + target_pid_to_str (process_ptid)); } target_detach (NULL, 0); -- 1.8.1.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Make remote follow fork 'Detaching' message match native 2015-05-22 18:55 ` [PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal @ 2015-05-23 11:18 ` Pedro Alves 0 siblings, 0 replies; 11+ messages in thread From: Pedro Alves @ 2015-05-23 11:18 UTC (permalink / raw) To: Don Breazeal, gdb-patches On 05/22/2015 07:55 PM, Don Breazeal wrote: > This patch fixes a couple of failures in gdb.base/foll-vfork.exp for > extended-remote targets. The failures were the result of the > verbose/debug "Detaching..." messages in infrun.c:follow_fork_inferior > not matching what was expected in the extended-remote case. > > The path modifies the ptids used in the messages to ensure that they > print "process nnn" instead of (possibly) "Thread nnn.nnn". ... > The > ptids for the native case are already in this form, so there the > change has no effect. This isn't true. What happens is that linux_nat_pid_to_str gives (pid,pid,0) special treatment: (top-gdb) p ptid $2 = {pid = 14246, lwp = 14246, tid = 0} (top-gdb) bt #0 linux_nat_pid_to_str (ops=0xe357f0, ptid=...) at /home/pedro/gdb/mygit/src/gdb/linux-nat.c:4078 #1 0x0000000000675bb5 in delegate_pid_to_str (self=0xe357f0, arg1=...) at /home/pedro/gdb/mygit/src/gdb/target-delegates.c:1438 #2 0x0000000000682641 in target_pid_to_str (ptid=...) at /home/pedro/gdb/mygit/src/gdb/target.c:2212 #3 0x00000000006245b2 in follow_fork_inferior (follow_child=0, detach_fork=1) at /home/pedro/gdb/mygit/src/gdb/infrun.c:449 #4 0x0000000000624e7e in follow_fork () at /home/pedro/gdb/mygit/src/gdb/infrun.c:722 #5 0x000000000062a531 in handle_inferior_event_1 (ecs=0x7fffffffd180) at /home/pedro/gdb/mygit/src/gdb/infrun.c:4072 > The ptids in the extended-remote case must be > reported by gdbserver in the (pid,pid,0) form in order to later > identify and remove new fork children that are reported prematurely > by remote_update_thread_list. (so this bit is actually irrelevant) > So here we generate process-style ptids > to get identical messages in both native and extended-remote cases. > > OK? Still, detach is a process-wide operation, so this makes sense anyway. OK with commit log fixed. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] Initialize last_resume_kind for remote fork child 2015-05-22 18:55 [PATCH 0/3] Extended-remote follow-fork cleanups Don Breazeal 2015-05-22 18:55 ` [PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal @ 2015-05-22 18:55 ` Don Breazeal 2015-05-23 12:05 ` Pedro Alves 2015-05-22 18:56 ` [PATCH 3/3] Disable exec-dependent follow fork tests for remote Don Breazeal 2015-05-28 22:12 ` [commit][PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal 3 siblings, 1 reply; 11+ messages in thread From: Don Breazeal @ 2015-05-22 18:55 UTC (permalink / raw) To: gdb-patches This patch fixes some intermittent test failures in gdb.base/foll-vfork.exp where a vfork child could sometimes be (incorrectly) resumed when handling the vfork event. In this case the result was a subsequent event reported to the client side as a SIGTRAP delivered to the as-yet-unknown child thread. The fix is to initialize the threadinfo.last_resume_kind field for new fork children in gdbserver/linux-low.c:handle_extended_wait. This prevents the event handler from incorrectly resuming the child if the stop event that is generated when it starts is reported before the vfork event that initiated it. The same thing is done for the native case in linux-nat.c:linux_child_follow_fork. In addition, it seemed prudent to initialize lwp_info.status_pending_p for the new fork child. I also rearranged the initialization code so that all of the lwp_info initialization was together, rather than intermixed with thread_info and process_info initialization. Tested native, native-gdbserver, native-extended-gdbserver on x86_64 GNU/Linux. OK? Thanks --Don gdb/gdbserver/ 2015-05-22 Don Breazeal <donb@codesourcery.com> * linux-low.c (handle_extended_wait): --- gdb/gdbserver/linux-low.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 9f3ea48..d763c66 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -457,6 +457,7 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) struct process_info *parent_proc; struct process_info *child_proc; struct lwp_info *child_lwp; + struct thread_info *child_thr; struct target_desc *tdesc; ptid = ptid_build (new_pid, new_pid, 0); @@ -479,6 +480,10 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) child_lwp = add_lwp (ptid); gdb_assert (child_lwp != NULL); child_lwp->stopped = 1; + child_lwp->must_set_ptrace_flags = 1; + child_lwp->status_pending_p = 0; + child_thr = get_lwp_thread (child_lwp); + child_thr->last_resume_kind = resume_stop; parent_proc = get_thread_process (event_thr); child_proc->attached = parent_proc->attached; clone_all_breakpoints (&child_proc->breakpoints, @@ -488,7 +493,6 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) tdesc = xmalloc (sizeof (struct target_desc)); copy_target_description (tdesc, parent_proc->tdesc); child_proc->tdesc = tdesc; - child_lwp->must_set_ptrace_flags = 1; /* Clone arch-specific process data. */ if (the_low_target.new_fork != NULL) -- 1.8.1.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Initialize last_resume_kind for remote fork child 2015-05-22 18:55 ` [PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal @ 2015-05-23 12:05 ` Pedro Alves 0 siblings, 0 replies; 11+ messages in thread From: Pedro Alves @ 2015-05-23 12:05 UTC (permalink / raw) To: Don Breazeal, gdb-patches On 05/22/2015 07:55 PM, Don Breazeal wrote: > This patch fixes some intermittent test failures in > gdb.base/foll-vfork.exp where a vfork child could sometimes be > (incorrectly) resumed when handling the vfork event. In this > case the result was a subsequent event reported to the client > side as a SIGTRAP delivered to the as-yet-unknown child thread. > > The fix is to initialize the threadinfo.last_resume_kind field > for new fork children in gdbserver/linux-low.c:handle_extended_wait. > This prevents the event handler from incorrectly resuming the child > if the stop event that is generated when it starts is reported > before the vfork event that initiated it. I'm not seeing how what comes first makes a difference, actually. > The same thing is done > for the native case in linux-nat.c:linux_child_follow_fork. The code that resumes the child by mistake in question must be resume_stopped_resumed_lwps, and that is called similarly by both backends after pulling events out of the kernel, and right after linux_*_filter_event (which is what calls into handle_extended_wait): /* Now that we've pulled all events out of the kernel, resume LWPs that don't have an interesting event to report. */ iterate_over_lwps (minus_one_ptid, resume_stopped_resumed_lwps, &minus_one_ptid); ... /* Now that we've pulled all events out of the kernel, resume LWPs that don't have an interesting event to report. */ if (stopping_threads == NOT_STOPPING_THREADS) for_each_inferior (&all_threads, resume_stopped_resumed_lwps); That happens before linux_child_follow_fork is reached. The first difference is that gdb's version does not add the fork child to the lwp list in handle_extended_wait. But even if it did, the way linux-nat.c tags lwps as "not stopped-resumed" is different in gdb and gdbserver. gdb's has: static int 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)) { while gdbserver's: static void resume_stopped_resumed_lwps (struct inferior_list_entry *entry) { struct thread_info *thread = (struct thread_info *) entry; struct lwp_info *lp = get_thread_lwp (thread); if (lp->stopped && !lp->status_pending_p && thread->last_resume_kind != resume_stop && thread->last_status.kind == TARGET_WAITKIND_IGNORE) { So the main difference here is that linux-nat.c checks that "resumed" flag which doesn't exist in the gdbserver version. Checking lwp->resumed in gdb is equivalent to checking: (thread->last_resume_kind != resume_stop && thread->last_status.kind == TARGET_WAITKIND_IGNORE) in gdbserver/linux-low.c. gdbserver/linux-low.c's add_thread function creates threads with last_resume_kind == resume_continue by default. So the fix is to make sure to override that to resume_stop. gdb's linux_child_follow_fork version does the equivalent, but that's not what exactly prevents a spurious resume (it's the fact that add_lwp creates lwps with the 'resume' flag cleared). > > In addition, it seemed prudent to initialize lwp_info.status_pending_p > for the new fork child. That's fine. I think the child can well report a signal other than SIGSTOP first (or even be killed/disappear), in which case we should leave it pending (thus set status_pending_p), but that's a separate, preexisting issue. > I also rearranged the initialization code > so that all of the lwp_info initialization was together, rather than > intermixed with thread_info and process_info initialization. > > Tested native, native-gdbserver, native-extended-gdbserver on > x86_64 GNU/Linux. > > OK? So the fix is OK, though I think the commit log could be clarified. > > Thanks > --Don > > gdb/gdbserver/ > 2015-05-22 Don Breazeal <donb@codesourcery.com> > > * linux-low.c (handle_extended_wait): Seems incomplete. :-) > > --- > gdb/gdbserver/linux-low.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 9f3ea48..d763c66 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -457,6 +457,7 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) > struct process_info *parent_proc; > struct process_info *child_proc; > struct lwp_info *child_lwp; > + struct thread_info *child_thr; > struct target_desc *tdesc; > > ptid = ptid_build (new_pid, new_pid, 0); > @@ -479,6 +480,10 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) > child_lwp = add_lwp (ptid); > gdb_assert (child_lwp != NULL); > child_lwp->stopped = 1; > + child_lwp->must_set_ptrace_flags = 1; > + child_lwp->status_pending_p = 0; > + child_thr = get_lwp_thread (child_lwp); > + child_thr->last_resume_kind = resume_stop; > parent_proc = get_thread_process (event_thr); > child_proc->attached = parent_proc->attached; > clone_all_breakpoints (&child_proc->breakpoints, > @@ -488,7 +493,6 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) > tdesc = xmalloc (sizeof (struct target_desc)); > copy_target_description (tdesc, parent_proc->tdesc); > child_proc->tdesc = tdesc; > - child_lwp->must_set_ptrace_flags = 1; > > /* Clone arch-specific process data. */ > if (the_low_target.new_fork != NULL) > Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] Disable exec-dependent follow fork tests for remote 2015-05-22 18:55 [PATCH 0/3] Extended-remote follow-fork cleanups Don Breazeal 2015-05-22 18:55 ` [PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal 2015-05-22 18:55 ` [PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal @ 2015-05-22 18:56 ` Don Breazeal 2015-05-23 12:11 ` Pedro Alves 2015-05-28 22:12 ` [commit][PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal 3 siblings, 1 reply; 11+ messages in thread From: Don Breazeal @ 2015-05-22 18:56 UTC (permalink / raw) To: gdb-patches The native-extended-gdbserver target now supports fork events and follow fork, but it does not yet support exec events. Some of the tests in gdb.base/foll-vfork.exp depend on exec events. This patch disables those tests for gdbserver targets. We can re-enable these once the exec event support goes in. OK? --Don gdb/testsuite/ 2015-05-22 Don Breazeal <donb@codesourcery.com> * gdb.base/foll-vfork.exp (main): Disable exec-dependent tests for remote targets by checking is_target_gdbserver. --- gdb/testsuite/gdb.base/foll-vfork.exp | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/gdb/testsuite/gdb.base/foll-vfork.exp b/gdb/testsuite/gdb.base/foll-vfork.exp index 78c5cc8..26d7afd 100644 --- a/gdb/testsuite/gdb.base/foll-vfork.exp +++ b/gdb/testsuite/gdb.base/foll-vfork.exp @@ -524,18 +524,22 @@ with_test_prefix "check vfork support" { check_vfork_catchpoints } -# Follow parent and follow child vfork tests with a child that execs. -with_test_prefix "exec" { - # These are tests of gdb's ability to follow the parent of a Unix - # vfork system call. The child will subsequently call a variant - # of the Unix exec system call. - do_vfork_and_follow_parent_tests - - # These are tests of gdb's ability to follow the child of a Unix - # vfork system call. The child will subsequently call a variant - # of a Unix exec system call. - # - do_vfork_and_follow_child_tests_exec +if { ![target_is_gdbserver] } { + # Follow parent and follow child vfork tests with a child that execs. + with_test_prefix "exec" { + # These are tests of gdb's ability to follow the parent of a Unix + # vfork system call. The child will subsequently call a variant + # of the Unix exec system call. + do_vfork_and_follow_parent_tests + + # These are tests of gdb's ability to follow the child of a Unix + # vfork system call. The child will subsequently call a variant + # of a Unix exec system call. + # + do_vfork_and_follow_child_tests_exec + } +} else { + untested "vfork with exec: exec events not supported for remote" } # Switch to test the case of the child exiting. We can't use -- 1.8.1.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Disable exec-dependent follow fork tests for remote 2015-05-22 18:56 ` [PATCH 3/3] Disable exec-dependent follow fork tests for remote Don Breazeal @ 2015-05-23 12:11 ` Pedro Alves 2015-05-23 12:14 ` Pedro Alves 0 siblings, 1 reply; 11+ messages in thread From: Pedro Alves @ 2015-05-23 12:11 UTC (permalink / raw) To: Don Breazeal, gdb-patches On 05/22/2015 07:55 PM, Don Breazeal wrote: > The native-extended-gdbserver target now supports fork events and > follow fork, but it does not yet support exec events. Some of the > tests in gdb.base/foll-vfork.exp depend on exec events. This patch > disables those tests for gdbserver targets. We can re-enable these > once the exec event support goes in. > > OK? Pedantically, I think this should check gdb_is_target_remote instead. We lack support for exec events in the RSP, so this would fail for any remote protocol stub, not just our gdbserver. > > --Don > > > gdb/testsuite/ > 2015-05-22 Don Breazeal <donb@codesourcery.com> > > * gdb.base/foll-vfork.exp (main): Disable exec-dependent > tests for remote targets by checking is_target_gdbserver. > > --- > gdb/testsuite/gdb.base/foll-vfork.exp | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/foll-vfork.exp b/gdb/testsuite/gdb.base/foll-vfork.exp > index 78c5cc8..26d7afd 100644 > --- a/gdb/testsuite/gdb.base/foll-vfork.exp > +++ b/gdb/testsuite/gdb.base/foll-vfork.exp > @@ -524,18 +524,22 @@ with_test_prefix "check vfork support" { > check_vfork_catchpoints > } > > -# Follow parent and follow child vfork tests with a child that execs. > -with_test_prefix "exec" { > - # These are tests of gdb's ability to follow the parent of a Unix > - # vfork system call. The child will subsequently call a variant > - # of the Unix exec system call. > - do_vfork_and_follow_parent_tests > - > - # These are tests of gdb's ability to follow the child of a Unix > - # vfork system call. The child will subsequently call a variant > - # of a Unix exec system call. > - # > - do_vfork_and_follow_child_tests_exec > +if { ![target_is_gdbserver] } { > + # Follow parent and follow child vfork tests with a child that execs. > + with_test_prefix "exec" { > + # These are tests of gdb's ability to follow the parent of a Unix > + # vfork system call. The child will subsequently call a variant > + # of the Unix exec system call. > + do_vfork_and_follow_parent_tests > + > + # These are tests of gdb's ability to follow the child of a Unix > + # vfork system call. The child will subsequently call a variant > + # of a Unix exec system call. > + # > + do_vfork_and_follow_child_tests_exec > + } > +} else { > + untested "vfork with exec: exec events not supported for remote" 'git am' caught that there's a spurious space at the end of this line. Please remove it. OK with these changes. > } > > # Switch to test the case of the child exiting. We can't use > Thanks! -- Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Disable exec-dependent follow fork tests for remote 2015-05-23 12:11 ` Pedro Alves @ 2015-05-23 12:14 ` Pedro Alves 0 siblings, 0 replies; 11+ messages in thread From: Pedro Alves @ 2015-05-23 12:14 UTC (permalink / raw) To: Don Breazeal, gdb-patches On 05/23/2015 01:11 PM, Pedro Alves wrote: > On 05/22/2015 07:55 PM, Don Breazeal wrote: >> The native-extended-gdbserver target now supports fork events and >> follow fork, but it does not yet support exec events. Some of the >> tests in gdb.base/foll-vfork.exp depend on exec events. This patch >> disables those tests for gdbserver targets. We can re-enable these >> once the exec event support goes in. >> >> OK? > > Pedantically, I think this should check gdb_is_target_remote instead. > We lack support for exec events in the RSP, so this would fail > for any remote protocol stub, not just our gdbserver. > >> >> --Don >> >> >> gdb/testsuite/ >> 2015-05-22 Don Breazeal <donb@codesourcery.com> >> >> * gdb.base/foll-vfork.exp (main): Disable exec-dependent >> tests for remote targets by checking is_target_gdbserver. >> >> --- >> gdb/testsuite/gdb.base/foll-vfork.exp | 28 ++++++++++++++++------------ >> 1 file changed, 16 insertions(+), 12 deletions(-) >> >> diff --git a/gdb/testsuite/gdb.base/foll-vfork.exp b/gdb/testsuite/gdb.base/foll-vfork.exp >> index 78c5cc8..26d7afd 100644 >> --- a/gdb/testsuite/gdb.base/foll-vfork.exp >> +++ b/gdb/testsuite/gdb.base/foll-vfork.exp >> @@ -524,18 +524,22 @@ with_test_prefix "check vfork support" { >> check_vfork_catchpoints >> } >> >> -# Follow parent and follow child vfork tests with a child that execs. >> -with_test_prefix "exec" { >> - # These are tests of gdb's ability to follow the parent of a Unix >> - # vfork system call. The child will subsequently call a variant >> - # of the Unix exec system call. >> - do_vfork_and_follow_parent_tests >> - >> - # These are tests of gdb's ability to follow the child of a Unix >> - # vfork system call. The child will subsequently call a variant >> - # of a Unix exec system call. >> - # >> - do_vfork_and_follow_child_tests_exec >> +if { ![target_is_gdbserver] } { >> + # Follow parent and follow child vfork tests with a child that execs. >> + with_test_prefix "exec" { >> + # These are tests of gdb's ability to follow the parent of a Unix >> + # vfork system call. The child will subsequently call a variant >> + # of the Unix exec system call. >> + do_vfork_and_follow_parent_tests >> + >> + # These are tests of gdb's ability to follow the child of a Unix >> + # vfork system call. The child will subsequently call a variant >> + # of a Unix exec system call. >> + # >> + do_vfork_and_follow_child_tests_exec >> + } >> +} else { >> + untested "vfork with exec: exec events not supported for remote" > > 'git am' caught that there's a spurious space at the end of this line. > Please remove it. > > OK with these changes. Oh, BTW, still from the pedantic department, that should probably be an "unsupported" call instead of "untested". > >> } >> >> # Switch to test the case of the child exiting. We can't use >> > > Thanks! > Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* [commit][PATCH 1/3] Make remote follow fork 'Detaching' message match native 2015-05-22 18:55 [PATCH 0/3] Extended-remote follow-fork cleanups Don Breazeal ` (2 preceding siblings ...) 2015-05-22 18:56 ` [PATCH 3/3] Disable exec-dependent follow fork tests for remote Don Breazeal @ 2015-05-28 22:12 ` Don Breazeal 2015-05-28 22:13 ` [commit][PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal 2015-05-28 22:13 ` [commit][PATCH 3/3] Disable exec-dependent follow vfork tests for remote Don Breazeal 3 siblings, 2 replies; 11+ messages in thread From: Don Breazeal @ 2015-05-28 22:12 UTC (permalink / raw) To: gdb-patches This patch fixes a couple of failures in gdb.base/foll-vfork.exp for extended-remote targets. The failures were the result of the verbose/debug "Detaching..." messages in infrun.c:follow_fork_inferior not matching what was expected in the extended-remote case. The path modifies the ptids used in the messages to ensure that they print "process nnn" instead of (possibly) "Thread nnn.nnn". The detach is a process-wide operation, so we need to use a process- style ptid regardless of what type of ptid target_pid_to_str returns. Tested on x86_64 GNU/Linux, native, remote, extended-remote. gdb/ * infrun.c (follow_fork_inferior): Ensure the use of process-style ptids (pid,0,0) in verbose/debug "Detaching" messages. --- gdb/ChangeLog | 6 ++++++ gdb/infrun.c | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index bd0292b..ab10166 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2015-05-28 Don Breazeal <donb@codesourcery.com> + + * infrun.c (follow_fork_inferior): Ensure the use of + process-style ptids (pid,0,0) in verbose/debug "Detaching" + messages. + 2015-05-28 Doug Evans <dje@google.com> * dwarf2read.c (record_line_ftype): Remove, duplicate. diff --git a/gdb/infrun.c b/gdb/infrun.c index 2f6bc41..d8eb0b0 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -445,11 +445,14 @@ holding the child stopped. Try \"set detach-on-fork\" or \ if (info_verbose || debug_infrun) { + /* Ensure that we have a process ptid. */ + ptid_t process_ptid = pid_to_ptid (ptid_get_pid (child_ptid)); + target_terminal_ours_for_output (); fprintf_filtered (gdb_stdlog, _("Detaching after %s from child %s.\n"), has_vforked ? "vfork" : "fork", - target_pid_to_str (child_ptid)); + target_pid_to_str (process_ptid)); } } else @@ -578,11 +581,14 @@ holding the child stopped. Try \"set detach-on-fork\" or \ { if (info_verbose || debug_infrun) { + /* Ensure that we have a process ptid. */ + ptid_t process_ptid = pid_to_ptid (ptid_get_pid (child_ptid)); + target_terminal_ours_for_output (); fprintf_filtered (gdb_stdlog, _("Detaching after fork from " "child %s.\n"), - target_pid_to_str (child_ptid)); + target_pid_to_str (process_ptid)); } target_detach (NULL, 0); -- 1.8.1.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [commit][PATCH 2/3] Initialize last_resume_kind for remote fork child 2015-05-28 22:12 ` [commit][PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal @ 2015-05-28 22:13 ` Don Breazeal 2015-05-28 22:13 ` [commit][PATCH 3/3] Disable exec-dependent follow vfork tests for remote Don Breazeal 1 sibling, 0 replies; 11+ messages in thread From: Don Breazeal @ 2015-05-28 22:13 UTC (permalink / raw) To: gdb-patches This patch fixes some intermittent test failures in gdb.base/foll-vfork.exp where a vfork child would be (incorrectly) resumed when handling the vfork event. In this case the result was a subsequent event reported to the client side as a SIGTRAP delivered to the as-yet-unknown child thread. The new thread was resumed (incorrectly) in linux-low.c when resume_stopped_resumed_lwps was called from linux_wait_for_event_filtered after the vfork event had been handled in handle_extended_wait. Gdbserver/linux-low.c's add_thread function creates threads with last_resume_kind == resume_continue by default. This field is used by resume_stopped_resumed_lwps to decide whether to perform the resume: static void resume_stopped_resumed_lwps (struct inferior_list_entry *entry) { struct thread_info *thread = (struct thread_info *) entry; struct lwp_info *lp = get_thread_lwp (thread); if (lp->stopped && !lp->status_pending_p && thread->last_resume_kind != resume_stop && thread->last_status.kind == TARGET_WAITKIND_IGNORE) { So the fix is to make sure to set thread->last_resume_kind to resume_stop. Here we do that for new fork children in gdbserver/linux-low.c:handle_extended_wait. In addition, it seemed prudent to initialize lwp_info.status_pending_p for the new fork child. I also rearranged the initialization code so that all of the lwp_info initialization was together, rather than intermixed with thread_info and process_info initialization. Tested native, native-gdbserver, native-extended-gdbserver on x86_64 GNU/Linux. gdb/gdbserver/ * linux-low.c (handle_extended_wait): Initialize thread_info.last_resume_kind for new fork children. --- gdb/gdbserver/ChangeLog | 5 +++++ gdb/gdbserver/linux-low.c | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 0f30c66..4e4f429 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,8 @@ +2015-05-28 Don Breazeal <donb@codesourcery.com> + + * linux-low.c (handle_extended_wait): Initialize + thread_info.last_resume_kind for new fork children. + 2015-05-15 Pedro Alves <palves@redhat.com> * target.h (target_handle_new_gdb_connection): Rewrite using if diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 9f3ea48..d763c66 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -457,6 +457,7 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) struct process_info *parent_proc; struct process_info *child_proc; struct lwp_info *child_lwp; + struct thread_info *child_thr; struct target_desc *tdesc; ptid = ptid_build (new_pid, new_pid, 0); @@ -479,6 +480,10 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) child_lwp = add_lwp (ptid); gdb_assert (child_lwp != NULL); child_lwp->stopped = 1; + child_lwp->must_set_ptrace_flags = 1; + child_lwp->status_pending_p = 0; + child_thr = get_lwp_thread (child_lwp); + child_thr->last_resume_kind = resume_stop; parent_proc = get_thread_process (event_thr); child_proc->attached = parent_proc->attached; clone_all_breakpoints (&child_proc->breakpoints, @@ -488,7 +493,6 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) tdesc = xmalloc (sizeof (struct target_desc)); copy_target_description (tdesc, parent_proc->tdesc); child_proc->tdesc = tdesc; - child_lwp->must_set_ptrace_flags = 1; /* Clone arch-specific process data. */ if (the_low_target.new_fork != NULL) -- 1.8.1.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [commit][PATCH 3/3] Disable exec-dependent follow vfork tests for remote 2015-05-28 22:12 ` [commit][PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal 2015-05-28 22:13 ` [commit][PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal @ 2015-05-28 22:13 ` Don Breazeal 1 sibling, 0 replies; 11+ messages in thread From: Don Breazeal @ 2015-05-28 22:13 UTC (permalink / raw) To: gdb-patches The native-extended-gdbserver target now supports fork events and follow fork, but it does not yet support exec events. Some of the tests in gdb.base/foll-vfork.exp depend on exec events. This patch disables those tests for remote targets. We can re-enable these once the exec event support goes in. gdb/testsuite/ * gdb.base/foll-vfork.exp (main): Disable exec-dependent tests for remote targets by checking is_target_gdbserver. --- gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/gdb.base/foll-vfork.exp | 29 +++++++++++++++++------------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 94a0329..b08b59d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2015-05-28 Don Breazeal <donb@codesourcery.com> + + * gdb.base/foll-vfork.exp (main): Disable exec-dependent + tests for remote targets by checking is_target_gdbserver. + 2015-05-27 Doug Evans <dje@google.com> * gdb.dwarf2/opaque-type-lookup-2.c: New file. diff --git a/gdb/testsuite/gdb.base/foll-vfork.exp b/gdb/testsuite/gdb.base/foll-vfork.exp index 78c5cc8..b94b7ea 100644 --- a/gdb/testsuite/gdb.base/foll-vfork.exp +++ b/gdb/testsuite/gdb.base/foll-vfork.exp @@ -524,18 +524,23 @@ with_test_prefix "check vfork support" { check_vfork_catchpoints } -# Follow parent and follow child vfork tests with a child that execs. -with_test_prefix "exec" { - # These are tests of gdb's ability to follow the parent of a Unix - # vfork system call. The child will subsequently call a variant - # of the Unix exec system call. - do_vfork_and_follow_parent_tests - - # These are tests of gdb's ability to follow the child of a Unix - # vfork system call. The child will subsequently call a variant - # of a Unix exec system call. - # - do_vfork_and_follow_child_tests_exec +# There is no support for exec events in the RSP yet. +if { ![gdb_is_target_remote] } { + # Follow parent and follow child vfork tests with a child that execs. + with_test_prefix "exec" { + # These are tests of gdb's ability to follow the parent of a Unix + # vfork system call. The child will subsequently call a variant + # of the Unix exec system call. + do_vfork_and_follow_parent_tests + + # These are tests of gdb's ability to follow the child of a Unix + # vfork system call. The child will subsequently call a variant + # of a Unix exec system call. + # + do_vfork_and_follow_child_tests_exec + } +} else { + unsupported "vfork with exec: exec events not supported for remote" } # Switch to test the case of the child exiting. We can't use -- 1.8.1.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-05-28 22:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-22 18:55 [PATCH 0/3] Extended-remote follow-fork cleanups Don Breazeal 2015-05-22 18:55 ` [PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal 2015-05-23 11:18 ` Pedro Alves 2015-05-22 18:55 ` [PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal 2015-05-23 12:05 ` Pedro Alves 2015-05-22 18:56 ` [PATCH 3/3] Disable exec-dependent follow fork tests for remote Don Breazeal 2015-05-23 12:11 ` Pedro Alves 2015-05-23 12:14 ` Pedro Alves 2015-05-28 22:12 ` [commit][PATCH 1/3] Make remote follow fork 'Detaching' message match native Don Breazeal 2015-05-28 22:13 ` [commit][PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal 2015-05-28 22:13 ` [commit][PATCH 3/3] Disable exec-dependent follow vfork tests for remote Don Breazeal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox