public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 1/2] gdb: fix failures in gdb.mi/mi-exec-run.exp with native-extended-gdbserver
Date: Wed, 27 Apr 2022 19:17:26 +0100	[thread overview]
Message-ID: <7e541f1c625c73f20694ac37f3c326e57f95c11d.1651083290.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1651083290.git.aburgess@redhat.com>

When running the gdb.mi/mi-exec-run.exp test using the
native-extended-gdbserver I see failures like this:

  FAIL: gdb.mi/mi-exec-run.exp: inferior-tty=main: mi=main: force-fail=1: run failure detected
  FAIL: gdb.mi/mi-exec-run.exp: inferior-tty=main: mi=separate: force-fail=1: run failure detected
  FAIL: gdb.mi/mi-exec-run.exp: inferior-tty=separate: mi=separate: force-fail=1: run failure detected

There's a race condition here, so you might see a slightly different
set of failures, but I always see some from the 'run failure detected'
test.

NOTE: I also see an additional test failure:

 FAIL: gdb.mi/mi-exec-run.exp: inferior-tty=separate: mi=separate: force-fail=0: breakpoint hit reported on console (timeout)

but that is a completely different issue, and is not being addressed
in this commit.

The problem for the 'run failure detected' test is that we end up
in gdb_expect looking for output from two spawn-ids, one from
gdbserver, and one from gdb.  We're looking for one output pattern
from each spawn-id, and for the test to pass we need to see both
patterns.

Now, if gdb exits then this is a test failure (this would indicate gdb
crashing, which is bad), so we have the eof pattern associated with
the gdb spawn-id.

However, in this particular test we expect gdbserver to fail to
execute the binary (the test binary is set non-executable), and so we
get an error message from gdbserver (which matches the pattern), and
then gdbserver exits, this is expected.

In our gdb_expect call we don't have an eof pattern for gdbserver, and
so, when gdbserver exits, expect sees eof and drops out of the
gdb_expect call.

If both the gdbserver AND gdb patterns are seen before the gdbserver
eof triggers then everything is fine.

But, if expect sees gdbserver's eof before seeing the output from gdb
then we will exit the gdb_expect call, and (as we have not seen both
patterns) we will call 'fail'.

I initially tried to fix this by adding a new eof pattern for the
gdbserver spawn-id that called exp_continue, however, this causes
expect to throw an error, it is not happy to have '-i' options that
make use of a closed spawn-id.

So, the solution I propose here is to wrap the gdb_expect call in a
loop.

I then dynamically build the set of patterns that will be passed to
the gdb_expect call, excluding the gdbserver related patterns if the
gdbserver has already exited.

Now, the first time around the loop, as gdb and gdbserver are alive, I
add all the patterns and enter gdb_expect just like before.

When gdbserver exits we drop out of gdb_expect and go around the loop
again.  This time we only add the gdb related patterns, and reenter
the gdb_expect call.  Once the gdb related pattern is seen we drop out
of the gdb_expect call and exit the loop.

I now see no failures relating to 'run failure detected'.
---
 gdb/testsuite/gdb.mi/mi-exec-run.exp | 89 +++++++++++++++++++++-------
 1 file changed, 67 insertions(+), 22 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-exec-run.exp b/gdb/testsuite/gdb.mi/mi-exec-run.exp
index f397159078f..d736e19090a 100644
--- a/gdb/testsuite/gdb.mi/mi-exec-run.exp
+++ b/gdb/testsuite/gdb.mi/mi-exec-run.exp
@@ -90,35 +90,80 @@ proc test {inftty_mode mi_mode force_fail} {
 	set saw_perm_error 0
 	set saw_mi_error 0
 	set already_failed 0
+	set inferior_exited false
+
 	set test "run failure detected"
 	send_gdb "-exec-run --start\n"
 
-	gdb_expect {
-	    -i "$inferior_spawn_id"
-	    -re ".*Cannot exec.*Permission denied" {
-		set saw_perm_error 1
-		verbose -log "saw perm error"
-		if {!$saw_mi_error} {
-		    exp_continue
+	# The following call to gdb_expect is embedded within this
+	# loop in order to allow testing with the
+	# native-extended-gdbserver board.  Please test with this
+	# board if making any changes to the following loop.
+	while { ! [expr ($saw_perm_error && $saw_mi_error) \
+		       || $already_failed] } {
+
+	    # Place a list of expect pattern/command blocks into CODE,
+	    # then pass these to a gdb_expect call at the end of this
+	    # loop.
+	    set code {}
+
+	    # If the inferior has not exited then we add two patterns,
+	    # the first looks for the expected output from the
+	    # inferior, the second detects eof from the inferior.  If
+	    # the inferior is gdbserver, then after complaining about
+	    # permission denied, gdbserver will exit (which triggers
+	    # the eof pattern).
+	    #
+	    # When we see the eof pattern we will exit the gdb_expect
+	    # call (at the end of this loop), then next time around
+	    # these patterns (relating to the now closed inferior)
+	    # will not be included.
+	    if { ! $inferior_exited } {
+		append code {
+		    -i "$inferior_spawn_id"
+		    -re ".*Cannot exec.*Permission denied" {
+			set saw_perm_error 1
+			verbose -log "saw perm error"
+			if {!$saw_mi_error} {
+			    exp_continue
+			}
+		    }
+
+		    -i "$inferior_spawn_id"
+		    eof {
+			set inferior_exited true
+		    }
 		}
 	    }
-	    -i "$gdb_spawn_id"
-	    -re "\\^error,msg=\"(During startup program exited with code 127|Running .* on the remote target failed)" {
-		set saw_mi_error 1
-		verbose -log "saw mi error"
-		if {!$saw_perm_error} {
-		    exp_continue
+
+	    # These patterns all relate to the main gdb spawn-id, if
+	    # gdb exits then this is an error, so we don't need the
+	    # same "has spawn-id exited?" protection that we do for
+	    # the inferior spawn-id above.
+	    append code {
+		-i "$gdb_spawn_id"
+		-re "\\^error,msg=\"(During startup program exited with code 127|Running .* on the remote target failed)" {
+		    set saw_mi_error 1
+		    verbose -log "saw mi error"
+		    if {!$saw_perm_error} {
+			exp_continue
+		    }		}
+
+		-i "$gdb_main_spawn_id"
+		eof {
+		    set already_failed 1
+		    fail "$test (eof)"
+		}
+
+		timeout {
+		    set already_failed 1
+		    fail "$test (timeout)"
 		}
 	    }
-	    timeout {
-		set already_failed 1
-		fail "$test (timeout)"
-	    }
-	    -i "$gdb_main_spawn_id"
-	    eof {
-		set already_failed 1
-		fail "$test (eof)"
-	    }
+
+	    # Now we can call expect with the set of pattern/command blocks
+	    # we collected above.
+	    gdb_expect $code
 	}
 
 	if {$saw_perm_error && $saw_mi_error} {
-- 
2.25.4


  reply	other threads:[~2022-04-27 18:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-27 18:17 [PATCH 0/2] Fix mi-exec-run.exp " Andrew Burgess
2022-04-27 18:17 ` Andrew Burgess [this message]
2022-04-27 18:42   ` [PATCH 1/2] gdb: fix failures in gdb.mi/mi-exec-run.exp " Pedro Alves
2022-04-27 18:17 ` [PATCH 2/2] gdb/testsuite: fix native-extended-gdbserver failure in mi-exec-run.exp Andrew Burgess
2022-04-28 11:27 ` [PATCHv2 0/2] Fix mi-exec-run.exp with native-extended-gdbserver Andrew Burgess
2022-04-28 11:27   ` [PATCHv2 1/2] gdb: fix failures in gdb.mi/mi-exec-run.exp " Andrew Burgess
2022-04-29 13:52     ` Pedro Alves
2022-04-28 11:27   ` [PATCHv2 2/2] gdb/testsuite: fix native-extended-gdbserver failure in mi-exec-run.exp Andrew Burgess
2022-04-29 14:13     ` Pedro Alves
2022-04-29 18:15       ` [PATCHv3 0/3] gdb/testsuite: fix mi-exec-run.exp Andrew Burgess
2022-04-29 18:15         ` [PATCHv3 1/3] gdb/testsuite: fix mi-exec-run.exp with native-extended-gdbserver board Andrew Burgess
2022-04-29 18:15         ` [PATCHv3 2/3] gdb/testsuite: change mi_gdb_start to take a list of flags Andrew Burgess
2022-04-29 18:15         ` [PATCHv3 3/3] gdb/testsuite: small cleanup in mi-break-qualified.exp Andrew Burgess
2022-05-02 16:37         ` [PATCHv3 0/3] gdb/testsuite: fix mi-exec-run.exp Pedro Alves
2022-05-03  9:49           ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7e541f1c625c73f20694ac37f3c326e57f95c11d.1651083290.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).