[PATCH] Unset attach_flag when running a new process

[PATCH] Unset attach_flag when running a new process

[Patrick Palka]

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.

[ The test is currently magically failing on extended-gdbserver.  The
  failure is:

(gdb) PASS: gdb.base/run-after-attach.exp: kill process
start
`target:/scratchpad/binutils-gdb-build/gdb/testsuite/gdb.base/run-after-attach' has disappeared; keeping its symbols.
Temporary breakpoint 1 at 0x4005aa: file /home/patrick/code/binutils-gdb/gdb/testsuite/gdb.base/run-after-attach.c, line 23.
Starting program: target:/scratchpad/binutils-gdb-build/gdb/testsuite/gdb.base/run-after-attach
Running the default executable on the remote target failed; try "set remote exec-file"?

  I do not know why restarting the program fails.  Is this a bug or a mistake on my part?

  Also, why is [is_remote target] true for gdbserver but not for extended-gdbserver?  ]

gdb/ChangeLog:

	* gdb/infcmd.c (run_comand_1): 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/infcmd.c                                |  4 ++
 gdb/testsuite/gdb.base/run-after-attach.c   | 25 ++++++++++
 gdb/testsuite/gdb.base/run-after-attach.exp | 73 +++++++++++++++++++++++++++++
 3 files changed, 102 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/infcmd.c b/gdb/infcmd.c
index 4948d27..bf8b94b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -601,6 +601,10 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
   /* Done with ARGS.  */
   do_cleanups (args_chain);
 
+  /* Unset attach_flag, it may be set if the previous process associated with
+     the inferior was attached to.  */
+  current_inferior ()->attach_flag = 0;
+
   /* We call get_inferior_args() because we might need to compute
      the value now.  */
   run_target->to_create_inferior (run_target, exec_file, get_inferior_args (),
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..19817f7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/run-after-attach.exp
@@ -0,0 +1,73 @@
+# 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 resets
+# 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 { [build_executable $testfile.exp $executable "" debug] == -1 } {
+    return -1
+}
+
+set testpid [spawn_wait_for_attach $binfile]
+
+gdb_start
+
+set test "attach to process"
+gdb_test "attach $testpid" "Attaching to process $testpid.*" $test
+
+set test "kill process"
+send_gdb "kill\n"
+gdb_expect {
+    -re ".y or n. $" {
+	send_gdb "y\n"
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	pass $test
+    }
+    default {
+	fail $test
+    }
+}
+
+set test "restart process"
+gdb_test "start" "Temporary breakpoint .*" $test
+
+set test "attempt kill via quit"
+send_gdb "quit\n"
+# The quit prompt should warn about killing the process, not about detaching the
+# process, since this process was not attached to.
+gdb_expect {
+    -re "will be killed.*.y or n. $" {
+	pass $test
+	send_gdb "n\n"
+    }
+    -re "will be detached.*.y or n. $" {
+	fail $test
+	send_gdb "n\n"
+    }
+    default {
+	fail $test
+    }
+}
+
+remote_exec host "kill $testpid"
-- 
2.5.0.rc2.22.g69b1679.dirty

[PATCH] Unset attach_flag when running a new process

[Patrick Palka]

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.

[ I fixed the extended-gdbserver test failure by using
  prepare_for_testing instead of using build_executable + gdb_start.  The
  former sets remote exec-file which is what makes the "run" command work as
  expected under extended-gdbserver.  ]

gdb/ChangeLog:

	* gdb/infcmd.c (run_comand_1): 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/infcmd.c                                |  4 ++
 gdb/testsuite/gdb.base/run-after-attach.c   | 25 +++++++++
 gdb/testsuite/gdb.base/run-after-attach.exp | 80 +++++++++++++++++++++++++++++
 3 files changed, 109 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/infcmd.c b/gdb/infcmd.c
index 4948d27..bf8b94b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -601,6 +601,10 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
   /* Done with ARGS.  */
   do_cleanups (args_chain);
 
+  /* Unset attach_flag, it may be set if the previous process associated with
+     the inferior was attached to.  */
+  current_inferior ()->attach_flag = 0;
+
   /* We call get_inferior_args() because we might need to compute
      the value now.  */
   run_target->to_create_inferior (run_target, exec_file, get_inferior_args (),
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..5e4ef49
--- /dev/null
+++ b/gdb/testsuite/gdb.base/run-after-attach.exp
@@ -0,0 +1,80 @@
+# 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 testpid [spawn_wait_for_attach $binfile]
+
+set test "attach to process"
+gdb_test "attach $testpid" "Attaching to program.*" $test
+
+set test "kill process"
+send_gdb "kill\n"
+gdb_expect {
+    -re ".y or n. $" {
+	send_gdb "y\n"
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	pass $test
+    }
+    default {
+	fail $test
+    }
+}
+
+set test "restart process"
+gdb_test "start" "Starting program.*Temporary breakpoint .*" $test
+
+set test "attempt kill via quit"
+send_gdb "quit\n"
+set ok 0
+# The quit prompt should warn about killing the process, not about detaching the
+# process, since this process was not attached to.
+gdb_expect {
+    -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 $" {
+	if $ok {
+	    pass $test
+	} else {
+	    fail $test
+	}
+    }
+    default {
+	fail $test
+    }
+}
+
+remote_exec host "kill $testpid"
-- 
2.5.0.rc2.22.g69b1679.dirty

Re: [PATCH] Unset attach_flag when running a new process

[Yao Qi]

On 30/07/15 16:28, Patrick Palka wrote:
> +  /* Unset attach_flag, it may be set if the previous process associated with
> +     the inferior was attached to.  */
> +  current_inferior ()->attach_flag = 0;

It is better to do such reset in target.c:target_pre_inferior, which
is created for such purpose, as far as I know.

I don't read the test part of this patch.

-- 
Yao (齐尧)

Re: [PATCH] Unset attach_flag when running a new process

[Patrick Palka]

On Thu, Jul 30, 2015 at 12:24 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On 30/07/15 16:28, Patrick Palka wrote:
>>
>> +  /* Unset attach_flag, it may be set if the previous process associated
>> with
>> +     the inferior was attached to.  */
>> +  current_inferior ()->attach_flag = 0;
>
>
> It is better to do such reset in target.c:target_pre_inferior, which
> is created for such purpose, as far as I know.

That makes sense, although I am a bit hesitant about such a change,
since target_pre_inferior has a lot of callers.  Is it OK, for
instance, to assume that current_inferior points to the right inferior
when target_pre_inferior gets called?  Currently current_inferior is
not called directly or indirectly from target_pre_inferior.

I wonder how this attach_flag issues manifests itself elsewhere, if at all.

Re: [PATCH] Unset attach_flag when running a new process

[Patrick Palka]

On Thu, Jul 30, 2015 at 11:28 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.
>
> [ I fixed the extended-gdbserver test failure by using
>   prepare_for_testing instead of using build_executable + gdb_start.  The
>   former sets remote exec-file which is what makes the "run" command work as
>   expected under extended-gdbserver.  ]
>
> gdb/ChangeLog:
>
>         * gdb/infcmd.c (run_comand_1): Unset attach_flag.
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.base/run-after-attach.exp: New test file.
>         * gdb.base/run-after-attach.c: New test file.

Ping.

Re: [PATCH] Unset attach_flag when running a new process

[Pedro Alves]

On 08/02/2015 09:20 PM, Patrick Palka wrote:
> On Thu, Jul 30, 2015 at 12:24 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
>> On 30/07/15 16:28, Patrick Palka wrote:
>>>
>>> +  /* Unset attach_flag, it may be set if the previous process associated
>>> with
>>> +     the inferior was attached to.  */
>>> +  current_inferior ()->attach_flag = 0;
>>
>>
>> It is better to do such reset in target.c:target_pre_inferior, which
>> is created for such purpose, as far as I know.
> 
> That makes sense, although I am a bit hesitant about such a change,
> since target_pre_inferior has a lot of callers.  

You mean, the whole lot of 3 callers.  :-)

infcmd.c:542:  target_pre_inferior (from_tty);
infcmd.c:2603:  target_pre_inferior (from_tty);
target.c:2191:  target_pre_inferior (from_tty);

> Is it OK, for
> instance, to assume that current_inferior points to the right inferior
> when target_pre_inferior gets called?  Currently current_inferior is
> not called directly or indirectly from target_pre_inferior.

Should be.  It's called from the run command, and from the
attach command.  Those assume that the current inferior is
the inferior that we're about to run/attach.

The call in target_preopen is more dubious.  That should probably
be iterating over all inferiors and calling target_pre_inferior
on each.  For the case where you have multiple inferiors, and then
connect to a remote target with "target remote".



> +set test "kill process"
> +send_gdb "kill\n"
> +gdb_expect {
> +    -re ".y or n. $" {
> +	send_gdb "y\n"
> +	exp_continue
> +    }
> +    -re "$gdb_prompt $" {
> +	pass $test
> +    }
> +    default {
> +	fail $test
> +    }
> +}

Avoid raw gdb_expect.  You can write:

        gdb_test "kill" \
            "" \
            "kill process" \
            "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"
> +send_gdb "quit\n"
> +set ok 0
> +# The quit prompt should warn about killing the process, not about detaching the
> +# process, since this process was not attached to.
> +gdb_expect {

Please use gdb_test_multiple.  That catches internal errors and such.

> +    -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 $" {
> +	if $ok {
> +	    pass $test
> +	} else {
> +	    fail $test
> +	}

gdb_assert $ok $test


> +    }
> +    default {
> +	fail $test
> +    }
> +}
> +
> +remote_exec host "kill $testpid"
> 

Thanks,
Pedro Alves

Re: [PATCH] Unset attach_flag when running a new process

[Patrick Palka]

On Thu, Aug 13, 2015 at 12:30 PM, Pedro Alves <palves@redhat.com> wrote:
> On 08/02/2015 09:20 PM, Patrick Palka wrote:
>> On Thu, Jul 30, 2015 at 12:24 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
>>> On 30/07/15 16:28, Patrick Palka wrote:
>>>>
>>>> +  /* Unset attach_flag, it may be set if the previous process associated
>>>> with
>>>> +     the inferior was attached to.  */
>>>> +  current_inferior ()->attach_flag = 0;
>>>
>>>
>>> It is better to do such reset in target.c:target_pre_inferior, which
>>> is created for such purpose, as far as I know.
>>
>> That makes sense, although I am a bit hesitant about such a change,
>> since target_pre_inferior has a lot of callers.
>
> You mean, the whole lot of 3 callers.  :-)
>
> infcmd.c:542:  target_pre_inferior (from_tty);
> infcmd.c:2603:  target_pre_inferior (from_tty);
> target.c:2191:  target_pre_inferior (from_tty);

The thing is that one of the callers, target_preopen, itself has lots
of callers all over the place.

>
>> Is it OK, for
>> instance, to assume that current_inferior points to the right inferior
>> when target_pre_inferior gets called?  Currently current_inferior is
>> not called directly or indirectly from target_pre_inferior.
>
> Should be.  It's called from the run command, and from the
> attach command.  Those assume that the current inferior is
> the inferior that we're about to run/attach.

Yeah, those are obviously fine.

>
> The call in target_preopen is more dubious.  That should probably
> be iterating over all inferiors and calling target_pre_inferior
> on each.  For the case where you have multiple inferiors, and then
> connect to a remote target with "target remote".

I see.  I will move the call to target_pre_inferior then.

>
>
>
>> +set test "kill process"
>> +send_gdb "kill\n"
>> +gdb_expect {
>> +    -re ".y or n. $" {
>> +     send_gdb "y\n"
>> +     exp_continue
>> +    }
>> +    -re "$gdb_prompt $" {
>> +     pass $test
>> +    }
>> +    default {
>> +     fail $test
>> +    }
>> +}
>
> Avoid raw gdb_expect.  You can write:
>
>         gdb_test "kill" \
>             "" \
>             "kill process" \
>             "Kill the program being debugged.*y or n. $" \
>             "y"

Cool, didn't know about that.

>
>> +
>> +set test "restart process"
>> +gdb_test "start" "Starting program.*Temporary breakpoint .*" $test
>> +
>> +set test "attempt kill via quit"
>> +send_gdb "quit\n"
>> +set ok 0
>> +# The quit prompt should warn about killing the process, not about detaching the
>> +# process, since this process was not attached to.
>> +gdb_expect {
>
> Please use gdb_test_multiple.  That catches internal errors and such.

Hmm, I wonder why I didn't use gdb_test_multiple in the first place.
Probably for no good reason.  I'll make this change then.

>
>> +    -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 $" {
>> +     if $ok {
>> +         pass $test
>> +     } else {
>> +         fail $test
>> +     }
>
> gdb_assert $ok $test

Done.

>
>
>> +    }
>> +    default {
>> +     fail $test
>> +    }
>> +}
>> +
>> +remote_exec host "kill $testpid"
>>
>
> Thanks,
> Pedro Alves
>

[PATCH] Unset attach_flag when running a new process

[Patrick Palka]

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

Re: [PATCH] Unset attach_flag when running a new process

[Patrick Palka]

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.

Re: [PATCH] Unset attach_flag when running a new process

[Pedro Alves]

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

Re: [PATCH] Unset attach_flag when running a new process

[Patrick Palka]

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.

Re: [PATCH] Unset attach_flag when running a new process

[Doug Evans]

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?