Re: [PATCH v2 1/6] gdb/record: Refactor record history

[Guinevere Larsen <guinevere@redhat.com>]

On 5/6/26 6:16 PM, Schimpe, Christina wrote:
> Hi Guinevere,
>
> Thank you for working on this.
> I found two items that might be worth looking at in more detail, but
> apart from that it’s mostly minor nits. Please find my feedback below.
>
> I don’t have much experience with the record feature, so my review is
> a bit slow here. To fully understand this patch, I probably need more time,
> but I wanted to share the comments I already have. Sorry for chiming in
> so late. I’ll try again tomorrow, or by Friday at the latest.

Hi! Thank you for noting all the inconsistencies, I'll fix almost all of 
them before pushing!

The one thing I don't think it's a good idea is to make the change from 
record_full_list -> record_full_log in this commit... mostly because it 
will introduce untold amounts of rebase conflicts. I'll introduce a 
following commit to this series that will make that renaming, though :)

-- 
Cheers,
Guinevere Larsen
It/she

>> -----Original Message-----
>> From: Guinevere Larsen <guinevere@redhat.com>
>> Sent: Donnerstag, 23. April 2026 17:26
>> To: gdb-patches@sourceware.org
>> Cc: Guinevere Larsen <guinevere@redhat.com>
>> Subject: [PATCH v2 1/6] gdb/record: Refactor record history
>>
>> This is the first step in a large refactor in how GDB keeps execution
>> history.  Rather than using a linked list where multiple entries can
>> describe a single instruction, the history will now be stored in an
>> std::deque, each instruction being one entry in the deque.
>>
>> The choice was initially to use an std::vector, but it would become
>> unwieldy because it needs all the memory to be consecutive, which is
>> hard for 200 thousand entries. Deque was picked because it was a nice
>> midpoint between vector (maximum cache cohesion) and linked list
>> (maximum ease of finding space to store more).
>>
>> Each instruction in memory will be now one record_full_instruction
>> entry, which for this commit just contains a vector of
>> record_full_entry for the effects of the instruction, and the data that
>> was stored in the record_full_end entry (that is, the instruction number
>> and the signal, if any).
>>
>> This change introduced a minimal performance improvement (what's
>> important is that it isn't a degradation) and a  reduction in the total
>> memory footprint of roughly 20% if the entire history is used.
>> ---
>>   gdb/aarch64-tdep.c     |    2 -
>>   gdb/amd64-linux-tdep.c |    3 -
>>   gdb/arm-tdep.c         |    2 -
>>   gdb/i386-linux-tdep.c  |    3 -
>>   gdb/i386-tdep.c        |    4 -
>>   gdb/loongarch-tdep.c   |    2 -
>>   gdb/moxie-tdep.c       |    2 -
>>   gdb/ppc-linux-tdep.c   |    3 -
>>   gdb/record-full.c      | 1180 ++++++++++++++++------------------------
>>   gdb/record-full.h      |    1 -
>>   gdb/riscv-tdep.c       |    3 -
>>   gdb/rs6000-tdep.c      |    4 -
>>   gdb/s390-linux-tdep.c  |    3 -
>>   gdb/s390-tdep.c        |    2 -
>>   14 files changed, 476 insertions(+), 738 deletions(-)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 4befaa2720d..a532992b1d8 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -6210,8 +6210,6 @@ aarch64_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>   	       aarch64_record.aarch64_mems[rec_no].len))
>>   	    ret = -1;
>>
>> -      if (record_full_arch_list_add_end ())
>> -	ret = -1;
>>       }
>>
>>     deallocate_reg_mem (&aarch64_record);
>> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
>> index a5ac26654cf..9b23db72bbe 100644
>> --- a/gdb/amd64-linux-tdep.c
>> +++ b/gdb/amd64-linux-tdep.c
>> @@ -1591,9 +1591,6 @@ amd64_linux_record_signal (struct gdbarch
>> *gdbarch,
>>   				     + AMD64_LINUX_frame_size))
>>       return -1;
>>
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>> -
>>     return 0;
>>   }
>>
>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>> index 08cce9dbad8..1ce0927f26b 100644
>> --- a/gdb/arm-tdep.c
>> +++ b/gdb/arm-tdep.c
>> @@ -14911,8 +14911,6 @@ arm_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>   	    }
>>   	}
>>
>> -      if (record_full_arch_list_add_end ())
>> -	ret = -1;
>>       }
>>
>>
>> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
>> index 23aeccac9fc..4f33766fcdd 100644
>> --- a/gdb/i386-linux-tdep.c
>> +++ b/gdb/i386-linux-tdep.c
>> @@ -949,9 +949,6 @@ i386_linux_record_signal (struct gdbarch *gdbarch,
>>   				     I386_LINUX_xstate +
>> I386_LINUX_frame_size))
>>       return -1;
>>
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>> -
>>     return 0;
>>   }
>>   
>>
>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
>> index fa935b5fcdb..fee36b46a73 100644
>> --- a/gdb/i386-tdep.c
>> +++ b/gdb/i386-tdep.c
>> @@ -5201,8 +5201,6 @@ i386_record_vex (struct i386_record_s *ir, uint8_t
>> vex_w, uint8_t vex_r,
>>       }
>>
>>     record_full_arch_list_add_reg (ir->regcache, ir-
>>> regmap[X86_RECORD_REIP_REGNUM]);
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>>
>>     return 0;
>>   }
>> @@ -8359,8 +8357,6 @@ Do you want to stop the program?"),
>>
>>     /* In the future, maybe still need to deal with need_dasm.  */
>>     I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REIP_REGNUM);
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>>
>>     return 0;
>>
>> diff --git a/gdb/loongarch-tdep.c b/gdb/loongarch-tdep.c
>> index 225b1abb703..5ec6b2a48d0 100644
>> --- a/gdb/loongarch-tdep.c
>> +++ b/gdb/loongarch-tdep.c
>> @@ -2789,8 +2789,6 @@ loongarch_process_record (struct gdbarch
>> *gdbarch, struct regcache *regcache,
>>   					 LOONGARCH_PC_REGNUM))
>>   	return -1;
>>
>> -      if (record_full_arch_list_add_end ())
>> -	return -1;
>>       }
>>
>>     return ret;
>> diff --git a/gdb/moxie-tdep.c b/gdb/moxie-tdep.c
>> index c8a04d21f59..2f8f0186488 100644
>> --- a/gdb/moxie-tdep.c
>> +++ b/gdb/moxie-tdep.c
>> @@ -1040,8 +1040,6 @@ moxie_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>
>>     if (record_full_arch_list_add_reg (regcache, MOXIE_PC_REGNUM))
>>       return -1;
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>>     return 0;
>>   }
>>
>> diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
>> index 19698eaacbe..e2f53551709 100644
>> --- a/gdb/ppc-linux-tdep.c
>> +++ b/gdb/ppc-linux-tdep.c
>> @@ -1565,9 +1565,6 @@ ppc_linux_record_signal (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>     if (record_full_arch_list_add_mem (sp, SIGNAL_FRAMESIZE +
>> sizeof_rt_sigframe))
>>       return -1;
>>
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>> -
>>     return 0;
>>   }
>>
>> diff --git a/gdb/record-full.c b/gdb/record-full.c
>> index 69d2c100e57..aac69cee17e 100644
>> --- a/gdb/record-full.c
>> +++ b/gdb/record-full.c
> I think we should include vector, optional and deque directly.
>
>> @@ -72,20 +72,17 @@
>>   #define DEFAULT_RECORD_FULL_INSN_MAX_NUM	200000
>>
>>   #define RECORD_FULL_IS_REPLAY \
>> -  (record_full_list->next || ::execution_direction == EXEC_REVERSE)
>> +  ( (record_full_next_insn != record_full_list.size ()) \
> Nit: No whitespace after the first bracket:
> => ((record_full_next_insn != record_full_list.size ()) \
>
>> +     || ::execution_direction == EXEC_REVERSE)
>>
>>   #define RECORD_FULL_FILE_MAGIC	netorder32(0x20091016)
>>
>>   /* These are the core structs of the process record functionality.
>>
>>      A record_full_entry is a record of the value change of a register
>> -   ("record_full_reg") or a part of memory ("record_full_mem").  And each
>> -   instruction must have a struct record_full_entry ("record_full_end")
>> -   that indicates that this is the last struct record_full_entry of this
>> -   instruction.
>> -
>> -   Each struct record_full_entry is linked to "record_full_list" by "prev"
>> -   and "next" pointers.  */
>> +   ("record_full_reg") or a part of memory ("record_full_mem").
>> +   These are saved on the record_full_instruction struct, which also
>> +   contains some extra information, such as delivered signals.  */
>>
>>   struct record_full_mem_entry
>>   {
>> @@ -112,47 +109,14 @@ struct record_full_reg_entry
>>     } u;
>>   };
>>
>> -struct record_full_end_entry
>> -{
>> -  enum gdb_signal sigval;
>> -  ULONGEST insn_num;
>> -};
>> -
>>   enum record_full_type
>>   {
>> -  record_full_end = 0,
>>     record_full_reg,
>>     record_full_mem
>>   };
>>
>> -/* This is the data structure that makes up the execution log.
>> -
>> -   The execution log consists of a single linked list of entries
>> -   of type "struct record_full_entry".  It is doubly linked so that it
>> -   can be traversed in either direction.
>> -
>> -   The start of the list is anchored by a struct called
>> -   "record_full_first".  The pointer "record_full_list" either points
>> -   to the last entry that was added to the list (in record mode), or to
>> -   the next entry in the list that will be executed (in replay mode).
>> -
>> -   Each list element (struct record_full_entry), in addition to next
>> -   and prev pointers, consists of a union of three entry types: mem,
>> -   reg, and end.  A field called "type" determines which entry type is
>> -   represented by a given list element.
>> -
>> -   Each instruction that is added to the execution log is represented
>> -   by a variable number of list elements ('entries').  The instruction
>> -   will have one "reg" entry for each register that is changed by
>> -   executing the instruction (including the PC in every case).  It
>> -   will also have one "mem" entry for each memory change.  Finally,
>> -   each instruction will have an "end" entry that separates it from
>> -   the changes associated with the next instruction.  */
>> -
>>   struct record_full_entry
>>   {
>> -  struct record_full_entry *prev;
>> -  struct record_full_entry *next;
>>     enum record_full_type type;
>>     union
>>     {
>> @@ -160,11 +124,31 @@ struct record_full_entry
>>       struct record_full_reg_entry reg;
>>       /* mem */
>>       struct record_full_mem_entry mem;
>> -    /* end */
>> -    struct record_full_end_entry end;
>>     } u;
>>   };
>>
>> +/* This is the main structure that comprises the execution log.
>> +   Each instruction is comprised of:
>> +   * The instruction number: How many instructions were recorded before
>> +     this one;
>> +   * sigval: Whether the inferior received a signal while the following
>> +     instruction was being recorded;
>> +   * effects: A list of record_full_entry structures, each of which
>> +     describing one effect that the instruction has on the inferior.
>> +
>> +   Note, the signal is stored in the previous instruction for historical
>> +   reasons.  This is how it was first implemented, and no one has gotten
>> +   around to changing it yet.  */
>> +
>> +struct record_full_instruction
>> +{
>> +  /* This might be different from the index if
>> +     we had to remove the first few instructions.  */
>> +  uint32_t insn_num;
>> +  std::optional<gdb_signal> sigval;
>> +  std::vector<record_full_entry> effects;
>> +};
>> +
>>   /* If true, query if PREC cannot record memory
>>      change of next instruction.  */
>>   bool record_full_memory_query = false;
>> @@ -181,27 +165,25 @@ static detached_regcache
>> *record_full_core_regbuf = NULL;
>>   static std::vector<target_section> record_full_core_sections;
>>   static struct record_full_core_buf_entry *record_full_core_buf_list = NULL;
>>
>> -/* The following variables are used for managing the linked list that
>> -   represents the execution log.
>> +/* The following variables are used for managing the history of executed
>> +   instructions from the inferior.
>>
>> -   record_full_first is the anchor that holds down the beginning of
>> -   the list.
>> +   record_full_list contains all instructions that were fully executed and
>> +   saved to the log, so that we can replay the execution.
>>
>> -   record_full_list serves two functions:
>> -     1) In record mode, it anchors the end of the list.
>> -     2) In replay mode, it traverses the list and points to
>> -	the next instruction that must be emulated.
>> +   record_full_next_insn always points to the next instruction that would
>> +   be executed if the inferior executes forward.  In the special case when
>> +   the inferior is not replaying, record_full_next_insn points past the
>> +   end of the history.
>>
>> -   record_full_arch_list_head and record_full_arch_list_tail are used
>> -   to manage a separate list, which is used to build up the change
>> -   elements of the currently executing instruction during record mode.
>> -   When this instruction has been completely annotated in the "arch
>> -   list", it will be appended to the main execution log.  */
>> +   record_full_incomplete_instruction holds a partial instruction, while
>> +   the lower target is disassembling the instruction, or as partial xfers are
>> +   happening.  It is manipulated by the "arch list" functions for historical
>> +   reasons.  */
>>
>> -static struct record_full_entry record_full_first;
>> -static struct record_full_entry *record_full_list = &record_full_first;
>> -static struct record_full_entry *record_full_arch_list_head = NULL;
>> -static struct record_full_entry *record_full_arch_list_tail = NULL;
>> +static std::deque<record_full_instruction> record_full_list;
> Given that this is not a list anymore I wonder if we should better call this sth. like
> record_full_history or record_full_log or even simply recorded_instructions?
>
>> +static record_full_instruction record_full_incomplete_instruction;
>> +int record_full_next_insn;
>>
>>   /* true ask user. false auto delete the last struct record_full_entry.  */
>>   static bool record_full_stop_at_limit = true;
>> @@ -361,6 +343,14 @@ record_full_target::kill ()
>>     record_kill (this);
>>   }
>>
>> +static void
>> +record_full_reset_incomplete ()
>> +{
>> +  record_full_incomplete_instruction.effects.clear ();
>> +  record_full_incomplete_instruction.sigval.reset ();
>> +  record_full_incomplete_instruction.insn_num = 0;
>> +}
>> +
>>   /* See record-full.h.  */
>>
>>   int
>> @@ -390,156 +380,120 @@ static struct cmd_list_element
>> *show_record_full_cmdlist;
>>   /* Command list for "record full".  */
>>   static struct cmd_list_element *record_full_cmdlist;
>>
>> -static void record_full_goto_insn (struct record_full_entry *entry,
>> +static void record_full_goto_insn (size_t target_insn,
>>   				   enum exec_direction_kind dir);
>>
>> -/* Alloc and free functions for record_full_reg, record_full_mem, and
>> -   record_full_end entries.  */
>> +/* Initialization and cleanup functions for record_full_reg and
>> +   record_full_mem entries.  */
>>
>> -/* Alloc a record_full_reg record entry.  */
>> +/* Init a record_full_reg record entry.  */
>>
>> -static inline struct record_full_entry *
>> -record_full_reg_alloc (struct regcache *regcache, int regnum)
>> +static inline struct record_full_entry
>> +record_full_reg_init (struct regcache *regcache, int regnum)
>>   {
>> -  struct record_full_entry *rec;
>> +  struct record_full_entry rec;
>>     struct gdbarch *gdbarch = regcache->arch ();
>>
>> -  rec = XCNEW (struct record_full_entry);
>> -  rec->type = record_full_reg;
>> -  rec->u.reg.num = regnum;
>> -  rec->u.reg.len = register_size (gdbarch, regnum);
>> -  if (rec->u.reg.len > sizeof (rec->u.reg.u.buf))
>> -    rec->u.reg.u.ptr = (gdb_byte *) xmalloc (rec->u.reg.len);
>> +  rec.type = record_full_reg;
>> +  rec.u.reg.num = regnum;
>> +  rec.u.reg.len = register_size (gdbarch, regnum);
>> +  if (rec.u.reg.len > sizeof (rec.u.reg.u.buf))
>> +    rec.u.reg.u.ptr = (gdb_byte *) xmalloc (rec.u.reg.len);
>>
>>     return rec;
>>   }
>>
>> -/* Free a record_full_reg record entry.  */
>> +/* Cleanup a record_full_reg record entry.  */
>>
>>   static inline void
>> -record_full_reg_release (struct record_full_entry *rec)
>> +record_full_reg_cleanup (struct record_full_entry rec)
>>   {
>> -  gdb_assert (rec->type == record_full_reg);
>> -  if (rec->u.reg.len > sizeof (rec->u.reg.u.buf))
>> -    xfree (rec->u.reg.u.ptr);
>> -  xfree (rec);
>> +  gdb_assert (rec.type == record_full_reg);
>> +  if (rec.u.reg.len > sizeof (rec.u.reg.u.buf))
>> +    xfree (rec.u.reg.u.ptr);
>>   }
>>
>> -/* Alloc a record_full_mem record entry.  */
>> +/* Init a record_full_mem record entry.  */
>>
>> -static inline struct record_full_entry *
>> -record_full_mem_alloc (CORE_ADDR addr, int len)
>> +static inline struct record_full_entry
>> +record_full_mem_init (CORE_ADDR addr, int len)
>>   {
>> -  struct record_full_entry *rec;
>> +  struct record_full_entry rec;
>>
>> -  rec = XCNEW (struct record_full_entry);
>> -  rec->type = record_full_mem;
>> -  rec->u.mem.addr = addr;
>> -  rec->u.mem.len = len;
>> -  if (rec->u.mem.len > sizeof (rec->u.mem.u.buf))
>> -    rec->u.mem.u.ptr = (gdb_byte *) xmalloc (len);
>> +  rec.type = record_full_mem;
>> +  rec.u.mem.addr = addr;
>> +  rec.u.mem.len = len;
>> +  if (rec.u.mem.len > sizeof (rec.u.mem.u.buf))
>> +    rec.u.mem.u.ptr = (gdb_byte *) xmalloc (len);
>> +  rec.u.mem.mem_entry_not_accessible = 0;
>>
>>     return rec;
>>   }
>>
>> -/* Free a record_full_mem record entry.  */
>> +/* Cleanup a record_full_mem record entry.  */
>>
>>   static inline void
>> -record_full_mem_release (struct record_full_entry *rec)
>> -{
>> -  gdb_assert (rec->type == record_full_mem);
>> -  if (rec->u.mem.len > sizeof (rec->u.mem.u.buf))
>> -    xfree (rec->u.mem.u.ptr);
>> -  xfree (rec);
>> -}
>> -
>> -/* Alloc a record_full_end record entry.  */
>> -
>> -static inline struct record_full_entry *
>> -record_full_end_alloc (void)
>> +record_full_mem_cleanup (struct record_full_entry rec)
>>   {
>> -  struct record_full_entry *rec;
>> -
>> -  rec = XCNEW (struct record_full_entry);
>> -  rec->type = record_full_end;
>> -
>> -  return rec;
>> +  gdb_assert (rec.type == record_full_mem);
>> +  if (rec.u.mem.len > sizeof (rec.u.mem.u.buf))
>> +    xfree (rec.u.mem.u.ptr);
>>   }
>>
>> -/* Free a record_full_end record entry.  */
>> +/* Free one record entry, any type.  */
>>
>>   static inline void
>> -record_full_end_release (struct record_full_entry *rec)
>> -{
>> -  xfree (rec);
>> -}
>> -
>> -/* Free one record entry, any type.
>> -   Return entry->type, in case caller wants to know.  */
>> -
>> -static inline enum record_full_type
>> -record_full_entry_release (struct record_full_entry *rec)
>> +record_full_entry_cleanup (struct record_full_entry rec)
>>   {
>> -  enum record_full_type type = rec->type;
>>
>> -  switch (type) {
>> +  switch (rec.type) {
>>     case record_full_reg:
>> -    record_full_reg_release (rec);
>> +    record_full_reg_cleanup (rec);
>>       break;
>>     case record_full_mem:
>> -    record_full_mem_release (rec);
>> -    break;
>> -  case record_full_end:
>> -    record_full_end_release (rec);
>> +    record_full_mem_cleanup (rec);
>>       break;
>>     }
>> -  return type;
>>   }
>>
>> -/* Free all record entries in list pointed to by REC.  */
>> -
>>   static void
>> -record_full_list_release (struct record_full_entry *rec)
>> +record_full_reset_history ()
>>   {
>> -  if (!rec)
>> -    return;
>> -
>> -  while (rec->next)
>> -    rec = rec->next;
>> +  record_full_insn_num = 0;
>> +  record_full_insn_count = 0;
>> +  record_full_next_insn = 0;
>>
>> -  while (rec->prev)
>> +  for (auto &insn : record_full_list)
>>       {
>> -      rec = rec->prev;
>> -      record_full_entry_release (rec->next);
>> +      for (auto &entry : insn.effects)
>> +	record_full_entry_cleanup (entry);
>>       }
>>
>> -  if (rec == &record_full_first)
>> -    {
>> -      record_full_insn_num = 0;
>> -      record_full_first.next = NULL;
>> -    }
>> -  else
>> -    record_full_entry_release (rec);
>> +  record_full_list.clear ();
>>   }
>>
>> -/* Free all record entries forward of the given list position.  */
>> -
>>   static void
>> -record_full_list_release_following (struct record_full_entry *rec)
>> +record_full_list_release_following (int index)
>>   {
>> -  struct record_full_entry *tmp = rec->next;
>> -
>> -  rec->next = NULL;
>> -  while (tmp)
>> +  for (int i = record_full_list.size () - 1; i > index; i--)
>>       {
>> -      rec = tmp->next;
>> -      if (record_full_entry_release (tmp) == record_full_end)
>> -	{
>> -	  record_full_insn_num--;
>> -	  record_full_insn_count--;
>> -	}
>> -      tmp = rec;
>> +      for (auto &entry : record_full_list[i].effects)
>> +	record_full_entry_cleanup (entry);
>> +      record_full_list.pop_back ();
>>       }
>> +  /* Set the next instruction to be past the end of the log so we
>> +     start recording if the user moves forward again.  */
>> +  record_full_next_insn = index;
>> +}
>> +
>> +static void
>> +record_full_save_instruction ()
>> +{
>> +  ++record_full_insn_count;
>> +  record_full_incomplete_instruction.insn_num = record_full_insn_count;
>> +  record_full_incomplete_instruction.effects.shrink_to_fit ();
>> +  record_full_list.push_back (record_full_incomplete_instruction);
>> +  record_full_next_insn++;
>>   }
>>
>>   /* Delete the first instruction from the beginning of the log, to make
>> @@ -550,52 +504,22 @@ record_full_list_release_following (struct
>> record_full_entry *rec)
>>   static void
>>   record_full_list_release_first (void)
>>   {
>> -  struct record_full_entry *tmp;
>> -
>> -  if (!record_full_first.next)
>> +  if (record_full_list.empty ())
>>       return;
>>
>> -  /* Loop until a record_full_end.  */
>> -  while (1)
>> -    {
>> -      /* Cut record_full_first.next out of the linked list.  */
>> -      tmp = record_full_first.next;
>> -      record_full_first.next = tmp->next;
>> -      tmp->next->prev = &record_full_first;
>> -
>> -      /* tmp is now isolated, and can be deleted.  */
>> -      if (record_full_entry_release (tmp) == record_full_end)
>> -	break;	/* End loop at first record_full_end.  */
>> +  for (auto &entry : record_full_list[0].effects)
>> +    record_full_entry_cleanup (entry);
>>
>> -      if (!record_full_first.next)
>> -	{
>> -	  gdb_assert (record_full_insn_num == 1);
>> -	  break;	/* End loop when list is empty.  */
>> -	}
>> -    }
>> +  record_full_list.pop_front ();
>> +  --record_full_next_insn;
>>   }
>>
>>   /* Add a struct record_full_entry to record_full_arch_list.  */
>>
>>   static void
>> -record_full_arch_list_add (struct record_full_entry *rec)
>> +record_full_arch_list_add (struct record_full_entry &rec)
>>   {
>> -  if (record_debug > 1)
>> -    gdb_printf (gdb_stdlog,
>> -		"Process record: record_full_arch_list_add %s.\n",
>> -		host_address_to_string (rec));
>> -
>> -  if (record_full_arch_list_tail)
>> -    {
>> -      record_full_arch_list_tail->next = rec;
>> -      rec->prev = record_full_arch_list_tail;
>> -      record_full_arch_list_tail = rec;
>> -    }
>> -  else
>> -    {
>> -      record_full_arch_list_head = rec;
>> -      record_full_arch_list_tail = rec;
>> -    }
>> +  record_full_incomplete_instruction.effects.push_back (rec);
>>   }
>>
>>   /* Return the value storage location of a record entry.  */
>> @@ -613,7 +537,6 @@ record_full_get_loc (struct record_full_entry *rec)
>>         return rec->u.reg.u.ptr;
>>       else
>>         return rec->u.reg.u.buf;
>> -  case record_full_end:
>>     default:
>>       gdb_assert_not_reached ("unexpected record_full_entry type");
>>       return NULL;
>> @@ -625,7 +548,7 @@ record_full_get_loc (struct record_full_entry *rec)
>>   int
>>   record_full_arch_list_add_reg (struct regcache *regcache, int regnum)
>>   {
>> -  struct record_full_entry *rec;
>> +  struct record_full_entry rec;
> I think we can omit the struct keyword here (there are some more occurrences in this patch).
>
>>     if (record_debug > 1)
>>       gdb_printf (gdb_stdlog,
>> @@ -633,9 +556,9 @@ record_full_arch_list_add_reg (struct regcache
>> *regcache, int regnum)
>>   		"record list.\n",
>>   		regnum);
>>
>> -  rec = record_full_reg_alloc (regcache, regnum);
>> +  rec = record_full_reg_init (regcache, regnum);
>>
>> -  regcache->cooked_read (regnum, record_full_get_loc (rec));
>> +  regcache->cooked_read (regnum, record_full_get_loc (&rec));
>>
>>     record_full_arch_list_add (rec);
>>
>> @@ -648,7 +571,7 @@ record_full_arch_list_add_reg (struct regcache
>> *regcache, int regnum)
>>   int
>>   record_full_arch_list_add_mem (CORE_ADDR addr, int len)
>>   {
>> -  struct record_full_entry *rec;
>> +  struct record_full_entry rec;
>>
>>     if (record_debug > 1)
>>       gdb_printf (gdb_stdlog,
>> @@ -659,12 +582,12 @@ record_full_arch_list_add_mem (CORE_ADDR addr,
>> int len)
>>     if (!addr)	/* FIXME: Why?  Some arch must permit it...  */
>>       return 0;
>>
>> -  rec = record_full_mem_alloc (addr, len);
>> +  rec = record_full_mem_init (addr, len);
>>
>>     if (record_read_memory (current_inferior ()->arch (), addr,
>> -			  record_full_get_loc (rec), len))
>> +			  record_full_get_loc (&rec), len))
>>       {
>> -      record_full_mem_release (rec);
>> +      record_full_mem_cleanup (rec);
>>         return -1;
>>       }
>>
>> @@ -673,27 +596,6 @@ record_full_arch_list_add_mem (CORE_ADDR addr,
>> int len)
>>     return 0;
>>   }
>>
>> -/* Add a record_full_end type struct record_full_entry to
>> -   record_full_arch_list.  */
>> -
>> -int
>> -record_full_arch_list_add_end (void)
>> -{
>> -  struct record_full_entry *rec;
>> -
>> -  if (record_debug > 1)
>> -    gdb_printf (gdb_stdlog,
>> -		"Process record: add end to arch list.\n");
>> -
>> -  rec = record_full_end_alloc ();
>> -  rec->u.end.sigval = GDB_SIGNAL_0;
>> -  rec->u.end.insn_num = ++record_full_insn_count;
>> -
>> -  record_full_arch_list_add (rec);
>> -
>> -  return 0;
>> -}
>> -
>>   static void
>>   record_full_check_insn_num (void)
>>   {
>> @@ -725,8 +627,7 @@ record_full_message (struct regcache *regcache,
>> enum gdb_signal signal)
>>
>>     try
>>       {
>> -      record_full_arch_list_head = NULL;
>> -      record_full_arch_list_tail = NULL;
>> +      record_full_reset_incomplete ();
>>
>>         /* Check record_full_insn_num.  */
>>         record_full_check_insn_num ();
>> @@ -746,20 +647,16 @@ record_full_message (struct regcache *regcache,
>> enum gdb_signal signal)
>>   	 the user says something different, like "deliver this signal"
>>   	 during the replay mode).
>>
>> -	 User should understand that nothing he does during the replay
>> -	 mode will change the behavior of the child.  If he tries,
>> +	 User should understand that nothing they do during the replay
>> +	 mode will change the behavior of the child.  If they try,
>>   	 then that is a user error.
>>
>>   	 But we should still deliver the signal to gdb during the replay,
>>   	 if we delivered it during the recording.  Therefore we should
>>   	 record the signal during record_full_wait, not
>>   	 record_full_resume.  */
>> -      if (record_full_list != &record_full_first)  /* FIXME better way
>> -						      to check */
>> -	{
>> -	  gdb_assert (record_full_list->type == record_full_end);
>> -	  record_full_list->u.end.sigval = signal;
>> -	}
>> +      if (signal != GDB_SIGNAL_0)
>> +	record_full_list[record_full_next_insn - 1].sigval = signal;
>>
>>         if (signal == GDB_SIGNAL_0
>>   	  || !gdbarch_process_record_signal_p (gdbarch))
>> @@ -778,13 +675,11 @@ record_full_message (struct regcache *regcache,
>> enum gdb_signal signal)
>>       }
>>     catch (const gdb_exception &ex)
>>       {
>> -      record_full_list_release (record_full_arch_list_tail);
>> +      record_full_reset_incomplete ();
>>         throw;
>>       }
>>
>> -  record_full_list->next = record_full_arch_list_head;
>> -  record_full_arch_list_head->prev = record_full_list;
>> -  record_full_list = record_full_arch_list_tail;
>> +  record_full_save_instruction ();
>>
>>     if (record_full_insn_num == record_full_insn_max_num)
>>       record_full_list_release_first ();
>> @@ -829,9 +724,9 @@ static enum target_stop_reason
>> record_full_stop_reason
>>      entries and memory entries, followed by an 'end' entry.  */
>>
>>   static inline void
>> -record_full_exec_insn (struct regcache *regcache,
>> -		       struct gdbarch *gdbarch,
>> -		       struct record_full_entry *entry)
>> +record_full_exec_entry (struct regcache *regcache,
>> +			struct gdbarch *gdbarch,
>> +			struct record_full_entry *entry)
> Nit: omit struct keywords
> And there are some more places in this series/patch, but I will stop commenting
> now on all of them.
>
>>   {
>>     switch (entry->type)
>>       {
>> @@ -909,6 +804,15 @@ record_full_exec_insn (struct regcache *regcache,
>>       }
>>   }
>>
>> +static inline void
>> +record_full_exec_insn (struct regcache *regcache,
>> +		       struct gdbarch *gdbarch,
>> +		       record_full_instruction &insn)
>> +{
>> +  for (auto &entry : insn.effects)
>> +    record_full_exec_entry (regcache, gdbarch, &entry);
>> +}
>> +
>>   static void record_full_restore (struct bfd &cbfd);
>>
>>   /* Asynchronous signal handle registered as event loop source for when
>> @@ -983,10 +887,7 @@ record_full_open (const char *args, int from_tty)
>>     record_preopen ();
>>
>>     /* Reset */
>> -  record_full_insn_num = 0;
>> -  record_full_insn_count = 0;
>> -  record_full_list = &record_full_first;
>> -  record_full_list->next = NULL;
>> +  record_full_reset_history ();
>>
>>     bfd *cbfd = get_inferior_core_bfd (current_inferior ());
>>     if (cbfd != nullptr)
>> @@ -1014,7 +915,7 @@ record_full_base_target::close ()
>>     if (record_debug)
>>       gdb_printf (gdb_stdlog, "Process record: record_full_close\n");
>>
>> -  record_full_list_release (record_full_list);
>> +  record_full_reset_history ();
>>
>>     /* Release record_full_core_regbuf.  */
>>     if (record_full_core_regbuf)
>> @@ -1313,7 +1214,6 @@ record_full_wait_1 (struct target_ops *ops,
>>         struct gdbarch *gdbarch = regcache->arch ();
>>         const address_space *aspace = current_inferior ()->aspace.get ();
>>         int continue_flag = 1;
>> -      int first_record_full_end = 1;
>>
>>         try
>>   	{
>> @@ -1322,7 +1222,6 @@ record_full_wait_1 (struct target_ops *ops,
>>   	  record_full_stop_reason = TARGET_STOPPED_BY_NO_REASON;
>>   	  status->set_stopped (GDB_SIGNAL_0);
>>
>> -	  /* Check breakpoint when forward execute.  */
>>   	  if (execution_direction == EXEC_FORWARD)
>>   	    {
>>   	      tmp_pc = regcache_read_pc (regcache);
>> @@ -1330,8 +1229,7 @@ record_full_wait_1 (struct target_ops *ops,
>>
>> &record_full_stop_reason))
>>   		{
>>   		  if (record_debug)
>> -		    gdb_printf (gdb_stdlog,
>> -				"Process record: break at %s.\n",
>> +		    gdb_printf (gdb_stdlog, "Process record: break at %s.\n",
>>   				paddress (gdbarch, tmp_pc));
>>   		  goto replay_out;
>>   		}
>> @@ -1344,10 +1242,10 @@ record_full_wait_1 (struct target_ops *ops,
>>   	     the signal.  */
>>   	  target_terminal::ours ();
>>
>> -	  /* In EXEC_FORWARD mode, record_full_list points to the tail of prev
>> -	     instruction.  */
>> -	  if (execution_direction == EXEC_FORWARD && record_full_list->next)
>> -	    record_full_list = record_full_list->next;
>> +	  /* In EXEC_FORWARD mode, record_full_next_insn is the next
>> +	     instruction to be executed.  */
>> +	  if (execution_direction == EXEC_REVERSE)
>> +	    record_full_next_insn--;
>>
>>   	  /* Loop over the record_full_list, looking for the next place to
>>   	     stop.  */
>> @@ -1355,108 +1253,92 @@ record_full_wait_1 (struct target_ops *ops,
>>   	    {
>>   	      /* Check for beginning and end of log.  */
>>   	      if (execution_direction == EXEC_REVERSE
>> -		  && record_full_list == &record_full_first)
>> +		  && record_full_next_insn < 0)
>>   		{
>>   		  /* Hit beginning of record log in reverse.  */
>>   		  status->set_no_history ();
>> +		  record_full_next_insn = 0;
>>   		  break;
>>   		}
>>   	      if (execution_direction != EXEC_REVERSE
>> -		  && !record_full_list->next)
>> +		  && record_full_next_insn == record_full_list.size ())
>>   		{
>>   		  /* Hit end of record log going forward.  */
>>   		  status->set_no_history ();
>>   		  break;
>>   		}
>>
>> -	      record_full_exec_insn (regcache, gdbarch, record_full_list);
>> +	      record_full_exec_insn
>> +		(regcache, gdbarch,
>> +		 record_full_list[record_full_next_insn]);
>>
>> -	      if (record_full_list->type == record_full_end)
>> +	      /* step */
>> +	      if (record_full_resume_step)
>>   		{
>>   		  if (record_debug > 1)
>> -		    gdb_printf
>> -		      (gdb_stdlog,
>> -		       "Process record: record_full_end %s to "
>> -		       "inferior.\n",
>> -		       host_address_to_string (record_full_list));
>> -
>> -		  if (first_record_full_end
>> -		      && execution_direction == EXEC_REVERSE)
>> -		    {
>> -		      /* When reverse execute, the first
>> -			 record_full_end is the part of current
>> -			 instruction.  */
>> -		      first_record_full_end = 0;
>> -		    }
>> -		  else
>> -		    {
>> -		      /* In EXEC_REVERSE mode, this is the
>> -			 record_full_end of prev instruction.  In
>> -			 EXEC_FORWARD mode, this is the
>> -			 record_full_end of current instruction.  */
>> -		      /* step */
>> -		      if (record_full_resume_step)
>> -			{
>> -			  if (record_debug > 1)
>> -			    gdb_printf (gdb_stdlog,
>> -					"Process record: step.\n");
>> -			  continue_flag = 0;
>> -			}
>> -
>> -		      /* check breakpoint */
>> -		      tmp_pc = regcache_read_pc (regcache);
>> -		      if (record_check_stopped_by_breakpoint
>> -			  (aspace, tmp_pc, &record_full_stop_reason))
>> -			{
>> -			  if (record_debug)
>> -			    gdb_printf (gdb_stdlog,
>> -					"Process record: break "
>> -					"at %s.\n",
>> -					paddress (gdbarch, tmp_pc));
>> +		    gdb_printf (gdb_stdlog,
>> +				"Process record: step.\n");
>> +		  continue_flag = 0;
>> +		}
>>
>> -			  continue_flag = 0;
>> -			}
>> +	      /* check breakpoint */
>> +	      tmp_pc = regcache_read_pc (regcache);
>> +	      if (record_check_stopped_by_breakpoint
>> +		  (aspace, tmp_pc, &record_full_stop_reason))
>> +		{
>> +		  if (record_debug)
>> +		    gdb_printf (gdb_stdlog,
>> +				"Process record: break "
>> +				"at %s.\n",
>> +				paddress (gdbarch, tmp_pc));
>>
>> -		      if (record_full_stop_reason
>> -			  == TARGET_STOPPED_BY_WATCHPOINT)
>> -			{
>> -			  if (record_debug)
>> -			    gdb_printf (gdb_stdlog,
>> -					"Process record: hit hw "
>> -					"watchpoint.\n");
>> -			  continue_flag = 0;
>> -			}
>> -		      /* Check target signal */
>> -		      if (record_full_list->u.end.sigval != GDB_SIGNAL_0)
>> -			/* FIXME: better way to check */
>> -			continue_flag = 0;
>> -		    }
>> +		  continue_flag = 0;
>>   		}
>>
>> -	      if (continue_flag)
>> +	      if (record_full_stop_reason
>> +		  == TARGET_STOPPED_BY_WATCHPOINT)
>>   		{
>> -		  if (execution_direction == EXEC_REVERSE)
>> -		    {
>> -		      if (record_full_list->prev)
>> -			record_full_list = record_full_list->prev;
>> -		    }
>> -		  else
>> -		    {
>> -		      if (record_full_list->next)
>> -			record_full_list = record_full_list->next;
>> -		    }
>> +		  if (record_debug)
>> +		    gdb_printf (gdb_stdlog,
>> +				"Process record: hit hw "
>> +				"watchpoint.\n");
>> +		  continue_flag = 0;
>>   		}
>> +	      if (record_full_list[record_full_next_insn].sigval.has_value ())
>> +		continue_flag = 0;
>> +
>> +	      if (execution_direction == EXEC_REVERSE)
>> +		record_full_next_insn--;
>> +	      else
>> +		record_full_next_insn++;
>>   	    }
>>   	  while (continue_flag);
>>
>> +	  if (record_full_next_insn < 0)
>> +	    {
>> +	      gdb_assert (execution_direction == EXEC_REVERSE);
>> +	      record_full_next_insn = 0;
>> +	    }
>> +	  else if (record_full_next_insn > record_full_list.size ())
>> +	    {
>> +	      gdb_assert (execution_direction == EXEC_FORWARD);
>> +	      record_full_next_insn = record_full_list.size ();
>> +	    }
>> +	  /* Reset the current instruciton to point to the one to be replayed
> Nit: instruction
>
>> +	     moving forward.  */
>> +	  else if (execution_direction == EXEC_REVERSE)
>> +	    record_full_next_insn++;
>> +
>>   	replay_out:
>>   	  if (status->kind () == TARGET_WAITKIND_STOPPED)
>>   	    {
>> +	      int insn = (execution_direction == EXEC_FORWARD)
>> +			 ? record_full_next_insn - 1 : record_full_next_insn;
>>   	      if (record_full_get_sig)
>>   		status->set_stopped (GDB_SIGNAL_INT);
>> -	      else if (record_full_list->u.end.sigval != GDB_SIGNAL_0)
>> -		/* FIXME: better way to check */
>> -		status->set_stopped (record_full_list->u.end.sigval);
>> +	      else if (record_full_list[insn].sigval.has_value ())
>> +		status->set_stopped
>> +		  (record_full_list[insn].sigval.value ());
>>   	      else
>>   		status->set_stopped (GDB_SIGNAL_TRAP);
>>   	    }
>> @@ -1464,12 +1346,9 @@ record_full_wait_1 (struct target_ops *ops,
>>         catch (const gdb_exception &ex)
>>   	{
>>   	  if (execution_direction == EXEC_REVERSE)
>> -	    {
>> -	      if (record_full_list->next)
>> -		record_full_list = record_full_list->next;
>> -	    }
>> +	    record_full_next_insn++;
>>   	  else
>> -	    record_full_list = record_full_list->prev;
>> +	    record_full_next_insn--;
>>
>>   	  throw;
>>   	}
>> @@ -1557,8 +1436,7 @@ record_full_registers_change (struct regcache
>> *regcache, int regnum)
>>     /* Check record_full_insn_num.  */
>>     record_full_check_insn_num ();
>>
>> -  record_full_arch_list_head = NULL;
>> -  record_full_arch_list_tail = NULL;
>> +  record_full_reset_incomplete ();
>>
>>     if (regnum < 0)
>>       {
>> @@ -1568,7 +1446,7 @@ record_full_registers_change (struct regcache
>> *regcache, int regnum)
>>   	{
>>   	  if (record_full_arch_list_add_reg (regcache, i))
>>   	    {
>> -	      record_full_list_release (record_full_arch_list_tail);
>> +	      record_full_reset_incomplete ();
>>   	      error (_("Process record: failed to record execution log."));
>>   	    }
>>   	}
>> @@ -1577,18 +1455,11 @@ record_full_registers_change (struct regcache
>> *regcache, int regnum)
>>       {
>>         if (record_full_arch_list_add_reg (regcache, regnum))
>>   	{
>> -	  record_full_list_release (record_full_arch_list_tail);
>> +	  record_full_reset_incomplete ();
>>   	  error (_("Process record: failed to record execution log."));
>>   	}
>>       }
>> -  if (record_full_arch_list_add_end ())
>> -    {
>> -      record_full_list_release (record_full_arch_list_tail);
>> -      error (_("Process record: failed to record execution log."));
>> -    }
>> -  record_full_list->next = record_full_arch_list_head;
>> -  record_full_arch_list_head->prev = record_full_list;
>> -  record_full_list = record_full_arch_list_tail;
>> +  record_full_save_instruction ();
>>
>>     if (record_full_insn_num == record_full_insn_max_num)
>>       record_full_list_release_first ();
>> @@ -1607,7 +1478,7 @@ record_full_target::store_registers (struct
>> regcache *regcache, int regno)
>>   	{
>>   	  int n;
>>
>> -	  /* Let user choose if he wants to write register or not.  */
>> +	  /* Let user choose if they want to write register or not.  */
>>   	  if (regno < 0)
>>   	    n =
>>   	      query (_("Because GDB is in replay mode, changing the "
>> @@ -1642,7 +1513,7 @@ record_full_target::store_registers (struct
>> regcache *regcache, int regno)
>>   	    }
>>
>>   	  /* Destroy the record from here forward.  */
>> -	  record_full_list_release_following (record_full_list);
>> +	  record_full_list_release_following (record_full_next_insn);
>>   	}
>>
>>         record_full_registers_change (regcache, regno);
>> @@ -1675,36 +1546,24 @@ record_full_target::xfer_partial (enum
>> target_object object,
>>   	    error (_("Process record canceled the operation."));
>>
>>   	  /* Destroy the record from here forward.  */
>> -	  record_full_list_release_following (record_full_list);
>> +	  record_full_list_release_following (record_full_next_insn);
>>   	}
>>
>>         /* Check record_full_insn_num */
>>         record_full_check_insn_num ();
>>
>>         /* Record registers change to list as an instruction.  */
>> -      record_full_arch_list_head = NULL;
>> -      record_full_arch_list_tail = NULL;
>> +      record_full_reset_incomplete ();
>>         if (record_full_arch_list_add_mem (offset, len))
>>   	{
>> -	  record_full_list_release (record_full_arch_list_tail);
>> +	  record_full_reset_incomplete ();
>>   	  if (record_debug)
>>   	    gdb_printf (gdb_stdlog,
>>   			"Process record: failed to record "
>>   			"execution log.");
>>   	  return TARGET_XFER_E_IO;
>>   	}
>> -      if (record_full_arch_list_add_end ())
>> -	{
>> -	  record_full_list_release (record_full_arch_list_tail);
>> -	  if (record_debug)
>> -	    gdb_printf (gdb_stdlog,
>> -			"Process record: failed to record "
>> -			"execution log.");
>> -	  return TARGET_XFER_E_IO;
>> -	}
>> -      record_full_list->next = record_full_arch_list_head;
>> -      record_full_arch_list_head->prev = record_full_list;
>> -      record_full_list = record_full_arch_list_tail;
>> +      record_full_save_instruction ();
>>
>>         if (record_full_insn_num == record_full_insn_max_num)
>>   	record_full_list_release_first ();
>> @@ -1866,8 +1725,7 @@ record_full_base_target::get_bookmark (const char
>> *args, int from_tty)
>>     char *ret = NULL;
>>
>>     /* Return stringified form of instruction count.  */
>> -  if (record_full_list && record_full_list->type == record_full_end)
>> -    ret = xstrdup (pulongest (record_full_list->u.end.insn_num));
>> +  ret = xstrdup (pulongest (record_full_list[record_full_next_insn].insn_num));
> I think we can get here with an empty record_full_list - in case we are in recording mode,
> no instructions have been logged and this function is called:
>
> ~~~
> (gdb) info record
> Active record target: record-full
> Record mode:
> No instructions have been logged.
> Max logged instructions is 200000.
> (gdb) bookmark
>
> Thread 1 "gdb-up" hit Breakpoint 1, record_full_base_target::get_bookmark (this=0x555558392900 <record_full_ops>, args=0x0, from_tty=1) at /tmp/record-full.c:1774
> 1774      char *ret = NULL;
> (gdb) p record_full_next_insn
> $1 = 0
> (gdb) p record_full_list.empty ()
> $2 = true
> ~~~
>
>>     if (record_debug)
>>       {
>> @@ -1923,30 +1781,22 @@ record_full_base_target::record_method (ptid_t
>> ptid)
>>   void
>>   record_full_base_target::info_record ()
>>   {
>> -  struct record_full_entry *p;
>> -
>>     if (RECORD_FULL_IS_REPLAY)
>>       gdb_printf (_("Replay mode:\n"));
>>     else
>>       gdb_printf (_("Record mode:\n"));
>>
>> -  /* Find entry for first actual instruction in the log.  */
>> -  for (p = record_full_first.next;
>> -       p != NULL && p->type != record_full_end;
>> -       p = p->next)
>> -    ;
>> -
>>     /* Do we have a log at all?  */
>> -  if (p != NULL && p->type == record_full_end)
>> +  if (!record_full_list.empty ())
>>       {
>>         /* Display instruction number for first instruction in the log.  */
>> -      gdb_printf (_("Lowest recorded instruction number is %s.\n"),
>> -		  pulongest (p->u.end.insn_num));
>> +      gdb_printf (_("Lowest recorded instruction number is %u.\n"),
>> +		  record_full_list[0].insn_num);
>>
>>         /* If in replay mode, display where we are in the log.  */
>>         if (RECORD_FULL_IS_REPLAY)
>> -	gdb_printf (_("Current instruction number is %s.\n"),
>> -		    pulongest (record_full_list->u.end.insn_num));
>> +	gdb_printf (_("Current instruction number is %u.\n"),
>> +		    record_full_list[record_full_next_insn].insn_num);
>>
>>         /* Display instruction number for last instruction in the log.  */
>>         gdb_printf (_("Highest recorded instruction number is %s.\n"),
>> @@ -1975,7 +1825,7 @@ record_full_base_target::supports_delete_record ()
>>   void
>>   record_full_base_target::delete_record ()
>>   {
>> -  record_full_list_release_following (record_full_list);
>> +  record_full_reset_history ();
>>   }
>>
>>   /* The "record_is_replaying" target method.  */
>> @@ -2001,23 +1851,23 @@ record_full_base_target::record_will_replay
>> (ptid_t ptid, int dir)
>>   /* Go to a specific entry.  */
>>
>>   static void
>> -record_full_goto_entry (struct record_full_entry *p)
>> +record_full_goto_entry (size_t target_insn)
>>   {
>> -  if (p == NULL)
>> +  if (target_insn >= record_full_list.size ())
>>       error (_("Target insn not found."));
>> -  else if (p == record_full_list)
>> +  else if (target_insn == record_full_next_insn)
>>       error (_("Already at target insn."));
>> -  else if (p->u.end.insn_num > record_full_list->u.end.insn_num)
>> +  else if (target_insn > record_full_next_insn)
>>       {
>>         gdb_printf (_("Go forward to insn number %s\n"),
>> -		  pulongest (p->u.end.insn_num));
>> -      record_full_goto_insn (p, EXEC_FORWARD);
>> +		  pulongest (record_full_list[target_insn].insn_num));
>> +      record_full_goto_insn (target_insn, EXEC_FORWARD);
>>       }
>>     else
>>       {
>>         gdb_printf (_("Go backward to insn number %s\n"),
>> -		  pulongest (p->u.end.insn_num));
>> -      record_full_goto_insn (p, EXEC_REVERSE);
>> +		  pulongest (target_insn));
>> +      record_full_goto_insn (target_insn, EXEC_REVERSE);
>>       }
>>
>>     registers_changed ();
>> @@ -2033,13 +1883,7 @@ record_full_goto_entry (struct record_full_entry
>> *p)
>>   void
>>   record_full_base_target::goto_record_begin ()
>>   {
>> -  struct record_full_entry *p = NULL;
>> -
>> -  for (p = &record_full_first; p != NULL; p = p->next)
>> -    if (p->type == record_full_end)
>> -      break;
>> -
>> -  record_full_goto_entry (p);
>> +  record_full_goto_entry (0);
>>   }
>>
>>   /* The "goto_record_end" target method.  */
>> @@ -2047,15 +1891,7 @@ record_full_base_target::goto_record_begin ()
>>   void
>>   record_full_base_target::goto_record_end ()
>>   {
>> -  struct record_full_entry *p = NULL;
>> -
>> -  for (p = record_full_list; p->next != NULL; p = p->next)
>> -    ;
>> -  for (; p!= NULL; p = p->prev)
>> -    if (p->type == record_full_end)
>> -      break;
>> -
>> -  record_full_goto_entry (p);
>> +  record_full_goto_entry (record_full_list.size () -1);
> Nit: whitespace before 1 => "record_full_list.size () - 1"
>
> And in case the record_full_list is empty, won't we call
> record_full_goto_entry with an invalid value for target_insn ?
>
>>   }
>>
>>   /* The "goto_record" target method.  */
>> @@ -2063,13 +1899,7 @@ record_full_base_target::goto_record_end ()
>>   void
>>   record_full_base_target::goto_record (ULONGEST target_insn)
>>   {
>> -  struct record_full_entry *p = NULL;
>> -
>> -  for (p = &record_full_first; p != NULL; p = p->next)
>> -    if (p->type == record_full_end && p->u.end.insn_num == target_insn)
>> -      break;
>> -
>> -  record_full_goto_entry (p);
>> +  record_full_goto_entry (target_insn);
>>   }
>>
>>   /* The "record_stop_replaying" target method.  */
>> @@ -2339,19 +2169,96 @@ netorder32 (uint32_t input)
>>     return ret;
>>   }
>>
>> +static void
>> +record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
>> +{
>> +  uint8_t rectype;
>> +  uint32_t regnum, len;
>> +  uint64_t addr;
>> +  regcache *cache = get_thread_regcache (inferior_thread ());
>> +
>> +  bfdcore_read (cbfd, osec, &rectype, sizeof (rectype), bfd_offset);
>> +  switch (rectype)
>> +    {
>> +    case record_full_reg: /* reg */
>> +      {
>> +	/* Get register number to regnum.  */
>> +	bfdcore_read (cbfd, osec, &regnum, sizeof (regnum), bfd_offset);
>> +	regnum = netorder32 (regnum);
>> +
>> +	record_full_entry rec;
>> +
>> +	rec = record_full_reg_init (cache, regnum);
>> +
>> +	/* Get val.  */
>> +	bfdcore_read (cbfd, osec, record_full_get_loc (&rec),
>> +		      rec.u.reg.len, bfd_offset);
>> +
>> +	if (record_debug)
>> +	  gdb_printf (gdb_stdlog,
>> +		      "  Reading register %d (1 "
>> +		      "plus %lu plus %d bytes)\n",
>> +		      rec.u.reg.num,
>> +		      (unsigned long) sizeof (regnum),
>> +		      rec.u.reg.len);
>> +
>> +	record_full_arch_list_add (rec);
>> +	break;
>> +      }
>> +
>> +    case record_full_mem: /* mem */
>> +      {
>> +	/* Get len.  */
>> +	bfdcore_read (cbfd, osec, &len, sizeof (len), bfd_offset);
>> +	len = netorder32 (len);
>> +
>> +	/* Get addr.  */
>> +	bfdcore_read (cbfd, osec, &addr, sizeof (addr),
>> +		      bfd_offset);
>> +	addr = netorder64 (addr);
>> +
>> +	record_full_entry rec;
>> +	rec = record_full_mem_init (addr, len);
>> +
>> +	/* Get val.  */
>> +	bfdcore_read (cbfd, osec, record_full_get_loc (&rec), len,
>> +		      bfd_offset);
>> +
>> +	if (record_debug)
>> +	  gdb_printf (gdb_stdlog,
>> +		      "  Reading memory %s (1 plus "
>> +		      "%lu plus %lu plus %d bytes)\n",
>> +		      paddress (get_current_arch (),
>> +				rec.u.mem.addr),
>> +		      (unsigned long) sizeof (addr),
>> +		      (unsigned long) sizeof (len),
>> +		      len);
>> +
>> +	record_full_arch_list_add (rec);
>> +	break;
>> +      }
>> +
>> +    default:
>> +      error (_("Bad entry type %d, offset %d of %lu in core file %ps."),
>> +	     rectype, *bfd_offset, bfd_section_size (osec),
>> +	     styled_string (file_name_style.style (),
>> +			    bfd_get_filename (cbfd)));
>> +      break;
>> +    }
>> +}
>> +
>>   /* Restore the execution log from core file CBFD.  */
>>
>>   static void
>>   record_full_restore (struct bfd &cbfd)
>>   {
>>     uint32_t magic;
>> -  struct record_full_entry *rec;
>>     asection *osec;
>>     uint32_t osec_size;
>>     int bfd_offset = 0;
>>
>>     /* "record_full_restore" can only be called when record list is empty.  */
>> -  gdb_assert (record_full_first.next == NULL);
>> +  gdb_assert (record_full_list.empty ());
>>
>>     if (record_debug)
>>       gdb_printf (gdb_stdlog, "Restoring recording from core file.\n");
>> @@ -2379,124 +2286,47 @@ record_full_restore (struct bfd &cbfd)
>>   		"RECORD_FULL_FILE_MAGIC (0x%s)\n",
>>   		phex_nz (netorder32 (magic), 4));
>>
>> -  /* Restore the entries in recfd into record_full_arch_list_head and
>> -     record_full_arch_list_tail.  */
>> -  record_full_arch_list_head = NULL;
>> -  record_full_arch_list_tail = NULL;
>>     record_full_insn_num = 0;
>>
>>     try
>>       {
>> -      regcache *regcache = get_thread_regcache (inferior_thread ());
>> -
>> -      while (1)
>> +      while (bfd_offset < osec_size)
>>   	{
>> -	  uint8_t rectype;
>> -	  uint32_t regnum, len, signal, count;
>> -	  uint64_t addr;
>> -
>> -	  /* We are finished when offset reaches osec_size.  */
>> -	  if (bfd_offset >= osec_size)
>> -	    break;
>> -	  bfdcore_read (&cbfd, osec, &rectype, sizeof (rectype), &bfd_offset);
>> -
>> -	  switch (rectype)
>> +	  record_full_reset_incomplete ();
>> +	  uint32_t eff_count = 0;
>> +	  uint8_t sigval;
>> +	  uint32_t insn_num;
>> +
>> +	  /* First read the generic information for an instruction.  */
>> +	  bfdcore_read (&cbfd, osec, &sigval, sizeof (uint8_t), &bfd_offset);
>> +	  bfdcore_read (&cbfd, osec, &eff_count, sizeof (uint32_t),
>> +			&bfd_offset);
>> +	  bfdcore_read (&cbfd, osec, &insn_num, sizeof (uint32_t),
>> +			&bfd_offset);
>> +
>> +	  record_full_incomplete_instruction.insn_num = netorder32
>> (insn_num);
>> +	  if (sigval != GDB_SIGNAL_0)
>> +	    record_full_incomplete_instruction.sigval = (gdb_signal) sigval;
>> +
>> +	  eff_count = netorder32 (eff_count);
>> +
>> +	  /* This deals with all the side effects.  */
>> +	 while (eff_count > 0)
>>   	    {
> There is one space too much indent.
>
>> -	    case record_full_reg: /* reg */
>> -	      /* Get register number to regnum.  */
>> -	      bfdcore_read (&cbfd, osec, &regnum, sizeof (regnum),
>> -			    &bfd_offset);
>> -	      regnum = netorder32 (regnum);
>> -
>> -	      rec = record_full_reg_alloc (regcache, regnum);
>> -
>> -	      /* Get val.  */
>> -	      bfdcore_read (&cbfd, osec, record_full_get_loc (rec),
>> -			    rec->u.reg.len, &bfd_offset);
>> -
>> -	      if (record_debug)
>> -		gdb_printf (gdb_stdlog,
>> -			    "  Reading register %d (1 "
>> -			    "plus %lu plus %d bytes)\n",
>> -			    rec->u.reg.num,
>> -			    (unsigned long) sizeof (regnum),
>> -			    rec->u.reg.len);
>> -	      break;
>> +	      eff_count--;
>>
>> -	    case record_full_mem: /* mem */
>> -	      /* Get len.  */
>> -	      bfdcore_read (&cbfd, osec, &len, sizeof (len), &bfd_offset);
>> -	      len = netorder32 (len);
>> -
>> -	      /* Get addr.  */
>> -	      bfdcore_read (&cbfd, osec, &addr, sizeof (addr), &bfd_offset);
>> -	      addr = netorder64 (addr);
>> -
>> -	      rec = record_full_mem_alloc (addr, len);
>> -
>> -	      /* Get val.  */
>> -	      bfdcore_read (&cbfd, osec, record_full_get_loc (rec),
>> -			    rec->u.mem.len, &bfd_offset);
>> -
>> -	      if (record_debug)
>> -		gdb_printf (gdb_stdlog,
>> -			    "  Reading memory %s (1 plus "
>> -			    "%lu plus %lu plus %d bytes)\n",
>> -			    paddress (get_current_arch (),
>> -				      rec->u.mem.addr),
>> -			    (unsigned long) sizeof (addr),
>> -			    (unsigned long) sizeof (len),
>> -			    rec->u.mem.len);
>> -	      break;
>> -
>> -	    case record_full_end: /* end */
>> -	      rec = record_full_end_alloc ();
>> -	      record_full_insn_num ++;
>> -
>> -	      /* Get signal value.  */
>> -	      bfdcore_read (&cbfd, osec, &signal, sizeof (signal),
>> -			    &bfd_offset);
>> -	      signal = netorder32 (signal);
>> -	      rec->u.end.sigval = (enum gdb_signal) signal;
>> -
>> -	      /* Get insn count.  */
>> -	      bfdcore_read (&cbfd, osec, &count, sizeof (count), &bfd_offset);
>> -	      count = netorder32 (count);
>> -	      rec->u.end.insn_num = count;
>> -	      record_full_insn_count = count + 1;
>> -	      if (record_debug)
>> -		gdb_printf (gdb_stdlog,
>> -			    "  Reading record_full_end (1 + "
>> -			    "%lu + %lu bytes), offset == %s\n",
>> -			    (unsigned long) sizeof (signal),
>> -			    (unsigned long) sizeof (count),
>> -			    paddress (get_current_arch (),
>> -				      bfd_offset));
>> -	      break;
>> -
>> -	    default:
>> -	      error (_("Bad entry type in core file %ps."),
>> -		     styled_string (file_name_style.style (),
>> -				    bfd_get_filename (&cbfd)));
>> -	      break;
>> +	      record_full_read_entry_from_bfd (&cbfd, osec, &bfd_offset);
>>   	    }
>>
>> -	  /* Add rec to record arch list.  */
>> -	  record_full_arch_list_add (rec);
>> +	  record_full_save_instruction ();
>>   	}
>>       }
>>     catch (const gdb_exception &ex)
>>       {
>> -      record_full_list_release (record_full_arch_list_tail);
>> +      record_full_reset_incomplete ();
>>         throw;
>>       }
>>
>> -  /* Add record_full_arch_list_head to the end of record list.  */
>> -  record_full_first.next = record_full_arch_list_head;
>> -  record_full_arch_list_head->prev = &record_full_first;
>> -  record_full_arch_list_tail->next = NULL;
>> -  record_full_list = &record_full_first;
>> -
>>     /* Update record_full_insn_max_num.  */
>>     if (record_full_insn_num > record_full_insn_max_num)
>>       {
>> @@ -2505,6 +2335,10 @@ record_full_restore (struct bfd &cbfd)
>>   	       record_full_insn_max_num);
>>       }
>>
>> +  /* When loading a recording, we'll always start at the oldest possible
>> +     instruction, no matter where the original recording was stopped.  */
>> +  record_full_next_insn = 0;
>> +
>>     /* Succeeded.  */
>>     gdb_printf (_("Restored records from core file %s.\n"),
>>   	      bfd_get_filename (&cbfd));
>> @@ -2538,13 +2372,73 @@ cmd_record_full_restore (const char *args, int
>> from_tty)
>>     record_full_open (nullptr, from_tty);
>>   }
>>
>> +static void
>> +record_full_write_entry_to_bfd (record_full_entry &entry,
>> +				gdb_bfd_ref_ptr obfd,
>> +				asection *osec, int *bfd_offset,
>> +				struct gdbarch *gdbarch)
>> +{
>> +  /* Save entry.  */
>> +  uint8_t type;
>> +  uint32_t regnum, len;
>> +  uint64_t addr;
>> +
>> +  type = entry.type;
>> +  bfdcore_write (obfd.get (), osec, &type, sizeof (type), bfd_offset);
>> +
>> +  switch (entry.type)
>> +    {
>> +    case record_full_reg: /* reg */
>> +      if (record_debug)
>> +	gdb_printf (gdb_stdlog,
>> +		    "  Writing register %d (1 "
>> +		    "plus %lu plus %d bytes)\n",
>> +		    entry.u.reg.num,
>> +		    (unsigned long) sizeof (regnum),
>> +		    entry.u.reg.len);
>> +
>> +      /* Write regnum.  */
>> +      regnum = netorder32 (entry.u.reg.num);
>> +      bfdcore_write (obfd.get (), osec, &regnum, sizeof (regnum), bfd_offset);
>> +
>> +      /* Write regval.  */
>> +      bfdcore_write (obfd.get (), osec, record_full_get_loc (&entry),
>> +		     entry.u.reg.len, bfd_offset);
>> +      break;
>> +
>> +    case record_full_mem: /* mem */
>> +      if (record_debug)
>> +	gdb_printf (gdb_stdlog,
>> +		    "  Writing memory %s (1 plus "
>> +		    "%lu plus %lu plus %d bytes)\n",
>> +		    paddress (gdbarch,
>> +			      entry.u.mem.addr),
>> +		    (unsigned long) sizeof (addr),
>> +		    (unsigned long) sizeof (len),
>> +		    entry.u.mem.len);
>> +
>> +      /* Write memlen.  */
>> +      len = netorder32 (entry.u.mem.len);
>> +      bfdcore_write (obfd.get (), osec, &len, sizeof (len), bfd_offset);
>> +
>> +      /* Write memaddr.  */
>> +      addr = netorder64 (entry.u.mem.addr);
>> +      bfdcore_write (obfd.get (), osec, &addr, sizeof (addr), bfd_offset);
>> +
>> +      /* Write memval.  */
>> +      bfdcore_write (obfd.get (), osec, record_full_get_loc (&entry),
>> +		     entry.u.mem.len, bfd_offset);
>> +      break;
>> +    }
>> +
>> +}
>> +
>>   /* Save the execution log to a file.  We use a modified elf corefile
>>      format, with an extra section for our data.  */
>>
>>   void
>>   record_full_base_target::save_record (const char *recfilename)
>>   {
>> -  struct record_full_entry *cur_record_full_list;
>>     uint32_t magic;
>>     struct gdbarch *gdbarch;
>>     int save_size = 0;
>> @@ -2562,9 +2456,6 @@ record_full_base_target::save_record (const char
>> *recfilename)
>>     /* Arrange to remove the output file on failure.  */
>>     gdb::unlinker unlink_file (recfilename);
>>
>> -  /* Save the current record entry to "cur_record_full_list".  */
>> -  cur_record_full_list = record_full_list;
>> -
>>     /* Get the values of regcache and gdbarch.  */
>>     regcache *regcache = get_thread_regcache (inferior_thread ());
>>     gdbarch = regcache->arch ();
>> @@ -2574,34 +2465,27 @@ record_full_base_target::save_record (const
>> char *recfilename)
>>       = record_full_gdb_operation_disable_set ();
>>
>>     /* Reverse execute to the begin of record list.  */
>> -  while (1)
>> -    {
>> -      /* Check for beginning and end of log.  */
>> -      if (record_full_list == &record_full_first)
>> -	break;
>> -
>> -      record_full_exec_insn (regcache, gdbarch, record_full_list);
>> -
>> -      if (record_full_list->prev)
>> -	record_full_list = record_full_list->prev;
>> -    }
>> +  for (int i = record_full_next_insn - 1; i >= 0; i--)
>> +    record_full_exec_insn (regcache, gdbarch,
>> +			   record_full_list[i]);
>>
>>     /* Compute the size needed for the extra bfd section.  */
>>     save_size = 4;	/* magic cookie */
>> -  for (record_full_list = record_full_first.next; record_full_list;
>> -       record_full_list = record_full_list->next)
>> -    switch (record_full_list->type)
>> -      {
>> -      case record_full_end:
>> -	save_size += 1 + 4 + 4;
>> -	break;
>> -      case record_full_reg:
>> -	save_size += 1 + 4 + record_full_list->u.reg.len;
>> -	break;
>> -      case record_full_mem:
>> -	save_size += 1 + 4 + 8 + record_full_list->u.mem.len;
>> -	break;
>> -      }
>> +  for (int i = record_full_list.size () - 1; i >= 0; i--)
>> +    {
>> +      /* Number of effects of an instruction.  */
>> +      save_size += sizeof (uint32_t) + sizeof (uint8_t) + sizeof (uint32_t);
>> +      for (auto &entry : record_full_list[i].effects)
>> +	switch (entry.type)
>> +	  {
>> +	  case record_full_reg:
>> +	    save_size += 1 + 4 + entry.u.reg.len;
>> +	    break;
>> +	  case record_full_mem:
>> +	    save_size += 1 + 4 + 8 + entry.u.mem.len;
>> +	    break;
>> +	  }
>> +    }
>>
>>     /* Make the new bfd section.  */
>>     osec = bfd_make_section_anyway_with_flags (obfd.get (), "precord",
>> @@ -2630,108 +2514,32 @@ record_full_base_target::save_record (const
>> char *recfilename)
>>
>>     /* Save the entries to recfd and forward execute to the end of
>>        record list.  */
>> -  record_full_list = &record_full_first;
>> -  while (1)
>> +  for (int i = 0; i < record_full_list.size (); i++)
>>       {
>> -      /* Save entry.  */
>> -      if (record_full_list != &record_full_first)
>> +      uint32_t eff_count = (uint32_t) record_full_list[i].effects.size ();
>> +      uint32_t insn_num = record_full_list[i].insn_num;
>> +      uint8_t sigval = (record_full_list[i].sigval.has_value ())
>> +			 ? record_full_list[i].sigval.value ()
>> +			 : GDB_SIGNAL_0;
>> +
>> +      /* Signal.  */
>> +      bfdcore_write (obfd.get (), osec, &sigval, sizeof (sigval), &bfd_offset);
>> +      eff_count = netorder32 (eff_count);
>> +      /* Number of effects.  */
>> +      bfdcore_write (obfd.get (), osec, &eff_count, sizeof (eff_count),
>> +		     &bfd_offset);
>> +      /* Instruction number.  */
>> +      bfdcore_write (obfd.get (), osec, &insn_num, sizeof (uint32_t),
>> +		     &bfd_offset);
>> +
>> +      for (auto &entry : record_full_list[i].effects)
>>   	{
>> -	  uint8_t type;
>> -	  uint32_t regnum, len, signal, count;
>> -	  uint64_t addr;
>> -
>> -	  type = record_full_list->type;
>> -	  bfdcore_write (obfd.get (), osec, &type, sizeof (type), &bfd_offset);
>> -
>> -	  switch (record_full_list->type)
>> -	    {
>> -	    case record_full_reg: /* reg */
>> -	      if (record_debug)
>> -		gdb_printf (gdb_stdlog,
>> -			    "  Writing register %d (1 "
>> -			    "plus %lu plus %d bytes)\n",
>> -			    record_full_list->u.reg.num,
>> -			    (unsigned long) sizeof (regnum),
>> -			    record_full_list->u.reg.len);
>> -
>> -	      /* Write regnum.  */
>> -	      regnum = netorder32 (record_full_list->u.reg.num);
>> -	      bfdcore_write (obfd.get (), osec, &regnum,
>> -			     sizeof (regnum), &bfd_offset);
>> -
>> -	      /* Write regval.  */
>> -	      bfdcore_write (obfd.get (), osec,
>> -			     record_full_get_loc (record_full_list),
>> -			     record_full_list->u.reg.len, &bfd_offset);
>> -	      break;
>> -
>> -	    case record_full_mem: /* mem */
>> -	      if (record_debug)
>> -		gdb_printf (gdb_stdlog,
>> -			    "  Writing memory %s (1 plus "
>> -			    "%lu plus %lu plus %d bytes)\n",
>> -			    paddress (gdbarch,
>> -				      record_full_list->u.mem.addr),
>> -			    (unsigned long) sizeof (addr),
>> -			    (unsigned long) sizeof (len),
>> -			    record_full_list->u.mem.len);
>> -
>> -	      /* Write memlen.  */
>> -	      len = netorder32 (record_full_list->u.mem.len);
>> -	      bfdcore_write (obfd.get (), osec, &len, sizeof (len),
>> -			     &bfd_offset);
>> -
>> -	      /* Write memaddr.  */
>> -	      addr = netorder64 (record_full_list->u.mem.addr);
>> -	      bfdcore_write (obfd.get (), osec, &addr,
>> -			     sizeof (addr), &bfd_offset);
>> -
>> -	      /* Write memval.  */
>> -	      bfdcore_write (obfd.get (), osec,
>> -			     record_full_get_loc (record_full_list),
>> -			     record_full_list->u.mem.len, &bfd_offset);
>> -	      break;
>> -
>> -	      case record_full_end:
>> -		if (record_debug)
>> -		  gdb_printf (gdb_stdlog,
>> -			      "  Writing record_full_end (1 + "
>> -			      "%lu + %lu bytes)\n",
>> -			      (unsigned long) sizeof (signal),
>> -			      (unsigned long) sizeof (count));
>> -		/* Write signal value.  */
>> -		signal = netorder32 (record_full_list->u.end.sigval);
>> -		bfdcore_write (obfd.get (), osec, &signal,
>> -			       sizeof (signal), &bfd_offset);
>> -
>> -		/* Write insn count.  */
>> -		count = netorder32 (record_full_list->u.end.insn_num);
>> -		bfdcore_write (obfd.get (), osec, &count,
>> -			       sizeof (count), &bfd_offset);
>> -		break;
>> -	    }
>> +	  record_full_write_entry_to_bfd (entry, obfd, osec, &bfd_offset,
>> +					  gdbarch);
>>   	}
>> -
>>         /* Execute entry.  */
>> -      record_full_exec_insn (regcache, gdbarch, record_full_list);
>> -
>> -      if (record_full_list->next)
>> -	record_full_list = record_full_list->next;
>> -      else
>> -	break;
>> -    }
>> -
>> -  /* Reverse execute to cur_record_full_list.  */
>> -  while (1)
>> -    {
>> -      /* Check for beginning and end of log.  */
>> -      if (record_full_list == cur_record_full_list)
>> -	break;
>> -
>> -      record_full_exec_insn (regcache, gdbarch, record_full_list);
>> -
>> -      if (record_full_list->prev)
>> -	record_full_list = record_full_list->prev;
>> +      if (i < record_full_next_insn)
>> +	record_full_exec_insn (regcache, gdbarch, record_full_list[i]);
>>       }
>>
>>     unlink_file.keep ();
>> @@ -2746,7 +2554,7 @@ record_full_base_target::save_record (const char
>> *recfilename)
>>      correspondingly.  */
>>
>>   static void
>> -record_full_goto_insn (struct record_full_entry *entry,
>> +record_full_goto_insn (size_t target_insn,
>>   		       enum exec_direction_kind dir)
> You could explain the new parameter in the function description.
>
>>   {
>>     scoped_restore restore_operation_disable
>> @@ -2757,17 +2565,12 @@ record_full_goto_insn (struct record_full_entry
>> *entry,
>>     /* Assume everything is valid: we will hit the entry,
>>        and we will not hit the end of the recording.  */
>>
>> -  if (dir == EXEC_FORWARD)
>> -    record_full_list = record_full_list->next;
>> -
>> -  do
>> -    {
>> -      record_full_exec_insn (regcache, gdbarch, record_full_list);
>> -      if (dir == EXEC_REVERSE)
>> -	record_full_list = record_full_list->prev;
>> -      else
>> -	record_full_list = record_full_list->next;
>> -    } while (record_full_list != entry);
>> +  if (dir == EXEC_REVERSE)
>> +    for (int i = record_full_next_insn; i > target_insn; i--)
>> +      record_full_exec_insn (regcache, gdbarch, record_full_list[i - 1]);
>> +  else
>> +    for (int i = record_full_next_insn; i < target_insn; i++)
>> +      record_full_exec_insn (regcache, gdbarch, record_full_list[i]);
>>   }
>>
>>   /* Alias for "target record-full".  */
>> @@ -2798,61 +2601,36 @@ set_record_full_insn_max_num (const char
>> *args, int from_tty,
>>   static void
>>   maintenance_print_record_instruction (const char *args, int from_tty)
>>   {
>> -  struct record_full_entry *to_print = record_full_list;
>> +  if (record_full_list.empty ())
>> +    error (_("Not enough recorded history"));
>>
>> +  int offset = record_full_next_insn - 1;
>> +  /* Reduce the offset by 1 if the record_full_next_insn is after the end
>> +     so that we show the last recorded instruction instead of crashing.  */
>> +  if (offset == record_full_list.size ())
>> +    offset--;
>>     if (args != nullptr)
>>       {
>> -      int offset = value_as_long (parse_and_eval (args));
>> -      if (offset > 0)
>> -	{
>> -	  /* Move forward OFFSET instructions.  We know we found the
>> -	     end of an instruction when to_print->type is record_full_end.  */
>> -	  while (to_print->next != nullptr && offset > 0)
>> -	    {
>> -	      to_print = to_print->next;
>> -	      if (to_print->type == record_full_end)
>> -		offset--;
>> -	    }
>> -	  if (offset != 0)
>> -	    error (_("Not enough recorded history"));
>> -	}
>> -      else
>> -	{
>> -	  while (to_print->prev != nullptr && offset < 0)
>> -	    {
>> -	      to_print = to_print->prev;
>> -	      if (to_print->type == record_full_end)
>> -		offset++;
>> -	    }
>> -	  if (offset != 0)
>> -	    error (_("Not enough recorded history"));
>> -	}
>> +      offset += value_as_long (parse_and_eval (args));
>> +      if (offset >= record_full_list.size () || offset < 0)
>> +	error (_("Not enough recorded history"));
>>       }
>> -  gdb_assert (to_print != nullptr);
>> +  auto to_print = record_full_list.begin () + offset;
>>
>>     gdbarch *arch = current_inferior ()->arch ();
>>
>> -  /* Go back to the start of the instruction.  */
>> -  while (to_print->prev != nullptr && to_print->prev->type != record_full_end)
>> -    to_print = to_print->prev;
>> -
>> -  /* if we're in the first record, there are no actual instructions
>> -     recorded.  Warn the user and leave.  */
>> -  if (to_print == &record_full_first)
>> -    error (_("Not enough recorded history"));
>> -
>> -  while (to_print->type != record_full_end)
>> +  for (auto entry : to_print->effects)
>>       {
>> -      switch (to_print->type)
>> +      switch (entry.type)
>>   	{
>>   	  case record_full_reg:
>>   	    {
>> -	      type *regtype = gdbarch_register_type (arch, to_print-
>>> u.reg.num);
>> +	      type *regtype = gdbarch_register_type (arch, entry.u.reg.num);
>>   	      value *val
>>   		  = value_from_contents (regtype,
>> -					 record_full_get_loc (to_print));
>> +					 record_full_get_loc (&entry));
>>   	      gdb_printf ("Register %s changed: ",
>> -			  gdbarch_register_name (arch, to_print-
>>> u.reg.num));
>> +			  gdbarch_register_name (arch, entry.u.reg.num));
>>   	      struct value_print_options opts;
>>   	      get_user_print_options (&opts);
>>   	      opts.raw = true;
>> @@ -2862,17 +2640,16 @@ maintenance_print_record_instruction (const
>> char *args, int from_tty)
>>   	    }
>>   	  case record_full_mem:
>>   	    {
>> -	      gdb_byte *b = record_full_get_loc (to_print);
>> +	      gdb_byte *b = record_full_get_loc (&entry);
>>   	      gdb_printf ("%d bytes of memory at address %s changed from:",
>> -			  to_print->u.mem.len,
>> -			  print_core_address (arch, to_print->u.mem.addr));
>> -	      for (int i = 0; i < to_print->u.mem.len; i++)
>> +			  entry.u.mem.len,
>> +			  print_core_address (arch, entry.u.mem.addr));
>> +	      for (int i = 0; i < entry.u.mem.len; i++)
>>   		gdb_printf (" %02x", b[i]);
>>   	      gdb_printf ("\n");
>>   	      break;
>>   	    }
>>   	}
>> -      to_print = to_print->next;
>>       }
>>   }
>>
>> @@ -2880,11 +2657,6 @@ INIT_GDB_FILE (record_full)
>>   {
>>     struct cmd_list_element *c;
>>
>> -  /* Init record_full_first.  */
>> -  record_full_first.prev = NULL;
>> -  record_full_first.next = NULL;
>> -  record_full_first.type = record_full_end;
>> -
>>     add_target (record_full_target_info, record_full_open);
>>     add_deprecated_target_alias (record_full_target_info, "record");
>>     add_target (record_full_core_target_info, record_full_open);
>> diff --git a/gdb/record-full.h b/gdb/record-full.h
>> index 51effe74560..c327d879a8a 100644
>> --- a/gdb/record-full.h
>> +++ b/gdb/record-full.h
>> @@ -41,7 +41,6 @@ enum record_result
>>
>>   extern int record_full_arch_list_add_reg (struct regcache *regcache, int
>> num);
>>   extern int record_full_arch_list_add_mem (CORE_ADDR addr, int len);
>> -extern int record_full_arch_list_add_end (void);
>>
>>   /* Returns true if the process record target is open.  */
>>   extern int record_full_is_used (void);
>> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
>> index 6b5c9c02d46..f5b1f66844c 100644
>> --- a/gdb/riscv-tdep.c
>> +++ b/gdb/riscv-tdep.c
>> @@ -5466,8 +5466,5 @@ riscv_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>     if (res != RECORD_SUCCESS)
>>       return res;
>>
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>> -
>>     return 0;
>>   }
>> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
>> index e135df94a40..f964889a49f 100644
>> --- a/gdb/rs6000-tdep.c
>> +++ b/gdb/rs6000-tdep.c
>> @@ -7138,8 +7138,6 @@ ppc_process_prefix_instruction (int insn_prefix, int
>> insn_suffix,
>>     if (record_full_arch_list_add_reg (regcache, PPC_PC_REGNUM))
>>       return -1;
>>
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>>     return 0;
>>   }
>>
>> @@ -7447,8 +7445,6 @@ ppc_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>
>>     if (record_full_arch_list_add_reg (regcache, PPC_PC_REGNUM))
>>       return -1;
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>>     return 0;
>>   }
>>
>> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
>> index 3cd6b359d13..6bea30f08dc 100644
>> --- a/gdb/s390-linux-tdep.c
>> +++ b/gdb/s390-linux-tdep.c
>> @@ -910,9 +910,6 @@ s390_linux_record_signal (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>     if (record_full_arch_list_add_mem (sp, sizeof_rt_sigframe))
>>       return -1;
>>
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>> -
>>     return 0;
>>   }
>>
>> diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
>> index f9d7bdd04e4..36ce659ea15 100644
>> --- a/gdb/s390-tdep.c
>> +++ b/gdb/s390-tdep.c
>> @@ -7019,8 +7019,6 @@ s390_process_record (struct gdbarch *gdbarch,
>> struct regcache *regcache,
>>
>>     if (record_full_arch_list_add_reg (regcache, S390_PSWA_REGNUM))
>>       return -1;
>> -  if (record_full_arch_list_add_end ())
>> -    return -1;
>>     return 0;
>>   }
>>
>> --
>> 2.53.0
>>
> Kind Regards,
> Christina
>
> Intel Deutschland GmbH
> Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
> Tel: +49 89 991 430, www.intel.de
> Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Seat: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>