[PATCH, 8.3][gdb/testsuite] Mark watchthreads-reorder.exp FAIL as KFAIL
Tom de Vries
tdevries@suse.de
Thu Sep 12 17:15:00 GMT 2019
[ resending, got "Undelivered Mail Returned to Sender" for
gdb-patches@sourceware.org ]
On 12-09-19 19:12, Tom de Vries wrote:
> [ was: Re: [RFAv2] Fix internal error and improve 'set debug infrun
> 1'/target wait kind trace ]
> On 12-09-19 13:30, Kevin Buettner wrote:
>> On Wed, 11 Sep 2019 19:05:19 +0200
>> Tom de Vries <tdevries@suse.de> wrote:
>>
>>> I ran today into the failure that this commit fixes:
>>> ...
>>> FAIL: gdb.threads/watchthreads-reorder.exp: reorder1: continue to
>>> breakpoint: break-at-exit (GDB internal error)
>>> ...
>>> on the 8.3 branch.
>>>
>>> My understanding from reading the rationale is that this is sufficiently
>>> cornercase to not merit a backport, but perhaps someone thinks otherwise?
>>>
>>> If we decide not to backport, we could perhaps mark this as as KFAIL in
>>> the 8.3 branch?
>> Marking it as a KFAIL is okay with me...
>>
> This patch implements the KFAIL, but does so by adding an extra argument
> to gdb_test_multiple, which is perhaps a bit intrusive for a release branch.
>
> I'm also fine with just doing:
> ...
> + setup_kfail gdb/24995 "*-*-*"
> gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*"
> ...
> or following up on any other suggestion.
>
> Thanks,
> - Tom
>
>
> 0001-gdb-testsuite-Mark-watchthreads-reorder.exp-FAIL-as-KFAIL.patch
>
> [gdb/testsuite] Mark watchthreads-reorder.exp FAIL as KFAIL
>
> When running gdb.threads/watchthreads-reorder.exp in parallel with:
> ...
> $ n=$(grep -c processor /proc/cpuinfo); n=$((($n + 1) / 2)); stress -c $n
> ...
> there's a reasonable change to trigger an internal gdb error:
> ...
> $ for n in $(seq 1 10); do ./test.sh; done 2>&1 \
> | grep "expected passes" \
> | sort \
> | uniq -c
> 1 # of expected passes 14
> 2 # of expected passes 15
> 1 # of expected passes 16
> 6 # of expected passes 17
> ...
> which look like this in gdb.sum:
> ...
> FAIL: gdb.threads/watchthreads-reorder.exp: reorder1: continue to breakpoint: \
> break-at-exit (GDB internal error)
> ...
>
> This FAIL is filed as PR gdb/24995 and fixed on master by commit c29705b71a
> "Fix internal error and improve 'set debug infrun 1'/target wait kind trace".
>
> Mark this as KFAIL for the 8.3 branch.
>
> It's trivial to do this by adding a setup_kfail:
> ...
> + setup_kfail gdb/24995 "*-*-*"
> gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*"
> ...
> but we'll get a fair amount of KPASSES:
> ...
> KPASS: gdb.threads/watchthreads-reorder.exp: reorder0: \
> continue to breakpoint: break-at-exit (PRMS gdb/24995)
> KPASS: gdb.threads/watchthreads-reorder.exp: reorder1: \
> continue to breakpoint: break-at-exit (PRMS gdb/24995)
> ...
>
> Instead, do this more precise by only KFAILing in case the internal error is detected.
>
> Tested on x86_64-linux.
>
> gdb/testsuite/ChangeLog:
>
> 2019-09-12 Tom de Vries <tdevries@suse.de>
>
> * gdb.threads/watchthreads-reorder.exp: Add PR gdb/24995 KFAIL.
> * lib/gdb.exp (prepare_user_code): New proc, factored out of ...
> (gdb_test_multiple): ... here. Add and handle optional argument
> early_user_code.
>
> ---
> gdb/testsuite/gdb.threads/watchthreads-reorder.exp | 15 +-
> gdb/testsuite/lib/gdb.exp | 167 ++++++++++++---------
> 2 files changed, 110 insertions(+), 72 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.threads/watchthreads-reorder.exp b/gdb/testsuite/gdb.threads/watchthreads-reorder.exp
> index 9bbbb6f2b8..efbaef1e63 100644
> --- a/gdb/testsuite/gdb.threads/watchthreads-reorder.exp
> +++ b/gdb/testsuite/gdb.threads/watchthreads-reorder.exp
> @@ -90,5 +90,18 @@ foreach reorder {0 1} { with_test_prefix "reorder$reorder" {
> # found in the DEBUG_INFRUN code path.
> gdb_test "set debug infrun 1"
>
> - gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*"
> + # Do:
> + # gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*"
> + # with setup_kfail.
> + set msg "continue to breakpoint: break-at-exit"
> + gdb_test_multiple "continue" $msg {
> + -re "internal-error: inferior\\* find_inferior_pid\\(int\\): Assertion .pid != 0. failed\\." {
> + setup_kfail gdb/24995 "*-*-*"
> + exp_continue
> + }
> + } {
> + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) .*break-at-exit.*\r\n$gdb_prompt $" {
> + pass $msg
> + }
> + }
> }}
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 3d5f8726f7..2cddd5cf60 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -694,14 +694,92 @@ proc gdb_internal_error_resync {} {
> return 0
> }
>
> +# Prepare expect arguments for execution in gdb_test_multiple.
> +#
> +proc prepare_user_code { user_code } {
> + # TCL/EXPECT WART ALERT
> + # Expect does something very strange when it receives a single braced
> + # argument. It splits it along word separators and performs substitutions.
> + # This means that { "[ab]" } is evaluated as "[ab]", but { "\[ab\]" } is
> + # evaluated as "\[ab\]". But that's not how TCL normally works; inside a
> + # double-quoted list item, "\[ab\]" is just a long way of representing
> + # "[ab]", because the backslashes will be removed by lindex.
> +
> + # Unfortunately, there appears to be no easy way to duplicate the splitting
> + # that expect will do from within TCL. And many places make use of the
> + # "\[0-9\]" construct, so we need to support that; and some places make use
> + # of the "[func]" construct, so we need to support that too. In order to
> + # get this right we have to substitute quoted list elements differently
> + # from braced list elements.
> +
> + # We do this roughly the same way that Expect does it. We have to use two
> + # lists, because if we leave unquoted newlines in the argument to uplevel
> + # they'll be treated as command separators, and if we escape newlines
> + # we mangle newlines inside of command blocks. This assumes that the
> + # input doesn't contain a pattern which contains actual embedded newlines
> + # at this point!
> + uplevel {
> + regsub -all {\n} ${user_code} { } subst_code
> + set subst_code [uplevel list $subst_code]
> +
> + set processed_code ""
> + set patterns ""
> + set expecting_action 0
> + set expecting_arg 0
> + foreach item $user_code subst_item $subst_code {
> + if { $item == "-n" || $item == "-notransfer" || $item == "-nocase" } {
> + lappend processed_code $item
> + continue
> + }
> + if { $item == "-indices" || $item == "-re" || $item == "-ex" } {
> + lappend processed_code $item
> + continue
> + }
> + if { $item == "-timeout" || $item == "-i" } {
> + set expecting_arg 1
> + lappend processed_code $item
> + continue
> + }
> + if { $expecting_arg } {
> + set expecting_arg 0
> + lappend processed_code $subst_item
> + continue
> + }
> + if { $expecting_action } {
> + lappend processed_code "uplevel [list $item]"
> + set expecting_action 0
> + # Cosmetic, no effect on the list.
> + append processed_code "\n"
> + continue
> + }
> + set expecting_action 1
> + lappend processed_code $subst_item
> + if {$patterns != ""} {
> + append patterns "; "
> + }
> + append patterns "\"$subst_item\""
> + }
> +
> + # Also purely cosmetic.
> + regsub -all {\r} $patterns {\\r} patterns
> + regsub -all {\n} $patterns {\\n} patterns
> +
> + if $verbose>2 then {
> + send_user "Looking to match \"$patterns\"\n"
> + }
> + return $processed_code
> + }
> +}
>
> -# gdb_test_multiple COMMAND MESSAGE EXPECT_ARGUMENTS
> +# gdb_test_multiple COMMAND MESSAGE [EARLY_EXPECT_ARGUMENTS] EXPECT_ARGUMENTS
> # Send a command to gdb; test the result.
> #
> # COMMAND is the command to execute, send to GDB with send_gdb. If
> # this is the null string no command is sent.
> # MESSAGE is a message to be printed with the built-in failure patterns
> # if one of them matches. If MESSAGE is empty COMMAND will be used.
> +# EARLY_EXPECT_ARGUMENTS as EXPECT_ARGUMENTS, but will be fed to expect
> +# before the standard patterns.
> # EXPECT_ARGUMENTS will be fed to expect in addition to the standard
> # patterns. Pattern elements will be evaluated in the caller's
> # context; action elements will be executed in the caller's context.
> @@ -744,7 +822,7 @@ proc gdb_internal_error_resync {} {
> # expected from $gdb_spawn_id. IOW, callers do not need to worry
> # about resetting "-i" back to $gdb_spawn_id explicitly.
> #
> -proc gdb_test_multiple { command message user_code } {
> +proc gdb_test_multiple { command message args } {
> global verbose use_gdb_stub
> global gdb_prompt pagination_prompt
> global GDB
> @@ -754,6 +832,16 @@ proc gdb_test_multiple { command message user_code } {
> upvar expect_out expect_out
> global any_spawn_id
>
> + if { [llength $args] == 2 } {
> + set early_user_code [lindex $args 0]
> + set user_code [lindex $args 1]
> + } elseif { [llength $args] == 1 } {
> + set early_user_code {}
> + set user_code [lindex $args 0]
> + } else {
> + error "Invalid number of arguments for gdb_test_multiple"
> + }
> +
> if { $message == "" } {
> set message $command
> }
> @@ -772,76 +860,12 @@ proc gdb_test_multiple { command message user_code } {
> error "gdbserver does not support $command without extended-remote"
> }
>
> - # TCL/EXPECT WART ALERT
> - # Expect does something very strange when it receives a single braced
> - # argument. It splits it along word separators and performs substitutions.
> - # This means that { "[ab]" } is evaluated as "[ab]", but { "\[ab\]" } is
> - # evaluated as "\[ab\]". But that's not how TCL normally works; inside a
> - # double-quoted list item, "\[ab\]" is just a long way of representing
> - # "[ab]", because the backslashes will be removed by lindex.
> -
> - # Unfortunately, there appears to be no easy way to duplicate the splitting
> - # that expect will do from within TCL. And many places make use of the
> - # "\[0-9\]" construct, so we need to support that; and some places make use
> - # of the "[func]" construct, so we need to support that too. In order to
> - # get this right we have to substitute quoted list elements differently
> - # from braced list elements.
> -
> - # We do this roughly the same way that Expect does it. We have to use two
> - # lists, because if we leave unquoted newlines in the argument to uplevel
> - # they'll be treated as command separators, and if we escape newlines
> - # we mangle newlines inside of command blocks. This assumes that the
> - # input doesn't contain a pattern which contains actual embedded newlines
> - # at this point!
> -
> - regsub -all {\n} ${user_code} { } subst_code
> - set subst_code [uplevel list $subst_code]
> -
> - set processed_code ""
> - set patterns ""
> - set expecting_action 0
> - set expecting_arg 0
> - foreach item $user_code subst_item $subst_code {
> - if { $item == "-n" || $item == "-notransfer" || $item == "-nocase" } {
> - lappend processed_code $item
> - continue
> - }
> - if { $item == "-indices" || $item == "-re" || $item == "-ex" } {
> - lappend processed_code $item
> - continue
> - }
> - if { $item == "-timeout" || $item == "-i" } {
> - set expecting_arg 1
> - lappend processed_code $item
> - continue
> - }
> - if { $expecting_arg } {
> - set expecting_arg 0
> - lappend processed_code $subst_item
> - continue
> - }
> - if { $expecting_action } {
> - lappend processed_code "uplevel [list $item]"
> - set expecting_action 0
> - # Cosmetic, no effect on the list.
> - append processed_code "\n"
> - continue
> - }
> - set expecting_action 1
> - lappend processed_code $subst_item
> - if {$patterns != ""} {
> - append patterns "; "
> - }
> - append patterns "\"$subst_item\""
> - }
> -
> - # Also purely cosmetic.
> - regsub -all {\r} $patterns {\\r} patterns
> - regsub -all {\n} $patterns {\\n} patterns
> -
> if $verbose>2 then {
> send_user "Sending \"$command\" to gdb\n"
> - send_user "Looking to match \"$patterns\"\n"
> + }
> + set processed_code [prepare_user_code $user_code]
> + set early_processed_code [prepare_user_code $early_user_code]
> + if $verbose>2 then {
> send_user "Message is \"$message\"\n"
> }
>
> @@ -891,7 +915,8 @@ proc gdb_test_multiple { command message user_code } {
> }
> }
>
> - set code {
> + set code $early_user_code
> + append code {
> -re ".*A problem internal to GDB has been detected" {
> fail "$message (GDB internal error)"
> gdb_internal_error_resync
>
More information about the Gdb-patches
mailing list