[PATCH v2 2/2] gdbserver: Add GNU/Linux support for ARC

[Simon Marchi]

Some minor comments, but otherwise this is fine.  Please merge after
going through the comments.

On 2020-10-06 12:42 p.m., Shahab Vahedi wrote:
> +/* Using a mere "uint16_t arc_linux_traps_s = TRAP_S_1_OPCODE" would
> +   work as well, because the endianness will end up correctly when
> +   the code is compiled for the same endianness as the target (see
> +   the notes for "low_breakpoint_at" in this file).  However, this
> +   illustrates how the __BIG_ENDIAN__ macro can be used to make
> +   easy-to-understand codes.  */

If you have a simpler way that  doesn't use __BIG_ENDIAN__, that's fine
too.  Don't feel forced to use it only because I suggested it.

> +bool
> +arc_target::low_cannot_store_register (int regno)
> +{
> +  return (regno >= current_process ()->tdesc->reg_defs.size ());
> +}
> +
> +/* This works for both endianness.  Below you see an illustration of how
> +   the "trap_s 1" instruction encoded for both endianness in the memory
> +   will end up as the TRAP_S_1_OPCODE constant:
> +
> +   BE: 0x78 0x3e --> at INSN addr: 0x78 0x3e --> INSN = 0x783e
> +   LE: 0x3e 0x78 --> at INSN addr: 0x3e 0x78 --> INSN = 0x783e
> +
> +   One can employ "memcmp()" for comparing the arrays too.
> +   A big shout-out to Simon Marchi (simark) for clearing this up.

Heh, please don't mention this :).

> +   */
> +
> +bool
> +arc_target::low_breakpoint_at (CORE_ADDR where)
> +{
> +  uint16_t insn;
> +
> +  /* Since "this" is "the_target", no point in using the latter.  */

This is not a very useful comment IMO.  "this" is the "current"
arc_target object, which is obvious to anybody familiar with C++.

> +  this->read_memory (where, (gdb_byte *) &insn, TRAP_S_1_SIZE);
> +  return (insn == TRAP_S_1_OPCODE);
> +}
> +
> +/* PTRACE_GETREGSET/NT_PRSTATUS and PTRACE_SETREGSET/NT_PRSTATUS work with
> +   regsets in a struct, "user_regs_struct", defined in the
> +   linux/arch/arc/include/uapi/asm/ptrace.h header.  This code supports
> +   ARC Linux ABI v3 and v4.  */
> +
> +/* Populate a ptrace NT_PRSTATUS regset from a regcache.
> +
> +   This appears to be a unique approach to populating the buffer, but
> +   being name, rather than offset based, it is robust to future API
> +   changes, as there is no need to create a regmap of registers in the
> +   user_regs_struct.  */
> +
> +static void
> +arc_fill_gregset (struct regcache *regcache, void *buf)
> +{
> +  struct user_regs_struct *regbuf = (struct user_regs_struct *) buf;
> +
> +  /* Core registers.  */
> +  collect_register_by_name (regcache, "r0", &(regbuf->scratch.r0));
> +  collect_register_by_name (regcache, "r1", &(regbuf->scratch.r1));
> +  collect_register_by_name (regcache, "r2", &(regbuf->scratch.r2));
> +  collect_register_by_name (regcache, "r3", &(regbuf->scratch.r3));
> +  collect_register_by_name (regcache, "r4", &(regbuf->scratch.r4));
> +  collect_register_by_name (regcache, "r5", &(regbuf->scratch.r5));
> +  collect_register_by_name (regcache, "r6", &(regbuf->scratch.r6));
> +  collect_register_by_name (regcache, "r7", &(regbuf->scratch.r7));
> +  collect_register_by_name (regcache, "r8", &(regbuf->scratch.r8));
> +  collect_register_by_name (regcache, "r9", &(regbuf->scratch.r9));
> +  collect_register_by_name (regcache, "r10", &(regbuf->scratch.r10));
> +  collect_register_by_name (regcache, "r11", &(regbuf->scratch.r11));
> +  collect_register_by_name (regcache, "r12", &(regbuf->scratch.r12));
> +  collect_register_by_name (regcache, "r13", &(regbuf->callee.r13));
> +  collect_register_by_name (regcache, "r14", &(regbuf->callee.r14));
> +  collect_register_by_name (regcache, "r15", &(regbuf->callee.r15));
> +  collect_register_by_name (regcache, "r16", &(regbuf->callee.r16));
> +  collect_register_by_name (regcache, "r17", &(regbuf->callee.r17));
> +  collect_register_by_name (regcache, "r18", &(regbuf->callee.r18));
> +  collect_register_by_name (regcache, "r19", &(regbuf->callee.r19));
> +  collect_register_by_name (regcache, "r20", &(regbuf->callee.r20));
> +  collect_register_by_name (regcache, "r21", &(regbuf->callee.r21));
> +  collect_register_by_name (regcache, "r22", &(regbuf->callee.r22));
> +  collect_register_by_name (regcache, "r23", &(regbuf->callee.r23));
> +  collect_register_by_name (regcache, "r24", &(regbuf->callee.r24));
> +  collect_register_by_name (regcache, "r25", &(regbuf->callee.r25));
> +  collect_register_by_name (regcache, "gp", &(regbuf->scratch.gp));
> +  collect_register_by_name (regcache, "fp", &(regbuf->scratch.fp));
> +  collect_register_by_name (regcache, "sp", &(regbuf->scratch.sp));
> +  collect_register_by_name (regcache, "blink", &(regbuf->scratch.blink));
> +
> +  /* Loop registers.  */
> +  collect_register_by_name (regcache, "lp_count", &(regbuf->scratch.lp_count));
> +  collect_register_by_name (regcache, "lp_start", &(regbuf->scratch.lp_start));
> +  collect_register_by_name (regcache, "lp_end", &(regbuf->scratch.lp_end));
> +
> +  /* The current "pc" value must be written to "eret" (exception return
> +     address) register, because that is the address that the kernel code
> +     will jump back to after a breakpoint exception has been raised.
> +     The "pc_stop" value is ignored by the genregs_set() in
> +     linux/arch/arc/kernel/ptrace.c.  */
> +  collect_register_by_name (regcache, "pc", &(regbuf->scratch.ret));
> +
> +  /* Currently ARC Linux ptrace doesn't allow writes to status32 because
> +     some of its bits are kernel mode-only and shoudn't be writable from
> +     user-space.  Writing status32 from debugger could be useful, though,
> +     so ability to write non-priviliged bits will be added to kernel
> +     sooner or later.  */
> +
> +  /* BTA.  */
> +  collect_register_by_name (regcache, "bta", &(regbuf->scratch.bta));
> +}
> +
> +/* Populate a regcache from a ptrace NT_PRSTATUS regset.  */
> +
> +static void
> +arc_store_gregset (struct regcache *regcache, const void *buf)
> +{
> +  const struct user_regs_struct *regbuf = (const struct user_regs_struct *) buf;
> +
> +  /* Core registers.  */
> +  supply_register_by_name (regcache, "r0", &(regbuf->scratch.r0));
> +  supply_register_by_name (regcache, "r1", &(regbuf->scratch.r1));
> +  supply_register_by_name (regcache, "r2", &(regbuf->scratch.r2));
> +  supply_register_by_name (regcache, "r3", &(regbuf->scratch.r3));
> +  supply_register_by_name (regcache, "r4", &(regbuf->scratch.r4));
> +  supply_register_by_name (regcache, "r5", &(regbuf->scratch.r5));
> +  supply_register_by_name (regcache, "r6", &(regbuf->scratch.r6));
> +  supply_register_by_name (regcache, "r7", &(regbuf->scratch.r7));
> +  supply_register_by_name (regcache, "r8", &(regbuf->scratch.r8));
> +  supply_register_by_name (regcache, "r9", &(regbuf->scratch.r9));
> +  supply_register_by_name (regcache, "r10", &(regbuf->scratch.r10));
> +  supply_register_by_name (regcache, "r11", &(regbuf->scratch.r11));
> +  supply_register_by_name (regcache, "r12", &(regbuf->scratch.r12));
> +  supply_register_by_name (regcache, "r13", &(regbuf->callee.r13));
> +  supply_register_by_name (regcache, "r14", &(regbuf->callee.r14));
> +  supply_register_by_name (regcache, "r15", &(regbuf->callee.r15));
> +  supply_register_by_name (regcache, "r16", &(regbuf->callee.r16));
> +  supply_register_by_name (regcache, "r17", &(regbuf->callee.r17));
> +  supply_register_by_name (regcache, "r18", &(regbuf->callee.r18));
> +  supply_register_by_name (regcache, "r19", &(regbuf->callee.r19));
> +  supply_register_by_name (regcache, "r20", &(regbuf->callee.r20));
> +  supply_register_by_name (regcache, "r21", &(regbuf->callee.r21));
> +  supply_register_by_name (regcache, "r22", &(regbuf->callee.r22));
> +  supply_register_by_name (regcache, "r23", &(regbuf->callee.r23));
> +  supply_register_by_name (regcache, "r24", &(regbuf->callee.r24));
> +  supply_register_by_name (regcache, "r25", &(regbuf->callee.r25));
> +  supply_register_by_name (regcache, "gp", &(regbuf->scratch.gp));
> +  supply_register_by_name (regcache, "fp", &(regbuf->scratch.fp));
> +  supply_register_by_name (regcache, "sp", &(regbuf->scratch.sp));
> +  supply_register_by_name (regcache, "blink", &(regbuf->scratch.blink));
> +
> +  /* Loop registers.  */
> +  supply_register_by_name (regcache, "lp_count", &(regbuf->scratch.lp_count));
> +  supply_register_by_name (regcache, "lp_start", &(regbuf->scratch.lp_start));
> +  supply_register_by_name (regcache, "lp_end", &(regbuf->scratch.lp_end));
> +
> +  /* The genregs_get() in linux/arch/arc/kernel/ptrace.c populates the
> +     pseudo register "stop_pc" with the "efa" (exception fault address)
> +     register.  This was deemed necessary, because the breakpoint
> +     instruction, "trap_s 1", is a committing one; i.e. the "eret"
> +     (excpetion return address) register will be pointing to the next

excpetion -> exception

Simon