Re: [PATCH v3 5/7] gdb/record: extract the PC to record_full_instruction

[Guinevere Larsen <guinevere@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2499 bytes --]

On 5/29/26 9:40 AM, Schimpe, Christina wrote:
>> -----Original Message-----
>> From: Guinevere Larsen<guinevere@redhat.com>
>> Sent: Freitag, 15. Mai 2026 18:37
>> To:gdb-patches@sourceware.org
>> Cc: Guinevere Larsen<guinevere@redhat.com>
>> Subject: [PATCH v3 5/7] gdb/record: extract the PC to record_full_instruction
>>
>> 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, to make them methods of
>> record_full_reg_entry and record_full_mem_entry, and also creates similar
>> methods for record_full_entry and record_full_instruction.
>> These could be turned into constructors in a future step of
>> c++ification, but it felt like too much change for a single commit.
>> ---
>>   gdb/record-full.c | 386 ++++++++++++++++++++++++++++------------------
>>   1 file changed, 239 insertions(+), 147 deletions(-)
>>
>> diff --git a/gdb/record-full.c b/gdb/record-full.c index
>> 8cabd6e9438..17696e07c0d 100644
>> --- a/gdb/record-full.c
>> +++ b/gdb/record-full.c
>> @@ -143,6 +143,14 @@ struct record_full_mem_entry
>>
>>     DISABLE_COPY_AND_ASSIGN (record_full_mem_entry);
>>
>> +  /* Create a mem_entry from a bfd file, when restoring a recording.
>> +*/
>> +  static record_full_mem_entry from_bfd (bfd*cbfd, asection* osec,
>> +					 int *bfd_offset);
>> +
>> +  /* Save this mem entry to a bfd file.  */
>> +  void to_bfd (gdb_bfd_ref_ptr obfd, asection *osec, int *bfd_offset,
>> +	       gdbarch *gdbarch);
>> +
> I believe only in one of those to_bfd functions the gdbarch parameter is used.
> Did you intentionally keep it the function which don't have use the parameter in to_bfd ?
Fixed. I didn't keep it intentionally.
>
> And it might be a good idea to describe the parameters a bit, too. 😊

I would love to describe the parameters, but to be fair, I only know 
what the self-explanatory ones do lol. No idea what the section is used 
for, and the bfd_ref_ptr is the ref-counted pointer to the BFD file...

-- 
Cheers,
Guinevere Larsen
It/she

[-- Attachment #2: Type: text/html, Size: 3660 bytes --]