Re: [PATCH 2/2] Test case for Bug 28308

[Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org>]

Hi Simon,

Thanks for your review.  See my responses, in line, below...

On Fri, 5 Nov 2021 12:21:23 -0400
Simon Marchi <simark@simark.ca> wrote:

> On 2021-10-01 9:00 p.m., Kevin Buettner via Gdb-patches wrote:
> > +# Make sure that there's no running gdb.  It's not clear to me
> > +# that this is necessary, but a search shows that all other uses of
> > +# gdb_spawn first do a gdb_exit.
> > +gdb_exit  
> 
> I'm not sure why that would be necessary here, as there is nothing above
> your gdb_exit that spawns GDB.  If you had used prepare_for_testing
> instead of build_executable, then that would have started GDB, and your
> gdb_exit would have been necessary (although I would have told you,
> don't use prepare_for_testing if you are to exit GDB right after, use
> build_executable :)).  So if things work without this gdb_exit (which I
> expect), let's just remove it.

Done.  (Testing shows that we don't need it.)

> > +# Create context in which the global, GDBFLAGS, will be restored at
> > +# the end of the block.  All commands run within the block are
> > +# actually run in the outer context.  (This is why 'res' is available
> > +# outside of the save_vars block.)
> > +save_vars { GDBFLAGS } {
> > +    # Set flags with which to start GDB.
> > +    append GDBFLAGS " -ex \"set height unlimited\""
> > +    append GDBFLAGS " -x \"$x_file\""
> > +    append GDBFLAGS " --args \"$binfile\""
> > +
> > +    # Start GDB with above flags.
> > +    set res [gdb_spawn]
> > +}
> > +
> > +set test "Load and run script with -x"  
> 
> Non-capital "l".

Fixed.

> > +if { $res != 0} {
> > +    fail $test
> > +    return -1
> > +}
> > +
> > +# The script loaded via -x contains a run command; while running, GDB
> > +# is expected to print three messages from dprintf breakpoints along
> > +# with three interspersed messages from an ordinary breakpoint (which
> > +# was set up with a continue command).  Set up pattern D to match
> > +# output from hitting the dprintf breakpoint and B for the ordinary
> > +# breakpoint.  Then set PAT to contain the entire pattern of expected
> > +# output from the interspersed dprintf and ordinary breakpoints along
> > +# with some (additional) expected output from the dprintf breakpoints,
> > +# i.e. 0, 1, and 2.
> > +set d "dprintf in increment.., vi="
> > +set b "Breakpoint ., inc_vi"
> > +set pat "${d}0.*?$b.*?${d}1.*?$b.*?${d}2.*?$b.*?"
> > +
> > +proc do_test {cmd test} {
> > +    gdb_test_multiple $cmd $test {
> > +	-re "$::pat$::inferior_exited_re normally.*$::gdb_prompt $" {
> > +	    pass $test
> > +	}
> > +	-re "Don't know how to run.*$::gdb_prompt $" {
> > +	    unsupported $test
> > +	}  
> 
> Given you used target_can_use_run_cmd at the top, when would this
> unsupported path be taken?

It won't be.  I've rewritten it to use gdb_test instead.

> > +    }
> > +}
> > +
> > +# Check output from running script with -x
> > +do_test "" $test
> > +
> > +# Restart GDB and 'source' the script; this will (still) run the program
> > +# due to the 'run' command in the script.
> > +clean_restart $binfile
> > +do_test "source $x_file" "Load and run script using source command"  
> 
> Non-capital "l".

Fixed.

> > +
> > +# This should leave us at the gdb prompt; Run program again using
> > +# already established breakpoints, i.e. those loaded from the
> > +# script.  Prior to fixing PR 28308, this was the only test that
> > +# would pass.
> > +do_test "run" "Run again"  
> 
> Non-capital "r".

Fixed.

This is what the dprintf-execution-x-script.exp part of the patch looks
like now:

diff --git a/gdb/testsuite/gdb.base/dprintf-execution-x-script.exp b/gdb/testsuite/gdb.base/dprintf-execution-x-script.exp
new file mode 100644
index 00000000000..9133af752be
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-execution-x-script.exp
@@ -0,0 +1,85 @@
+# Copyright 2021 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 commands in a GDB script file run via GDB's -x flag work
+# as expected.  Specifically, the script creates a dprintf breakpoint
+# as well as a normal breakpoint that has "continue" in its command
+# list, and then does "run".  Correct output from GDB is checked as
+# part of this test.
+
+# Bail out if the target can't use the 'run' command.
+if ![target_can_use_run_cmd] {
+    return 0
+}
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# This is the name of the GDB script to load.
+set x_file ${srcdir}/${subdir}/$testfile.gdb
+
+# Create context in which the global, GDBFLAGS, will be restored at
+# the end of the block.  All commands run within the block are
+# actually run in the outer context.  (This is why 'res' is available
+# outside of the save_vars block.)
+save_vars { GDBFLAGS } {
+    # Set flags with which to start GDB.
+    append GDBFLAGS " -ex \"set height unlimited\""
+    append GDBFLAGS " -x \"$x_file\""
+    append GDBFLAGS " --args \"$binfile\""
+
+    # Start GDB with above flags.
+    set res [gdb_spawn]
+}
+
+set test "load and run script with -x"
+if { $res != 0} {
+    fail $test
+    return -1
+}
+
+# The script loaded via -x contains a run command; while running, GDB
+# is expected to print three messages from dprintf breakpoints along
+# with three interspersed messages from an ordinary breakpoint (which
+# was set up with a continue command).  Set up pattern D to match
+# output from hitting the dprintf breakpoint and B for the ordinary
+# breakpoint.  Then set PAT to contain the entire pattern of expected
+# output from the interspersed dprintf and ordinary breakpoints along
+# with some (additional) expected output from the dprintf breakpoints,
+# i.e. 0, 1, and 2.
+set d "dprintf in increment.., vi="
+set b "Breakpoint ., inc_vi"
+set pat "${d}0.*?$b.*?${d}1.*?$b.*?${d}2.*?$b.*?"
+
+proc do_test {cmd test} {
+    gdb_test $cmd "$::pat$::inferior_exited_re normally.*" $test
+}
+
+# Check output from running script with -x
+do_test "" $test
+
+# Restart GDB and 'source' the script; this will (still) run the program
+# due to the 'run' command in the script.
+clean_restart $binfile
+do_test "source $x_file" "load and run script using source command"
+
+# This should leave us at the gdb prompt; Run program again using
+# already established breakpoints, i.e. those loaded from the
+# script.  Prior to fixing PR 28308, this was the only test that
+# would pass.
+do_test "run" "run again"

I plan to push this (fixed up) commit along with the commit which
fixes Bug 28308.

Kevin