* [PATCH] gdb/testsuite: fix race in gdb.base/async-shell.exp
@ 2022-10-10 1:21 Simon Marchi
2022-10-10 7:07 ` Tom de Vries
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2022-10-10 1:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries, Simon Marchi
I see some random failures in this test:
It can be reliably reproduced (on Linux) with this change:
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 44cc28b30051..2a3c8253ba5a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1301,6 +1301,7 @@ proc gdb_test_multiple { command message args } {
}
set gdb_test_name "$message"
+ sleep 2
set result 0
set code [catch {gdb_expect $code} string]
The problem is that the `run &` command prints some things after the
prompt:
(gdb) [Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
If DejaGNU is quick enough, it will consume only up to the prompt. But
if it is slow enough, it will consume those messages at the same time as
the prompt, in which case the gdb_test used for "run &" won't match. By
default, the prompt used by gdb_test uses a `$` to anchor the match at
the end of the buffer. If there's anything following the prompt, it
won't match.
The diff above aadds a delay between sending the command and consuming
the output, giving GDB more time to output the messages, giving a good
chance that DejaGNU consumes them at the same time as the prompt.
This is normally handled by using gdb_test_multiple and specifying a
pattern that ends with "$gdb_prompt", but not a trailing $. I think
this is common enough that it deserves its own gdb_test option.
Therefore, add the -no-anchor-prompt option to gdb_test, and
gdb_test_no_output for completeness. Use it in
gdb.base/async-shell.exp.
Change-Id: I9051d8800d1c10a2e95db1a575991f7723492f1b
---
gdb/testsuite/gdb.base/async-shell.exp | 2 +-
gdb/testsuite/lib/gdb.exp | 29 +++++++++++++++++++-------
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/gdb/testsuite/gdb.base/async-shell.exp b/gdb/testsuite/gdb.base/async-shell.exp
index e2d71826bab2..2c09863ad593 100644
--- a/gdb/testsuite/gdb.base/async-shell.exp
+++ b/gdb/testsuite/gdb.base/async-shell.exp
@@ -34,7 +34,7 @@ save_vars { GDBFLAGS } {
set gdbindex_warning_re "warning: Skipping \[^\r\n\]+ \\.gdb_index section \[^\r\n\]*\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\."
-gdb_test "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
+gdb_test -no-prompt-anchor "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
# `sleep 5' here would workaround the bug, do not sleep here.
# "shell" could eat waitpid event from the asynchronous inferior process.
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 44cc28b30051..c8d2aa511fb8 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -861,9 +861,18 @@ proc gdb_internal_error_resync {} {
}
# Fill in the default prompt if PROMPT_REGEXP is empty.
-proc fill_in_default_prompt {prompt_regexp} {
+#
+# If WITH_ANCHOR is true and the default prompt is used, append a `$` at the end
+# of the regexp, to anchor the match at the end of the buffer.
+proc fill_in_default_prompt {prompt_regexp with_anchor} {
if { "$prompt_regexp" == "" } {
- return "$::gdb_prompt $"
+ set prompt "$::gdb_prompt "
+
+ if { $with_anchor } {
+ append prompt "$"
+ }
+
+ return $prompt
}
return $prompt_regexp
}
@@ -993,7 +1002,7 @@ proc gdb_test_multiple { command message args } {
error "Too few arguments to gdb_test_multiple"
}
- set prompt_regexp [fill_in_default_prompt $prompt_regexp]
+ set prompt_regexp [fill_in_default_prompt $prompt_regexp true]
if { $message == "" } {
set message $command
@@ -1365,6 +1374,10 @@ proc gdb_test_multiline { name args } {
#
# -prompt PROMPT_REGEXP specifies a regexp matching the expected prompt
# after the command output. If empty, defaults to "$gdb_prompt $".
+# -no-prompt-anchor specifies that if the default prompt regexp is used, it
+# should not be anchored at the end of the buffer. This means that the
+# pattern can match even if there is stuff output after the prompt. Does not
+# have any effect if -prompt is specified.
# -lbl specifies that line-by-line matching will be used.
# -nopass specifies that a PASS should not be issued.
#
@@ -1379,6 +1392,7 @@ proc gdb_test { args } {
parse_args {
{prompt ""}
+ {no-prompt-anchor}
{lbl}
{nopass}
}
@@ -1394,7 +1408,7 @@ proc gdb_test { args } {
set message $command
}
- set prompt [fill_in_default_prompt $prompt]
+ set prompt [fill_in_default_prompt $prompt [expr !${no-prompt-anchor}]]
set saw_question 0
@@ -1471,20 +1485,21 @@ if { [tcl_version_at_least 8 5] == 0 } {
# gdb_test_no_output [-prompt PROMPT_REGEXP] [-nopass] COMMAND [MESSAGE]
# Send a command to GDB and verify that this command generated no output.
#
-# See gdb_test for a description of the -prompt, -nopass, COMMAND, and
-# MESSAGE parameters.
+# See gdb_test for a description of the -prompt, -no-prompt-anchor, -nopass,
+# COMMAND, and MESSAGE parameters.
proc gdb_test_no_output { args } {
global gdb_prompt
parse_args {
{prompt ""}
+ {no-prompt-anchor}
{nopass}
}
lassign $args command message
- set prompt [fill_in_default_prompt $prompt]
+ set prompt [fill_in_default_prompt $prompt [expr !${no-prompt-anchor}]]
set command_regex [string_to_regexp $command]
gdb_test_multiple $command $message -prompt $prompt {
base-commit: bbcf4de01867fd2ac2587920d22541db25de68ba
--
2.38.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb/testsuite: fix race in gdb.base/async-shell.exp
2022-10-10 1:21 [PATCH] gdb/testsuite: fix race in gdb.base/async-shell.exp Simon Marchi
@ 2022-10-10 7:07 ` Tom de Vries
2022-10-11 0:44 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2022-10-10 7:07 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 10/10/22 03:21, Simon Marchi wrote:
> I see some random failures in this test:
>
> It can be reliably reproduced (on Linux) with this change:
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 44cc28b30051..2a3c8253ba5a 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1301,6 +1301,7 @@ proc gdb_test_multiple { command message args } {
> }
> set gdb_test_name "$message"
>
> + sleep 2
> set result 0
> set code [catch {gdb_expect $code} string]
>
I first tried reproducing on openSUSE Leap 15.4, and failed because that
one has glibc 2.31, but managed on openSUSE Tumbleweed with glibc 2.36,
which has the "pthread merged into glibc" feature that's been around
since 2.34.
> The problem is that the `run &` command prints some things after the
> prompt:
>
> (gdb) [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
>
> If DejaGNU is quick enough, it will consume only up to the prompt. But
FWIW, I would use expect instead of DejaGNU, because it's more specific.
> if it is slow enough, it will consume those messages at the same time as
> the prompt, in which case the gdb_test used for "run &" won't match. By
> default, the prompt used by gdb_test uses a `$` to anchor the match at
> the end of the buffer. If there's anything following the prompt, it
> won't match.
>
> The diff above aadds a delay between sending the command and consuming
adds
> the output, giving GDB more time to output the messages, giving a good
> chance that DejaGNU consumes them at the same time as the prompt.
>
> This is normally handled by using gdb_test_multiple and specifying a
> pattern that ends with "$gdb_prompt", but not a trailing $.
Ack.
> I think
> this is common enough that it deserves its own gdb_test option.
Agreed.
> Therefore, add the -no-anchor-prompt option to gdb_test, and
> gdb_test_no_output for completeness. Use it in
> gdb.base/async-shell.exp.
>
I've tested it and confirmed that it fixes the problem.
FWIW, I can also imagine we'll add this to gdb_test_multiple at some
point, but that's certainly not necessary for now.
> Change-Id: I9051d8800d1c10a2e95db1a575991f7723492f1b
> ---
> gdb/testsuite/gdb.base/async-shell.exp | 2 +-
> gdb/testsuite/lib/gdb.exp | 29 +++++++++++++++++++-------
> 2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/async-shell.exp b/gdb/testsuite/gdb.base/async-shell.exp
> index e2d71826bab2..2c09863ad593 100644
> --- a/gdb/testsuite/gdb.base/async-shell.exp
> +++ b/gdb/testsuite/gdb.base/async-shell.exp
> @@ -34,7 +34,7 @@ save_vars { GDBFLAGS } {
>
> set gdbindex_warning_re "warning: Skipping \[^\r\n\]+ \\.gdb_index section \[^\r\n\]*\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\."
>
> -gdb_test "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
> +gdb_test -no-prompt-anchor "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
>
> # `sleep 5' here would workaround the bug, do not sleep here.
> # "shell" could eat waitpid event from the asynchronous inferior process.
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 44cc28b30051..c8d2aa511fb8 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -861,9 +861,18 @@ proc gdb_internal_error_resync {} {
> }
>
> # Fill in the default prompt if PROMPT_REGEXP is empty.
> -proc fill_in_default_prompt {prompt_regexp} {
> +#
> +# If WITH_ANCHOR is true and the default prompt is used, append a `$` at the end
> +# of the regexp, to anchor the match at the end of the buffer.
> +proc fill_in_default_prompt {prompt_regexp with_anchor} {
> if { "$prompt_regexp" == "" } {
> - return "$::gdb_prompt $"
> + set prompt "$::gdb_prompt "
> +
> + if { $with_anchor } {
> + append prompt "$"
> + }
> +
> + return $prompt
> }
> return $prompt_regexp
> }
> @@ -993,7 +1002,7 @@ proc gdb_test_multiple { command message args } {
> error "Too few arguments to gdb_test_multiple"
> }
>
> - set prompt_regexp [fill_in_default_prompt $prompt_regexp]
> + set prompt_regexp [fill_in_default_prompt $prompt_regexp true]
>
> if { $message == "" } {
> set message $command
> @@ -1365,6 +1374,10 @@ proc gdb_test_multiline { name args } {
> #
> # -prompt PROMPT_REGEXP specifies a regexp matching the expected prompt
> # after the command output. If empty, defaults to "$gdb_prompt $".
> +# -no-prompt-anchor specifies that if the default prompt regexp is used, it
> +# should not be anchored at the end of the buffer. This means that the
> +# pattern can match even if there is stuff output after the prompt. Does not
> +# have any effect if -prompt is specified.
> # -lbl specifies that line-by-line matching will be used.
> # -nopass specifies that a PASS should not be issued.
> #
> @@ -1379,6 +1392,7 @@ proc gdb_test { args } {
>
> parse_args {
> {prompt ""}
> + {no-prompt-anchor}
> {lbl}
> {nopass}
> }
> @@ -1394,7 +1408,7 @@ proc gdb_test { args } {
> set message $command
> }
>
> - set prompt [fill_in_default_prompt $prompt]
> + set prompt [fill_in_default_prompt $prompt [expr !${no-prompt-anchor}]]
>
> set saw_question 0
>
> @@ -1471,20 +1485,21 @@ if { [tcl_version_at_least 8 5] == 0 } {
> # gdb_test_no_output [-prompt PROMPT_REGEXP] [-nopass] COMMAND [MESSAGE]
> # Send a command to GDB and verify that this command generated no output.
> #
> -# See gdb_test for a description of the -prompt, -nopass, COMMAND, and
> -# MESSAGE parameters.
> +# See gdb_test for a description of the -prompt, -no-prompt-anchor, -nopass,
> +# COMMAND, and MESSAGE parameters.
>
> proc gdb_test_no_output { args } {
> global gdb_prompt
>
> parse_args {
> {prompt ""}
> + {no-prompt-anchor}
> {nopass}
> }
>
> lassign $args command message
>
> - set prompt [fill_in_default_prompt $prompt]
> + set prompt [fill_in_default_prompt $prompt [expr !${no-prompt-anchor}]]
>
FWIW, I've wondered whether giving fill_in_default_prompt a
no-prompt-anchor parameter instead of with_anchor would be better,
avoiding the inversion here and using consistent naming for the same
concept across different procs, but I haven't been able to make up my
mind about whether I really like it better ... I thought I just mention it.
Anyway, LGTM.
Thanks,
- Tom
> set command_regex [string_to_regexp $command]
> gdb_test_multiple $command $message -prompt $prompt {
>
> base-commit: bbcf4de01867fd2ac2587920d22541db25de68ba
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb/testsuite: fix race in gdb.base/async-shell.exp
2022-10-10 7:07 ` Tom de Vries
@ 2022-10-11 0:44 ` Simon Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2022-10-11 0:44 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2022-10-10 03:07, Tom de Vries wrote:
> On 10/10/22 03:21, Simon Marchi wrote:
>> I see some random failures in this test:
>>
>> It can be reliably reproduced (on Linux) with this change:
>>
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 44cc28b30051..2a3c8253ba5a 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -1301,6 +1301,7 @@ proc gdb_test_multiple { command message args } {
>> }
>> set gdb_test_name "$message"
>>
>> + sleep 2
>> set result 0
>> set code [catch {gdb_expect $code} string]
>>
>
> I first tried reproducing on openSUSE Leap 15.4, and failed because
> that one has glibc 2.31, but managed on openSUSE Tumbleweed with glibc
> 2.36, which has the "pthread merged into glibc" feature that's been
> around since 2.34.
Ah, that must explain why this failure appeared after upgrading my test
machine from Ubuntu 20.04 to 22.04. I'll add a precision about that in
the commit message.
>> The problem is that the `run &` command prints some things after the
>> prompt:
>>
>> (gdb) [Thread debugging using libthread_db enabled]
>> Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
>>
>> If DejaGNU is quick enough, it will consume only up to the prompt. But
>
> FWIW, I would use expect instead of DejaGNU, because it's more specific.
Done.
>> if it is slow enough, it will consume those messages at the same time as
>> the prompt, in which case the gdb_test used for "run &" won't match. By
>> default, the prompt used by gdb_test uses a `$` to anchor the match at
>> the end of the buffer. If there's anything following the prompt, it
>> won't match.
>>
>> The diff above aadds a delay between sending the command and consuming
>
> adds
Done.
> I've tested it and confirmed that it fixes the problem.
>
> FWIW, I can also imagine we'll add this to gdb_test_multiple at some point, but that's certainly not necessary for now.
Ack.
>
>> Change-Id: I9051d8800d1c10a2e95db1a575991f7723492f1b
>> ---
>> gdb/testsuite/gdb.base/async-shell.exp | 2 +-
>> gdb/testsuite/lib/gdb.exp | 29 +++++++++++++++++++-------
>> 2 files changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/async-shell.exp b/gdb/testsuite/gdb.base/async-shell.exp
>> index e2d71826bab2..2c09863ad593 100644
>> --- a/gdb/testsuite/gdb.base/async-shell.exp
>> +++ b/gdb/testsuite/gdb.base/async-shell.exp
>> @@ -34,7 +34,7 @@ save_vars { GDBFLAGS } {
>> set gdbindex_warning_re "warning: Skipping \[^\r\n\]+ \\.gdb_index section \[^\r\n\]*\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\."
>> -gdb_test "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
>> +gdb_test -no-prompt-anchor "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
>> # `sleep 5' here would workaround the bug, do not sleep here.
>> # "shell" could eat waitpid event from the asynchronous inferior process.
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 44cc28b30051..c8d2aa511fb8 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -861,9 +861,18 @@ proc gdb_internal_error_resync {} {
>> }
>> # Fill in the default prompt if PROMPT_REGEXP is empty.
>> -proc fill_in_default_prompt {prompt_regexp} {
>> +#
>> +# If WITH_ANCHOR is true and the default prompt is used, append a `$` at the end
>> +# of the regexp, to anchor the match at the end of the buffer.
>> +proc fill_in_default_prompt {prompt_regexp with_anchor} {
>> if { "$prompt_regexp" == "" } {
>> - return "$::gdb_prompt $"
>> + set prompt "$::gdb_prompt "
>> +
>> + if { $with_anchor } {
>> + append prompt "$"
>> + }
>> +
>> + return $prompt
>> }
>> return $prompt_regexp
>> }
>> @@ -993,7 +1002,7 @@ proc gdb_test_multiple { command message args } {
>> error "Too few arguments to gdb_test_multiple"
>> }
>> - set prompt_regexp [fill_in_default_prompt $prompt_regexp]
>> + set prompt_regexp [fill_in_default_prompt $prompt_regexp true]
>> if { $message == "" } {
>> set message $command
>> @@ -1365,6 +1374,10 @@ proc gdb_test_multiline { name args } {
>> #
>> # -prompt PROMPT_REGEXP specifies a regexp matching the expected prompt
>> # after the command output. If empty, defaults to "$gdb_prompt $".
>> +# -no-prompt-anchor specifies that if the default prompt regexp is used, it
>> +# should not be anchored at the end of the buffer. This means that the
>> +# pattern can match even if there is stuff output after the prompt. Does not
>> +# have any effect if -prompt is specified.
>> # -lbl specifies that line-by-line matching will be used.
>> # -nopass specifies that a PASS should not be issued.
>> #
>> @@ -1379,6 +1392,7 @@ proc gdb_test { args } {
>> parse_args {
>> {prompt ""}
>> + {no-prompt-anchor}
>> {lbl}
>> {nopass}
>> }
>> @@ -1394,7 +1408,7 @@ proc gdb_test { args } {
>> set message $command
>> }
>> - set prompt [fill_in_default_prompt $prompt]
>> + set prompt [fill_in_default_prompt $prompt [expr !${no-prompt-anchor}]]
>> set saw_question 0
>> @@ -1471,20 +1485,21 @@ if { [tcl_version_at_least 8 5] == 0 } {
>> # gdb_test_no_output [-prompt PROMPT_REGEXP] [-nopass] COMMAND [MESSAGE]
>> # Send a command to GDB and verify that this command generated no output.
>> #
>> -# See gdb_test for a description of the -prompt, -nopass, COMMAND, and
>> -# MESSAGE parameters.
>> +# See gdb_test for a description of the -prompt, -no-prompt-anchor, -nopass,
>> +# COMMAND, and MESSAGE parameters.
>> proc gdb_test_no_output { args } {
>> global gdb_prompt
>> parse_args {
>> {prompt ""}
>> + {no-prompt-anchor}
>> {nopass}
>> }
>> lassign $args command message
>> - set prompt [fill_in_default_prompt $prompt]
>> + set prompt [fill_in_default_prompt $prompt [expr !${no-prompt-anchor}]]
>>
>
> FWIW, I've wondered whether giving fill_in_default_prompt a
> no-prompt-anchor parameter instead of with_anchor would be better,
> avoiding the inversion here and using consistent naming for the same
> concept across different procs, but I haven't been able to make up my
> mind about whether I really like it better ... I thought I just
> mention it.
That's what I did initially, but then it feels weird for the call sites
that do want the prompt, they have to pass false to mean "I want
no-no-prompt-anchor". That makes a double negative, which I find
confusing. I don't have a strong opinion otherwise.
> Anyway, LGTM.
Thanks for the review, will push (and add you Approved-By).
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-11 0:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-10 1:21 [PATCH] gdb/testsuite: fix race in gdb.base/async-shell.exp Simon Marchi
2022-10-10 7:07 ` Tom de Vries
2022-10-11 0:44 ` Simon Marchi
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).