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 ADC193858D32 for ; Tue, 18 Oct 2022 16:42:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ADC193858D32 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29IGd1Mq020180 for ; Tue, 18 Oct 2022 16:42:32 GMT Received: from ppma04dal.us.ibm.com (7a.29.35a9.ip4.static.sl-reverse.com [169.53.41.122]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3k9yfvrpq0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 18 Oct 2022 16:42:32 +0000 Received: from pps.filterd (ppma04dal.us.ibm.com [127.0.0.1]) by ppma04dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 29IGZMGM026211 for ; Tue, 18 Oct 2022 16:42:31 GMT Received: from b03cxnp08027.gho.boulder.ibm.com (b03cxnp08027.gho.boulder.ibm.com [9.17.130.19]) by ppma04dal.us.ibm.com with ESMTP id 3k7mgb2esf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 18 Oct 2022 16:42:31 +0000 Received: from smtpav06.dal12v.mail.ibm.com ([9.208.128.130]) by b03cxnp08027.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 29IGgSEH30737036 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 18 Oct 2022 16:42:28 GMT Received: from smtpav06.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CFDF05805E; Tue, 18 Oct 2022 16:42:29 +0000 (GMT) Received: from smtpav06.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7497258043; Tue, 18 Oct 2022 16:42:29 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.211.71.228]) by smtpav06.dal12v.mail.ibm.com (Postfix) with ESMTP; Tue, 18 Oct 2022 16:42:29 +0000 (GMT) Message-ID: <23ac041b4d1423f50fc4f933653fde30a1bf9b92.camel@us.ibm.com> Subject: [PATCH] Remove REPARSE condition to force hardware resource checking when updating watchpoints From: Carl Love To: "gdb-patches@sourceware.org" , Ulrich Weigand Date: Tue, 18 Oct 2022 09:42:29 -0700 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: UZnSSLvq_DHMUKxwOpX1cS-Kt7jzXTnD X-Proofpoint-GUID: UZnSSLvq_DHMUKxwOpX1cS-Kt7jzXTnD X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-10-18_06,2022-10-18_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 priorityscore=1501 malwarescore=0 mlxscore=0 bulkscore=0 spamscore=0 adultscore=0 mlxlogscore=999 impostorscore=0 phishscore=0 lowpriorityscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000 definitions=main-2210180094 X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP 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: Tue, 18 Oct 2022 16:42:35 -0000 GDB maintainers: The hardware resource checking done in function update_watchpoints in file gdb/breakpoints.c is currently only done when the input argument REPARSE is set to one. Function update_watchpoints is called with REPARSE set to 1 to verify there are enough hardware resources available to use hardware watchpoings when executing the watch command. The variable being watched can later change and thus change the number of hardware resources required for the watchpoint. However, the resource checking is not redone in update_watchpoint since the function is not called again with REPARSE set to 1. This can result the number of addresses to be watched exceeding the available hardware watch registers. This issue occurs on Power 10 when executing the gdb.base/watchpoint.exe test. When the issue occurs the test fails on an internal gdb error. The issue can be fixed by forcing the resource checking to be redone each time update_watchpoints is called. This patch makes forces the resources to be rechecked on each call to update_watchpoints by removing the "if (reparse)" line in the function. The change allows gdb to catch that the resource requirements changed and handle the situation correctly. The patch has been tested on Power 8, Power 10 and X86-64 without regressions. The patch has no effect on Power 9 as hardware watchpoints are disabled on that processor. Please let me know if this patch is acceptable for mainline. Thanks. Carl Love ---------------------------- Remove REPARSE condition to force hardware resource checking when updating watchpoints Currently the resource checking is done if REPARSE is true. The hardware watchpoint resource checking in update_watchpoint needs to be redone on each call to function update_watchpoints as the value chain may have changed. The number of hardware registers needed for a watchpoint can change if the variable being watched changes. This situation occurs in this test when watching variable **global_ptr_ptr. Initially when the watch command is issued, only two addresses need to be watched as **global_ptr_ptr has not yet been initialized. Once the value of **global_ptr_ptr is initialized the locations to be tracked increase to three addresses. However, update_watchpoints is not called again with REPARSE set to 1 to force the resource checking to be redone. When the test is run on Power 10, an internal gdb error occurs when the PowerPC routine tries to setup the three hardware watchpoint address since the hw only has two hardware watchpoint registers. The error occurs because the resource checking was not redone in update_watchpoints after **global_ptr_ptr changed. The following descibes the situation in detail that occurs on Power 10 with gdb running on the binary for gdb.base/watchpoint.c. 1 break func4 2 run 3 watch *global_ptr 4 next execute source code: buf[0] = 3; 5 next execute source code: global_ptr = buf; 6 next execute source code: buf[0] = 7; 7 delete 2 (delete watch *global_ptr) 8 watch **global_ptr_ptr 9 next execute source code: buf[1] = 5; 10 next global_ptr_ptr = &global_ptr; 11 next buf[0] = 9; In step 8, the the watch **global_ptr_prt command calls update_watchpoint in breakpoint.c with REPARSE set to 1. The function update_watchpoint calls can_use_hardware_watchpoint to see if there are enough resources available to add the watchpoint since REPARSE is set to 1. At this point, **global_ptr_ptr has not been initialized so only two addresses are watched. The val_chain contains the address for **global_ptr_ptr and 0 since **global_ptr_ptr has not been initialized. The update_watchpoint updates the breakpoint list as follows: breakpoint 0 loc 0: b->address = 0x100009c0 breakpoint 1 loc 1: b->address = 0x7ffff7f838a0 breakpoint 2 loc 2: b->address = 0x7ffff7b7fc54 breakpoint 3 loc 3: b->address = 0x7ffff7a5788c breakpoint 4 loc 4: b->address = 0x0 <-- location pointed to by global_ptr_ptr loc 5: b->address = 0x100200b8 <-- global_ptr_ptr watchpoint breakpoint 5 loc 6: b->address = 0x7ffff7b7fc54 In step 10, the next command executes the source code global_ptr_ptr = &global_ptr. This changes the set of locations to be watched for the watchpoint **global_ptr_prt. The list of addresses for the breakpoint consist of the address for global_ptr_prt, global_ptr and buf. The breakpoint list gets updated by update_watchpoint as follows: breakpoint 0 loc 0: b->address = 0x100009c0 breakpoint 1 loc 1: b->address = 0x7ffff7f838a0 breakpoint 2 loc 2: b->address = 0x7ffff7b7fc54 breakpoint 3 loc 3: b->address = 0x7ffff7a5788c breakpoint 4 loc 4: b->address = 0x10020050 buf loc 5: b->address = 0x100200b0 watch *global_ptr loc 6: b->address = 0x100200b8 watch **global_ptr_ptr breakpoint 5 loc 7: b->address = 0x7ffff7b7fc54 breakpoint 6 However, the hardware resource checking was not redone because update_breakpoint was called with REPARSE equal to 0. Step 11, execute the third next command. The function ppc_linux_nat_target::low_prepare_to_resume() attempts a ptrace call to setup each of the three address for breakpoint 4. The slot value returned for the third ptrace call is -1 indicating an error because there are only two hardware watchpoint registers available on Power 10. This patch removes just the statement "if (reparse)" in function update_watchpoint to force the resources to be rechecked on every call to the function. This ensures that any changes to the val_chain resulting in needing more resources then available will be caught. The patch has been tested on Power 8, Power 10 and X86-64. Note the patch has no effect on Power 9 since hardware watchpoint support is disabled on that processor. --- gdb/breakpoint.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index e9aee6315fa..24c4af3c968 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2126,10 +2126,9 @@ update_watchpoint (struct watchpoint *b, int reparse) } /* Change the type of breakpoint between hardware assisted or - an ordinary watchpoint depending on the hardware support - and free hardware slots. REPARSE is set when the inferior - is started. */ - if (reparse) + an ordinary watchpoint depending on the hardware support and + free hardware slots. Recheck the number of free hardware slots + as the value chain may have changed. */ { int reg_cnt; enum bp_loc_type loc_type; -- 2.37.2