From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6091 invoked by alias); 12 Sep 2019 17:15:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 5496 invoked by uid 89); 12 Sep 2019 17:15:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=fed, Cosmetic, stress X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Sep 2019 17:15:45 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 81EB8AC26; Thu, 12 Sep 2019 17:15:43 +0000 (UTC) Subject: Re: [PATCH, 8.3][gdb/testsuite] Mark watchthreads-reorder.exp FAIL as KFAIL From: Tom de Vries To: Kevin Buettner , gdb-patches@sourceware.org References: <20190330095831.5616-1-philippe.waroquiers@skynet.be> <20190401093248.615b2c5c@f29-4.lan> <1554145505.1854.7.camel@skynet.be> <57392d44-7d10-7009-2f5e-fd790adcbef4@suse.de> <20190912043043.1f57c212@f29-4.lan> Openpgp: preference=signencrypt Message-ID: <07c39dcb-deb1-c59c-7dbe-6ff8d6dec07d@suse.de> Date: Thu, 12 Sep 2019 17:15:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00221.txt.bz2 [ 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 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 > > * 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 >