[PATCH 3/6] gdb/record: c++ify internal structures of record-full.c

Guinevere Larsen
2026-04-15T18:59:58Z

This commit adds a constructor, destructor, and some methods to the
structures record_full_entry, record_full_reg_entry and
record_full_mem_entry. This is a move to disentangle the internal
representation of the data and how record-full manipulates it for
replaying.

Along with this change, record_full_entry is changed to use an
std::variant, since it was basically doing that already, but now we have
the stdlibc++ error checking to make sure we're only accessing elements
we're allowed to.
---
gdb/record-full.c | 517 +++++++++++++++++++++++++---------------------
1 file changed, 279 insertions(+), 238 deletions(-)

diff --git a/gdb/record-full.c b/gdb/record-full.c
index 95776679f21..f3737fdbc1f 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -48,6 +48,7 @@
#include "cli/cli-style.h"

#include <signal.h>
+#include <variant>

/* This module implements "target record-full", also known as "process
record and replay". This target sits on top of a "normal" target
@@ -90,12 +91,87 @@ struct record_full_mem_entry
int len;
/* Set this flag if target memory for this entry
can no longer be accessed. */
- int mem_entry_not_accessible;
+ bool mem_entry_not_accessible;
union
{
gdb_byte *ptr;
gdb_byte buf[sizeof (gdb_byte *)];
} u;
+
+ record_full_mem_entry () : addr (0), len (0) { }
+
+ record_full_mem_entry (CORE_ADDR mem_addr, int mem_len)
+ {
+ addr = mem_addr;
+ len = mem_len;
+ if (len > sizeof (u.buf))
+ u.ptr = new gdb_byte[len];
+ mem_entry_not_accessible = false;
+ }
+
+ gdb_byte *get_loc ()
+ {
+ if (len > sizeof (u.buf))
+ return u.ptr;
+ else
+ return u.buf;
+ }
+
+ bool execute (struct regcache *regcache,
+ struct gdbarch *gdbarch)
+ {
+ /* Nothing to do if the memory is flagged not_accessible. */
+ if (!mem_entry_not_accessible)
+ {
+ gdb::byte_vector buf (len);
+
+ if (record_debug > 1)
+ gdb_printf (gdb_stdlog,
+ "Process record: record_full_mem %s to "
+ "inferior addr = %s len = %d.\n",
+ host_address_to_string (this),
+ paddress (gdbarch, addr),
+ len);
+
+ if (record_read_memory (gdbarch,
+ addr, buf.data (),
+ len))
+ mem_entry_not_accessible = 1;
+ else
+ {
+ if (target_write_memory (addr,
+ get_loc (),
+ len))
+ {
+ mem_entry_not_accessible = 1;
+ if (record_debug)
+ warning (_("Process record: error writing memory at "
+ "addr = %s len = %d."),
+ paddress (gdbarch, addr),
+ len);
+ }
+ else
+ {
+ memcpy (get_loc (), buf.data (),
+ len);
+
+ /* We've changed memory --- check if a hardware
+ watchpoint should trap. Note that this
+ presently assumes the target beneath supports
+ continuable watchpoints. On non-continuable
+ watchpoints target, we'll want to check this
+ _before_ actually doing the memory change, and
+ not doing the change at all if the watchpoint
+ traps. */
+ if (hardware_watchpoint_inserted_in_range
+ (current_inferior ()->aspace.get (),
+ addr, len))
+ return true;
+ }
+ }
+ }
+ return false;
+ }
};

struct record_full_reg_entry
@@ -107,6 +183,42 @@ struct record_full_reg_entry
gdb_byte *ptr;
gdb_byte buf[2 * sizeof (gdb_byte *)];
} u;
+
+ record_full_reg_entry () : num (0), len (0) { }
+
+ record_full_reg_entry (gdbarch *gdbarch, int regnum)
+ {
+ num = regnum;
+ len = register_size (gdbarch, regnum);
+ if (len > sizeof (u.buf))
+ u.ptr = new gdb_byte[len];
+ }
+
+ gdb_byte *get_loc ()
+ {
+ if (len > sizeof (u.buf))
+ return u.ptr;
+ else
+ return u.buf;
+ }
+
+ bool execute (struct regcache *regcache,
+ struct gdbarch *gdbarch)
+ {
+ gdb::byte_vector buf (len);
+
+ if (record_debug > 1)
+ gdb_printf (gdb_stdlog,
+ "Process record: record_full_reg %s to "
+ "inferior num = %d.\n",
+ host_address_to_string (this),
+ num);
+
+ regcache->cooked_read (num, buf.data ());
+ regcache->cooked_write (num, get_loc ());
+ memcpy (get_loc (), buf.data (), len);
+ return false;
+ }
};

enum record_full_type
@@ -115,16 +227,82 @@ enum record_full_type
record_full_mem
};

-struct record_full_entry
+class record_full_entry
{
- enum record_full_type type;
- union
+ std::variant<record_full_reg_entry, record_full_mem_entry> entry;
+
+public:
+ record_full_entry () : entry (record_full_reg_entry ()) {}
+
+ /* Constructor for a register entry. Type is here to make it
+ easier to recognize it in the constructor calls, it isn't
+ actually important. */
+ record_full_entry (record_full_type reg_type, gdbarch *gdbarch,
+ int regnum)
+ : entry(record_full_reg_entry (gdbarch, regnum))
{
- /* reg */
- struct record_full_reg_entry reg;
- /* mem */
- struct record_full_mem_entry mem;
- } u;
+ gdb_assert (reg_type == record_full_reg);
+ }
+
+ record_full_entry (record_full_type mem_type, CORE_ADDR addr, int len)
+ : entry(record_full_mem_entry (addr, len))
+ {
+ gdb_assert (mem_type == record_full_mem);
+ }
+
+ record_full_reg_entry& reg ()
+ {
+ gdb_assert (type () == record_full_reg);
+ return std::get<record_full_reg_entry> (entry);
+ }
+
+ record_full_mem_entry& mem ()
+ {
+ gdb_assert (type () == record_full_mem);
+ return std::get<record_full_mem_entry> (entry);
+ }
+
+ record_full_type type ()
+ {
+ switch (entry.index ())
+ {
+ case 0:
+ return record_full_reg;
+ case 1:
+ return record_full_mem;
+ }
+ gdb_assert_not_reached ("Impossible variant index");
+ }
+
+ /* Get the pointer to the data stored by this entry. */
+ gdb_byte *get_loc ()
+ {
+ switch (type ())
+ {
+ case record_full_reg:
+ return reg ().get_loc ();
+ case record_full_mem:
+ return mem ().get_loc ();
+ }
+ gdb_assert_not_reached ("Impossible entry type");
+ }
+
+ /* Execute this entry, swapping the appropriate values from memory or
+ register and the recorded ones. Returns TRUE if the execution was
+ stopped by a watchpoint. */
+
+ bool execute (struct regcache *regcache,
+ struct gdbarch *gdbarch)
+ {
+ switch (type ())
+ {
+ case record_full_reg:
+ return reg ().execute (regcache, gdbarch);
+ case record_full_mem:
+ return mem ().execute (regcache, gdbarch);
+ }
+ return false;
+ }
};

/* This is the main structure that comprises the execution log.
@@ -381,61 +559,30 @@ static struct cmd_list_element *record_full_cmdlist;
static void record_full_goto_insn (size_t target_insn,
enum exec_direction_kind dir);

-/* Initialization and cleanup functions for record_full_reg and
- record_full_mem entries. */
-
-/* Init a record_full_reg record entry. */
-
-static inline struct record_full_entry
-record_full_reg_init (struct regcache *regcache, int regnum)
-{
- struct record_full_entry rec;
- struct gdbarch *gdbarch = regcache->arch ();
-
- 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;
-}
-
-/* Cleanup a record_full_reg record entry. */
+/* Cleanup a record_full_reg_entry. This would ideally be a
+ destructor for the classes, but I kept running into issues with
+ double free, so this is left as a future improvement. */

static inline void
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);
+ gdb_assert (rec.type () == record_full_reg);
+ auto reg = rec.reg ();
+ if (reg.len > sizeof (reg.u.buf))
+ delete reg.u.ptr;
}

-/* Init a record_full_mem record entry. */
-
-static inline struct record_full_entry
-record_full_mem_init (CORE_ADDR addr, int len)
-{
- struct record_full_entry rec;
-
- 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;
-}
-
-/* Cleanup a record_full_mem record entry. */
+/* Cleanup a record_full_mem_entry. This would ideally be a
+ destructor for the classes, but I kept running into issues with
+ double free, so this is left as a future improvement. */

static inline void
record_full_mem_cleanup (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);
+ gdb_assert (rec.type () == record_full_mem);
+ auto mem = rec.mem ();
+ if (mem.len > sizeof (mem.u.buf))
+ delete mem.u.ptr;
}

/* Free one record entry, any type.
@@ -444,8 +591,8 @@ record_full_mem_cleanup (struct record_full_entry rec)
static inline void
record_full_entry_cleanup (struct record_full_entry rec)
{
-
- switch (rec.type) {
+ switch (rec.type ())
+ {
case record_full_reg:
record_full_reg_cleanup (rec);
break;
@@ -518,33 +665,12 @@ record_full_arch_list_add (struct record_full_entry &rec)
record_full_incomplete_instruction.effects.push_back (rec);
}

-/* Return the value storage location of a record entry. */
-static inline gdb_byte *
-record_full_get_loc (struct record_full_entry *rec)
-{
- switch (rec->type) {
- case record_full_mem:
- if (rec->u.mem.len > sizeof (rec->u.mem.u.buf))
- return rec->u.mem.u.ptr;
- else
- return rec->u.mem.u.buf;
- case record_full_reg:
- if (rec->u.reg.len > sizeof (rec->u.reg.u.buf))
- return rec->u.reg.u.ptr;
- else
- return rec->u.reg.u.buf;
- default:
- gdb_assert_not_reached ("unexpected record_full_entry type");
- return NULL;
- }
-}
-
/* Record the value of a register NUM to record_full_arch_list. */

int
record_full_arch_list_add_reg (struct regcache *regcache, int regnum)
{
- struct record_full_entry rec;
+ struct record_full_entry rec (record_full_reg, regcache->arch (), regnum);

if (record_debug > 1)
gdb_printf (gdb_stdlog,
@@ -552,9 +678,7 @@ record_full_arch_list_add_reg (struct regcache *regcache, int regnum)
"record list.\n",
regnum);

- rec = record_full_reg_init (regcache, regnum);
-
- regcache->cooked_read (regnum, record_full_get_loc (&rec));
+ regcache->cooked_read (regnum, rec.get_loc ());

record_full_arch_list_add (rec);

@@ -567,7 +691,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 (record_full_mem, addr, len);

if (record_debug > 1)
gdb_printf (gdb_stdlog,
@@ -578,10 +702,8 @@ 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_init (addr, len);
-
if (record_read_memory (current_inferior ()->arch (), addr,
- record_full_get_loc (&rec), len))
+ rec.get_loc (), len))
{
record_full_mem_cleanup (rec);
return -1;
@@ -713,98 +835,14 @@ record_full_gdb_operation_disable_set (void)
static enum target_stop_reason record_full_stop_reason
= TARGET_STOPPED_BY_NO_REASON;

-/* Execute one instruction from the record log. Each instruction in
- the log will be represented by an arbitrary sequence of register
- entries and memory entries, followed by an 'end' entry. */
-
-static inline void
-record_full_exec_entry (struct regcache *regcache,
- struct gdbarch *gdbarch,
- struct record_full_entry *entry)
-{
- switch (entry->type)
- {
- case record_full_reg: /* reg */
- {
- gdb::byte_vector reg (entry->u.reg.len);
-
- if (record_debug > 1)
- gdb_printf (gdb_stdlog,
- "Process record: record_full_reg %s to "
- "inferior num = %d.\n",
- host_address_to_string (entry),
- entry->u.reg.num);
-
- regcache->cooked_read (entry->u.reg.num, reg.data ());
- regcache->cooked_write (entry->u.reg.num, record_full_get_loc (entry));
- memcpy (record_full_get_loc (entry), reg.data (), entry->u.reg.len);
- }
- break;
-
- case record_full_mem: /* mem */
- {
- /* Nothing to do if the entry is flagged not_accessible. */
- if (!entry->u.mem.mem_entry_not_accessible)
- {
- gdb::byte_vector mem (entry->u.mem.len);
-
- if (record_debug > 1)
- gdb_printf (gdb_stdlog,
- "Process record: record_full_mem %s to "
- "inferior addr = %s len = %d.\n",
- host_address_to_string (entry),
- paddress (gdbarch, entry->u.mem.addr),
- entry->u.mem.len);
-
- if (record_read_memory (gdbarch,
- entry->u.mem.addr, mem.data (),
- entry->u.mem.len))
- entry->u.mem.mem_entry_not_accessible = 1;
- else
- {
- if (target_write_memory (entry->u.mem.addr,
- record_full_get_loc (entry),
- entry->u.mem.len))
- {
- entry->u.mem.mem_entry_not_accessible = 1;
- if (record_debug)
- warning (_("Process record: error writing memory at "
- "addr = %s len = %d."),
- paddress (gdbarch, entry->u.mem.addr),
- entry->u.mem.len);
- }
- else
- {
- memcpy (record_full_get_loc (entry), mem.data (),
- entry->u.mem.len);
-
- /* We've changed memory --- check if a hardware
- watchpoint should trap. Note that this
- presently assumes the target beneath supports
- continuable watchpoints. On non-continuable
- watchpoints target, we'll want to check this
- _before_ actually doing the memory change, and
- not doing the change at all if the watchpoint
- traps. */
- if (hardware_watchpoint_inserted_in_range
- (current_inferior ()->aspace.get (),
- entry->u.mem.addr, entry->u.mem.len))
- record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
- }
- }
- }
- }
- break;
- }
-}
-
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);
+ if (entry.execute (regcache, gdbarch))
+ record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
}

static void record_full_restore (struct bfd &cbfd);
@@ -2165,21 +2203,19 @@ record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
bfd_offset);
regnum = netorder32 (regnum);

- record_full_entry rec;
-
- rec = record_full_reg_init (cache, regnum);
+ record_full_entry rec (record_full_reg, cache->arch (), regnum);

/* Get val. */
- bfdcore_read (cbfd, osec, record_full_get_loc (&rec),
- rec.u.reg.len, bfd_offset);
+ bfdcore_read (cbfd, osec, rec.get_loc (),
+ rec.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,
+ rec.reg ().num,
(unsigned long) sizeof (regnum),
- rec.u.reg.len);
+ rec.reg ().len);

record_full_arch_list_add (rec);
break;
@@ -2196,11 +2232,10 @@ record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
bfd_offset);
addr = netorder64 (addr);

- record_full_entry rec;
- rec = record_full_mem_init (addr, len);
+ record_full_entry rec (record_full_mem, addr, len);

/* Get val. */
- bfdcore_read (cbfd, osec, record_full_get_loc (&rec),
+ bfdcore_read (cbfd, osec, rec.get_loc (),
len, bfd_offset);

if (record_debug)
@@ -2208,7 +2243,7 @@ record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
" Reading memory %s (1 plus "
"%lu plus %lu plus %d bytes)\n",
paddress (get_current_arch (),
- rec.u.mem.addr),
+ rec.mem ().addr),
(unsigned long) sizeof (addr),
(unsigned long) sizeof (len),
len);
@@ -2363,57 +2398,62 @@ record_full_write_entry_to_bfd (record_full_entry &entry,
uint32_t regnum, len;
uint64_t addr;

- type = entry.type;
+ type = entry.type ();
bfdcore_write (obfd.get (), osec, &type, sizeof (type), bfd_offset);

- switch (entry.type)
+ switch (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;
+ {
+ auto reg = entry.reg ();
+ if (record_debug)
+ gdb_printf (gdb_stdlog,
+ " Writing register %d (1 "
+ "plus %lu plus %d bytes)\n",
+ reg.num,
+ (unsigned long) sizeof (regnum),
+ reg.len);
+
+ /* Write regnum. */
+ regnum = netorder32 (reg.num);
+ bfdcore_write (obfd.get (), osec, &regnum,
+ sizeof (regnum), bfd_offset);
+
+ /* Write regval. */
+ bfdcore_write (obfd.get (), osec,
+ entry.get_loc (),
+ 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;
+ {
+ auto mem = entry.mem ();
+ if (record_debug)
+ gdb_printf (gdb_stdlog,
+ " Writing memory %s (1 plus "
+ "%lu plus %lu plus %d bytes)\n",
+ paddress (gdbarch, mem.addr),
+ (unsigned long) sizeof (addr),
+ (unsigned long) sizeof (len),
+ mem.len);
+
+ /* Write memlen. */
+ len = netorder32 (mem.len);
+ bfdcore_write (obfd.get (), osec, &len, sizeof (len),
+ bfd_offset);
+
+ /* Write memaddr. */
+ addr = netorder64 (mem.addr);
+ bfdcore_write (obfd.get (), osec, &addr,
+ sizeof (addr), bfd_offset);
+
+ /* Write memval. */
+ bfdcore_write (obfd.get (), osec,
+ entry.get_loc (),
+ mem.len, bfd_offset);
+ break;
+ }
}

}
@@ -2461,13 +2501,13 @@ record_full_base_target::save_record (const char *recfilename)
/* 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)
+ switch (entry.type ())
{
case record_full_reg:
- save_size += 1 + 4 + entry.u.reg.len;
+ save_size += 1 + 4 + entry.reg ().len;
break;
case record_full_mem:
- save_size += 1 + 4 + 8 + entry.u.mem.len;
+ save_size += 1 + 4 + 8 + entry.mem ().len;
break;
}
}
@@ -2603,16 +2643,16 @@ maintenance_print_record_instruction (const char *args, int from_tty)

for (auto entry : to_print->effects)
{
- switch (entry.type)
+ switch (entry.type ())
{
case record_full_reg:
{
- type *regtype = gdbarch_register_type (arch, entry.u.reg.num);
+ type *regtype = gdbarch_register_type (arch, entry.reg ().num);
value *val
= value_from_contents (regtype,
- record_full_get_loc (&entry));
+ entry.get_loc ());
gdb_printf ("Register %s changed: ",
- gdbarch_register_name (arch, entry.u.reg.num));
+ gdbarch_register_name (arch, entry.reg ().num));
struct value_print_options opts;
get_user_print_options (&opts);
opts.raw = true;
@@ -2622,11 +2662,12 @@ maintenance_print_record_instruction (const char *args, int from_tty)
}
case record_full_mem:
{
- gdb_byte *b = record_full_get_loc (&entry);
+ record_full_mem_entry& mem = entry.mem ();
+ gdb_byte *b = entry.get_loc ();
gdb_printf ("%d bytes of memory at address %s changed from:",
- entry.u.mem.len,
- print_core_address (arch, entry.u.mem.addr));
- for (int i = 0; i < entry.u.mem.len; i++)
+ mem.len,
+ print_core_address (arch, mem.addr));
+ for (int i = 0; i < mem.len; i++)
gdb_printf (" %02x", b[i]);
gdb_printf ("\n");
break;
--
2.53.0