Re: [PATCH v4 3/5] Support for recording syscall on aarch64-linux

[Yao Qi <qiyaoltc at gmail dot com>]

Omair Javaid <omair.javaid@linaro.org> writes:

> 	* aarch64-linux-tdep.c (linux-record): Include.

s/linux-record/linux-record.h/

> 	(record-full.h): Include.
> 	(struct linux_record_tdep aarch64_linux_record_tdep): Declare.
> 	(aarch64_canonicalize_syscall): New function to translate syscall
> 	numbers from aarch64 to canonical.

"New function" only should be fine.

> 	(aarch64_all_but_pc_registers_record): New function.
> 	(aarch64_linux_syscall_record): New function.
> 	(aarch64_linux_init_abi): Update to handle syscall recording.
> 	* aarch64-linux-tdep.h (aarch64_syscall): New enum.
> 	* aarch64-tdep.c (aarch64_record_branch_except_sys): Add code to
> 	handle recording of syscalls.
> 	* aarch64-tdep.h
> 	(struct gdbarch_tdep) <aarch64_syscall_record>: Defined.
> 	* linux-record.h (struct linux_record_tdep): Add two more syscall
> 	argument fields.

	* linux-record.h (struct linux_record_tdep) <arg7, arg8>: New fields.

> +/* aarch64_canonicalize_syscall maps syscall ids from the native AArch64
> +   linux set of syscall ids into a canonical set of syscall ids used by
> +   process record.  */
> +
> +static enum gdb_syscall
> +aarch64_canonicalize_syscall (enum aarch64_syscall syscall_number)
> +{
> +  switch (syscall_number) {
> +  case aarch64_sys_read:
> +    return gdb_sys_read;
> +

Can we add a macro which does such replacement,

   SYSCALL_MAP (read) -> case aarch64_sys_read: return gdb_sys_read;

so that this function should be shorter.

> +
> +  case aarch64_sys_mmap:
> +    return gdb_sys_mmap2;
> +
> +  default:
> +    return -1;
> +  }
> +}
> +
> +/* Record all registers but PC register for process-record.  */
> +
> +static int
> +aarch64_all_but_pc_registers_record (struct regcache *regcache)
> +{
> +  int i;
> +
> +  for (i = 0; i < AARCH64_PC_REGNUM; i++)
> +    if (record_full_arch_list_add_reg (regcache, AARCH64_X0_REGNUM + i))
> +      return -1;

Nit, better that "i" starts from AARCH64_X0_REGNUM, like,

  for (i = AARCH64_X0_REGNUM; i < AARCH64_PC_REGNUM; i++)
     if (record_full_arch_list_add_reg (regcache, i))
        return -1;
> +
> +  /* The AArch64 syscall calling convention: reg x0-x7 for arguments,
> +     reg x8 for syscall number and return value in reg x0.  */
> +  aarch64_linux_record_tdep.arg1 = AARCH64_X0_REGNUM + 0;
> +  aarch64_linux_record_tdep.arg2 = AARCH64_X0_REGNUM + 1;
> +  aarch64_linux_record_tdep.arg3 = AARCH64_X0_REGNUM + 2;
> +  aarch64_linux_record_tdep.arg4 = AARCH64_X0_REGNUM + 3;
> +  aarch64_linux_record_tdep.arg5 = AARCH64_X0_REGNUM + 4;
> +  aarch64_linux_record_tdep.arg6 = AARCH64_X0_REGNUM + 5;
> +  aarch64_linux_record_tdep.arg7 = AARCH64_X0_REGNUM + 6;
> +  aarch64_linux_record_tdep.arg8 = AARCH64_X0_REGNUM + 7;

x7 is not used for arguments in linux syscall.  At least, that is what I
am told from glibc source sysdeps/unix/sysv/linux/aarch64/sysdep.h:

/* Linux takes system call args in registers:
        syscall number  x8
        arg 1           x0
        arg 2           x1
        arg 3           x2
        arg 4           x3
        arg 5           x4
        arg 6           x5
        arg 7           x6

>  }
>  
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
> diff --git a/gdb/aarch64-linux-tdep.h b/gdb/aarch64-linux-tdep.h
> index 9d09ae6..4475f2e 100644
> --- a/gdb/aarch64-linux-tdep.h
> +++ b/gdb/aarch64-linux-tdep.h
> @@ -32,3 +32,269 @@
>  
>  extern const struct regset aarch64_linux_gregset;
>  extern const struct regset aarch64_linux_fpregset;
> +
> +/* Enum that defines the AArch64 linux specific syscall identifiers used for
> +   process record/replay.  */
> +
> +enum aarch64_syscall {
....
> +};

Why don't define this enum in aarch64-linux-tdep.c?

-- 
Yao (éå)