[1/2] This patch fixes GDBServer's run control for single stepping

Message ID 86d1cy4umo.fsf@gmail.com
State New, archived
Headers

Commit Message

Yao Qi March 30, 2017, 4:06 p.m. UTC
  Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

>> In software single step, we calculate the next pcs, and select
>> breakpoint kinds of them, according to current pc.  If current
>> pc is not within IT block (!InITBlock ()) or the last instruction
>> in IT block (LastInITBlock ()), we can safely use 16-bit thumb
>> breakpoint for any thumb instruction.  That is, in
>> gdbserver/linux-aarch32-low.c:arm_breakpoint_kind_from_current_state,
>> we can return ARM_BP_KIND_THUMB if (!InITBlock () || LastInITBlock ()).
>
> This is not entirely true since we have to check if the next pcs are in
> an IT block rather then only the current one, so there's multiple
> scenarios.

In fact, we need to know whether the next pcs will be conditionally
executed or not, right?  If they won't be conditionally executed, we can
use 16-bit thumb breakpoint instruction.  By checking CPSR, if
current PC is not in IT block or on the last instruction in IT block,
the next pcs can't be conditionally executed.  I've already had a patch
below to implement what I described.

The problem of this patch is that we end up inserting different
kinds of breakpoints on the same instruction.  For a given 32-bit thumb
instruction, GDB and GDBserver knows 32-bit thumb breakpoint instruction
is used for GDB breakpoint, but only GDBserver knows 16-bit thumb
breakpoint is used for GDBserver single-step breakpoint, so GDB will be
confused on this.  I stopped here, and start to do something else.

>
> Consider if current PC is the IT instruction for example, then there's
> at least 2 next pcs inside the IT block where we will need to install an THUMB2
> breakpoint and get_next_pcs will return that.
  

Patch

diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c
index 2b710ba..7409050 100644
--- a/gdb/gdbserver/linux-aarch32-low.c
+++ b/gdb/gdbserver/linux-aarch32-low.c
@@ -288,7 +288,30 @@  arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr)
   if (arm_is_thumb_mode ())
     {
       *pcptr = MAKE_THUMB_ADDR (*pcptr);
-      return arm_breakpoint_kind_from_pc (pcptr);
+      int kind = arm_breakpoint_kind_from_pc (pcptr);
+
+      if (kind == ARM_BP_KIND_THUMB2)
+	{
+	    unsigned long cpsr;
+	    struct regcache *regcache
+	      = get_thread_regcache (current_thread, 1);
+
+	    collect_register_by_name (regcache, "cpsr", &cpsr);
+	    /* Only use 32-bit thumb-2 breakpoint if *PCPTR is within
+	       IT block, because it takes two PTRACE_POKETEXT calls to
+	       write 32-bit thumb-2 breakpoint to a 2-byte aligned
+	       address, and other threads can observe the partially
+	       modified instruction in the memory between two
+	       PTRACE_POKETEXT calls.
+
+	       Don't use 32-bit thumb-2 breakpoint if program is not
+	       in IT block or on the last instruction of IT block,
+	       (ITSTATE.IT<2:0> == 000).  These bits are from CPSR bit
+	       10, 25, and 26.  */
+	    if ((cpsr & 0x06000400) == 0)
+	      kind = ARM_BP_KIND_THUMB;
+	}
+      return kind;
     }
   else
     {
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 6e6926a..f3845cf 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -855,7 +855,21 @@  set_breakpoint_type_at (enum bkpt_type type, CORE_ADDR where,
 {
   int err_ignored;
   CORE_ADDR placed_address = where;
-  int breakpoint_kind = target_breakpoint_kind_from_pc (&placed_address);
+  int breakpoint_kind;
+
+  /* Get the kind of breakpoint to PLACED_ADDRESS except single-step
+     breakpoint.  Get the kind of single-step breakpoint according to
+     the current register state.  */
+  if (type == single_step_breakpoint)
+    {
+      breakpoint_kind
+	= target_breakpoint_kind_from_current_state (&placed_address);
+    }
+  else
+    {
+      breakpoint_kind
+	= target_breakpoint_kind_from_pc (&placed_address);
+    }
 
   return set_breakpoint (type, raw_bkpt_type_sw,
 			 placed_address, breakpoint_kind, handler,