[binutils-gdb] [gdb/testsuite] Reduce errors after gdb exit in default_gdb_start

[binutils-gdb] [gdb/testsuite] Reduce errors after gdb exit in default_gdb_start

[Tom de Vries]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2016d3e60f871ea77fb089b5bc7bcfacffab1eab

commit 2016d3e60f871ea77fb089b5bc7bcfacffab1eab
Author: Tom de Vries <tdevries@suse.de>
Date:   Fri Apr 24 12:21:49 2020 +0200

    [gdb/testsuite] Reduce errors after gdb exit in default_gdb_start
    
    When running test-case gdb.base/readnever.exp with target board readnow, and
    without commit 96038148d0e "[gdb/testsuite] Skip gdb.base/readnever.exp with
    target board readnow", we run into a bunch of errors, starting with:
    ...
    spawn gdb -nw -nx -data-directory data-directory -ex set sysroot -readnow \
      --readnever^M
    gdb: '--readnow' and '--readnever' cannot be specified simultaneously^M
    ERROR: : spawn id exp9 not open
        while executing
    "expect {
    -i exp9 -timeout 10
            -re "$gdb_prompt $" {
                verbose "Setting height to 0." 2
            }
    ...
    
    The illegal combination of --readnow and --readnever causes gdb to start,
    print an error message and exit.  There's a gdb_expect in default_gdb_start
    that is supposed to detect the initial gdb prompt and handle related problems,
    but since there's no eof case it succeeds, and default_gdb_start continues as
    if the gdb prompt had been detected, causing the error above.
    
    Fix this by adding an eof case to the gdb_expect, such that we have the more
    accurate:
    ...
    ERROR: (eof) GDB never initialized.
    ...
    
    Further errors are triggered in clean_restart, because we're not testing for
    gdb_start success.  Fix this by detecting gdb_start failure, and bailing out.
    
    Finally, we're running into further errors in gdb.base/readnever.exp because
    we're not testing for clean_restart success.  Fix this by making clean_restart
    return -1 upon error, and testing for this.
    
    Tested on x86_64-linux.
    
    gdb/testsuite/ChangeLog:
    
    2020-04-24  Tom de Vries  <tdevries@suse.de>
    
            * lib/gdb.exp (default_gdb_start): Handle eof.
            (clean_restart): Detect and handle gdb_start failure.  Return -1 upon
            failure.
            * gdb.base/readnever.exp: Handle clean_restart failure.

Diff:
---
 gdb/testsuite/ChangeLog              |  7 +++++++
 gdb/testsuite/gdb.base/readnever.exp |  4 +++-
 gdb/testsuite/lib/gdb.exp            | 23 ++++++++++++++++++++++-
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 10683db566d..ecccb613045 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2020-04-24  Tom de Vries  <tdevries@suse.de>
+
+	* lib/gdb.exp (default_gdb_start): Handle eof.
+	(clean_restart): Detect and handle gdb_start failure.  Return -1 upon
+	failure.
+	* gdb.base/readnever.exp: Handle clean_restart failure.
+
 2020-04-23  Tom de Vries  <tdevries@suse.de>
 
 	* gdb.base/decl-before-def.exp: Run to main and print a again.
diff --git a/gdb/testsuite/gdb.base/readnever.exp b/gdb/testsuite/gdb.base/readnever.exp
index ab2e18e2260..113176c1789 100644
--- a/gdb/testsuite/gdb.base/readnever.exp
+++ b/gdb/testsuite/gdb.base/readnever.exp
@@ -29,7 +29,9 @@ if { [lsearch -exact $GDBFLAGS -readnow] != -1 \
 
 save_vars { GDBFLAGS } {
     append GDBFLAGS " --readnever"
-    clean_restart ${binfile}
+    if { [clean_restart ${binfile}] == -1 } {
+       return -1
+    }
 }
 
 if ![runto_main] then {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8418c3d8753..cdf96e3c703 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1894,6 +1894,11 @@ proc default_gdb_start { } {
 	    unset gdb_spawn_id
 	    return -1
 	}
+	eof {
+	    perror "(eof) GDB never initialized."
+	    unset gdb_spawn_id
+	    return -1
+	}
     }
 
     # force the height to "unlimited", so no pagers get used
@@ -6097,24 +6102,40 @@ proc build_executable { testname executable {sources ""} {options {debug}} } {
 # Starts fresh GDB binary and loads an optional executable into GDB.
 # Usage: clean_restart [executable]
 # EXECUTABLE is the basename of the binary.
+# Return -1 if starting gdb or loading the executable failed.
 
 proc clean_restart { args } {
     global srcdir
     global subdir
+    global errcnt
 
     if { [llength $args] > 1 } {
 	error "bad number of args: [llength $args]"
     }
 
     gdb_exit
+
+    # We'd like to do:
+    #   if { [gdb_start] == -1 } {
+    #     return -1
+    #   }
+    # but gdb_start is a ${tool}_start proc, which doesn't have a defined
+    # return value.  So instead, we test for errcnt.
+    set saved_errcnt $errcnt
     gdb_start
+    if { $errcnt > $saved_errcnt } {
+	return -1
+    }
+
     gdb_reinitialize_dir $srcdir/$subdir
 
     if { [llength $args] >= 1 } {
 	set executable [lindex $args 0]
 	set binfile [standard_output_file ${executable}]
-	gdb_load ${binfile}
+	return [gdb_load ${binfile}]
     }
+
+    return 0
 }
 
 # Prepares for testing by calling build_executable_full, then