From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id AFBF53858C78 for ; Fri, 12 May 2023 12:54:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AFBF53858C78 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683896057; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=TTV1ngsDA/mR0xo284rlFpJkT/ipL6BtMQo8cL5sOIM=; b=gUE8/oFaL+BF91jbl694Ph049Eke81Az+cc4vWP+iH37vj84bQ+TGNZHFGOP3ppWWBTOpl 8wGiyIVaNN7pO4rhmTuy/hRtyHXsxD7vZHhcr58Vk968tH2wOZfX44y9+2wHgf7xJdp/q3 qTCN99BnDfDUX7w1ATDuRgOxHbgHmSE= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-343-sWKEAokVO8qg3bkjKcZQPQ-1; Fri, 12 May 2023 08:54:16 -0400 X-MC-Unique: sWKEAokVO8qg3bkjKcZQPQ-1 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-3f422dc5fafso41048125e9.0 for ; Fri, 12 May 2023 05:54:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683896054; x=1686488054; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=TTV1ngsDA/mR0xo284rlFpJkT/ipL6BtMQo8cL5sOIM=; b=AilMKen9Iyz+WuB0iEC0BHUnn9WmeY+8DgtC6mEXeHNVS+Yj8ybw803Es/+XTZdLd1 Fa38y7B/AqGcekGGpwAG2UHibgeb13FY2q+u/zVPl0/Pr6F3L08tWbof2VA6bfr3fssm 8N1F4DRAEanN3OacMmKIVqLOGasVpDWidUD+ZsHvBV4ePp3P2a1dcpnSI+pjpHpSvkJ3 El/H+cl3sTGFWEtooMMbEQvGIBQkqL8oJzuxWBfib2sETVyizCURRddNvByaYnbE1Dc9 NuaTZbRuhxgRgUSW54CR9aVxmiMYCrsKWKpVeoC4SHB4h9XkxZOWcNUrcVtpq4k1TQT2 8IxA== X-Gm-Message-State: AC+VfDyhp0ywbNFD2VdG3Vu/xiAUOXAzMmBcFKjynMLNVFyw6RIu05U+ 6LCq6Ak/O2QnpKBEMgenv2T27UdT8+Bni1C2/nIybNRuzQJZdBpLeifyri/VX7ZeSugc3HrT2eM m3bI4uwo/WPzPI/OQ+h1bM9ck2Hb4oQ== X-Received: by 2002:adf:e547:0:b0:2e4:bfa0:8c30 with SMTP id z7-20020adfe547000000b002e4bfa08c30mr18735451wrm.47.1683896054631; Fri, 12 May 2023 05:54:14 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6PwgpMS7d1TJAkha4oWmtOXvigRIooxvx8stEbzmolJMOkwMNjpvstg62hlHgCTuh/yu2NvQ== X-Received: by 2002:adf:e547:0:b0:2e4:bfa0:8c30 with SMTP id z7-20020adfe547000000b002e4bfa08c30mr18735435wrm.47.1683896054163; Fri, 12 May 2023 05:54:14 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id l11-20020a7bc34b000000b003f195d2f1a9sm28448132wmj.15.2023.05.12.05.54.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 May 2023 05:54:13 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [PATCH 4/5] gdb/testsuite: change newline patterns used in gdb_test In-Reply-To: <298cc5cd-1427-c9b7-7e7e-1077a2b776c5@suse.de> References: <464e64e3a3483c228f0a73c778bcaf79e4595abd.1680293848.git.aburgess@redhat.com> <7552d3ad-c148-f0ea-b219-dd2d9458de0c@suse.de> <871qk09l5j.fsf@redhat.com> <14333280-6e66-0cc5-7e34-46176588ee37@suse.de> <87ttwv7zq8.fsf@redhat.com> <87354a90g5.fsf@redhat.com> <87r0rp7ru7.fsf@redhat.com> <298cc5cd-1427-c9b7-7e7e-1077a2b776c5@suse.de> Date: Fri, 12 May 2023 13:54:12 +0100 Message-ID: <87r0rl677v.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom de Vries writes: > On 5/9/23 11:54, Andrew Burgess wrote: >> Andrew Burgess writes: >> >>> Tom de Vries writes: >>> >>>> On 5/2/23 13:13, Andrew Burgess wrote: >>>>> Tom de Vries writes: >>>>> >>>>>> On 5/1/23 16:33, Andrew Burgess wrote: >>>>>>> Tom de Vries writes: >>>>>>> >>>>>>>> On 3/31/23 22:20, Andrew Burgess via Gdb-patches wrote: >>>>>>>>> This commit makes two changes to how we match newline characters in >>>>>>>>> the gdb_test proc. >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> the -wrap used in gdb_test_multiple is defined in terms of gdb_test >>>>>>>> semantics, but it doesn't seem to have been updated to match the new >>>>>>>> behaviour in gdb_test. >>>>>>>> >>>>>>>> I've filed a PR about this regression ( >>>>>>>> https://sourceware.org/bugzilla/show_bug.cgi?id=30403 ). >>>>>>> >>>>>>> Sorry for any problems caused. I'm not working today, but if this has >>>>>>> not been addressed, I'll look at this on Tuesday. >>>>>> >>>>>> AFAIU it's a silent regression, so there are no problems in term of >>>>>> FAILs, it's just that more work is required. >>>>>> >>>>>> FWIW, I think the root cause for introducing this regression silently is >>>>>> that we try to implement the same thing in two different locations, and >>>>>> it's just easy for things to get out of sync. I recently fixed >>>>>> something similar in commit 4fa173cfd79 ("[gdb/testsuite] Fix -wrap in >>>>>> presence of -prompt in gdb_test_multiple"), that's why I noticed it. >>>>> >>>>> So I believe the patch below brings gdb_test_multiple with '-wrap' back >>>>> into line with gdb_test. I also updated a couple of other places that >>>>> used the same (old) gdb_test pattern. >>>>> >>>>> There were nowhere near as many regressions with this change as with >>>>> gdb_test. Let me know what you think. >>>>> >>>> >>>> I've applied the patch and tested it, and saw no regression. I've also >>>> reviewed the patch and LGTM. >>> >>> Thanks Tom, >>> >>> I've gone ahead and pushed this patch -- fixing up the newline issues. >>> >>>> >>>> However, I've now also realized that the ^ bit is missing, which was >>>> also added in this patch series. >>>> >>>> In other words, say we have: >>>> ... >>>> gdb_test "print 1" "^.$decimal = 1" >>>> ... >>>> which passes fine. >>>> >>>> But then we want to annotate the PASS message with the captured >>>> $decimal, and rewrite into: >>>> ... >>>> gdb_test_multiple "print 1" "" { >>>> -re -wrap "^.($decimal) = 1" { >>>> set var_nr $expect_output(1,string) >>>> pass "$gdb_test_name (var_nr: $var_nr)" >>>> } >>>> } >>>> ... >>>> This FAILs because ^ at the start of the pattern is not handled the same >>>> way by -wrap as by gdb_test. >>> >>> I'm working on an updated patch that addresses the '^' feature for >>> gdb_test_multiple. I'm still testing this locally, and it will probably >>> be next week now before I post this -- but I will get this done, watch >>> this space :) >> >> Tom, >> >> Below is a patch that extends the '^' support to gdb_test_multiple (when >> -wrap is used). >> >> Let me know what you think. >> > > Hi Andrew, > > I've applied the patch and tested it. I ran into trouble in two recent > (more recent than your patch) test-cases, gdb.tui/wrap-line.exp and > gdb.base/wrap-line.exp, which both need updating. I made the obvious fixes for these two tests, and pushed this patch. Thanks for the review. Andrew > > Otherwise LGTM. > > Reviewed-by: Tom de Vries > > Thanks, > - Tom > >> Thanks, >> Andrew >> >> --- >> >> commit 1355a1d7eca5c1dac1c74c98634389525c78d877 >> Author: Andrew Burgess >> Date: Tue May 9 10:28:42 2023 +0100 >> >> gdb/testsuite: extend special '^' handling to gdb_test_multiple >> >> The commit: >> >> commit 08ec06d6440745ef9204d39197aa1e732df41056 >> Date: Wed Mar 29 10:41:07 2023 +0100 >> >> gdb/testsuite: special case '^' in gdb_test pattern >> >> Added some special handling of '^' to gdb_test -- a leading '^' will >> cause the command regexp to automatically be included in the expected >> output pattern. >> >> It was pointed out that the '-wrap' flag of gdb_test_multiple is >> supposed to work in the same way as gdb_test, and that the recent >> changes for '^' had not been replicated for gdb_test_multiple. This >> patch addresses this issue. >> >> So, after this commit, the following two constructs should have the >> same meaning: >> >> gdb_test "command" "^output" "test name" >> >> gdb_test_multiple "command" "test name" { >> -re -wrap "^output" { >> pass $gdb_test_name >> } >> } >> >> In both cases the '^' will case gdb.exp to inject a regexp that >> matches 'command' after the '^' and before the 'output', this is in >> addition to adding the $gdb_prompt pattern after 'output' in the >> normal way. >> >> The special '^' handling is only applied when '-wrap' is used, as this >> is the only mode that aims to mimic gdb_test. >> >> While working on this patch I realised that I could actually improve >> the logic for the special '^' handling in the case where the expected >> output pattern is empty. I replicated these updates for both gdb_test >> and gdb_test_multiple in order to keep these two paths in sync. >> >> There were a small number of tests that needed adjustment after this >> change, mostly just removing command regexps that are now added >> automatically, but the gdb.base/settings.exp case was a little weird >> as it turns out trying to match a single blank line is probably harder >> now than it used to be -- still, I suspect this is a pretty rare case, >> so I think the benefits (improved anchoring) outweigh this small >> downside (IMHO). >> >> diff --git a/gdb/testsuite/gdb.base/bitshift.exp b/gdb/testsuite/gdb.base/bitshift.exp >> index adc5996d736..5ea0cd870ed 100644 >> --- a/gdb/testsuite/gdb.base/bitshift.exp >> +++ b/gdb/testsuite/gdb.base/bitshift.exp >> @@ -24,19 +24,17 @@ clean_restart >> # expected error. If WARNING_OR_ERROR is empty, it is expected that >> # GDB prints no text other than the print result. >> proc test_shift {lang cmd result_re {warning_or_error ""}} { >> - set cmd_re [string_to_regexp $cmd] >> - >> if {$lang == "go"} { >> if {$warning_or_error != ""} { >> set error_re "[string_to_regexp $warning_or_error]" >> gdb_test_multiple $cmd "" { >> - -re -wrap "^$cmd_re\r\n$error_re" { >> + -re -wrap "^$error_re" { >> pass $gdb_test_name >> } >> } >> } else { >> gdb_test_multiple $cmd "" { >> - -re -wrap "^$cmd_re\r\n\\$$::decimal$result_re" { >> + -re -wrap "^\\$$::decimal$result_re" { >> pass $gdb_test_name >> } >> } >> @@ -49,7 +47,7 @@ proc test_shift {lang cmd result_re {warning_or_error ""}} { >> } >> >> gdb_test_multiple $cmd "" { >> - -re -wrap "^$cmd_re\r\n$warning_re\\$$::decimal$result_re" { >> + -re -wrap "^$warning_re\\$$::decimal$result_re" { >> pass $gdb_test_name >> } >> } >> diff --git a/gdb/testsuite/gdb.base/maint-print-frame-id.exp b/gdb/testsuite/gdb.base/maint-print-frame-id.exp >> index 2ad9b6ddfd7..9e88f37205f 100644 >> --- a/gdb/testsuite/gdb.base/maint-print-frame-id.exp >> +++ b/gdb/testsuite/gdb.base/maint-print-frame-id.exp >> @@ -33,10 +33,6 @@ proc get_frame_id { level } { >> set id "**unknown**" >> >> gdb_test_multiple "maint print frame-id ${level}" "" { >> - -re "^maint print frame-id\[^\r\n\]+\r\n" { >> - exp_continue >> - } >> - >> -wrap -re "^frame-id for frame #\[0-9\]+: (\[^\r\n\]+)" { >> set id $expect_out(1,string) >> pass $gdb_test_name >> diff --git a/gdb/testsuite/gdb.base/settings.exp b/gdb/testsuite/gdb.base/settings.exp >> index eb127d246d2..1d9ee64ab0d 100644 >> --- a/gdb/testsuite/gdb.base/settings.exp >> +++ b/gdb/testsuite/gdb.base/settings.exp >> @@ -542,7 +542,7 @@ proc test-string {variant} { >> if {$variant != "filename"} { >> # This odd expected output here is because we expect GDB to >> # emit a single blank line as a result of this command. >> - gdb_test "$show_cmd" "^" "$show_cmd: show default" >> + gdb_test -nonl "$show_cmd" "^\r\n" "$show_cmd: show default" >> } else { >> gdb_test "$show_cmd" "/foo/bar" "$show_cmd: show default" >> } >> @@ -574,7 +574,7 @@ proc test-string {variant} { >> gdb_test_no_output "$set_cmd" >> # This odd expected output here is because we expect GDB to >> # emit a single blank line as a result of this command. >> - gdb_test "$show_cmd" "^" "$show_cmd: empty second time" >> + gdb_test -nonl "$show_cmd" "^\r\n" "$show_cmd: empty second time" >> } >> } >> >> diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-nodebug.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-nodebug.exp >> index 5aebd2a8606..be666cb9dfd 100644 >> --- a/gdb/testsuite/gdb.dwarf2/gdb-index-nodebug.exp >> +++ b/gdb/testsuite/gdb.dwarf2/gdb-index-nodebug.exp >> @@ -49,7 +49,7 @@ gdb_test_multiple $cmd "try to save gdb index" { >> -re -wrap $no_debug_re { >> pass $gdb_test_name >> } >> - -re -wrap "^$cmd" { >> + -re -wrap "^" { >> pass $gdb_test_name >> } >> } >> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp >> index 50c10333df1..aed29652b87 100644 >> --- a/gdb/testsuite/lib/gdb.exp >> +++ b/gdb/testsuite/lib/gdb.exp >> @@ -980,6 +980,8 @@ proc fill_in_default_prompt {prompt_regexp with_anchor} { >> # pass $gdb_test_name >> # } >> # } >> +# The special handling of '^' that is available in gdb_test is also >> +# supported in gdb_test_multiple when -wrap is used. >> # >> # In EXPECT_ARGUMENTS, a pattern flag -early can be used. It makes sure the >> # pattern is inserted before any implicit pattern added by gdb_test_multiple. >> @@ -1125,6 +1127,19 @@ proc gdb_test_multiple { command message args } { >> set expecting_action 1 >> if { $wrap_pattern } { >> # Wrap subst_item as is done for the gdb_test PATTERN argument. >> + if {[string range $subst_item 0 0] eq "^"} { >> + if {$command ne ""} { >> + set command_regex [string_to_regexp $command] >> + set subst_item [string range $subst_item 1 end] >> + if {[string length "$subst_item"] > 0} { >> + # We have an output pattern (other than the '^'), >> + # add a newline at the start, this will eventually >> + # sit between the command and the output pattern. >> + set subst_item "\r\n${subst_item}" >> + } >> + set subst_item "^${command_regex}${subst_item}" >> + } >> + } >> lappend $current_list \ >> "(?:$subst_item)\r\n$prompt_regexp" >> set wrap_pattern 0 >> @@ -1465,10 +1480,16 @@ proc gdb_test { args } { >> # additional pattern that matches the command immediately after >> # the '^'. >> if {[string range $pattern 0 0] eq "^"} { >> - set command_regex [string_to_regexp $command] >> - set pattern [string range $pattern 1 end] >> - if {$command_regex ne ""} { >> - set pattern "^${command_regex}\r\n$pattern" >> + if {$command ne ""} { >> + set command_regex [string_to_regexp $command] >> + set pattern [string range $pattern 1 end] >> + if {[string length "$pattern"] > 0} { >> + # We have an output pattern (other than the '^'), add a >> + # newline at the start, this will eventually sit between the >> + # command and the output pattern. >> + set pattern "\r\n$pattern" >> + } >> + set pattern "^${command_regex}${pattern}" >> } >> } >> >> @@ -6167,9 +6188,8 @@ proc with_set { var val body } { >> perror "Did not manage to set $var" >> } else { >> # Set var. >> - set cmd "set $var $val" >> - gdb_test_multiple $cmd "" { >> - -re -wrap "^$cmd" { >> + gdb_test_multiple "set $var $val" "" { >> + -re -wrap "^" { >> } >> -re -wrap " is set to \"?$val\"?\\." { >> } >> @@ -6180,9 +6200,8 @@ proc with_set { var val body } { >> >> # Restore saved setting. >> if { $save != "" } { >> - set cmd "set $var $save" >> - gdb_test_multiple $cmd "" { >> - -re -wrap "^$cmd" { >> + gdb_test_multiple "set $var $save" "" { >> + -re -wrap "^" { >> } >> -re -wrap "is set to \"?$save\"?( \\(\[^)\]*\\))?\\." { >> } >> @@ -7746,7 +7765,7 @@ proc get_valueof { fmt exp default {test ""} } { >> >> set val ${default} >> gdb_test_multiple "print${fmt} ${exp}" "$test" { >> - -re "\\$\[0-9\]* = (\[^\r\n\]*)\r\n$gdb_prompt $" { >> + -re -wrap "^\\$\[0-9\]* = (\[^\r\n\]*)" { >> set val $expect_out(1,string) >> pass "$test" >> } >> @@ -7795,7 +7814,7 @@ proc get_integer_valueof { exp default {test ""} } { >> >> set val ${default} >> gdb_test_multiple "print /d ${exp}" "$test" { >> - -re "\\$\[0-9\]* = (\[-\]*\[0-9\]*).*$gdb_prompt $" { >> + -re -wrap "^\\$\[0-9\]* = (\[-\]*\[0-9\]*).*" { >> set val $expect_out(1,string) >> pass "$test" >> } >>