From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11750 invoked by alias); 4 Nov 2015 18:08:00 -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 11653 invoked by uid 89); 4 Nov 2015 18:07:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 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; Wed, 04 Nov 2015 18:07:58 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id B1E3F8BA63; Wed, 4 Nov 2015 18:07:56 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tA4I7tWe031113; Wed, 4 Nov 2015 13:07:55 -0500 Message-ID: <563A497A.5090900@redhat.com> Date: Wed, 04 Nov 2015 18:08:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Antoine Tremblay , gdb-patches@sourceware.org Subject: Re: [PATCH 02/10] Fix instruction skipping when using software single step in GDBServer References: <1446138583-13268-1-git-send-email-antoine.tremblay@ericsson.com> <1446138583-13268-3-git-send-email-antoine.tremblay@ericsson.com> In-Reply-To: <1446138583-13268-3-git-send-email-antoine.tremblay@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-11/txt/msg00149.txt.bz2 On 10/29/2015 05:09 PM, Antoine Tremblay wrote: > --- > gdb/gdbserver/linux-low.c | 23 +++++++++++++++-------- > gdb/gdbserver/mem-break.c | 17 +++++++++++++++++ > gdb/gdbserver/mem-break.h | 4 ++++ > 3 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 3b6c131..853a289 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -2993,14 +2993,21 @@ linux_wait_1 (ptid_t ptid, > return ptid_of (current_thread); > } > > - /* If step-over executes a breakpoint instruction, it means a > - gdb/gdbserver breakpoint had been planted on top of a permanent > - breakpoint. The PC has been adjusted by > - check_stopped_by_breakpoint to point at the breakpoint address. > - Advance the PC manually past the breakpoint, otherwise the > - program would keep trapping the permanent breakpoint forever. */ > - if (!ptid_equal (step_over_bkpt, null_ptid) > - && event_child->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT) > + /* If step-over executes a breakpoint instruction, in the case of a > + hardware single step it means a gdb/gdbserver breakpoint had been > + planted on top of a permanent breakpoint, in the case of a software > + single step it may just mean that gdbserver hit the reinsert breakpoint. > + The PC has been adjusted by check_stopped_by_breakpoint to point at > + the breakpoint address. > + So in the case of the hardware single step advance the PC manually > + past the breakpoint and in the case of software single step advance only > + if it's not the reinsert_breakpoint we are hitting. > + This avoids that a program would keep trapping a permanent breakpoint > + forever. */ > + if ((!ptid_equal (step_over_bkpt, null_ptid) > + && event_child->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT) && > + (event_child->stepping || > + !reinsert_breakpoint_inserted_here (event_child->stop_pc))) Formatting isn't right. && and || go at the beginning of the next line. Unnecessary parens. Like: if (!ptid_equal (step_over_bkpt, null_ptid) && event_child->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT && (event_child->stepping || !reinsert_breakpoint_inserted_here (event_child->stop_pc))) Looks good to me with that change. Thanks, Pedro Alves