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

Yao Qi
2015-12-03T11:17:20Z

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 (齐尧)