* Re: [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem
@ 2014-05-15 20:43 Don Breazeal
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
0 siblings, 2 replies; 6+ messages in thread
From: Don Breazeal @ 2014-05-15 20:43 UTC (permalink / raw)
To: gdb-patches, yao
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PING][PATCH] Fix for PR gdb/14808, vfork/exec inferior problem
2014-05-15 20:43 [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem Don Breazeal
@ 2014-05-23 18:47 ` Breazeal, Don
2014-05-24 0:59 ` [PATCH] Fix for gdb/PR 14808, " Yao Qi
1 sibling, 0 replies; 6+ messages in thread
From: Breazeal, Don @ 2014-05-23 18:47 UTC (permalink / raw)
To: gdb-patches
Ping!
On 5/15/2014 1:43 PM, Don Breazeal wrote:
> 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 {} {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem
2014-05-15 20:43 [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem Don Breazeal
2014-05-23 18:47 ` [PING][PATCH] Fix for PR gdb/14808, " Breazeal, Don
@ 2014-05-24 0:59 ` Yao Qi
2014-05-27 17:01 ` Breazeal, Don
1 sibling, 1 reply; 6+ messages in thread
From: Yao Qi @ 2014-05-24 0:59 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
A nit on changelog entry:
On 05/16/2014 04:43 AM, Don Breazeal wrote:
> gdb/
> 2014-05-15 Don Breazeal <donb@codesourcery.com>
>
PR gdb/14808
> * 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.
With this, the magic script can connect this commit to the right PR.
The patch looks good to me, but I can't approve it.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem
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
0 siblings, 1 reply; 6+ messages in thread
From: Breazeal, Don @ 2014-05-27 17:01 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 5/23/2014 5:57 PM, Yao Qi wrote:
> A nit on changelog entry:
>
> On 05/16/2014 04:43 AM, Don Breazeal wrote:
>> gdb/
>> 2014-05-15 Don Breazeal <donb@codesourcery.com>
>>
> PR gdb/14808
>> * 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.
>
> With this, the magic script can connect this commit to the right PR.
>
> The patch looks good to me, but I can't approve it.
>
Thanks for checking this. An updated ChangeLog and (unchanged) patch
follow.
--Don
gdb/
2014-05-12 Don Breazeal <donb@codesourcery.com>
PR gdb/14808
* 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 and aspace for the
parent inferior.
gdb/testsuite
2014-05-12 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 | 42
++++++++++++++++++++++-----------
gdb/testsuite/gdb.base/foll-vfork.exp | 11 ++++++++
2 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index ab39b6e..82a67d5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -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;
/* follow-fork child, detach-on-fork on. */
@@ -665,27 +666,39 @@ 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 INF 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)
+ {
+ /* 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 and aspace
+ 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 ();
+ parent_inf->pspace = add_program_space (parent_inf->aspace);
+ parent_inf->removable = inf->removable;
+ set_current_program_space (parent_inf->pspace);
+ clone_program_space (parent_inf->pspace, pspace);
+ }
+
if (debug_infrun || info_verbose)
{
target_terminal_ours ();
@@ -702,9 +715,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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ping] [PATCH] Fix for PR gdb/14808, vfork/exec inferior problem
2014-05-27 17:01 ` Breazeal, Don
@ 2014-06-04 18:35 ` Breazeal, Don
2014-06-19 15:51 ` Breazeal, Don
0 siblings, 1 reply; 6+ messages in thread
From: Breazeal, Don @ 2014-06-04 18:35 UTC (permalink / raw)
To: gdb-patches
Ping.
On 5/27/2014 10:01 AM, Breazeal, Don wrote:
> On 5/23/2014 5:57 PM, Yao Qi wrote:
>> A nit on changelog entry:
>>
>> On 05/16/2014 04:43 AM, Don Breazeal wrote:
>>> gdb/
>>> 2014-05-15 Don Breazeal <donb@codesourcery.com>
>>>
>> PR gdb/14808
>>> * 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.
>>
>> With this, the magic script can connect this commit to the right PR.
>>
>> The patch looks good to me, but I can't approve it.
>>
> Thanks for checking this. An updated ChangeLog and (unchanged) patch
> follow.
> --Don
>
> gdb/
> 2014-05-12 Don Breazeal <donb@codesourcery.com>
>
> PR gdb/14808
> * 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 and aspace for the
> parent inferior.
>
> gdb/testsuite
> 2014-05-12 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 | 42
> ++++++++++++++++++++++-----------
> gdb/testsuite/gdb.base/foll-vfork.exp | 11 ++++++++
> 2 files changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index ab39b6e..82a67d5 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -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;
>
> /* follow-fork child, detach-on-fork on. */
>
> @@ -665,27 +666,39 @@ 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 INF 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)
> + {
> + /* 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 and aspace
> + 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 ();
> + parent_inf->pspace = add_program_space (parent_inf->aspace);
> + parent_inf->removable = inf->removable;
> + set_current_program_space (parent_inf->pspace);
> + clone_program_space (parent_inf->pspace, pspace);
> + }
> +
> if (debug_infrun || info_verbose)
> {
> target_terminal_ours ();
> @@ -702,9 +715,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 {} {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ping] [PATCH] Fix for PR gdb/14808, vfork/exec inferior problem
2014-06-04 18:35 ` [ping] [PATCH] Fix for PR gdb/14808, " Breazeal, Don
@ 2014-06-19 15:51 ` Breazeal, Don
0 siblings, 0 replies; 6+ messages in thread
From: Breazeal, Don @ 2014-06-19 15:51 UTC (permalink / raw)
To: gdb-patches
Ping.
thanks
--Don
On 6/4/2014 11:35 AM, Breazeal, Don wrote:
> Ping.
>
> On 5/27/2014 10:01 AM, Breazeal, Don wrote:
>> On 5/23/2014 5:57 PM, Yao Qi wrote:
>>> A nit on changelog entry:
>>>
>>> On 05/16/2014 04:43 AM, Don Breazeal wrote:
>>>> gdb/
>>>> 2014-05-15 Don Breazeal <donb@codesourcery.com>
>>>>
>>> PR gdb/14808
>>>> * 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.
>>>
>>> With this, the magic script can connect this commit to the right PR.
>>>
>>> The patch looks good to me, but I can't approve it.
>>>
>> Thanks for checking this. An updated ChangeLog and (unchanged) patch
>> follow.
>> --Don
>>
>> gdb/
>> 2014-05-12 Don Breazeal <donb@codesourcery.com>
>>
>> PR gdb/14808
>> * 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 and aspace for the
>> parent inferior.
>>
>> gdb/testsuite
>> 2014-05-12 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 | 42
>> ++++++++++++++++++++++-----------
>> gdb/testsuite/gdb.base/foll-vfork.exp | 11 ++++++++
>> 2 files changed, 39 insertions(+), 14 deletions(-)
>>
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index ab39b6e..82a67d5 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -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;
>>
>> /* follow-fork child, detach-on-fork on. */
>>
>> @@ -665,27 +666,39 @@ 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 INF 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)
>> + {
>> + /* 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 and aspace
>> + 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 ();
>> + parent_inf->pspace = add_program_space (parent_inf->aspace);
>> + parent_inf->removable = inf->removable;
>> + set_current_program_space (parent_inf->pspace);
>> + clone_program_space (parent_inf->pspace, pspace);
>> + }
>> +
>> if (debug_infrun || info_verbose)
>> {
>> target_terminal_ours ();
>> @@ -702,9 +715,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 {} {
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-19 15:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-15 20:43 [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem Don Breazeal
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
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).