* [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 ` [PATCH 2/3] Initialize last_resume_kind for remote fork child Don Breazeal
@ 2015-05-22 18:55 ` Don Breazeal
2015-05-23 11:18 ` 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 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
* [PATCH 0/3] Extended-remote follow-fork cleanups
@ 2015-05-22 18:55 Don Breazeal
2015-05-22 18:55 ` [PATCH 2/3] Initialize last_resume_kind for remote fork child 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 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 ` Don Breazeal
2015-05-23 12:05 ` Pedro Alves
2015-05-22 18:55 ` [PATCH 1/3] Make remote follow fork 'Detaching' message match native 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 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
* [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 2/3] Initialize last_resume_kind for remote fork child Don Breazeal
2015-05-22 18:55 ` [PATCH 1/3] Make remote follow fork 'Detaching' message match native 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 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
* 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
* 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 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
* [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
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 2/3] Initialize last_resume_kind for remote fork child Don Breazeal
2015-05-23 12:05 ` Pedro Alves
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: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