[v2] gdb/riscv: Add record support for rv64gc instructions

  Hello!

Thanks for review and sorry for such late answer.

> ---
>   gdb/configure.tgt                             |   4 +-
>   gdb/riscv-canonicalize-syscall.c              | 555 ++++++++++++++
>   gdb/riscv-linux-tdep.c                        | 261 +++++++
>   gdb/riscv-linux-tdep.h                        |  29 +
>   gdb/riscv-tdep.c                              | 703 +++++++++++++++++-
>   gdb/riscv-tdep.h                              |  15 +
>   .../riscv-canonicalize-syscall-gen.py         | 126 ++++
>   gdb/testsuite/lib/gdb.exp                     |   3 +-
>   8 files changed, 1688 insertions(+), 8 deletions(-)
>   create mode 100644 gdb/riscv-canonicalize-syscall.c
>   create mode 100644 gdb/riscv-linux-tdep.h
>   create mode 100644 gdb/syscalls/riscv-canonicalize-syscall-gen.py
>
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 62df71b13fa..f965e03e84d 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -545,8 +545,8 @@ riscv*-*-freebsd*)
>     riscv*-*-linux*)
>         # Target: Linux/RISC-V
> -       gdb_target_obs="riscv-linux-tdep.o glibc-tdep.o \
> -                       linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
> +       gdb_target_obs="riscv-linux-tdep.o riscv-canonicalize-syscall.o \
> +       glibc-tdep.o linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
> Please keep the indentation as it was.

Addressed

>         ;;
>     riscv*-*-*)
> diff --git a/gdb/riscv-canonicalize-syscall.c b/gdb/riscv-canonicalize-syscall.c
> new file mode 100644
> index 00000000000..779bbd9335c
> --- /dev/null
> +++ b/gdb/riscv-canonicalize-syscall.c
> Generated files should end with the -gen suffix, so this should be
> riscv-canonicalize-syscall-gen.c if the final version of this change
> continues using the python script.

As for now I decided to leave it, changed it with suffix '-gen'

> @@ -0,0 +1,555 @@
> +/* DO NOT EDIT: Autogenerated by riscv-canonicalize-syscall-gen.py  */
> Even for generated files, the copyright blurb should be added here.

Addressed

> +
> +#include "defs.h"
> +#include "riscv-linux-tdep.h"
> +
> +enum gdb_syscall
> +riscv64_canonicalize_syscall (int syscall)
> +{
> +  switch (syscall)
> +    {
> +    case 0: // #define __NR_io_setup 0
>
> All comments should be in the form /* */, rather than //
>
> I would also prefer if this followed what other architectures do,
> defining an enum to relate syscall numbers in a RISCV system to their
> names, and then this function returning the gdb_sys enum numbers,
> however I don't expect to be working on RISCV in the near future, so
> feel free to ignore this comment if the other RISCV maintainers are ok
> with this style. This would make it so no #define comment is necessary

Decided to make a script generating function that canonicalizes RISC-V syscall.
This solution will simplify adding record support for rv32 system. I also left
place for corner cases in generating script as they always may exist. Also
removed this #define comment from generating script.

> +    case 35: // #define __NR_unlinkat 35
> +      return gdb_sys_unlinkat;
> +    case 36: // #define __NR_symlinkat 36
> +      return gdb_sys_symlinkat;
> +    case 37: // #define __NR_linkat 37
> +      return gdb_sys_linkat;
> +    // case 39: return gdb_sys_umount2;
>
> I don't think these provide much help, since syscall numbers are os+cpu
> specific, and is the whole reason why we have this function canonicalizing
> the syscall numbers.  Again, take this as only a suggestion, as I am not
> familiar with riscv > code.

Deleted unnecessary comments.

>     /* Initialize RISC-V Linux target support.  */
> diff --git a/gdb/riscv-linux-tdep.h b/gdb/riscv-linux-tdep.h
> new file mode 100644
> index 00000000000..5ee2c94387f
> --- /dev/null
> +++ b/gdb/riscv-linux-tdep.h
> @@ -0,0 +1,29 @@
> +/* Copyright (C) 2024 Free Software Foundation, Inc.
> +   Contributed by Timur Golubovich
> I think we don't use these "contributed by" lines anymore.

Addressed

> +           || is_lr_w_insn (ival) || is_lr_d_insn (ival)
> +           || is_fcvt_l_s_insn (ival) || is_fcvt_lu_s_insn (ival)
> +           || is_feq_d_insn (ival) || is_flt_d_insn (ival)
> +           || is_fle_d_insn (ival) || is_fclass_d_insn (ival)
> +           || is_fcvt_w_d_insn (ival) || is_fcvt_wu_d_insn (ival)
> +           || is_fcvt_d_w_insn (ival) || is_fcvt_l_d_insn (ival)
> +           || is_fcvt_lu_d_insn (ival) || is_fmv_x_d_insn (ival);
> +  }
> +
> +  /* Returns true if instruction is classified. Needs error checking after */
> Comments should end in a period and 2 spaces.

Addressed

> +
> +  bool
> +  record (gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR addr) noexcept
> +  {
> +    int m_length = 0;
> +    auto ival = riscv_insn::fetch_instruction (gdbarch, addr, &m_length);
> +    if (!save_reg (RISCV_PC_REGNUM))
> +      return false;
> +
> +    if (m_length == 4)
> +      return record_insn_len4 (ival, regcache);
> +
> +    if (m_length == 2)
> +      return record_insn_len2 (ival, regcache, gdbarch);
> +
> +    /* 6 bytes or more.  If the instruction is longer than 8 bytes, we don't
> +have full instruction bits in ival.  At least, such long instructions
> +are not defined yet, so just ignore it.  */
> Again, the indentation here is incorrect. Lines should start in the same column as the number 6.

Addressed

Best wishes,
Timur Golubovich

Timur Golubovich timurgol007@gmail.com
---
 gdb/configure.tgt                             |   4 +-
 gdb/riscv-linux-tdep.c                        | 252 +++++++
 gdb/riscv-linux-tdep.h                        |  28 +
 gdb/riscv-tdep.c                              | 703 +++++++++++++++++-
 gdb/riscv-tdep.h                              |  15 +
 .../riscv-canonicalize-syscall-gen.py         | 143 ++++
 gdb/testsuite/lib/gdb.exp                     |   3 +-
 7 files changed, 1140 insertions(+), 8 deletions(-)
 create mode 100644 gdb/riscv-linux-tdep.h
 create mode 100644 gdb/syscalls/riscv-canonicalize-syscall-gen.py

--
2.34.1
  

On 11/26/24 11:17 AM, Timur wrote:
> Hello!
>
> Thanks for review and sorry for such late answer.
>
>> ---
>>    gdb/configure.tgt                             |   4 +-
>>    gdb/riscv-canonicalize-syscall.c              | 555 ++++++++++++++
>>    gdb/riscv-linux-tdep.c                        | 261 +++++++
>>    gdb/riscv-linux-tdep.h                        |  29 +
>>    gdb/riscv-tdep.c                              | 703 +++++++++++++++++-
>>    gdb/riscv-tdep.h                              |  15 +
>>    .../riscv-canonicalize-syscall-gen.py         | 126 ++++
>>    gdb/testsuite/lib/gdb.exp                     |   3 +-
>>    8 files changed, 1688 insertions(+), 8 deletions(-)
>>    create mode 100644 gdb/riscv-canonicalize-syscall.c
>>    create mode 100644 gdb/riscv-linux-tdep.h
>>    create mode 100644 gdb/syscalls/riscv-canonicalize-syscall-gen.py
>>
>> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
>> index 62df71b13fa..f965e03e84d 100644
>> --- a/gdb/configure.tgt
>> +++ b/gdb/configure.tgt
>> @@ -545,8 +545,8 @@ riscv*-*-freebsd*)
>>      riscv*-*-linux*)
>>          # Target: Linux/RISC-V
>> -       gdb_target_obs="riscv-linux-tdep.o glibc-tdep.o \
>> -                       linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
>> +       gdb_target_obs="riscv-linux-tdep.o riscv-canonicalize-syscall.o \
>> +       glibc-tdep.o linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
>> Please keep the indentation as it was.
> Addressed
>
>>          ;;
>>      riscv*-*-*)
>> diff --git a/gdb/riscv-canonicalize-syscall.c b/gdb/riscv-canonicalize-syscall.c
>> new file mode 100644
>> index 00000000000..779bbd9335c
>> --- /dev/null
>> +++ b/gdb/riscv-canonicalize-syscall.c
>> Generated files should end with the -gen suffix, so this should be
>> riscv-canonicalize-syscall-gen.c if the final version of this change
>> continues using the python script.
> As for now I decided to leave it, changed it with suffix '-gen'
>
>> @@ -0,0 +1,555 @@
>> +/* DO NOT EDIT: Autogenerated by riscv-canonicalize-syscall-gen.py  */
>> Even for generated files, the copyright blurb should be added here.
> Addressed
>
>> +
>> +#include "defs.h"
>> +#include "riscv-linux-tdep.h"
>> +
>> +enum gdb_syscall
>> +riscv64_canonicalize_syscall (int syscall)
>> +{
>> +  switch (syscall)
>> +    {
>> +    case 0: // #define __NR_io_setup 0
>>
>> All comments should be in the form /* */, rather than //
>>
>> I would also prefer if this followed what other architectures do,
>> defining an enum to relate syscall numbers in a RISCV system to their
>> names, and then this function returning the gdb_sys enum numbers,
>> however I don't expect to be working on RISCV in the near future, so
>> feel free to ignore this comment if the other RISCV maintainers are ok
>> with this style. This would make it so no #define comment is necessary
> Decided to make a script generating function that canonicalizes RISC-V syscall.
> This solution will simplify adding record support for rv32 system. I also left
> place for corner cases in generating script as they always may exist. Also
> removed this #define comment from generating script.

I see. This makes sense.

I would like to see this explanation on the commit message, so that it 
is recorded why Risc-V works different to all other arches.

>
>> +    case 35: // #define __NR_unlinkat 35
>> +      return gdb_sys_unlinkat;
>> +    case 36: // #define __NR_symlinkat 36
>> +      return gdb_sys_symlinkat;
>> +    case 37: // #define __NR_linkat 37
>> +      return gdb_sys_linkat;
>> +    // case 39: return gdb_sys_umount2;
>>
>> I don't think these provide much help, since syscall numbers are os+cpu
>> specific, and is the whole reason why we have this function canonicalizing
>> the syscall numbers.  Again, take this as only a suggestion, as I am not
>> familiar with riscv > code.
> Deleted unnecessary comments.
>
>>      /* Initialize RISC-V Linux target support.  */
>> diff --git a/gdb/riscv-linux-tdep.h b/gdb/riscv-linux-tdep.h
>> new file mode 100644
>> index 00000000000..5ee2c94387f
>> --- /dev/null
>> +++ b/gdb/riscv-linux-tdep.h
>> @@ -0,0 +1,29 @@
>> +/* Copyright (C) 2024 Free Software Foundation, Inc.
>> +   Contributed by Timur Golubovich
>> I think we don't use these "contributed by" lines anymore.
> Addressed
>
>> +           || is_lr_w_insn (ival) || is_lr_d_insn (ival)
>> +           || is_fcvt_l_s_insn (ival) || is_fcvt_lu_s_insn (ival)
>> +           || is_feq_d_insn (ival) || is_flt_d_insn (ival)
>> +           || is_fle_d_insn (ival) || is_fclass_d_insn (ival)
>> +           || is_fcvt_w_d_insn (ival) || is_fcvt_wu_d_insn (ival)
>> +           || is_fcvt_d_w_insn (ival) || is_fcvt_l_d_insn (ival)
>> +           || is_fcvt_lu_d_insn (ival) || is_fmv_x_d_insn (ival);
>> +  }
>> +
>> +  /* Returns true if instruction is classified. Needs error checking after */
>> Comments should end in a period and 2 spaces.
> Addressed
>
>> +
>> +  bool
>> +  record (gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR addr) noexcept
>> +  {
>> +    int m_length = 0;
>> +    auto ival = riscv_insn::fetch_instruction (gdbarch, addr, &m_length);
>> +    if (!save_reg (RISCV_PC_REGNUM))
>> +      return false;
>> +
>> +    if (m_length == 4)
>> +      return record_insn_len4 (ival, regcache);
>> +
>> +    if (m_length == 2)
>> +      return record_insn_len2 (ival, regcache, gdbarch);
>> +
>> +    /* 6 bytes or more.  If the instruction is longer than 8 bytes, we don't
>> +have full instruction bits in ival.  At least, such long instructions
>> +are not defined yet, so just ignore it.  */
>> Again, the indentation here is incorrect. Lines should start in the same column as the number 6.
> Addressed
>
> Best wishes,
> Timur Golubovich
>
> Timur Golubovich timurgol007@gmail.com
As I mentioned in the first review, please add some commit message 
explaining a bit of what your patch is doing. I would recommend 
explaining the inspiration for how riscv-tdep handles the recording (I 
guess aarch64) and why you changed it; plus explaining the idea behind 
the python script
> ---
>   gdb/configure.tgt                             |   4 +-
>   gdb/riscv-linux-tdep.c                        | 252 +++++++
>   gdb/riscv-linux-tdep.h                        |  28 +
>   gdb/riscv-tdep.c                              | 703 +++++++++++++++++-
>   gdb/riscv-tdep.h                              |  15 +
>   .../riscv-canonicalize-syscall-gen.py         | 143 ++++
>   gdb/testsuite/lib/gdb.exp                     |   3 +-
>   7 files changed, 1140 insertions(+), 8 deletions(-)
>   create mode 100644 gdb/riscv-linux-tdep.h
>   create mode 100644 gdb/syscalls/riscv-canonicalize-syscall-gen.py
>
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 62df71b13fa..b350e1f6505 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -545,8 +545,8 @@ riscv*-*-freebsd*)
>
>   riscv*-*-linux*)
>   	# Target: Linux/RISC-V
> -	gdb_target_obs="riscv-linux-tdep.o glibc-tdep.o \
> - 			linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
> +	gdb_target_obs="riscv-linux-tdep.o riscv-canonicalize-syscall-gen.o \
> +			glibc-tdep.o linux-tdep.o solib-svr4.o symfile-mem.o linux-record.o"
>   	;;
>
>   riscv*-*-*)
> diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
> index ff478cf4c28..12cab06f276 100644
> --- a/gdb/riscv-linux-tdep.c
> +++ b/gdb/riscv-linux-tdep.c
> @@ -25,6 +25,11 @@
>   #include "tramp-frame.h"
>   #include "trad-frame.h"
>   #include "gdbarch.h"
> +#include "record-full.h"
> +#include "linux-record.h"
> +#include "riscv-linux-tdep.h"
> +
> +extern unsigned int record_debug;
>
>   /* The following value is derived from __NR_rt_sigreturn in
>      <include/uapi/asm-generic/unistd.h> from the Linux source tree.  */
> @@ -173,6 +178,250 @@ riscv_linux_syscall_next_pc (const frame_info_ptr &frame)
>     return pc + 4 /* Length of the ECALL insn.  */;
>   }
>
> +/* RISC-V process record-replay constructs: syscall, signal etc.  */
> +
> +static linux_record_tdep riscv_linux_record_tdep;
> +
> +/* Record all registers but PC register for process-record.  */
> +
> +using regnum_type = int;
> +
> +static bool
> +save_registers (struct regcache *regcache, regnum_type fir, regnum_type last)

Please fully type the variable names (fir -> first). Even if this makes 
you need to use multiple lines for the function, I think this makes the 
function easier to understand in the future.

Sorry about not mentioning this before, I missed it in my first pass.

> +{
> +  for (regnum_type i = fir; i != last; ++i)
> +    if (record_full_arch_list_add_reg (regcache, i))
> +      return false;
> +  return true;
> +};
> +
> +static bool
> +riscv_all_but_pc_registers_record (struct regcache *regcache)
> +{
> +  auto &&gdbarch = regcache->arch ();
> +  auto &&tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
> +  auto &&features = tdep->isa_features;
> +
> +  if (!save_registers (regcache, RISCV_ZERO_REGNUM + 1, RISCV_PC_REGNUM))
> +    return false;
> +
> +  if (features.flen
> +      && !save_registers (regcache, RISCV_FIRST_FP_REGNUM,
> +                          RISCV_LAST_FP_REGNUM + 1))
> +    return false;
> +
> +  return true;
> +}
> +
> +/* Handler for riscv system call instruction recording.  */
> +
> +static int
> +riscv_linux_syscall_record (struct regcache *regcache,
> +                            unsigned long svc_number)
> +{
> +  auto syscall_gdb = riscv64_canonicalize_syscall (svc_number);
> +
> +  if (record_debug > 1)
> +    {
> +      gdb_printf (gdb_stdlog, "Made syscall %s.\n", plongest (svc_number));
> +    }
> +
> +  if (syscall_gdb == gdb_sys_no_syscall)
> +    {
> +      gdb_printf (gdb_stderr,
> +                  _ ("Process record and replay target doesn't "
> +                     "support syscall number %s\n"),
> +                  plongest (svc_number));
> +      return -1;
> +    }
> +
> +  if (syscall_gdb == gdb_sys_sigreturn || syscall_gdb == gdb_sys_rt_sigreturn)
> +    {
> +      if (!riscv_all_but_pc_registers_record (regcache))
> +        return -1;
> +      return 0;
> +    }
> +
> +  auto ret = record_linux_system_call (syscall_gdb, regcache,
> +                                       &riscv_linux_record_tdep);
> +  if (ret != 0)
> +    return ret;
> +
> +  /* Record the return value of the system call.  */
> +  if (record_full_arch_list_add_reg (regcache, RISCV_A0_REGNUM))
> +    return -1;
> +
> +  return 0;
> +}
> +
> +/* Initialize the riscv64_linux_record_tdep.  */
> +/* These values are the size of the type that will be used in a system
> +    call.  They are obtained from Linux Kernel source.  */
> +static void
> +riscv64_linux_record_tdep_init (
> +    struct gdbarch *gdbarch, struct linux_record_tdep &riscv_linux_record_tdep)

Oops, also missed this one on the first pass.

The preferred way to do multi-line arguments is:

static void
riscv64_linux_record_tdep_init (struct gdbarch *gdbarch,
                                                         struct 
linux_record_tdep &riscv_linux_record_tdep)
{

(with the "struct" keywords being aligned)

> +{
> +  /* Initialize the riscv_linux_record_tdep.  */
> +  /* These values are the size of the type that will be used in a system
> +     call.  They are obtained from Linux Kernel source.  */
> +  riscv_linux_record_tdep.size_pointer
> +      = gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT;
> +  riscv_linux_record_tdep.size__old_kernel_stat = 48;
> +  riscv_linux_record_tdep.size_tms = 32;
> +  riscv_linux_record_tdep.size_loff_t = 8;
> +  riscv_linux_record_tdep.size_flock = 32;
> +  riscv_linux_record_tdep.size_oldold_utsname = 45;
> +  riscv_linux_record_tdep.size_ustat = 32;
> +  riscv_linux_record_tdep.size_old_sigaction = 32;
> +  riscv_linux_record_tdep.size_old_sigset_t = 8;
> +  riscv_linux_record_tdep.size_rlimit = 16;
> +  riscv_linux_record_tdep.size_rusage = 144;
> +  riscv_linux_record_tdep.size_timeval = 8;
> +  riscv_linux_record_tdep.size_timezone = 8;
> +  riscv_linux_record_tdep.size_old_gid_t = 2;
> +  riscv_linux_record_tdep.size_old_uid_t = 2;
> +  riscv_linux_record_tdep.size_fd_set = 128;
> +  riscv_linux_record_tdep.size_old_dirent = 268;
> +  riscv_linux_record_tdep.size_statfs = 120;
> +  riscv_linux_record_tdep.size_statfs64 = 120;
> +  riscv_linux_record_tdep.size_sockaddr = 16;
> +  riscv_linux_record_tdep.size_int
> +    = gdbarch_int_bit (gdbarch) / TARGET_CHAR_BIT;
> +  riscv_linux_record_tdep.size_long
> +    = gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT;
> +  riscv_linux_record_tdep.size_ulong
> +    = gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT;
> +  riscv_linux_record_tdep.size_msghdr = 104;
> +  riscv_linux_record_tdep.size_itimerval = 16;
> +  riscv_linux_record_tdep.size_stat = 128;
> +  riscv_linux_record_tdep.size_old_utsname = 325;
> +  riscv_linux_record_tdep.size_sysinfo = 112;
> +  riscv_linux_record_tdep.size_msqid_ds = 104;
> +  riscv_linux_record_tdep.size_shmid_ds = 88;
> +  riscv_linux_record_tdep.size_new_utsname = 390;
> +  riscv_linux_record_tdep.size_timex = 188;
> +  riscv_linux_record_tdep.size_mem_dqinfo = 72;
> +  riscv_linux_record_tdep.size_if_dqblk = 68;
> +  riscv_linux_record_tdep.size_fs_quota_stat = 64;
> +  riscv_linux_record_tdep.size_timespec = 16;
> +  riscv_linux_record_tdep.size_pollfd = 8;
> +  riscv_linux_record_tdep.size_NFS_FHSIZE = 32;
> +  riscv_linux_record_tdep.size_knfsd_fh = 36;
> +  riscv_linux_record_tdep.size_TASK_COMM_LEN = 4;
> +  riscv_linux_record_tdep.size_sigaction = 24;
> +  riscv_linux_record_tdep.size_sigset_t = 8;
> +  riscv_linux_record_tdep.size_siginfo_t = 128;
> +  riscv_linux_record_tdep.size_cap_user_data_t = 8;
> +  riscv_linux_record_tdep.size_stack_t = 24;
> +  riscv_linux_record_tdep.size_off_t = riscv_linux_record_tdep.size_long;
> +  riscv_linux_record_tdep.size_stat64 = 136;
> +  riscv_linux_record_tdep.size_gid_t = 4;
> +  riscv_linux_record_tdep.size_uid_t = 4;
> +  riscv_linux_record_tdep.size_PAGE_SIZE = 4096;
> +  riscv_linux_record_tdep.size_flock64 = 32;
> +  riscv_linux_record_tdep.size_user_desc = 37;
> +  riscv_linux_record_tdep.size_io_event = 32;
> +  riscv_linux_record_tdep.size_iocb = 64;
> +  riscv_linux_record_tdep.size_epoll_event = 16;
> +  riscv_linux_record_tdep.size_itimerspec
> +    = riscv_linux_record_tdep.size_timespec * 2;
> +  riscv_linux_record_tdep.size_mq_attr = 64;
> +  riscv_linux_record_tdep.size_termios = 36;
> +  riscv_linux_record_tdep.size_termios2 = 44;
> +  riscv_linux_record_tdep.size_pid_t = 4;
> +  riscv_linux_record_tdep.size_winsize = 8;
> +  riscv_linux_record_tdep.size_serial_struct = 72;
> +  riscv_linux_record_tdep.size_serial_icounter_struct = 80;
> +  riscv_linux_record_tdep.size_hayes_esp_config = 12;
> +  riscv_linux_record_tdep.size_size_t = 8;
> +  riscv_linux_record_tdep.size_iovec = 16;
> +  riscv_linux_record_tdep.size_time_t = 8;
> +
> +  /* These values are the second argument of system call "sys_ioctl".
> +     They are obtained from Linux Kernel source.  */
> +  riscv_linux_record_tdep.ioctl_TCGETS = 0x5401;
> +  riscv_linux_record_tdep.ioctl_TCSETS = 0x5402;
> +  riscv_linux_record_tdep.ioctl_TCSETSW = 0x5403;
> +  riscv_linux_record_tdep.ioctl_TCSETSF = 0x5404;
> +  riscv_linux_record_tdep.ioctl_TCGETA = 0x5405;
> +  riscv_linux_record_tdep.ioctl_TCSETA = 0x5406;
> +  riscv_linux_record_tdep.ioctl_TCSETAW = 0x5407;
> +  riscv_linux_record_tdep.ioctl_TCSETAF = 0x5408;
> +  riscv_linux_record_tdep.ioctl_TCSBRK = 0x5409;
> +  riscv_linux_record_tdep.ioctl_TCXONC = 0x540a;
> +  riscv_linux_record_tdep.ioctl_TCFLSH = 0x540b;
> +  riscv_linux_record_tdep.ioctl_TIOCEXCL = 0x540c;
> +  riscv_linux_record_tdep.ioctl_TIOCNXCL = 0x540d;
> +  riscv_linux_record_tdep.ioctl_TIOCSCTTY = 0x540e;
> +  riscv_linux_record_tdep.ioctl_TIOCGPGRP = 0x540f;
> +  riscv_linux_record_tdep.ioctl_TIOCSPGRP = 0x5410;
> +  riscv_linux_record_tdep.ioctl_TIOCOUTQ = 0x5411;
> +  riscv_linux_record_tdep.ioctl_TIOCSTI = 0x5412;
> +  riscv_linux_record_tdep.ioctl_TIOCGWINSZ = 0x5413;
> +  riscv_linux_record_tdep.ioctl_TIOCSWINSZ = 0x5414;
> +  riscv_linux_record_tdep.ioctl_TIOCMGET = 0x5415;
> +  riscv_linux_record_tdep.ioctl_TIOCMBIS = 0x5416;
> +  riscv_linux_record_tdep.ioctl_TIOCMBIC = 0x5417;
> +  riscv_linux_record_tdep.ioctl_TIOCMSET = 0x5418;
> +  riscv_linux_record_tdep.ioctl_TIOCGSOFTCAR = 0x5419;
> +  riscv_linux_record_tdep.ioctl_TIOCSSOFTCAR = 0x541a;
> +  riscv_linux_record_tdep.ioctl_FIONREAD = 0x541b;
> +  riscv_linux_record_tdep.ioctl_TIOCINQ
> +      = riscv_linux_record_tdep.ioctl_FIONREAD;
> +  riscv_linux_record_tdep.ioctl_TIOCLINUX = 0x541c;
> +  riscv_linux_record_tdep.ioctl_TIOCCONS = 0x541d;
> +  riscv_linux_record_tdep.ioctl_TIOCGSERIAL = 0x541e;
> +  riscv_linux_record_tdep.ioctl_TIOCSSERIAL = 0x541f;
> +  riscv_linux_record_tdep.ioctl_TIOCPKT = 0x5420;
> +  riscv_linux_record_tdep.ioctl_FIONBIO = 0x5421;
> +  riscv_linux_record_tdep.ioctl_TIOCNOTTY = 0x5422;
> +  riscv_linux_record_tdep.ioctl_TIOCSETD = 0x5423;
> +  riscv_linux_record_tdep.ioctl_TIOCGETD = 0x5424;
> +  riscv_linux_record_tdep.ioctl_TCSBRKP = 0x5425;
> +  riscv_linux_record_tdep.ioctl_TIOCTTYGSTRUCT = 0x5426;
> +  riscv_linux_record_tdep.ioctl_TIOCSBRK = 0x5427;
> +  riscv_linux_record_tdep.ioctl_TIOCCBRK = 0x5428;
> +  riscv_linux_record_tdep.ioctl_TIOCGSID = 0x5429;
> +  riscv_linux_record_tdep.ioctl_TCGETS2 = 0x802c542a;
> +  riscv_linux_record_tdep.ioctl_TCSETS2 = 0x402c542b;
> +  riscv_linux_record_tdep.ioctl_TCSETSW2 = 0x402c542c;
> +  riscv_linux_record_tdep.ioctl_TCSETSF2 = 0x402c542d;
> +  riscv_linux_record_tdep.ioctl_TIOCGPTN = 0x80045430;
> +  riscv_linux_record_tdep.ioctl_TIOCSPTLCK = 0x40045431;
> +  riscv_linux_record_tdep.ioctl_FIONCLEX = 0x5450;
> +  riscv_linux_record_tdep.ioctl_FIOCLEX = 0x5451;
> +  riscv_linux_record_tdep.ioctl_FIOASYNC = 0x5452;
> +  riscv_linux_record_tdep.ioctl_TIOCSERCONFIG = 0x5453;
> +  riscv_linux_record_tdep.ioctl_TIOCSERGWILD = 0x5454;
> +  riscv_linux_record_tdep.ioctl_TIOCSERSWILD = 0x5455;
> +  riscv_linux_record_tdep.ioctl_TIOCGLCKTRMIOS = 0x5456;
> +  riscv_linux_record_tdep.ioctl_TIOCSLCKTRMIOS = 0x5457;
> +  riscv_linux_record_tdep.ioctl_TIOCSERGSTRUCT = 0x5458;
> +  riscv_linux_record_tdep.ioctl_TIOCSERGETLSR = 0x5459;
> +  riscv_linux_record_tdep.ioctl_TIOCSERGETMULTI = 0x545a;
> +  riscv_linux_record_tdep.ioctl_TIOCSERSETMULTI = 0x545b;
> +  riscv_linux_record_tdep.ioctl_TIOCMIWAIT = 0x545c;
> +  riscv_linux_record_tdep.ioctl_TIOCGICOUNT = 0x545d;
> +  riscv_linux_record_tdep.ioctl_TIOCGHAYESESP = 0x545e;
> +  riscv_linux_record_tdep.ioctl_TIOCSHAYESESP = 0x545f;
> +  riscv_linux_record_tdep.ioctl_FIOQSIZE = 0x5460;
> +
> +  /* These values are the second argument of system call "sys_fcntl"
> +     and "sys_fcntl64".  They are obtained from Linux Kernel source.  */
> +  riscv_linux_record_tdep.fcntl_F_GETLK = 5;
> +  riscv_linux_record_tdep.fcntl_F_GETLK64 = 12;
> +  riscv_linux_record_tdep.fcntl_F_SETLK64 = 13;
> +  riscv_linux_record_tdep.fcntl_F_SETLKW64 = 14;
> +
> +  riscv_linux_record_tdep.arg1 = RISCV_A0_REGNUM;
> +  riscv_linux_record_tdep.arg2 = RISCV_A1_REGNUM;
> +  riscv_linux_record_tdep.arg3 = RISCV_A2_REGNUM;
> +  riscv_linux_record_tdep.arg4 = RISCV_A3_REGNUM;
> +  riscv_linux_record_tdep.arg5 = RISCV_A4_REGNUM;
> +  riscv_linux_record_tdep.arg6 = RISCV_A5_REGNUM;
> +  riscv_linux_record_tdep.arg7 = RISCV_A7_REGNUM;
> +}
> +
>   /* Initialize RISC-V Linux ABI info.  */
>
>   static void
> @@ -205,6 +454,9 @@ riscv_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>     tramp_frame_prepend_unwinder (gdbarch, &riscv_linux_sigframe);
>
>     tdep->syscall_next_pc = riscv_linux_syscall_next_pc;
> +  tdep->riscv_syscall_record = riscv_linux_syscall_record;
> +
> +  riscv64_linux_record_tdep_init (gdbarch, riscv_linux_record_tdep);
>   }
>
>   /* Initialize RISC-V Linux target support.  */
> diff --git a/gdb/riscv-linux-tdep.h b/gdb/riscv-linux-tdep.h
> new file mode 100644
> index 00000000000..a0d67003a81
> --- /dev/null
> +++ b/gdb/riscv-linux-tdep.h
> @@ -0,0 +1,28 @@
> +/* Copyright (C) 2024 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef RISCV_LINUX_TDEP_H
> +#define RISCV_LINUX_TDEP_H
> +
> +#include "linux-record.h"
> +
> +/* riscv64_canonicalize_syscall maps from the native riscv Linux set
> +   of syscall ids into a canonical set of syscall ids used by
> +   process record.  */
> +extern enum gdb_syscall riscv64_canonicalize_syscall (int syscall);
> +
> +#endif /* RISCV_LINUX_TDEP_H */
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 932708ca4e9..c6c7b7a63fd 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -54,9 +54,12 @@
>   #include "observable.h"
>   #include "prologue-value.h"
>   #include "arch/riscv.h"
> +#include "record-full.h"
>   #include "riscv-ravenscar-thread.h"
>   #include "gdbsupport/gdb-safe-ctype.h"
>
> +#include <vector>
> +
>   /* The stack must be 16-byte aligned.  */
>   #define SP_ALIGNMENT 16
>
> @@ -1655,6 +1658,11 @@ class riscv_insn
>     int imm_signed () const
>     { return m_imm.s; }
>
> +  /* Fetch instruction from target memory at ADDR, return the content of
> +     the instruction, and update LEN with the instruction length.  */
> +  static ULONGEST fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr,
> +                                     int *len);
> +
>   private:
>
>     /* Extract 5 bit register field at OFFSET from instruction OPCODE.  */
> @@ -1800,11 +1808,6 @@ class riscv_insn
>       m_rs2 = decode_register_index_short (ival, OP_SH_CRS2S);
>     }
>
> -  /* Fetch instruction from target memory at ADDR, return the content of
> -     the instruction, and update LEN with the instruction length.  */
> -  static ULONGEST fetch_instruction (struct gdbarch *gdbarch,
> -				     CORE_ADDR addr, int *len);
> -
>     /* The length of the instruction in bytes.  Should be 2 or 4.  */
>     int m_length;
>
> @@ -4415,6 +4418,9 @@ riscv_gdbarch_init (struct gdbarch_info info,
>     set_gdbarch_stap_register_indirection_suffixes
>       (gdbarch, stap_register_indirection_suffixes);
>
> +  /* Process record-replay */
> +  set_gdbarch_process_record (gdbarch, riscv_process_record);
> +
>     /* Hook in OS ABI-specific overrides, if they have been registered.  */
>     gdbarch_init_osabi (info, gdbarch);
>
> @@ -4833,3 +4839,690 @@ this option can be used."),
>   				&setriscvcmdlist,
>   				&showriscvcmdlist);
>   }
> +
> +static bool
> +try_read (struct regcache *regcache, int regnum, ULONGEST &addr)
> +{
> +  gdb_assert (regcache);
> +
> +  if (regcache->raw_read<ULONGEST> (regnum, &addr)
> +      != register_status::REG_VALID)
> +    {
> +      warning (_ ("Can not read at address %lx"), addr);
> +      return false;
> +    }
> +  return true;
> +}
> +
> +class riscv_recorded_insn final
> +{
> +public:
> +  using regnum_type = int;
> +  using memory_type = std::pair<CORE_ADDR, int>;
> +
> +  enum class record_type
> +  {
> +    UNKNOWN,
> +    ORDINARY,
> +
> +    /* Corner cases.  */
> +    ECALL,
> +    EBREAK,
> +  };
> +
> +private:
> +  using recorded_regs = std::vector<regnum_type>;
> +  using recorded_mems = std::vector<memory_type>;
> +
> +  using mem_addr = decltype (std::declval<memory_type> ().first);
> +  using mem_len = decltype (std::declval<memory_type> ().second);
> +
> +  record_type m_record_type = record_type::UNKNOWN;
> +
> +  bool m_error_occured = false;
> +
> +  recorded_regs m_regs;
> +  recorded_mems m_mems;
> +
> +  /* Helper for decode 16-bit instruction RS1. */
> +  static regnum_type
> +  decode_crs1_short (ULONGEST opcode) noexcept
> +  {
> +    return ((opcode >> OP_SH_CRS1S) & OP_MASK_CRS1S) + 8;
> +  }
> +
> +  /* Helper for decode 16-bit instruction RS2. */
> +  static regnum_type
> +  decode_crs2_short (ULONGEST opcode) noexcept
> +  {
> +    return ((opcode >> OP_SH_CRS2S) & OP_MASK_CRS2S) + 8;
> +  }
> +
> +  /* Helper for decode 16-bit instruction CRS1. */
> +  static regnum_type
> +  decode_crs1 (ULONGEST opcode) noexcept
> +  {
> +    return ((opcode >> OP_SH_RD) & OP_MASK_RD);
> +  }
> +
> +  /* Helper for decode 16-bit instruction CRS2. */
> +  static regnum_type
> +  decode_crs2 (ULONGEST opcode) noexcept
> +  {
> +    return ((opcode >> OP_SH_CRS2) & OP_MASK_CRS2);
> +  }
> +
> +  /* Helper for decode 32-bit instruction RD. */
> +  static regnum_type
> +  decode_rd (ULONGEST ival) noexcept
> +  {
> +    return (ival >> OP_SH_RD) & OP_MASK_RD;
> +  }
> +
> +  /* Helper for decode 32-bit instruction RS1. */
> +  static regnum_type
> +  decode_rs1 (ULONGEST ival) noexcept
> +  {
> +    return (ival >> OP_SH_RS1) & OP_MASK_RS1;
> +  }
> +
> +  /* Helper for decode 32-bit instruction RS2. */
> +  static regnum_type
> +  decode_rs2 (ULONGEST ival) noexcept
> +  {
> +    return (ival >> OP_SH_RS2) & OP_MASK_RS2;
> +  }
> +
> +  /* Helper for decode 32-bit instruction CSR. */
> +  static regnum_type
> +  decode_csr (ULONGEST ival) noexcept
> +  {
> +    return (ival >> OP_SH_CSR) & OP_MASK_CSR;
> +  }
> +
> +  bool
> +  set_ordinary_record_type () noexcept
> +  {
> +    m_record_type = record_type::ORDINARY;
> +    return true;
> +  }
> +
> +  bool
> +  set_error () noexcept
> +  {
> +    m_error_occured = true;
> +    return false;
> +  }
> +
> +  bool
> +  has_error () const noexcept
> +  {
> +    return m_error_occured;
> +  }
> +
> +  bool
> +  read_reg (struct regcache *regcache, regnum_type reg,
> +            ULONGEST &addr) noexcept
> +  {
> +    gdb_assert (regcache);
> +
> +    if (!try_read (regcache, reg, addr))
> +      {
> +        return set_error ();
> +      }
> +    return true;
> +  }
> +
> +  bool
> +  save_reg (regnum_type regnum) noexcept
> +  {
> +    try
> +      {
> +        m_regs.emplace_back (regnum);

Similar to my thoughts on Loongarch, I don't think this approach is the 
best.

In short, recording is already considered very slow. The strategy this 
patch uses is to first collect all the data to be recorded (which 
demands dynamic allocations to std::vector), and then call 
record_full_arch_list_add_reg or record_full_arch_list_add_mem, which 
will do their own dynamic allocations. This seems wasteful to me.

I believe it would be better if, instead, the save_reg and save_mem 
methods would directly call the correct record_full_arch_list_add 
function, so that only one allocation is needed per unit of data that 
needs recording.

Since aarch64 already uses the strategy in this patch I won't block 
merging based on this feedback, if you're set on this idea, but I think 
direct calls would be better.

> +        return true;
> +      }
> +    catch (const std::bad_alloc &ex)
> +      {
> +        warning (_ ("Exception was caught during reverse execution: %s"),
> +                 ex.what ());
> +        return set_error ();
> +      }
> +  }
> +
> +  bool
> +  save_mem (mem_addr addr, mem_len len) noexcept
> +  {
> +    try
> +      {
> +        m_mems.emplace_back (addr, len);
> +        return true;
> +      }
> +    catch (const std::bad_alloc &ex)
> +      {
> +        warning (_ ("Exception was caught during reverse execution: %s"),
> +                 ex.what ());
> +        return set_error ();
> +      }
> +  }
> +
> +  static bool
> +  need_save_pc (ULONGEST ival) noexcept
> +  {
> +    return is_beq_insn (ival) || is_bne_insn (ival) || is_blt_insn (ival)
> +           || is_bge_insn (ival) || is_bltu_insn (ival) || is_bgeu_insn (ival)
> +           || is_fence_insn (ival) || is_pause_insn (ival)
> +           || is_fence_i_insn (ival);
> +  }
> +
> +  /* Returns true if instruction is classified.  */
> +  bool
> +  try_save_pc (ULONGEST ival) noexcept
> +  {
> +    if (!need_save_pc (ival))
> +      return false;
> +
> +    return set_ordinary_record_type ();
> +  }
> +
> +  static bool
> +  need_save_pc_rd (ULONGEST ival) noexcept
> +  {
> +    return is_lui_insn (ival) || is_auipc_insn (ival) || is_jal_insn (ival)
> +           || is_jalr_insn (ival) || is_lb_insn (ival) || is_lh_insn (ival)
> +           || is_lw_insn (ival) || is_ld_insn (ival) || is_lbu_insn (ival)
> +           || is_lhu_insn (ival) || is_lb_insn (ival) || is_lh_insn (ival)
> +           || is_lw_insn (ival) || is_ld_insn (ival) || is_lbu_insn (ival)
> +           || is_lhu_insn (ival) || is_addi_insn (ival) || is_slti_insn (ival)
> +           || is_sltiu_insn (ival) || is_xori_insn (ival) || is_ori_insn (ival)
> +           || is_andi_insn (ival) || is_slli_insn (ival) || is_srli_insn (ival)
> +           || is_srai_insn (ival) || is_add_insn (ival) || is_sub_insn (ival)
> +           || is_sll_insn (ival) || is_slt_insn (ival) || is_sltu_insn (ival)
> +           || is_xor_insn (ival) || is_srl_insn (ival) || is_sra_insn (ival)
> +           || is_or_insn (ival) || is_and_insn (ival) || is_lwu_insn (ival)
> +           || is_addiw_insn (ival) || is_slliw_insn (ival)
> +           || is_srliw_insn (ival) || is_sraiw_insn (ival)
> +           || is_addw_insn (ival) || is_subw_insn (ival) || is_sllw_insn (ival)
> +           || is_srlw_insn (ival) || is_sraw_insn (ival) || is_mul_insn (ival)
> +           || is_mulh_insn (ival) || is_mulhsu_insn (ival)
> +           || is_mulhu_insn (ival) || is_div_insn (ival) || is_divu_insn (ival)
> +           || is_rem_insn (ival) || is_remu_insn (ival) || is_mulw_insn (ival)
> +           || is_divw_insn (ival) || is_divuw_insn (ival)
> +           || is_remw_insn (ival) || is_remuw_insn (ival)
> +           || is_lr_w_insn (ival) || is_lr_d_insn (ival)
> +           || is_fcvt_l_s_insn (ival) || is_fcvt_lu_s_insn (ival)
> +           || is_feq_d_insn (ival) || is_flt_d_insn (ival)
> +           || is_fle_d_insn (ival) || is_fclass_d_insn (ival)
> +           || is_fcvt_w_d_insn (ival) || is_fcvt_wu_d_insn (ival)
> +           || is_fcvt_d_w_insn (ival) || is_fcvt_l_d_insn (ival)
> +           || is_fcvt_lu_d_insn (ival) || is_fmv_x_d_insn (ival);
> +  }
> +
> +  /* Returns true if instruction is classified. Needs error checking after.  */
> +  bool
> +  try_save_pc_rd (ULONGEST ival) noexcept
> +  {
> +    if (!need_save_pc_rd (ival))
> +      return false;
> +
> +    return !save_reg (decode_rd (ival)) || set_ordinary_record_type ();
> +  }
> +
> +  static bool
> +  need_save_pc_fprd (ULONGEST ival) noexcept
> +  {
> +    return is_fmadd_s_insn (ival) || is_fmsub_s_insn (ival)
> +           || is_fnmsub_s_insn (ival) || is_fnmadd_s_insn (ival)
> +           || is_fadd_s_insn (ival) || is_fsub_s_insn (ival)
> +           || is_fmul_s_insn (ival) || is_fdiv_s_insn (ival)
> +           || is_fsqrt_s_insn (ival) || is_fsgnj_s_insn (ival)
> +           || is_fsgnjn_s_insn (ival) || is_fsgnjx_s_insn (ival)
> +           || is_fmin_s_insn (ival) || is_fmax_s_insn (ival)
> +           || is_flw_insn (ival) || is_fld_insn (ival)
> +           || is_fcvt_s_w_insn (ival) || is_fcvt_s_wu_insn (ival)
> +           || is_fmv_s_x_insn (ival) || is_fcvt_s_l_insn (ival)
> +           || is_fcvt_s_lu_insn (ival) || is_fmadd_d_insn (ival)
> +           || is_fmsub_d_insn (ival) || is_fnmsub_d_insn (ival)
> +           || is_fnmadd_d_insn (ival) || is_fadd_d_insn (ival)
> +           || is_fsub_d_insn (ival) || is_fmul_d_insn (ival)
> +           || is_fdiv_d_insn (ival) || is_fsqrt_d_insn (ival)
> +           || is_fsgnj_d_insn (ival) || is_fsgnjn_d_insn (ival)
> +           || is_fsgnjx_d_insn (ival) || is_fmin_d_insn (ival)
> +           || is_fmax_d_insn (ival) || is_fcvt_s_d_insn (ival)
> +           || is_fcvt_d_s_insn (ival) || is_fcvt_d_wu_insn (ival)
> +           || is_fcvt_d_l_insn (ival) || is_fcvt_d_lu_insn (ival)
> +           || is_fmv_d_x_insn (ival);
> +  }
> +
> +  /* Returns true if instruction is classified. Needs error checking after.  */
> +  bool
> +  try_save_pc_fprd (ULONGEST ival) noexcept
> +  {
> +    if (!need_save_pc_fprd (ival))
> +      return false;
> +
> +    return !save_reg (RISCV_FIRST_FP_REGNUM + decode_rd (ival))
> +           || set_ordinary_record_type ();
> +  }
> +
> +  static bool
> +  need_save_pc_rd_csr (ULONGEST ival) noexcept
> +  {
> +    return is_csrrw_insn (ival) || is_csrrs_insn (ival) || is_csrrc_insn (ival)
> +           || is_csrrwi_insn (ival) || is_csrrsi_insn (ival)
> +           || is_csrrc_insn (ival);
> +  }
> +
> +  /* Returns true if instruction is classified. Needs error checking after.  */
> +  bool
> +  try_save_pc_rd_csr (ULONGEST ival) noexcept
> +  {
> +    if (!need_save_pc_rd_csr (ival))
> +      return false;
> +
> +    return !save_reg (decode_rd (ival))
> +           || !save_reg (RISCV_FIRST_CSR_REGNUM + decode_csr (ival))
> +           || set_ordinary_record_type ();
> +  }
> +
> +  /* If return value is > 0, than instruction needs saving memory len.  */
> +  static mem_len
> +  need_save_pc_mem (ULONGEST ival) noexcept
> +  {
> +    if (is_sb_insn (ival))
> +      return 1;
> +    if (is_sh_insn (ival))
> +      return 2;
> +    if (is_sw_insn (ival) || is_fsw_insn (ival))
> +      return 4;
> +    if (is_sd_insn (ival) || is_fsd_insn (ival))
> +      return 8;
> +    return 0;
> +  }
> +
> +  /* Returns true if instruction is classified. Needs error checking after.  */
> +  bool
> +  try_save_pc_mem (ULONGEST ival, struct regcache *regcache) noexcept
> +  {
> +    gdb_assert (regcache);
> +
> +    mem_addr addr = 0;
> +    auto len = need_save_pc_mem (ival);
> +    if (len <= 0)
> +      return false;
> +
> +    mem_len offset = EXTRACT_STYPE_IMM (ival);
> +    return !read_reg (regcache, decode_rs1 (ival), addr)
> +           || !save_mem (addr + offset, len) || set_ordinary_record_type ();
> +  }
> +
> +  /* If return value is > 0, than instruction needs saving memory len.  */
> +  static mem_len
> +  need_save_pc_rd_mem (ULONGEST ival) noexcept
> +  {
> +    if (is_sc_w_insn (ival) || is_amoadd_w_insn (ival)
> +        || is_amoxor_w_insn (ival) || is_amoand_w_insn (ival)
> +        || is_amoor_w_insn (ival) || is_amomin_w_insn (ival)
> +        || is_amomax_w_insn (ival) || is_amominu_w_insn (ival)
> +        || is_amomaxu_w_insn (ival))
> +      return 4;
> +    if (is_sc_d_insn (ival) || is_amoadd_d_insn (ival)
> +        || is_amoxor_d_insn (ival) || is_amoand_d_insn (ival)
> +        || is_amoor_d_insn (ival) || is_amomin_d_insn (ival)
> +        || is_amomax_d_insn (ival) || is_amominu_d_insn (ival)
> +        || is_amomaxu_d_insn (ival))
> +      return 8;
> +    return 0;
> +  }
> +
> +  /* Returns true if instruction is classified. Needs error checking after.  */
> +  bool
> +  try_save_pc_rd_mem (ULONGEST ival, struct regcache *regcache) noexcept
> +  {
> +    gdb_assert (regcache);
> +
> +    auto len = need_save_pc_rd_mem (ival);
> +    mem_addr addr = 0;
> +    if (len <= 0)
> +      return false;
> +
> +    return !read_reg (regcache, decode_rs1 (ival), addr)
> +           || !save_mem (addr, len) || !save_reg (decode_rd (ival))
> +           || set_ordinary_record_type ();
> +  }
> +
> +  /* If return value is > 0, than instruction needs saving memory len.  */
> +  static mem_len
> +  need_save_pc_rs2_rd_mem (ULONGEST ival) noexcept
> +  {
> +    if (is_amoswap_w_insn (ival))
> +      return 4;
> +    if (is_amoswap_d_insn (ival))
> +      return 8;
> +    return 0;
> +  }
> +
> +  /* Returns true if instruction is classified. Needs error checking after.  */
> +  bool
> +  try_save_pc_rs2_rd_mem (ULONGEST ival, struct regcache *regcache) noexcept
> +  {
> +    gdb_assert (regcache);
> +
> +    mem_addr addr = 0;
> +    auto len = need_save_pc_rs2_rd_mem (ival);
> +    if (len <= 0)
> +      return false;
> +
> +    return !read_reg (regcache, decode_rs1 (ival), addr)
> +           || !save_mem (addr, len) || !save_reg (decode_rs2 (ival))
> +           || !save_reg (decode_rd (ival)) || set_ordinary_record_type ();
> +  }
> +
> +  /* Returns true if instruction is successfully recorder.
> +     Else returns false.  */
> +  bool
> +  record_insn_len4 (ULONGEST ival, struct regcache *regcache) noexcept
> +  {
> +    gdb_assert (regcache);
> +
> +    if (is_ecall_insn (ival))
> +      {
> +        m_record_type = record_type::ECALL;
> +        return true;
> +      }
> +
> +    if (is_ebreak_insn (ival))
> +      {
> +        m_record_type = record_type::EBREAK;
> +        return true;
> +      }
> +
> +    if (try_save_pc (ival) || try_save_pc_rd (ival) || try_save_pc_fprd (ival)
> +        || try_save_pc_rd_csr (ival) || try_save_pc_mem (ival, regcache)
> +        || try_save_pc_rd_mem (ival, regcache)
> +        || try_save_pc_rs2_rd_mem (ival, regcache))
> +      {
> +        return !has_error ();
> +      }
Minor nit, but I noticed around here that this (and some lines later, 
maybe some before) don't need the braces.
> +
> +    warning (_ ("Currently this instruction with len 4(%lx) is unsupported"),
> +             ival);
> +    return false;
> +  }
> +
> +  /* Returns true if instruction is successfully recorder.
> +     Else returns false.  */
> +  bool
> +  record_insn_len2 (ULONGEST ival, struct regcache *regcache,
> +                    struct gdbarch *gdbarch) noexcept
> +  {
> +    gdb_assert (regcache);
> +    gdb_assert (gdbarch);
> +
> +    mem_addr addr = 0;
> +    auto xlen = riscv_isa_xlen (gdbarch);
> +
> +    /* The order here is very important, because
> +       opcodes of some instructions may be the same.  */
> +
> +    if (is_c_addi4spn_insn (ival) || is_c_lw_insn (ival)
> +        || (xlen == 8 && is_c_ld_insn (ival)))
> +      {
> +        return !save_reg (decode_crs2_short (ival))
> +               || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_fld_insn (ival) || (xlen == 4 && is_c_flw_insn (ival)))
> +      {
> +        return !save_reg (RISCV_FIRST_FP_REGNUM + decode_crs2_short (ival))
> +               || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_fsd_insn (ival) || (xlen == 8 && is_c_sd_insn (ival)))
> +      {
> +        int offset = EXTRACT_CLTYPE_LD_IMM (ival);
> +        return !read_reg (regcache, decode_crs1_short (ival), addr)
> +               || !save_mem (addr + offset, 8) || set_ordinary_record_type ();
> +      }
> +
> +    if ((xlen == 4 && is_c_fsw_insn (ival)) || is_c_sw_insn (ival))
> +      {
> +        int offset = EXTRACT_CLTYPE_LW_IMM (ival);
> +        return !read_reg (regcache, decode_crs1_short (ival), addr)
> +               || !save_mem (addr + offset, 4) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_nop_insn (ival))
> +      {
> +        return set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_addi_insn (ival))
> +      {
> +        return !save_reg (decode_crs1 (ival)) || set_ordinary_record_type ();
> +      }
> +
> +    if (xlen == 4 && is_c_jal_insn (ival))
> +      {
> +        return !save_reg (RISCV_RA_REGNUM) || set_ordinary_record_type ();
> +      }
> +
> +    if ((xlen == 8 && is_c_addiw_insn (ival)) || is_c_li_insn (ival))
> +      {
> +        return !save_reg (decode_crs1 (ival)) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_addi16sp_insn (ival))
> +      {
> +        return !save_reg (RISCV_SP_REGNUM) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_lui_insn (ival))
> +      {
> +        return !save_reg (decode_crs1 (ival)) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_srli_insn (ival) || is_c_srai_insn (ival) || is_c_andi_insn (ival)
> +        || is_c_sub_insn (ival) || is_c_xor_insn (ival) || is_c_or_insn (ival)
> +        || is_c_and_insn (ival) || (xlen == 8 && is_c_subw_insn (ival))
> +        || (xlen == 8 && is_c_addw_insn (ival)))
> +      {
> +        return !save_reg (decode_crs1_short (ival))
> +               || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_j_insn (ival) || is_c_beqz_insn (ival) || is_c_bnez_insn (ival))
> +      {
> +        return set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_slli_insn (ival))
> +      {
> +        return !save_reg (decode_crs1 (ival)) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_fldsp_insn (ival) || (xlen == 4 && is_c_flwsp_insn (ival)))
> +      {
> +        return !save_reg (RISCV_FIRST_FP_REGNUM + decode_crs1 (ival))
> +               || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_lwsp_insn (ival) || (xlen == 8 && is_c_ldsp_insn (ival)))
> +      {
> +        return !save_reg (decode_crs1 (ival)) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_jr_insn (ival))
> +      {
> +        return set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_mv_insn (ival))
> +      {
> +        return !save_reg (decode_crs1 (ival)) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_ebreak_insn (ival))
> +      {
> +        m_record_type = record_type::EBREAK;
> +        return true;
> +      }
> +
> +    if (is_c_jalr_insn (ival))
> +      {
> +        return !save_reg (RISCV_RA_REGNUM) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_add_insn (ival))
> +      {
> +        return !save_reg (decode_crs1 (ival)) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_fsdsp_insn (ival) || (xlen == 8 && is_c_sdsp_insn (ival)))
> +      {
> +        int offset = EXTRACT_CSSTYPE_SDSP_IMM (ival);
> +        return !read_reg (regcache, RISCV_SP_REGNUM, addr)
> +               || !save_mem (addr + offset, 8) || set_ordinary_record_type ();
> +      }
> +
> +    if (is_c_swsp_insn (ival) || (xlen == 4 && is_c_fswsp_insn (ival)))
> +      {
> +        int offset = EXTRACT_CSSTYPE_SWSP_IMM (ival);
> +        return !read_reg (regcache, RISCV_SP_REGNUM, addr)
> +               || !save_mem (addr + offset, 4) || set_ordinary_record_type ();
> +      }
> +
> +    warning (_ ("Currently this instruction with len 2(%lx) is unsupported"),
> +             ival);
> +    return false;
> +  }
> +
> +public:
> +  using regs_iter = recorded_regs::const_iterator;
> +  using mems_iter = recorded_mems::const_iterator;
> +
> +  bool
> +  record (gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR addr) noexcept
> +  {
> +    int m_length = 0;
> +    auto ival = riscv_insn::fetch_instruction (gdbarch, addr, &m_length);
> +    if (!save_reg (RISCV_PC_REGNUM))
> +      return false;
> +
> +    if (m_length == 4)
> +      return record_insn_len4 (ival, regcache);
> +
> +    if (m_length == 2)
> +      return record_insn_len2 (ival, regcache, gdbarch);
> +
> +    /* 6 bytes or more.  If the instruction is longer than 8 bytes, we don't
> +       have full instruction bits in ival.  At least, such long instructions
> +       are not defined yet, so just ignore it.  */
> +    gdb_assert (m_length > 0 && m_length % 2 == 0);
> +
> +    warning (_ ("Can not record unknown instruction (opcode = %lx)"), ival);
> +    return false;
> +  }
> +
> +  record_type
> +  get_record_type () const noexcept
> +  {
> +    return m_record_type;
> +  }
> +
> +  regs_iter
> +  regs_begin () const noexcept
> +  {
> +    return m_regs.begin ();
> +  }
> +
> +  regs_iter
> +  regs_end () const noexcept
> +  {
> +    return m_regs.end ();
> +  }
> +
> +  mems_iter
> +  mems_begin () const noexcept
> +  {
> +    return m_mems.begin ();
> +  }
> +
> +  mems_iter
> +  mems_end () const noexcept
> +  {
> +    return m_mems.end ();
> +  }
> +};
> +
> +static int
> +riscv_make_record_process (struct gdbarch *gdbarch, struct regcache *regcache,
> +                           const riscv_recorded_insn &insn)
> +{
> +  gdb_assert (gdbarch && regcache);
> +
> +  riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
> +  auto regs_begin = insn.regs_begin ();
> +  auto regs_end = insn.regs_end ();
> +  if (std::any_of (regs_begin, regs_end, [&regcache] (auto &&reg_it) {
> +        return record_full_arch_list_add_reg (regcache, reg_it);
> +      }))
> +    return -1;
> +
> +  auto mems_begin = insn.mems_begin ();
> +  auto mems_end = insn.mems_end ();
> +  if (std::any_of (mems_begin, mems_end, [] (auto &&mem_it) {
> +        return record_full_arch_list_add_mem (mem_it.first, mem_it.second);
> +      }))
> +    return -1;
> +
> +  switch (insn.get_record_type ())
> +    {
> +    case riscv_recorded_insn::record_type::ORDINARY:
> +      break;
> +
> +    case riscv_recorded_insn::record_type::ECALL:
> +      {
> +        if (!tdep->riscv_syscall_record)
> +          {
> +            warning (_ ("Syscall record is not supported"));
> +            return -1;
> +          }
> +        ULONGEST reg_val = 0;
> +        if (!try_read (regcache, RISCV_A7_REGNUM, reg_val))
> +          {
> +            return -1;
> +          }
> +        return tdep->riscv_syscall_record (regcache, reg_val);
> +      }
> +
> +    case riscv_recorded_insn::record_type::EBREAK:
> +      break;
> +
> +    default:
> +      return -1;
> +    }
> +  return 0;
> +}
> +
> +int
> +riscv_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
> +                      CORE_ADDR addr)
> +{
> +  gdb_assert (gdbarch && regcache);
null pointer checks should be explicit, ie, gdb_assert (gdbarch != 
nullptr && regcache != nullptr)