From patchwork Fri Jul 31 14:36:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antoine Tremblay X-Patchwork-Id: 7953 Received: (qmail 123899 invoked by alias); 31 Jul 2015 14:36:23 -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 123865 invoked by uid 89); 31 Jul 2015 14:36:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 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, 31 Jul 2015 14:36:21 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id C9.74.12958.38B2BB55; Fri, 31 Jul 2015 10:02:11 +0200 (CEST) Received: from elxa4wqvvz1.dyn.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.96) with Microsoft SMTP Server (TLS) id 14.3.210.2; Fri, 31 Jul 2015 10:36:17 -0400 From: Antoine Tremblay To: CC: Antoine Tremblay Subject: [PATCH] Fix instruction skipping when using software single step in gdbserver. Date: Fri, 31 Jul 2015 10:36:07 -0400 Message-ID: <1438353367-24936-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 skip 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 insctruction 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. gdb/gdbserver/ChangeLog: * gdb/gdbserver/linux-low.c (linux_wait_1): Fix pc advance condition. * gdb/gdbserver/mem-break.c (reinsert_breakpoint_inserted_here): New function. * gdb/gdbserver/mem-break.h: Add reinsert_breakpoint_inserted_here. --- gdb/gdbserver/linux-low.c | 23 +++++++++++++++-------- gdb/gdbserver/mem-break.c | 15 +++++++++++++++ gdb/gdbserver/mem-break.h | 4 ++++ 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 2dafb03..e5fb430 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -2852,14 +2852,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))) { unsigned int increment_pc = the_low_target.breakpoint_len; diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c index 4eaa52b..489caf2 100644 --- a/gdb/gdbserver/mem-break.c +++ b/gdb/gdbserver/mem-break.c @@ -1660,6 +1660,21 @@ hardware_breakpoint_inserted_here (CORE_ADDR addr) return 0; } +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 b5a3208..8afefef 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. */