diff mbox

[v8,5/7] Support software single step on ARM in GDBServer

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

Commit Message

Yao Qi Jan. 13, 2016, 4:13 p.m. UTC 
  Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

> +
> +/* Get the raw next possible addresses.  PC in next_pcs is the current program
> +   counter, which is assumed to be executing in ARM mode.
> +
> +   The values returned have the execution state of the next instruction
> +   encoded in it.  Use IS_THUMB_ADDR () to see whether the instruction is
> +   in Thumb-State, and gdbarch_addr_bits_remove () to get the plain memory
> +   address in GDB and arm_addr_bits_remove in GDBServer.  */
> +
> +VEC (CORE_ADDR) *
> +arm_get_next_pcs_raw (struct arm_get_next_pcs *self,
> +		      CORE_ADDR pc)
> +{
> +  int byte_order = self->byte_order;
> +  unsigned long pc_val;
> +  unsigned long this_instr = 0;
> +  unsigned long status;
> +  CORE_ADDR nextpc;
> +  struct regcache *regcache = self->regcache;
> +  VEC (CORE_ADDR) *next_pcs = NULL;
> +
> +  pc_val = (unsigned long) pc;
> +  this_instr = self->ops->read_mem_uint (pc, 4, byte_order);
                                                   ^^^^^^^^^^

> -
> -/* Get the raw next address.  PC is the current program counter, in 
> -   FRAME, which is assumed to be executing in ARM mode.
> -
> -   The value returned has the execution state of the next instruction 
> -   encoded in it.  Use IS_THUMB_ADDR () to see whether the instruction is
> -   in Thumb-State, and gdbarch_addr_bits_remove () to get the plain memory
> -   address.  */
> -
> -static CORE_ADDR
> -arm_get_next_pc_raw (struct regcache *regcache, CORE_ADDR pc)
> -{
> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> -  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
> -  unsigned long pc_val;
> -  unsigned long this_instr;
> -  unsigned long status;
> -  CORE_ADDR nextpc;
> -
> -  pc_val = (unsigned long) pc;
> -  this_instr = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
                                                       ^^^^^^^^^^^^^^^^^^^

The code change above introduces a bug as I see.  The original code uses
byte_order_for_code which is right to me, but it becomes byte_order
after the change.  Patch below fixes it.

Comments

Antoine Tremblay Jan. 13, 2016, 7:10 p.m. UTC | #1 
On 01/13/2016 11:13 AM, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>> +
>> +/* Get the raw next possible addresses.  PC in next_pcs is the current program
>> +   counter, which is assumed to be executing in ARM mode.
>> +
>> +   The values returned have the execution state of the next instruction
>> +   encoded in it.  Use IS_THUMB_ADDR () to see whether the instruction is
>> +   in Thumb-State, and gdbarch_addr_bits_remove () to get the plain memory
>> +   address in GDB and arm_addr_bits_remove in GDBServer.  */
>> +
>> +VEC (CORE_ADDR) *
>> +arm_get_next_pcs_raw (struct arm_get_next_pcs *self,
>> +		      CORE_ADDR pc)
>> +{
>> +  int byte_order = self->byte_order;
>> +  unsigned long pc_val;
>> +  unsigned long this_instr = 0;
>> +  unsigned long status;
>> +  CORE_ADDR nextpc;
>> +  struct regcache *regcache = self->regcache;
>> +  VEC (CORE_ADDR) *next_pcs = NULL;
>> +
>> +  pc_val = (unsigned long) pc;
>> +  this_instr = self->ops->read_mem_uint (pc, 4, byte_order);
>                                                     ^^^^^^^^^^
>
>> -
>> -/* Get the raw next address.  PC is the current program counter, in
>> -   FRAME, which is assumed to be executing in ARM mode.
>> -
>> -   The value returned has the execution state of the next instruction
>> -   encoded in it.  Use IS_THUMB_ADDR () to see whether the instruction is
>> -   in Thumb-State, and gdbarch_addr_bits_remove () to get the plain memory
>> -   address.  */
>> -
>> -static CORE_ADDR
>> -arm_get_next_pc_raw (struct regcache *regcache, CORE_ADDR pc)
>> -{
>> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
>> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> -  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
>> -  unsigned long pc_val;
>> -  unsigned long this_instr;
>> -  unsigned long status;
>> -  CORE_ADDR nextpc;
>> -
>> -  pc_val = (unsigned long) pc;
>> -  this_instr = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
>                                                         ^^^^^^^^^^^^^^^^^^^
>
> The code change above introduces a bug as I see.  The original code uses
> byte_order_for_code which is right to me, but it becomes byte_order
> after the change.  Patch below fixes it.
>

Thank you!

Antoine
diff mbox

Patch

diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
index f9c2592..a30916a 100644
--- a/gdb/arch/arm-get-next-pcs.c
+++ b/gdb/arch/arm-get-next-pcs.c
@@ -644,6 +644,7 @@  static VEC (CORE_ADDR) *
 arm_get_next_pcs_raw (struct arm_get_next_pcs *self)
 {
   int byte_order = self->byte_order;
+  int byte_order_for_code = self->byte_order_for_code;
   unsigned long pc_val;
   unsigned long this_instr = 0;
   unsigned long status;
@@ -653,7 +654,7 @@  arm_get_next_pcs_raw (struct arm_get_next_pcs *self)
   VEC (CORE_ADDR) *next_pcs = NULL;
 
   pc_val = (unsigned long) pc;
-  this_instr = self->ops->read_mem_uint (pc, 4, byte_order);
+  this_instr = self->ops->read_mem_uint (pc, 4, byte_order_for_code);
 
   status = regcache_raw_get_unsigned (regcache, ARM_PS_REGNUM);
   nextpc = (CORE_ADDR) (pc_val + 4);	/* Default case */