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 E5EE3383B7AE for ; Fri, 27 May 2022 16:19:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E5EE3383B7AE Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-570-iBAjt3rOO5yEU54pnqt0Fg-1; Fri, 27 May 2022 12:19:49 -0400 X-MC-Unique: iBAjt3rOO5yEU54pnqt0Fg-1 Received: by mail-qk1-f199.google.com with SMTP id o18-20020a05620a2a1200b006a0cc3d8463so4204672qkp.4 for ; Fri, 27 May 2022 09:19:48 -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=FUaeOQ51lZBQddht6tL+GkuMZz1aeWce5+fpIE9nPow=; b=nPtjTIenltIrkEk0JiqCUsus3J/LZL5R+zeCqRGdvFnnTqBy+W+Me4FozcJW3tYVuF FhSxzclklSzUlBamh6nBD0wfFscEUuD2ezJbS9DwR+TOAx9gzKbBgesaOp6xzX2S79c1 2qazIAGUdsQslKm+ySZyh/+kHretHzNcXYgUW17/XZkbE4EFyMqMtXVNHbcd1OdO1xze +5XrC9ETEVmfkR4+cEuFMPxtHp+/lEjp1URkBg8/uhliBjCFuRAQpm+P1XGozO4e+FJF dGFh3xSBcsI+LCtS2PhtQtfjmEGmYpCcg9x+gjwYFMpLCeqL5STDHvW8pI4mJ1W7ffF/ HLhw== X-Gm-Message-State: AOAM530jifuZWsH3o+8y7qPl7EzPN/eK60R/UXyOE4bIHxA+EOHh1jgK DDRbjKpARX3H7V6c0nb+KqoeWIB0oXfex80qwxg3g+LhjggUW29V1xsTfGUlVZDDGKhp2cnT76n HPQNROPSjk/e9hAKWCAQ3cQ== X-Received: by 2002:a05:622a:50a:b0:2f9:2af9:7dbc with SMTP id l10-20020a05622a050a00b002f92af97dbcmr25446336qtx.279.1653668387620; Fri, 27 May 2022 09:19:47 -0700 (PDT) X-Google-Smtp-Source: ABdhPJydmzsXK+jxqOeVC65i1rGpUXO8G9tjFAlZDJDWW8+BDUKSSVb9NG2DfBsTr7is1K+bhsrcww== X-Received: by 2002:a05:622a:50a:b0:2f9:2af9:7dbc with SMTP id l10-20020a05622a050a00b002f92af97dbcmr25446308qtx.279.1653668387303; Fri, 27 May 2022 09:19:47 -0700 (PDT) Received: from localhost (host109-152-215-36.range109-152.btcentralplus.com. [109.152.215.36]) by smtp.gmail.com with ESMTPSA id z6-20020a05622a028600b002f3d0e07693sm2946670qtw.88.2022.05.27.09.19.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 May 2022 09:19:47 -0700 (PDT) From: Andrew Burgess To: Bruno Larsen , gdb-patches@sourceware.org Subject: Re: [PATCH v3 01/14] gdb/testsuite: introduce gdb_step_until_regexp In-Reply-To: <20220526151041.23223-2-blarsen@redhat.com> References: <20220526151041.23223-1-blarsen@redhat.com> <20220526151041.23223-2-blarsen@redhat.com> Date: Fri, 27 May 2022 17:19:44 +0100 Message-ID: <87ilprgd7j.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.6 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: Fri, 27 May 2022 16:19:53 -0000 Bruno Larsen via Gdb-patches writes: > Currently, GDB's testsuite uses a set amount of step commands to exit > functions. This is a problem if a compiler emits different epilogue > information from gcc, or emits no epilogue information at all. It was > most noticeable if Clang was used to test GDB. > > To fix this unreliability, this commit introduces a new proc that will > single step the inferior until it is stopped at a line that matches the > given regexp, or until it steps too many times - defined as an optional > argument. If the line is found, it shows up as a single PASS in the > test, and if the line is not found, a single FAIL is emitted. > > This patch only introduces this proc, but does not add it to any > existing tests, these will be introduced in the following commit. > --- > > No change in v3 > > New patch in v2 > > --- > gdb/testsuite/lib/gdb.exp | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index b04fbb89e4e..c0ca1d04cc2 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -8648,5 +8648,35 @@ proc get_set_option_choices {set_cmd} { > return $values > } > > +# This proc is used mainly to exit function in a compiler agnostic way > +# It makes gdb single step and evaluate the output at every step, to see > +# if the regexp is present. > +# > +# The proc takes 2 optional arguments, the first being the name of the > +# test and the second the maximum amount of iterations until we expect to > +# see the function. The default is 10 steps, since this is meant as the > +# last step by default, and we don't expect any compiler generated epilogue > +# longer than 10 steps. I feel like you are being overly prescriptive in how this function should be used. I would rewrite this to just describe what the function does, and let folk use it as they see fit. Sure, initially it will only be used as you imagine - that's why you're adding it. But once it's there, who knows what uses it might be put too. I'd go with something like: # Single step until the pattern REGEXP is found. Step at most # MAX_STEPS times, but stop stepping once REGEXP is found. # # If REGEXP is found then a single pass is emitted, otherwise, after # MAX_STEPS steps, a single fail is emitted. # # TEST_NAME is the name used in the pass/fail calls. > + > +proc gdb_step_until_regexp { regexp {test_name "single stepping until regexp"} {max_steps 10} } { You should keep this line under 80 characters. You can wrap the arguments I believe, like: proc gdb_step_until_regexp { regexp {test_name "single stepping until regexp"} {max_steps 10} } { However, I'd be tempted to take a different approach, like this: proc gdb_step_until_regexp { regexp {test_name ""} {max_steps 10} } { if { $test_name == "" } { set test_name "single stepping until regexp" } The benefit I see in this approach is that if a user wants to adjust max_steps, but doesn't care about the test name, they can do this: gdb_step_until_regexp "SOME_PATTERN" "" 50 And still get the default test name. > + global gdb_prompt I think this is OK, there's certainly lots of precedent for this approach, but I think more often these days, we just refer to the global directly as: $::gdb_prompt As this removes the need for the 'global gdb_prompt' line. But I don't think this is a hard requirement if you prefer what you have. Thanks, Andrew > + > + set count 0 > + gdb_test_multiple "step" "$test_name" { > + -re "$regexp\r\n$gdb_prompt $" { > + pass $test_name > + } > + -re ".*$gdb_prompt $" { > + if {$count < $max_steps} { > + incr count > + send_gdb "step\n" > + exp_continue > + } else { > + fail $test_name > + } > + } > + } > +} > + > # Always load compatibility stuff. > load_lib future.exp > -- > 2.31.1