From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id /1HAIytPA2FkUwAAWB0awg (envelope-from ) for ; Thu, 29 Jul 2021 21:00:27 -0400 Received: by simark.ca (Postfix, from userid 112) id 840731EDFB; Thu, 29 Jul 2021 21:00:27 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 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 8D75B1E813 for ; Thu, 29 Jul 2021 21:00:26 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E9B8D3835C0E for ; Fri, 30 Jul 2021 01:00:25 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E9B8D3835C0E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1627606826; bh=XN6Bp3Y7IHR3hMjM74zI3W/pa4n9LfMuKG5TFeM5hVg=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=AQ8iB0tqe5xSTgvI0mJiryRr5uQ9FFlx40xlX3kmn2gsyjiXBfSvl+gvI+sV5/za6 xk31sqPDx6Hw2ElPyPkaT1eZ4aCnn+J4Afwlf2QGz+32zHmjH0MYqoHYW60fodXA96 w2m890YXFtX0/Spv6Rvs8lAfXtaIhlnz7qPcssWA= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id C88D3385781A for ; Fri, 30 Jul 2021 01:00:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C88D3385781A Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 16U0xxCK032550 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 29 Jul 2021 21:00:04 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 16U0xxCK032550 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 43E3D1E813; Thu, 29 Jul 2021 20:59:59 -0400 (EDT) Subject: Re: [PATCH] Fix displaced stepping watchpoint check order To: Luis Machado , gdb-patches@sourceware.org References: <20210608154230.354202-1-luis.machado@linaro.org> Message-ID: <9c697f35-1059-5ea8-83c3-ea75fcc95b32@polymtl.ca> Date: Thu, 29 Jul 2021 20:59:58 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 30 Jul 2021 00:59:59 +0000 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: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 2021-07-29 4:17 p.m., Luis Machado wrote: > Hi, > > On 7/29/21 4:36 PM, Simon Marchi wrote: >> I think this is ok, but in all honestly I don't completely understand >> how the interaction between watchpoints and displaced stepping is >> expected to work. > > Let me try to clarify. When we attempt to execute an instruction in the scratch space (displaced stepping), we may potentially trigger a hardware watchpoint. > > For AArch64, hardware watchpoints are non-steppable, so that means GDB will need to step over that hardware watchpoint so the instruction's execution completes (if there is no hardware watchpoint trigger, the instruction gets executed just fine). What does "need to step over that hardware watchpoint" means, concretely? After the watchpoint has triggered, are the side effects of the instruction committed to memory and registers? Or are we in a state as if the instruction didn't complete? How is that step over done? >> >> Just some nits: >> >> On 2021-06-08 11:42 a.m., Luis Machado via Gdb-patches wrote: >>> When checking the stopped data address, I noticed, under some circumstances, >>> that the instruction at PC wasn't the expected one. This happens because the >>> displaced stepping machinery restores the buffer before checking if the >>> instruction executed successfully, which in turn calls the watchpoint check. >>> >>> I guess this was never noticed because stopped data address checks usually >>> don't need to fetch the instruction at PC, but AArch64 needs to do it from >>> now on. >> >> Can you clarify what you mean by "from now on"? Can you indicate what >> change you are referring to? >> > > From the following change (https://sourceware.org/pipermail/gdb-patches/2021-July/181095.html) onwards, we need to look at the load/store instruction to figure out the memory access size so we can reliably tell if a hardware watchpoint has triggered. This is due to how AArch64's spec defines how to provide a stopped data address, and the valid ranges. Ok, but that patch you linked isn't merged yet? So it sounds strange to say "from now on", it sounds like there's a dependency between the two patches. Let's say the current patch is merged before the other one, maybe it should say "but AArch64 will need to do it it an upcoming patch", and then you can given the link. > With the old code, if we try to fetch the instruction at PC, we will get a bogus value that is not the real instruction that caused the hardware watchpoint trigger. Hence why the patch moves the call to displaced_step_instruction_executed_successfully (...) up and before we restore the displaced stepping buffer. > > If a hardware watchpoint trigger takes place and GDB doesn't recognize it, then displaced_step_instruction_executed_successfully (...) will return true and GDB will move on and will attempt to execute the same instruction again, only to be halted due to the same hardware watchpoint trigger that it can't detect. So GDB gets into an infinite loop. > > More generally, if we ever fail to acknowledge a hardware watchpoint trigger on an architecture with non-steppable watchpoints and displaced stepping support, we will run into this infinite loop (as far as I can tell). > > Does that make sense? Yes, this help. Please feel free to include in the commit message any additional detail that you gave here, since it might help somebody else in the future. The code change is fine, so once you send a revised version it should go fairly quickly. Sorry for the delay. Thanks! Simon