public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/testsuite: fail if gdb_start_cmd fails
@ 2022-11-18 16:06 Simon Marchi
  2022-11-28 10:51 ` Bruno Larsen
  2022-11-28 14:34 ` Andrew Burgess
  0 siblings, 2 replies; 4+ messages in thread
From: Simon Marchi @ 2022-11-18 16:06 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

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).

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gdb/testsuite: fail if gdb_start_cmd fails
  2022-11-18 16:06 [PATCH] gdb/testsuite: fail if gdb_start_cmd fails Simon Marchi
@ 2022-11-28 10:51 ` Bruno Larsen
  2022-11-28 14:40   ` Simon Marchi
  2022-11-28 14:34 ` Andrew Burgess
  1 sibling, 1 reply; 4+ messages in thread
From: Bruno Larsen @ 2022-11-28 10:51 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 18/11/2022 17:06, Simon Marchi via Gdb-patches wrote:
> 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).
>
> Change-Id: I097370851213e798ff29fb6cf8ba25ef7d2be007

Hi Simon,

This patch makes sense to me, and testing shows no regressions.

Reviewed-By: Bruno Larsen <blarsen@redhat.com>

--
Cheers,
Bruno


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gdb/testsuite: fail if gdb_start_cmd fails
  2022-11-18 16:06 [PATCH] gdb/testsuite: fail if gdb_start_cmd fails Simon Marchi
  2022-11-28 10:51 ` Bruno Larsen
@ 2022-11-28 14:34 ` Andrew Burgess
  1 sibling, 0 replies; 4+ messages in thread
From: Andrew Burgess @ 2022-11-28 14:34 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches, gdb-patches; +Cc: Simon Marchi

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gdb/testsuite: fail if gdb_start_cmd fails
  2022-11-28 10:51 ` Bruno Larsen
@ 2022-11-28 14:40   ` Simon Marchi
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi @ 2022-11-28 14:40 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches



On 11/28/22 05:51, Bruno Larsen wrote:
> On 18/11/2022 17:06, Simon Marchi via Gdb-patches wrote:
>> 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).
>>
>> Change-Id: I097370851213e798ff29fb6cf8ba25ef7d2be007
> 
> Hi Simon,
> 
> This patch makes sense to me, and testing shows no regressions.
> 
> Reviewed-By: Bruno Larsen <blarsen@redhat.com>

Thanks to you and Andrew, will push.

Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-28 14:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-18 16:06 [PATCH] gdb/testsuite: fail if gdb_start_cmd fails Simon Marchi
2022-11-28 10:51 ` Bruno Larsen
2022-11-28 14:40   ` Simon Marchi
2022-11-28 14:34 ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).