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 BA16D3858C74 for ; Tue, 21 Jun 2022 15:52:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BA16D3858C74 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-444-XN6SuJUzMmmFJr6X1MMaQQ-1; Tue, 21 Jun 2022 11:52:06 -0400 X-MC-Unique: XN6SuJUzMmmFJr6X1MMaQQ-1 Received: by mail-wr1-f71.google.com with SMTP id i16-20020adfa510000000b0021b8e9f7666so1742885wrb.19 for ; Tue, 21 Jun 2022 08:52:05 -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=FhdB5YpGI7ErjVWc2uDXcenTHrqj3vKgs8x6bb+q4+w=; b=qtlUi/a2iXF/UhR0ZBjHGuj/st6RLPoFvKhFnRBSwyzq3ubN3NEZrGXaxG+KtpZ7cY z5JVE4Eh3r1k1uoNIAEHEjvapirqvDxqVpRYiGfJ3MNpgENJB8wpC1ke4KYMw6kbUsHp a8AHZ16QqCyW4Kj9ADC+1VPqUcwcy07EP16ZQ1LqUy+KhE7a/B7+0ycMz+R5lF7gXnuG +BO/2jMA6HiPS4axwWJORN0pzPiIuie7YX+sIMvEBBemhLK7kiDk1+BToQKc8W4jU+H/ EOoJ1YNZoTh5IeKUogYfZ6Vu0H4lzFPN6y5X3DzBkXYjtUfy8+fMTndnqbP608m+M7cj dVMQ== X-Gm-Message-State: AJIora+W50k1UbfXIfFAPAnB9ke2KBvzTEQr7PL0EY4ulpUjvvO/sL+g 3ri6W4PTPudJzIYcmSV3SYAKz709avFtSiybmwVMhavQr7CxJ8q+NU4KubYbLa1TKi2urhuqDfx ++kw+mFzFpESTp/pry7tz2w== X-Received: by 2002:adf:fb52:0:b0:216:9eff:342b with SMTP id c18-20020adffb52000000b002169eff342bmr29162651wrs.356.1655826724421; Tue, 21 Jun 2022 08:52:04 -0700 (PDT) X-Google-Smtp-Source: AGRyM1u+ZyoPtJQIcUAF+v+jx0ysy6PXeoAk2bGrTrKdwD0LdeYvb2z0Lp9k4MRrfj+GouZD0JYUQA== X-Received: by 2002:adf:fb52:0:b0:216:9eff:342b with SMTP id c18-20020adffb52000000b002169eff342bmr29162629wrs.356.1655826724075; Tue, 21 Jun 2022 08:52:04 -0700 (PDT) Received: from localhost ([213.31.44.107]) by smtp.gmail.com with ESMTPSA id n125-20020a1c2783000000b003974cb37a94sm22327966wmn.22.2022.06.21.08.52.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jun 2022 08:52:03 -0700 (PDT) From: Andrew Burgess To: Bruno Larsen , gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp In-Reply-To: <20220509180431.31032-1-blarsen@redhat.com> References: <20220509180431.31032-1-blarsen@redhat.com> Date: Tue, 21 Jun 2022 16:52:02 +0100 Message-ID: <87edzic8v1.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=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, 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: Tue, 21 Jun 2022 15:52:09 -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. > --- > gdb/testsuite/gdb.base/maint.exp | 109 ++++++++----------------------- > 1 file changed, 29 insertions(+), 80 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp > index 45ccdc6584e..2efdda9aed7 100644 > --- a/gdb/testsuite/gdb.base/maint.exp > +++ b/gdb/testsuite/gdb.base/maint.exp > @@ -155,18 +155,10 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )] > # guo: on linux this command output is huge. for some reason splitting up > # the regexp checks works. I think this comment should either be deleted, or rewritten. Plus, this comment seems to directly contradict the preceeding comment that says: # this command does not produce any output # unless there is some problem with the symtabs and psymtabs # so that branch will really never be covered in this tests here!! Which is it? The command produces no output, or it produces a huge amount of output? Turns out, the first comment is correct, there's no output, at least on my Linux machine. Looking at maintenance_check_psymtabs, I guess there could, potentially, be lots of output, if there is a lot of problems with the generated file that we're debugging for this test, so maybe when the code below, that you're changing, was added, some targets were seeing such problems? If we assume that there are still targets out there that do have errors then we should probably write a comment that's not so contradictory... > # > -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" } > - } > +gdb_test_multiple "maint check-psymtabs" "maint check-psymtabs" { You can just do: gdb_test_multiple "maint check-psymtabs" "" { the message will default to the command. > + -re -wrap "^maint check-psymtabs.*" { I suspect that this only works because for you, like me, 'maint check-psymtabs' doesn't produce any output. The original checks didn't require the entire command output to be loaded into the expect buffer, while your updated pattern does. The "for some reason" that is mentioned in the 'guo' comment is almost certainly that the expect buffer was getting full. The patterns should probably be: -re "^maint check-psymtabs\r\n" { exp_continue } -re "^$gdb_prompt $" { pass $gdb_test_name } -re "^\[^\r\n\]+\r\n" { exp_continue } This will allow the command to run, and discard any lines of output that are not the prompt. We could make this test stricter by changing the last pattern to match all possible lines that maintenance_check_psymtabs can produce (I think there's only 4 or 5). We could also make the test stricter by ensuring that the initial 'maint check-psymtabs' line is seen. > + pass "maint check-psymtabs" You can do: pass $gdb_test_name the '$gdb_test_name' is automatically created based on the value of the message by gdb_test_multiple. > } > - -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 > @@ -270,62 +262,38 @@ 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_multiple "$command" "$test_name" { > + -re -wrap "^maint print psymbols \[^\n\]*" { > + gdb_test_multiple "shell grep 'main.*function' $psymbols_output"\ > + "$test_name internal"{ I think there should be a space before the `\` and `{` at the end of the two previous lines. Not required, that just seems to be the GDB style. > + -re -wrap ".main., function, $hex.*" { > + gdb_test "shell rm -f $psymbols_output" ".*" \ > + "${test_name}: shell rm -f psymbols_output" I wonder why the 'rm' needed to move into both of the pass blocks? It seemed like having it after the gdb_test_multiple would be better. > + pass "$test_name 1" You could use: pass "$gdb_test_name, pattern 1" here, and similar, with ', pattern 2' for the next 'pass' call. > + } > + -re -wrap ".*main. .., function, $hex.*" { > gdb_test "shell rm -f $psymbols_output" ".*" \ > "${test_name}: shell rm -f psymbols_output" > + pass "$test_name 2" > } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > - } > } > - -re ".*$gdb_prompt $" { fail "$test_name" } > - timeout { fail "$test_name (timeout)" } > } > + } > } > } > > > 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)" } > - } > +gdb_test_multiple "maint print msymbols -objfile ${binfile} $msymbols_output" \ > +"printing msymbols" { I think this line should be indented. > + -re -wrap "^maint print msymbols \[^\n\]*" { > + 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" > } Couldn't these tests just be serialised, rather than being nested? e.g.: gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \ "printing msymbols" 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" I haven't tested this, so maybe there's a reason why this wouldn't work. Thanks, Andrew > - -re ".*$gdb_prompt $" { fail "maint print msymbols" } > - timeout { fail "maint print msymbols (timeout)" } > } > > # Check that maint print msymbols allows relative pathnames > @@ -363,31 +331,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