[PATCH 1/2] gdb/testsuite: make gdb_test_multiple return immediately if send_gdb fails

[PATCH 1/2] gdb/testsuite: make gdb_test_multiple return immediately if send_gdb fails

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

In the failure seen by Philippe here:

  https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/

... the testsuite only outputs PASSes, and an ERROR, resulting from an
uncaught exception.  This is a bit sneaky, because ERRORs are not
reported in the test summary.  In certain circumstances, it can be easy
to miss.

Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes.  But
this is only if it manages to send the command, and it's that command
that crashes GDB.  Here, the ERROR is due to the fact that GDB had
already crashed by the time we entered gdb_test_multiple and tried to
send a command.  GDB was crashed by the previous "file" command, sent by
gdb_unload.  Because gdb_unload uses bare expect, it didn't record a
test failure when crashing GDB (this will be addressed separately).

In this patch, I propose to make gdb_test_multiple call unresolved
directly and return -1 send_gdb fails.  This way, if GDB is already
crashed by the time we enter gdb_test_multiple, it will leave a trace in
the test results in the form of an UNRESOLVED.  It will also spare us of
the not-so-useful-in-my-opinion TCL backtrace.

Before, it looks like:

    ERROR: Couldn't send python print(objfile.filename) to GDB.
    ERROR: : spawn id exp9 not open
        while executing
    "expect {
    -i exp9 -timeout 10
            -re ".*A problem internal to GDB has been detected" {
                fail "$message (GDB internal error)"
                gdb_internal_error..."
        ("uplevel" body line 1)
        invoked from within
    "uplevel $body" NONE : spawn id exp9 not open

And after:

    Couldn't send python print(objfile.filename) to GDB.
    UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded

Change-Id: I72af8dc0d687826fc3f76911c27a9e5f91b677ba
---
 gdb/testsuite/lib/gdb.exp | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 7d05fbe557bb..fcd54c88f251 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1161,7 +1161,9 @@ proc gdb_test_multiple { command message args } {
 	    if { $foo < [expr $len - 1] } {
 		set str [string range "$string" 0 $foo]
 		if { [send_gdb "$str"] != "" } {
-		    perror "Couldn't send $command to GDB."
+		    verbose -log "Couldn't send $command to GDB."
+		    unresolved $message
+		    return -1
 		}
 		# since we're checking if each line of the multi-line
 		# command are 'accepted' by GDB here,
@@ -1180,7 +1182,9 @@ proc gdb_test_multiple { command message args } {
 	}
 	if { "$string" != "" } {
 	    if { [send_gdb "$string"] != "" } {
-		perror "Couldn't send $command to GDB."
+		verbose -log "Couldn't send $command to GDB."
+		unresolved $message
+		return -1
 	    }
 	}
     }
-- 
2.38.1

[PATCH 2/2] gdb/testsuite: make gdb_unload use gdb_test_multiple

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

In the failure seen by Philippe here:

  https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/

gdb_unload crashed GDB, leaving no trace in the test results.  Change it
to use gdb_test_multiple, so that it leaves an UNRESOLVED result.  I
think it is good practice anyway.

Make it return the result of gdb_test_multiple directly, change
gdb.python/py-objfile.exp accordingly.

Change gdb.base/endian.exp as well to avoid duplicate test names.

Change gdb.base/gnu-debugdata.exp to avoid recording a test result,
since gdb_unload does it already now.

Change-Id: I59a1e4947691330797e6ce23277942547c437a48
---
 gdb/testsuite/gdb.base/endian.exp        |  6 ++---
 gdb/testsuite/gdb.base/gnu-debugdata.exp |  7 ++---
 gdb/testsuite/gdb.python/py-objfile.exp  |  9 ++++---
 gdb/testsuite/lib/gdb.exp                | 34 ++++++++++--------------
 4 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/gdb/testsuite/gdb.base/endian.exp b/gdb/testsuite/gdb.base/endian.exp
index 05cf1280bad0..daa49b516545 100644
--- a/gdb/testsuite/gdb.base/endian.exp
+++ b/gdb/testsuite/gdb.base/endian.exp
@@ -59,7 +59,7 @@ if { [gdb_test_multiple "show endian" "$test" {
 
 # Now check that the automatic endianness is updated
 # according to the executable selected.
-gdb_unload
+gdb_unload "unload 1"
 gdb_test "set endian big" "$en_set big endian\\." \
     "override target endianness big"
 gdb_test "set endian auto" "$en_auto \\\(currently big endian\\\)\\." \
@@ -69,7 +69,7 @@ gdb_file_cmd $binfile
 gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
     "previously big default executable endianness"
 
-gdb_unload
+gdb_unload "unload 2"
 gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
     "previously big default no executable endianness"
 gdb_test "set endian little" "$en_set little endian\\." \
@@ -81,6 +81,6 @@ gdb_file_cmd $binfile
 gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
     "previously little default executable endianness"
 
-gdb_unload
+gdb_unload "unload 3"
 gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
     "previously little default no executable endianness"
diff --git a/gdb/testsuite/gdb.base/gnu-debugdata.exp b/gdb/testsuite/gdb.base/gnu-debugdata.exp
index 732eadcbd306..0900e2fc2d86 100644
--- a/gdb/testsuite/gdb.base/gnu-debugdata.exp
+++ b/gdb/testsuite/gdb.base/gnu-debugdata.exp
@@ -150,10 +150,7 @@ if {$gdb_file_cmd_debug_info == "lzma"} {
 }
 
 # Be sure to test the 'close' method on the MiniDebugInfo BFD.
-if {[gdb_unload]} {
-    fail "unload MiniDebugInfo"
-} else {
-    pass "unload MiniDebugInfo"
-}
+# gdb_unload records a pass/fail.
+gdb_unload
 
 gdb_exit
diff --git a/gdb/testsuite/gdb.python/py-objfile.exp b/gdb/testsuite/gdb.python/py-objfile.exp
index 9565c16af96b..f42bc5fb9f0f 100644
--- a/gdb/testsuite/gdb.python/py-objfile.exp
+++ b/gdb/testsuite/gdb.python/py-objfile.exp
@@ -92,7 +92,9 @@ gdb_test "python print (objfile.progspace)" "<gdb\.Progspace object at .*>" \
   "Get objfile program space"
 gdb_test "python print (objfile.is_valid())" "True" \
   "Get objfile validity"
-gdb_unload
+
+gdb_unload "unload 1"
+
 gdb_test "python print (objfile.is_valid())" "False" \
   "Get objfile validity after unload"
 
@@ -103,9 +105,8 @@ gdb_test "python print (objfile.random_attribute)" "42" \
 
 # Verify invalid objfile handling.
 
-if { [gdb_unload] < 0 } {
-    fail "unload all files"
-    return -1
+if { [gdb_unload "unload 2"] != 0 } {
+    return
 }
 
 gdb_test "python print(objfile.filename)" "None" \
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index fcd54c88f251..264d3ff435b3 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -311,38 +311,32 @@ proc gdb_version { } {
     return [default_gdb_version]
 }
 
-#
 # gdb_unload -- unload a file if one is loaded
-# Return 0 on success, -1 on error.
 #
+# Returns the same as gdb_test_multiple.
 
-proc gdb_unload {} {
+proc gdb_unload { {msg "file"} } {
     global GDB
     global gdb_prompt
-    send_gdb "file\n"
-    gdb_expect 60 {
-	-re "No executable file now\[^\r\n\]*\[\r\n\]" { exp_continue }
-	-re "No symbol file now\[^\r\n\]*\[\r\n\]" { exp_continue }
-	-re "A program is being debugged already.*Are you sure you want to change the file.*y or n. $" {
+    return [gdb_test_multiple "file" $msg {
+	-re "A program is being debugged already.\r\nAre you sure you want to change the file. .y or n. $" {
 	    send_gdb "y\n" answer
 	    exp_continue
 	}
-	-re "Discard symbol table from .*y or n.*$" {
-	    send_gdb "y\n" answer
+
+	-re "No executable file now\\.\r\n" {
 	    exp_continue
 	}
-	-re "$gdb_prompt $" {}
-	-re "A problem internal to GDB has been detected" {
-	    perror "Couldn't unload file in $GDB (GDB internal error)."
-	    gdb_internal_error_resync
-	    return -1
+
+	-re "Discard symbol table from `.*'. .y or n. $" {
+	    send_gdb "y\n" answer
+	    exp_continue
 	}
-	timeout {
-	    perror "couldn't unload file in $GDB (timeout)."
-	    return -1
+
+	-re -wrap "No symbol file now\\." {
+	    pass $gdb_test_name
 	}
-    }
-    return 0
+    }]
 }
 
 # Many of the tests depend on setting breakpoints at various places and
-- 
2.38.1

Re: [PATCH 1/2] gdb/testsuite: make gdb_test_multiple return immediately if send_gdb fails

[Philippe Waroquiers]

Thanks for this, it will for sure help me in the future to not miss anymore
some regressions I introduce.

One small question:
Is a GDB crash the only reason for which we could have a message such as:
   Couldn't send python print(objfile.filename) to GDB.
?
Possibly better then to indicate that GDB crashed (on an internal error) ?
Or if there are some other reasons, it might be good to give more details about the
reason ?
Note that if this is not straightforward to do, not a big deal
as just making the problem visible with UNRESOLVED should draw the attention
and the detailed log will show the crash I guess.

Thanks
Philippe


On Tue, 2022-11-22 at 10:55 -0500, Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> In the failure seen by Philippe here:
> 
>   https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/
> 
> ... the testsuite only outputs PASSes, and an ERROR, resulting from an
> uncaught exception.  This is a bit sneaky, because ERRORs are not
> reported in the test summary.  In certain circumstances, it can be easy
> to miss.
> 
> Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes.  But
> this is only if it manages to send the command, and it's that command
> that crashes GDB.  Here, the ERROR is due to the fact that GDB had
> already crashed by the time we entered gdb_test_multiple and tried to
> send a command.  GDB was crashed by the previous "file" command, sent by
> gdb_unload.  Because gdb_unload uses bare expect, it didn't record a
> test failure when crashing GDB (this will be addressed separately).
> 
> In this patch, I propose to make gdb_test_multiple call unresolved
> directly and return -1 send_gdb fails.  This way, if GDB is already
> crashed by the time we enter gdb_test_multiple, it will leave a trace in
> the test results in the form of an UNRESOLVED.  It will also spare us of
> the not-so-useful-in-my-opinion TCL backtrace.
> 
> Before, it looks like:
> 
>     ERROR: Couldn't send python print(objfile.filename) to GDB.
>     ERROR: : spawn id exp9 not open
>         while executing
>     "expect {
>     -i exp9 -timeout 10
>             -re ".*A problem internal to GDB has been detected" {
>                 fail "$message (GDB internal error)"
>                 gdb_internal_error..."
>         ("uplevel" body line 1)
>         invoked from within
>     "uplevel $body" NONE : spawn id exp9 not open
> 
> And after:
> 
>     Couldn't send python print(objfile.filename) to GDB.
>     UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded
> 
> Change-Id: I72af8dc0d687826fc3f76911c27a9e5f91b677ba
> ---
>  gdb/testsuite/lib/gdb.exp | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 7d05fbe557bb..fcd54c88f251 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1161,7 +1161,9 @@ proc gdb_test_multiple { command message args } {
>  	    if { $foo < [expr $len - 1] } {
>  		set str [string range "$string" 0 $foo]
>  		if { [send_gdb "$str"] != "" } {
> -		    perror "Couldn't send $command to GDB."
> +		    verbose -log "Couldn't send $command to GDB."
> +		    unresolved $message
> +		    return -1
>  		}
>  		# since we're checking if each line of the multi-line
>  		# command are 'accepted' by GDB here,
> @@ -1180,7 +1182,9 @@ proc gdb_test_multiple { command message args } {
>  	}
>  	if { "$string" != "" } {
>  	    if { [send_gdb "$string"] != "" } {
> -		perror "Couldn't send $command to GDB."
> +		verbose -log "Couldn't send $command to GDB."
> +		unresolved $message
> +		return -1
>  	    }
>  	}
>      }

Re: [PATCH 1/2] gdb/testsuite: make gdb_test_multiple return immediately if send_gdb fails

[Simon Marchi]

On 11/23/22 05:59, Philippe Waroquiers wrote:
> Thanks for this, it will for sure help me in the future to not miss anymore
> some regressions I introduce.
> 
> One small question:
> Is a GDB crash the only reason for which we could have a message such as:
>    Couldn't send python print(objfile.filename) to GDB.
> ?
> Possibly better then to indicate that GDB crashed (on an internal error) ?
> Or if there are some other reasons, it might be good to give more details about the
> reason ?

If send_gdb fails, it's generally because the underlying send (the
expect proc) has failed, because GDB's spawn id is no longer valid.  And
that is normally due to a crash.  But the reality is that we do not know
for sure, it could also be a testsuite problem, where we closed GDB and
failed to reinitialize $gdb_spawn_id.  So saying GDB crashed could also
give you a wrong lead.  I prefer to stick with what we know.

> Note that if this is not straightforward to do, not a big deal
> as just making the problem visible with UNRESOLVED should draw the attention
> and the detailed log will show the crash I guess.

I think that's the important part, it's the previous command that caused
the crash, that one should show up in the logs.  Looking at lib/gdb.exp
there are many more uses of gdb_expect that could be converted to
gdb_test_multiple.

Simon

Re: [PATCH 1/2] gdb/testsuite: make gdb_test_multiple return immediately if send_gdb fails

[Tom de Vries]

On 11/22/22 16:55, Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> In the failure seen by Philippe here:
> 
>    https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/
> 
> ... the testsuite only outputs PASSes, and an ERROR, resulting from an
> uncaught exception.  This is a bit sneaky, because ERRORs are not
> reported in the test summary.  In certain circumstances, it can be easy
> to miss.
> 
> Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes.  But > this is only if it manages to send the command, and it's that command
> that crashes GDB.  Here, the ERROR is due to the fact that GDB had
> already crashed by the time we entered gdb_test_multiple and tried to
> send a command.  GDB was crashed by the previous "file" command, sent by
> gdb_unload.  Because gdb_unload uses bare expect, it didn't record a
> test failure when crashing GDB (this will be addressed separately).
> 
> In this patch, I propose to make gdb_test_multiple call unresolved
> directly and return -1 send_gdb fails.  This way, if GDB is already
> crashed by the time we enter gdb_test_multiple, it will leave a trace in
> the test results in the form of an UNRESOLVED.  It will also spare us of

spare us of the -> spare us the ?

> the not-so-useful-in-my-opinion TCL backtrace.
> 

Agreed.

> Before, it looks like:
> 
>      ERROR: Couldn't send python print(objfile.filename) to GDB.
>      ERROR: : spawn id exp9 not open
>          while executing
>      "expect {
>      -i exp9 -timeout 10
>              -re ".*A problem internal to GDB has been detected" {
>                  fail "$message (GDB internal error)"
>                  gdb_internal_error..."
>          ("uplevel" body line 1)
>          invoked from within
>      "uplevel $body" NONE : spawn id exp9 not open
> 
> And after:
> 
>      Couldn't send python print(objfile.filename) to GDB.
>      UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded
> 

LGTM, thanks for doing this.

Thanks,
- Tom

> Change-Id: I72af8dc0d687826fc3f76911c27a9e5f91b677ba
> ---
>   gdb/testsuite/lib/gdb.exp | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 7d05fbe557bb..fcd54c88f251 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1161,7 +1161,9 @@ proc gdb_test_multiple { command message args } {
>   	    if { $foo < [expr $len - 1] } {
>   		set str [string range "$string" 0 $foo]
>   		if { [send_gdb "$str"] != "" } {
> -		    perror "Couldn't send $command to GDB."
> +		    verbose -log "Couldn't send $command to GDB."
> +		    unresolved $message
> +		    return -1
>   		}
>   		# since we're checking if each line of the multi-line
>   		# command are 'accepted' by GDB here,
> @@ -1180,7 +1182,9 @@ proc gdb_test_multiple { command message args } {
>   	}
>   	if { "$string" != "" } {
>   	    if { [send_gdb "$string"] != "" } {
> -		perror "Couldn't send $command to GDB."
> +		verbose -log "Couldn't send $command to GDB."
> +		unresolved $message
> +		return -1
>   	    }
>   	}
>       }

Re: [PATCH 2/2] gdb/testsuite: make gdb_unload use gdb_test_multiple

[Tom de Vries]

On 11/22/22 16:55, Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> In the failure seen by Philippe here:
> 
>    https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/
> 
> gdb_unload crashed GDB, leaving no trace in the test results.  Change it
> to use gdb_test_multiple, so that it leaves an UNRESOLVED result.  I
> think it is good practice anyway.
> 

Agreed.  I do remember some cases where I had to downgrade from 
gdb_test_multiple to gdb_expect, but I guess for the common case 
gdb_test_multiple is the best.

> Make it return the result of gdb_test_multiple directly, change
> gdb.python/py-objfile.exp accordingly.
> 
> Change gdb.base/endian.exp as well to avoid duplicate test names.
> 
> Change gdb.base/gnu-debugdata.exp to avoid recording a test result,
> since gdb_unload does it already now.
> 

LGTM.

Thanks,
- Tom

> Change-Id: I59a1e4947691330797e6ce23277942547c437a48
> ---
>   gdb/testsuite/gdb.base/endian.exp        |  6 ++---
>   gdb/testsuite/gdb.base/gnu-debugdata.exp |  7 ++---
>   gdb/testsuite/gdb.python/py-objfile.exp  |  9 ++++---
>   gdb/testsuite/lib/gdb.exp                | 34 ++++++++++--------------
>   4 files changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/endian.exp b/gdb/testsuite/gdb.base/endian.exp
> index 05cf1280bad0..daa49b516545 100644
> --- a/gdb/testsuite/gdb.base/endian.exp
> +++ b/gdb/testsuite/gdb.base/endian.exp
> @@ -59,7 +59,7 @@ if { [gdb_test_multiple "show endian" "$test" {
>   
>   # Now check that the automatic endianness is updated
>   # according to the executable selected.
> -gdb_unload
> +gdb_unload "unload 1"
>   gdb_test "set endian big" "$en_set big endian\\." \
>       "override target endianness big"
>   gdb_test "set endian auto" "$en_auto \\\(currently big endian\\\)\\." \
> @@ -69,7 +69,7 @@ gdb_file_cmd $binfile
>   gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
>       "previously big default executable endianness"
>   
> -gdb_unload
> +gdb_unload "unload 2"
>   gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
>       "previously big default no executable endianness"
>   gdb_test "set endian little" "$en_set little endian\\." \
> @@ -81,6 +81,6 @@ gdb_file_cmd $binfile
>   gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
>       "previously little default executable endianness"
>   
> -gdb_unload
> +gdb_unload "unload 3"
>   gdb_test "show endian" "$en_auto \\\(currently $endian endian\\\)\\." \
>       "previously little default no executable endianness"
> diff --git a/gdb/testsuite/gdb.base/gnu-debugdata.exp b/gdb/testsuite/gdb.base/gnu-debugdata.exp
> index 732eadcbd306..0900e2fc2d86 100644
> --- a/gdb/testsuite/gdb.base/gnu-debugdata.exp
> +++ b/gdb/testsuite/gdb.base/gnu-debugdata.exp
> @@ -150,10 +150,7 @@ if {$gdb_file_cmd_debug_info == "lzma"} {
>   }
>   
>   # Be sure to test the 'close' method on the MiniDebugInfo BFD.
> -if {[gdb_unload]} {
> -    fail "unload MiniDebugInfo"
> -} else {
> -    pass "unload MiniDebugInfo"
> -}
> +# gdb_unload records a pass/fail.
> +gdb_unload
>   
>   gdb_exit
> diff --git a/gdb/testsuite/gdb.python/py-objfile.exp b/gdb/testsuite/gdb.python/py-objfile.exp
> index 9565c16af96b..f42bc5fb9f0f 100644
> --- a/gdb/testsuite/gdb.python/py-objfile.exp
> +++ b/gdb/testsuite/gdb.python/py-objfile.exp
> @@ -92,7 +92,9 @@ gdb_test "python print (objfile.progspace)" "<gdb\.Progspace object at .*>" \
>     "Get objfile program space"
>   gdb_test "python print (objfile.is_valid())" "True" \
>     "Get objfile validity"
> -gdb_unload
> +
> +gdb_unload "unload 1"
> +
>   gdb_test "python print (objfile.is_valid())" "False" \
>     "Get objfile validity after unload"
>   
> @@ -103,9 +105,8 @@ gdb_test "python print (objfile.random_attribute)" "42" \
>   
>   # Verify invalid objfile handling.
>   
> -if { [gdb_unload] < 0 } {
> -    fail "unload all files"
> -    return -1
> +if { [gdb_unload "unload 2"] != 0 } {
> +    return
>   }
>   
>   gdb_test "python print(objfile.filename)" "None" \
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index fcd54c88f251..264d3ff435b3 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -311,38 +311,32 @@ proc gdb_version { } {
>       return [default_gdb_version]
>   }
>   
> -#
>   # gdb_unload -- unload a file if one is loaded
> -# Return 0 on success, -1 on error.
>   #
> +# Returns the same as gdb_test_multiple.
>   
> -proc gdb_unload {} {
> +proc gdb_unload { {msg "file"} } {
>       global GDB
>       global gdb_prompt
> -    send_gdb "file\n"
> -    gdb_expect 60 {
> -	-re "No executable file now\[^\r\n\]*\[\r\n\]" { exp_continue }
> -	-re "No symbol file now\[^\r\n\]*\[\r\n\]" { exp_continue }
> -	-re "A program is being debugged already.*Are you sure you want to change the file.*y or n. $" {
> +    return [gdb_test_multiple "file" $msg {
> +	-re "A program is being debugged already.\r\nAre you sure you want to change the file. .y or n. $" {
>   	    send_gdb "y\n" answer
>   	    exp_continue
>   	}
> -	-re "Discard symbol table from .*y or n.*$" {
> -	    send_gdb "y\n" answer
> +
> +	-re "No executable file now\\.\r\n" {
>   	    exp_continue
>   	}
> -	-re "$gdb_prompt $" {}
> -	-re "A problem internal to GDB has been detected" {
> -	    perror "Couldn't unload file in $GDB (GDB internal error)."
> -	    gdb_internal_error_resync
> -	    return -1
> +
> +	-re "Discard symbol table from `.*'. .y or n. $" {
> +	    send_gdb "y\n" answer
> +	    exp_continue
>   	}
> -	timeout {
> -	    perror "couldn't unload file in $GDB (timeout)."
> -	    return -1
> +
> +	-re -wrap "No symbol file now\\." {
> +	    pass $gdb_test_name
>   	}
> -    }
> -    return 0
> +    }]
>   }
>   
>   # Many of the tests depend on setting breakpoints at various places and

Re: [PATCH 1/2] gdb/testsuite: make gdb_test_multiple return immediately if send_gdb fails

[Simon Marchi]

On 11/29/22 11:21, Tom de Vries wrote:
> On 11/22/22 16:55, Simon Marchi via Gdb-patches wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>> In the failure seen by Philippe here:
>>
>>    https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/
>>
>> ... the testsuite only outputs PASSes, and an ERROR, resulting from an
>> uncaught exception.  This is a bit sneaky, because ERRORs are not
>> reported in the test summary.  In certain circumstances, it can be easy
>> to miss.
>>
>> Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes.  But > this is only if it manages to send the command, and it's that command
>> that crashes GDB.  Here, the ERROR is due to the fact that GDB had
>> already crashed by the time we entered gdb_test_multiple and tried to
>> send a command.  GDB was crashed by the previous "file" command, sent by
>> gdb_unload.  Because gdb_unload uses bare expect, it didn't record a
>> test failure when crashing GDB (this will be addressed separately).
>>
>> In this patch, I propose to make gdb_test_multiple call unresolved
>> directly and return -1 send_gdb fails.  This way, if GDB is already
>> crashed by the time we enter gdb_test_multiple, it will leave a trace in
>> the test results in the form of an UNRESOLVED.  It will also spare us of
> 
> spare us of the -> spare us the ?

Fixed.

>> the not-so-useful-in-my-opinion TCL backtrace.
>>
> 
> Agreed.
> 
>> Before, it looks like:
>>
>>      ERROR: Couldn't send python print(objfile.filename) to GDB.
>>      ERROR: : spawn id exp9 not open
>>          while executing
>>      "expect {
>>      -i exp9 -timeout 10
>>              -re ".*A problem internal to GDB has been detected" {
>>                  fail "$message (GDB internal error)"
>>                  gdb_internal_error..."
>>          ("uplevel" body line 1)
>>          invoked from within
>>      "uplevel $body" NONE : spawn id exp9 not open
>>
>> And after:
>>
>>      Couldn't send python print(objfile.filename) to GDB.
>>      UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded
>>
> 
> LGTM, thanks for doing this.

Thanks, will push.

Simon

Re: [PATCH 2/2] gdb/testsuite: make gdb_unload use gdb_test_multiple

[Simon Marchi]

On 11/29/22 11:28, Tom de Vries wrote:
> On 11/22/22 16:55, Simon Marchi via Gdb-patches wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>> In the failure seen by Philippe here:
>>
>>    https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/
>>
>> gdb_unload crashed GDB, leaving no trace in the test results.  Change it
>> to use gdb_test_multiple, so that it leaves an UNRESOLVED result.  I
>> think it is good practice anyway.
>>
> 
> Agreed.  I do remember some cases where I had to downgrade from gdb_test_multiple to gdb_expect, but I guess for the common case gdb_test_multiple is the best.

Ack.

>> Make it return the result of gdb_test_multiple directly, change
>> gdb.python/py-objfile.exp accordingly.
>>
>> Change gdb.base/endian.exp as well to avoid duplicate test names.
>>
>> Change gdb.base/gnu-debugdata.exp to avoid recording a test result,
>> since gdb_unload does it already now.
>>
> 
> LGTM.

Thanks, will push.

Simon