* [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit
@ 2020-09-14 19:21 Tom de Vries
2020-09-15 13:05 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2020-09-14 19:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
Hi,
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 read_prompt 0
expect {
-i "$gdb_spawn_id" -re "$gdb_prompt $" {
+ set read_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 {$read_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: $read_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: $read_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.
Any comments?
Thanks,
- Tom
[gdb/testsuite] Detect gdb prompt after monitor exit
---
gdb/testsuite/lib/gdbserver-support.exp | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index a2cc80f28d..3636505aab 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -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 read_prompt 0
expect {
-i "$gdb_spawn_id" -re "$gdb_prompt $" {
+ set read_prompt 1
exp_continue
}
-i "$server_spawn_id" eof {
@@ -463,6 +465,15 @@ proc gdbserver_exit { is_mi } {
warning "Timed out waiting for EOF in server after $monitor_exit"
}
}
+ if { ! read_prompt } {
+ expect {
+ -i "$gdb_spawn_id" -re "$gdb_prompt $" {
+ }
+ timeout {
+ warning "Timed out waiting for prompt after $monitor_exit"
+ }
+ }
+ }
}
}
close_gdbserver
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit
2020-09-14 19:21 [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit Tom de Vries
@ 2020-09-15 13:05 ` Pedro Alves
2020-09-15 14:17 ` Tom de Vries
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2020-09-15 13:05 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 9/14/20 8:21 PM, Tom de Vries wrote:
> After issuing a "monitor exit" command, we should always get a prompt back, so
> check for that.
>
> Tested on x86_64-linux.
>
> Any comments?
>
Good find.
> @@ -463,6 +465,15 @@ proc gdbserver_exit { is_mi } {
> warning "Timed out waiting for EOF in server after $monitor_exit"
> }
> }
> + if { ! read_prompt } {
> + expect {
> + -i "$gdb_spawn_id" -re "$gdb_prompt $" {
> + }
> + timeout {
> + warning "Timed out waiting for prompt after $monitor_exit"
> + }
> + }
> + }
So if we get the prompt first, we go back to waiting for the eof.
But if the eof is first, we currently don't continue waiting for
the prompt. Do we really need this new block of code instead
of just putting:
if {!$read_prompt} {
exp_continue
}
in the eof case?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit
2020-09-15 13:05 ` Pedro Alves
@ 2020-09-15 14:17 ` Tom de Vries
2020-09-16 10:59 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2020-09-15 14:17 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 9/15/20 3:05 PM, Pedro Alves wrote:
> On 9/14/20 8:21 PM, Tom de Vries wrote:
>
>> After issuing a "monitor exit" command, we should always get a prompt back, so
>> check for that.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>>
>
> Good find.
>> @@ -463,6 +465,15 @@ proc gdbserver_exit { is_mi } {
>> warning "Timed out waiting for EOF in server after $monitor_exit"
>> }
>> }
>> + if { ! read_prompt } {
>> + expect {
>> + -i "$gdb_spawn_id" -re "$gdb_prompt $" {
>> + }
>> + timeout {
>> + warning "Timed out waiting for prompt after $monitor_exit"
>> + }
>> + }
>> + }
>
> So if we get the prompt first, we go back to waiting for the eof.
> But if the eof is first, we currently don't continue waiting for
> the prompt. Do we really need this new block of code instead
> of just putting:
>
> if {!$read_prompt} {
> exp_continue
> }
>
> in the eof case?
>
AFAIU, if we do that, and we first hit the server_spawn_id case, we'll
do exp_continue, after which we'll hit the gdb_spawn_id case, after
which we'll do exp_continue ... after which we should run into the
timeout. I couldn't trigger that though.
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
+ 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.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit
2020-09-15 14:17 ` Tom de Vries
@ 2020-09-16 10:59 ` Pedro Alves
2020-09-16 12:55 ` Tom de Vries
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2020-09-16 10:59 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
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.
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. "
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit
2020-09-16 10:59 ` Pedro Alves
@ 2020-09-16 12:55 ` Tom de Vries
0 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2020-09-16 12:55 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
[-- 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"
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-16 12:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 19:21 [PATCH][gdb/testsuite] Detect gdb prompt after monitor exit Tom de Vries
2020-09-15 13:05 ` Pedro Alves
2020-09-15 14:17 ` Tom de Vries
2020-09-16 10:59 ` Pedro Alves
2020-09-16 12:55 ` Tom de Vries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox