* [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