Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Schimpe, Christina" <christina.schimpe@intel.com>
To: Guinevere Larsen <guinevere@redhat.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Cc: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Subject: RE: [PATCH v3 1/7] gdb/record: Refactor record history
Date: Tue, 26 May 2026 08:33:36 +0000	[thread overview]
Message-ID: <SN7PR11MB76381B296613D9957C996AF2F90B2@SN7PR11MB7638.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20260515163706.3355686-2-guinevere@redhat.com>

Hi Guinevere,

Please find some further comments below.

> -----Original Message-----
> From: Guinevere Larsen <guinevere@redhat.com>
> Sent: Freitag, 15. Mai 2026 18:37
> To: gdb-patches@sourceware.org
> Cc: Guinevere Larsen <guinevere@redhat.com>; Thiago Jung Bauermann
> <thiago.bauermann@linaro.org>
> Subject: [PATCH v3 1/7] 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.
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> ---
>  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      | 1186 ++++++++++++++++------------------------
>  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, 484 insertions(+), 736 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..02a57726a95 100644
> --- a/gdb/record-full.c
> +++ b/gdb/record-full.c
> @@ -47,6 +47,9 @@
>  #include "interps.h"
>  #include "cli/cli-style.h"
> 
> +#include <vector>
> +#include <optional>
> +#include <deque>
>  #include <signal.h>
> 
>  /* This module implements "target record-full", also known as "process
> @@ -72,20 +75,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 ()) \
> +    || ::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.  */

Optional suggestion (since it has been like this before already):

The description for record_full_entry is not above the actual struct and the
further structs miss a comment. In the description above the enum values
are described '("record_full_reg") or a part of memory ("record_full_mem")',
but not the actual structs.  I think it would be helpful to write this out a bit more, 
add comments to each struct and move the one for record_full_entry
above its actual struct.
Given that this commit is already big enough, a separate commit might be better. 

>  struct record_full_mem_entry
>  {
> @@ -112,47 +112,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 +127,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 +168,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;
> +static record_full_instruction record_full_incomplete_instruction;
> +static 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 +346,14 @@ record_full_target::kill ()
>    record_kill (this);
>  }

This  function is still missing a comment.

> +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 +383,124 @@ 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);

Is this some general guideline to call this cleanup/init instead of release/alloc ?
Since this patch is already so big, I just wonder why you changed it. 😊

> -/* 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 record_full_entry
> +record_full_reg_init (struct regcache *regcache, int regnum)
>  {
> -  struct record_full_entry *rec;
> +  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 (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 record_full_entry
> +record_full_mem_init (CORE_ADDR addr, int len)
>  {
> -  struct record_full_entry *rec;
> +  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)
> +record_full_mem_cleanup (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);
> +  gdb_assert (rec.type == record_full_mem);
> +  if (rec.u.mem.len > sizeof (rec.u.mem.u.buf))
> +    xfree (rec.u.mem.u.ptr);
>  }
> 
> -/* Alloc a record_full_end record entry.  */
> -
> -static inline struct record_full_entry *
> -record_full_end_alloc (void)
> -{
> -  struct record_full_entry *rec;
> -
> -  rec = XCNEW (struct record_full_entry);
> -  rec->type = record_full_end;
> -
> -  return rec;
> -}
> -
> -/* 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)
> +record_full_entry_cleanup (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)
> -{
> -  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_list.clear ();
> +}

This function is missing a comment.

> +static void
> +record_full_list_release_following (int index)
> +{
> +  for (int i = record_full_list.size () - 1; i > index; i--)
>      {
> -      record_full_insn_num = 0;
> -      record_full_first.next = NULL;
> +      for (auto &entry : record_full_list[i].effects)
> +	record_full_entry_cleanup (entry);
> +      record_full_list.pop_back ();
>      }
> -  else
> -    record_full_entry_release (rec);
> +  /* 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;
>  }
> 
> -/* Free all record entries forward of the given list position.  */
> +/* Save the incomplete instruction in the log.  */
> 
>  static void
> -record_full_list_release_following (struct record_full_entry *rec)
> +record_full_save_instruction ()
>  {
> -  struct record_full_entry *tmp = rec->next;
> +  ++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 (std::move
> (record_full_incomplete_instruction));
> +  record_full_next_insn++;
> 
> -  rec->next = NULL;
> -  while (tmp)
> -    {
> -      rec = tmp->next;
> -      if (record_full_entry_release (tmp) == record_full_end)
> -	{
> -	  record_full_insn_num--;
> -	  record_full_insn_count--;
> -	}
> -      tmp = rec;
> -    }
> +  record_full_reset_incomplete ();
>  }
> 
>  /* Delete the first instruction from the beginning of the log, to make
> @@ -550,52 +511,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 (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);

Now since this is one line only, do we really need a separate function for this?

>  }
> 
>  /* Return the value storage location of a record entry.  */
> @@ -613,7 +544,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 +555,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;
> +  record_full_entry rec;
> 
>    if (record_debug > 1)
>      gdb_printf (gdb_stdlog,
> @@ -633,9 +563,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 +578,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;
> +  record_full_entry rec;
> 
>    if (record_debug > 1)
>      gdb_printf (gdb_stdlog,
> @@ -659,12 +589,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 +603,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 +634,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 +654,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.empty ())
> +	record_full_list[record_full_next_insn - 1].sigval = signal;
> 
>        if (signal == GDB_SIGNAL_0
>  	  || !gdbarch_process_record_signal_p (gdbarch))
> @@ -778,13 +682,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 +731,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 (regcache *regcache,
> +			gdbarch *gdbarch,
> +			record_full_entry *entry)
>  {
>    switch (entry->type)
>      {
> @@ -909,6 +811,15 @@ record_full_exec_insn (struct regcache *regcache,
>      }
>  }

Again function comment.. 😉

> +static inline void
> +record_full_exec_insn (regcache *regcache,
> +		       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 +894,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 +922,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 +1221,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 +1229,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 +1236,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 +1249,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 +1260,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 instruction to point to the one to be replayed
> +	     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 +1353,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 +1443,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 +1453,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 +1462,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 +1485,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.  */

It might be cleaner to move such changes in a separate commit,
since they just make this big patch even bigger.

>  	  if (regno < 0)
>  	    n =
>  	      query (_("Because GDB is in replay mode, changing the "
> @@ -1642,7 +1520,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 +1553,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);
> -	  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);
> +	  record_full_reset_incomplete ();
>  	  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 ();
> @@ -1865,9 +1731,11 @@ record_full_base_target::get_bookmark (const
> char *args, int from_tty)
>  {
>    char *ret = NULL;
> 
> +  if (record_full_list.empty ())
> +    return (gdb_byte *) ret;
> +
>    /* 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));
> 
>    if (record_debug)
>      {
> @@ -1923,30 +1791,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 +1835,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 +1861,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 +1893,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 +1901,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);
>  }
> 
>  /* The "goto_record" target method.  */
> @@ -2063,13 +1909,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 +2179,96 @@ netorder32 (uint32_t input)
>    return ret;
>  }

Function comment. And I think it would be cleaner to move this refactoring in
a separate patch, too. Or is this separate function only possible due to the change
from using a list to std::deque? 

> +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 +2296,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;
> +	  record_full_reset_incomplete ();
> +	  uint32_t eff_count = 0;
> +	  uint8_t sigval;
> +	  uint32_t insn_num;
> 
> -	  /* We are finished when offset reaches osec_size.  */
> -	  if (bfd_offset >= osec_size)
> -	    break;
> -	  bfdcore_read (&cbfd, osec, &rectype, sizeof (rectype), &bfd_offset);
> +	  /* 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);
> 
> -	  switch (rectype)
> -	    {
> -	    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;
> +	  record_full_incomplete_instruction.insn_num = netorder32
> (insn_num);
> +	  if (sigval != GDB_SIGNAL_0)
> +	    record_full_incomplete_instruction.sigval = (gdb_signal) sigval;
> 
> -	    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;
> +	  eff_count = netorder32 (eff_count);
> 
> -	    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;
> +	  /* This deals with all the side effects.  */
> +	 while (eff_count > 0)
> +	   {
> +	     eff_count--;
> 
> -	    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 +2345,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 +2382,73 @@ cmd_record_full_restore (const char *args, int
> from_tty)
>    record_full_open (nullptr, from_tty);
>  }

Function comment. And similar comment for separate patch.

I believe sth. like a follow-up commit that introduces the functions
record_full_read_entry_from_bfd and record_full_write_entry_to_bfd
would make this much easier to review.

> +static void
> +record_full_write_entry_to_bfd (record_full_entry &entry,
> +				gdb_bfd_ref_ptr obfd,
> +				asection *osec, int *bfd_offset,
> +				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 +2466,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 +2475,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 +2524,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 ();
> @@ -2742,11 +2560,11 @@ record_full_base_target::save_record (const
> char *recfilename)
>  }
> 
>  /* record_full_goto_insn -- rewind the record log (forward or backward,
> -   depending on DIR) to the given entry, changing the program state
> -   correspondingly.  */
> +   depending on DIR) to the entry in position TARGET_INSN in the history,
> +   changing the program state 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)
>  {
>    scoped_restore restore_operation_disable
> @@ -2757,17 +2575,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 +2611,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 +2650,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 +2667,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.54.0
> 

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

  reply	other threads:[~2026-05-26  8:34 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 16:36 [PATCH v3 0/7] refactor the internals of record-full Guinevere Larsen
2026-05-15 16:36 ` [PATCH v3 1/7] gdb/record: Refactor record history Guinevere Larsen
2026-05-26  8:33   ` Schimpe, Christina [this message]
2026-05-28 13:20     ` Guinevere Larsen
2026-05-29 13:47       ` Schimpe, Christina
2026-05-15 16:36 ` [PATCH v3 2/7] gdb/record: remove record_full_insn_num Guinevere Larsen
2026-05-26  8:35   ` Schimpe, Christina
2026-05-15 16:37 ` [PATCH v3 3/7] gdb/record: c++ify internal structures of record-full.c Guinevere Larsen
2026-05-26  9:00   ` Schimpe, Christina
2026-05-27 20:13     ` Guinevere Larsen
2026-05-29 12:09       ` Schimpe, Christina
2026-05-29 16:38         ` Guinevere Larsen
2026-05-15 16:37 ` [PATCH v3 4/7] gdb/record: make record_full_history more c++-like Guinevere Larsen
2026-05-16  1:03   ` Thiago Jung Bauermann
2026-05-26  9:04   ` Schimpe, Christina
2026-05-26 20:07     ` Guinevere Larsen
2026-05-15 16:37 ` [PATCH v3 5/6] gdb/record: Define new version of the record-save section Guinevere Larsen
2026-05-15 16:37 ` [PATCH v3 5/7] gdb/record: extract the PC to record_full_instruction Guinevere Larsen
2026-05-16  1:04   ` Thiago Jung Bauermann
2026-05-29 12:40   ` Schimpe, Christina
2026-06-01 20:24     ` Guinevere Larsen
2026-05-15 16:37 ` [PATCH v3 6/7] gdb/record: Define new version of the record-save section Guinevere Larsen
2026-05-15 16:37 ` [PATCH v3 6/6] gdb/record: rename record_full_list to record_full_log Guinevere Larsen
2026-05-29 12:05   ` Schimpe, Christina
2026-05-15 16:37 ` [PATCH v3 7/7] " Guinevere Larsen
2026-05-16  1:07   ` Thiago Jung Bauermann
2026-05-15 17:45 ` [PATCH v3 0/7] refactor the internals of record-full Guinevere Larsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN7PR11MB76381B296613D9957C996AF2F90B2@SN7PR11MB7638.namprd11.prod.outlook.com \
    --to=christina.schimpe@intel.com \
    --cc=gdb-patches@sourceware.org \
    --cc=guinevere@redhat.com \
    --cc=thiago.bauermann@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox