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 547C33858D1E for ; Thu, 8 Sep 2022 12:05:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 547C33858D1E 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.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-649--6O1JWJ6MaKoEEGOtNnjxQ-1; Thu, 08 Sep 2022 08:05:01 -0400 X-MC-Unique: -6O1JWJ6MaKoEEGOtNnjxQ-1 Received: by mail-wr1-f71.google.com with SMTP id v9-20020adfa1c9000000b002287d591b37so3891948wrv.18 for ; Thu, 08 Sep 2022 05:05:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date; bh=IzDVEzTvOuqBIYDJeg5Wu4/Sg0Gpp6GqvltAJb4rO8I=; b=mBSIjSBd0EIdkz9Cx2uc/0RDnT1zjxJnsFE7xeq0ntO30hikIZeyCOP2n6xA/OWBM9 boGdPAO4GFH+CQUzKNvB7kHqRA0X9YSpsppdsHucXq2mcilrzLIkb3e5fFYc/m3rmPlm 1Zeu+VidNE7t1SgoMR+bxq9f18GBUf1E6G6h8FV/h7kPredAbKsTsQz5IaUze7xFYSnC MHR+wsR+59RbUxV+i3x8JH4VxvCsDjHcEjsMLHy3Qa/ag5TcQ8XMCbfVrQEec/ltkWno Daz+P3sA8U792euuKqOiqMEH8GeziChbYMo0wx5T7ZjrLBNB0b09c6qod8OLFqWMiZil JNmw== X-Gm-Message-State: ACgBeo0XMCYGnOgItvRMCeMwhX8REBW8+R9qLqbNDGUdUN4jdLE7URbY Xs5d7QsB9+ytuS9rBS11k2zX9lcogGmm7xoiS8tNB+BzlD8wsKJ0HrAUFrXgmDkFUaUsVCjoYo4 6idOKkwEOnEixCA0p+CtKYg== X-Received: by 2002:a5d:42c3:0:b0:228:7c5e:e243 with SMTP id t3-20020a5d42c3000000b002287c5ee243mr4741567wrr.481.1662638700499; Thu, 08 Sep 2022 05:05:00 -0700 (PDT) X-Google-Smtp-Source: AA6agR6+lgCaOqUEs8PLdrqCDdLGBQdjkC0DIaBHLG8JbIEFCgqPFYaVDDnnI9NO3wGir2JPtEVyrw== X-Received: by 2002:a5d:42c3:0:b0:228:7c5e:e243 with SMTP id t3-20020a5d42c3000000b002287c5ee243mr4741550wrr.481.1662638700203; Thu, 08 Sep 2022 05:05:00 -0700 (PDT) Received: from localhost (52.72.115.87.dyn.plus.net. [87.115.72.52]) by smtp.gmail.com with ESMTPSA id m7-20020a056000008700b0022a2c600d5csm2677121wrx.55.2022.09.08.05.04.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Sep 2022 05:04:59 -0700 (PDT) From: Andrew Burgess To: Bruno Larsen , gdb-patches@sourceware.org Subject: Re: [PATCH v4 15/15] gdb/testsuite: Add test to step through function epilogue In-Reply-To: <20220720194441.168906-17-blarsen@redhat.com> References: <20220720194441.168906-1-blarsen@redhat.com> <20220720194441.168906-17-blarsen@redhat.com> Date: Thu, 08 Sep 2022 13:04:58 +0100 Message-ID: <874jxim5qt.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, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, 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: Thu, 08 Sep 2022 12:05:05 -0000 Bruno Larsen via Gdb-patches writes: > The testsuite implicitly tests GDB's ability to step through epilogues > in multiple tests, without doing it explicitly anywhere. This is > unfortunate, as clang does not emit epilogue information, so using clang > on our testsuite makes many tests fail. This patch adds a central, > explicit test for walking through the epilogue so we can safely remove > this from other tests and have them working with clang. Sounds sensible. > > The test created attempts to step through a simple empilogue, an s/empilogue/epilogue/ > epilogue that ends on another epilogue, and epilogues leading to other > function calls. > --- > .../gdb.base/step-through-epilogue.c | 15 ++++ > .../gdb.base/step-through-epilogue.exp | 86 +++++++++++++++++++ > 2 files changed, 101 insertions(+) > create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.c > create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.exp > > diff --git a/gdb/testsuite/gdb.base/step-through-epilogue.c b/gdb/testsuite/gdb.base/step-through-epilogue.c > new file mode 100644 > index 00000000000..3e0ea5a2522 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/step-through-epilogue.c Source files should include a copyright notice at the top, checkout gdb.base/break.c as an example. > @@ -0,0 +1,15 @@ > +int multiply (int a, int b){ > + return a*b; > +} Where possible, test source files should follow GNU convention, so: int multiply (int a, int b) { ... } if you need to break convention for some reason, maybe having everything on one line is required by the test, then this should be explained in a comment. Also, I notice everything is indented by 4 spaces instead of 2. > + > +int square (int x){ > + return multiply (x, x); > +} > + > +int main(){ Should be 'main (void)' I think. > + int x; > + x = multiply (1,2); > + x = square(2); Space before '(2)'. > + x = multiply (square (1), square (2)); > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/step-through-epilogue.exp b/gdb/testsuite/gdb.base/step-through-epilogue.exp > new file mode 100644 > index 00000000000..762623cf72c > --- /dev/null > +++ b/gdb/testsuite/gdb.base/step-through-epilogue.exp > @@ -0,0 +1,86 @@ > +# Copyright 2022 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# This test is used to confirm that GDB is able to step, stopping at an > +# epilogue line, then step out of the function. > + > +standard_testfile > + > +if { [get_compiler_info "clang-*"] } { This should be using 'test_compiler_info' here, the get call doesn't do what you would expect given its name. > + untested "This test can't be used with clang" > + return > +} > + > +if { [prepare_for_testing "failed to prepare" "skip" \ > + {step-through-epilogue.c}] } { That's unexpected! I suspect the use of "skip" here for the binary name is the result of a cut & paste. You probably want: if { [prepare_for_testing "failed to prepare" $testfile \ {step-through-epilogue.c}] } { untested "failed to prepare" return } which will result in the test binary being named after the test script. > + untested "failed to prepare" > + return > +} > + > +if { ![runto_main] } { > + untested "couldn't run to main" > + return > +} > + > +# Some gcc versions, at least 6.5.0 and 9.2.0, can mess this test by > +# requiring extra steps in a few locations. We test here and use > +# this knowledge later to decide if the extra steps need to be taken. > +set slow_gcc 0 > +if { [test_compiler_info "gcc-6-5-0"] || [test_compiler_info "gcc-9-2-0"]} { > + set slow_gcc 1 > +} > + > +proc step_till_epilogue_multiply {} { > + gdb_test "step" ".*return a.b;.*" "step into multiply" > + gdb_test "step" ".*3\\s+\\\}.*" "stop at the epilogue of multiply" When you update the source file, you'll find this pattern breaks as the line number changes. One trick to avoid placing these line numbers is to add a comment to the source file, like: } /* Epilogue line of multiply. */ then the pattern can be: gdb_test "step" \ ".*$::decimal\\s+\\\}\[^\r\n\]+Epilogue line of multiply.*" \ "stop at the epilogue of multiply" reducing the number of hard-coded line numbers is usually a good thing. > +} > + > +proc step_till_epilogue_square {} { > + gdb_test "step" ".*return multiply.*" "step into square" > + step_till_epilogue_multiply > + gdb_test "step" ".*7\\s+\\\}.*" "stop at epilogue of square" > +} > + > +with_test_prefix "multiply" { > + step_till_epilogue_multiply > + gdb_test "step" ".*x = square\\(2\\);" "leave function" > +} > + > +with_test_prefix "square" { > + step_till_epilogue_square > + gdb_test "step" ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\ > + "leave function" > +} > + > +with_test_prefix "square, first argument" { > + step_till_epilogue_square > + if {$slow_gcc} { > + gdb_test "step"\ > + ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\ > + "step back to main" > + } I'm kind-of bothered by the $slow_gcc test. It feels pretty random that we only check for those two specific gcc versions. Plus returning back to main for that one line doesn't feel that wrong to me, so it seems like this is less working around a bug in specific compiler versions, and more just variance in how the debug is emitted. I wonder if we could make use of gdb_test_multiple to avoid the need for $slow_gcc? Something like this completely untested code: gcc_test_multiple "step" "step into square" { -re -wrap ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*" { gdb_send "step\n" exp_continue } -re -wrap ".*return multiply.*" { pass $gdb_test_name } } Of course, that would mean you couldn't then use step_till_epilogue_square in its current form, as you do next, but maybe with some refactoring, the above might be made to work? > +} > +with_test_prefix "square, second argument" { > + step_till_epilogue_square > + if {$slow_gcc} { > + gdb_test "step"\ > + ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\ > + "step back to main" > + } > +} > +with_test_prefix "multiply with function args" { > + step_till_epilogue_multiply > + gdb_test "step" ".*return 0;.*" "leave last function" > +} > -- > 2.31.1