From patchwork Fri Nov 6 19:41:32 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antoine Tremblay X-Patchwork-Id: 9584 Received: (qmail 79513 invoked by alias); 6 Nov 2015 19:41:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 79433 invoked by uid 89); 6 Nov 2015 19:41:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg20.ericsson.net Received: from usevmg20.ericsson.net (HELO usevmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 06 Nov 2015 19:41:54 +0000 Received: from EUSAAHC005.ericsson.se (Unknown_Domain [147.117.188.87]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id AB.08.32596.F01AC365; Fri, 6 Nov 2015 13:46:08 +0100 (CET) Received: from elxa4wqvvz1.dyn.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.87) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 6 Nov 2015 14:41:50 -0500 From: Antoine Tremblay To: CC: Antoine Tremblay Subject: [PATCH v2 02/10] Fix instruction skipping when using software single step in GDBServer Date: Fri, 6 Nov 2015 14:41:32 -0500 Message-ID: <1446838900-14604-3-git-send-email-antoine.tremblay@ericsson.com> In-Reply-To: <1446838900-14604-1-git-send-email-antoine.tremblay@ericsson.com> References: <1446838900-14604-1-git-send-email-antoine.tremblay@ericsson.com> MIME-Version: 1.0 X-IsSubscribed: yes Without this patch, when doing a software single step, with for example a conditional breakpoint, gdbserver would wrongly avance the pc of breakpoint_len and skips an instruction. This is due to gdbserver assuming that it's hardware single stepping. When it resumes from the breakpoint address it expects the trap to be caused by ptrace and if it's rather caused by a software breakpoint it assumes this is a permanent breakpoint and that it needs to skip over it. However when software single stepping, this breakpoint is legitimate as it's the reinsert breakpoint gdbserver has put in place to break at the next instruction. Thus gdbserver wrongly advances the pc and skips an instruction. This patch fixes this behavior so that gdbserver checks if it is a reinsert breakpoint from software single stepping. If it is it won't advance the pc. And if there's no reinsert breakpoint there we assume then that it's a permanent breakpoint and advance the pc. Here's a commented log of what would happen before and after the fix on gdbserver : /* Here there is a conditional breakpoint at 0x10428 that needs to be stepped over. */ Need step over [LWP 11204]? yes, found breakpoint at 0x10428 ... /* e7f001f0 is a breakpoint instruction on arm Here gdbserver writes the software breakpoint we would like to hit */ Writing e7f001f0 to 0x0001042c in process 11204 ... Resuming lwp 11220 (continue, signal 0, stop not expected) pending reinsert at 0x10428 stop pc is 00010428 continue from pc 0x10428 ... /* Here gdbserver hit the software breakpoint that was in place for the step over */ stop pc is 0001042c pc is 0x1042c step-over for LWP 11220.11220 executed software breakpoint Finished step over. Could not find fast tracepoint jump at 0x10428 in list (reinserting). /* Here gdbserver writes back the original instruction */ Writing e50b3008 to 0x0001042c in process 11220 Step-over finished. Need step over [LWP 11220]? No /* Here because gdbserver assumes this is a permenant breakpoint it advances the pc of breakpoint_len, in this case 4 bytes, so we have just skipped the instruction that was written back here : Writing e50b3008 to 0x0001042c in process 11220 */ stop pc is 00010430 pc is 0x10430 Need step over [LWP 11220]? No, no breakpoint found at 0x10430 Proceeding, no step-over needed proceed_one_lwp: lwp 11220 stop pc is 00010430 This patch fixes this situation and we get the right behavior : Writing e50b3008 to 0x0001042c in process 11245 Hit a gdbserver breakpoint. Hit a gdbserver breakpoint. Step-over finished. proceeding all threads. Need step over [LWP 11245]? No stop pc is 0001042c pc is 0x1042c Need step over [LWP 11245]? No, no breakpoint found at 0x1042c Proceeding, no step-over needed proceed_one_lwp: lwp 11245 stop pc is 0001042c pc is 0x1042c Resuming lwp 11245 (continue, signal 0, stop not expected) stop pc is 0001042c continue from pc 0x1042c It also works if the value at 0x0001042c is a permanent breakpoint. If so gdbserver will finish the step over, remove the reinserted breakpoint, resume at that location and on the next SIGTRAP gdbserver will trigger the advance PC condition as reinsert_breakpoint_inserted_here will be false. I also tested this against bp-permanent.exp on arm (with a work in progress software single step patchset) without any regressions. It's also tested against x86 bp-permanent.exp without any regression. So both software and hardware single step are tested. No regressions on Ubuntu 14.04 on ARMv7 and x86. With gdbserver-{native,extended} / { -marm -mthumb } gdb/gdbserver/ChangeLog: * linux-low.c (linux_wait_1): Fix pc advance condition. * mem-break.c (reinsert_breakpoint_inserted_here): New function. * mem-break.h (reinsert_breakpoint_inserted_here): New declaration. --- gdb/gdbserver/linux-low.c | 21 ++++++++++++++------- gdb/gdbserver/mem-break.c | 17 +++++++++++++++++ gdb/gdbserver/mem-break.h | 4 ++++ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 3b6c131..63ac8b3 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 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->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT + && (event_child->stepping + || !reinsert_breakpoint_inserted_here (event_child->stop_pc))) { int increment_pc = 0; int breakpoint_kind = 0; diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c index c808a84..267f29d 100644 --- a/gdb/gdbserver/mem-break.c +++ b/gdb/gdbserver/mem-break.c @@ -1665,6 +1665,23 @@ hardware_breakpoint_inserted_here (CORE_ADDR addr) return 0; } +/* See mem-break.h. */ + +int +reinsert_breakpoint_inserted_here (CORE_ADDR addr) +{ + struct process_info *proc = current_process (); + struct breakpoint *bp; + + for (bp = proc->breakpoints; bp != NULL; bp = bp->next) + if (bp->type == reinsert_breakpoint + && bp->raw->pc == addr + && bp->raw->inserted) + return 1; + + return 0; +} + static int validate_inserted_breakpoint (struct raw_breakpoint *bp) { diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h index d199cc4..40b66a7 100644 --- a/gdb/gdbserver/mem-break.h +++ b/gdb/gdbserver/mem-break.h @@ -100,6 +100,10 @@ int software_breakpoint_inserted_here (CORE_ADDR addr); int hardware_breakpoint_inserted_here (CORE_ADDR addr); +/* Returns TRUE if there's any reinsert breakpoint at ADDR. */ + +int reinsert_breakpoint_inserted_here (CORE_ADDR addr); + /* Clear all breakpoint conditions and commands associated with a breakpoint. */