Re: [PATCH v4 4/6] Support software single step on ARM in GDBServer.

[Yao Qi <qiyaoltc at gmail dot com>]

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

Some comments on the design,

> +/* Context for a get_next_pcs call on ARM.  */
> +struct arm_get_next_pcs
> +{
> +  /* Operations implementations.  */
> +  struct arm_get_next_pcs_ops *ops;

> +  /* Byte order for data.  */
> +  int byte_order;
> +  /* Byte order for code.  */
> +  int byte_order_for_code;
> +  /* Is the pc in thumb mode.  */
> +  int is_thumb;
> +  /* Use 32bit or 26 bit pc.  */
> +  int arm_apcs_32;
> +  /* Thumb2 breakpoint instruction.  */
> +  const gdb_byte *arm_thumb2_breakpoint;

These fields are GDB specific,  GDBserver doesn't need them at all.
Can we move them to arm_gdb_get_next_pcs?  Field is_thumb is used in
both sides, but can't we compute it in two sides (through arm_is_thumb
and arm_is_thumb_mode) respectively, rather than having a field here?

> +  /* Registry cache.  */
> +  struct regcache *regcache;
> +};
> +

> +/* get_next_pcs operations.  */
> +struct arm_get_next_pcs_ops
> +{
> +  ULONGEST (*read_memory_unsigned_integer) (CORE_ADDR memaddr,
> +					    int len,
> +					    int byte_order);

We need argument struct arm_get_next_pcs *self, and get rid of argument
byte_order, which can be got through self.

> +  CORE_ADDR (*syscall_next_pc) (struct arm_get_next_pcs *self, CORE_ADDR pc);
> +  CORE_ADDR (*addr_bits_remove) (struct arm_get_next_pcs *self, CORE_ADDR val);
> +};

> +/* Context for a get_next_pcs call on ARM in GDB.  */
> +struct arm_gdb_get_next_pcs
> +{
> +  /* Common context for gdb/gdbserver.  */
> +  struct arm_get_next_pcs base;
> +  /* Frame information.  */
> +  struct frame_info *frame;

FRAME is still used in arm_get_next_pcs_syscall_next_pc, but we should
use regcache instead of frame there.  Then we can remove frame here.

> +  /* Architecture dependent information.  */
> +  struct gdbarch *gdbarch;

Is gdbarch used?

> +};


-- 
Yao (éå)