Re: [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2662 bytes --]

On 9/16/20 12:59 PM, Pedro Alves wrote:
> On 9/15/20 3:17 PM, Tom de Vries wrote:
> 
>> Anyway, this seems to work:
>> ...
>> index a2cc80f28d..24733275ae 100644
>> --- a/gdb/testsuite/lib/gdbserver-support.exp
>> +++ b/gdb/testsuite/lib/gdbserver-support.exp
>> @@ -451,13 +451,20 @@ proc gdbserver_exit { is_mi } {
>>             # We use expect rather than gdb_expect because
>>             # we want to suppress printing exception messages, otherwise,
>>             # remote_expect, invoked by gdb_expect, prints the exceptions.
>> +           set have_prompt 0
>>             expect {
>>                 -i "$gdb_spawn_id" -re "$gdb_prompt $" {
>> -                   exp_continue
>> +                   if { [info exists server_spawn_id] } {
>> +                       set have_prompt 1
> 
> I'd put this "set have_prompt 1" outside of the if, seems would read
> clearer to me.
> 

Done.

> Otherwise LGTM.
> 
>> +                       exp_continue
>> +                   }
>>                 }
>>                 -i "$server_spawn_id" eof {
>>                     wait -i $expect_out(spawn_id)
>>                     unset server_spawn_id
>> +                   if { ! $have_prompt } {
>> +                       exp_continue
>> +                   }
>>                 }
>>                 timeout {
>>                     warning "Timed out waiting for EOF in server after
>> $monitor_exit"
>> ...
>>
>> I do wonder though about unsetting server_spawn_id and doing
>> exp_continue while we still have the clause -i $server_spawn_id eof.  I
>> wonder this is supported behaviour.
> 
> I'm not super sure, but I think it is, in that I don't think
> the -i expression is reevaluated after exp_continue.
> 
> For example, this works:
> 
> $ cat exp.exp 
> set fake_spawn_id foo
> 
> expect {
>     -re "foo" {
>         puts " got foo"
>         if [info exists fake_spawn_id] {
>             puts " unset"
>             unset fake_spawn_id
>         }
>         exp_continue
>     }
>     -i "$fake_spawn_id" -re "bar" {
>             puts " got bar?"
>     }
> }
> 
> $ expect exp.exp 
> foo
>  got foo
>  unset
> foo
>  got foo
> foo
>  got foo
> bar
> foo
>  got foo
> 
> If it turns out to be an issue, we can always use an indirect
> spawn id instead:
> 
>   "The -i flag may also name a global variable in which case the variable is read for a list of spawn ids. The variable is reread   
>    whenever it changes. This provides a way of changing the I/O source while the command is in execution. Spawn ids provided this way 
>    are called "indirect" spawn ids. "

OK, then let's try it like this for now.

Committed as attached.

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-testsuite-Detect-gdb-prompt-after-monitor-exit.patch --]
[-- Type: text/x-patch, Size: 2720 bytes --]

[gdb/testsuite] Detect gdb prompt after monitor exit

With this gdbserver-support.exp patch:
...
@@ -451,8 +451,10 @@ proc gdbserver_exit { is_mi } {
 	    # We use expect rather than gdb_expect because
 	    # we want to suppress printing exception messages, otherwise,
 	    # remote_expect, invoked by gdb_expect, prints the exceptions.
+	    set have_prompt 0
 	    expect {
 		-i "$gdb_spawn_id" -re "$gdb_prompt $" {
+		    set have_prompt 1
 		    exp_continue
 		}
 		-i "$server_spawn_id" eof {
@@ -463,6 +465,7 @@ proc gdbserver_exit { is_mi } {
                    warning "Timed out waiting for EOF in server after $monitor_exit"
                }
 	    }
+	    gdb_assert {$have_prompt}
 	}
     }
     close_gdbserver
...
and with this in parallel:
...
$ stress -c 5
...
we run into this and similar FAILs:
...
FAIL: gdb.multi/multi-target.exp: continue: non-stop=on: $have_prompt
...

In more detail:
...
(gdb) PASS: gdb.multi/multi-target.exp: continue: non-stop=on: inferior 5
Remote debugging from host ::1, port 40712^M
Process build/gdb/testsuite/outputs/gdb.multi/multi-target/multi-target \
  created; pid = 11098^M
monitor exit^M
Killing process(es): 11098^M
FAIL: gdb.multi/multi-target.exp: continue: non-stop=on: $have_prompt
spawn build/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory \
  build/gdb/testsuite/../data-directory^M
...

After issuing a "monitor exit" command, we should always get a prompt back, so
check for that.

Tested on x86_64-linux.

gdb/testsuite/ChangeLog:

2020-09-16  Tom de Vries  <tdevries@suse.de>

	* lib/gdbserver-support.exp (gdbserver_exit): Make sure we
	get the gdb prompt after issuing "monitor exit".

---
 gdb/testsuite/lib/gdbserver-support.exp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index a2cc80f28d..2734ca6c87 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -451,13 +451,20 @@ proc gdbserver_exit { is_mi } {
 	    # We use expect rather than gdb_expect because
 	    # we want to suppress printing exception messages, otherwise,
 	    # remote_expect, invoked by gdb_expect, prints the exceptions.
+	    set have_prompt 0
 	    expect {
 		-i "$gdb_spawn_id" -re "$gdb_prompt $" {
-		    exp_continue
+		    set have_prompt 1
+		    if { [info exists server_spawn_id] } {
+			exp_continue
+		    }
 		}
 		-i "$server_spawn_id" eof {
 		    wait -i $expect_out(spawn_id)
 		    unset server_spawn_id
+		    if { ! $have_prompt } {
+			exp_continue
+		    }
 		}
                timeout {
                    warning "Timed out waiting for EOF in server after $monitor_exit"