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

[Yao Qi <qiyaoltc at gmail dot com>]

Antoine Tremblay <antoine.tremblay@ericsson.com> 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.

> +
> +/* Wrapper over syscall_next_pc for use in get_next_pcs.  */
> +
> +CORE_ADDR
> +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
https://sourceware.org/ml/gdb-patches/2007-04/msg00218.html but we
changed to argument frame
https://sourceware.org/ml/gdb-patches/2007-04/msg00218.html  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.

-- 
Yao (éå)