From: "Schimpe, Christina" <christina.schimpe@intel.com>
To: Guinevere Larsen <guinevere@redhat.com>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [PATCH v2 1/6] gdb/record: Refactor record history
Date: Wed, 13 May 2026 07:54:03 +0000 [thread overview]
Message-ID: <SN7PR11MB763827776662E108B20CF99FF9062@SN7PR11MB7638.namprd11.prod.outlook.com> (raw)
In-Reply-To: <23236510-ef59-4396-91eb-5763725f01cc@redhat.com>
Hi Guinevere,
> -----Original Message-----
> From: Guinevere Larsen <guinevere@redhat.com>
> Sent: Dienstag, 12. Mai 2026 20:51
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v2 1/6] gdb/record: Refactor record history
>
> On 5/11/26 12:23 PM, Schimpe, Christina wrote:
> > Hi Guinevere,
> >
> > Please find some further comments below.
>
> Hi Christina!
>
> Thanks again for taking such a detailed look at this patch! I have some
> inlined comments, thanks for finding these issues!
No problem, it's always good to see new places in GDB. 😊
I still did not fully get the recording infrastructure TBH, but I am getting closer...
> >> -----Original Message-----
> >> From: Guinevere Larsen <guinevere@redhat.com>
> >> Sent: Donnerstag, 7. Mai 2026 19:38
> >> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >> patches@sourceware.org
> >> Subject: Re: [PATCH v2 1/6] gdb/record: Refactor record history
> >>
> >> On 5/6/26 6:16 PM, Schimpe, Christina wrote:
> >>> Hi Guinevere,
> >>>
> >>> Thank you for working on this.
> >>> I found two items that might be worth looking at in more detail, but
> >>> apart from that it’s mostly minor nits. Please find my feedback below.
> >>>
> >>> I don’t have much experience with the record feature, so my review is
> >>> a bit slow here. To fully understand this patch, I probably need more
> time,
> >>> but I wanted to share the comments I already have. Sorry for chiming in
> >>> so late. I’ll try again tomorrow, or by Friday at the latest.
> >> Hi! Thank you for noting all the inconsistencies, I'll fix almost all of
> >> them before pushing!
> >>
> >> The one thing I don't think it's a good idea is to make the change from
> >> record_full_list -> record_full_log in this commit... mostly because it
> >> will introduce untold amounts of rebase conflicts. I'll introduce a
> >> following commit to this series that will make that renaming, though :)
> >> --
> >> Cheers,
> >> Guinevere Larsen
> >> It/she
> >>
> >>>> -----Original Message-----
> >>>> From: Guinevere Larsen <guinevere@redhat.com>
> >>>> Sent: Donnerstag, 23. April 2026 17:26
> >>>> To: gdb-patches@sourceware.org
> >>>> Cc: Guinevere Larsen <guinevere@redhat.com>
> >>>> Subject: [PATCH v2 1/6] gdb/record: Refactor record history
> >>>>
> >>>> This is the first step in a large refactor in how GDB keeps execution
> >>>> history. Rather than using a linked list where multiple entries can
> >>>> describe a single instruction, the history will now be stored in an
> >>>> std::deque, each instruction being one entry in the deque.
> >>>>
> >>>> The choice was initially to use an std::vector, but it would become
> >>>> unwieldy because it needs all the memory to be consecutive, which is
> >>>> hard for 200 thousand entries. Deque was picked because it was a nice
> >>>> midpoint between vector (maximum cache cohesion) and linked list
> >>>> (maximum ease of finding space to store more).
> >>>>
> >>>> Each instruction in memory will be now one record_full_instruction
> >>>> entry, which for this commit just contains a vector of
> >>>> record_full_entry for the effects of the instruction, and the data that
> >>>> was stored in the record_full_end entry (that is, the instruction number
> >>>> and the signal, if any).
> >>>>
> >>>> This change introduced a minimal performance improvement (what's
> >>>> important is that it isn't a degradation) and a reduction in the total
> >>>> memory footprint of roughly 20% if the entire history is used.
> >>>> ---
> >>>> gdb/aarch64-tdep.c | 2 -
> >>>> gdb/amd64-linux-tdep.c | 3 -
> >>>> gdb/arm-tdep.c | 2 -
> >>>> gdb/i386-linux-tdep.c | 3 -
> >>>> gdb/i386-tdep.c | 4 -
> >>>> gdb/loongarch-tdep.c | 2 -
> >>>> gdb/moxie-tdep.c | 2 -
> >>>> gdb/ppc-linux-tdep.c | 3 -
> >>>> gdb/record-full.c | 1180 ++++++++++++++++------------------------
> >>>> gdb/record-full.h | 1 -
> >>>> gdb/riscv-tdep.c | 3 -
> >>>> gdb/rs6000-tdep.c | 4 -
> >>>> gdb/s390-linux-tdep.c | 3 -
> >>>> gdb/s390-tdep.c | 2 -
> >>>> 14 files changed, 476 insertions(+), 738 deletions(-)
> >>>>
> >>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> >>>> index 4befaa2720d..a532992b1d8 100644
> >>>> --- a/gdb/aarch64-tdep.c
> >>>> +++ b/gdb/aarch64-tdep.c
> >>>> @@ -6210,8 +6210,6 @@ aarch64_process_record (struct gdbarch
> >> *gdbarch,
> >>>> struct regcache *regcache,
> >>>> aarch64_record.aarch64_mems[rec_no].len))
> >>>> ret = -1;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - ret = -1;
> >>>> }
> >>>>
> >>>> deallocate_reg_mem (&aarch64_record);
> >>>> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> >>>> index a5ac26654cf..9b23db72bbe 100644
> >>>> --- a/gdb/amd64-linux-tdep.c
> >>>> +++ b/gdb/amd64-linux-tdep.c
> >>>> @@ -1591,9 +1591,6 @@ amd64_linux_record_signal (struct gdbarch
> >>>> *gdbarch,
> >>>> + AMD64_LINUX_frame_size))
> >>>> return -1;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> -
> >>>> return 0;
> >>>> }
> >>>>
> >>>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> >>>> index 08cce9dbad8..1ce0927f26b 100644
> >>>> --- a/gdb/arm-tdep.c
> >>>> +++ b/gdb/arm-tdep.c
> >>>> @@ -14911,8 +14911,6 @@ arm_process_record (struct gdbarch
> >> *gdbarch,
> >>>> struct regcache *regcache,
> >>>> }
> >>>> }
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - ret = -1;
> >>>> }
> >>>>
> >>>>
> >>>> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> >>>> index 23aeccac9fc..4f33766fcdd 100644
> >>>> --- a/gdb/i386-linux-tdep.c
> >>>> +++ b/gdb/i386-linux-tdep.c
> >>>> @@ -949,9 +949,6 @@ i386_linux_record_signal (struct gdbarch
> >> *gdbarch,
> >>>> I386_LINUX_xstate +
> >>>> I386_LINUX_frame_size))
> >>>> return -1;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> -
> >>>> return 0;
> >>>> }
> >>>>
> >>>>
> >>>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> >>>> index fa935b5fcdb..fee36b46a73 100644
> >>>> --- a/gdb/i386-tdep.c
> >>>> +++ b/gdb/i386-tdep.c
> >>>> @@ -5201,8 +5201,6 @@ i386_record_vex (struct i386_record_s *ir,
> >> uint8_t
> >>>> vex_w, uint8_t vex_r,
> >>>> }
> >>>>
> >>>> record_full_arch_list_add_reg (ir->regcache, ir-
> >>>>> regmap[X86_RECORD_REIP_REGNUM]);
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>>
> >>>> return 0;
> >>>> }
> >>>> @@ -8359,8 +8357,6 @@ Do you want to stop the program?"),
> >>>>
> >>>> /* In the future, maybe still need to deal with need_dasm. */
> >>>> I386_RECORD_FULL_ARCH_LIST_ADD_REG
> >> (X86_RECORD_REIP_REGNUM);
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>>
> >>>> return 0;
> >>>>
> >>>> diff --git a/gdb/loongarch-tdep.c b/gdb/loongarch-tdep.c
> >>>> index 225b1abb703..5ec6b2a48d0 100644
> >>>> --- a/gdb/loongarch-tdep.c
> >>>> +++ b/gdb/loongarch-tdep.c
> >>>> @@ -2789,8 +2789,6 @@ loongarch_process_record (struct gdbarch
> >>>> *gdbarch, struct regcache *regcache,
> >>>> LOONGARCH_PC_REGNUM))
> >>>> return -1;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> }
> >>>>
> >>>> return ret;
> >>>> diff --git a/gdb/moxie-tdep.c b/gdb/moxie-tdep.c
> >>>> index c8a04d21f59..2f8f0186488 100644
> >>>> --- a/gdb/moxie-tdep.c
> >>>> +++ b/gdb/moxie-tdep.c
> >>>> @@ -1040,8 +1040,6 @@ moxie_process_record (struct gdbarch
> >> *gdbarch,
> >>>> struct regcache *regcache,
> >>>>
> >>>> if (record_full_arch_list_add_reg (regcache, MOXIE_PC_REGNUM))
> >>>> return -1;
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> return 0;
> >>>> }
> >>>>
> >>>> diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
> >>>> index 19698eaacbe..e2f53551709 100644
> >>>> --- a/gdb/ppc-linux-tdep.c
> >>>> +++ b/gdb/ppc-linux-tdep.c
> >>>> @@ -1565,9 +1565,6 @@ ppc_linux_record_signal (struct gdbarch
> >> *gdbarch,
> >>>> struct regcache *regcache,
> >>>> if (record_full_arch_list_add_mem (sp, SIGNAL_FRAMESIZE +
> >>>> sizeof_rt_sigframe))
> >>>> return -1;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> -
> >>>> return 0;
> >>>> }
> >>>>
> >>>> diff --git a/gdb/record-full.c b/gdb/record-full.c
> >>>> index 69d2c100e57..aac69cee17e 100644
> >>>> --- a/gdb/record-full.c
> >>>> +++ b/gdb/record-full.c
> >>> I think we should include vector, optional and deque directly.
> >>>
> >>>> @@ -72,20 +72,17 @@
> >>>> #define DEFAULT_RECORD_FULL_INSN_MAX_NUM 200000
> >>>>
> >>>> #define RECORD_FULL_IS_REPLAY \
> >>>> - (record_full_list->next || ::execution_direction == EXEC_REVERSE)
> >>>> + ( (record_full_next_insn != record_full_list.size ()) \
> >>> Nit: No whitespace after the first bracket:
> >>> => ((record_full_next_insn != record_full_list.size ()) \
> >>>
> >>>> + || ::execution_direction == EXEC_REVERSE)
> >>>>
> >>>> #define RECORD_FULL_FILE_MAGIC netorder32(0x20091016)
> >>>>
> >>>> /* These are the core structs of the process record functionality.
> >>>>
> >>>> A record_full_entry is a record of the value change of a register
> >>>> - ("record_full_reg") or a part of memory ("record_full_mem"). And
> each
> >>>> - instruction must have a struct record_full_entry ("record_full_end")
> >>>> - that indicates that this is the last struct record_full_entry of this
> >>>> - instruction.
> >>>> -
> >>>> - Each struct record_full_entry is linked to "record_full_list" by "prev"
> >>>> - and "next" pointers. */
> >>>> + ("record_full_reg") or a part of memory ("record_full_mem").
> >>>> + These are saved on the record_full_instruction struct, which also
> >>>> + contains some extra information, such as delivered signals. */
> >>>>
> >>>> struct record_full_mem_entry
> >>>> {
> >>>> @@ -112,47 +109,14 @@ struct record_full_reg_entry
> >>>> } u;
> >>>> };
> >>>>
> >>>> -struct record_full_end_entry
> >>>> -{
> >>>> - enum gdb_signal sigval;
> >>>> - ULONGEST insn_num;
> >>>> -};
> >>>> -
> >>>> enum record_full_type
> >>>> {
> >>>> - record_full_end = 0,
> >>>> record_full_reg,
> >>>> record_full_mem
> >>>> };
> >>>>
> >>>> -/* This is the data structure that makes up the execution log.
> >>>> -
> >>>> - The execution log consists of a single linked list of entries
> >>>> - of type "struct record_full_entry". It is doubly linked so that it
> >>>> - can be traversed in either direction.
> >>>> -
> >>>> - The start of the list is anchored by a struct called
> >>>> - "record_full_first". The pointer "record_full_list" either points
> >>>> - to the last entry that was added to the list (in record mode), or to
> >>>> - the next entry in the list that will be executed (in replay mode).
> >>>> -
> >>>> - Each list element (struct record_full_entry), in addition to next
> >>>> - and prev pointers, consists of a union of three entry types: mem,
> >>>> - reg, and end. A field called "type" determines which entry type is
> >>>> - represented by a given list element.
> >>>> -
> >>>> - Each instruction that is added to the execution log is represented
> >>>> - by a variable number of list elements ('entries'). The instruction
> >>>> - will have one "reg" entry for each register that is changed by
> >>>> - executing the instruction (including the PC in every case). It
> >>>> - will also have one "mem" entry for each memory change. Finally,
> >>>> - each instruction will have an "end" entry that separates it from
> >>>> - the changes associated with the next instruction. */
> >>>> -
> >>>> struct record_full_entry
> >>>> {
> >>>> - struct record_full_entry *prev;
> >>>> - struct record_full_entry *next;
> >>>> enum record_full_type type;
> >>>> union
> >>>> {
> >>>> @@ -160,11 +124,31 @@ struct record_full_entry
> >>>> struct record_full_reg_entry reg;
> >>>> /* mem */
> >>>> struct record_full_mem_entry mem;
> >>>> - /* end */
> >>>> - struct record_full_end_entry end;
> >>>> } u;
> >>>> };
> >>>>
> >>>> +/* This is the main structure that comprises the execution log.
> >>>> + Each instruction is comprised of:
> >>>> + * The instruction number: How many instructions were recorded
> before
> >>>> + this one;
> >>>> + * sigval: Whether the inferior received a signal while the following
> >>>> + instruction was being recorded;
> > It might be worth describing in the comment in which cases this does not
> > have a value, to point out the reason why this is now optional.
> > As I understand it before this patch we used GDB_SIGNAL_0 for this case, is
> that right?
>
> well, yeah, we used to use GDB_SIGNAL_0, but I think it is better to
> explicitly say "there was no signal" other than say "the signal was 0"
> which is treated as a nothing. It's the problem with NULL/nullptr,
> versus languages where nullptrs aren't allowed and instead accessing an
> unset pointer is just disallowed. I just want to add more mindful usage
> of it to make code safer.
>
> So I don't think it makes sense to add in a comment why it changed, when
> we instead needed a comment earlier about how to deal with "no signal
> was delivered"
Yes, fine with me.
> >>>> + * effects: A list of record_full_entry structures, each of which
> >>>> + describing one effect that the instruction has on the inferior.
> >>>> +
> >>>> + Note, the signal is stored in the previous instruction for historical
> >>>> + reasons. This is how it was first implemented, and no one has gotten
> >>>> + around to changing it yet. */
> >>>> +
> >>>> +struct record_full_instruction
> >>>> +{
> >>>> + /* This might be different from the index if
> >>>> + we had to remove the first few instructions. */
> >>>> + uint32_t insn_num;
> >>>> + std::optional<gdb_signal> sigval;
> >>>> + std::vector<record_full_entry> effects;
> >>>> +};
> >>>> +
> >>>> /* If true, query if PREC cannot record memory
> >>>> change of next instruction. */
> >>>> bool record_full_memory_query = false;
> >>>> @@ -181,27 +165,25 @@ static detached_regcache
> >>>> *record_full_core_regbuf = NULL;
> >>>> static std::vector<target_section> record_full_core_sections;
> >>>> static struct record_full_core_buf_entry *record_full_core_buf_list =
> >> NULL;
> >>>> -/* The following variables are used for managing the linked list that
> >>>> - represents the execution log.
> >>>> +/* The following variables are used for managing the history of
> executed
> >>>> + instructions from the inferior.
> >>>>
> >>>> - record_full_first is the anchor that holds down the beginning of
> >>>> - the list.
> >>>> + record_full_list contains all instructions that were fully executed and
> >>>> + saved to the log, so that we can replay the execution.
> >>>>
> >>>> - record_full_list serves two functions:
> >>>> - 1) In record mode, it anchors the end of the list.
> >>>> - 2) In replay mode, it traverses the list and points to
> >>>> - the next instruction that must be emulated.
> >>>> + record_full_next_insn always points to the next instruction that
> would
> >>>> + be executed if the inferior executes forward. In the special case when
> >>>> + the inferior is not replaying, record_full_next_insn points past the
> >>>> + end of the history.
> >>>>
> >>>> - record_full_arch_list_head and record_full_arch_list_tail are used
> >>>> - to manage a separate list, which is used to build up the change
> >>>> - elements of the currently executing instruction during record mode.
> >>>> - When this instruction has been completely annotated in the "arch
> >>>> - list", it will be appended to the main execution log. */
> >>>> + record_full_incomplete_instruction holds a partial instruction, while
> >>>> + the lower target is disassembling the instruction, or as partial xfers
> are
> >>>> + happening. It is manipulated by the "arch list" functions for historical
> >>>> + reasons. */
> >>>>
> >>>> -static struct record_full_entry record_full_first;
> >>>> -static struct record_full_entry *record_full_list = &record_full_first;
> >>>> -static struct record_full_entry *record_full_arch_list_head = NULL;
> >>>> -static struct record_full_entry *record_full_arch_list_tail = NULL;
> >>>> +static std::deque<record_full_instruction> record_full_list;
> >>> Given that this is not a list anymore I wonder if we should better call this
> >> sth. like
> >>> record_full_history or record_full_log or even simply
> >> recorded_instructions?
> >>>> +static record_full_instruction record_full_incomplete_instruction;
> >>>> +int record_full_next_insn;
> > Any particular reason why this is not static?
> no, I'll make that change.
> >
> >>>> /* true ask user. false auto delete the last struct record_full_entry. */
> >>>> static bool record_full_stop_at_limit = true;
> >>>> @@ -361,6 +343,14 @@ record_full_target::kill ()
> >>>> record_kill (this);
> >>>> }
> >>>>
> >>>> +static void
> >>>> +record_full_reset_incomplete ()
> >>>> +{
> >>>> + record_full_incomplete_instruction.effects.clear ();
> >>>> + record_full_incomplete_instruction.sigval.reset ();
> >>>> + record_full_incomplete_instruction.insn_num = 0;
> >>>> +}
> >>>> +
> >>>> /* See record-full.h. */
> >>>>
> >>>> int
> >>>> @@ -390,156 +380,120 @@ static struct cmd_list_element
> >>>> *show_record_full_cmdlist;
> >>>> /* Command list for "record full". */
> >>>> static struct cmd_list_element *record_full_cmdlist;
> >>>>
> >>>> -static void record_full_goto_insn (struct record_full_entry *entry,
> >>>> +static void record_full_goto_insn (size_t target_insn,
> >>>> enum exec_direction_kind dir);
> >>>>
> >>>> -/* Alloc and free functions for record_full_reg, record_full_mem, and
> >>>> - record_full_end entries. */
> >>>> +/* Initialization and cleanup functions for record_full_reg and
> >>>> + record_full_mem entries. */
> >>>>
> >>>> -/* Alloc a record_full_reg record entry. */
> >>>> +/* Init a record_full_reg record entry. */
> >>>>
> >>>> -static inline struct record_full_entry *
> >>>> -record_full_reg_alloc (struct regcache *regcache, int regnum)
> >>>> +static inline struct record_full_entry
> >>>> +record_full_reg_init (struct regcache *regcache, int regnum)
> >>>> {
> >>>> - struct record_full_entry *rec;
> >>>> + struct record_full_entry rec;
> >>>> struct gdbarch *gdbarch = regcache->arch ();
> >>>>
> >>>> - rec = XCNEW (struct record_full_entry);
> >>>> - rec->type = record_full_reg;
> >>>> - rec->u.reg.num = regnum;
> >>>> - rec->u.reg.len = register_size (gdbarch, regnum);
> >>>> - if (rec->u.reg.len > sizeof (rec->u.reg.u.buf))
> >>>> - rec->u.reg.u.ptr = (gdb_byte *) xmalloc (rec->u.reg.len);
> >>>> + rec.type = record_full_reg;
> >>>> + rec.u.reg.num = regnum;
> >>>> + rec.u.reg.len = register_size (gdbarch, regnum);
> >>>> + if (rec.u.reg.len > sizeof (rec.u.reg.u.buf))
> >>>> + rec.u.reg.u.ptr = (gdb_byte *) xmalloc (rec.u.reg.len);
> >>>>
> >>>> return rec;
> >>>> }
> >>>>
> >>>> -/* Free a record_full_reg record entry. */
> >>>> +/* Cleanup a record_full_reg record entry. */
> >>>>
> >>>> static inline void
> >>>> -record_full_reg_release (struct record_full_entry *rec)
> >>>> +record_full_reg_cleanup (struct record_full_entry rec)
> >>>> {
> >>>> - gdb_assert (rec->type == record_full_reg);
> >>>> - if (rec->u.reg.len > sizeof (rec->u.reg.u.buf))
> >>>> - xfree (rec->u.reg.u.ptr);
> >>>> - xfree (rec);
> >>>> + gdb_assert (rec.type == record_full_reg);
> >>>> + if (rec.u.reg.len > sizeof (rec.u.reg.u.buf))
> >>>> + xfree (rec.u.reg.u.ptr);
> >>>> }
> >>>>
> >>>> -/* Alloc a record_full_mem record entry. */
> >>>> +/* Init a record_full_mem record entry. */
> >>>>
> >>>> -static inline struct record_full_entry *
> >>>> -record_full_mem_alloc (CORE_ADDR addr, int len)
> >>>> +static inline struct record_full_entry
> >>>> +record_full_mem_init (CORE_ADDR addr, int len)
> >>>> {
> >>>> - struct record_full_entry *rec;
> >>>> + struct record_full_entry rec;
> >>>>
> >>>> - rec = XCNEW (struct record_full_entry);
> >>>> - rec->type = record_full_mem;
> >>>> - rec->u.mem.addr = addr;
> >>>> - rec->u.mem.len = len;
> >>>> - if (rec->u.mem.len > sizeof (rec->u.mem.u.buf))
> >>>> - rec->u.mem.u.ptr = (gdb_byte *) xmalloc (len);
> >>>> + rec.type = record_full_mem;
> >>>> + rec.u.mem.addr = addr;
> >>>> + rec.u.mem.len = len;
> >>>> + if (rec.u.mem.len > sizeof (rec.u.mem.u.buf))
> >>>> + rec.u.mem.u.ptr = (gdb_byte *) xmalloc (len);
> >>>> + rec.u.mem.mem_entry_not_accessible = 0;
> >>>>
> >>>> return rec;
> >>>> }
> >>>>
> >>>> -/* Free a record_full_mem record entry. */
> >>>> +/* Cleanup a record_full_mem record entry. */
> >>>>
> >>>> static inline void
> >>>> -record_full_mem_release (struct record_full_entry *rec)
> >>>> -{
> >>>> - gdb_assert (rec->type == record_full_mem);
> >>>> - if (rec->u.mem.len > sizeof (rec->u.mem.u.buf))
> >>>> - xfree (rec->u.mem.u.ptr);
> >>>> - xfree (rec);
> >>>> -}
> >>>> -
> >>>> -/* Alloc a record_full_end record entry. */
> >>>> -
> >>>> -static inline struct record_full_entry *
> >>>> -record_full_end_alloc (void)
> >>>> +record_full_mem_cleanup (struct record_full_entry rec)
> >>>> {
> >>>> - struct record_full_entry *rec;
> >>>> -
> >>>> - rec = XCNEW (struct record_full_entry);
> >>>> - rec->type = record_full_end;
> >>>> -
> >>>> - return rec;
> >>>> + gdb_assert (rec.type == record_full_mem);
> >>>> + if (rec.u.mem.len > sizeof (rec.u.mem.u.buf))
> >>>> + xfree (rec.u.mem.u.ptr);
> >>>> }
> >>>>
> >>>> -/* Free a record_full_end record entry. */
> >>>> +/* Free one record entry, any type. */
> >>>>
> >>>> static inline void
> >>>> -record_full_end_release (struct record_full_entry *rec)
> >>>> -{
> >>>> - xfree (rec);
> >>>> -}
> >>>> -
> >>>> -/* Free one record entry, any type.
> >>>> - Return entry->type, in case caller wants to know. */
> >>>> -
> >>>> -static inline enum record_full_type
> >>>> -record_full_entry_release (struct record_full_entry *rec)
> >>>> +record_full_entry_cleanup (struct record_full_entry rec)
> >>>> {
> >>>> - enum record_full_type type = rec->type;
> >>>>
> >>>> - switch (type) {
> >>>> + switch (rec.type) {
> >>>> case record_full_reg:
> >>>> - record_full_reg_release (rec);
> >>>> + record_full_reg_cleanup (rec);
> >>>> break;
> >>>> case record_full_mem:
> >>>> - record_full_mem_release (rec);
> >>>> - break;
> >>>> - case record_full_end:
> >>>> - record_full_end_release (rec);
> >>>> + record_full_mem_cleanup (rec);
> >>>> break;
> >>>> }
> >>>> - return type;
> >>>> }
> >>>>
> >>>> -/* Free all record entries in list pointed to by REC. */
> >>>> -
> >>>> static void
> >>>> -record_full_list_release (struct record_full_entry *rec)
> >>>> +record_full_reset_history ()
> >>>> {
> >>>> - if (!rec)
> >>>> - return;
> >>>> -
> >>>> - while (rec->next)
> >>>> - rec = rec->next;
> >>>> + record_full_insn_num = 0;
> >>>> + record_full_insn_count = 0;
> >>>> + record_full_next_insn = 0;
> >>>>
> >>>> - while (rec->prev)
> >>>> + for (auto &insn : record_full_list)
> >>>> {
> >>>> - rec = rec->prev;
> >>>> - record_full_entry_release (rec->next);
> >>>> + for (auto &entry : insn.effects)
> >>>> + record_full_entry_cleanup (entry);
> >>>> }
> >>>>
> >>>> - if (rec == &record_full_first)
> >>>> - {
> >>>> - record_full_insn_num = 0;
> >>>> - record_full_first.next = NULL;
> >>>> - }
> >>>> - else
> >>>> - record_full_entry_release (rec);
> >>>> + record_full_list.clear ();
> >>>> }
> >>>>
> >>>> -/* Free all record entries forward of the given list position. */
> >>>> -
> >>>> static void
> >>>> -record_full_list_release_following (struct record_full_entry *rec)
> >>>> +record_full_list_release_following (int index)
> >>>> {
> >>>> - struct record_full_entry *tmp = rec->next;
> >>>> -
> >>>> - rec->next = NULL;
> >>>> - while (tmp)
> >>>> + for (int i = record_full_list.size () - 1; i > index; i--)
> >>>> {
> >>>> - rec = tmp->next;
> >>>> - if (record_full_entry_release (tmp) == record_full_end)
> >>>> - {
> >>>> - record_full_insn_num--;
> >>>> - record_full_insn_count--;
> >>>> - }
> >>>> - tmp = rec;
> >>>> + for (auto &entry : record_full_list[i].effects)
> >>>> + record_full_entry_cleanup (entry);
> >>>> + record_full_list.pop_back ();
> >>>> }
> >>>> + /* Set the next instruction to be past the end of the log so we
> >>>> + start recording if the user moves forward again. */
> >>>> + record_full_next_insn = index;
> >>>> +}
> >>>> +static void
> >>>> +record_full_save_instruction ()
> > A function comment would be nice.
> >
> >>>> +{
> >>>> + ++record_full_insn_count;
> >>>> + record_full_incomplete_instruction.insn_num =
> record_full_insn_count;
> >>>> + record_full_incomplete_instruction.effects.shrink_to_fit ();
> >>>> + record_full_list.push_back (record_full_incomplete_instruction);
> > I think this line will at some point call the default copy constructor of
> record_full_mem_entry
> > and record_full_reg_entry, which have those buffers:
> >
> > gdb_byte buf[sizeof (gdb_byte *)];
> >
> > IIUC you'll only have a shallow copy. Should we use a std::move instead
> here?
> Might be worth, yeah...
> >
> > I further wonder if this problem might be related to the double free issues
> you
> > describe in patch #3. Would you mind checking?
> I did check it! When I was working on patch #3, I disabled copy
> construction, only allowed move constructor/assignment because I
> specifically thought that the vector push_back was the reason for the
> double frees, but it still happened.
That's weird, it would have perfectly explained it.
So with std::move we still have this problem?
Would you mind sharing some backtrace of this double free error ?
> > And if you add the destructor, I think it would make sense to add a copy
> constructor etc., too:
> > https://en.cppreference.com/cpp/language/rule_of_three
> >
> >>>> + record_full_next_insn++;
> >>>> }
> >>>>
> >>>> /* Delete the first instruction from the beginning of the log, to make
> >>>> @@ -550,52 +504,22 @@ record_full_list_release_following (struct
> >>>> record_full_entry *rec)
> >>>> static void
> >>>> record_full_list_release_first (void)
> >>>> {
> >>>> - struct record_full_entry *tmp;
> >>>> -
> >>>> - if (!record_full_first.next)
> >>>> + if (record_full_list.empty ())
> >>>> return;
> >>>>
> >>>> - /* Loop until a record_full_end. */
> >>>> - while (1)
> >>>> - {
> >>>> - /* Cut record_full_first.next out of the linked list. */
> >>>> - tmp = record_full_first.next;
> >>>> - record_full_first.next = tmp->next;
> >>>> - tmp->next->prev = &record_full_first;
> >>>> -
> >>>> - /* tmp is now isolated, and can be deleted. */
> >>>> - if (record_full_entry_release (tmp) == record_full_end)
> >>>> - break; /* End loop at first record_full_end. */
> >>>> + for (auto &entry : record_full_list[0].effects)
> >>>> + record_full_entry_cleanup (entry);
> >>>>
> >>>> - if (!record_full_first.next)
> >>>> - {
> >>>> - gdb_assert (record_full_insn_num == 1);
> >>>> - break; /* End loop when list is empty. */
> >>>> - }
> >>>> - }
> >>>> + record_full_list.pop_front ();
> >>>> + --record_full_next_insn;
> >>>> }
> >>>>
> >>>> /* Add a struct record_full_entry to record_full_arch_list. */
> >>>>
> >>>> static void
> >>>> -record_full_arch_list_add (struct record_full_entry *rec)
> >>>> +record_full_arch_list_add (struct record_full_entry &rec)
> >>>> {
> >>>> - if (record_debug > 1)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - "Process record: record_full_arch_list_add %s.\n",
> >>>> - host_address_to_string (rec));
> >>>> -
> >>>> - if (record_full_arch_list_tail)
> >>>> - {
> >>>> - record_full_arch_list_tail->next = rec;
> >>>> - rec->prev = record_full_arch_list_tail;
> >>>> - record_full_arch_list_tail = rec;
> >>>> - }
> >>>> - else
> >>>> - {
> >>>> - record_full_arch_list_head = rec;
> >>>> - record_full_arch_list_tail = rec;
> >>>> - }
> >>>> + record_full_incomplete_instruction.effects.push_back (rec);
> >>>> }
> >>>>
> >>>> /* Return the value storage location of a record entry. */
> >>>> @@ -613,7 +537,6 @@ record_full_get_loc (struct record_full_entry
> *rec)
> >>>> return rec->u.reg.u.ptr;
> >>>> else
> >>>> return rec->u.reg.u.buf;
> >>>> - case record_full_end:
> >>>> default:
> >>>> gdb_assert_not_reached ("unexpected record_full_entry type");
> >>>> return NULL;
> >>>> @@ -625,7 +548,7 @@ record_full_get_loc (struct record_full_entry
> *rec)
> >>>> int
> >>>> record_full_arch_list_add_reg (struct regcache *regcache, int regnum)
> >>>> {
> >>>> - struct record_full_entry *rec;
> >>>> + struct record_full_entry rec;
> >>> I think we can omit the struct keyword here (there are some more
> >> occurrences in this patch).
> >>>> if (record_debug > 1)
> >>>> gdb_printf (gdb_stdlog,
> >>>> @@ -633,9 +556,9 @@ record_full_arch_list_add_reg (struct regcache
> >>>> *regcache, int regnum)
> >>>> "record list.\n",
> >>>> regnum);
> >>>>
> >>>> - rec = record_full_reg_alloc (regcache, regnum);
> >>>> + rec = record_full_reg_init (regcache, regnum);
> >>>>
> >>>> - regcache->cooked_read (regnum, record_full_get_loc (rec));
> >>>> + regcache->cooked_read (regnum, record_full_get_loc (&rec));
> >>>>
> >>>> record_full_arch_list_add (rec);
> >>>>
> >>>> @@ -648,7 +571,7 @@ record_full_arch_list_add_reg (struct regcache
> >>>> *regcache, int regnum)
> >>>> int
> >>>> record_full_arch_list_add_mem (CORE_ADDR addr, int len)
> >>>> {
> >>>> - struct record_full_entry *rec;
> >>>> + struct record_full_entry rec;
> >>>>
> >>>> if (record_debug > 1)
> >>>> gdb_printf (gdb_stdlog,
> >>>> @@ -659,12 +582,12 @@ record_full_arch_list_add_mem
> (CORE_ADDR
> >> addr,
> >>>> int len)
> >>>> if (!addr) /* FIXME: Why? Some arch must permit it... */
> >>>> return 0;
> >>>>
> >>>> - rec = record_full_mem_alloc (addr, len);
> >>>> + rec = record_full_mem_init (addr, len);
> >>>>
> >>>> if (record_read_memory (current_inferior ()->arch (), addr,
> >>>> - record_full_get_loc (rec), len))
> >>>> + record_full_get_loc (&rec), len))
> >>>> {
> >>>> - record_full_mem_release (rec);
> >>>> + record_full_mem_cleanup (rec);
> >>>> return -1;
> >>>> }
> >>>>
> >>>> @@ -673,27 +596,6 @@ record_full_arch_list_add_mem (CORE_ADDR
> >> addr,
> >>>> int len)
> >>>> return 0;
> >>>> }
> >>>>
> >>>> -/* Add a record_full_end type struct record_full_entry to
> >>>> - record_full_arch_list. */
> >>>> -
> >>>> -int
> >>>> -record_full_arch_list_add_end (void)
> >>>> -{
> >>>> - struct record_full_entry *rec;
> >>>> -
> >>>> - if (record_debug > 1)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - "Process record: add end to arch list.\n");
> >>>> -
> >>>> - rec = record_full_end_alloc ();
> >>>> - rec->u.end.sigval = GDB_SIGNAL_0;
> >>>> - rec->u.end.insn_num = ++record_full_insn_count;
> >>>> -
> >>>> - record_full_arch_list_add (rec);
> >>>> -
> >>>> - return 0;
> >>>> -}
> >>>> -
> >>>> static void
> >>>> record_full_check_insn_num (void)
> >>>> {
> >>>> @@ -725,8 +627,7 @@ record_full_message (struct regcache
> *regcache,
> >>>> enum gdb_signal signal)
> >>>>
> >>>> try
> >>>> {
> >>>> - record_full_arch_list_head = NULL;
> >>>> - record_full_arch_list_tail = NULL;
> >>>> + record_full_reset_incomplete ();
> >>>>
> >>>> /* Check record_full_insn_num. */
> >>>> record_full_check_insn_num ();
> >>>> @@ -746,20 +647,16 @@ record_full_message (struct regcache
> >> *regcache,
> >>>> enum gdb_signal signal)
> >>>> the user says something different, like "deliver this signal"
> >>>> during the replay mode).
> >>>>
> >>>> - User should understand that nothing he does during the replay
> >>>> - mode will change the behavior of the child. If he tries,
> >>>> + User should understand that nothing they do during the replay
> >>>> + mode will change the behavior of the child. If they try,
> >>>> then that is a user error.
> >>>>
> >>>> But we should still deliver the signal to gdb during the replay,
> >>>> if we delivered it during the recording. Therefore we should
> >>>> record the signal during record_full_wait, not
> >>>> record_full_resume. */
> >>>> - if (record_full_list != &record_full_first) /* FIXME better way
> >>>> - to check */
> >>>> - {
> >>>> - gdb_assert (record_full_list->type == record_full_end);
> >>>> - record_full_list->u.end.sigval = signal;
> >>>> - }
> >>>> + if (signal != GDB_SIGNAL_0)
> >>>> + record_full_list[record_full_next_insn - 1].sigval = signal;
> > Similar comment as already in my earlier review:
> > Can record_full_next_insn be == 0 here or may record_full_list be empty ?
> Oh you're right. This is probably because I misunderstood the order of
> operations and was saving it to the current one, not the previous one,
> and when I fixed that I forgot to check the original
> >
> > I think it might be worth checking the patch if there are more similar issues.
> > The old patch had more guards to check this.
> yeah good point, I'll look through the changes and see if I notice it
> anywhere else.
> >
> > And I noticed in some other locations you cast it:
> >
> > if (sigval != GDB_SIGNAL_0)
> > record_full_incomplete_instruction.sigval = (gdb_signal) sigval;
> >
> > Not sure if this is necessary or not, but it might be for checking and making
> it consistent?
> I cast it in the other location because it is reading from the BFD and
> so sigval is declared as uint8_t, not as gdb_signal.
> > And is it intended that now we don't clear sigval anymore?
> >
> > Shouldn't we set it to std::nullopt in case signal == GDB_SIGNAL_0, to
> match the original
> > behaviour?
>
> yeah this is a mistake. The plan was that record_full_save_instruction
> clears the incomplete instruction, but I apparently forgot that.
>
> With that, it should always be clear so we won't need to explicitly set
> it to nullopt.
>
> I'll update the correct function.
>
> >
> >>>> if (signal == GDB_SIGNAL_0
> >>>> || !gdbarch_process_record_signal_p (gdbarch))
> >>>> @@ -778,13 +675,11 @@ record_full_message (struct regcache
> >> *regcache,
> >>>> enum gdb_signal signal)
> >>>> }
> >>>> catch (const gdb_exception &ex)
> >>>> {
> >>>> - record_full_list_release (record_full_arch_list_tail);
> >>>> + record_full_reset_incomplete ();
> >>>> throw;
> >>>> }
> >>>>
> >>>> - record_full_list->next = record_full_arch_list_head;
> >>>> - record_full_arch_list_head->prev = record_full_list;
> >>>> - record_full_list = record_full_arch_list_tail;
> >>>> + record_full_save_instruction ();
> >>>>
> >>>> if (record_full_insn_num == record_full_insn_max_num)
> >>>> record_full_list_release_first ();
> >>>> @@ -829,9 +724,9 @@ static enum target_stop_reason
> >>>> record_full_stop_reason
> >>>> entries and memory entries, followed by an 'end' entry. */
> >>>>
> >>>> static inline void
> >>>> -record_full_exec_insn (struct regcache *regcache,
> >>>> - struct gdbarch *gdbarch,
> >>>> - struct record_full_entry *entry)
> >>>> +record_full_exec_entry (struct regcache *regcache,
> >>>> + struct gdbarch *gdbarch,
> >>>> + struct record_full_entry *entry)
> >>> Nit: omit struct keywords
> >>> And there are some more places in this series/patch, but I will stop
> >> commenting
> >>> now on all of them.
> >>>
> >>>> {
> >>>> switch (entry->type)
> >>>> {
> >>>> @@ -909,6 +804,15 @@ record_full_exec_insn (struct regcache
> >> *regcache,
> >>>> }
> >>>> }
> >>>>
> >>>> +static inline void
> >>>> +record_full_exec_insn (struct regcache *regcache,
> >>>> + struct gdbarch *gdbarch,
> >>>> + record_full_instruction &insn)
> >>>> +{
> >>>> + for (auto &entry : insn.effects)
> >>>> + record_full_exec_entry (regcache, gdbarch, &entry);
> >>>> +}
> >>>> +
> >>>> static void record_full_restore (struct bfd &cbfd);
> >>>>
> >>>> /* Asynchronous signal handle registered as event loop source for
> when
> >>>> @@ -983,10 +887,7 @@ record_full_open (const char *args, int
> from_tty)
> >>>> record_preopen ();
> >>>>
> >>>> /* Reset */
> >>>> - record_full_insn_num = 0;
> >>>> - record_full_insn_count = 0;
> >>>> - record_full_list = &record_full_first;
> >>>> - record_full_list->next = NULL;
> >>>> + record_full_reset_history ();
> >>>>
> >>>> bfd *cbfd = get_inferior_core_bfd (current_inferior ());
> >>>> if (cbfd != nullptr)
> >>>> @@ -1014,7 +915,7 @@ record_full_base_target::close ()
> >>>> if (record_debug)
> >>>> gdb_printf (gdb_stdlog, "Process record: record_full_close\n");
> >>>>
> >>>> - record_full_list_release (record_full_list);
> >>>> + record_full_reset_history ();
> >>>>
> >>>> /* Release record_full_core_regbuf. */
> >>>> if (record_full_core_regbuf)
> >>>> @@ -1313,7 +1214,6 @@ record_full_wait_1 (struct target_ops *ops,
> >>>> struct gdbarch *gdbarch = regcache->arch ();
> >>>> const address_space *aspace = current_inferior ()->aspace.get ();
> >>>> int continue_flag = 1;
> >>>> - int first_record_full_end = 1;
> >>>>
> >>>> try
> >>>> {
> >>>> @@ -1322,7 +1222,6 @@ record_full_wait_1 (struct target_ops *ops,
> >>>> record_full_stop_reason = TARGET_STOPPED_BY_NO_REASON;
> >>>> status->set_stopped (GDB_SIGNAL_0);
> >>>>
> >>>> - /* Check breakpoint when forward execute. */
> >>>> if (execution_direction == EXEC_FORWARD)
> >>>> {
> >>>> tmp_pc = regcache_read_pc (regcache);
> >>>> @@ -1330,8 +1229,7 @@ record_full_wait_1 (struct target_ops *ops,
> >>>>
> >>>> &record_full_stop_reason))
> >>>> {
> >>>> if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - "Process record: break at %s.\n",
> >>>> + gdb_printf (gdb_stdlog, "Process record: break at %s.\n",
> >>>> paddress (gdbarch, tmp_pc));
> >>>> goto replay_out;
> >>>> }
> >>>> @@ -1344,10 +1242,10 @@ record_full_wait_1 (struct target_ops
> *ops,
> >>>> the signal. */
> >>>> target_terminal::ours ();
> >>>>
> >>>> - /* In EXEC_FORWARD mode, record_full_list points to the tail of
> >> prev
> >>>> - instruction. */
> >>>> - if (execution_direction == EXEC_FORWARD && record_full_list-
> >>> next)
> >>>> - record_full_list = record_full_list->next;
> >>>> + /* In EXEC_FORWARD mode, record_full_next_insn is the next
> >>>> + instruction to be executed. */
> >>>> + if (execution_direction == EXEC_REVERSE)
> >>>> + record_full_next_insn--;
> >>>>
> >>>> /* Loop over the record_full_list, looking for the next place to
> >>>> stop. */
> >>>> @@ -1355,108 +1253,92 @@ record_full_wait_1 (struct target_ops
> *ops,
> >>>> {
> >>>> /* Check for beginning and end of log. */
> >>>> if (execution_direction == EXEC_REVERSE
> >>>> - && record_full_list == &record_full_first)
> >>>> + && record_full_next_insn < 0)
> >>>> {
> >>>> /* Hit beginning of record log in reverse. */
> >>>> status->set_no_history ();
> >>>> + record_full_next_insn = 0;
> >>>> break;
> >>>> }
> >>>> if (execution_direction != EXEC_REVERSE
> >>>> - && !record_full_list->next)
> >>>> + && record_full_next_insn == record_full_list.size ())
> >>>> {
> >>>> /* Hit end of record log going forward. */
> >>>> status->set_no_history ();
> >>>> break;
> >>>> }
> >>>>
> >>>> - record_full_exec_insn (regcache, gdbarch, record_full_list);
> >>>> + record_full_exec_insn
> >>>> + (regcache, gdbarch,
> >>>> + record_full_list[record_full_next_insn]);
> >>>>
> >>>> - if (record_full_list->type == record_full_end)
> >>>> + /* step */
> >>>> + if (record_full_resume_step)
> >>>> {
> >>>> if (record_debug > 1)
> >>>> - gdb_printf
> >>>> - (gdb_stdlog,
> >>>> - "Process record: record_full_end %s to "
> >>>> - "inferior.\n",
> >>>> - host_address_to_string (record_full_list));
> >>>> -
> >>>> - if (first_record_full_end
> >>>> - && execution_direction == EXEC_REVERSE)
> >>>> - {
> >>>> - /* When reverse execute, the first
> >>>> - record_full_end is the part of current
> >>>> - instruction. */
> >>>> - first_record_full_end = 0;
> >>>> - }
> >>>> - else
> >>>> - {
> >>>> - /* In EXEC_REVERSE mode, this is the
> >>>> - record_full_end of prev instruction. In
> >>>> - EXEC_FORWARD mode, this is the
> >>>> - record_full_end of current instruction. */
> >>>> - /* step */
> >>>> - if (record_full_resume_step)
> >>>> - {
> >>>> - if (record_debug > 1)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - "Process record: step.\n");
> >>>> - continue_flag = 0;
> >>>> - }
> >>>> -
> >>>> - /* check breakpoint */
> >>>> - tmp_pc = regcache_read_pc (regcache);
> >>>> - if (record_check_stopped_by_breakpoint
> >>>> - (aspace, tmp_pc, &record_full_stop_reason))
> >>>> - {
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - "Process record: break "
> >>>> - "at %s.\n",
> >>>> - paddress (gdbarch, tmp_pc));
> >>>> + gdb_printf (gdb_stdlog,
> >>>> + "Process record: step.\n");
> >>>> + continue_flag = 0;
> >>>> + }
> >>>>
> >>>> - continue_flag = 0;
> >>>> - }
> >>>> + /* check breakpoint */
> >>>> + tmp_pc = regcache_read_pc (regcache);
> >>>> + if (record_check_stopped_by_breakpoint
> >>>> + (aspace, tmp_pc, &record_full_stop_reason))
> >>>> + {
> >>>> + if (record_debug)
> >>>> + gdb_printf (gdb_stdlog,
> >>>> + "Process record: break "
> >>>> + "at %s.\n",
> >>>> + paddress (gdbarch, tmp_pc));
> >>>>
> >>>> - if (record_full_stop_reason
> >>>> - == TARGET_STOPPED_BY_WATCHPOINT)
> >>>> - {
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - "Process record: hit hw "
> >>>> - "watchpoint.\n");
> >>>> - continue_flag = 0;
> >>>> - }
> >>>> - /* Check target signal */
> >>>> - if (record_full_list->u.end.sigval != GDB_SIGNAL_0)
> >>>> - /* FIXME: better way to check */
> >>>> - continue_flag = 0;
> >>>> - }
> >>>> + continue_flag = 0;
> >>>> }
> >>>>
> >>>> - if (continue_flag)
> >>>> + if (record_full_stop_reason
> >>>> + == TARGET_STOPPED_BY_WATCHPOINT)
> >>>> {
> >>>> - if (execution_direction == EXEC_REVERSE)
> >>>> - {
> >>>> - if (record_full_list->prev)
> >>>> - record_full_list = record_full_list->prev;
> >>>> - }
> >>>> - else
> >>>> - {
> >>>> - if (record_full_list->next)
> >>>> - record_full_list = record_full_list->next;
> >>>> - }
> >>>> + if (record_debug)
> >>>> + gdb_printf (gdb_stdlog,
> >>>> + "Process record: hit hw "
> >>>> + "watchpoint.\n");
> >>>> + continue_flag = 0;
> >>>> }
> >>>> + if (record_full_list[record_full_next_insn].sigval.has_value ())
> >>>> + continue_flag = 0;
> >>>> +
> >>>> + if (execution_direction == EXEC_REVERSE)
> >>>> + record_full_next_insn--;
> >>>> + else
> >>>> + record_full_next_insn++;
> >>>> }
> >>>> while (continue_flag);
> >>>>
> >>>> + if (record_full_next_insn < 0)
> >>>> + {
> >>>> + gdb_assert (execution_direction == EXEC_REVERSE);
> >>>> + record_full_next_insn = 0;
> >>>> + }
> >>>> + else if (record_full_next_insn > record_full_list.size ())
> >>>> + {
> >>>> + gdb_assert (execution_direction == EXEC_FORWARD);
> >>>> + record_full_next_insn = record_full_list.size ();
> >>>> + }
> >>>> + /* Reset the current instruciton to point to the one to be replayed
> >>> Nit: instruction
> >>>
> >>>> + moving forward. */
> >>>> + else if (execution_direction == EXEC_REVERSE)
> >>>> + record_full_next_insn++;
> >>>> +
> >>>> replay_out:
> >>>> if (status->kind () == TARGET_WAITKIND_STOPPED)
> >>>> {
> >>>> + int insn = (execution_direction == EXEC_FORWARD)
> >>>> + ? record_full_next_insn - 1 : record_full_next_insn;
> >>>> if (record_full_get_sig)
> >>>> status->set_stopped (GDB_SIGNAL_INT);
> >>>> - else if (record_full_list->u.end.sigval != GDB_SIGNAL_0)
> >>>> - /* FIXME: better way to check */
> >>>> - status->set_stopped (record_full_list->u.end.sigval);
> >>>> + else if (record_full_list[insn].sigval.has_value ())
> >>>> + status->set_stopped
> >>>> + (record_full_list[insn].sigval.value ());
> >>>> else
> >>>> status->set_stopped (GDB_SIGNAL_TRAP);
> >>>> }
> >>>> @@ -1464,12 +1346,9 @@ record_full_wait_1 (struct target_ops *ops,
> >>>> catch (const gdb_exception &ex)
> >>>> {
> >>>> if (execution_direction == EXEC_REVERSE)
> >>>> - {
> >>>> - if (record_full_list->next)
> >>>> - record_full_list = record_full_list->next;
> >>>> - }
> >>>> + record_full_next_insn++;
> >>>> else
> >>>> - record_full_list = record_full_list->prev;
> >>>> + record_full_next_insn--;
> >>>>
> >>>> throw;
> >>>> }
> >>>> @@ -1557,8 +1436,7 @@ record_full_registers_change (struct
> regcache
> >>>> *regcache, int regnum)
> >>>> /* Check record_full_insn_num. */
> >>>> record_full_check_insn_num ();
> >>>>
> >>>> - record_full_arch_list_head = NULL;
> >>>> - record_full_arch_list_tail = NULL;
> >>>> + record_full_reset_incomplete ();
> >>>>
> >>>> if (regnum < 0)
> >>>> {
> >>>> @@ -1568,7 +1446,7 @@ record_full_registers_change (struct
> regcache
> >>>> *regcache, int regnum)
> >>>> {
> >>>> if (record_full_arch_list_add_reg (regcache, i))
> >>>> {
> >>>> - record_full_list_release (record_full_arch_list_tail);
> >>>> + record_full_reset_incomplete ();
> >>>> error (_("Process record: failed to record execution log."));
> >>>> }
> >>>> }
> >>>> @@ -1577,18 +1455,11 @@ record_full_registers_change (struct
> >> regcache
> >>>> *regcache, int regnum)
> >>>> {
> >>>> if (record_full_arch_list_add_reg (regcache, regnum))
> >>>> {
> >>>> - record_full_list_release (record_full_arch_list_tail);
> >>>> + record_full_reset_incomplete ();
> >>>> error (_("Process record: failed to record execution log."));
> >>>> }
> >>>> }
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - {
> >>>> - record_full_list_release (record_full_arch_list_tail);
> >>>> - error (_("Process record: failed to record execution log."));
> >>>> - }
> >>>> - record_full_list->next = record_full_arch_list_head;
> >>>> - record_full_arch_list_head->prev = record_full_list;
> >>>> - record_full_list = record_full_arch_list_tail;
> >>>> + record_full_save_instruction ();
> >>>>
> >>>> if (record_full_insn_num == record_full_insn_max_num)
> >>>> record_full_list_release_first ();
> >>>> @@ -1607,7 +1478,7 @@ record_full_target::store_registers (struct
> >>>> regcache *regcache, int regno)
> >>>> {
> >>>> int n;
> >>>>
> >>>> - /* Let user choose if he wants to write register or not. */
> >>>> + /* Let user choose if they want to write register or not. */
> >>>> if (regno < 0)
> >>>> n =
> >>>> query (_("Because GDB is in replay mode, changing the "
> >>>> @@ -1642,7 +1513,7 @@ record_full_target::store_registers (struct
> >>>> regcache *regcache, int regno)
> >>>> }
> >>>>
> >>>> /* Destroy the record from here forward. */
> >>>> - record_full_list_release_following (record_full_list);
> >>>> + record_full_list_release_following (record_full_next_insn);
> >>>> }
> >>>>
> >>>> record_full_registers_change (regcache, regno);
> >>>> @@ -1675,36 +1546,24 @@ record_full_target::xfer_partial (enum
> >>>> target_object object,
> >>>> error (_("Process record canceled the operation."));
> >>>>
> >>>> /* Destroy the record from here forward. */
> >>>> - record_full_list_release_following (record_full_list);
> >>>> + record_full_list_release_following (record_full_next_insn);
> >>>> }
> >>>>
> >>>> /* Check record_full_insn_num */
> >>>> record_full_check_insn_num ();
> >>>>
> >>>> /* Record registers change to list as an instruction. */
> >>>> - record_full_arch_list_head = NULL;
> >>>> - record_full_arch_list_tail = NULL;
> >>>> + record_full_reset_incomplete ();
> >>>> if (record_full_arch_list_add_mem (offset, len))
> >>>> {
> >>>> - record_full_list_release (record_full_arch_list_tail);
> >>>> + record_full_reset_incomplete ();
> >>>> if (record_debug)
> >>>> gdb_printf (gdb_stdlog,
> >>>> "Process record: failed to record "
> >>>> "execution log.");
> >>>> return TARGET_XFER_E_IO;
> >>>> }
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - {
> >>>> - record_full_list_release (record_full_arch_list_tail);
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - "Process record: failed to record "
> >>>> - "execution log.");
> >>>> - return TARGET_XFER_E_IO;
> >>>> - }
> >>>> - record_full_list->next = record_full_arch_list_head;
> >>>> - record_full_arch_list_head->prev = record_full_list;
> >>>> - record_full_list = record_full_arch_list_tail;
> >>>> + record_full_save_instruction ();
> >>>>
> >>>> if (record_full_insn_num == record_full_insn_max_num)
> >>>> record_full_list_release_first ();
> >>>> @@ -1866,8 +1725,7 @@ record_full_base_target::get_bookmark
> (const
> >> char
> >>>> *args, int from_tty)
> >>>> char *ret = NULL;
> >>>>
> >>>> /* Return stringified form of instruction count. */
> >>>> - if (record_full_list && record_full_list->type == record_full_end)
> >>>> - ret = xstrdup (pulongest (record_full_list->u.end.insn_num));
> >>>> + ret = xstrdup (pulongest
> >> (record_full_list[record_full_next_insn].insn_num));
> >>> I think we can get here with an empty record_full_list - in case we are in
> >> recording mode,
> >>> no instructions have been logged and this function is called:
> >>>
> >>> ~~~
> >>> (gdb) info record
> >>> Active record target: record-full
> >>> Record mode:
> >>> No instructions have been logged.
> >>> Max logged instructions is 200000.
> >>> (gdb) bookmark
> >>>
> >>> Thread 1 "gdb-up" hit Breakpoint 1,
> record_full_base_target::get_bookmark
> >> (this=0x555558392900 <record_full_ops>, args=0x0, from_tty=1) at
> >> /tmp/record-full.c:1774
> >>> 1774 char *ret = NULL;
> >>> (gdb) p record_full_next_insn
> >>> $1 = 0
> >>> (gdb) p record_full_list.empty ()
> >>> $2 = true
> >>> ~~~
> > I wonder, do we somewhere have a test for bookmarks?
> > I also find a specific description for this in the manual... what's the
> difference to checkpoints?
>
> Checkpoints fork the inferior and stop the fork immediately, allowing
> you to go back to that one point in time, but not executing backwards or
> anything, just changing forks and letting you see what things were like
> back then (that's why they won't work with multi-threaded programs).
> entirely unrelated to reverse debugging
>
> Bookmarks let you mark one instruction in the history and easily go back
> to it using the reverse mechanism... at least that's what I've been able
> to get from the code, this predates me and I have no idea how it is
> actually used....
>
> I quickly grep'd for bookmark tests and couldn't find any. We should
> probably add them at some point, if anyone even uses that feature......
Or at least adding some documentation, maybe more people would use it then ... 😉
Otherwise, if nobody uses it, we could deprecate and remove it at some point?
It just adds even more code to maintain...
> >
> > I am a bit confused about the bookmark feature (of course, this is nothing
> to fix for this series...).
> >
> >>>> if (record_debug)
> >>>> {
> >>>> @@ -1923,30 +1781,22 @@ record_full_base_target::record_method
> >> (ptid_t
> >>>> ptid)
> >>>> void
> >>>> record_full_base_target::info_record ()
> >>>> {
> >>>> - struct record_full_entry *p;
> >>>> -
> >>>> if (RECORD_FULL_IS_REPLAY)
> >>>> gdb_printf (_("Replay mode:\n"));
> >>>> else
> >>>> gdb_printf (_("Record mode:\n"));
> >>>>
> >>>> - /* Find entry for first actual instruction in the log. */
> >>>> - for (p = record_full_first.next;
> >>>> - p != NULL && p->type != record_full_end;
> >>>> - p = p->next)
> >>>> - ;
> >>>> -
> >>>> /* Do we have a log at all? */
> >>>> - if (p != NULL && p->type == record_full_end)
> >>>> + if (!record_full_list.empty ())
> >>>> {
> >>>> /* Display instruction number for first instruction in the log. */
> >>>> - gdb_printf (_("Lowest recorded instruction number is %s.\n"),
> >>>> - pulongest (p->u.end.insn_num));
> >>>> + gdb_printf (_("Lowest recorded instruction number is %u.\n"),
> >>>> + record_full_list[0].insn_num);
> >>>>
> >>>> /* If in replay mode, display where we are in the log. */
> >>>> if (RECORD_FULL_IS_REPLAY)
> >>>> - gdb_printf (_("Current instruction number is %s.\n"),
> >>>> - pulongest (record_full_list->u.end.insn_num));
> >>>> + gdb_printf (_("Current instruction number is %u.\n"),
> >>>> + record_full_list[record_full_next_insn].insn_num);
> >>>>
> >>>> /* Display instruction number for last instruction in the log. */
> >>>> gdb_printf (_("Highest recorded instruction number is %s.\n"),
> >>>> @@ -1975,7 +1825,7 @@
> >> record_full_base_target::supports_delete_record ()
> >>>> void
> >>>> record_full_base_target::delete_record ()
> >>>> {
> >>>> - record_full_list_release_following (record_full_list);
> >>>> + record_full_reset_history ();
> >>>> }
> >>>>
> >>>> /* The "record_is_replaying" target method. */
> >>>> @@ -2001,23 +1851,23 @@
> record_full_base_target::record_will_replay
> >>>> (ptid_t ptid, int dir)
> >>>> /* Go to a specific entry. */
> >>>>
> >>>> static void
> >>>> -record_full_goto_entry (struct record_full_entry *p)
> >>>> +record_full_goto_entry (size_t target_insn)
> >>>> {
> >>>> - if (p == NULL)
> >>>> + if (target_insn >= record_full_list.size ())
> >>>> error (_("Target insn not found."));
> >>>> - else if (p == record_full_list)
> >>>> + else if (target_insn == record_full_next_insn)
> >>>> error (_("Already at target insn."));
> >>>> - else if (p->u.end.insn_num > record_full_list->u.end.insn_num)
> >>>> + else if (target_insn > record_full_next_insn)
> >>>> {
> >>>> gdb_printf (_("Go forward to insn number %s\n"),
> >>>> - pulongest (p->u.end.insn_num));
> >>>> - record_full_goto_insn (p, EXEC_FORWARD);
> >>>> + pulongest (record_full_list[target_insn].insn_num));
> >>>> + record_full_goto_insn (target_insn, EXEC_FORWARD);
> >>>> }
> >>>> else
> >>>> {
> >>>> gdb_printf (_("Go backward to insn number %s\n"),
> >>>> - pulongest (p->u.end.insn_num));
> >>>> - record_full_goto_insn (p, EXEC_REVERSE);
> >>>> + pulongest (target_insn));
> >>>> + record_full_goto_insn (target_insn, EXEC_REVERSE);
> >>>> }
> >>>>
> >>>> registers_changed ();
> >>>> @@ -2033,13 +1883,7 @@ record_full_goto_entry (struct
> >> record_full_entry
> >>>> *p)
> >>>> void
> >>>> record_full_base_target::goto_record_begin ()
> >>>> {
> >>>> - struct record_full_entry *p = NULL;
> >>>> -
> >>>> - for (p = &record_full_first; p != NULL; p = p->next)
> >>>> - if (p->type == record_full_end)
> >>>> - break;
> >>>> -
> >>>> - record_full_goto_entry (p);
> >>>> + record_full_goto_entry (0);
> >>>> }
> >>>>
> >>>> /* The "goto_record_end" target method. */
> >>>> @@ -2047,15 +1891,7 @@
> record_full_base_target::goto_record_begin ()
> >>>> void
> >>>> record_full_base_target::goto_record_end ()
> >>>> {
> >>>> - struct record_full_entry *p = NULL;
> >>>> -
> >>>> - for (p = record_full_list; p->next != NULL; p = p->next)
> >>>> - ;
> >>>> - for (; p!= NULL; p = p->prev)
> >>>> - if (p->type == record_full_end)
> >>>> - break;
> >>>> -
> >>>> - record_full_goto_entry (p);
> >>>> + record_full_goto_entry (record_full_list.size () -1);
> >>> Nit: whitespace before 1 => "record_full_list.size () - 1"
> >>>
> >>> And in case the record_full_list is empty, won't we call
> >>> record_full_goto_entry with an invalid value for target_insn ?
> >>>
> >>>> }
> >>>>
> >>>> /* The "goto_record" target method. */
> >>>> @@ -2063,13 +1899,7 @@ record_full_base_target::goto_record_end
> ()
> >>>> void
> >>>> record_full_base_target::goto_record (ULONGEST target_insn)
> >>>> {
> >>>> - struct record_full_entry *p = NULL;
> >>>> -
> >>>> - for (p = &record_full_first; p != NULL; p = p->next)
> >>>> - if (p->type == record_full_end && p->u.end.insn_num == target_insn)
> >>>> - break;
> >>>> -
> >>>> - record_full_goto_entry (p);
> >>>> + record_full_goto_entry (target_insn);
> >>>> }
> >>>>
> >>>> /* The "record_stop_replaying" target method. */
> >>>> @@ -2339,19 +2169,96 @@ netorder32 (uint32_t input)
> >>>> return ret;
> >>>> }
> >>>>
> >>>> +static void
> >>>> +record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int
> >> *bfd_offset)
> >>>> +{
> >>>> + uint8_t rectype;
> >>>> + uint32_t regnum, len;
> >>>> + uint64_t addr;
> >>>> + regcache *cache = get_thread_regcache (inferior_thread ());
> >>>> +
> >>>> + bfdcore_read (cbfd, osec, &rectype, sizeof (rectype), bfd_offset);
> >>>> + switch (rectype)
> >>>> + {
> >>>> + case record_full_reg: /* reg */
> >>>> + {
> >>>> + /* Get register number to regnum. */
> >>>> + bfdcore_read (cbfd, osec, ®num, sizeof (regnum), bfd_offset);
> >>>> + regnum = netorder32 (regnum);
> >>>> +
> >>>> + record_full_entry rec;
> >>>> +
> >>>> + rec = record_full_reg_init (cache, regnum);
> >>>> +
> >>>> + /* Get val. */
> >>>> + bfdcore_read (cbfd, osec, record_full_get_loc (&rec),
> >>>> + rec.u.reg.len, bfd_offset);
> >>>> +
> >>>> + if (record_debug)
> >>>> + gdb_printf (gdb_stdlog,
> >>>> + " Reading register %d (1 "
> >>>> + "plus %lu plus %d bytes)\n",
> >>>> + rec.u.reg.num,
> >>>> + (unsigned long) sizeof (regnum),
> >>>> + rec.u.reg.len);
> >>>> +
> >>>> + record_full_arch_list_add (rec);
> >>>> + break;
> >>>> + }
> >>>> +
> >>>> + case record_full_mem: /* mem */
> >>>> + {
> >>>> + /* Get len. */
> >>>> + bfdcore_read (cbfd, osec, &len, sizeof (len), bfd_offset);
> >>>> + len = netorder32 (len);
> >>>> +
> >>>> + /* Get addr. */
> >>>> + bfdcore_read (cbfd, osec, &addr, sizeof (addr),
> >>>> + bfd_offset);
> >>>> + addr = netorder64 (addr);
> >>>> +
> >>>> + record_full_entry rec;
> >>>> + rec = record_full_mem_init (addr, len);
> >>>> +
> >>>> + /* Get val. */
> >>>> + bfdcore_read (cbfd, osec, record_full_get_loc (&rec), len,
> >>>> + bfd_offset);
> >>>> +
> >>>> + if (record_debug)
> >>>> + gdb_printf (gdb_stdlog,
> >>>> + " Reading memory %s (1 plus "
> >>>> + "%lu plus %lu plus %d bytes)\n",
> >>>> + paddress (get_current_arch (),
> >>>> + rec.u.mem.addr),
> >>>> + (unsigned long) sizeof (addr),
> >>>> + (unsigned long) sizeof (len),
> >>>> + len);
> >>>> +
> >>>> + record_full_arch_list_add (rec);
> >>>> + break;
> >>>> + }
> >>>> +
> >>>> + default:
> >>>> + error (_("Bad entry type %d, offset %d of %lu in core file %ps."),
> >>>> + rectype, *bfd_offset, bfd_section_size (osec),
> >>>> + styled_string (file_name_style.style (),
> >>>> + bfd_get_filename (cbfd)));
> >>>> + break;
> >>>> + }
> >>>> +}
> >>>> +
> >>>> /* Restore the execution log from core file CBFD. */
> >>>>
> >>>> static void
> >>>> record_full_restore (struct bfd &cbfd)
> >>>> {
> >>>> uint32_t magic;
> >>>> - struct record_full_entry *rec;
> >>>> asection *osec;
> >>>> uint32_t osec_size;
> >>>> int bfd_offset = 0;
> >>>>
> >>>> /* "record_full_restore" can only be called when record list is empty.
> */
> >>>> - gdb_assert (record_full_first.next == NULL);
> >>>> + gdb_assert (record_full_list.empty ());
> >>>>
> >>>> if (record_debug)
> >>>> gdb_printf (gdb_stdlog, "Restoring recording from core file.\n");
> >>>> @@ -2379,124 +2286,47 @@ record_full_restore (struct bfd &cbfd)
> >>>> "RECORD_FULL_FILE_MAGIC (0x%s)\n",
> >>>> phex_nz (netorder32 (magic), 4));
> >>>>
> >>>> - /* Restore the entries in recfd into record_full_arch_list_head and
> >>>> - record_full_arch_list_tail. */
> >>>> - record_full_arch_list_head = NULL;
> >>>> - record_full_arch_list_tail = NULL;
> >>>> record_full_insn_num = 0;
> >>>>
> >>>> try
> >>>> {
> >>>> - regcache *regcache = get_thread_regcache (inferior_thread ());
> >>>> -
> >>>> - while (1)
> >>>> + while (bfd_offset < osec_size)
> >>>> {
> >>>> - uint8_t rectype;
> >>>> - uint32_t regnum, len, signal, count;
> >>>> - uint64_t addr;
> >>>> -
> >>>> - /* We are finished when offset reaches osec_size. */
> >>>> - if (bfd_offset >= osec_size)
> >>>> - break;
> >>>> - bfdcore_read (&cbfd, osec, &rectype, sizeof (rectype), &bfd_offset);
> >>>> -
> >>>> - switch (rectype)
> >>>> + record_full_reset_incomplete ();
> >>>> + uint32_t eff_count = 0;
> >>>> + uint8_t sigval;
> >>>> + uint32_t insn_num;
> >>>> +
> >>>> + /* First read the generic information for an instruction. */
> >>>> + bfdcore_read (&cbfd, osec, &sigval, sizeof (uint8_t), &bfd_offset);
> >>>> + bfdcore_read (&cbfd, osec, &eff_count, sizeof (uint32_t),
> >>>> + &bfd_offset);
> >>>> + bfdcore_read (&cbfd, osec, &insn_num, sizeof (uint32_t),
> >>>> + &bfd_offset);
> >>>> +
> >>>> + record_full_incomplete_instruction.insn_num = netorder32
> >>>> (insn_num);
> >>>> + if (sigval != GDB_SIGNAL_0)
> >>>> + record_full_incomplete_instruction.sigval = (gdb_signal) sigval;
> >>>> +
> >>>> + eff_count = netorder32 (eff_count);
> >>>> +
> >>>> + /* This deals with all the side effects. */
> >>>> + while (eff_count > 0)
> >>>> {
> >>> There is one space too much indent.
> >>>
> >>>> - case record_full_reg: /* reg */
> >>>> - /* Get register number to regnum. */
> >>>> - bfdcore_read (&cbfd, osec, ®num, sizeof (regnum),
> >>>> - &bfd_offset);
> >>>> - regnum = netorder32 (regnum);
> >>>> -
> >>>> - rec = record_full_reg_alloc (regcache, regnum);
> >>>> -
> >>>> - /* Get val. */
> >>>> - bfdcore_read (&cbfd, osec, record_full_get_loc (rec),
> >>>> - rec->u.reg.len, &bfd_offset);
> >>>> -
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - " Reading register %d (1 "
> >>>> - "plus %lu plus %d bytes)\n",
> >>>> - rec->u.reg.num,
> >>>> - (unsigned long) sizeof (regnum),
> >>>> - rec->u.reg.len);
> >>>> - break;
> >>>> + eff_count--;
> >>>>
> >>>> - case record_full_mem: /* mem */
> >>>> - /* Get len. */
> >>>> - bfdcore_read (&cbfd, osec, &len, sizeof (len), &bfd_offset);
> >>>> - len = netorder32 (len);
> >>>> -
> >>>> - /* Get addr. */
> >>>> - bfdcore_read (&cbfd, osec, &addr, sizeof (addr), &bfd_offset);
> >>>> - addr = netorder64 (addr);
> >>>> -
> >>>> - rec = record_full_mem_alloc (addr, len);
> >>>> -
> >>>> - /* Get val. */
> >>>> - bfdcore_read (&cbfd, osec, record_full_get_loc (rec),
> >>>> - rec->u.mem.len, &bfd_offset);
> >>>> -
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - " Reading memory %s (1 plus "
> >>>> - "%lu plus %lu plus %d bytes)\n",
> >>>> - paddress (get_current_arch (),
> >>>> - rec->u.mem.addr),
> >>>> - (unsigned long) sizeof (addr),
> >>>> - (unsigned long) sizeof (len),
> >>>> - rec->u.mem.len);
> >>>> - break;
> >>>> -
> >>>> - case record_full_end: /* end */
> >>>> - rec = record_full_end_alloc ();
> >>>> - record_full_insn_num ++;
> >>>> -
> >>>> - /* Get signal value. */
> >>>> - bfdcore_read (&cbfd, osec, &signal, sizeof (signal),
> >>>> - &bfd_offset);
> >>>> - signal = netorder32 (signal);
> >>>> - rec->u.end.sigval = (enum gdb_signal) signal;
> >>>> -
> >>>> - /* Get insn count. */
> >>>> - bfdcore_read (&cbfd, osec, &count, sizeof (count), &bfd_offset);
> >>>> - count = netorder32 (count);
> >>>> - rec->u.end.insn_num = count;
> >>>> - record_full_insn_count = count + 1;
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - " Reading record_full_end (1 + "
> >>>> - "%lu + %lu bytes), offset == %s\n",
> >>>> - (unsigned long) sizeof (signal),
> >>>> - (unsigned long) sizeof (count),
> >>>> - paddress (get_current_arch (),
> >>>> - bfd_offset));
> >>>> - break;
> >>>> -
> >>>> - default:
> >>>> - error (_("Bad entry type in core file %ps."),
> >>>> - styled_string (file_name_style.style (),
> >>>> - bfd_get_filename (&cbfd)));
> >>>> - break;
> >>>> + record_full_read_entry_from_bfd (&cbfd, osec, &bfd_offset);
> >>>> }
> >>>>
> >>>> - /* Add rec to record arch list. */
> >>>> - record_full_arch_list_add (rec);
> >>>> + record_full_save_instruction ();
> >>>> }
> >>>> }
> >>>> catch (const gdb_exception &ex)
> >>>> {
> >>>> - record_full_list_release (record_full_arch_list_tail);
> >>>> + record_full_reset_incomplete ();
> >>>> throw;
> >>>> }
> >>>>
> >>>> - /* Add record_full_arch_list_head to the end of record list. */
> >>>> - record_full_first.next = record_full_arch_list_head;
> >>>> - record_full_arch_list_head->prev = &record_full_first;
> >>>> - record_full_arch_list_tail->next = NULL;
> >>>> - record_full_list = &record_full_first;
> >>>> -
> >>>> /* Update record_full_insn_max_num. */
> >>>> if (record_full_insn_num > record_full_insn_max_num)
> >>>> {
> >>>> @@ -2505,6 +2335,10 @@ record_full_restore (struct bfd &cbfd)
> >>>> record_full_insn_max_num);
> >>>> }
> >>>>
> >>>> + /* When loading a recording, we'll always start at the oldest possible
> >>>> + instruction, no matter where the original recording was stopped. */
> >>>> + record_full_next_insn = 0;
> >>>> +
> >>>> /* Succeeded. */
> >>>> gdb_printf (_("Restored records from core file %s.\n"),
> >>>> bfd_get_filename (&cbfd));
> >>>> @@ -2538,13 +2372,73 @@ cmd_record_full_restore (const char
> *args,
> >> int
> >>>> from_tty)
> >>>> record_full_open (nullptr, from_tty);
> >>>> }
> >>>>
> >>>> +static void
> >>>> +record_full_write_entry_to_bfd (record_full_entry &entry,
> >>>> + gdb_bfd_ref_ptr obfd,
> >>>> + asection *osec, int *bfd_offset,
> >>>> + struct gdbarch *gdbarch)
> >>>> +{
> >>>> + /* Save entry. */
> >>>> + uint8_t type;
> >>>> + uint32_t regnum, len;
> >>>> + uint64_t addr;
> >>>> +
> >>>> + type = entry.type;
> >>>> + bfdcore_write (obfd.get (), osec, &type, sizeof (type), bfd_offset);
> >>>> +
> >>>> + switch (entry.type)
> >>>> + {
> >>>> + case record_full_reg: /* reg */
> >>>> + if (record_debug)
> >>>> + gdb_printf (gdb_stdlog,
> >>>> + " Writing register %d (1 "
> >>>> + "plus %lu plus %d bytes)\n",
> >>>> + entry.u.reg.num,
> >>>> + (unsigned long) sizeof (regnum),
> >>>> + entry.u.reg.len);
> >>>> +
> >>>> + /* Write regnum. */
> >>>> + regnum = netorder32 (entry.u.reg.num);
> >>>> + bfdcore_write (obfd.get (), osec, ®num, sizeof (regnum),
> >> bfd_offset);
> >>>> +
> >>>> + /* Write regval. */
> >>>> + bfdcore_write (obfd.get (), osec, record_full_get_loc (&entry),
> >>>> + entry.u.reg.len, bfd_offset);
> >>>> + break;
> >>>> +
> >>>> + case record_full_mem: /* mem */
> >>>> + if (record_debug)
> >>>> + gdb_printf (gdb_stdlog,
> >>>> + " Writing memory %s (1 plus "
> >>>> + "%lu plus %lu plus %d bytes)\n",
> >>>> + paddress (gdbarch,
> >>>> + entry.u.mem.addr),
> >>>> + (unsigned long) sizeof (addr),
> >>>> + (unsigned long) sizeof (len),
> >>>> + entry.u.mem.len);
> >>>> +
> >>>> + /* Write memlen. */
> >>>> + len = netorder32 (entry.u.mem.len);
> >>>> + bfdcore_write (obfd.get (), osec, &len, sizeof (len), bfd_offset);
> >>>> +
> >>>> + /* Write memaddr. */
> >>>> + addr = netorder64 (entry.u.mem.addr);
> >>>> + bfdcore_write (obfd.get (), osec, &addr, sizeof (addr), bfd_offset);
> >>>> +
> >>>> + /* Write memval. */
> >>>> + bfdcore_write (obfd.get (), osec, record_full_get_loc (&entry),
> >>>> + entry.u.mem.len, bfd_offset);
> >>>> + break;
> >>>> + }
> >>>> +
> >>>> +}
> >>>> +
> >>>> /* Save the execution log to a file. We use a modified elf corefile
> >>>> format, with an extra section for our data. */
> >>>>
> >>>> void
> >>>> record_full_base_target::save_record (const char *recfilename)
> >>>> {
> >>>> - struct record_full_entry *cur_record_full_list;
> >>>> uint32_t magic;
> >>>> struct gdbarch *gdbarch;
> >>>> int save_size = 0;
> >>>> @@ -2562,9 +2456,6 @@ record_full_base_target::save_record (const
> >> char
> >>>> *recfilename)
> >>>> /* Arrange to remove the output file on failure. */
> >>>> gdb::unlinker unlink_file (recfilename);
> >>>>
> >>>> - /* Save the current record entry to "cur_record_full_list". */
> >>>> - cur_record_full_list = record_full_list;
> >>>> -
> >>>> /* Get the values of regcache and gdbarch. */
> >>>> regcache *regcache = get_thread_regcache (inferior_thread ());
> >>>> gdbarch = regcache->arch ();
> >>>> @@ -2574,34 +2465,27 @@ record_full_base_target::save_record
> (const
> >>>> char *recfilename)
> >>>> = record_full_gdb_operation_disable_set ();
> >>>>
> >>>> /* Reverse execute to the begin of record list. */
> >>>> - while (1)
> >>>> - {
> >>>> - /* Check for beginning and end of log. */
> >>>> - if (record_full_list == &record_full_first)
> >>>> - break;
> >>>> -
> >>>> - record_full_exec_insn (regcache, gdbarch, record_full_list);
> >>>> -
> >>>> - if (record_full_list->prev)
> >>>> - record_full_list = record_full_list->prev;
> >>>> - }
> >>>> + for (int i = record_full_next_insn - 1; i >= 0; i--)
> >>>> + record_full_exec_insn (regcache, gdbarch,
> >>>> + record_full_list[i]);
> >>>>
> >>>> /* Compute the size needed for the extra bfd section. */
> >>>> save_size = 4; /* magic cookie */
> >>>> - for (record_full_list = record_full_first.next; record_full_list;
> >>>> - record_full_list = record_full_list->next)
> >>>> - switch (record_full_list->type)
> >>>> - {
> >>>> - case record_full_end:
> >>>> - save_size += 1 + 4 + 4;
> >>>> - break;
> >>>> - case record_full_reg:
> >>>> - save_size += 1 + 4 + record_full_list->u.reg.len;
> >>>> - break;
> >>>> - case record_full_mem:
> >>>> - save_size += 1 + 4 + 8 + record_full_list->u.mem.len;
> >>>> - break;
> >>>> - }
> >>>> + for (int i = record_full_list.size () - 1; i >= 0; i--)
> >>>> + {
> >>>> + /* Number of effects of an instruction. */
> >>>> + save_size += sizeof (uint32_t) + sizeof (uint8_t) + sizeof (uint32_t);
> >>>> + for (auto &entry : record_full_list[i].effects)
> >>>> + switch (entry.type)
> >>>> + {
> >>>> + case record_full_reg:
> >>>> + save_size += 1 + 4 + entry.u.reg.len;
> >>>> + break;
> >>>> + case record_full_mem:
> >>>> + save_size += 1 + 4 + 8 + entry.u.mem.len;
> >>>> + break;
> >>>> + }
> >>>> + }
> >>>>
> >>>> /* Make the new bfd section. */
> >>>> osec = bfd_make_section_anyway_with_flags (obfd.get (), "precord",
> >>>> @@ -2630,108 +2514,32 @@ record_full_base_target::save_record
> >> (const
> >>>> char *recfilename)
> >>>>
> >>>> /* Save the entries to recfd and forward execute to the end of
> >>>> record list. */
> >>>> - record_full_list = &record_full_first;
> >>>> - while (1)
> >>>> + for (int i = 0; i < record_full_list.size (); i++)
> >>>> {
> >>>> - /* Save entry. */
> >>>> - if (record_full_list != &record_full_first)
> >>>> + uint32_t eff_count = (uint32_t) record_full_list[i].effects.size ();
> >>>> + uint32_t insn_num = record_full_list[i].insn_num;
> >>>> + uint8_t sigval = (record_full_list[i].sigval.has_value ())
> >>>> + ? record_full_list[i].sigval.value ()
> >>>> + : GDB_SIGNAL_0;
> >>>> +
> >>>> + /* Signal. */
> >>>> + bfdcore_write (obfd.get (), osec, &sigval, sizeof (sigval),
> &bfd_offset);
> >>>> + eff_count = netorder32 (eff_count);
> >>>> + /* Number of effects. */
> >>>> + bfdcore_write (obfd.get (), osec, &eff_count, sizeof (eff_count),
> >>>> + &bfd_offset);
> >>>> + /* Instruction number. */
> >>>> + bfdcore_write (obfd.get (), osec, &insn_num, sizeof (uint32_t),
> >>>> + &bfd_offset);
> >>>> +
> >>>> + for (auto &entry : record_full_list[i].effects)
> >>>> {
> >>>> - uint8_t type;
> >>>> - uint32_t regnum, len, signal, count;
> >>>> - uint64_t addr;
> >>>> -
> >>>> - type = record_full_list->type;
> >>>> - bfdcore_write (obfd.get (), osec, &type, sizeof (type), &bfd_offset);
> >>>> -
> >>>> - switch (record_full_list->type)
> >>>> - {
> >>>> - case record_full_reg: /* reg */
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - " Writing register %d (1 "
> >>>> - "plus %lu plus %d bytes)\n",
> >>>> - record_full_list->u.reg.num,
> >>>> - (unsigned long) sizeof (regnum),
> >>>> - record_full_list->u.reg.len);
> >>>> -
> >>>> - /* Write regnum. */
> >>>> - regnum = netorder32 (record_full_list->u.reg.num);
> >>>> - bfdcore_write (obfd.get (), osec, ®num,
> >>>> - sizeof (regnum), &bfd_offset);
> >>>> -
> >>>> - /* Write regval. */
> >>>> - bfdcore_write (obfd.get (), osec,
> >>>> - record_full_get_loc (record_full_list),
> >>>> - record_full_list->u.reg.len, &bfd_offset);
> >>>> - break;
> >>>> -
> >>>> - case record_full_mem: /* mem */
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - " Writing memory %s (1 plus "
> >>>> - "%lu plus %lu plus %d bytes)\n",
> >>>> - paddress (gdbarch,
> >>>> - record_full_list->u.mem.addr),
> >>>> - (unsigned long) sizeof (addr),
> >>>> - (unsigned long) sizeof (len),
> >>>> - record_full_list->u.mem.len);
> >>>> -
> >>>> - /* Write memlen. */
> >>>> - len = netorder32 (record_full_list->u.mem.len);
> >>>> - bfdcore_write (obfd.get (), osec, &len, sizeof (len),
> >>>> - &bfd_offset);
> >>>> -
> >>>> - /* Write memaddr. */
> >>>> - addr = netorder64 (record_full_list->u.mem.addr);
> >>>> - bfdcore_write (obfd.get (), osec, &addr,
> >>>> - sizeof (addr), &bfd_offset);
> >>>> -
> >>>> - /* Write memval. */
> >>>> - bfdcore_write (obfd.get (), osec,
> >>>> - record_full_get_loc (record_full_list),
> >>>> - record_full_list->u.mem.len, &bfd_offset);
> >>>> - break;
> >>>> -
> >>>> - case record_full_end:
> >>>> - if (record_debug)
> >>>> - gdb_printf (gdb_stdlog,
> >>>> - " Writing record_full_end (1 + "
> >>>> - "%lu + %lu bytes)\n",
> >>>> - (unsigned long) sizeof (signal),
> >>>> - (unsigned long) sizeof (count));
> >>>> - /* Write signal value. */
> >>>> - signal = netorder32 (record_full_list->u.end.sigval);
> >>>> - bfdcore_write (obfd.get (), osec, &signal,
> >>>> - sizeof (signal), &bfd_offset);
> >>>> -
> >>>> - /* Write insn count. */
> >>>> - count = netorder32 (record_full_list->u.end.insn_num);
> >>>> - bfdcore_write (obfd.get (), osec, &count,
> >>>> - sizeof (count), &bfd_offset);
> >>>> - break;
> >>>> - }
> >>>> + record_full_write_entry_to_bfd (entry, obfd, osec, &bfd_offset,
> >>>> + gdbarch);
> >>>> }
> >>>> -
> >>>> /* Execute entry. */
> >>>> - record_full_exec_insn (regcache, gdbarch, record_full_list);
> >>>> -
> >>>> - if (record_full_list->next)
> >>>> - record_full_list = record_full_list->next;
> >>>> - else
> >>>> - break;
> >>>> - }
> >>>> -
> >>>> - /* Reverse execute to cur_record_full_list. */
> >>>> - while (1)
> >>>> - {
> >>>> - /* Check for beginning and end of log. */
> >>>> - if (record_full_list == cur_record_full_list)
> >>>> - break;
> >>>> -
> >>>> - record_full_exec_insn (regcache, gdbarch, record_full_list);
> >>>> -
> >>>> - if (record_full_list->prev)
> >>>> - record_full_list = record_full_list->prev;
> >>>> + if (i < record_full_next_insn)
> >>>> + record_full_exec_insn (regcache, gdbarch, record_full_list[i]);
> >>>> }
> >>>>
> >>>> unlink_file.keep ();
> >>>> @@ -2746,7 +2554,7 @@ record_full_base_target::save_record (const
> >> char
> >>>> *recfilename)
> >>>> correspondingly. */
> >>>>
> >>>> static void
> >>>> -record_full_goto_insn (struct record_full_entry *entry,
> >>>> +record_full_goto_insn (size_t target_insn,
> >>>> enum exec_direction_kind dir)
> >>> You could explain the new parameter in the function description.
> >>>
> >>>> {
> >>>> scoped_restore restore_operation_disable
> >>>> @@ -2757,17 +2565,12 @@ record_full_goto_insn (struct
> >> record_full_entry
> >>>> *entry,
> >>>> /* Assume everything is valid: we will hit the entry,
> >>>> and we will not hit the end of the recording. */
> >>>>
> >>>> - if (dir == EXEC_FORWARD)
> >>>> - record_full_list = record_full_list->next;
> >>>> -
> >>>> - do
> >>>> - {
> >>>> - record_full_exec_insn (regcache, gdbarch, record_full_list);
> >>>> - if (dir == EXEC_REVERSE)
> >>>> - record_full_list = record_full_list->prev;
> >>>> - else
> >>>> - record_full_list = record_full_list->next;
> >>>> - } while (record_full_list != entry);
> >>>> + if (dir == EXEC_REVERSE)
> >>>> + for (int i = record_full_next_insn; i > target_insn; i--)
> >>>> + record_full_exec_insn (regcache, gdbarch, record_full_list[i - 1]);
> >>>> + else
> >>>> + for (int i = record_full_next_insn; i < target_insn; i++)
> >>>> + record_full_exec_insn (regcache, gdbarch, record_full_list[i]);
> >>>> }
> >>>>
> >>>> /* Alias for "target record-full". */
> >>>> @@ -2798,61 +2601,36 @@ set_record_full_insn_max_num (const
> char
> >>>> *args, int from_tty,
> >>>> static void
> >>>> maintenance_print_record_instruction (const char *args, int from_tty)
> >>>> {
> >>>> - struct record_full_entry *to_print = record_full_list;
> >>>> + if (record_full_list.empty ())
> >>>> + error (_("Not enough recorded history"));
> >>>>
> >>>> + int offset = record_full_next_insn - 1;
> >>>> + /* Reduce the offset by 1 if the record_full_next_insn is after the end
> >>>> + so that we show the last recorded instruction instead of crashing.
> */
> >>>> + if (offset == record_full_list.size ())
> >>>> + offset--;
> >>>> if (args != nullptr)
> >>>> {
> >>>> - int offset = value_as_long (parse_and_eval (args));
> >>>> - if (offset > 0)
> >>>> - {
> >>>> - /* Move forward OFFSET instructions. We know we found the
> >>>> - end of an instruction when to_print->type is record_full_end. */
> >>>> - while (to_print->next != nullptr && offset > 0)
> >>>> - {
> >>>> - to_print = to_print->next;
> >>>> - if (to_print->type == record_full_end)
> >>>> - offset--;
> >>>> - }
> >>>> - if (offset != 0)
> >>>> - error (_("Not enough recorded history"));
> >>>> - }
> >>>> - else
> >>>> - {
> >>>> - while (to_print->prev != nullptr && offset < 0)
> >>>> - {
> >>>> - to_print = to_print->prev;
> >>>> - if (to_print->type == record_full_end)
> >>>> - offset++;
> >>>> - }
> >>>> - if (offset != 0)
> >>>> - error (_("Not enough recorded history"));
> >>>> - }
> >>>> + offset += value_as_long (parse_and_eval (args));
> >>>> + if (offset >= record_full_list.size () || offset < 0)
> >>>> + error (_("Not enough recorded history"));
> >>>> }
> >>>> - gdb_assert (to_print != nullptr);
> >>>> + auto to_print = record_full_list.begin () + offset;
> >>>>
> >>>> gdbarch *arch = current_inferior ()->arch ();
> >>>>
> >>>> - /* Go back to the start of the instruction. */
> >>>> - while (to_print->prev != nullptr && to_print->prev->type !=
> >> record_full_end)
> >>>> - to_print = to_print->prev;
> >>>> -
> >>>> - /* if we're in the first record, there are no actual instructions
> >>>> - recorded. Warn the user and leave. */
> >>>> - if (to_print == &record_full_first)
> >>>> - error (_("Not enough recorded history"));
> >>>> -
> >>>> - while (to_print->type != record_full_end)
> >>>> + for (auto entry : to_print->effects)
> >>>> {
> >>>> - switch (to_print->type)
> >>>> + switch (entry.type)
> >>>> {
> >>>> case record_full_reg:
> >>>> {
> >>>> - type *regtype = gdbarch_register_type (arch, to_print-
> >>>>> u.reg.num);
> >>>> + type *regtype = gdbarch_register_type (arch, entry.u.reg.num);
> >>>> value *val
> >>>> = value_from_contents (regtype,
> >>>> - record_full_get_loc (to_print));
> >>>> + record_full_get_loc (&entry));
> >>>> gdb_printf ("Register %s changed: ",
> >>>> - gdbarch_register_name (arch, to_print-
> >>>>> u.reg.num));
> >>>> + gdbarch_register_name (arch, entry.u.reg.num));
> >>>> struct value_print_options opts;
> >>>> get_user_print_options (&opts);
> >>>> opts.raw = true;
> >>>> @@ -2862,17 +2640,16 @@ maintenance_print_record_instruction
> >> (const
> >>>> char *args, int from_tty)
> >>>> }
> >>>> case record_full_mem:
> >>>> {
> >>>> - gdb_byte *b = record_full_get_loc (to_print);
> >>>> + gdb_byte *b = record_full_get_loc (&entry);
> >>>> gdb_printf ("%d bytes of memory at address %s changed from:",
> >>>> - to_print->u.mem.len,
> >>>> - print_core_address (arch, to_print->u.mem.addr));
> >>>> - for (int i = 0; i < to_print->u.mem.len; i++)
> >>>> + entry.u.mem.len,
> >>>> + print_core_address (arch, entry.u.mem.addr));
> >>>> + for (int i = 0; i < entry.u.mem.len; i++)
> >>>> gdb_printf (" %02x", b[i]);
> >>>> gdb_printf ("\n");
> >>>> break;
> >>>> }
> >>>> }
> >>>> - to_print = to_print->next;
> >>>> }
> >>>> }
> >>>>
> >>>> @@ -2880,11 +2657,6 @@ INIT_GDB_FILE (record_full)
> >>>> {
> >>>> struct cmd_list_element *c;
> >>>>
> >>>> - /* Init record_full_first. */
> >>>> - record_full_first.prev = NULL;
> >>>> - record_full_first.next = NULL;
> >>>> - record_full_first.type = record_full_end;
> >>>> -
> >>>> add_target (record_full_target_info, record_full_open);
> >>>> add_deprecated_target_alias (record_full_target_info, "record");
> >>>> add_target (record_full_core_target_info, record_full_open);
> >>>> diff --git a/gdb/record-full.h b/gdb/record-full.h
> >>>> index 51effe74560..c327d879a8a 100644
> >>>> --- a/gdb/record-full.h
> >>>> +++ b/gdb/record-full.h
> >>>> @@ -41,7 +41,6 @@ enum record_result
> >>>>
> >>>> extern int record_full_arch_list_add_reg (struct regcache *regcache,
> int
> >>>> num);
> >>>> extern int record_full_arch_list_add_mem (CORE_ADDR addr, int len);
> >>>> -extern int record_full_arch_list_add_end (void);
> >>>>
> >>>> /* Returns true if the process record target is open. */
> >>>> extern int record_full_is_used (void);
> >>>> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> >>>> index 6b5c9c02d46..f5b1f66844c 100644
> >>>> --- a/gdb/riscv-tdep.c
> >>>> +++ b/gdb/riscv-tdep.c
> >>>> @@ -5466,8 +5466,5 @@ riscv_process_record (struct gdbarch
> *gdbarch,
> >>>> struct regcache *regcache,
> >>>> if (res != RECORD_SUCCESS)
> >>>> return res;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> -
> >>>> return 0;
> >>>> }
> >>>> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> >>>> index e135df94a40..f964889a49f 100644
> >>>> --- a/gdb/rs6000-tdep.c
> >>>> +++ b/gdb/rs6000-tdep.c
> >>>> @@ -7138,8 +7138,6 @@ ppc_process_prefix_instruction (int
> insn_prefix,
> >> int
> >>>> insn_suffix,
> >>>> if (record_full_arch_list_add_reg (regcache, PPC_PC_REGNUM))
> >>>> return -1;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> return 0;
> >>>> }
> >>>>
> >>>> @@ -7447,8 +7445,6 @@ ppc_process_record (struct gdbarch
> *gdbarch,
> >>>> struct regcache *regcache,
> >>>>
> >>>> if (record_full_arch_list_add_reg (regcache, PPC_PC_REGNUM))
> >>>> return -1;
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> return 0;
> >>>> }
> >>>>
> >>>> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
> >>>> index 3cd6b359d13..6bea30f08dc 100644
> >>>> --- a/gdb/s390-linux-tdep.c
> >>>> +++ b/gdb/s390-linux-tdep.c
> >>>> @@ -910,9 +910,6 @@ s390_linux_record_signal (struct gdbarch
> >> *gdbarch,
> >>>> struct regcache *regcache,
> >>>> if (record_full_arch_list_add_mem (sp, sizeof_rt_sigframe))
> >>>> return -1;
> >>>>
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> -
> >>>> return 0;
> >>>> }
> >>>>
> >>>> diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
> >>>> index f9d7bdd04e4..36ce659ea15 100644
> >>>> --- a/gdb/s390-tdep.c
> >>>> +++ b/gdb/s390-tdep.c
> >>>> @@ -7019,8 +7019,6 @@ s390_process_record (struct gdbarch
> *gdbarch,
> >>>> struct regcache *regcache,
> >>>>
> >>>> if (record_full_arch_list_add_reg (regcache, S390_PSWA_REGNUM))
> >>>> return -1;
> >>>> - if (record_full_arch_list_add_end ())
> >>>> - return -1;
> >>>> return 0;
> >>>> }
> >>>>
> >>>> --
> >>>> 2.53.0
> >>>>
> >>> Kind Regards,
> >>> Christina
> >>>
> >>> Intel Deutschland GmbH
> >>> Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
> >>> Tel: +49 89 991 430, www.intel.de
> >>> Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong
> Sorrell
> >>> Chairperson of the Supervisory Board: Nicole Lau
> >>> Registered Seat: Munich
> >>> Commercial Register: Amtsgericht Muenchen HRB 186928
> >>>
> > 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
> >
>
> --
> Cheers,
> Guinevere Larsen
> It/she
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
next prev parent reply other threads:[~2026-05-13 7:55 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-23 15:25 [PATCH v2 0/6] refactor the internals of record-full Guinevere Larsen
2026-04-23 15:25 ` [PATCH v2 1/6] gdb/record: Refactor record history Guinevere Larsen
2026-04-28 3:28 ` Thiago Jung Bauermann
2026-04-28 19:54 ` Guinevere Larsen
2026-05-06 21:16 ` Schimpe, Christina
2026-05-07 17:38 ` Guinevere Larsen
2026-05-11 15:23 ` Schimpe, Christina
2026-05-12 18:50 ` Guinevere Larsen
2026-05-13 7:54 ` Schimpe, Christina [this message]
2026-05-14 15:20 ` Guinevere Larsen
2026-04-23 15:26 ` [PATCH v2 2/6] gdb/record: remove record_full_insn_num Guinevere Larsen
2026-04-23 15:26 ` [PATCH v2 3/6] gdb/record: c++ify internal structures of record-full.c Guinevere Larsen
2026-04-28 3:29 ` Thiago Jung Bauermann
2026-04-23 15:26 ` [PATCH v2 4/6] gdb/record: make record_full_history more c++-like Guinevere Larsen
2026-04-28 3:29 ` Thiago Jung Bauermann
2026-04-23 15:26 ` [PATCH v2 5/6] gdb/record: extract the PC to record_full_instruction Guinevere Larsen
2026-04-28 3:30 ` Thiago Jung Bauermann
2026-04-23 15:26 ` [PATCH v2 6/6] gdb/record: Define new version of the record-save section Guinevere Larsen
2026-04-23 15:45 ` Eli Zaretskii
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=SN7PR11MB763827776662E108B20CF99FF9062@SN7PR11MB7638.namprd11.prod.outlook.com \
--to=christina.schimpe@intel.com \
--cc=gdb-patches@sourceware.org \
--cc=guinevere@redhat.com \
/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