From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42781 invoked by alias); 11 Apr 2016 14:31:26 -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 38517 invoked by uid 89); 11 Apr 2016 14:31:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=hitting, pause, Hx-languages-length:4692 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 11 Apr 2016 14:31:13 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A3094C003582; Mon, 11 Apr 2016 14:31:12 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3BEVBTt003156; Mon, 11 Apr 2016 10:31:12 -0400 Subject: Re: [PATCH 3/7] Force to insert software single step breakpoint To: Yao Qi , gdb-patches@sourceware.org References: <1458749384-19793-1-git-send-email-yao.qi@linaro.org> <1458749384-19793-4-git-send-email-yao.qi@linaro.org> From: Pedro Alves Message-ID: <570BB52F.7@redhat.com> Date: Mon, 11 Apr 2016 14:31:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <1458749384-19793-4-git-send-email-yao.qi@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-04/txt/msg00211.txt.bz2 On 03/23/2016 04:09 PM, Yao Qi wrote: > GDB doesn't insert software single step breakpoint if the instruction > branches to itself, so that the program can't stop after command "si". > > (gdb) b 32 > Breakpoint 2 at 0x8680: file git/gdb/testsuite/gdb.base/branch-to-self.c, line 32. > (gdb) c > Continuing. > > Breakpoint 2, main () at gdb/git/gdb/testsuite/gdb.base/branch-to-self.c:32 > 32 asm (".Lhere: " BRANCH_INSN " .Lhere"); /* loop-line */ > (gdb) si > infrun: clear_proceed_status_thread (Thread 3991.3991) > infrun: proceed (addr=0xffffffff, signal=GDB_SIGNAL_DEFAULT) > infrun: step-over queue now empty > infrun: resuming [Thread 3991.3991] for step-over > infrun: skipping breakpoint: stepping past insn at: 0x8680 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Sending packet: $Z0,8678,4#f3...Packet received: OK > infrun: skipping breakpoint: stepping past insn at: 0x8680 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Sending packet: $Z0,b6fe86c8,4#82...Packet received: OK > infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=1, current thread [Thread 3991.3991] at 0x868 > > breakpoint.c:should_be_inserted thinks the breakpoint shouldn't be > inserted, which is wrong. This patch restrict the condition that only > return false if breakpoint is NOT single step breakpoint. > > gdb: > > 2016-03-23 Yao Qi > > * breakpoint.c (should_be_inserted): Don't return 0 if single > step breakpoint is inserted at the address we're stepping over. > * gdbarch.sh (software_single_step): Update comments. > * gdbarch.h: Regenerated. > --- > gdb/breakpoint.c | 9 ++++++++- > gdb/gdbarch.h | 5 ++++- > gdb/gdbarch.sh | 5 ++++- > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index f99a7ab..9ecfb07 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -2219,9 +2219,16 @@ should_be_inserted (struct bp_location *bl) > return 0; > > /* Don't insert a breakpoint if we're trying to step past its > - location. */ > + location except single step breakpoint, because the single step > + breakpoint may be inserted at the location we're trying to step > + if the instruction branches to itself. However, the instruction > + won't be executed at all and it may break the semantics of the > + instruction, for example, the instruction is a conditional > + branch or updates some flags. We can't fix it unless GDB is able > + to emulate the instruction or switch to displaced stepping. */ > if ((bl->loc_type == bp_loc_software_breakpoint > || bl->loc_type == bp_loc_hardware_breakpoint) > + && bl->owner->type != bp_single_step > && stepping_past_instruction_at (bl->pspace->aspace, > bl->address)) Another scenario occurred to me: - Thread A is software single-stepping. - Thread B hits single-step breakpoint of thread A. - We pause all threads and set thread B stepping past the single-step breakpoint of thread A. But if the single-step breakpoint is for another thread, then we won't actually manage to have thread B step past it, resulting in spurious re-hits and no-guaranteed forward progress. See e.g., non-stop-fair-events.exp: # On software single-step targets that don't support displaced # stepping, threads keep hitting each others' single-step # breakpoints, and then GDB needs to pause all threads to step # past those. The end result is that progress in the main # thread will be slower and it may take a bit longer for the # signal to be queued; bump the timeout. Sounds like we may need to look at the single-step breakpoint's thread id, and only insert it if it is for the thread that is going to be doing the step-over? We may need to record that in step_over_info and pass more info to stepping_past_instruction_at. > --- a/gdb/gdbarch.sh > +++ b/gdb/gdbarch.sh > @@ -609,7 +609,10 @@ m:CORE_ADDR:addr_bits_remove:CORE_ADDR addr:addr::core_addr_identity::0 > # target can single step. If not, then implement single step using breakpoints. > # > # A return value of 1 means that the software_single_step breakpoints > -# were inserted; 0 means they were not. > +# were inserted; 0 means they were not. Multiple breakpoints may be > +# inserted for some instructions such as conditional branch. However, > +# each implementation must always evaluate the condition and only put > +# the breakpoint at the branch destination if the condition is true. I'd add: (...) condition is true, so that we ensure forward progress when stepping past a conditional branch to self. This will help porters evaluate whether that's really necessary for their ports. Thanks, Pedro Alves