Re: [PATCH] gdb/testsuite: fail if gdb_start_cmd fails

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> I broke gdb.ada/start.exp, and did not notice it, because it outputs an
> UNTESTED if gdb_start_cmd fails.  I don't really see when start would
> fail and it's not a problem that should be looked at.  Change all spots
> that call untested after a gdb_start_cmd failure, use fail instead.
>
> Doing so caused some failures with the native-gdbserver board.  Some
> tests that use "start" were relying on the fact that start would fail
> with that board to just return with "untested".  Change them to add an
> early return if use_gdb_stub returns true.
>
> Some gdb.pascal tests also failed with native-gdbserver, because they
> did use gdb_start_cmd to start the inferior, for no good reason.
> Convert them to use runto_main instead, which does the right thing if
> the target is a stub.
>
> A further refactoring could be to make gdb_start_cmd match the expected
> breakpoint hit and the prompt, which it doesn't do currently (it leaves
> that to the callers, but not all of them do).

LGTM too.

Thanks,
Andrew

>
> Change-Id: I097370851213e798ff29fb6cf8ba25ef7d2be007
> ---
>  gdb/testsuite/gdb.ada/exec_changed.exp       |  2 +-
>  gdb/testsuite/gdb.ada/start.exp              |  2 +-
>  gdb/testsuite/gdb.base/start-cpp.exp         |  7 ++++++-
>  gdb/testsuite/gdb.base/start.exp             |  6 +++++-
>  gdb/testsuite/gdb.base/watchpoint-hw.exp     |  6 +++++-
>  gdb/testsuite/gdb.dwarf2/main-subprogram.exp |  7 ++++++-
>  gdb/testsuite/gdb.pascal/floats.exp          | 12 ++++--------
>  gdb/testsuite/gdb.pascal/gdb11492.exp        | 12 +++++-------
>  gdb/testsuite/gdb.pascal/hello.exp           | 18 +++++-------------
>  gdb/testsuite/gdb.pascal/integers.exp        | 13 +++++--------
>  10 files changed, 43 insertions(+), 42 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.ada/exec_changed.exp b/gdb/testsuite/gdb.ada/exec_changed.exp
> index 52868f61c893..e09f59e52695 100644
> --- a/gdb/testsuite/gdb.ada/exec_changed.exp
> +++ b/gdb/testsuite/gdb.ada/exec_changed.exp
> @@ -54,7 +54,7 @@ gdb_load ${common_binfile}
>  
>  # Start the program, we should land in the program main procedure
>  if { [gdb_start_cmd] < 0 } {
> -    untested start
> +    fail start
>      return -1
>  }
>  
> diff --git a/gdb/testsuite/gdb.ada/start.exp b/gdb/testsuite/gdb.ada/start.exp
> index de080711f7b9..63dcc619f1fb 100644
> --- a/gdb/testsuite/gdb.ada/start.exp
> +++ b/gdb/testsuite/gdb.ada/start.exp
> @@ -34,7 +34,7 @@ clean_restart ${testfile}
>  
>  # Verify that "start" lands inside the right procedure.
>  if { [gdb_start_cmd] < 0 } {
> -    untested "start failed to land inside the right procedure"
> +    fail "start failed to land inside the right procedure"
>      return -1
>  }
>  
> diff --git a/gdb/testsuite/gdb.base/start-cpp.exp b/gdb/testsuite/gdb.base/start-cpp.exp
> index 9fabab4fe640..63b6ec006ca6 100644
> --- a/gdb/testsuite/gdb.base/start-cpp.exp
> +++ b/gdb/testsuite/gdb.base/start-cpp.exp
> @@ -13,6 +13,11 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> +if { [use_gdb_stub] } {
> +    unsupported "test requires running"
> +    return
> +}
> +
>  standard_testfile .cc
>  
>  if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> @@ -28,7 +33,7 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
>  
>  # For C++ programs, "start" should stop in main().
>  if { [gdb_start_cmd] < 0 } {
> -    untested start
> +    fail start
>      return -1
>  }
>  
> diff --git a/gdb/testsuite/gdb.base/start.exp b/gdb/testsuite/gdb.base/start.exp
> index 9de3db31469e..a0c8c8c41c1c 100644
> --- a/gdb/testsuite/gdb.base/start.exp
> +++ b/gdb/testsuite/gdb.base/start.exp
> @@ -13,6 +13,10 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> +if { [use_gdb_stub] } {
> +    unsupported "test requires running"
> +    return
> +}
>  
>  standard_testfile
>  
> @@ -25,7 +29,7 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
>  
>  # For C programs, "start" should stop in main().
>  if { [gdb_start_cmd] < 0 } {
> -    untested start
> +    fail start
>      return -1
>  }
>  
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw.exp b/gdb/testsuite/gdb.base/watchpoint-hw.exp
> index 23de0c3a91e9..d0fd1621efe4 100644
> --- a/gdb/testsuite/gdb.base/watchpoint-hw.exp
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw.exp
> @@ -13,6 +13,10 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> +if { [use_gdb_stub] } {
> +    unsupported "test requires running"
> +    return
> +}
>  
>  if {[skip_hw_watchpoint_tests]} {
>      return 0
> @@ -33,7 +37,7 @@ gdb_test "watch watchee" "atchpoint 1: watchee"
>  # `runto_main' or `runto main' would delete the watchpoint created above.
>  
>  if { [gdb_start_cmd] < 0 } {
> -    untested start
> +    fail start
>      return -1
>  }
>  gdb_test "" "main .* at .*" "start"
> diff --git a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp b/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
> index df302ff94281..14adf66fba97 100644
> --- a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
> +++ b/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
> @@ -19,6 +19,11 @@ if {![dwarf2_support]} {
>      return 0
>  }
>  
> +if { [use_gdb_stub] } {
> +    unsupported "test requires running"
> +    return
> +}
> +
>  standard_testfile .c -dw.S
>  
>  # Make some DWARF for the test.
> @@ -62,7 +67,7 @@ set have_index [exec_has_index_section $binfile]
>  # that this was the real "main".
>  
>  if {[gdb_start_cmd] < 0} {
> -    untested "could not start ${testfile}"
> +    fail "could not start ${testfile}"
>      return -1
>  }
>  
> diff --git a/gdb/testsuite/gdb.pascal/floats.exp b/gdb/testsuite/gdb.pascal/floats.exp
> index 32a8ea4352de..1804597e902e 100644
> --- a/gdb/testsuite/gdb.pascal/floats.exp
> +++ b/gdb/testsuite/gdb.pascal/floats.exp
> @@ -24,6 +24,10 @@ if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
>  
>  clean_restart ${testfile}
>  
> +if { ![runto_main] } {
> +    return
> +}
> +
>  set bp_location1 [gdb_get_line_number "set breakpoint 1 here"]
>  set bp_location2 [gdb_get_line_number "set breakpoint 2 here"]
>  
> @@ -34,14 +38,6 @@ if { [gdb_breakpoint ${srcfile}:${bp_location2}] } {
>      pass "setting breakpoint 2"
>  }
>  
> -# Verify that "start" lands inside the right procedure.
> -if { [gdb_start_cmd] < 0 } {
> -    untested start
> -    return -1
> -}
> -
> -gdb_test "" ".* at .*${srcfile}.*" "start"
> -
>  gdb_test "cont" "Breakpoint .*:${bp_location1}.*" "going to first breakpoint"
>  gdb_test "print r" ".* = 0" "print r before assigned to 1.25"
>  
> diff --git a/gdb/testsuite/gdb.pascal/gdb11492.exp b/gdb/testsuite/gdb.pascal/gdb11492.exp
> index 66c2de1c4a2e..84dcca75ae2d 100644
> --- a/gdb/testsuite/gdb.pascal/gdb11492.exp
> +++ b/gdb/testsuite/gdb.pascal/gdb11492.exp
> @@ -23,19 +23,17 @@ if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
>  }
>  
>  clean_restart ${testfile}
> +
> +if { ![runto_main] } {
> +    return
> +}
> +
>  set bp_location1 [gdb_get_line_number "set breakpoint 1 here"]
>  
>  if { [gdb_breakpoint ${srcfile}:${bp_location1}] } {
>      pass "setting breakpoint 1"
>  }
>  
> -# Verify that "start" lands inside the right procedure.
> -if { [gdb_start_cmd] < 0 } {
> -    untested start
> -    return -1
> -}
> -
> -gdb_test "" ".* at .*${srcfile}.*" "start"
>  gdb_test "continue" ""
>  
>  gdb_test "print integer_array" { = \{50, 51, 52, 53, 54, 55, 56, 57\}}
> diff --git a/gdb/testsuite/gdb.pascal/hello.exp b/gdb/testsuite/gdb.pascal/hello.exp
> index f7ae8ed89d01..e744045ac4fa 100644
> --- a/gdb/testsuite/gdb.pascal/hello.exp
> +++ b/gdb/testsuite/gdb.pascal/hello.exp
> @@ -22,6 +22,11 @@ if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
>  }
>  
>  clean_restart ${testfile}
> +
> +if { ![runto_main] } {
> +    return
> +}
> +
>  set bp_location1 [gdb_get_line_number "set breakpoint 1 here"]
>  set bp_location2 [gdb_get_line_number "set breakpoint 2 here"]
>  
> @@ -32,19 +37,6 @@ if { [gdb_breakpoint ${srcfile}:${bp_location2}] } {
>      pass "setting breakpoint 2"
>  }
>  
> -# Verify that "start" lands inside the right procedure.
> -if { [gdb_start_cmd] < 0 } {
> -    untested start
> -    return -1
> -}
> -
> -# This test fails for gpc
> -# because debug information for 'main'
> -# is in some <implicit code>
> -gdb_test "" \
> -         ".* at .*hello.pas.*" \
> -         "start"
> -
>  gdb_test "cont" \
>           "Breakpoint .*:${bp_location1}.*" \
>           "Going to first breakpoint"
> diff --git a/gdb/testsuite/gdb.pascal/integers.exp b/gdb/testsuite/gdb.pascal/integers.exp
> index e11335a5faec..8a2f517c6731 100644
> --- a/gdb/testsuite/gdb.pascal/integers.exp
> +++ b/gdb/testsuite/gdb.pascal/integers.exp
> @@ -22,6 +22,11 @@ if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
>  }
>  
>  clean_restart ${testfile}
> +
> +if { ![runto_main] } {
> +    return
> +}
> +
>  set bp_location1 [gdb_get_line_number "set breakpoint 1 here"]
>  set bp_location2 [gdb_get_line_number "set breakpoint 2 here"]
>  
> @@ -32,14 +37,6 @@ if { [gdb_breakpoint ${srcfile}:${bp_location2}] } {
>      pass "setting breakpoint 2"
>  }
>  
> -# Verify that "start" lands inside the right procedure.
> -if { [gdb_start_cmd] < 0 } {
> -    untested start
> -    return -1
> -}
> -
> -gdb_test "" ".* at .*${srcfile}.*" "start"
> -
>  gdb_test "cont" "Breakpoint .*:${bp_location1}.*" "going to first breakpoint"
>  
>  gdb_test "print i" ".* = 0" "print i before assigned to 1"
>
> base-commit: 6533cbeeb831224e2d2dd2a7bea54b22b798fa39
> -- 
> 2.38.1