Unset attach_flag when running a new process
Commit Message
We currently set attach_flag when attaching to a process, so we should
make sure to unset it when forking a new process. Otherwise attach_flag
would remain set after forking, if the previous process associated with
the inferior was attached to.
gdb/ChangeLog:
* target.c (target_pre_inferior): Unset attach_flag.
gdb/testsuite/ChangeLog:
* gdb.base/run-after-attach.exp: New test file.
* gdb.base/run-after-attach.c: New test file.
---
gdb/target.c | 4 ++
gdb/testsuite/gdb.base/run-after-attach.c | 25 +++++++++++
gdb/testsuite/gdb.base/run-after-attach.exp | 65 +++++++++++++++++++++++++++++
3 files changed, 94 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/run-after-attach.c
create mode 100644 gdb/testsuite/gdb.base/run-after-attach.exp
Comments
On Mon, Aug 17, 2015 at 9:01 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> We currently set attach_flag when attaching to a process, so we should
> make sure to unset it when forking a new process. Otherwise attach_flag
> would remain set after forking, if the previous process associated with
> the inferior was attached to.
>
> gdb/ChangeLog:
>
> * target.c (target_pre_inferior): Unset attach_flag.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/run-after-attach.exp: New test file.
> * gdb.base/run-after-attach.c: New test file.
> ---
> gdb/target.c | 4 ++
> gdb/testsuite/gdb.base/run-after-attach.c | 25 +++++++++++
> gdb/testsuite/gdb.base/run-after-attach.exp | 65 +++++++++++++++++++++++++++++
> 3 files changed, 94 insertions(+)
> create mode 100644 gdb/testsuite/gdb.base/run-after-attach.c
> create mode 100644 gdb/testsuite/gdb.base/run-after-attach.exp
>
> diff --git a/gdb/target.c b/gdb/target.c
> index e41a338..96e7df7 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2138,6 +2138,10 @@ target_pre_inferior (int from_tty)
> target_clear_description ();
> }
>
> + /* attach_flag may be set if the previous process associated with
> + the inferior was attached to. */
> + current_inferior ()->attach_flag = 0;
> +
> agent_capability_invalidate ();
> }
>
> diff --git a/gdb/testsuite/gdb.base/run-after-attach.c b/gdb/testsuite/gdb.base/run-after-attach.c
> new file mode 100644
> index 0000000..2398f00
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/run-after-attach.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2015 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include <unistd.h>
> +
> +int
> +main (void)
> +{
> + sleep (600);
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/run-after-attach.exp b/gdb/testsuite/gdb.base/run-after-attach.exp
> new file mode 100644
> index 0000000..8de7ac6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/run-after-attach.exp
> @@ -0,0 +1,65 @@
> +# Copyright (C) 2015 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Check that forking a process after a previous process was attached to unsets
> +# attach_flag. This is done indirectly by inspecting GDB's quit prompt.
> +
> +if ![can_spawn_for_attach] {
> + return 0
> +}
> +
> +standard_testfile
> +set executable $testfile
> +
> +if [prepare_for_testing $testfile.exp $executable] {
> + return -1
> +}
> +
> +set test_spawn_id [spawn_wait_for_attach $binfile]
> +set test_pid [spawn_id_get_pid $test_spawn_id]
> +
> +set test "attach to process"
> +gdb_test "attach $test_pid" "Attaching to program.*" $test
> +
> +set test "kill process"
> +gdb_test "kill" "" $test \
> + "Kill the program being debugged.*y or n. $" "y"
> +
> +set test "restart process"
> +gdb_test "start" "Starting program.*Temporary breakpoint .*" $test
> +
> +set test "attempt kill via quit"
> +# The quit prompt should warn about killing the process, not about detaching the
> +# process, since this process was not attached to.
> +set ok 0
> +gdb_test_multiple "quit" $test {
> + -re "will be killed.*.y or n. $" {
> + set ok 1
> + send_gdb "n\n"
> + exp_continue
> + }
> + -re "will be detached.*.y or n. $" {
> + send_gdb "n\n"
> + exp_continue
> + }
> + -re "$gdb_prompt $" {
> + gdb_assert $ok $test
> + }
> + default {
> + fail $test
> + }
> +}
> +
> +kill_wait_spawned_process $test_spawn_id
> --
> 2.5.0.248.g1233236.dirty
>
Ping.
On 08/17/2015 02:01 PM, Patrick Palka wrote:
> We currently set attach_flag when attaching to a process, so we should
> make sure to unset it when forking a new process. Otherwise attach_flag
> would remain set after forking, if the previous process associated with
> the inferior was attached to.
>
> gdb/ChangeLog:
>
> * target.c (target_pre_inferior): Unset attach_flag.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/run-after-attach.exp: New test file.
> * gdb.base/run-after-attach.c: New test file.
OK, but,
> +set test "attempt kill via quit"
> +# The quit prompt should warn about killing the process, not about detaching the
> +# process, since this process was not attached to.
> +set ok 0
> +gdb_test_multiple "quit" $test {
> + -re "will be killed.*.y or n. $" {
> + set ok 1
> + send_gdb "n\n"
> + exp_continue
> + }
> + -re "will be detached.*.y or n. $" {
> + send_gdb "n\n"
> + exp_continue
> + }
> + -re "$gdb_prompt $" {
> + gdb_assert $ok $test
> + }
> + default {
> + fail $test
> + }
I don't think you need this default? gdb_test_multiple
handles timeout and eof itself.
> +}
> +
> +kill_wait_spawned_process $test_spawn_id
>
OK.
Thanks,
Pedro Alves
On Wed, Aug 26, 2015 at 1:06 PM, Pedro Alves <palves@redhat.com> wrote:
> On 08/17/2015 02:01 PM, Patrick Palka wrote:
>> We currently set attach_flag when attaching to a process, so we should
>> make sure to unset it when forking a new process. Otherwise attach_flag
>> would remain set after forking, if the previous process associated with
>> the inferior was attached to.
>>
>> gdb/ChangeLog:
>>
>> * target.c (target_pre_inferior): Unset attach_flag.
>>
>> gdb/testsuite/ChangeLog:
>>
>> * gdb.base/run-after-attach.exp: New test file.
>> * gdb.base/run-after-attach.c: New test file.
>
> OK, but,
>
>> +set test "attempt kill via quit"
>> +# The quit prompt should warn about killing the process, not about detaching the
>> +# process, since this process was not attached to.
>> +set ok 0
>> +gdb_test_multiple "quit" $test {
>> + -re "will be killed.*.y or n. $" {
>> + set ok 1
>> + send_gdb "n\n"
>> + exp_continue
>> + }
>> + -re "will be detached.*.y or n. $" {
>> + send_gdb "n\n"
>> + exp_continue
>> + }
>> + -re "$gdb_prompt $" {
>> + gdb_assert $ok $test
>> + }
>> + default {
>> + fail $test
>> + }
>
> I don't think you need this default? gdb_test_multiple
> handles timeout and eof itself.
Ah, yeah, that is an artifact from the previous version which used
gdb_expect. Committed with that change.
Patrick Palka <patrick@parcs.ath.cx> writes:
> We currently set attach_flag when attaching to a process, so we should
> make sure to unset it when forking a new process. Otherwise attach_flag
> would remain set after forking, if the previous process associated with
> the inferior was attached to.
>
> gdb/ChangeLog:
>
> * target.c (target_pre_inferior): Unset attach_flag.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/run-after-attach.exp: New test file.
> * gdb.base/run-after-attach.c: New test file.
> ---
> gdb/target.c | 4 ++
> gdb/testsuite/gdb.base/run-after-attach.c | 25 +++++++++++
> gdb/testsuite/gdb.base/run-after-attach.exp | 65 +++++++++++++++++++++++++++++
> 3 files changed, 94 insertions(+)
> create mode 100644 gdb/testsuite/gdb.base/run-after-attach.c
> create mode 100644 gdb/testsuite/gdb.base/run-after-attach.exp
>
> diff --git a/gdb/target.c b/gdb/target.c
> index e41a338..96e7df7 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2138,6 +2138,10 @@ target_pre_inferior (int from_tty)
> target_clear_description ();
> }
>
> + /* attach_flag may be set if the previous process associated with
> + the inferior was attached to. */
> + current_inferior ()->attach_flag = 0;
> +
> agent_capability_invalidate ();
> }
Hi.
A couple of comments for discussion's sake.
There's a lot of state in struct inferior.
Is there anything else that needs to be reset?
And how can we set things up so that future readers
don't walk away with the same question?
A function to reset struct inferior in preparation for a
"new" inferior is something I'd expect to find in the API that
inferior.[ch] should be providing to gdb.
Thoughts?
@@ -2138,6 +2138,10 @@ target_pre_inferior (int from_tty)
target_clear_description ();
}
+ /* attach_flag may be set if the previous process associated with
+ the inferior was attached to. */
+ current_inferior ()->attach_flag = 0;
+
agent_capability_invalidate ();
}
new file mode 100644
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2015 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+
+int
+main (void)
+{
+ sleep (600);
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,65 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Check that forking a process after a previous process was attached to unsets
+# attach_flag. This is done indirectly by inspecting GDB's quit prompt.
+
+if ![can_spawn_for_attach] {
+ return 0
+}
+
+standard_testfile
+set executable $testfile
+
+if [prepare_for_testing $testfile.exp $executable] {
+ return -1
+}
+
+set test_spawn_id [spawn_wait_for_attach $binfile]
+set test_pid [spawn_id_get_pid $test_spawn_id]
+
+set test "attach to process"
+gdb_test "attach $test_pid" "Attaching to program.*" $test
+
+set test "kill process"
+gdb_test "kill" "" $test \
+ "Kill the program being debugged.*y or n. $" "y"
+
+set test "restart process"
+gdb_test "start" "Starting program.*Temporary breakpoint .*" $test
+
+set test "attempt kill via quit"
+# The quit prompt should warn about killing the process, not about detaching the
+# process, since this process was not attached to.
+set ok 0
+gdb_test_multiple "quit" $test {
+ -re "will be killed.*.y or n. $" {
+ set ok 1
+ send_gdb "n\n"
+ exp_continue
+ }
+ -re "will be detached.*.y or n. $" {
+ send_gdb "n\n"
+ exp_continue
+ }
+ -re "$gdb_prompt $" {
+ gdb_assert $ok $test
+ }
+ default {
+ fail $test
+ }
+}
+
+kill_wait_spawned_process $test_spawn_id