From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107875 invoked by alias); 19 Feb 2020 13:30:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 107730 invoked by uid 89); 19 Feb 2020 13:30:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-13.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_LOTSOFHASH,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=robust X-HELO: mail-qk1-f196.google.com Received: from mail-qk1-f196.google.com (HELO mail-qk1-f196.google.com) (209.85.222.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Feb 2020 13:30:21 +0000 Received: by mail-qk1-f196.google.com with SMTP id d11so57230qko.8 for ; Wed, 19 Feb 2020 05:30:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=HWXV4IvcXeTaMkQJQVdIrN1hgJFHK1Lh3MTa4+HOvTg=; b=tggv1MHg0hW+irvShbvkn6ZrULv8KNMJj/rNsmQIvCs4fgbFG0Z8td6JnSmht08Rsq mLikVEUcBmb0NUvlfeIv+GE4YkBFtSRUMxg0RdfKb04pDYifNlCzJ7tnuz02Rcho4OOs 98fPrKdXX2Nd85l6rv79lLovzbINZYLy2f+VGEMYsNyGP3gEC4Zk2HEAxysE8am7M7zP TMzVImnZea5olVFq1Lc1MNQ4KbadVHIAlHE6S+BsbjZCAZCP9tkLIWo5BkJRDwyGeJ+z qthtST802D3XR6TgyHT05irr9byicLURji5+cELl2QQGPEtlBo/Ww0o3GwRNbd9Jj+5o 46cw== Return-Path: Received: from [192.168.0.185] ([191.34.219.95]) by smtp.gmail.com with ESMTPSA id y197sm966890qka.65.2020.02.19.05.30.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Feb 2020 05:30:03 -0800 (PST) Subject: Re: [PATCH v2 3/4] Remove breakpoint step-over information if failed to resume To: "Maciej W. Rozycki" , Pedro Alves , gdb-patches@sourceware.org Cc: Jim Wilson References: From: Luis Machado Message-ID: Date: Wed, 19 Feb 2020 13:30:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00771.txt.bz2 On 11/6/19 5:52 PM, Maciej W. Rozycki wrote: > Complement commit 31e77af205cf ("PR breakpoints/7143 - Watchpoint does > not trigger when first set") and commit 71d378ae60a4 > ("gdb.base/breakpoint-in-ro-region.exp regression on sss targets (PR > gdb/22583)") and also remove breakpoint step-over information if the > inferior has failed to resume, which may be due to for example a failure > to insert a software breakpoint at an inaccessible location. If this > happens, the state of GDB becomes inconsistent and further attempts to > start execution hang. > > This was observed with a `riscv64-linux-gnu' cross-GDB and RISC-V/Linux > `gdbserver' being developed and an attempt to step over the `ret' aka > `c.jr ra' instruction where the value held in the `ra' aka `x1' register > and therefore the address of a software-stepping breakpoint to insert is > 0. Here's a record of a remote debug session leading to the issue: > > Sending packet: $vCont?#49...Packet received: vCont;c;C;t > Packet vCont (verbose-resume) is supported > Sending packet: $vCont;c:p23cb.-1#08...Packet received: T05swbreak:;02:20da080000000000;20:b807565515000000;thread:p23cb.23cb;core:3; > Sending packet: $qXfer:libraries-svr4:read::0,1000#20...Packet received: l > Sending packet: $X15555607b8,2:\202\200#2e...Packet received: OK > Sending packet: $m15555607b8,2#07...Packet received: 8280 > Sending packet: $m15555607b8,2#07...Packet received: 8280 > Sending packet: $g#67...Packet received: 0000000000000000000000000000000020da08000000000000000000000000000000000000000000d0ae040000000000696e74000000000000000000000000000100000000000000020000000000000080d708000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000000e408000000000020e908000000000000000000000000000000000000000000d0ae04000000000072697363765f646f00000000000000000100000000000000030000000000000000e408000000000020f208000000000000000000000000000000000000000000d0ae04000000000072697363765f646f0000000000000000[552 bytes omitted] > Sending packet: $m0,40#2d...Packet received: E01 > Sending packet: $m0,1#fa...Packet received: E01 > Sending packet: $m0,40#2d...Packet received: E01 > Sending packet: $m0,1#fa...Packet received: E01 > Cannot access memory at address 0x0 > (gdb) stepi > Sending packet: $m15555607b8,4#09...Packet received: 82802a87 > Sending packet: $m15555607b4,4#05...Packet received: 01452dbd > ^Cremote_pass_ctrlc called > remote_interrupt called > ^Cremote_pass_ctrlc called > Interrupted while waiting for the program. > Give up waiting? (y or n) y > Quit > (gdb) > > As observed here the `stepi' command does not even attempt to reinsert > the breakpoint, let alone resume execution. > > Correct the issue by making a call to clear global breakpoint step-over > from the exception catch clause in `resume'. > > With the change applied further `stepi' commands correctly retry > breakpoint insertion: > > (gdb) stepi > Sending packet: $m15555607b8,4#09...Packet received: 82802a87 > Sending packet: $m15555607b4,4#05...Packet received: 01452dbd > Sending packet: $m15555607b8,2#07...Packet received: 8280 > Sending packet: $m15555607b8,2#07...Packet received: 8280 > Sending packet: $m0,40#2d...Packet received: E01 > Sending packet: $m0,1#fa...Packet received: E01 > Sending packet: $m0,40#2d...Packet received: E01 > Sending packet: $m0,1#fa...Packet received: E01 > Cannot access memory at address 0x0 > (gdb) > > and changing the value of the `pc' register so as not to point at a > `ret' instruction allows execution to be actually resumed. > > gdb/ > * infrun.c (resume): Also call `clear_step_over_info' in the > `catch' clause. > --- > Changes from v1: > > - Add a thread number argument to `clear_step_over_info'. > --- > gdb/infrun.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > gdb-resume-clear-step-over-info.diff > Index: binutils-gdb/gdb/infrun.c > =================================================================== > --- binutils-gdb.orig/gdb/infrun.c > +++ binutils-gdb/gdb/infrun.c > @@ -2620,7 +2620,12 @@ resume (gdb_signal sig) > single-step breakpoints perturbing other threads, in case > we're running in non-stop mode. */ > if (inferior_ptid != null_ptid) > - delete_single_step_breakpoints (inferior_thread ()); > + { > + thread_info *tp = inferior_thread (); > + > + delete_single_step_breakpoints (tp); > + clear_step_over_info (tp->global_num); > + } > throw; > } > } > I'd suggest the same approach of checking if the thread has step-over information and then clearing the state without the need to add a global thread number parameter to clear_step_over_info. Otherwise this looks reasonable to me. It would've been nice to have more robust rollback logic to handle such cases of failure.