]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Fix paginate-*.exp races
authorPedro Alves <palves@redhat.com>
Fri, 25 Jul 2014 09:21:12 +0000 (10:21 +0100)
committerPedro Alves <palves@redhat.com>
Fri, 25 Jul 2014 09:21:12 +0000 (10:21 +0100)
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 7.8: lib/gdb-utils.exp didn't exist in the branch yet, so this patch adds it.]

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: New file.
* lib/gdb.exp: Load gdb-utils.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.

gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/double-prompt-target-event-error.exp
gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp
gdb/testsuite/gdb.base/paginate-bg-execution.exp
gdb/testsuite/gdb.base/paginate-execution-startup.exp
gdb/testsuite/gdb.base/paginate-inferior-exit.exp
gdb/testsuite/lib/gdb-utils.exp [new file with mode: 0644]
gdb/testsuite/lib/gdb.exp

index 291c81cf01f4f61d53bb7e941bfaeaee434a39f9..300ef643c8f06058f522631a9fe56623636e8a6e 100644 (file)
@@ -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.
index 5571cdff771c55646944b74ce8c65c3a2947e0f9..84c6c45a1d886a2f3fdb0d9524653eda7fb152e9 100644 (file)
@@ -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.
index 0ed8c926cf2ffa6666e9def6aafddab06d5965e7..5898d5b33eaabba90e0f88a2d5fea631c3574950 100644 (file)
@@ -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.
index dcff8ad29e7118f27b14f2036e357f05d712e26e..116cc2b6e832eb0ca7f35772ad9e909585aa274d 100644 (file)
@@ -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"
index dc713ec996add9d5b3982305b3bdfab1caa11676..1628a0fe5ec810a7a768c2f7f7ba27a6277ee330 100644 (file)
@@ -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"
index 0e37be97994352de0b42cc31d1adfd77131255b9..7c6328981f790aa545abd49e396871c8b0d4c2a9 100644 (file)
@@ -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
new file mode 100644 (file)
index 0000000..b9968d0
--- /dev/null
@@ -0,0 +1,25 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Utility procedures, shared between test suite domains.
+
+# 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
+}
index 3533ee390846d9b3004720bc4a5f3d293963608e..7650d2a9753afb12cdcd00fce912a8151e463b0f 100644 (file)
@@ -27,6 +27,7 @@ if {$tool == ""} {
 
 load_lib libgloss.exp
 load_lib cache.exp
+load_lib gdb-utils.exp
 
 global GDB
 
@@ -70,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/ 
@@ -648,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
@@ -872,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"
@@ -1108,14 +1109,6 @@ proc test_print_reject { args } {
     }
 }
 \f
-# 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.