[PATCH 5/6] gdb/record: extract the PC to record_full_instruction

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

This commit makes it so the PC is not saved as part of the
record_full_instruction effects, but rather gets a special location.
That is because a couple of commands would really benefit from it being
easy to find the PC (especially ones from record-btrace that's haven't
been implemented to record-full yet, such as the ones in PR
record/18059), while also possibly allowing for one fewer resizing of
the effect vector (and saving an entire byte in the process).

This commit also refactored record_full_read_entry_from_bfd and
record_full_write_entry_to_bfd, removing the parts specific to mem and
reg entries, to avoid obscure code paths or writing the code to do the
same thing again.
---
gdb/record-full.c | 268 ++++++++++++++++++++++++++++------------------
1 file changed, 165 insertions(+), 103 deletions(-)

diff --git a/gdb/record-full.c b/gdb/record-full.c
index c9e66fac578..297b5b76ae7 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -250,6 +250,16 @@ class record_full_entry
gdb_assert (mem_type == record_full_mem);
}

+ void set_entry (record_full_reg_entry reg)
+ {
+ entry = reg;
+ }
+
+ void set_entry (record_full_mem_entry mem)
+ {
+ entry = mem;
+ }
+
record_full_reg_entry& reg ()
{
gdb_assert (type () == record_full_reg);
@@ -325,6 +335,7 @@ struct record_full_instruction
uint32_t insn_num;
std::optional<gdb_signal> sigval;
std::vector<record_full_entry> effects;
+ record_full_reg_entry pc;

/* Execute the full instruction. As a side effect, set
record_full_stop_reason. */
@@ -684,7 +695,10 @@ record_full_arch_list_add_reg (struct regcache *regcache, int regnum)

regcache->cooked_read (regnum, rec.get_loc ());

- record_full_arch_list_add (rec);
+ if (regnum == gdbarch_pc_regnum (regcache->arch ()))
+ record_full_incomplete_instruction.pc = rec.reg ();
+ else
+ record_full_arch_list_add (rec);

return 0;
}
@@ -841,6 +855,7 @@ static enum target_stop_reason record_full_stop_reason

void record_full_instruction::exec_insn (regcache *regcache, gdbarch *gdbarch)
{
+ pc.execute (regcache, gdbarch);
for (auto &entry : effects)
if (entry.execute (regcache, gdbarch))
record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
@@ -2184,13 +2199,73 @@ netorder32 (uint32_t input)
return ret;
}

+static record_full_reg_entry
+record_full_read_reg_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
+{
+ uint32_t regnum;
+ regcache *cache = get_thread_regcache (inferior_thread ());
+
+ /* Get register number to regnum. */
+ bfdcore_read (cbfd, osec, &regnum, sizeof (regnum),
+ bfd_offset);
+ regnum = netorder32 (regnum);
+
+ record_full_reg_entry reg (cache->arch (), regnum);
+
+ /* Get val. */
+ bfdcore_read (cbfd, osec, reg.get_loc (),
+ reg.len, bfd_offset);
+
+ if (record_debug)
+ gdb_printf (gdb_stdlog,
+ " Reading register %d (1 "
+ "plus %lu plus %d bytes)\n",
+ reg.num,
+ (unsigned long) sizeof (regnum),
+ reg.len);
+ return reg;
+
+}
+
+static record_full_mem_entry
+record_full_read_mem_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
+{
+ uint32_t len;
+ uint64_t addr;
+
+ /* 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_mem_entry mem (addr, len);
+
+ /* Get val. */
+ bfdcore_read (cbfd, osec, mem.get_loc (),
+ 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 (),
+ mem.addr),
+ (unsigned long) sizeof (addr),
+ (unsigned long) sizeof (len),
+ len);
+
+ return mem;
+}
+
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 ());
+ record_full_entry rec;

bfdcore_read (cbfd, osec, &rectype, sizeof (rectype),
bfd_offset);
@@ -2198,57 +2273,13 @@ record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
{
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 (record_full_reg, cache->arch (), regnum);
-
- /* Get val. */
- 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.reg ().num,
- (unsigned long) sizeof (regnum),
- rec.reg ().len);
-
- record_full_arch_list_add (rec);
+ rec.set_entry (record_full_read_reg_from_bfd (cbfd, osec, bfd_offset));
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 (record_full_mem, addr, len);
-
- /* Get val. */
- bfdcore_read (cbfd, osec, rec.get_loc (),
- 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.mem ().addr),
- (unsigned long) sizeof (addr),
- (unsigned long) sizeof (len),
- len);
-
- record_full_arch_list_add (rec);
+ rec.set_entry (record_full_read_mem_from_bfd (cbfd, osec, bfd_offset));
break;
}

@@ -2259,6 +2290,7 @@ record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset)
bfd_get_filename (cbfd)));
break;
}
+ record_full_arch_list_add (rec);
}

/* Restore the execution log from core file CBFD. */
@@ -2323,6 +2355,9 @@ record_full_restore (struct bfd &cbfd)
if (sigval != GDB_SIGNAL_0)
record_full_incomplete_instruction.sigval = (gdb_signal) sigval;

+ record_full_incomplete_instruction.pc
+ = record_full_read_reg_from_bfd (&cbfd, osec, &bfd_offset);
+
eff_count = netorder32 (eff_count);

/* This deals with all the side effects. */
@@ -2387,6 +2422,67 @@ cmd_record_full_restore (const char *args, int from_tty)
record_full_open (nullptr, from_tty);
}

+static void
+record_full_write_reg_to_bfd (record_full_reg_entry &reg,
+ gdb_bfd_ref_ptr obfd,
+ asection *osec, int *bfd_offset,
+ gdbarch *gdbarch)
+{
+ uint32_t regnum;
+
+ 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,
+ reg.get_loc (),
+ reg.len, bfd_offset);
+}
+
+static void
+record_full_write_mem_to_bfd (record_full_mem_entry &mem,
+ gdb_bfd_ref_ptr obfd,
+ asection *osec, int *bfd_offset,
+ gdbarch *gdbarch)
+{
+ uint32_t len;
+ uint64_t addr;
+
+ 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,
+ mem.get_loc (),
+ mem.len, bfd_offset);
+}
+
static void
record_full_write_entry_to_bfd (record_full_entry &entry,
gdb_bfd_ref_ptr obfd,
@@ -2395,8 +2491,6 @@ record_full_write_entry_to_bfd (record_full_entry &entry,
{
/* 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);
@@ -2404,56 +2498,14 @@ record_full_write_entry_to_bfd (record_full_entry &entry,
switch (type)
{
case record_full_reg: /* reg */
- {
- 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;
- }
+ record_full_write_reg_to_bfd (entry.reg (), obfd, osec,
+ bfd_offset, gdbarch);
+ break;

case record_full_mem: /* mem */
- {
- 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;
- }
+ record_full_write_mem_to_bfd (entry.mem (), obfd, osec,
+ bfd_offset, gdbarch);
+ break;
}

}
@@ -2499,6 +2551,7 @@ 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);
+ save_size += 4 + record_full_list[i].pc.len;
for (auto &entry : record_full_list[i].effects)
switch (entry.type ())
{
@@ -2557,6 +2610,9 @@ record_full_base_target::save_record (const char *recfilename)
bfdcore_write (obfd.get (), osec, &insn_num,
sizeof (uint32_t), &bfd_offset);

+ record_full_write_reg_to_bfd (record_full_list[i].pc, obfd, osec,
+ &bfd_offset, gdbarch);
+
for (auto &entry : record_full_list[i].effects)
{
record_full_write_entry_to_bfd (entry, obfd, osec, &bfd_offset,
@@ -2639,6 +2695,9 @@ maintenance_print_record_instruction (const char *args, int from_tty)
auto to_print = record_full_list.begin () + offset;

gdbarch *arch = current_inferior ()->arch ();
+ struct value_print_options opts;
+ get_user_print_options (&opts);
+ opts.raw = true;

for (auto entry : to_print->effects)
{
@@ -2652,9 +2711,6 @@ maintenance_print_record_instruction (const char *args, int from_tty)
entry.get_loc ());
gdb_printf ("Register %s changed: ",
gdbarch_register_name (arch, entry.reg ().num));
- struct value_print_options opts;
- get_user_print_options (&opts);
- opts.raw = true;
value_print (val, gdb_stdout, &opts);
gdb_printf ("\n");
break;
@@ -2673,6 +2729,12 @@ maintenance_print_record_instruction (const char *args, int from_tty)
}
}
}
+ type *regtype = gdbarch_register_type (arch, to_print->pc.num);
+ value *val = value_from_contents (regtype, to_print->pc.get_loc ());
+ gdb_printf ("Register %s changed: ",
+ gdbarch_register_name (arch, to_print->pc.num));
+ value_print (val, gdb_stdout, &opts);
+ gdb_printf ("\n");
}

INIT_GDB_FILE (record_full)
--
2.53.0