[PATCH] PR17525 - breakpoint commands not executed when program run, from -x script

[PATCH] PR17525 - breakpoint commands not executed when program run, from -x script

[Pedro Alves]

Executing a gdb script that runs the inferior (from the command line
with -x), and has it hit breakpoints with breakpoint commands that
themselves run the target, is currently broken on async targets
(Linux, remote).

While we're executing a command list or a script, we force the
interpreter to be sync, which results in some functions nesting an
event loop and waiting for the target to stop, instead of returning
immediately and having the top level event loop handle the stop.

The issue with this bug is simply that bpstat_do_actions misses
checking whether the interpreter is sync.  When we get here, in the
case of executing a script (or, when the interpreter is sync), the
program has already advanced to the next breakpoint, through
maybe_wait_sync_command_done.  We need to process its breakpoints
immediately, just like with a sync target.

Tested on x86_64 Fedora 20.

gdb/
2015-01-12  Pedro Alves  <palves@redhat.com>

	PR gdb/17525
	* breakpoint.c: Include "interps.h".
	(bpstat_do_actions_1): Also check whether the interpreter is
	async.

gdb/testsuite/
2015-01-12  Pedro Alves  <palves@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	PR gdb/17525
	* gdb.base/bp-cmds-execution-x-script.c: New file.
	* gdb.base/bp-cmds-execution-x-script.exp: New file.
	* gdb.base/bp-cmds-execution-x-script.gdb: New file.
---
 gdb/breakpoint.c                                   |  4 +-
 .../gdb.base/bp-cmds-execution-x-script.c          | 35 ++++++++++
 .../gdb.base/bp-cmds-execution-x-script.exp        | 80 ++++++++++++++++++++++
 .../gdb.base/bp-cmds-execution-x-script.gdb        | 21 ++++++
 4 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c
 create mode 100644 gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp
 create mode 100644 gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c64c2a9..af0c2cf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -66,7 +66,7 @@
 #include "skip.h"
 #include "ax-gdb.h"
 #include "dummy-frame.h"
-
+#include "interps.h"
 #include "format.h"
 
 /* readline include files */
@@ -4635,7 +4635,7 @@ bpstat_do_actions_1 (bpstat *bsp)
 
       if (breakpoint_proceeded)
 	{
-	  if (target_can_async_p ())
+	  if (interpreter_async && target_can_async_p ())
 	    /* If we are in async mode, then the target might be still
 	       running, not stopped at any breakpoint, so nothing for
 	       us to do here -- just return to the event loop.  */
diff --git a/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c
new file mode 100644
index 0000000..f7101b6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c
@@ -0,0 +1,35 @@
+/* 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/>.  */
+
+int
+increment (int i)
+{
+  return i + 1;
+}
+
+#define N_INCR 20
+
+int
+main (void)
+{
+  int val = 1;
+  int j;
+
+  for (j = 0; j < N_INCR; j++)
+    val = increment (val);
+  return val == N_INCR;
+}
diff --git a/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp
new file mode 100644
index 0000000..19c9f1a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp
@@ -0,0 +1,80 @@
+# 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/>.  */
+
+# Test that GDB isn't silent if it fails to remove a breakpoint from
+# the main program, independently of whether the program was loaded
+# with "file PROGRAM" or directly from the command line with "gdb
+# PROGRAM".
+
+# Test that execution commands in a GDB script file run through -x
+# work as expected.  Specifically, the scripts creates a breakpoint
+# that has "continue" in its command list, and then does "run".
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+set x_file ${srcdir}/${subdir}/$testfile.gdb
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+
+# Run the test proper.
+
+proc test {} {
+    global inferior_exited_re
+    global binfile
+    global gdb_prompt
+    global saved_gdbflags GDBFLAGS
+    global x_file
+
+    gdb_exit
+
+    set GDBFLAGS "$GDBFLAGS -x $x_file $binfile"
+
+    set GDBFLAGS $saved_gdbflags
+    append GDBFLAGS " -ex \"set height unlimited\""
+    append GDBFLAGS " -x \"$x_file\""
+    append GDBFLAGS " --args \"$binfile\""
+
+    set test "run to end"
+
+    gdb_exit
+    set res [gdb_spawn]
+    if { $res != 0} {
+	fail $test
+	return -1
+    }
+
+    set res -1
+    gdb_test_multiple "" $test {
+	-re "$inferior_exited_re normally.*$gdb_prompt $" {
+	    pass $test
+	}
+	-re "Don't know how to run.*$gdb_prompt $" {
+	    set res 0
+	    unsupported $test
+	}
+    }
+
+    # In case the board file wants to send further commands.
+    return $res
+}
+
+test
+
+set GDBFLAGS $saved_gdbflags
diff --git a/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb
new file mode 100644
index 0000000..b49c346
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb
@@ -0,0 +1,21 @@
+# 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/>.  */
+
+set pagination off
+break increment
+commands
+   cont
+end
+run
-- 
1.9.3

Re: [PATCH] PR17525 - breakpoint commands not executed when program run, from -x script

[Joel Brobecker]

Hi Pedro,

> gdb/
> 2015-01-12  Pedro Alves  <palves@redhat.com>
> 
> 	PR gdb/17525
> 	* breakpoint.c: Include "interps.h".
> 	(bpstat_do_actions_1): Also check whether the interpreter is
> 	async.
> 
> gdb/testsuite/
> 2015-01-12  Pedro Alves  <palves@redhat.com>
> 	    Joel Brobecker  <brobecker@adacore.com>
> 
> 	PR gdb/17525
> 	* gdb.base/bp-cmds-execution-x-script.c: New file.
> 	* gdb.base/bp-cmds-execution-x-script.exp: New file.
> 	* gdb.base/bp-cmds-execution-x-script.gdb: New file.

Many many thanks for the fix.

I completely missed the fact that we were running in non-async
mode, and now I am slapping the front of my head yelling a "duh!".
I learnt something from your help, so thank you. And the bonus
is that the fix looks fairly simple, which is good, because...

... this is an issue which was identified as a regression for
7.8 and 7.9. I suggest we backport them to both branches, if
you agree, and once the patch is pushed to master, I volunteer
doing so if that saves you a bit of time.

Regarding GDBFLAGS, I'm wondering whether this might be one of
those globals that we should be restoring at the start of each
testcase (via gdb_init)...

-- 
Joel

Re: [PATCH] PR17525 - breakpoint commands not executed when program run, from -x script

[Pedro Alves]

On 01/14/2015 11:29 AM, Joel Brobecker wrote:

> ... this is an issue which was identified as a regression for
> 7.8 and 7.9. I suggest we backport them to both branches, if
> you agree, and once the patch is pushed to master, I volunteer
> doing so if that saves you a bit of time.

Nah, I already needed to push
  https://sourceware.org/ml/gdb-patches/2015-01/msg00372.html
to the branch, and as this one is going to 7.8, I pushed
that one in as well, as it's kind of related.

Below's what I pushed to master, 7.9 and 7.8.  I removed a
couple of unnecessary lines from the .exp and .gdb files.

> Regarding GDBFLAGS, I'm wondering whether this might be one of
> those globals that we should be restoring at the start of each
> testcase (via gdb_init)...

Hmm, that might be a good idea.

----------------
[PATCH] PR17525 - breakpoint commands not executed when program run from -x script

Executing a gdb script that runs the inferior (from the command line
with -x), and has it hit breakpoints with breakpoint commands that
themselves run the target, is currently broken on async targets
(Linux, remote).

While we're executing a command list or a script, we force the
interpreter to be sync, which results in some functions nesting an
event loop and waiting for the target to stop, instead of returning
immediately and having the top level event loop handle the stop.

The issue with this bug is simply that bpstat_do_actions misses
checking whether the interpreter is sync.  When we get here, in the
case of executing a script (or, when the interpreter is sync), the
program has already advanced to the next breakpoint, through
maybe_wait_sync_command_done.  We need to process its breakpoints
immediately, just like with a sync target.

Tested on x86_64 Fedora 20.

gdb/
2015-01-14  Pedro Alves  <palves@redhat.com>

	PR gdb/17525
	* breakpoint.c: Include "interps.h".
	(bpstat_do_actions_1): Also check whether the interpreter is
	async.

gdb/testsuite/
2015-01-14  Pedro Alves  <palves@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	PR gdb/17525
	* gdb.base/bp-cmds-execution-x-script.c: New file.
	* gdb.base/bp-cmds-execution-x-script.exp: New file.
	* gdb.base/bp-cmds-execution-x-script.gdb: New file.
---
 gdb/ChangeLog                                      |  7 ++
 gdb/testsuite/ChangeLog                            |  8 +++
 gdb/breakpoint.c                                   |  4 +-
 .../gdb.base/bp-cmds-execution-x-script.c          | 35 ++++++++++
 .../gdb.base/bp-cmds-execution-x-script.exp        | 75 ++++++++++++++++++++++
 .../gdb.base/bp-cmds-execution-x-script.gdb        | 20 ++++++
 6 files changed, 147 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c
 create mode 100644 gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp
 create mode 100644 gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b84ffc8..648b341 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2015-01-14  Pedro Alves  <palves@redhat.com>

+	PR gdb/17525
+	* breakpoint.c: Include "interps.h".
+	(bpstat_do_actions_1): Also check whether the interpreter is
+	async.
+
+2015-01-14  Pedro Alves  <palves@redhat.com>
+
 	PR cli/17828
 	* infrun.c (reinstall_readline_callback_handler_cleanup): Don't
 	reinstall if the interpreter is sync.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 7e74501..2f747b5 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,4 +1,12 @@
 2015-01-14  Pedro Alves  <palves@redhat.com>
+	    Joel Brobecker  <brobecker@adacore.com>
+
+	PR gdb/17525
+	* gdb.base/bp-cmds-execution-x-script.c: New file.
+	* gdb.base/bp-cmds-execution-x-script.exp: New file.
+	* gdb.base/bp-cmds-execution-x-script.gdb: New file.
+
+2015-01-14  Pedro Alves  <palves@redhat.com>

 	PR cli/17828
 	* gdb.base/batch-preserve-term-settings.c: New file.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c64c2a9..af0c2cf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -66,7 +66,7 @@
 #include "skip.h"
 #include "ax-gdb.h"
 #include "dummy-frame.h"
-
+#include "interps.h"
 #include "format.h"

 /* readline include files */
@@ -4635,7 +4635,7 @@ bpstat_do_actions_1 (bpstat *bsp)

       if (breakpoint_proceeded)
 	{
-	  if (target_can_async_p ())
+	  if (interpreter_async && target_can_async_p ())
 	    /* If we are in async mode, then the target might be still
 	       running, not stopped at any breakpoint, so nothing for
 	       us to do here -- just return to the event loop.  */
diff --git a/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c
new file mode 100644
index 0000000..f7101b6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.c
@@ -0,0 +1,35 @@
+/* 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/>.  */
+
+int
+increment (int i)
+{
+  return i + 1;
+}
+
+#define N_INCR 20
+
+int
+main (void)
+{
+  int val = 1;
+  int j;
+
+  for (j = 0; j < N_INCR; j++)
+    val = increment (val);
+  return val == N_INCR;
+}
diff --git a/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp
new file mode 100644
index 0000000..e75a4fd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.exp
@@ -0,0 +1,75 @@
+# 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/>.  */
+
+# Test that GDB isn't silent if it fails to remove a breakpoint from
+# the main program, independently of whether the program was loaded
+# with "file PROGRAM" or directly from the command line with "gdb
+# PROGRAM".
+
+# Test that execution commands in a GDB script file run through -x
+# work as expected.  Specifically, the scripts creates a breakpoint
+# that has "continue" in its command list, and then does "run".
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+set x_file ${srcdir}/${subdir}/$testfile.gdb
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+
+# Run the test proper.
+
+proc test {} {
+    global inferior_exited_re
+    global binfile
+    global gdb_prompt
+    global saved_gdbflags GDBFLAGS
+    global x_file
+
+    gdb_exit
+
+    set GDBFLAGS "$GDBFLAGS -x $x_file $binfile"
+
+    set GDBFLAGS $saved_gdbflags
+    append GDBFLAGS " -ex \"set height unlimited\""
+    append GDBFLAGS " -x \"$x_file\""
+    append GDBFLAGS " --args \"$binfile\""
+
+    set test "run to end"
+
+    gdb_exit
+    set res [gdb_spawn]
+    if { $res != 0} {
+	fail $test
+	return -1
+    }
+
+    gdb_test_multiple "" $test {
+	-re "$inferior_exited_re normally.*$gdb_prompt $" {
+	    pass $test
+	}
+	-re "Don't know how to run.*$gdb_prompt $" {
+	    unsupported $test
+	}
+    }
+}
+
+test
+
+set GDBFLAGS $saved_gdbflags
diff --git a/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb
new file mode 100644
index 0000000..6d5b4e4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bp-cmds-execution-x-script.gdb
@@ -0,0 +1,20 @@
+# 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/>.  */
+
+break increment
+commands
+   cont
+end
+run
-- 
1.9.3