This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] Fix instruction skipping when using software single step in GDBServer


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2d97cd356e0f0320ecb71cf6a10616ba4618f318

commit 2d97cd356e0f0320ecb71cf6a10616ba4618f318
Author: Antoine Tremblay <antoine.tremblay@ericsson.com>
Date:   Mon Nov 30 15:16:22 2015 -0500

    Fix instruction skipping when using software single step in GDBServer
    
    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.

Diff:
---
 gdb/gdbserver/ChangeLog   |  6 ++++++
 gdb/gdbserver/linux-low.c | 21 ++++++++++++++-------
 gdb/gdbserver/mem-break.c | 17 +++++++++++++++++
 gdb/gdbserver/mem-break.h |  4 ++++
 4 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 87ce9e7..f48d7f2 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,11 @@
 2015-11-30  Antoine Tremblay  <antoine.tremblay@ericsson.com>
 
+	* 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.
+
+2015-11-30  Antoine Tremblay  <antoine.tremblay@ericsson.com>
+
 	* linux-arm-low.c (arm_is_thumb_mode): New function.
 	(arm_breakpoint_at): Use arm_is_thumb_mode.
 	(arm_breakpoint_kind_from_current_state): New function.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 207a5ba..b29f54e 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -3071,14 +3071,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 11c21db..ea1140a 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -1662,6 +1662,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.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]