diff options
author | Pedro Alves <palves@redhat.com> | 2014-07-25 10:07:38 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2014-07-25 10:07:38 +0100 |
commit | c3f814a14336b9d395f3abad739592929e2faaa0 (patch) | |
tree | bc9a845f1ae13c8616d0d987eec9ba86a07a61a3 | |
parent | a3673aac3c2904957579a23f9d61bd3fd13d4d55 (diff) | |
download | gdb-c3f814a14336b9d395f3abad739592929e2faaa0.zip gdb-c3f814a14336b9d395f3abad739592929e2faaa0.tar.gz gdb-c3f814a14336b9d395f3abad739592929e2faaa0.tar.bz2 |
Fix paginate-*.exp races
Jan pointed out in
<https://sourceware.org/ml/gdb-patches/2014-07/msg00553.html> that
these testcases have racy results:
gdb.base/double-prompt-target-event-error.exp
gdb.base/paginate-after-ctrl-c-running.exp
gdb.base/paginate-bg-execution.exp
gdb.base/paginate-execution-startup.exp
gdb.base/paginate-inferior-exit.exp
This is easily reproducible with "read1" from:
[reproducer for races of expect incomplete reads]
http://sourceware.org/bugzilla/show_bug.cgi?id=12649
The '-notransfer -re "<return>" { exp_continue }' trick in the current
tests doesn't actually work.
The issue that led to the -notransfer trick was that
"---Type <return> to continue, or q <return> to quit---"
has two "<return>"s. If one wants gdb_test_multiple to not hit the
built-in "<return>" match that results in FAIL, one has to expect the
pagination prompt in chunks, first up to the first "<return>", then
again, up to the second. Something around these lines:
gdb_test_multiple "" $test {
-re "<return>" {
exp_continue
}
-re "to quit ---" {
pass $test
}
}
The intent was for -notransfer+exp_continue to make expect fetch more
input, and rerun the matches against the now potentially fuller
buffer, and then eventually the -re that includes the full pagination
prompt regex would match instead (because it's listed higher up, it
would match first). But, once that "<return>" -notransfer -re
matches, it keeps re-matching forever. It seems like with
exp_continue, expect immediately retries matching, instead of first
reading in more data into the buffer, if available.
Fix this like I should have done in the first place. There's actually
no good reason for gdb_test_multiple to only match "<return>". We can
make gdb_test_multiple expect the whole pagination prompt text
instead, which is store in the 'pagination_prompt' global (similar to
'gdb_prompt'). Then a gdb_test_multiple caller that doesn't want the
default match to trigger, because it wants to see one pagination
prompt, does simply:
gdb_test_multiple "" $test {
-re "$pagination_prompt$" {
pass $test
}
}
which is just like when we don't want the default $gdb_prompt match
within gdb_test_multiple to trigger, like:
gdb_test_multiple "" $test {
-re "$gdb_prompt $" {
pass $test
}
}
Tested on x86_64 Fedora 20. In addition, I've let the racy tests run
all in parallel in a loop for 30 minutes, and they never failed.
gdb/testsuite/
2014-07-25 Pedro Alves <palves@redhat.com>
* gdb.base/double-prompt-target-event-error.exp
(cancel_pagination_in_target_event): Remove '-notransfer <return>'
match.
(cancel_pagination_in_target_event): Rework double prompt
detection.
* gdb.base/paginate-after-ctrl-c-running.exp
(test_ctrlc_while_target_running_paginates): Remove '-notransfer
<return>' match.
* gdb.base/paginate-bg-execution.exp
(test_bg_execution_pagination_return)
(test_bg_execution_pagination_cancel): Remove '-notransfer
<return>' matches.
* gdb.base/paginate-execution-startup.exp
(test_fg_execution_pagination_return)
(test_fg_execution_pagination_cancel): Remove '-notransfer
<return>' matches.
* gdb.base/paginate-inferior-exit.exp
(test_paginate_inferior_exited): Remove '-notransfer <return>'
match.
* lib/gdb-utils.exp (string_to_regexp): Move here from lib/gdb.exp.
* lib/gdb.exp (pagination_prompt): Run text through
string_to_regexp.
(gdb_test_multiple): Match $pagination_prompt instead of
"<return>".
(string_to_regexp): Move to lib/gdb-utils.exp.
-rw-r--r-- | gdb/testsuite/ChangeLog | 28 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/double-prompt-target-event-error.exp | 26 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp | 4 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/paginate-bg-execution.exp | 9 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/paginate-execution-startup.exp | 8 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/paginate-inferior-exit.exp | 4 | ||||
-rw-r--r-- | gdb/testsuite/lib/gdb-utils.exp | 9 | ||||
-rw-r--r-- | gdb/testsuite/lib/gdb.exp | 14 |
8 files changed, 56 insertions, 46 deletions
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index f534f18..bd8cf44 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,31 @@ +2014-07-25 Pedro Alves <palves@redhat.com> + + * gdb.base/double-prompt-target-event-error.exp + (cancel_pagination_in_target_event): Remove '-notransfer <return>' + match. + (cancel_pagination_in_target_event): Rework double prompt + detection. + * gdb.base/paginate-after-ctrl-c-running.exp + (test_ctrlc_while_target_running_paginates): Remove '-notransfer + <return>' match. + * gdb.base/paginate-bg-execution.exp + (test_bg_execution_pagination_return) + (test_bg_execution_pagination_cancel): Remove '-notransfer + <return>' matches. + * gdb.base/paginate-execution-startup.exp + (test_fg_execution_pagination_return) + (test_fg_execution_pagination_cancel): Remove '-notransfer + <return>' matches. + * gdb.base/paginate-inferior-exit.exp + (test_paginate_inferior_exited): Remove '-notransfer <return>' + match. + * lib/gdb-utils.exp (string_to_regexp): Move here from lib/gdb.exp. + * lib/gdb.exp (pagination_prompt): Run text through + string_to_regexp. + (gdb_test_multiple): Match $pagination_prompt instead of + "<return>". + (string_to_regexp): Move to lib/gdb-utils.exp. + 2014-07-22 Jan Kratochvil <jan.kratochvil@redhat.com> * gdb.arch/amd64-entry-value-paramref.S: New file. diff --git a/gdb/testsuite/gdb.base/double-prompt-target-event-error.exp b/gdb/testsuite/gdb.base/double-prompt-target-event-error.exp index 5571cdf..84c6c45 100644 --- a/gdb/testsuite/gdb.base/double-prompt-target-event-error.exp +++ b/gdb/testsuite/gdb.base/double-prompt-target-event-error.exp @@ -75,10 +75,6 @@ proc cancel_pagination_in_target_event { command } { set saw_continuing 1 exp_continue } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an error. - exp_continue - } } # We're now stopped in a pagination query while handling a @@ -87,18 +83,28 @@ proc cancel_pagination_in_target_event { command } { # output. send_gdb "\003p 1\n" + # Note gdb_test_multiple has a default match for the prompt, + # which issues a FAIL. Consume the first prompt. + set test "first prompt" + gdb_test_multiple "" $test { + -re "$gdb_prompt" { + pass "first prompt" + } + } + + # We should only see one prompt more, and it should be + # preceeded by print's output. set test "no double prompt" gdb_test_multiple "" $test { - -re "$gdb_prompt.*$gdb_prompt.*$gdb_prompt $" { + -re "$gdb_prompt.*$gdb_prompt $" { + # The bug is present, and expect managed to read + # enough characters into the buffer to fill it with + # both prompts. fail $test } - -re "$gdb_prompt .* = 1\r\n$gdb_prompt $" { + -re " = 1\r\n$gdb_prompt $" { pass $test } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an error. - exp_continue - } } # In case the board file wants to send further commands. diff --git a/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp b/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp index 0ed8c92..5898d5b 100644 --- a/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp +++ b/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp @@ -70,10 +70,6 @@ proc test_ctrlc_while_target_running_paginates {} { -re "$gdb_prompt $" { gdb_assert $saw_pagination_prompt $test } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an error. - exp_continue - } } # Confirm GDB can still process input. diff --git a/gdb/testsuite/gdb.base/paginate-bg-execution.exp b/gdb/testsuite/gdb.base/paginate-bg-execution.exp index dcff8ad..116cc2b 100644 --- a/gdb/testsuite/gdb.base/paginate-bg-execution.exp +++ b/gdb/testsuite/gdb.base/paginate-bg-execution.exp @@ -51,11 +51,6 @@ proc test_bg_execution_pagination_return {} { send_gdb "\n" exp_continue } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an - # error. - exp_continue - } -re "after sleep\[^\r\n\]+\r\n$" { gdb_assert $saw_pagination_prompt $test } @@ -96,10 +91,6 @@ proc test_bg_execution_pagination_cancel { how } { -re "$pagination_prompt$" { pass $test } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an error. - exp_continue - } } set test "cancel pagination" diff --git a/gdb/testsuite/gdb.base/paginate-execution-startup.exp b/gdb/testsuite/gdb.base/paginate-execution-startup.exp index dc713ec..1628a0f 100644 --- a/gdb/testsuite/gdb.base/paginate-execution-startup.exp +++ b/gdb/testsuite/gdb.base/paginate-execution-startup.exp @@ -111,10 +111,6 @@ proc test_fg_execution_pagination_return {} { send_gdb "\n" exp_continue } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an error. - exp_continue - } -re "$gdb_prompt $" { gdb_assert $saw_pagination_prompt $test } @@ -154,10 +150,6 @@ proc test_fg_execution_pagination_cancel { how } { -re "$pagination_prompt$" { pass $test } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an error. - exp_continue - } } set test "cancel pagination" diff --git a/gdb/testsuite/gdb.base/paginate-inferior-exit.exp b/gdb/testsuite/gdb.base/paginate-inferior-exit.exp index 0e37be9..7c63289 100644 --- a/gdb/testsuite/gdb.base/paginate-inferior-exit.exp +++ b/gdb/testsuite/gdb.base/paginate-inferior-exit.exp @@ -58,10 +58,6 @@ proc test_paginate_inferior_exited {} { set saw_starting 1 exp_continue } - -notransfer -re "<return>" { - # Otherwise gdb_test_multiple considers this an error. - exp_continue - } } # We're now stopped in a pagination output while handling a diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp index 7e03cbf..0af437e 100644 --- a/gdb/testsuite/lib/gdb-utils.exp +++ b/gdb/testsuite/lib/gdb-utils.exp @@ -28,3 +28,12 @@ proc gdb_init_commands {} { } return $commands } + +# Given an input string, adds backslashes as needed to create a +# regexp that will match the string. + +proc string_to_regexp {str} { + set result $str + regsub -all {[]*+.|()^$\[\\]} $str {\\&} result + return $result +} diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 7a00efb..8cb98ae 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -71,7 +71,7 @@ if ![info exists gdb_prompt] then { } # A regexp that matches the pagination prompt. -set pagination_prompt "---Type <return> to continue, or q <return> to quit---" +set pagination_prompt [string_to_regexp "---Type <return> to continue, or q <return> to quit---"] # The variable fullname_syntax_POSIX is a regexp which matches a POSIX # absolute path ie. /foo/ @@ -649,7 +649,7 @@ proc gdb_internal_error_resync {} { # proc gdb_test_multiple { command message user_code } { global verbose use_gdb_stub - global gdb_prompt + global gdb_prompt pagination_prompt global GDB global inferior_exited_re upvar timeout timeout @@ -873,7 +873,7 @@ proc gdb_test_multiple { command message user_code } { } set result 1 } - "<return>" { + -re "$pagination_prompt" { send_gdb "\n" perror "Window too small." fail "$message" @@ -1109,14 +1109,6 @@ proc test_print_reject { args } { } } -# Given an input string, adds backslashes as needed to create a -# regexp that will match the string. - -proc string_to_regexp {str} { - set result $str - regsub -all {[]*+.|()^$\[\\]} $str {\\&} result - return $result -} # Same as gdb_test, but the second parameter is not a regexp, # but a string that must match exactly. |