From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Q0jzMvbw82JPdiQAWB0awg (envelope-from ) for ; Wed, 10 Aug 2022 13:55:02 -0400 Received: by simark.ca (Postfix, from userid 112) id C4BFC1E5EA; Wed, 10 Aug 2022 13:55:02 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=Kmn9l/m3; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 425381E222 for ; Wed, 10 Aug 2022 13:55:02 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A6D72385740E for ; Wed, 10 Aug 2022 17:55:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A6D72385740E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1660154101; bh=S098VjOVCyc9sMSP+JD4cjeOIWrbqgljc6mXwO96rKA=; h=Subject:To:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Kmn9l/m3LPnNgaiBNGK9sVu+ToEIc5YttDQQp8LIvhy2PSv1qaUPSyXqTIsTFKpHg uIq6tZhurz5rdrrcUPDSWhj1MqfbiOp8c9WnJ2gRq4XEIrnq/HruMKToZjK66cBYat 6HVazg0Vs0DsebHjlsTx5NmwLzpmKziQ/5PkinMw= Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id E5C2E3858D1E for ; Wed, 10 Aug 2022 17:54:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E5C2E3858D1E Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 27AHZxXF000936 for ; Wed, 10 Aug 2022 17:54:40 GMT Received: from ppma05wdc.us.ibm.com (1b.90.2fa9.ip4.static.sl-reverse.com [169.47.144.27]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3hv65wgdaw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 10 Aug 2022 17:54:39 +0000 Received: from pps.filterd (ppma05wdc.us.ibm.com [127.0.0.1]) by ppma05wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 27AHqi3S018057 for ; Wed, 10 Aug 2022 17:54:38 GMT Received: from b03cxnp08025.gho.boulder.ibm.com (b03cxnp08025.gho.boulder.ibm.com [9.17.130.17]) by ppma05wdc.us.ibm.com with ESMTP id 3hvcmr9hm8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 10 Aug 2022 17:54:38 +0000 Received: from b03ledav001.gho.boulder.ibm.com (b03ledav001.gho.boulder.ibm.com [9.17.130.232]) by b03cxnp08025.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 27AHsc0Y37618024 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 10 Aug 2022 17:54:38 GMT Received: from b03ledav001.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E7CE86E050; Wed, 10 Aug 2022 17:54:37 +0000 (GMT) Received: from b03ledav001.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id AC6F46E04E; Wed, 10 Aug 2022 17:54:37 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.211.44.164]) by b03ledav001.gho.boulder.ibm.com (Postfix) with ESMTP; Wed, 10 Aug 2022 17:54:37 +0000 (GMT) Message-ID: <5596a4dc585b51beea3ba1138262ab71014e7c89.camel@us.ibm.com> Subject: Re: [PATCH] Fix hardware watchpoint check in test gdb.base/watchpoint-reuse-slot.exp To: will schmidt , "gdb-patches@sourceware.org" , Ulrich Weigand Date: Wed, 10 Aug 2022 10:54:37 -0700 In-Reply-To: <20f1b450af2746c38b98e7e1d29805d35b475be1.camel@vnet.ibm.com> References: <3783e7e44fe188af5ca1f2ddcfa4c7f5cb7a818e.camel@us.ibm.com> <20f1b450af2746c38b98e7e1d29805d35b475be1.camel@vnet.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-ORIG-GUID: PfHdGYDWsUY5psD8vNv_6bp09kVFYTMX X-Proofpoint-GUID: PfHdGYDWsUY5psD8vNv_6bp09kVFYTMX 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_11,2022-08-10_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 mlxlogscore=999 impostorscore=0 lowpriorityscore=0 malwarescore=0 clxscore=1015 suspectscore=0 priorityscore=1501 phishscore=0 mlxscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2208100053 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: , From: Carl Love via Gdb-patches Reply-To: Carl Love Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On Wed, 2022-08-10 at 10:17 -0500, will schmidt wrote: > 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? The tests are run with HW watchpoints disabled. Then, if the processor supports HW watchpoints, the expect script runs the tests again with HW watchpoints enabled. Will reword to make it clearer. > > > 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. Yes, and it is important that the skip_hw_watchpoint_access_tests check is run first and not after the test is started because the check will restart gdb and thus mess up the 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. OK, will update the wording. > > > > > 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 True, it is really replacing the existing check with a new check. > 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. > > Yes. > > > > 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(). Yes, we can drop the part specifically calling out Power 9. > > > > +# 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? Yes, I hadn't thought about that. I updated the patch to just check the cached value. > > > > # Run test with H/W enabled. > > setup_and_run_watchpoints_tests 1 > > }