Re: [PATCH 6/6] gdb/record: Define new version of the record-save section

[Guinevere Larsen <guinevere@redhat.com>]

On 4/18/26 2:51 AM, Thiago Jung Bauermann wrote:
> Guinevere Larsen <guinevere@redhat.com> writes:
>
>> With the changes to the internal representation of the history, we can
>> no longer support the previous record save format. This commit makes it
>> official, documenting the new format and changing the magic number.
>> ---
>>   gdb/NEWS          |  3 +++
>>   gdb/record-full.c | 35 +++++++++++++++++++++++++++++++----
>>   2 files changed, 34 insertions(+), 4 deletions(-)
> Assuming that the order switch between number of effect entries and
> signal I mention below is fixed (or I misunderstood it):
>
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>
>> diff --git a/gdb/record-full.c b/gdb/record-full.c
>> index 297b5b76ae7..f7cfe5ca1e9 100644
>> --- a/gdb/record-full.c
>> +++ b/gdb/record-full.c
>> @@ -76,7 +76,8 @@
>>     ( (record_full_next_insn != record_full_list.size ()) \
>>        || ::execution_direction == EXEC_REVERSE)
>>   
>> -#define RECORD_FULL_FILE_MAGIC	netorder32(0x20091016)
>> +#define RECORD_FULL_FILE_MAGIC_OLD	netorder32(0x20091016)
>> +#define RECORD_FULL_FILE_MAGIC	netorder32(0x20260415)
> Out of curiosity: how was the magic number chosen? It doesn't seem to
> based on ASCII. Is it worth adding a comment to explain it?

the previous number was 2009 10 16, ie, 16th of october 2009. So I just 
went with a similar idea, and made the new number 2026 04 15 (15th of 
april, 2026).

I thought that was pretty self explanatory, but I can add a comment 
explaining it if you'd like. In the end it is just an arbitrary number 
that shouldn't overlap with previous ones, so I don't find it that 
important that this is followed in the future honestly.

>
>>   /* These are the core structs of the process record functionality.
>>   
>> @@ -2161,6 +2162,27 @@ record_full_core_target::has_execution (inferior *inf)
>>          8 bytes: memory address (network byte order).
>>          n bytes: memory value (n == memory length).
>>   
>> +   Version 3 (all numbers are in network order).
>> +     4 bytes: Magic number (0x20260415).
>> +       NOTE: be sure to change whenever this file format changes!
>> +
>> +    Records:
>> +      record_full_instruction:
>> +	4 bytes: number of reg and mem entries for this instruction.
>> +	1 byte: signal.
> Looking at the code in record_full_restore, signal comes before the
> number of effect entries.
Ah, nice catch. I knew I wanted to swap the order of things at some 
point and I think I forgot to update the comment. Fixed
>
>> +	4 bytes: instruction count.
> Maybe it's just me or I'm a bit tired, but "instruction sequence number"
> sounds clearer to me than "instruction count".
Make sense, will change the wording
>
>> +	4 bytes: PC register ID.
> Considering that we know that it will be the PC in this "register slot",
> it's slightly wasteful to include the register ID here. But I suppose
> it's not a problem in practice.
Yeah those 4 bytes aren't really required, but they allow me to reuse 
the write_reg_to_bfd and read_reg_from_bfd functions. Figured the more 
compact code was worth the extra space required.
>
>> +	N bytes: PC address of instruction (N == size of PC).
>> +	Effects:
>> +	  record_full_reg:
>> +	    1 byte: record_type (record_full_reg, see enum record_full_type).
>> +	    4 bytes: Register ID.
>> +	    n bytes: register value (n == actual register size).
>> +	  record_full_mem:
>> +	    1 byte: record_type (record_full_mem, see enum record_full_type).
>> +	    4 bytes: memory length.
>> +	    8 bytes: memory address.
>> +	    n bytes: memory value (n = memory length).
>>   */
>>   
>>   /* bfdcore_read -- read bytes from a core file section.  */


-- 
Cheers,
Guinevere Larsen
It/she