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 1576D3841885 for ; Mon, 27 Jun 2022 13:40:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1576D3841885 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-640-7hCG0AhFMd6At7-1S1zbVA-1; Mon, 27 Jun 2022 09:40:37 -0400 X-MC-Unique: 7hCG0AhFMd6At7-1S1zbVA-1 Received: by mail-wm1-f71.google.com with SMTP id j19-20020a05600c191300b003a048196712so2283785wmq.4 for ; Mon, 27 Jun 2022 06:40:36 -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=3WlDHgO7TqP2BMWmJZZ4ly8mLcxAc6BF3DqUEBGI9D8=; b=wEVoZOsZwjJW5xFHNW1yfrpyAgVPiRAxW3BuLlV3L0Zc2WcVRTW82gIWEZLgBmuRgI VO0Gm1/P0FDeqhBLve7RGxSwGVbnp8m6K5XcL5oRg1PKDuKtlt2l1J6VBsRF5gO1OLfT 5dANsCLDCLWaFrekYt2XXm9rNJagCy95oVMyTrVRCZq8fy8KBDoSuKb1gCS2b5yY1xJL oMhFn2wF/nYBWbbNC+/mKuDsC4XazojqCSg/2WEjevVqtSp5Eu/a9GBVRuB/R9+jmM0N FKTZ+UL3MVl0J0M22O3MR2bsYbmZT4REiePWVNDYjf4u71VKR9Jow/+jqP6StUDPnjXT haDg== X-Gm-Message-State: AJIora/HXMlfJV677eoGd+6HsKY7xHRk120Kim+zvMF4HSOEfq7SbUMh z4Q31QtXW5nz6gNNdzgcsWrT16XblHnE69iudWIKFCXUsFKL7i7agJ/zVXeGoqGMjXQr/xU99XZ p9GkPLfvetEvTU/KM2cEAVMT6EM9bBjmMQtbfqEhy1jnzB53xsVbj6CAWzao5to52lZRmVioN0w == X-Received: by 2002:a5d:4592:0:b0:21b:8e50:7fb9 with SMTP id p18-20020a5d4592000000b0021b8e507fb9mr12138712wrq.428.1656337235553; Mon, 27 Jun 2022 06:40:35 -0700 (PDT) X-Google-Smtp-Source: AGRyM1vh+Wkb52S1nvNA1TiohyZHmaHXpvESU5SsBWzAl6h2TTiGmod6B+7DF4A5n9tEj4x53xebWw== X-Received: by 2002:a5d:4592:0:b0:21b:8e50:7fb9 with SMTP id p18-20020a5d4592000000b0021b8e507fb9mr12138683wrq.428.1656337235245; Mon, 27 Jun 2022 06:40:35 -0700 (PDT) Received: from localhost (15.72.115.87.dyn.plus.net. [87.115.72.15]) by smtp.gmail.com with ESMTPSA id q5-20020adff945000000b0021b9585276dsm10328721wrr.101.2022.06.27.06.40.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Jun 2022 06:40:34 -0700 (PDT) From: Andrew Burgess To: Carl Love via Gdb-patches , gdb-patches@sourceware.org, will schmidt , cel@us.ibm.com, Ulrich Weigand Subject: Re: [PATCH] Fix for gdb.base/break-idempotent.exp In-Reply-To: <8338e122e114bb9091f2f06cd0012db826ece6cb.camel@us.ibm.com> References: <8338e122e114bb9091f2f06cd0012db826ece6cb.camel@us.ibm.com> Date: Mon, 27 Jun 2022 14:40:33 +0100 Message-ID: <87letidy26.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.1 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: Mon, 27 Jun 2022 13:40:39 -0000 Hi Carl, First, an apology, I posted this patch: https://sourceware.org/pipermail/gdb-patches/2022-June/190257.html without first checking the mailing list, so I missed that you had already posted this. You'll see that our fixes are pretty similar. Given you posted this first, I'm happy for you to merge your patch, however, I do have some feedback... Carl Love via Gdb-patches writes: > GDB maintainers: > > The gdb.base/break-idempotent.exp test generates errors trying to set > and remove breakpoints. The issue is gdb is trying to set and remove > breakpoints in skip_hw_watchpoint_tests not in break-idempotent. I'm not sure I agree with this. What I saw was problems when delete_breakpoints was called from break-idempotent.exp AFTER the skip_hw_watchpoint_tests call has caused GDB to exit. > The > issue is the skip_hw_watchpoint_test restarts gdb on the test hw > watchpoint test binary after gdb has been started for the break- > idempotent.exp test messing up the test. It's the fact that GDB is exited at the end of has_hw_wp_support that causes the immediate problem. Of couse, if we changed has_hw_wp_support so that GDB was left running then the restart would become the problem... > > This patch moves the check for hardware breakpoints before running the > break-idempotent tests. The move also improves the performance as the > skip_hw_watchpoint_test only needs to be run once rather for each test > case. The has_hw_wp_support check is cached, so there's no (significant) performance improvement; has_hw_wp_support is only ever called once. The key to the fix is thta skip_hw_watchpoint_tests, and hence has_hw_wp_support is now called before GDB is started as part of the main test. > > The patch has been tested on Power 10 and fixes the failures seen on > PowerPC. > > Please let me know if this patch is acceptable for mainline. > > Carl Love > > > ------------------------------------------------- > Fix for gdb.base/break-idempotent.exp > > --- > gdb/testsuite/gdb.base/break-idempotent.exp | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/gdb/testsuite/gdb.base/break-idempotent.exp b/gdb/testsuite/gdb.base/break-idempotent.exp > index 29002f103a8..38b7632a7fc 100644 > --- a/gdb/testsuite/gdb.base/break-idempotent.exp > +++ b/gdb/testsuite/gdb.base/break-idempotent.exp > @@ -145,6 +145,16 @@ proc test_break { always_inserted break_command } { > } > } > > +# The skip_hw_watchpoint_tests generates a small test source file to test if HW > +# watchpoints are supported. Do not want the compile and test for the skip > +# hw watchpoint to restart gdb after the break-idempotemt test has is started > +# on gdb. Do the skip_hw_watchpoint_tests first. How about this updated comment? # The skip_hw_watchpoint_tests starts GDB on a small test program to # check if HW watchpoints are supported. We do not want to restart # GDB after this test script has itself started GDB, so call # skip_hw_watchpoint_tests first. > +if {[skip_hw_watchpoint_tests]} { > + set skip_hw_wp 1 > +} else { > + set skip_hw_wp 0 > +} In my patch I wrote something similar, but, mentioned that skip_hw_watchpoint_tests caches its result, so just called skip_hw_watchpoint_tests without changing anything else. If you prefer your version, then that's fine, but please use true/false instead of 1/0 when setting skip_hw_wp. Thanks, Andrew > + > # The testcase uses the "file" command to force breakpoint re-set in > # GDB. Test both with and without PIE, as GDB used to mishandle > # breakpoint re-set when reloading PIEs. > @@ -174,7 +184,7 @@ foreach_with_prefix pie { "nopie" "pie" } { > test_break $always_inserted "hbreak" > } > > - if {![skip_hw_watchpoint_tests]} { > + if {!$skip_hw_wp} { > test_break $always_inserted "watch" > } > > -- > 2.31.1