Re: [RFA] gdb.mi/*.exp, send_gdb vs. gdb_test

[Pedro Alves <pedro at codesourcery dot com>]

On Wednesday 26 May 2010 01:36:02, Michael Snyder wrote:
>         * gdb.mi/mi-nsmoribund.exp: Replace send_gdb with gdb_test.

> Index: mi-nsmoribund.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-nsmoribund.exp,v
> retrieving revision 1.5
> diff -u -p -r1.5 mi-nsmoribund.exp
> --- mi-nsmoribund.exp   1 Jan 2010 07:32:03 -0000       1.5
> +++ mi-nsmoribund.exp   26 May 2010 00:34:29 -0000
> @@ -37,14 +37,13 @@ mi_gdb_reinitialize_dir $srcdir/$subdir

(...)

> @@ -128,20 +123,14 @@ mi_check_thread_states \
>  # a target bug if it can step over breakpoints itself), a spurious
>  # SIGTRAP/SIGSEGV can come at any time after deleting the breakpoint.
>  
> -send_gdb "102-break-delete\n"
> -send_gdb "print done = 1\n"
> -send_gdb "103-exec-continue --all\n"
> -
> -gdb_expect {
> -    -re "\\*stopped,reason=\"exited-normally\"" {
> -       pass "resume all, program exited normally"
> -    }
> -    -re "\\*stopped" {
> -       fail "unexpected stop"
> -    }
> -    timeout {
> -       fail "resume all, waiting for program exit (timeout)"
> +gdb_test_multiple "102-break-delete\nprint done = 1\n103-exec-continue --all" \
> +    "resume all, program exited normally" {
> +       -re "\\*stopped,reason=\"exited-normally\"" {
> +           pass "resume all, program exited normally"
> +       }
> +       -re "\\*stopped" {
> +           fail "unexpected stop"
> +       }
>      }
> -}
>  

This particular change introduces a race that causes an
ocasional spurious FAIL:

 FAIL: gdb.mi/mi-nsmoribund.exp: resume all, program exited normally

The test says:

 # (...) Send all commands in a row, since if something
 # goes wrong with moribund locations support or displaced stepping (or
 # a target bug if it can step over breakpoints itself), a spurious
 # SIGTRAP/SIGSEGV can come at any time after deleting the breakpoint.

Each of those commands will produce one prompt:

 102-break-delete
 102^done
 (gdb) 
 print done = 1
 103-exec-continue --all
 &"print done = 1\n"
 ~"$1 = 1"
 ~"\n"
 ^done
 (gdb) 
 103^running
 *running,thread-id="5"
 (gdb) FAIL: gdb.mi/mi-nsmoribund.exp: resume all, program exited normally

which depending on how data appears on expect's buffer, can
make gdb_test_multiple think that's an error:

gdb_test_multiple has:
...
	 -re "\r\n$gdb_prompt $" {
	    if ![string match "" $message] then {
		fail "$message"
	    }
...


I've switched back that particular test in mi-nsmoribund.exp
to send_gdb/gdb_expect to fix this.

This particular test is just more susceptible to this race
than other uses of gdb_test_multiple in gdb.mi/, but I think
all such uses are in fact wrong, since $gdb_prompt is a prefix
of $mi_gdb_prompt,

    set gdb_prompt "\[(\]gdb\[)\]"
    set mi_gdb_prompt "\[(\]gdb\[)\] \r\n"

and that happens to match that gdb_test_multiple regex shown
above, so, any gdb_test_multiple that has 

 -re "... .. ... $mi_gdb_prompt"

is racy.  Plus, gdb_test_multiple is tied to CLI's output internally.
The fact that it works most of the time appears only to be a fortunate
accident.  Should we have an mi_gdb_test_multiple instead?  Either
that, or I think we should consider reverting all gdb_test_multiple
uses under gdb.mi/ back to send_gdb/gdb_test.

-- 
Pedro Alves

2010-08-26  Pedro Alves  <pedro@codesourcery.com>

	* gdb.mi/mi-nsmoribund.exp: Revert back to send_gdb/gdb_expect in
	one test.

---
 gdb/testsuite/gdb.mi/mi-nsmoribund.exp |   22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Index: src/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.mi/mi-nsmoribund.exp	2010-08-26 15:35:37.000000000 +0100
+++ src/gdb/testsuite/gdb.mi/mi-nsmoribund.exp	2010-08-26 15:47:16.000000000 +0100
@@ -121,15 +121,21 @@ mi_check_thread_states \
 # goes wrong with moribund locations support or displaced stepping (or
 # a target bug if it can step over breakpoints itself), a spurious
 # SIGTRAP/SIGSEGV can come at any time after deleting the breakpoint.
+# Note that this causes multiple prompts to appear before the output
+# we are interested in, so we can't use mi_gdb_test or
+# gdb_test_multiple (or an MI equivalent).
 
-gdb_test_multiple "102-break-delete\nprint done = 1\n103-exec-continue --all" \
-    "resume all, program exited normally" {
-	-re "\\*stopped,reason=\"exited-normally\"" {
-	    pass "resume all, program exited normally"
-	}
-	-re "\\*stopped" {
-	    fail "unexpected stop"
-	}
+send_gdb "102-break-delete\n"
+send_gdb "print done = 1\n"
+send_gdb "103-exec-continue --all\n"
+
+gdb_expect {
+    -re "\\*stopped,reason=\"exited-normally\"" {
+       pass "resume all, program exited normally"
+    }
+    timeout {
+       fail "resume all, waiting for program exit (timeout)"
     }
+}
 
 mi_gdb_exit