[PATCH] Fix test suite failure in file-then-restart.exp

[PATCH] Fix test suite failure in file-then-restart.exp

[Tom Tromey]

Simon pointed out that the new file-then-restart.exp test fails with
the extended-remote target board.

The problem is that the test suite doesn't use gdb_file_cmd -- which
handles things like "set remote exec-file".  This patch changes
gdb_file_cmd to make the "kill" command optional, and then switches
the test case to use it.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30933
---
 .../boards/native-extended-gdbserver.exp      |  4 +--
 gdb/testsuite/gdb.ada/file-then-restart.exp   |  8 ++----
 gdb/testsuite/lib/gdb.exp                     | 25 +++++++++++--------
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/gdb/testsuite/boards/native-extended-gdbserver.exp b/gdb/testsuite/boards/native-extended-gdbserver.exp
index 844d032ed73..64ff3060ca2 100644
--- a/gdb/testsuite/boards/native-extended-gdbserver.exp
+++ b/gdb/testsuite/boards/native-extended-gdbserver.exp
@@ -95,8 +95,8 @@ proc extended_gdbserver_load_last_file {} {
 if { [info procs extended_gdbserver_gdb_file_cmd] == "" } {
     rename gdb_file_cmd extended_gdbserver_gdb_file_cmd
 }
-proc gdb_file_cmd { arg } {
-    if [extended_gdbserver_gdb_file_cmd $arg] {
+proc gdb_file_cmd { arg {kill_flag 1} } {
+    if [extended_gdbserver_gdb_file_cmd $arg $kill_flag] {
 	return -1
     }
     return [extended_gdbserver_load_last_file]
diff --git a/gdb/testsuite/gdb.ada/file-then-restart.exp b/gdb/testsuite/gdb.ada/file-then-restart.exp
index e2c76b666e3..6940d7f5ba2 100644
--- a/gdb/testsuite/gdb.ada/file-then-restart.exp
+++ b/gdb/testsuite/gdb.ada/file-then-restart.exp
@@ -50,12 +50,8 @@ foreach_with_prefix scenario {kill no-kill} {
 
     gdb_test_no_output "set confirm off"
 
-    if {$scenario == "kill"} {
-	gdb_test "kill" "Inferior $decimal .*killed.*"
-    }
-
-    gdb_test "file $binfile2" "Reading symbols from .*" \
-	"switch to second executable"
+    set result [gdb_file_cmd $binfile2 [expr {$scenario == "kill"}]]
+    gdb_assert {$result == 0} "switch to second executable"
 
     # Start the program a second time, GDB should land in procedure
     # Second this time.
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0a908e0af0f..63885860795 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2146,6 +2146,9 @@ proc default_gdb_exit {} {
 # Load a file into the debugger.
 # The return value is 0 for success, -1 for failure.
 #
+# ARG is the file name.
+# KILL_FLAG, if given, indicates whether a "kill" command should be used.
+#
 # This procedure also set the global variable GDB_FILE_CMD_DEBUG_INFO
 # to one of these values:
 #
@@ -2165,7 +2168,7 @@ proc default_gdb_exit {} {
 # TODO: gdb.base/sepdebug.exp and gdb.stabs/weird.exp might be able to use
 # this if they can get more information set.
 
-proc gdb_file_cmd { arg } {
+proc gdb_file_cmd { arg {kill_flag 1} } {
     global gdb_prompt
     global GDB
     global last_loaded_file
@@ -2194,15 +2197,17 @@ proc gdb_file_cmd { arg } {
     # The file command used to kill the remote target.  For the benefit
     # of the testsuite, preserve this behavior.  Mark as optional so it doesn't
     # get written to the stdin log.
-    send_gdb "kill\n" optional
-    gdb_expect 120 {
-	-re "Kill the program being debugged. .y or n. $" {
-	    send_gdb "y\n" answer
-	    verbose "\t\tKilling previous program being debugged"
-	    exp_continue
-	}
-	-re "$gdb_prompt $" {
-	    # OK.
+    if {$kill_flag} {
+	send_gdb "kill\n" optional
+	gdb_expect 120 {
+	    -re "Kill the program being debugged. .y or n. $" {
+		send_gdb "y\n" answer
+		verbose "\t\tKilling previous program being debugged"
+		exp_continue
+	    }
+	    -re "$gdb_prompt $" {
+		# OK.
+	    }
 	}
     }
 
-- 
2.40.1

Re: [PATCH] Fix test suite failure in file-then-restart.exp

[Simon Marchi]

On 10/11/23 12:21, Tom Tromey wrote:
> Simon pointed out that the new file-then-restart.exp test fails with
> the extended-remote target board.
> 
> The problem is that the test suite doesn't use gdb_file_cmd -- which
> handles things like "set remote exec-file".  This patch changes
> gdb_file_cmd to make the "kill" command optional, and then switches
> the test case to use it.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30933

Thanks, I will give the patch a try.

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 0a908e0af0f..63885860795 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -2146,6 +2146,9 @@ proc default_gdb_exit {} {
>  # Load a file into the debugger.
>  # The return value is 0 for success, -1 for failure.
>  #
> +# ARG is the file name.
> +# KILL_FLAG, if given, indicates whether a "kill" command should be used.
> +#
>  # This procedure also set the global variable GDB_FILE_CMD_DEBUG_INFO
>  # to one of these values:
>  #
> @@ -2165,7 +2168,7 @@ proc default_gdb_exit {} {
>  # TODO: gdb.base/sepdebug.exp and gdb.stabs/weird.exp might be able to use
>  # this if they can get more information set.
>  
> -proc gdb_file_cmd { arg } {
> +proc gdb_file_cmd { arg {kill_flag 1} } {
>      global gdb_prompt
>      global GDB
>      global last_loaded_file
> @@ -2194,15 +2197,17 @@ proc gdb_file_cmd { arg } {
>      # The file command used to kill the remote target.  For the benefit
>      # of the testsuite, preserve this behavior.  Mark as optional so it doesn't
>      # get written to the stdin log.

Wow, I didn't know about (or forgot) this behavior of gdb_file_cmd.
This seems like a relic of the past.  I wonder how important that kill
is, if we can get rid of it.  I would expect gdb_file_cmd to more or
less just issue the "file" command.

Simon

Re: [PATCH] Fix test suite failure in file-then-restart.exp

[Simon Marchi]

On 10/11/23 14:19, Simon Marchi wrote:
> On 10/11/23 12:21, Tom Tromey wrote:
>> Simon pointed out that the new file-then-restart.exp test fails with
>> the extended-remote target board.
>>
>> The problem is that the test suite doesn't use gdb_file_cmd -- which
>> handles things like "set remote exec-file".  This patch changes
>> gdb_file_cmd to make the "kill" command optional, and then switches
>> the test case to use it.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30933
> 
> Thanks, I will give the patch a try.
> 
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 0a908e0af0f..63885860795 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -2146,6 +2146,9 @@ proc default_gdb_exit {} {
>>  # Load a file into the debugger.
>>  # The return value is 0 for success, -1 for failure.
>>  #
>> +# ARG is the file name.
>> +# KILL_FLAG, if given, indicates whether a "kill" command should be used.
>> +#
>>  # This procedure also set the global variable GDB_FILE_CMD_DEBUG_INFO
>>  # to one of these values:
>>  #
>> @@ -2165,7 +2168,7 @@ proc default_gdb_exit {} {
>>  # TODO: gdb.base/sepdebug.exp and gdb.stabs/weird.exp might be able to use
>>  # this if they can get more information set.
>>  
>> -proc gdb_file_cmd { arg } {
>> +proc gdb_file_cmd { arg {kill_flag 1} } {
>>      global gdb_prompt
>>      global GDB
>>      global last_loaded_file
>> @@ -2194,15 +2197,17 @@ proc gdb_file_cmd { arg } {
>>      # The file command used to kill the remote target.  For the benefit
>>      # of the testsuite, preserve this behavior.  Mark as optional so it doesn't
>>      # get written to the stdin log.
> 
> Wow, I didn't know about (or forgot) this behavior of gdb_file_cmd.
> This seems like a relic of the past.  I wonder how important that kill
> is, if we can get rid of it.  I would expect gdb_file_cmd to more or
> less just issue the "file" command.

I confirm that your patch fixes the fail I see on my CI.  If we want to
get rid of the kill behavior in gdb_file_cmd, your patch is a good step
towards that, as it adds a knob for it.  So it LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon