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.133.124]) by sourceware.org (Postfix) with ESMTPS id A2490385356F for ; Wed, 29 Jun 2022 10:33:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A2490385356F 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.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-318-Wc9qVCU1NfOjFI34pKtb-Q-1; Wed, 29 Jun 2022 06:33:55 -0400 X-MC-Unique: Wc9qVCU1NfOjFI34pKtb-Q-1 Received: by mail-wm1-f72.google.com with SMTP id j19-20020a05600c191300b003a048196712so5160804wmq.4 for ; Wed, 29 Jun 2022 03:33:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=+diLGtuphFx1FOzIzlwt7DFlgkuF4poS8/VtNKeEHCA=; b=7sOmd0EdUgkUk7RlWY6LtCtBwUgMKlFeeAYKSHRbeKhgVlfw3plRwLOz1Dpgx+F2Vc BrurYs0AgKoL68o9jZQhE0CI/YA4boQmZLnS14veg/7HeviptTR451+Ng6H7QQgO5zzC tu0TV4puQjcM05fa21dxpuWiqn87KWMc6CQtXVah88uemG8DWRPaD5BAA8zghNbddXlY CwPcaQVbXAmrzM3rLFmzt+4yJHu1pgW1UoslbT29z+MiC5MiAFWz0YQ4ksvugiy61EIK segmMryl7qfUfDDpyqZizi8ykoGlUE3AspdghSUpoF1pINfQii6c3pHVwmfS3SsZDNP4 7wlw== X-Gm-Message-State: AJIora8O7Vjm2K1tZmx3FVd+oJVjbGO9cXEGvQydaLkLAksBjupds36n UKF7UHfYFq6L1lC/lsGLiJ2Yt/2WUtePHduj1tNp0AtHuN2Fk/X8Xe1jsU2dBKPqV144FfiyqhQ 2zCJgxRW5s296X55YjHgBPg== X-Received: by 2002:adf:d1c6:0:b0:21b:ca92:283b with SMTP id b6-20020adfd1c6000000b0021bca92283bmr2374576wrd.537.1656498833509; Wed, 29 Jun 2022 03:33:53 -0700 (PDT) X-Google-Smtp-Source: AGRyM1vipIwYUl/0G/PneNUeKBMSNMRF28ChmadX7Q8C6mliSioaaYOMOOSXU2o/pxfiEsCU7lnZjg== X-Received: by 2002:adf:d1c6:0:b0:21b:ca92:283b with SMTP id b6-20020adfd1c6000000b0021bca92283bmr2374550wrd.537.1656498833171; Wed, 29 Jun 2022 03:33:53 -0700 (PDT) Received: from localhost (15.72.115.87.dyn.plus.net. [87.115.72.15]) by smtp.gmail.com with ESMTPSA id q11-20020adfea0b000000b0020fff0ea0a3sm15971818wrm.116.2022.06.29.03.33.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jun 2022 03:33:52 -0700 (PDT) From: Andrew Burgess To: Bruno Larsen , gdb-patches@sourceware.org Subject: Re: [PATCHv3] gdb/testsuite: modernize gdb.base/maint.exp In-Reply-To: <20220628183644.34139-1-blarsen@redhat.com> References: <20220509180431.31032-1-blarsen@redhat.com> <20220628183644.34139-1-blarsen@redhat.com> Date: Wed, 29 Jun 2022 11:33:51 +0100 Message-ID: <87wnczdai8.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=-10.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2022 10:34:01 -0000 Bruno Larsen via Gdb-patches writes: > gdb.base/maint.exp was using several gdb_expect statements, probably > because this test case predates the existance of gdb_test_multiple. This > commit updates the test case to use gdb_test_multiple, making it more > resilient to internal errors and such. > > The only gdb_expect left in the testcase is one that specifically looks > for an internal error being triggered as a PASS. > --- > > Changes for version 3: > * changed patterns when testing psymbols, now using trailing > parenthesis > * used -lbl when running "check psymtabs" > * made use of boolean variables and gdb_assert when testing > psymtabs > * Simplified msymbols and psymbosl test to 3 tests, instead of > nested tests > > Changes for v2: > - Addressed Andrew's comments > - rebased on current master. > > --- > gdb/testsuite/gdb.base/maint.exp | 142 ++++++++++--------------------- > 1 file changed, 43 insertions(+), 99 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp > index 2817c6eafb9..aea8a10df0c 100644 > --- a/gdb/testsuite/gdb.base/maint.exp > +++ b/gdb/testsuite/gdb.base/maint.exp > @@ -151,22 +151,20 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )] > # unless there is some problem with the symtabs and psymtabs > # so that branch will really never be covered in this tests here!! > # > +# When there is a problem, there may be loads of output, which can > +# overwhelm the expect buffer. Splitting it seems to fix those > +# issues. > + > +set seen_command false > +gdb_test_multiple "maint check-psymtabs" "" -lbl { Bruno, Thanks for continuing to work on this... Sorry to be a pain, but I don't think -lbl will actually do what you want here. To me the implementation of -lbl just looks weird, and I'm not sure how it is expected to help. Your patterns are: -re "^maint check-psymtabs\r\n" { set seen_command true exp_continue } -re "^$gdb_prompt $" { gdb_assert { $seen_command } $gdb_test_name } And -lbl adds: -re "\r\n\[^\r\n\]*(?=\r\n)" { exp_continue } The problem I think exists here is that your first pattern runs from the star of the buffer and consumes the trailing \r\n. Your prompt pattern also expects to start from the start of the buffer. If the output is: maint check-psymtabs\r\n (gdb) Then these patterns will work fine. But if your output is: maint check-psymtabs\r\n Some other line here\r\n (gdb) Then your first pattern will consume the first line, including the trailing \r\n. The -lbl pattern will then fail to match the second line as the -lbl pattern expects a leading \r\n. And now your pattern will fail to match the third line as the second line is still in the buffer. The -lbl pattern always leaves a trailing \r\n in the expect buffer, which will cause your prompt pattern to fail. Personally, I'd just drop the use of -lbl and add what I think is the more correct pattern: -re "^\[^\r\n\]+\r\n" { exp_continue } which is what you originally proposed. But if you really want to make use of -lbl then you can change your patterns to: -re "^maint check-psymtabs(?=\r\n)" { set seen_command true exp_continue } -re "\r\n$gdb_prompt $" { gdb_assert { $seen_command } $gdb_test_name } I don't think that's an improvement, but I think that should work. > + -re "^maint check-psymtabs\r\n" { > + set seen_command true > + exp_continue > + } > > -# guo: on linux this command output is huge. for some reason splitting up > -# the regexp checks works. > -# > -send_gdb "maint check-psymtabs\n" > -gdb_expect { > - -re "^maint check-psymtabs" { > - gdb_expect { > - -re "$gdb_prompt $" { > - pass "maint check-psymtabs" > - } > - timeout { fail "(timeout) maint check-psymtabs" } > - } > + -re "^$gdb_prompt $" { > + gdb_assert { $seen_command } $gdb_test_name > } > - -re ".*$gdb_prompt $" { fail "maint check-psymtabs" } > - timeout { fail "(timeout) maint check-psymtabs" } > } > > # This command does not produce any output unless there is some problem > @@ -272,63 +270,33 @@ if { $have_psyms } { > "maint print psymbols -pc" \ > "maint print psymbols -pc main $psymbols_output"] > foreach { test_name command } $test_list { > - send_gdb "$command\n" > - gdb_expect { > - -re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" { > - send_gdb "shell ls $psymbols_output\n" > - gdb_expect { > - -re "$psymbols_output_re\r\n$gdb_prompt $" { > - # We want this grep to be as specific as possible, > - # so it's less likely to match symbol file names in > - # psymbols_output. Yes, this actually happened; > - # poor expect got tons of output, and timed out > - # trying to match it. --- Jim Blandy > - send_gdb "shell grep 'main.*function' $psymbols_output\n" > - gdb_expect { > - -re ".main., function, $hex.*$gdb_prompt $" { > - pass "$test_name 1" > - } > - -re ".*main. .., function, $hex.*$gdb_prompt $" { > - pass "$test_name 2" > - } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > - } > - gdb_test "shell rm -f $psymbols_output" ".*" \ > - "${test_name}: shell rm -f psymbols_output" > + gdb_test_multiple "$command" "$test_name" { > + -re -wrap "^maint print psymbols \[^\n\]*" { > + gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \ > + "$test_name internal" { > + -re -wrap ".main., function, $hex.*" { > + pass "$test_name (pattern 1)" > + } > + -re -wrap ".*main. .., function, $hex.*" { > + pass "$test_name (pattern 2)" > } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > - } > } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > } > + } > + gdb_test "shell rm -f $psymbols_output" ".*" \ > + "${test_name}: shell rm -f psymbols_output" > } Again, apologies for only just spotting this, but I think this test can be further simplified to just: foreach { test_name command } $test_list { with_test_prefix "$test_name" { gdb_test_no_output "$command" "collect output" gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \ "$analyse output" { -re -wrap ".main., function, $hex.*" { pass "$gdb_test_name (pattern 1)" } -re -wrap ".*main. .., function, $hex.*" { pass "$gdb_test_name (pattern 2)" } } gdb_test "shell rm -f $psymbols_output" ".*" \ "shell rm -f psymbols_output" } } I think we already made a similar change later on in this script, so this just brings this test into line with the later ones. Thanks, Andrew > } > > > set msymbols_output [standard_output_file msymbols_output] > set msymbols_output_re [string_to_regexp $msymbols_output] > -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n" > -gdb_expect { > - -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" { > - send_gdb "shell ls $msymbols_output\n" > - gdb_expect { > - -re "$msymbols_output_re\r\n$gdb_prompt $" { > - gdb_test "shell grep factorial $msymbols_output" \ > - "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \ > - "maint print msymbols, absolute pathname" > - gdb_test "shell rm -f $msymbols_output" ".*" \ > - "shell rm -f msymbols_output" > - } > - -re ".*$gdb_prompt $" { fail "maint print msymbols" } > - timeout { fail "maint print msymbols (timeout)" } > - } > - } > - -re ".*$gdb_prompt $" { fail "maint print msymbols" } > - timeout { fail "maint print msymbols (timeout)" } > -} > +gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \ > + "print msymbols to file, with absolute path" > +gdb_test "shell grep factorial $msymbols_output" \ > + "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \ > + "maint print msymbols, absolute pathname" > +gdb_test "shell rm -f $msymbols_output" ".*" "remove absolute path msymbols" > > # Check that maint print msymbols allows relative pathnames > set mydir [pwd] > @@ -336,18 +304,13 @@ gdb_test "cd [standard_output_file {}]" \ > "Working directory .*\..*" \ > "cd to objdir" > > -gdb_test_multiple "maint print msymbols -objfile ${testfile} msymbols_output2" "maint print msymbols" { > - -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" { > - gdb_test_multiple "shell ls msymbols_output2" "maint print msymbols" { > - -re "msymbols_output2\r\n$gdb_prompt $" { > - gdb_test "shell grep factorial msymbols_output2" \ > - "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \ > - "maint print msymbols, relative pathname" > - gdb_test "shell rm -f msymbols_output2" ".*" > - } > - } > - } > -} > +gdb_test_no_output "maint print msymbols -objfile ${testfile} $msymbols_output"\ > + "print msymbols to file, with relative path" > +gdb_test "shell grep factorial $msymbols_output" \ > + "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \ > + "maint print msymbols, relative pathname" > +gdb_test "shell rm -f msymbols_output" ".*" "remove relative path msymbols" > + > gdb_test "cd ${mydir}" \ > "Working directory [string_to_regexp ${mydir}]\..*" \ > "cd to mydir" > @@ -365,31 +328,12 @@ set test_list [list \ > "maint print symbols -pc" \ > "maint print symbols -pc main $symbols_output"] > foreach { test_name command } $test_list { > - send_gdb "$command\n" > - gdb_expect { > - -re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" { > - send_gdb "shell ls $symbols_output\n" > - gdb_expect { > - -re "$symbols_output_re\r\n$gdb_prompt $" { > - # See comments for `maint print psymbols'. > - send_gdb "shell grep 'main(.*block' $symbols_output\n" > - gdb_expect { > - -re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" { > - pass "$test_name" > - } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > - } > - gdb_test "shell rm -f $symbols_output" ".*" \ > - "$test_name: shell rm -f symbols_output" > - } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > - } > - } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > - } > + gdb_test_no_output "$command" "$test_name generate" > + gdb_test "shell grep 'main(.*block' $symbols_output"\ > + "int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\ > + "$test_name read" > + gdb_test "shell rm -f $symbols_output" ".*" \ > + "$test_name: shell rm -f symbols_output" > } > > set msg "maint print type" > -- > 2.31.1