Fix for gdb/PR 14808, vfork/exec inferior problem
Commit Message
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(-)
Comments
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 {} {
>
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 {} {
>>
>
@@ -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;
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 {} {