Re: [PATCH v4] This commit adds record full support for rv64gc instruction set.

Guinevere Larsen
2024-12-17T14:24:25Z

On 12/16/24 9:10 PM, Timur wrote:
> It includes changes to the following files:
> - gdb/riscv-linux-tdep.c, gdb/riscv-linux-tdep.h: adds facilities to record
> syscalls.
> - gdb/riscv-tdep.c, gdb/riscv-tdep.h: adds facilities to record execution of
> rv64gc instructions.
> - gdb/configure.tgt: adds new files for compilation.
> - gdb/testsuite/lib/gdb.exp: enables testing of full record mode for RISC-V
> targets.
> - gdb/syscalls/riscv-canonicalize-syscall-gen.py: a script to generate function
> that canonicalizes RISC-V syscall. This script can simplify support for syscalls
> on rv32 system. To use this script you need to pass a path to a file with
As I mentioned  in the previous email, please mention both rv32 and
rv64, as the current wording doesn't make it clear that it helps both
> syscalls description from riscv-glibc (example is in the help message). The
> script produces a mapping from syscall names to gdb_syscall enum.
> - gdb/riscv-canonicalize-syscall.c: the file generated by the previous script.
>
> Timur Golubovich timurgol007@gmail.com
>
> Hi! I am sorry, I did incorrect previos time (made tab so as `config.lt` was
> added). So I will duplicate my answers from the previous letter.

Hi!

Thanks for sending the updated version. Commit message is ok apart from
the one nitpick, though I think we tend to do more paragraph style
messages, rather than changelog style.

I have some comments on the code, but with those fixed I'll be happy to
add my +1 to this patch, but I'll ask that you wait for a risc-v or
global maintainer to approve it, as I can only review stuff related to
recording itself.

Additionally, since this patch seems to be close to being approved, I
checked and I can't see your name in past contributions to GDB. Do you
have a copyright assignment in place? If not, you might want to start
the process soon, since it can take a little while. Take a look at the
gdb wiki for more information:
https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment

>
>>>> + 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.
> Could you please double-check the commit message? I tried to make it more
> detailed. Do you think I need to provide additional information?
>
>>>> +
>>>> + 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.c
> Added this information to the commit message.
>
>>> /* 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.
> Addressed
>
>>> + 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)
> Addressed
>
>>> + {
>>> + 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.
> I think that this overhead is negligeble, since the number of allocation
> because most instructions affect only 1 register. Moreover, I feel that the
> suggested approach simplifies error handling, since I can decouple decoding
> procedure from the actual error detection. If you don't mind, I would suggest
> to leave it as it is.
>
>>> + 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.
> Addressed
>
>>> + 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)
> Addressed.
>
> Best wishes,
> Timur Golubovich
>
> ---
> gdb/configure.tgt | 4 +-
> gdb/riscv-canonicalize-syscall-gen.c | 338 +++++++++
> gdb/riscv-linux-tdep.c | 260 ++++++-
> gdb/riscv-linux-tdep.h | 28 +
> gdb/riscv-tdep.c | 666 +++++++++++++++++-
> gdb/riscv-tdep.h | 15 +
> .../riscv-canonicalize-syscall-gen.py | 143 ++++
> gdb/testsuite/lib/gdb.exp | 3 +-
> 8 files changed, 1448 insertions(+), 9 deletions(-)
> create mode 100644 gdb/riscv-canonicalize-syscall-gen.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..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-canonicalize-syscall-gen.c b/gdb/riscv-canonicalize-syscall-gen.c
> new file mode 100644
> index 00000000000..60d80aae0a6
> --- /dev/null
> +++ b/gdb/riscv-canonicalize-syscall-gen.c
> @@ -0,0 +1,338 @@
> +/* DO NOT EDIT: Autogenerated by riscv-canonicalize-syscall-gen.py
> +
> + 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/>. */
> +
> +#include "defs.h"
> +#include "riscv-linux-tdep.h"
> +
> +enum gdb_syscall
> +riscv64_canonicalize_syscall (int syscall)
> +{
> + switch (syscall)
> + {
> + case 0: return gdb_sys_io_setup;
> + case 1: return gdb_sys_io_destroy;
> + case 2: return gdb_sys_io_submit;
> + case 3: return gdb_sys_io_cancel;
> + case 4: return gdb_sys_io_getevents;
> + case 5: return gdb_sys_setxattr;
> + case 6: return gdb_sys_lsetxattr;
> + case 7: return gdb_sys_fsetxattr;
> + case 8: return gdb_sys_getxattr;
> + case 9: return gdb_sys_lgetxattr;
> + case 10: return gdb_sys_fgetxattr;
> + case 11: return gdb_sys_listxattr;
> + case 12: return gdb_sys_llistxattr;
> + case 13: return gdb_sys_flistxattr;
> + case 14: return gdb_sys_removexattr;
> + case 15: return gdb_sys_lremovexattr;
> + case 16: return gdb_sys_fremovexattr;
> + case 17: return gdb_sys_getcwd;
> + case 18: return gdb_sys_lookup_dcookie;
> + case 19: return gdb_sys_eventfd2;
> + case 20: return gdb_sys_epoll_create1;
> + case 21: return gdb_sys_epoll_ctl;
> + case 22: return gdb_sys_epoll_pwait;
> + case 23: return gdb_sys_dup;
> + case 24: return gdb_sys_dup3;
> + case 25: return gdb_sys_fcntl;
> + case 26: return gdb_sys_inotify_init1;
> + case 27: return gdb_sys_inotify_add_watch;
> + case 28: return gdb_sys_inotify_rm_watch;
> + case 29: return gdb_sys_ioctl;
> + case 30: return gdb_sys_ioprio_set;
> + case 31: return gdb_sys_ioprio_get;
> + case 32: return gdb_sys_flock;
> + case 33: return gdb_sys_mknodat;
> + case 34: return gdb_sys_mkdirat;
> + case 35: return gdb_sys_unlinkat;
> + case 36: return gdb_sys_symlinkat;
> + case 37: return gdb_sys_linkat;
> + /* case 39: return gdb_sys_umount2; */
> + case 40: return gdb_sys_mount;
> + case 41: return gdb_sys_pivot_root;
> + case 42: return gdb_sys_nfsservctl;
> + case 43: return gdb_sys_statfs;
> + case 44: return gdb_sys_fstatfs;
> + case 45: return gdb_sys_truncate;
> + case 46: return gdb_sys_ftruncate;
> + case 47: return gdb_sys_fallocate;
> + case 48: return gdb_sys_faccessat;
> + case 49: return gdb_sys_chdir;
> + case 50: return gdb_sys_fchdir;
> + case 51: return gdb_sys_chroot;
> + case 52: return gdb_sys_fchmod;
> + case 53: return gdb_sys_fchmodat;
> + case 54: return gdb_sys_fchownat;
> + case 55: return gdb_sys_fchown;
> + case 56: return gdb_sys_openat;
> + case 57: return gdb_sys_close;
> + case 58: return gdb_sys_vhangup;
> + case 59: return gdb_sys_pipe2;
> + case 60: return gdb_sys_quotactl;
> + case 61: return gdb_sys_getdents64;
> + case 62: return gdb_sys_lseek;
> + case 63: return gdb_sys_read;
> + case 64: return gdb_sys_write;
> + case 65: return gdb_sys_readv;
> + case 66: return gdb_sys_writev;
> + case 67: return gdb_sys_pread64;
> + case 68: return gdb_sys_pwrite64;
> + /* case 69: return gdb_sys_preadv; */
> + /* case 70: return gdb_sys_pwritev; */
> + case 71: return gdb_sys_sendfile;
> + case 72: return gdb_sys_pselect6;
> + case 73: return gdb_sys_ppoll;
> + /* case 74: return gdb_sys_signalfd4; */
> + case 75: return gdb_sys_vmsplice;
> + case 76: return gdb_sys_splice;
> + case 77: return gdb_sys_tee;
> + case 78: return gdb_sys_readlinkat;
> + case 79: return gdb_sys_newfstatat;
> + case 80: return gdb_sys_fstat;
> + case 81: return gdb_sys_sync;
> + case 82: return gdb_sys_fsync;
> + case 83: return gdb_sys_fdatasync;
> + case 84: return gdb_sys_sync_file_range;
> + /* case 85: return gdb_sys_timerfd_create; */
> + /* case 86: return gdb_sys_timerfd_settime; */
> + /* case 87: return gdb_sys_timerfd_gettime; */
> + /* case 88: return gdb_sys_utimensat; */
> + case 89: return gdb_sys_acct;
> + case 90: return gdb_sys_capget;
> + case 91: return gdb_sys_capset;
> + case 92: return gdb_sys_personality;
> + case 93: return gdb_sys_exit;
> + case 94: return gdb_sys_exit_group;
> + case 95: return gdb_sys_waitid;
> + case 96: return gdb_sys_set_tid_address;
> + case 97: return gdb_sys_unshare;
> + case 98: return gdb_sys_futex;
> + case 99: return gdb_sys_set_robust_list;
> + case 100: return gdb_sys_get_robust_list;
> + case 101: return gdb_sys_nanosleep;
> + case 102: return gdb_sys_getitimer;
> + case 103: return gdb_sys_setitimer;
> + case 104: return gdb_sys_kexec_load;
> + case 105: return gdb_sys_init_module;
> + case 106: return gdb_sys_delete_module;
> + case 107: return gdb_sys_timer_create;
> + case 108: return gdb_sys_timer_gettime;
> + case 109: return gdb_sys_timer_getoverrun;
> + case 110: return gdb_sys_timer_settime;
> + case 111: return gdb_sys_timer_delete;
> + case 112: return gdb_sys_clock_settime;
> + case 113: return gdb_sys_clock_gettime;
> + case 114: return gdb_sys_clock_getres;
> + case 115: return gdb_sys_clock_nanosleep;
> + case 116: return gdb_sys_syslog;
> + case 117: return gdb_sys_ptrace;
> + case 118: return gdb_sys_sched_setparam;
> + case 119: return gdb_sys_sched_setscheduler;
> + case 120: return gdb_sys_sched_getscheduler;
> + case 121: return gdb_sys_sched_getparam;
> + case 122: return gdb_sys_sched_setaffinity;
> + case 123: return gdb_sys_sched_getaffinity;
> + case 124: return gdb_sys_sched_yield;
> + case 125: return gdb_sys_sched_get_priority_max;
> + case 126: return gdb_sys_sched_get_priority_min;
> + case 127: return gdb_sys_sched_rr_get_interval;
> + case 128: return gdb_sys_restart_syscall;
> + case 129: return gdb_sys_kill;
> + case 130: return gdb_sys_tkill;
> + case 131: return gdb_sys_tgkill;
> + case 132: return gdb_sys_sigaltstack;
> + case 133: return gdb_sys_rt_sigsuspend;
> + case 134: return gdb_sys_rt_sigaction;
> + case 135: return gdb_sys_rt_sigprocmask;
> + case 136: return gdb_sys_rt_sigpending;
> + case 137: return gdb_sys_rt_sigtimedwait;
> + case 138: return gdb_sys_rt_sigqueueinfo;
> + case 139: return gdb_sys_rt_sigreturn;
> + case 140: return gdb_sys_setpriority;
> + case 141: return gdb_sys_getpriority;
> + case 142: return gdb_sys_reboot;
> + case 143: return gdb_sys_setregid;
> + case 144: return gdb_sys_setgid;
> + case 145: return gdb_sys_setreuid;
> + case 146: return gdb_sys_setuid;
> + case 147: return gdb_sys_setresuid;
> + case 148: return gdb_sys_getresuid;
> + case 149: return gdb_sys_setresgid;
> + case 150: return gdb_sys_getresgid;
> + case 151: return gdb_sys_setfsuid;
> + case 152: return gdb_sys_setfsgid;
> + case 153: return gdb_sys_times;
> + case 154: return gdb_sys_setpgid;
> + case 155: return gdb_sys_getpgid;
> + case 156: return gdb_sys_getsid;
> + case 157: return gdb_sys_setsid;
> + case 158: return gdb_sys_getgroups;
> + case 159: return gdb_sys_setgroups;
> + case 160: return gdb_sys_uname;
> + case 161: return gdb_sys_sethostname;
> + case 162: return gdb_sys_setdomainname;
> + case 163: return gdb_sys_getrlimit;
> + case 164: return gdb_sys_setrlimit;
> + case 165: return gdb_sys_getrusage;
> + case 166: return gdb_sys_umask;
> + case 167: return gdb_sys_prctl;
> + case 168: return gdb_sys_getcpu;
> + case 169: return gdb_sys_gettimeofday;
> + case 170: return gdb_sys_settimeofday;
> + case 171: return gdb_sys_adjtimex;
> + case 172: return gdb_sys_getpid;
> + case 173: return gdb_sys_getppid;
> + case 174: return gdb_sys_getuid;
> + case 175: return gdb_sys_geteuid;
> + case 176: return gdb_sys_getgid;
> + case 177: return gdb_sys_getegid;
> + case 178: return gdb_sys_gettid;
> + case 179: return gdb_sys_sysinfo;
> + case 180: return gdb_sys_mq_open;
> + case 181: return gdb_sys_mq_unlink;
> + case 182: return gdb_sys_mq_timedsend;
> + case 183: return gdb_sys_mq_timedreceive;
> + case 184: return gdb_sys_mq_notify;
> + case 185: return gdb_sys_mq_getsetattr;
> + case 186: return gdb_sys_msgget;
> + case 187: return gdb_sys_msgctl;
> + case 188: return gdb_sys_msgrcv;
> + case 189: return gdb_sys_msgsnd;
> + case 190: return gdb_sys_semget;
> + case 191: return gdb_sys_semctl;
> + case 192: return gdb_sys_semtimedop;
> + case 193: return gdb_sys_semop;
> + case 194: return gdb_sys_shmget;
> + case 195: return gdb_sys_shmctl;
> + case 196: return gdb_sys_shmat;
> + case 197: return gdb_sys_shmdt;
> + case 198: return gdb_sys_socket;
> + case 199: return gdb_sys_socketpair;
> + case 200: return gdb_sys_bind;
> + case 201: return gdb_sys_listen;
> + case 202: return gdb_sys_accept;
> + case 203: return gdb_sys_connect;
> + case 204: return gdb_sys_getsockname;
> + case 205: return gdb_sys_getpeername;
> + case 206: return gdb_sys_sendto;
> + case 207: return gdb_sys_recvfrom;
> + case 208: return gdb_sys_setsockopt;
> + case 209: return gdb_sys_getsockopt;
> + case 210: return gdb_sys_shutdown;
> + case 211: return gdb_sys_sendmsg;
> + case 212: return gdb_sys_recvmsg;
> + case 213: return gdb_sys_readahead;
> + case 214: return gdb_sys_brk;
> + case 215: return gdb_sys_munmap;
> + case 216: return gdb_sys_mremap;
> + case 217: return gdb_sys_add_key;
> + case 218: return gdb_sys_request_key;
> + case 219: return gdb_sys_keyctl;
> + case 220: return gdb_sys_clone;
> + case 221: return gdb_sys_execve;
> + case 222: return gdb_old_mmap;
> + case 223: return gdb_sys_fadvise64;
> + case 224: return gdb_sys_swapon;
> + case 225: return gdb_sys_swapoff;
> + case 226: return gdb_sys_mprotect;
> + case 227: return gdb_sys_msync;
> + case 228: return gdb_sys_mlock;
> + case 229: return gdb_sys_munlock;
> + case 230: return gdb_sys_mlockall;
> + case 231: return gdb_sys_munlockall;
> + case 232: return gdb_sys_mincore;
> + case 233: return gdb_sys_madvise;
> + case 234: return gdb_sys_remap_file_pages;
> + case 235: return gdb_sys_mbind;
> + case 236: return gdb_sys_get_mempolicy;
> + case 237: return gdb_sys_set_mempolicy;
> + case 238: return gdb_sys_migrate_pages;
> + case 239: return gdb_sys_move_pages;
> + /* case 240: return gdb_sys_rt_tgsigqueueinfo; */
> + /* case 241: return gdb_sys_perf_event_open; */
> + /* case 242: return gdb_sys_accept4; */
> + /* case 243: return gdb_sys_recvmmsg; */
> + /* case 258: return gdb_sys_riscv_hwprobe; */
> + /* case 259: return gdb_sys_riscv_flush_icache; */
> + case 260: return gdb_sys_wait4;
> + /* case 261: return gdb_sys_prlimit64; */
> + /* case 262: return gdb_sys_fanotify_init; */
> + /* case 263: return gdb_sys_fanotify_mark; */
> + /* case 264: return gdb_sys_name_to_handle_at; */
> + /* case 265: return gdb_sys_open_by_handle_at; */
> + /* case 266: return gdb_sys_clock_adjtime; */
> + /* case 267: return gdb_sys_syncfs; */
> + /* case 268: return gdb_sys_setns; */
> + /* case 269: return gdb_sys_sendmmsg; */
> + /* case 270: return gdb_sys_process_vm_readv; */
> + /* case 271: return gdb_sys_process_vm_writev; */
> + /* case 272: return gdb_sys_kcmp; */
> + /* case 273: return gdb_sys_finit_module; */
> + /* case 274: return gdb_sys_sched_setattr; */
> + /* case 275: return gdb_sys_sched_getattr; */
> + /* case 276: return gdb_sys_renameat2; */
> + /* case 277: return gdb_sys_seccomp; */
> + case 278: return gdb_sys_getrandom;
> + /* case 279: return gdb_sys_memfd_create; */
> + /* case 280: return gdb_sys_bpf; */
> + /* case 281: return gdb_sys_execveat; */
> + /* case 282: return gdb_sys_userfaultfd; */
> + /* case 283: return gdb_sys_membarrier; */
> + /* case 284: return gdb_sys_mlock2; */
> + /* case 285: return gdb_sys_copy_file_range; */
> + /* case 286: return gdb_sys_preadv2; */
> + /* case 287: return gdb_sys_pwritev2; */
> + /* case 288: return gdb_sys_pkey_mprotect; */
> + /* case 289: return gdb_sys_pkey_alloc; */
> + /* case 290: return gdb_sys_pkey_free; */
> + case 291: return gdb_sys_statx;
> + /* case 292: return gdb_sys_io_pgetevents; */
> + /* case 293: return gdb_sys_rseq; */
> + /* case 294: return gdb_sys_kexec_file_load; */
> + /* case 424: return gdb_sys_pidfd_send_signal; */
> + /* case 425: return gdb_sys_io_uring_setup; */
> + /* case 426: return gdb_sys_io_uring_enter; */
> + /* case 427: return gdb_sys_io_uring_register; */
> + /* case 428: return gdb_sys_open_tree; */
> + /* case 429: return gdb_sys_move_mount; */
> + /* case 430: return gdb_sys_fsopen; */
> + /* case 431: return gdb_sys_fsconfig; */
> + /* case 432: return gdb_sys_fsmount; */
> + /* case 433: return gdb_sys_fspick; */
> + /* case 434: return gdb_sys_pidfd_open; */
> + /* case 435: return gdb_sys_clone3; */
> + /* case 436: return gdb_sys_close_range; */
> + /* case 437: return gdb_sys_openat2; */
> + /* case 438: return gdb_sys_pidfd_getfd; */
> + /* case 439: return gdb_sys_faccessat2; */
> + /* case 440: return gdb_sys_process_madvise; */
> + /* case 441: return gdb_sys_epoll_pwait2; */
> + /* case 442: return gdb_sys_mount_setattr; */
> + /* case 443: return gdb_sys_quotactl_fd; */
> + /* case 444: return gdb_sys_landlock_create_ruleset; */
> + /* case 445: return gdb_sys_landlock_add_rule; */
> + /* case 446: return gdb_sys_landlock_restrict_self; */
> + /* case 447: return gdb_sys_memfd_secret; */
> + /* case 448: return gdb_sys_process_mrelease; */
> + /* case 449: return gdb_sys_futex_waitv; */
> + /* case 450: return gdb_sys_set_mempolicy_home_node; */
> + default:
> + return gdb_sys_no_syscall;
> + }
> +}
> diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
> index ff478cf4c28..ebaed9b4508 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,12 +178,262 @@ 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 first, regnum_type last)

I also missed this before, but all functions should have a comment
explaining what they do. It can be short, but it should be there.

There's a couple more instances where it is needed below.

> +{
> + gdb_assert (regcache != nullptr);
> +
> + for (regnum_type i = first; 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)
> +{
> + gdb_assert (regcache != nullptr);
> +
> + 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))
When indentation reaches 8 spaces, it should be changed to a tab
instead. There are a couple more instances in the patch as well
> + return false;
> +
> + return true;
> +}
> +
> +/* Handler for riscv system call instruction recording. */
> +
> +static int
> +riscv_linux_syscall_record (struct regcache *regcache,
> + unsigned long svc_number)
> +{
> + gdb_assert (regcache != nullptr);
> +
> + 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)
> +{
> + gdb_assert (gdbarch != nullptr);
> +
> + /* 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
> riscv_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> {
> - riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
> + auto tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
>
> linux_init_abi (info, gdbarch, 0);
>
> @@ -205,6 +460,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..37ec30fff91 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,653 @@ this option can be used."),
> &setriscvcmdlist,
> &showriscvcmdlist);
> }
> +
> +static bool
> +try_read (struct regcache *regcache, int regnum, ULONGEST &addr)
> +{
> + gdb_assert (regcache != nullptr);
> +
> + 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 != nullptr);
> +
> + if (!try_read (regcache, reg, addr))
> + return set_error ();
> + return true;
> + }
> +
> + bool
> + save_reg (regnum_type regnum) noexcept
> + {
> + try
> + {
> + m_regs.emplace_back (regnum);
> + 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 != nullptr);
> +
> + auto addr = mem_addr{};
> + auto len = need_save_pc_mem (ival);
> + if (len <= 0)
> + return false;
> +
> + auto offset = mem_len{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 != nullptr);
> +
> + auto len = need_save_pc_rd_mem (ival);
> + auto addr = mem_addr{};
> + 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 != nullptr);
> +
> + auto addr = mem_addr{};
> + 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 != nullptr);
> +
> + 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 ();
> +
> + 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 != nullptr);
> + gdb_assert (gdbarch != nullptr);
> +
> + auto addr = mem_addr{};
> + 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)))
> + {
> + auto offset = int{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))
> + {
> + auto offset = int{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)))
> + {
> + auto offset = int{EXTRACT_CSSTYPE_SDSP_IMM (ival)};

When trying to compile this patch, this line caused compilation errors,
due to "narrowing conversion". It should use ULONGEST instead of int.

Also, we try to restrict the usage of "auto", so that's it's easier to
understand things line-by-line instead of needing to double check with
something like a variable or function declaration. Did you have any
issues with using:

ULONGEST offset = EXTRACT_CSSTYPE_SDSP_IMM (ival);

?

> + return !read_reg (regcache, RISCV_SP_REGNUM, addr)
> + || !save_mem (addr + offset, 8) || set_ordinary_record_type ();

I'm not sure I like the idea of using conditional execution due to
logical operands. It feels much more bash-y than C, I'd prefer if you
used if conditions, or exceptions.

However, this is personal preference, and I don't think I'll be touching
this file often, so if a RISC-V or global maintainer is ok with it, feel
free to ignore this comment.

> + }
> +
> + if (is_c_swsp_insn (ival) || (xlen == 4 && is_c_fswsp_insn (ival)))
> + {
> + auto offset = int{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
> + {
> + gdb_assert (gdbarch != nullptr);
> + gdb_assert (regcache != nullptr);
> +
> + auto m_length = int{};
> + 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 != nullptr);
> + gdb_assert (regcache != nullptr);
> +
> + auto 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;
> + }
> + auto reg_val = ULONGEST{};
> + 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 != nullptr);
> + gdb_assert (regcache != nullptr);
> +
> + riscv_recorded_insn insn;
> + if (!insn.record (gdbarch, regcache, addr))
> + {
> + record_full_arch_list_add_end ();
> + return -1;
> + }
> +
> + auto ret_val = riscv_make_record_process (gdbarch, regcache, insn);
> +
> + if (record_full_arch_list_add_end ())
> + return -1;
> +
> + return ret_val;
> +}
> diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
> index 4bdc2e7a3d5..505371ba4f6 100644
> --- a/gdb/riscv-tdep.h
> +++ b/gdb/riscv-tdep.h
> @@ -35,6 +35,10 @@ enum
> RISCV_FP_REGNUM = 8, /* Frame Pointer. */
> RISCV_A0_REGNUM = 10, /* First argument. */
> RISCV_A1_REGNUM = 11, /* Second argument. */
> + RISCV_A2_REGNUM = 12, /* Third argument. */
> + RISCV_A3_REGNUM = 13, /* Forth argument. */
> + RISCV_A4_REGNUM = 14, /* Fifth argument. */
> + RISCV_A5_REGNUM = 15, /* Sixth argument. */
> RISCV_A7_REGNUM = 17, /* Seventh argument. */
> RISCV_PC_REGNUM = 32, /* Program Counter. */
>
> @@ -113,6 +117,11 @@ struct riscv_gdbarch_tdep : gdbarch_tdep_base
> /* Return the expected next PC assuming FRAME is stopped at a syscall
> instruction. */
> CORE_ADDR (*syscall_next_pc) (const frame_info_ptr &frame) = nullptr;
> +
> + /* Syscall record. */
> + int (*riscv_syscall_record) (struct regcache *regcache,
> + unsigned long svc_number)
> + = nullptr;
> };
>
>
> @@ -177,6 +186,12 @@ extern void riscv_supply_regset (const struct regset *regset,
> struct regcache *regcache, int regnum,
> const void *regs, size_t len);
>
> +/* Parse the current instruction, and record the values of the
> + registers and memory that will be changed by the current
> + instruction. Returns -1 if something goes wrong, 0 otherwise. */
> +extern int riscv_process_record (struct gdbarch *gdbarch,
> + struct regcache *regcache, CORE_ADDR addr);
> +
> /* The names of the RISC-V target description features. */
> extern const char *riscv_feature_name_csr;
>
> diff --git a/gdb/syscalls/riscv-canonicalize-syscall-gen.py b/gdb/syscalls/riscv-canonicalize-syscall-gen.py
> new file mode 100644
> index 00000000000..333e5d60c07
> --- /dev/null
> +++ b/gdb/syscalls/riscv-canonicalize-syscall-gen.py
> @@ -0,0 +1,143 @@
> +#!/usr/bin/env python3
> +# pylint: disable=invalid-name
> +
> +# Copyright (C) 2024 Free Software Foundation, Inc.
> +# Contributed by Timur Golubovich
> +
> +# 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/>.
> +
> +import argparse
> +import re
> +import sys
> +from pathlib import Path as _Path
> +
> +head = """\
> +/* DO NOT EDIT: Autogenerated by riscv-canonicalize-syscall-gen.py
> +
> + 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/>. */
> +
> +#include "defs.h"
> +#include "riscv-linux-tdep.h"
> +
> +enum gdb_syscall
> +riscv64_canonicalize_syscall (int syscall)
> +{
> + switch (syscall)
> + {
> +"""
> +
> +tail = """\
> + default:
> + return gdb_sys_no_syscall;
> + }
> +}
> +"""
> +
> +
> +class Generator:
> + def _get_gdb_syscalls(self, gdb_syscalls_path: _Path) -> list[str]:
> + gdb_syscalls: list[str] = []
> + with open(gdb_syscalls_path, "r", encoding="UTF-8") as file:
> + lines = file.readlines()
> + for line in lines:
> + match = re.search(r"\s*(?P<name>gdb_sys_[^S]+)\S*=", line)
> + if match:
> + gdb_syscalls.append(match.group("name").strip())
> + return gdb_syscalls
> +
> + def _get_canon_syscalls_lines(self, syscalls_path: _Path, gdb_syscalls: list[str]) -> list[str]:
> + canon_syscalls: dict[int, str] = {}
> + with open(syscalls_path, "r", encoding="UTF-8") as file:
> + lines = file.readlines()
> + for line in lines:
> + match = re.match(r"#define\s+__NR_(?P<name>[^\s]+)\s+(?P<number>\d+)", line)
> + if match:
> + syscall_name = match.group("name")
> + syscall_num = int(match.group("number"))
> + gdb_syscall_name = f"gdb_sys_{syscall_name}"
> + if gdb_syscall_name in gdb_syscalls:
> + canon_syscalls[syscall_num] = (
> + f" case {syscall_num}: return {gdb_syscall_name};\n"
> + )
> + # this is a place for corner cases
> + elif syscall_name == "mmap":
> + gdb_old_syscall_name = "gdb_old_mmap"
> + canon_syscalls[syscall_num] = f" case {syscall_num}: return {gdb_old_syscall_name};\n"
> + else:
> + canon_syscalls[syscall_num] = f" /* case {syscall_num}: return {gdb_syscall_name}; */\n"
> + return [canon_syscalls[syscall_num] for syscall_num in sorted(canon_syscalls)]
> +
> + def generate(self, syscalls_path: _Path) -> None:
> + repo_path = _Path(__file__).parent.parent.parent
> + gdb_syscalls_path = repo_path / "gdb" / "linux-record.h"
> + canon_syscalls_path = repo_path / "gdb" / "riscv-canonicalize-syscall-gen.c"
> +
> + gdb_syscalls = self._get_gdb_syscalls(gdb_syscalls_path)
> + canon_syscalls_lines = self._get_canon_syscalls_lines(syscalls_path, gdb_syscalls)
> +
> + with open(canon_syscalls_path, "w", encoding="UTF-8") as file:
> + file.writelines(head)
> + file.writelines(canon_syscalls_lines)
> + file.writelines(tail)
> +
> +
> +help_message = """\
> +Generate file gdb/riscv-canonicalize-syscall-gen.c
> +from path to riscv linux syscalls.
> +"""
> +
> +
> +def setup_parser() -> argparse.ArgumentParser:
> + parser = argparse.ArgumentParser(description=help_message)
> + parser.add_argument(
> + "-i",
> + "--input",
> + type=_Path,
> + required=True,
> + help="path to riscv linux syscalls (riscv-glibc/sysdeps/unix/sysv/linux/riscv/rv64/arch-syscall.h)",
> + )
> + return parser
> +
> +
> +def main(argv: list[str]) -> int:
> + try:
> + parser = setup_parser()
> + args = parser.parse_args(argv)
> + generator = Generator()
> + generator.generate(args.input)
> + return 0
> + except RuntimeError as e:
> + print(str(e))
> + return -1
> +
> +
> +if __name__ == "__main__":
> + sys.exit(main(sys.argv[1:]))
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 410f99e3350..b048ac7078b 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -3718,7 +3718,8 @@ proc supports_reverse {} {
> || [istarget "i\[34567\]86-*-linux*"]
> || [istarget "aarch64*-*-linux*"]
> || [istarget "powerpc*-*-linux*"]
> - || [istarget "s390*-*-linux*"] } {
> + || [istarget "s390*-*-linux*"]
> + || [istarget "riscv*-*-*"] } {
> return 1
> }
>
> --
> 2.34.1
>

--
Cheers,
Guinevere Larsen
She/Her/Hers