Re: [PATCH v8 5/7] Support software single step on ARM in GDBServer

Antoine Tremblay
2016-01-13T19:10:22Z

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