From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 38E3B3858D1E for ; Wed, 10 Aug 2022 15:17:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 38E3B3858D1E Received: from pps.filterd (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 27AF79DQ024610 for ; Wed, 10 Aug 2022 15:17:28 GMT Received: from ppma01dal.us.ibm.com (83.d6.3fa9.ip4.static.sl-reverse.com [169.63.214.131]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3hvd4wn09x-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 10 Aug 2022 15:17:27 +0000 Received: from pps.filterd (ppma01dal.us.ibm.com [127.0.0.1]) by ppma01dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 27AF7cME021847 for ; Wed, 10 Aug 2022 15:17:21 GMT Received: from b03cxnp07027.gho.boulder.ibm.com (b03cxnp07027.gho.boulder.ibm.com [9.17.130.14]) by ppma01dal.us.ibm.com with ESMTP id 3huwvregew-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 10 Aug 2022 15:17:21 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp07027.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 27AFHJ1F33423652 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 10 Aug 2022 15:17:19 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5C0AA7805C; Wed, 10 Aug 2022 15:17:19 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D75017805F; Wed, 10 Aug 2022 15:17:18 +0000 (GMT) Received: from lexx (unknown [9.160.66.1]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Wed, 10 Aug 2022 15:17:18 +0000 (GMT) Message-ID: <20f1b450af2746c38b98e7e1d29805d35b475be1.camel@vnet.ibm.com> Subject: Re: [PATCH] Fix hardware watchpoint check in test gdb.base/watchpoint-reuse-slot.exp From: will schmidt To: Carl Love , "gdb-patches@sourceware.org" , Ulrich Weigand Date: Wed, 10 Aug 2022 10:17:18 -0500 In-Reply-To: <3783e7e44fe188af5ca1f2ddcfa4c7f5cb7a818e.camel@us.ibm.com> References: <3783e7e44fe188af5ca1f2ddcfa4c7f5cb7a818e.camel@us.ibm.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: xroxcUH0GL4KAG0ckPECMSG76h9hWXXi X-Proofpoint-ORIG-GUID: xroxcUH0GL4KAG0ckPECMSG76h9hWXXi X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-10_09,2022-08-10_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 phishscore=0 malwarescore=0 priorityscore=1501 adultscore=0 lowpriorityscore=0 clxscore=1015 impostorscore=0 spamscore=0 suspectscore=0 mlxlogscore=999 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2208100048 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, 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, 10 Aug 2022 15:17:31 -0000 On Tue, 2022-08-09 at 17:00 -0700, Carl Love wrote: > GDB maintainers: > > The gdb.base/watchpoint-reuse-slot.exp uses the check: > > if { ![target_info exists gdb,no_hardware_watchpoints]} > Some of the wording of the description made me think this patch was incomplete. a few comments below. > to determine if the test should be re-run with HW watchpoints. The test re-run or just run? > is TRUE on Power 9 however Power 9 does not support HW watchpoints. > Not all PowerPC processors support HW watchpoints. The test needs to > use the skip_hw_watchpoint_access_tests test to correctly determine if > the processor supports HW watchpoints. Power9 DD2 has breakpoints disabled by default, yes. > > The skip_hw_watchpoint_access_tests starts a small gdb test on the > platform to determine if the processor supports HW watchpoints or not. > Thus the check must be done before the actual test is run to ensure the > HW watchpoint check does not mess up the actual test. Per what I see in the sources (snapshot from a couple weeks ago), The existing skip_hw_watchpoint_access_tests proc calls skip_hw_watchpoint_tests that checks the targets and the cached has_hw_wp_support value. The has_hw_wp_support cached value holds the results of the small gdb test. > > This patch replaces the [target_info exists gdb, > no_hardware_watchpoints] with the skip_hw_watchpoint_access_tests > before starting the watchpoint-reuse-slot.exp test. ... in order to properly determine whether the watchpoints are disabled. > > The fix disables the HW watchpoint tests on Power 9 thus eliminating 48 > testsuite failures. > > The patch has been tested on Power 9, Power 10 and x86-64 > > Please let me know if this patch is acceptable for mainline. Thanks. > > Carl Love > > -------------------------------------------------------- > [PATCH] Fix hardware watchpoint check in test gdb.base/watchpoint-reuse-slot.exp > > This test generates 48 failures on Power 9 when testing with HW watchpoints > enabled. Note Power 9 does not support HW watchpoints. > > Not all PowerPC processors support HW watchpoints. Add the > skip_hw_watchpoint_access_tests to determine if the processor supports HW > watchpoints. Note this proceedure runs a small test on PowerPC under gdb to > determine if the processor supports HW watchpoints. Thus the check must be > done before the actual test to prevent disrupting the actual test. Noting that this patch does not "add" the skip_hw_watchpoint_access_tests proc. The existing skip_hw_watchpoint_tests proc includes the comment, and the relevant stanza # These targets support hardware watchpoints natively # Note, not all Power 9 processors support hardware watchpoints due to a HW # bug. Use has_hw_wp_support to check do a runtime check for hardware # watchpoint support on Powerpc. ... || ([istarget "powerpc*-*-linux*"] && [has_hw_wp_support]) The has_hw_wp_support() proc includes the comment # Power 9, proc rev 2.2 does not support HW watchpoints due to HW bug. # Need to use a runtime test to determine if the Power processor has # support for HW watchpoints. > > This patch was tested on Power 9, Power 10 and X86-64 with no regressions. > --- > gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp > index e2ea137424b..829252a65b4 100644 > --- a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp > +++ b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp > @@ -22,6 +22,18 @@ > # operation. (Note that we don't have any of these watchpoints > # trigger.) > > +# The skip_hw_watchpoint_tests checks if watchpoints are supported by the > +# processor. Not all PowerPC proceesors support HW watchpoints. The Calling out PowerPC explicitly here doesn't seem correct, unless you also call out every other target that is not in the list in skip_hw_watchpoint_tests(). > +# skip_hw_watchpoint_access_tests starts GDB on a small test program to > +# dynamically 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 and save the result for use later. > +if {[skip_hw_watchpoint_access_tests]} { > + set supports_hw_wp 0 > +} else { > + set supports_hw_wp 1 > +} > + > standard_testfile > > if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { > @@ -285,7 +297,7 @@ proc setup_and_run_watchpoints_tests { hw_wp_p } { > > # Run tests with hardware watchpoints disabled, then again with them > # enabled (if this target supports hardware watchpoints). > -if { ![target_info exists gdb,no_hardware_watchpoints]} { > +if { $supports_hw_wp } { So.. could this be simplified with a check against the existing cached has_hw_wp_support value? > # Run test with H/W enabled. > setup_and_run_watchpoints_tests 1 > }