This is the mail archive of the 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]

Re: [PATCH v3 08/10] Support software single step on ARM in GDBServer.

On 11/26/2015 07:48 AM, Yao Qi wrote:
Antoine Tremblay <> writes:

Limitations :

GDBServer can't step over a sigreturn or rt_sigreturn syscall since this would
require the DWARF information to identify the caller PC. This situation
only prints a warning for the moment.

They are using frame not DWARF to identify the PC, however, it is not
necessary.  When GDB or GDBserver is doing software single step, it must
be in the inner-most frame, so we can get the caller PC by examining the
stack at a certain offset from SP (from regcache), like what are doing in
arm_linux_sigreturn_init and arm_linux_rt_sigreturn_init, rather than
replying frame unwinding.  See my comments to arm_get_next_pcs_syscall_next_pc.

Humm and frame can use the dwarf unwinder can it not ? frame is still very confusing to me.

But I'm glad it's not the only option, I'll examine the stack and fix it.

+/* Wrapper over syscall_next_pc for use in get_next_pcs.  */
+arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self, CORE_ADDR pc)
+  struct arm_gdb_get_next_pcs *next_pcs = (struct arm_gdb_get_next_pcs *) self;
+  struct gdbarch_tdep *tdep;
+  tdep = gdbarch_tdep (next_pcs->gdbarch);
+  if (tdep->syscall_next_pc != NULL)
+    return tdep->syscall_next_pc (next_pcs->frame);
+  return 0;

+/* When PC is at a syscall instruction, return the PC of the next
+   instruction to be executed.  */
+static CORE_ADDR
+get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self, CORE_ADDR pc)
+  CORE_ADDR return_addr = 0;
+  int is_thumb = self->is_thumb;
+  ULONGEST svc_number = 0;
+  if (is_thumb)
+    {
+      svc_number = self->ops->collect_register_unsigned (self, 7);
+      return_addr = pc + 2;
+    }
+  else
+    {
+      unsigned long this_instr = self->ops->read_memory_unsigned_integer
+	((CORE_ADDR) pc, 4, self->byte_order_for_code);
+      unsigned long svc_operand = (0x00ffffff & this_instr);
+      if (svc_operand)  /* OABI.  */
+	{
+	  svc_number = svc_operand - 0x900000;
+	}
+      else /* EABI.  */
+	{
+	  svc_number = self->ops->collect_register_unsigned (self, 7);
+	}
+      return_addr = pc + 4;
+    }
+  /* Is this a sigreturn or rt_sigreturn syscall?
+     If so it is currently not handeled.  */
+  if (svc_number == 119 || svc_number == 173)
+    {
+      if (debug_threads)
+	debug_printf ("Unhandled sigreturn or rt_sigreturn syscall\n");

We can still compute the caller pc by examining stack.

+    }
+  /* Addresses for calling Thumb functions have the bit 0 set.  */
+  if (is_thumb)
+    return_addr = MAKE_THUMB_ADDR (return_addr);
+  return return_addr;

This leads me thinking more about current software single step code in
GDB.  In GDB side, we are using frame to access registers, while in
GDBserver, we are using regcache.  Why don't we use regcache in both
sides? so that the "struct arm_get_next_pcs_ops" can be simplified, for
example, field collect_register_unsigned may be no longer necessary.

In fact, software_single_step used to have regcache argument, added by but we
changed to argument frame

This is the same link as the previous one...

 IMO, it is
better to use regcache than frame.  We have two options,

  #1, switch from frame apis to regcache apis to access registers in arm
   software single step.  We can get regcache by get_current_regcache ().
  #2, change argument of gdbarch method software_single_step from frame
   to regcache, which means all its implementations need update, and
   switch to regcache apis to access registers.

#2 is the right way to go in long term, and we really need to improve
software_single_step.  Let me what do you think.

Looking at the impacts of #2, I do not feel comfortable including these changes in this patch set. I feel they would require a patch set of their own.

However #1 seems like something possible I would start by this option if that's fine with you ?

Also, I can still do the refactoring before this patch but it will require more work since I'll have to diff the functions moved etc.. do you feel it's required to do so or the refactoring could be done after this patch ?

Thank you,

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