From: Don Breazeal <donb@codesourcery.com>
To: <gdb-patches@sourceware.org>, <yao@codesourcery.com>
Subject: Re: [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem
Date: Thu, 15 May 2014 20:43:00 -0000 [thread overview]
Message-ID: <1400186622-3321-1-git-send-email-donb@codesourcery.com> (raw)
On 5/15/2014 12:06 AM, Yao Qi wrote:
> 05/13/2014 07:34 AM, donb@codesourcery.com wrote:
>> @@ -649,6 +649,7 @@ handle_vfork_child_exec_or_exit (int exec)
>> struct cleanup *old_chain;
>> struct program_space *pspace;
>> struct address_space *aspace;
>> + struct inferior *parent_inf;
>>
>
> Local parent_inf is only used in the "if (exec)" block below, so better
> to declare it there.
OK, done.
--snip--
>> + where the parent called vfork. Now that the child has
>> + called exec and we are detaching from the parent, the
>> + parent inferior needs to have its own pspace and aspace
>
> parent inferior has its own pspace, but may not have its own aspace,
> depending on gdbarch_has_shared_address_space.
Fixed.
>
>> + so that changes in the child don't affect it. We have
>> + to give the new spaces to the parent since we saved the
>> + child's spaces as the current spaces above. Even though
>> + we are detaching the parent, we want to keep the
>> + corresponding entry in the inferiors list intact. */
>> + parent_inf = current_inferior ();
>> + parent_inf->aspace = new_address_space ();
>
> Rather than creating a new address space, use maybe_new_address_space, like
>> + parent_inf->pspace = add_program_space (parent_inf->aspace);
>
> parent_inf->pspace
> = add_program_space (maybe_new_address_space ());
> parent_inf->aspace = parent_inf->pspace->aspace;
Done.
>
>> + parent_inf->removable = inf->removable;
>
> Field removable of parent inferior should be unchanged, IMO.
It turns out this assignment was a NOP. Removed.
>
>> + set_current_program_space (parent_inf->pspace);
>> + clone_program_space (parent_inf->pspace, pspace);
>
> Do we need to unlink parent and child? I am not very sure.
>
> /* Break the bonds. */
> inf->vfork_parent->vfork_child = NULL;
>
We don't need to do it here, since this happens in target_detach. Since parent_inf is the parent, it's fork_child pointer is set. The unlinking happens in inferior.c:exit_inferior_1, which is ultimately called via target_detach. The child's vfork_parent pointer is cleared by existing code near the end of handle_vfork_child_exec_or_exit.
Here is the updated patch.
--Don
gdb/
2014-05-15 Don Breazeal <donb@codesourcery.com>
* infrun.c (handle_vfork_child_exec_or_exit): For the case
of a vfork where we follow the child and detach the parent,
and the child execs, create a new pspace for the parent
inferior.
gdb/testsuite
2014-05-15 Don Breazeal <donb@codesourcery.com>
* gdb.base/foll-vfork.exp (vfork_relations_in_info_inferiors):
Test that after a vfork and child exec, the parent's exec file
name has not been changed.
---
gdb/infrun.c | 44 ++++++++++++++++++++++----------
gdb/testsuite/gdb.base/foll-vfork.exp | 11 ++++++++
2 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index ab39b6e..20ab003 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -665,27 +665,42 @@ handle_vfork_child_exec_or_exit (int exec)
else
old_chain = save_current_space_and_thread ();
- /* We're letting loose of the parent. */
+ /* Make the parent the current inferior for target_detach. */
tp = any_live_thread_of_process (inf->vfork_parent->pid);
switch_to_thread (tp->ptid);
- /* We're about to detach from the parent, which implicitly
- removes breakpoints from its address space. There's a
- catch here: we want to reuse the spaces for the child,
- but, parent/child are still sharing the pspace at this
- point, although the exec in reality makes the kernel give
- the child a fresh set of new pages. The problem here is
- that the breakpoints module being unaware of this, would
- likely chose the child process to write to the parent
- address space. Swapping the child temporarily away from
- the spaces has the desired effect. Yes, this is "sort
- of" a hack. */
-
+ /* The child inferior may be dead, so avoid giving the
+ breakpoints module the option to write through to it
+ by swapping the child temporarily away from the spaces
+ (cloning a program space resets breakpoints). */
pspace = inf->pspace;
aspace = inf->aspace;
inf->aspace = NULL;
inf->pspace = NULL;
+ if (exec)
+ {
+ struct inferior *parent_inf;
+
+ /* The parent and child inferiors have been sharing
+ program and address space structures from the point
+ where the parent called vfork. Now that the child has
+ called exec and we are detaching from the parent, the
+ parent inferior needs to have its own pspace so that
+ changes in the child don't affect it. We have to give
+ the new pspace to the parent (instead of the child)
+ since we saved the child's spaces as the current spaces
+ above. Even though we are detaching the parent, we
+ want to keep the corresponding entry in the inferiors
+ list intact. */
+ parent_inf = current_inferior ();
+ parent_inf->pspace
+ = add_program_space (maybe_new_address_space ());
+ parent_inf->aspace = parent_inf->pspace->aspace;
+ set_current_program_space (parent_inf->pspace);
+ clone_program_space (parent_inf->pspace, pspace);
+ }
+
if (debug_infrun || info_verbose)
{
target_terminal_ours ();
@@ -702,9 +717,10 @@ handle_vfork_child_exec_or_exit (int exec)
inf->vfork_parent->pid);
}
+ /* Detach the parent. */
target_detach (NULL, 0);
- /* Put it back. */
+ /* Put the child spaces back. */
inf->pspace = pspace;
inf->aspace = aspace;
diff --git a/gdb/testsuite/gdb.base/foll-vfork.exp b/gdb/testsuite/gdb.base/foll-vfork.exp
index fe3663c..e9b0110 100644
--- a/gdb/testsuite/gdb.base/foll-vfork.exp
+++ b/gdb/testsuite/gdb.base/foll-vfork.exp
@@ -442,6 +442,17 @@ proc vfork_relations_in_info_inferiors { variant } {
pass $test
}
}
+
+ # Make sure the exec file name of the vfork parent is not
+ # changed when the child's is changed.
+ if { $variant == "exec" } {
+ set test "exec file name change"
+ gdb_test_multiple "info inferiors" $test {
+ -re " 2 .*vforked-prog.* 1 .*foll-vfork.*$gdb_prompt " {
+ pass $test
+ }
+ }
+ }
}}
proc do_vfork_and_follow_parent_tests {} {
--
1.7.0.4
next reply other threads:[~2014-05-15 20:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-15 20:43 Don Breazeal [this message]
2014-05-23 18:47 ` [PING][PATCH] Fix for PR gdb/14808, " Breazeal, Don
2014-05-24 0:59 ` [PATCH] Fix for gdb/PR 14808, " Yao Qi
2014-05-27 17:01 ` Breazeal, Don
2014-06-04 18:35 ` [ping] [PATCH] Fix for PR gdb/14808, " Breazeal, Don
2014-06-19 15:51 ` Breazeal, Don
-- strict thread matches above, loose matches on Subject: below --
2014-05-12 23:34 [PATCH] Fix for gdb/PR 14808, " donb
2014-05-15 7:08 ` Yao Qi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1400186622-3321-1-git-send-email-donb@codesourcery.com \
--to=donb@codesourcery.com \
--cc=gdb-patches@sourceware.org \
--cc=yao@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).